diff --git a/.fusa-reqs.json b/.fusa-reqs.json index 533cf5a..8c6a5b3 100644 --- a/.fusa-reqs.json +++ b/.fusa-reqs.json @@ -611,56 +611,56 @@ }, { "id": "REQ-SAFETY-001", - "title": "DataID embedded in header bytes 0-1", - "description": "Protect shall write Config.DataID as a little-endian uint16 into bytes 0-1 of the protected payload.", + "title": "DataID bound into CRC input, never transmitted", + "description": "Protect shall include Config.DataID as CRC input; DataID is never written verbatim onto the wire (the 3-byte header has no room for it).", "asil": "ASIL-B", "rationale": "DataID is included in the CRC computation and identifies the logical data element.", "tags": ["safety", "header"] }, { "id": "REQ-SAFETY-002", - "title": "SourceID embedded in header bytes 2-3", - "description": "Protect shall write Config.SourceID as a little-endian uint16 into bytes 2-3 of the protected payload.", + "title": "SourceID bound into CRC input, never transmitted", + "description": "Protect shall include Config.SourceID as CRC input; SourceID is never written verbatim onto the wire (the 3-byte header has no room for it).", "asil": "ASIL-B", "rationale": "SourceID is included in the CRC computation and identifies the sender node.", "tags": ["safety", "header"] }, { "id": "REQ-SAFETY-003", - "title": "SequenceCounter starts at 0 and increments", - "description": "The Protector's SequenceCounter shall start at 0 and increment by 1 with each Protect call.", + "title": "SequenceCounter starts at 0, increments, wraps mod 256", + "description": "The Protector's SequenceCounter shall start at 0 and increment by 1 with each Protect call, wrapping mod 256 (the on-wire counter is a single byte).", "asil": "ASIL-B", "rationale": "Monotonically increasing counter enables detection of missed or replayed frames.", "tags": ["safety", "counter"] }, { "id": "REQ-SAFETY-004", - "title": "SequenceCounter embedded in header bytes 4-7", - "description": "Protect shall write the current SequenceCounter as a little-endian uint32 into bytes 4-7.", + "title": "SequenceCounter in header byte 0", + "description": "Protect shall write the current SequenceCounter as a uint8 into byte 0.", "asil": "ASIL-B", "rationale": "SG-04: counter must be present in the wire format for gap detection.", "tags": ["safety", "header", "counter"] }, { "id": "REQ-SAFETY-005", - "title": "CRC computed over header and payload", - "description": "Protect shall compute CRC-16/CCITT-FALSE (poly=0x1021, init=0xFFFF) over the header with CRC slot zeroed, concatenated with the payload.", + "title": "CRC computed over DataID/SourceID/counter/payload", + "description": "Protect shall compute CRC-16/CCITT-FALSE (poly=0x1021, init=0xFFFF) over DataID, SourceID, and SequenceCounter (none of which are transmitted verbatim), concatenated with the payload.", "asil": "ASIL-B", "rationale": "SG-04: CRC must cover all bytes including DataID and SourceID.", "tags": ["safety", "crc"] }, { "id": "REQ-SAFETY-006", - "title": "CRC embedded in header bytes 8-9", - "description": "Protect shall write the computed CRC as a little-endian uint16 into bytes 8-9.", + "title": "CRC embedded in header bytes 1-2", + "description": "Protect shall write the computed CRC as a little-endian uint16 into bytes 1-2.", "asil": "ASIL-B", "rationale": "SG-04: CRC must be transmitted so the receiver can verify it.", "tags": ["safety", "header", "crc"] }, { "id": "REQ-SAFETY-007", - "title": "Unwrap returns ErrHeaderTooShort for payload < 10 bytes", - "description": "Unwrap shall return E2EError{Kind: ErrHeaderTooShort} when the input is shorter than 10 bytes.", + "title": "Unwrap returns ErrHeaderTooShort for payload < 3 bytes", + "description": "Unwrap shall return E2EError{Kind: ErrHeaderTooShort} when the input is shorter than 3 bytes.", "asil": "ASIL-B", "rationale": "SG-04: truncated header cannot be validated; must be rejected.", "tags": ["safety", "receiver", "error"] @@ -668,7 +668,7 @@ { "id": "REQ-SAFETY-008", "title": "Unwrap detects byte corruption via CRC", - "description": "Unwrap shall return E2EError{Kind: ErrCRCMismatch} when the recomputed CRC does not match bytes 8-9.", + "description": "Unwrap shall return E2EError{Kind: ErrCRCMismatch} when the recomputed CRC does not match bytes 1-2.", "asil": "ASIL-B", "rationale": "SG-04: any single-byte corruption in header or payload must be detected.", "tags": ["safety", "receiver", "crc"] @@ -684,7 +684,7 @@ { "id": "REQ-SAFETY-010", "title": "Unwrap returns original payload on success", - "description": "Unwrap shall strip the 10-byte header and return a copy of the original payload when all checks pass.", + "description": "Unwrap shall strip the 3-byte header and return a copy of the original payload when all checks pass.", "asil": "ASIL-B", "rationale": "Application code must receive the original payload unchanged.", "tags": ["safety", "receiver"] @@ -699,8 +699,8 @@ }, { "id": "REQ-SAFETY-012", - "title": "Protect output length equals 10 + len(payload)", - "description": "The byte slice returned by Protect shall have length exactly headerSize (10) + len(payload).", + "title": "Protect output length equals kHeaderSize (3) + len(payload)", + "description": "The byte slice returned by Protect shall have length exactly headerSize (3) + len(payload); this fits within kLINMaxDataLen (8) for payloads up to 5 bytes.", "asil": "ASIL-B", "rationale": "SG-04: receivers must be able to compute the expected output length to detect truncation.", "tags": ["safety", "length"] diff --git a/SAFETY_MANUAL.md b/SAFETY_MANUAL.md index b1159fb..552d856 100644 --- a/SAFETY_MANUAL.md +++ b/SAFETY_MANUAL.md @@ -101,16 +101,31 @@ process_frame(hw_frame); ### 4.2 E2E Protection for Safety-Critical Payloads (SG-05) -Use `lin::safety::Protector` and `Receiver` for all ASIL-B data paths: +Use `lin::safety::Protector` and `Receiver` for all ASIL-B data paths. + +`Protector::protect()` prepends a fixed `lin::safety::kHeaderSize` (3-byte) +header to the payload it is given. Because a LIN frame's data field is at +most `lin::kLINMaxDataLen` (8) bytes, **the raw payload passed to `protect()` +must be at most `kLINMaxDataLen - kHeaderSize` (5) bytes** for the protected +output to fit in a single frame — `bus->publish()` rejects (returns a +non-empty `std::error_code`) any payload that doesn't fit, so a +budget violation is caught at the call site rather than silently corrupting +or truncating the frame: ```cpp // Sender side (e.g., sensor ECU) lin::safety::Config cfg{.data_id = 0x0042, .source_id = 0x0001}; lin::safety::Protector protector{cfg}; -auto raw_payload = read_sensor_value(); -auto safe_payload = protector.protect(raw_payload); -bus->publish(FRAME_ID_SENSOR, safe_payload); +auto raw_payload = read_sensor_value(); // at most kLINMaxDataLen - kHeaderSize (5) bytes +auto safe_payload = protector.protect(raw_payload); +if (auto err = bus->publish(FRAME_ID_SENSOR, safe_payload)) { + // raw_payload was too large for a single LIN frame once protected — + // programming error, not a runtime/bus condition. Fix the payload + // budget rather than retrying. + log_fault(err); + return; +} // Receiver side (e.g., actuator ECU) lin::safety::Receiver receiver{cfg}; @@ -126,6 +141,9 @@ try { **NEVER** ignore `E2EError` exceptions — they indicate data corruption or replay. +**NEVER** ignore `bus->publish()`'s returned error for an E2E-protected +payload — an oversized payload is rejected rather than transmitted. + ### 4.3 Error Handling for ErrNoResponse (SG-01) A `lin::ErrNoResponse` from `send_header()` or `master::Node::run()` means a diff --git a/include/lin/safety/e2e.hpp b/include/lin/safety/e2e.hpp index 8155400..01935e1 100644 --- a/include/lin/safety/e2e.hpp +++ b/include/lin/safety/e2e.hpp @@ -5,12 +5,24 @@ // safety/e2e.hpp — End-to-end data protection for LIN payloads. // -// Wire format (little-endian, 10 bytes followed by original payload): -// Bytes 0–1 DataID (uint16) -// Bytes 2–3 SourceID (uint16) -// Bytes 4–7 SequenceCounter (uint32, monotonically increasing) -// Bytes 8–9 CRC-16/CCITT-FALSE over bytes 0–7 (CRC slot zeroed) plus payload -// Bytes 10+ Original payload +// Wire format (little-endian, 3-byte header followed by original payload): +// Byte 0 SequenceCounter (uint8, monotonically increasing, wraps mod 256) +// Bytes 1–2 CRC-16/CCITT-FALSE (little-endian) over DataID (2B) + SourceID (2B) +// + SequenceCounter (1B) + payload +// Bytes 3+ Original payload +// +// DataID and SourceID are never transmitted on the wire — both ends of an E2E +// link must be configured with an identical Config (this is the normal +// deployment model for automotive E2E profiles operating over short PDUs). +// They are still bound into the CRC, so a receiver configured with the wrong +// DataID/SourceID will observe a CRC mismatch rather than silently accepting +// a message meant for a different logical data element. +// +// kHeaderSize (3) is deliberately small relative to kLINMaxDataLen (8): a LIN +// frame can carry at most kLINMaxDataLen - kHeaderSize = 5 bytes of protected +// payload in a single frame. An earlier 10-byte header made every protected +// payload — including an empty one — exceed kLINMaxDataLen, so no protected +// message could ever be published on an actual LIN bus (see cpp-LIN#17). // // fusa:req REQ-SAFETY-001 REQ-SAFETY-002 REQ-SAFETY-003 REQ-SAFETY-004 // fusa:req REQ-SAFETY-005 REQ-SAFETY-006 REQ-SAFETY-007 REQ-SAFETY-008 @@ -30,7 +42,7 @@ namespace lin::safety { // fusa:req REQ-SAFETY-001 REQ-SAFETY-002 -inline constexpr std::size_t kHeaderSize = 10; +inline constexpr std::size_t kHeaderSize = 3; // fusa:req REQ-SAFETY-001 REQ-SAFETY-002 struct Config { diff --git a/requirements/requirements.json b/requirements/requirements.json index 03121d6..7e77aee 100644 --- a/requirements/requirements.json +++ b/requirements/requirements.json @@ -84,18 +84,18 @@ {"id": "REQ-SLAVE-006", "title": "subscribe delegates to bus", "asil": "ASIL-B", "tags": ["slave"]}, {"id": "REQ-SLAVE-007", "title": "registered_ids returns empty slice when none", "asil": "ASIL-B", "tags": ["slave", "boundary"]}, {"id": "REQ-SLAVE-008", "title": "set_response overwrites previous registration", "asil": "ASIL-B", "tags": ["slave", "update"]}, - {"id": "REQ-SAFETY-001", "title": "DataID embedded in header bytes 0-1", "asil": "ASIL-B", "tags": ["safety", "header"]}, - {"id": "REQ-SAFETY-002", "title": "SourceID embedded in header bytes 2-3", "asil": "ASIL-B", "tags": ["safety", "header"]}, - {"id": "REQ-SAFETY-003", "title": "SequenceCounter starts at 0 and increments", "asil": "ASIL-B", "tags": ["safety", "counter"]}, - {"id": "REQ-SAFETY-004", "title": "SequenceCounter in header bytes 4-7", "asil": "ASIL-B", "tags": ["safety", "header", "counter"]}, - {"id": "REQ-SAFETY-005", "title": "CRC computed over header and payload", "asil": "ASIL-B", "tags": ["safety", "crc"]}, - {"id": "REQ-SAFETY-006", "title": "CRC embedded in header bytes 8-9", "asil": "ASIL-B", "tags": ["safety", "header", "crc"]}, - {"id": "REQ-SAFETY-007", "title": "unwrap: ErrHeaderTooShort for <10 bytes", "asil": "ASIL-B", "tags": ["safety", "receiver", "error"]}, + {"id": "REQ-SAFETY-001", "title": "DataID bound into CRC input, never transmitted", "asil": "ASIL-B", "tags": ["safety", "header"]}, + {"id": "REQ-SAFETY-002", "title": "SourceID bound into CRC input, never transmitted", "asil": "ASIL-B", "tags": ["safety", "header"]}, + {"id": "REQ-SAFETY-003", "title": "SequenceCounter starts at 0, increments, wraps mod 256", "asil": "ASIL-B", "tags": ["safety", "counter"]}, + {"id": "REQ-SAFETY-004", "title": "SequenceCounter in header byte 0", "asil": "ASIL-B", "tags": ["safety", "header", "counter"]}, + {"id": "REQ-SAFETY-005", "title": "CRC computed over DataID/SourceID/counter/payload", "asil": "ASIL-B", "tags": ["safety", "crc"]}, + {"id": "REQ-SAFETY-006", "title": "CRC embedded in header bytes 1-2", "asil": "ASIL-B", "tags": ["safety", "header", "crc"]}, + {"id": "REQ-SAFETY-007", "title": "unwrap: ErrHeaderTooShort for <3 bytes", "asil": "ASIL-B", "tags": ["safety", "receiver", "error"]}, {"id": "REQ-SAFETY-008", "title": "unwrap detects byte corruption via CRC", "asil": "ASIL-B", "tags": ["safety", "receiver", "crc"]}, {"id": "REQ-SAFETY-009", "title": "unwrap detects sequence gap", "asil": "ASIL-B", "tags": ["safety", "receiver", "counter"]}, {"id": "REQ-SAFETY-010", "title": "unwrap returns original payload on success", "asil": "ASIL-B", "tags": ["safety", "receiver"]}, {"id": "REQ-SAFETY-011", "title": "protect/unwrap round-trip preserves payload", "asil": "ASIL-B", "tags": ["safety", "roundtrip"]}, - {"id": "REQ-SAFETY-012", "title": "protect output length == 10 + len(payload)", "asil": "ASIL-B", "tags": ["safety", "length"]}, + {"id": "REQ-SAFETY-012", "title": "protect output length == kHeaderSize (3) + len(payload); fits kLINMaxDataLen for payload <= 5 bytes", "asil": "ASIL-B", "tags": ["safety", "length"]}, {"id": "REQ-SAFETY-013", "title": "unwrap accepts first message with any counter", "asil": "ASIL-B", "tags": ["safety", "receiver", "counter"]}, {"id": "REQ-SAFETY-014", "title": "protect is safe for concurrent calls", "asil": "ASIL-B", "tags": ["safety", "concurrency"]}, {"id": "REQ-SAFETY-015", "title": "unwrap returns an independent payload copy", "asil": "ASIL-B", "tags": ["safety", "copy"]}, diff --git a/src/safety/e2e.cpp b/src/safety/e2e.cpp index 8b1b1e1..4d721ca 100644 --- a/src/safety/e2e.cpp +++ b/src/safety/e2e.cpp @@ -29,47 +29,40 @@ uint16_t crc16(const uint8_t* data, std::size_t len) noexcept { return crc; } -// Builds the 10-byte header with CRC filled in. +// Builds the 3-byte header ([0]=SequenceCounter, [1:2]=CRC) with CRC filled +// in. DataID/SourceID are never written to the header — they exist only as +// CRC input, so a mismatched Config on either side surfaces as a CRC error +// rather than being silently accepted or spending header bytes on identity +// fields that a LIN frame's 8-byte budget cannot afford (see cpp-LIN#17). static std::vector build_header(uint16_t data_id, uint16_t source_id, - uint32_t seq, + uint8_t seq, const std::vector& payload) { std::vector hdr(kHeaderSize, 0); - // bytes 0-1: DataID little-endian - hdr[0] = static_cast(data_id); - hdr[1] = static_cast(data_id >> 8); - // bytes 2-3: SourceID little-endian - hdr[2] = static_cast(source_id); - hdr[3] = static_cast(source_id >> 8); - // bytes 4-7: SequenceCounter little-endian - hdr[4] = static_cast(seq); - hdr[5] = static_cast(seq >> 8); - hdr[6] = static_cast(seq >> 16); - hdr[7] = static_cast(seq >> 24); - // hdr[8:9] = 0 during CRC computation - - // CRC over header bytes 0–7 (CRC slot zeroed) and payload + hdr[0] = seq; + // hdr[1:2] = 0 during CRC computation + constexpr uint16_t poly = 0x1021; uint16_t crc_val = 0xFFFF; - for (int i = 0; i < 8; ++i) { - crc_val ^= static_cast(hdr[i]) << 8; - for (int j = 0; j < 8; ++j) { - crc_val = (crc_val & 0x8000) - ? static_cast((crc_val << 1) ^ poly) - : static_cast(crc_val << 1); - } - } - for (auto b : payload) { + auto crc_byte = [&crc_val](uint8_t b) { crc_val ^= static_cast(b) << 8; for (int j = 0; j < 8; ++j) { crc_val = (crc_val & 0x8000) ? static_cast((crc_val << 1) ^ poly) : static_cast(crc_val << 1); } - } - - // bytes 8-9: CRC little-endian - hdr[8] = static_cast(crc_val); - hdr[9] = static_cast(crc_val >> 8); + }; + + // CRC input: DataID (2B) + SourceID (2B) + SequenceCounter (1B) + payload. + crc_byte(static_cast(data_id)); + crc_byte(static_cast(data_id >> 8)); + crc_byte(static_cast(source_id)); + crc_byte(static_cast(source_id >> 8)); + crc_byte(seq); + for (auto b : payload) crc_byte(b); + + // bytes 1-2: CRC little-endian + hdr[1] = static_cast(crc_val); + hdr[2] = static_cast(crc_val >> 8); return hdr; } @@ -78,7 +71,9 @@ static std::vector build_header(uint16_t data_id, uint16_t source_id, // fusa:req REQ-SAFETY-001 REQ-SAFETY-002 REQ-SAFETY-003 REQ-SAFETY-004 // fusa:req REQ-SAFETY-005 REQ-SAFETY-006 REQ-SAFETY-012 REQ-SAFETY-014 std::vector Protector::protect(const std::vector& payload) { - uint32_t seq = seq_++; + // On-wire SequenceCounter is a single byte (wraps mod 256) — see + // e2e.hpp's wire-format note for why the header must stay this small. + uint8_t seq = static_cast(seq_++); auto hdr = build_header(cfg_.data_id, cfg_.source_id, seq, payload); std::vector out; out.reserve(kHeaderSize + payload.size()); @@ -97,25 +92,23 @@ std::vector Receiver::unwrap(const std::vector& data) { "need " + std::to_string(kHeaderSize) + " bytes, got " + std::to_string(data.size())); - uint32_t seq = static_cast(data[4]) - | static_cast(data[5]) << 8 - | static_cast(data[6]) << 16 - | static_cast(data[7]) << 24; + uint8_t seq = data[0]; - uint16_t received_crc = static_cast(data[8]) - | static_cast(data[9]) << 8; + uint16_t received_crc = static_cast(data[1]) + | static_cast(data[2]) << 8; std::vector payload(data.begin() + kHeaderSize, data.end()); auto expected_hdr = build_header(cfg_.data_id, cfg_.source_id, seq, payload); - uint16_t expected_crc = static_cast(expected_hdr[8]) - | static_cast(expected_hdr[9]) << 8; + uint16_t expected_crc = static_cast(expected_hdr[1]) + | static_cast(expected_hdr[2]) << 8; if (received_crc != expected_crc) throw E2EError(E2EErrorKind::CRCMismatch, seq, "CRC mismatch"); std::lock_guard lk(mu_); - if (!first_ && seq != last_seq_ + 1) { - uint32_t expected_seq = last_seq_ + 1; + // Sequence comparison wraps mod 256 to match the on-wire counter width. + if (!first_ && seq != static_cast(last_seq_ + 1)) { + uint8_t expected_seq = static_cast(last_seq_ + 1); last_seq_ = seq; throw E2EError(E2EErrorKind::SequenceGap, seq, "expected " + std::to_string(expected_seq) + diff --git a/src/virtual/bus.cpp b/src/virtual/bus.cpp index 2c7626a..4ac9871 100644 --- a/src/virtual/bus.cpp +++ b/src/virtual/bus.cpp @@ -35,6 +35,14 @@ std::error_code Bus::do_publish(uint8_t id, std::vector data, ChecksumT error_count_.fetch_add(1); return lin::make_error_code(lin::Errc::invalid_frame); } + // Reject oversized payloads instead of silently accepting a response + // that can never be sent as a valid LIN frame (§5.3; cpp-LIN#17). Empty + // data is intentionally exempt — it is the documented signal to clear a + // previously registered response, not a frame to be transmitted. + if (data.size() > kLINMaxDataLen) { + error_count_.fetch_add(1); + return lin::make_error_code(lin::Errc::invalid_frame); + } std::unique_lock lk(mu_); if (closed_) { diff --git a/tests/test_safety.cpp b/tests/test_safety.cpp index f727f67..2ba52c0 100644 --- a/tests/test_safety.cpp +++ b/tests/test_safety.cpp @@ -8,6 +8,7 @@ // fusa:test REQ-SAFETY-009 REQ-SAFETY-010 REQ-SAFETY-011 REQ-SAFETY-012 // fusa:test REQ-SAFETY-013 REQ-SAFETY-014 REQ-SAFETY-015 REQ-SEOOC-001 +#include #include #include #include @@ -21,8 +22,19 @@ TEST_CASE("Config has DataID and SourceID fields", "[safety][REQ-SAFETY-001][REQ CHECK(cfg.source_id == 0x12); } -TEST_CASE("E2E header is exactly 10 bytes", "[safety][REQ-SAFETY-004]") { - CHECK(kHeaderSize == 10u); +TEST_CASE("E2E header is exactly 3 bytes", "[safety][REQ-SAFETY-004]") { + CHECK(kHeaderSize == 3u); +} + +// Regression test for cpp-LIN#17: the documented ASIL-B publish pattern +// (Protector::protect() output fed straight into Bus::publish()) must be +// able to fit within a single LIN frame for at least some non-trivial +// payload size. A header that alone exceeds kLINMaxDataLen makes the +// documented safety pattern unimplementable on a real LIN bus. +TEST_CASE("kHeaderSize leaves room for a non-empty payload within kLINMaxDataLen", + "[safety][REQ-SAFETY-012][regression]") { + CHECK(kHeaderSize < lin::kLINMaxDataLen); + CHECK(lin::kLINMaxDataLen - kHeaderSize >= 1u); } TEST_CASE("E2EErrorKind values are distinct", "[safety]") { @@ -35,21 +47,28 @@ TEST_CASE("Protector starts with seq=0", "[safety][REQ-SAFETY-003]") { Config cfg{0x0001, 0x01}; Protector p{cfg}; auto frame = p.protect({0x00}); - uint32_t seq = static_cast(frame[4]) - | static_cast(frame[5]) << 8 - | static_cast(frame[6]) << 16 - | static_cast(frame[7]) << 24; - CHECK(seq == 0); + CHECK(frame[0] == 0); } TEST_CASE("protect output length == kHeaderSize + len(payload)", "[safety][REQ-SAFETY-012]") { Config cfg{0x0001, 0x0010}; Protector p{cfg}; - std::vector payload = {1, 2, 3, 4}; + std::vector payload = {1, 2, 3}; auto out = p.protect(payload); CHECK(out.size() == kHeaderSize + payload.size()); } +// Fits entirely within one LIN frame — the pattern documented in +// SAFETY_MANUAL.md §4.2 must actually be publishable. +TEST_CASE("protect output for a small payload fits within kLINMaxDataLen", + "[safety][REQ-SAFETY-012][regression]") { + Config cfg{0x0001, 0x0010}; + Protector p{cfg}; + std::vector payload = {1, 2}; // 2 bytes: 3-byte header + 2 == 5 <= 8 + auto out = p.protect(payload); + CHECK(out.size() <= lin::kLINMaxDataLen); +} + TEST_CASE("protect and unwrap round-trip", "[safety][REQ-SAFETY-011]") { Config cfg{0x0001, 0x0010}; Protector protector{cfg}; @@ -70,38 +89,50 @@ TEST_CASE("protect increments sequence counter", "[safety][REQ-SAFETY-003][REQ-S auto p0 = protector.protect({0xAA}); auto p1 = protector.protect({0xBB}); - uint32_t s0 = static_cast(p0[4]) - | static_cast(p0[5]) << 8 - | static_cast(p0[6]) << 16 - | static_cast(p0[7]) << 24; - uint32_t s1 = static_cast(p1[4]) - | static_cast(p1[5]) << 8 - | static_cast(p1[6]) << 16 - | static_cast(p1[7]) << 24; - CHECK(s1 == s0 + 1); + CHECK(p1[0] == static_cast(p0[0] + 1)); } -TEST_CASE("DataID embedded in bytes 0-1 little-endian", "[safety][REQ-SAFETY-001]") { - Config cfg{0xABCD, 0x0000}; - Protector p{cfg}; - auto out = p.protect({0x00}); - uint16_t data_id = static_cast(out[0]) | static_cast(out[1]) << 8; - CHECK(data_id == 0xABCD); +TEST_CASE("sequence counter wraps mod 256", "[safety][REQ-SAFETY-003][regression]") { + Config cfg{0x0001, 0x0010}; + Protector protector{cfg}; + + std::vector first_out; + for (int i = 0; i < 256; ++i) { + auto out = protector.protect({0x00}); + if (i == 0) first_out = out; + } + auto wrapped = protector.protect({0x00}); + // After 257 total protect() calls (indices 0..256), the 257th call's + // counter byte must equal the 1st call's counter byte (wrap mod 256). + CHECK(wrapped[0] == first_out[0]); } -TEST_CASE("SourceID embedded in bytes 2-3 little-endian", "[safety][REQ-SAFETY-002]") { - Config cfg{0x0000, 0x1234}; +TEST_CASE("DataID and SourceID are never written to the wire", "[safety][REQ-SAFETY-001][REQ-SAFETY-002]") { + Config cfg{0xABCD, 0x1234}; Protector p{cfg}; auto out = p.protect({0x00}); - uint16_t source_id = static_cast(out[2]) | static_cast(out[3]) << 8; - CHECK(source_id == 0x1234); + // The header is only kHeaderSize (3) bytes: SequenceCounter + CRC. + // Neither DataID nor SourceID appears verbatim anywhere in the header. + CHECK(out.size() == kHeaderSize + 1); +} + +TEST_CASE("DataID/SourceID mismatch is detected as a CRC error", "[safety][REQ-SAFETY-001][REQ-SAFETY-002][regression]") { + Config sender_cfg{0xABCD, 0x1234}; + Config receiver_cfg{0xABCD, 0x9999}; // different SourceID + Protector protector{sender_cfg}; + Receiver receiver{receiver_cfg}; + + auto protected_payload = protector.protect({0x01}); + REQUIRE_THROWS_AS(receiver.unwrap(protected_payload), E2EError); + try { receiver.unwrap(protected_payload); } + catch (const E2EError& e) { CHECK(e.kind() == E2EErrorKind::CRCMismatch); } } TEST_CASE("unwrap: header too short throws E2EError", "[safety][REQ-SAFETY-007]") { Config cfg{}; Receiver receiver{cfg}; - REQUIRE_THROWS_AS(receiver.unwrap({0x01, 0x02, 0x03}), E2EError); - try { receiver.unwrap({1, 2, 3}); } + REQUIRE_THROWS_AS(receiver.unwrap({0x01, 0x02}), E2EError); + try { receiver.unwrap({0x01, 0x02}); } catch (const E2EError& e) { CHECK(e.kind() == E2EErrorKind::HeaderTooShort); } } @@ -111,7 +142,7 @@ TEST_CASE("unwrap: CRC mismatch throws E2EError", "[safety][REQ-SAFETY-008]") { Receiver receiver{cfg}; auto protected_payload = protector.protect({0x01, 0x02}); - protected_payload[8] ^= 0xFF; // corrupt CRC + protected_payload[1] ^= 0xFF; // corrupt CRC REQUIRE_THROWS_AS(receiver.unwrap(protected_payload), E2EError); try { receiver.unwrap(protected_payload); } catch (const E2EError& e) { CHECK(e.kind() == E2EErrorKind::CRCMismatch); } diff --git a/tests/test_slave.cpp b/tests/test_slave.cpp index c6833d7..94a2f53 100644 --- a/tests/test_slave.cpp +++ b/tests/test_slave.cpp @@ -66,6 +66,21 @@ TEST_CASE("set_response rejects ID > 0x3F", "[slave][REQ-SLAVE-004]") { bus->close(); } +// Regression test for cpp-LIN#17: set_response() only validated the frame +// ID, never data.size(), so it delegated an oversized payload straight +// through to the bus. Covered here at the slave::Node layer since the +// safety-manual-documented pattern calls set_response() directly with a +// safety::Protector::protect() output. +TEST_CASE("set_response rejects data longer than kLINMaxDataLen", "[slave][REQ-SLAVE-004][regression]") { + auto bus = Bus::create(); + Node node(bus); + std::vector oversized(kLINMaxDataLen + 1, 0xAA); + auto err = node.set_response(0x10, oversized); + CHECK(err); + CHECK(node.registered_ids().empty()); + bus->close(); +} + TEST_CASE("registered_ids reflects current state", "[slave][REQ-SLAVE-005]") { auto bus = Bus::create(); Node node(bus); diff --git a/tests/test_virtual.cpp b/tests/test_virtual.cpp index 97ec4d8..1decad6 100644 --- a/tests/test_virtual.cpp +++ b/tests/test_virtual.cpp @@ -105,6 +105,21 @@ TEST_CASE("publish rejects ID > 0x3F", "[virtual][REQ-VIRT-004]") { bus->close(); } +// Regression test for cpp-LIN#17: no publish path validated data.size() +// against kLINMaxDataLen, so an oversized payload (e.g. the output of +// safety::Protector::protect() with a large payload) was silently accepted +// and stored as a "valid" response despite being unsendable on a real bus. +TEST_CASE("publish rejects data longer than kLINMaxDataLen", "[virtual][REQ-VIRT-004][regression]") { + auto bus = Bus::create(); + std::vector oversized(kLINMaxDataLen + 1, 0xAA); + auto err = bus->publish(0x10, oversized); + CHECK(err); + // Confirm it was not stored: send_header must report no response. + auto [f, herr] = bus->send_header(0x10); + CHECK(herr); + bus->close(); +} + TEST_CASE("publish(id, {}) removes registration", "[virtual][REQ-VIRT-003]") { auto bus = Bus::create(); REQUIRE_FALSE(bus->publish(0x10, {0xAA}));