Skip to content

fix: shrink E2E header so protected payloads fit a LIN frame; validate publish length #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SoundMatt wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: shrink E2E header so protected payloads fit a LIN frame; validate publish length #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions .fusa-reqs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,64 +611,64 @@
},
{
"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"]
},
{
"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"]
Expand All @@ -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"]
Expand All @@ -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"]
Expand Down
26 changes: 22 additions & 4 deletions SAFETY_MANUAL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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
Expand Down
26 changes: 19 additions & 7 deletions include/lin/safety/e2e.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions requirements/requirements.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]},
Expand Down
75 changes: 34 additions & 41 deletions src/safety/e2e.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t> build_header(uint16_t data_id, uint16_t source_id,
uint32_t seq,
uint8_t seq,
const std::vector<uint8_t>& payload) {
std::vector<uint8_t> hdr(kHeaderSize, 0);
// bytes 0-1: DataID little-endian
hdr[0] = static_cast<uint8_t>(data_id);
hdr[1] = static_cast<uint8_t>(data_id >> 8);
// bytes 2-3: SourceID little-endian
hdr[2] = static_cast<uint8_t>(source_id);
hdr[3] = static_cast<uint8_t>(source_id >> 8);
// bytes 4-7: SequenceCounter little-endian
hdr[4] = static_cast<uint8_t>(seq);
hdr[5] = static_cast<uint8_t>(seq >> 8);
hdr[6] = static_cast<uint8_t>(seq >> 16);
hdr[7] = static_cast<uint8_t>(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<uint16_t>(hdr[i]) << 8;
for (int j = 0; j < 8; ++j) {
crc_val = (crc_val & 0x8000)
? static_cast<uint16_t>((crc_val << 1) ^ poly)
: static_cast<uint16_t>(crc_val << 1);
}
}
for (auto b : payload) {
auto crc_byte = [&crc_val](uint8_t b) {
crc_val ^= static_cast<uint16_t>(b) << 8;
for (int j = 0; j < 8; ++j) {
crc_val = (crc_val & 0x8000)
? static_cast<uint16_t>((crc_val << 1) ^ poly)
: static_cast<uint16_t>(crc_val << 1);
}
}

// bytes 8-9: CRC little-endian
hdr[8] = static_cast<uint8_t>(crc_val);
hdr[9] = static_cast<uint8_t>(crc_val >> 8);
};

// CRC input: DataID (2B) + SourceID (2B) + SequenceCounter (1B) + payload.
crc_byte(static_cast<uint8_t>(data_id));
crc_byte(static_cast<uint8_t>(data_id >> 8));
crc_byte(static_cast<uint8_t>(source_id));
crc_byte(static_cast<uint8_t>(source_id >> 8));
crc_byte(seq);
for (auto b : payload) crc_byte(b);

// bytes 1-2: CRC little-endian
hdr[1] = static_cast<uint8_t>(crc_val);
hdr[2] = static_cast<uint8_t>(crc_val >> 8);
return hdr;
}

Expand All @@ -78,7 +71,9 @@ static std::vector<uint8_t> 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<uint8_t> Protector::protect(const std::vector<uint8_t>& 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<uint8_t>(seq_++);
auto hdr = build_header(cfg_.data_id, cfg_.source_id, seq, payload);
std::vector<uint8_t> out;
out.reserve(kHeaderSize + payload.size());
Expand All @@ -97,25 +92,23 @@ std::vector<uint8_t> Receiver::unwrap(const std::vector<uint8_t>& data) {
"need " + std::to_string(kHeaderSize) +
" bytes, got " + std::to_string(data.size()));

uint32_t seq = static_cast<uint32_t>(data[4])
| static_cast<uint32_t>(data[5]) << 8
| static_cast<uint32_t>(data[6]) << 16
| static_cast<uint32_t>(data[7]) << 24;
uint8_t seq = data[0];

uint16_t received_crc = static_cast<uint16_t>(data[8])
| static_cast<uint16_t>(data[9]) << 8;
uint16_t received_crc = static_cast<uint16_t>(data[1])
| static_cast<uint16_t>(data[2]) << 8;

std::vector<uint8_t> 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<uint16_t>(expected_hdr[8])
| static_cast<uint16_t>(expected_hdr[9]) << 8;
uint16_t expected_crc = static_cast<uint16_t>(expected_hdr[1])
| static_cast<uint16_t>(expected_hdr[2]) << 8;

if (received_crc != expected_crc)
throw E2EError(E2EErrorKind::CRCMismatch, seq, "CRC mismatch");

std::lock_guard<std::mutex> 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<uint8_t>(last_seq_ + 1)) {
uint8_t expected_seq = static_cast<uint8_t>(last_seq_ + 1);
last_seq_ = seq;
throw E2EError(E2EErrorKind::SequenceGap, seq,
"expected " + std::to_string(expected_seq) +
Expand Down
8 changes: 8 additions & 0 deletions src/virtual/bus.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ std::error_code Bus::do_publish(uint8_t id, std::vector<uint8_t> 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<std::shared_mutex> lk(mu_);
if (closed_) {
Expand Down
Loading
Loading