Opp: enforce semantic message header validation in apply_consensus#508
Opp: enforce semantic message header validation in apply_consensus#508heifner wants to merge 8 commits into
Conversation
opp.proto now specifies the MessageHeader normatively: every checksum/id is keccak256 over the field-complete canonical encoding of its sub-message, message_id is the header checksum with its first 8 bytes replaced by the big-endian sequence number (previous message's + 1; a stream starts at 1 with an empty previous_message_id), all OPP wire timestamps are milliseconds since the Unix epoch, endpoints stays reserved-default (routing derives from the proven delivery chain), and data_size must equal the data length with receivers sizing by the data itself. Remove encoding_flags and the EncodingFlags/Endianness/HashAlgorithm/LengthEncoding types: no implementation reads them, parse conventions cannot be self-described (they must be known before the field is readable), and a self-declared hash algorithm is a downgrade knob. Per-chain hash selection, if ever needed, belongs on the sysio.chains registry row. Slot 4 is reserved, and the Envelope's previously removed slots gain real reserved statements so reuse fails at protoc. sysio.msgch buildenv derives the semantic header on every (re)build inside the trim loop, chains message ids per outpost through a new last_message_id column on the one-deep outenvelopes row, and emits real millisecond timestamps (WIRE's half-second blocks make sub-second resolution meaningful; seconds-unit table columns are unchanged). The canonical codec drops the removed field and gains shared derivation helpers for the emitter, the tests, and the upcoming inbound validator. Golden vectors are regenerated with spec-derived headers -- vector B chains from A at both the envelope and message level -- and the identical values are to be pinned in the wire-solana and wire-ethereum vector suites, which re-confirm them against the Rust and Solidity codecs. The outbound chain tests additionally assert the full header derivation, sequence continuity, and the stored stream tip.
Every message in a consensus-winning inbound envelope must now recompute per the opp.proto MessageHeader spec before anything dispatches: each attestation's data_size must equal its data length, payload_size and payload_checksum must recompute over the payload's canonical bytes, header_checksum over the field-complete header with message_id/header_checksum blanked, and message_id must be that checksum with its first 8 bytes carrying previous_message_id's sequence number + 1. A mismatch drops the envelope before any dispatch, consensus record, or cleanup -- deliver itself never throws, and delivery rows stay intact for audit. Checks are self-contained per message: stream ordering is already enforced by the envelope-level chain, so no inbound message tip is tracked. Zero-message envelopes (the EVM outpost's empty epoch acks) are trivially valid. The host-side canonical-encoding oracle moves from sysio.msgch_chain_tests.cpp into a shared contracts/tests/opp_envelope_oracle.hpp, and every inbound test fixture (msgch chain, dispatch, dispute) now derives spec headers through it. A new forgery test proves each field independently: forged payload_checksum, header_checksum, payload_size, sequence number, and data_size are all dropped on correctly-chained envelopes -- attributable to the header check, not the chain check -- with no attestation rows, no consensus record, and no tip movement, and the stream resumes on the next well-formed delivery.
A wire message id is either empty (stream genesis) or exactly 32 bytes; message_sequence previously read any id shorter than 8 bytes as genesis and any longer one as a sequence source, so a verifier deriving the expected message_id through it would accept headers whose previous_message_id carries a non-canonical length. It now returns std::optional and yields nullopt for every length other than 0 or 32. buildenv hard-checks the sequence of its own stored tip -- a non-canonical length there is corrupted own-state and fails loudly, unlike the inbound never-throw path.
…-depot-header-validation # Conflicts: # contracts/sysio.msgch/sysio.msgch.wasm
semantic_headers_ok now rejects a previous_message_id that is neither empty nor exactly 32 bytes before deriving the expected message_id, using the strict optional-returning message_sequence: a truncated or padded value can no longer alias stream genesis or a valid sequence source even when the header checksum and message id were honestly re-derived over it. The shared test oracle mirrors the strict form, and the forgery suite gains a colluding-emitter case proving the drop comes from the length rule alone.
| /// | ||
| /// Every check is SELF-CONTAINED (no cross-envelope state): stream ordering is already enforced | ||
| /// by the envelope-level chain in `apply_consensus`, so a per-outpost inbound message tip is | ||
| /// deliberately not tracked here. A zero-message envelope is trivially valid -- the EVM outpost |
There was a problem hiding this comment.
[P1] Enforce the message-stream link to prevent replay
This validates each message only against its self-declared previous_message_id; it never requires that ID to equal the last accepted message for this outpost or the preceding message in the envelope. A correctly envelope-chained successor can therefore replay an earlier valid Message unchanged and it is accepted and dispatched. I confirmed this with a contract test: the replay created another attestation row and advanced outpcons. For OPERATOR_ACTION(DEPOSIT_REQUEST), depositinle has no message-ID deduplication and credits the balance again, so one escrow deposit can be credited multiple times. Envelope chaining orders envelopes but does not enforce the separate message chain. Persist the inbound message tip per outpost, require the first message to continue it, chain subsequent messages to one another, and update the tip only after acceptance.
There was a problem hiding this comment.
Fixed in 8347d1b. The depot now persists a per-outpost inbound message tip (message_tip on the outpcons row) and semantic_headers_ok requires every accepted message to continue it: the first message's previous_message_id must equal the stored tip, subsequent messages in an envelope chain to one another, and the tip advances only on acceptance (unmoved by zero-message acks). As with the envelope chain, the first message ever accepted for an outpost bootstraps regardless. New test inbound_message_replay_dropped reproduces your exact scenario — an old value-bearing Message replayed verbatim inside a correctly envelope-chained successor is now dropped before dispatch, with no new attestation row and the tip unmoved.
semantic_headers_ok validated each message only against its self-declared previous_message_id, never against the last accepted message for the outpost. A malicious batch-operator quorum could therefore carry an earlier valid Message verbatim inside a fresh, correctly envelope-chained successor: the envelope chain accepts it (previous_envelope_hash continues the tip), the message header self-validates, and the attestations re-dispatch. For OPERATOR_ACTION(DEPOSIT_REQUEST), depositinle has no message-id dedup, so one escrow deposit is credited twice. Persist a per-outpost inbound message tip (message_tip on the outpcons row) and require every accepted message to continue it: the first message's previous_message_id must equal the stored tip, subsequent messages chain to one another, and the tip advances only on acceptance (unmoved by zero-message acks). As with the envelope chain, the first message ever accepted for an outpost bootstraps regardless. This binds the message stream that envelope chaining alone leaves unbound. Adds inbound_message_replay_dropped reproducing the replay; threads message ids through the consecutive-acceptance tests; corrects the seq forgery now that a chained base shifts the expected sequence.
heifner
changed the base branch from
fix/sec-102-envelope-header-semantics
to
master
| checksum256 new_message_tip{}; | ||
| if (!semantic_headers_ok(envelope, inbound_message_tip, new_message_tip, chain_code, | ||
| epoch_index)) { | ||
| return; |
There was a problem hiding this comment.
[P1] Do not resolve a dispute when its winner is rejected
This soft return is correct on the normal delivery path, but resolvedisp also calls this void apply_consensus. chalg::chkdispute marks the dispute RESOLVED before invoking resolvedisp, then unpauses the epoch afterward. If the voted winner fails this new semantic validation, this return therefore reports success without writing outpcons. The epoch is then permanently stranded: chkcons waits for the current-epoch consensus row, while evalcons returns whenever a dispute row exists, including a RESOLVED one. A contract-level reproduction ends with the dispute RESOLVED and the epoch unpaused but no outpcons; another evalcons is a no-op. Please make resolvedisp assert that the selected winner was recorded (or otherwise propagate the validation failure), so the enclosing chkdispute transaction rolls back and leaves the dispute OPEN/paused.
There was a problem hiding this comment.
Fixed in a260f48. apply_consensus now returns bool (nodiscard) — true on acceptance or an idempotent re-fire, false on an envelope-chain / semantic-header drop. evalcons (void)-discards it (soft drop preserved — never throw on the deliver path), while resolvedisp check()s it, so a rejected voted winner asserts and rolls the whole chkdispute crank back: the dispute stays OPEN/paused and recoverable rather than resolve-and-stranded, the same safe-stall contract as the existing check(found, ...) above it. New test chkdispute_rejects_invalid_winner_stays_open reproduces your scenario (voted winner with a forged payload_checksum → chkdispute reverts, dispute stays OPEN, epoch paused, no outpcons row); chkdispute_fast_path_resolves remains the valid-winner control.
…ing the epoch apply_consensus soft-returned on an envelope-chain or semantic-header drop, which is correct on the evalcons path (the epoch waits for a valid delivery; never throw where a revert would stall the delivering operator). But resolvedisp also calls it, and chkdispute marks the dispute RESOLVED and unpauses the epoch in the same transaction that inline-calls resolvedisp. A dropped winner there reported success without writing outpcons, permanently stranding the epoch: chkcons waits for the consensus row while evalcons no-ops on the lingering RESOLVED dispute. apply_consensus now returns bool (nodiscard) — true on acceptance or an idempotent re-fire, false on a drop. evalcons discards it (soft drop preserved); resolvedisp check()s it so a rejected winner asserts and rolls the whole chkdispute crank back, leaving the dispute OPEN/paused and recoverable, the same safe-stall contract as the existing check(found) guard. Adds chkdispute_rejects_invalid_winner_stays_open (forged payload_checksum winner → chkdispute reverts, dispute stays OPEN, epoch paused, no outpcons); chkdispute_fast_path_resolves stays the valid-winner control.
| // result is intentionally discarded and the epoch simply stays un-consensused (no outpcons row) | ||
| // until a valid delivery arrives or the split opens a dispute -- never throw here, a throw | ||
| // reverts the delivering operator's tx and stalls consensus chain-wide. | ||
| (void)apply_consensus(get_self(), chain_code, epoch_index, |
There was a problem hiding this comment.
[P1] Make a rejected consensus winner recoverable
Discarding false here does not actually leave a path for a later valid delivery. If all operators (or a post-boundary majority) relay the same envelope with an invalid semantic header, that checksum keeps satisfying the consensus test, but apply_consensus never writes the current outpcons row. Those operators cannot submit a correction because deliver rejects a second row from the same operator, and any remaining minority cannot displace the existing majority; maybe_open_dispute is also never reached because consensus_reached remains true. chkcons then waits forever. The forgery test masks this by directly authorizing epoch::advance rather than exercising the production chkcons gate. Please quarantine/dispute/clear the rejected winner or otherwise allow replacement so this soft-drop path can recover without privileged manual state changes.
There was a problem hiding this comment.
Fixed in f141560 by validating the semantic header + inbound chain at ingress rather than only at consensus. deliver now runs the same inbound_envelope_valid check (envelope-chain + semantic_headers_ok) and reverts on failure — factored into a helper shared with apply_consensus. So a forged/malformed/stale envelope is never recorded, can't reach the consensus tally, and can't leave a phantom consensus_reached with no outpcons row; a reverted tx records nothing, so the operator can immediately re-deliver a corrected envelope. The check runs after the duplicate check, so a same-operator re-delivery still reports "already delivered". apply_consensus keeps the check as defense in depth (soft-drop on the evalcons path, check() on resolvedisp).
New forged_delivery_does_not_strand_chkcons exercises the production chkcons gate (not a direct epoch::advance): a forged delivery reverts, a valid delivery reaches consensus, and chkcons proceeds to attempt advance — proving no stall. The forgery/replay/cross-stream tests now assert the ingress revert, and chkdispute_rejects_invalid_winner_stays_open is superseded by forged_dispute_winner_rejected_at_delivery (a forged winner can no longer be delivered, so it can never reach resolvedisp — that guard remains as defense in depth).
A forged or chain-breaking envelope was recorded by deliver (which only checked epoch/size/decode/duplicate) and dropped later by apply_consensus. If a post-boundary majority relayed the same invalid envelope it reached consensus, apply_consensus soft-dropped it (no outpcons row), consensus_reached stayed true so no dispute opened, and deliver blocked re-delivery -- chkcons then waited forever. Validate at ingress instead: factor the envelope-chain + semantic-header checks into inbound_envelope_valid and have deliver check() it (revert) after the duplicate check, so an invalid envelope is never recorded and can never reach the consensus tally; a reverted tx records nothing, so the operator can re-deliver a corrected envelope. apply_consensus keeps the check as defense in depth (soft-drop on the evalcons path, check() on resolvedisp). Tests: the forgery/replay/cross-stream/chain-break cases now assert the ingress revert; chkdispute_rejects_invalid_winner_stays_open becomes forged_dispute_winner_rejected_at_delivery (the forged winner can no longer be delivered, so resolvedisp's guard is defense in depth); new forged_delivery_does_not_strand_chkcons drives the production chkcons gate to prove a forged delivery does not stall the epoch.
2 participants
Final wire-sysio half of SEC-102 (WSA-005 item 3), stacked on #506, completing the emitter PRs (#506, Wire-Network/wire-solana#370, Wire-Network/wire-ethereum#137). Direct enforcement — no staged mode — per the pre-launch dispensation.
apply_consensusvalidates every message's semantic header against the spec derivation before dispatch:data_size == len(data)per attestation,payload_size/payload_checksumover the canonical payload bytes,header_checksumover the blanked field-complete header, andmessage_id= header checksum + big-endian sequence splice at previous + 1. Mismatches drop the envelope (never-throw); delivery rows stay intact for audit. Checks are self-contained per message — stream ordering is already covered by the envelope-level chain — and zero-message envelopes (EVM epoch acks) remain valid.contracts/tests/opp_envelope_oracle.hpp; the msgch chain, dispatch, and dispute fixtures all derive spec headers through it.Deployment ordering: wire-solana#370 must be deployed to a shared network before this rolls out there, or its previously-empty-header envelopes drop. Ethereum already emits populated headers.
Targets master but depends on #506 (spec + depot emit) — merge #506 first; until then this PR's diff also shows #506's changes.