protocol: drop a frame that fails signature verification (ibx#275)

[userFRM]

Summary

• Connection::unsign returned (Vec<u8>, bool) and every caller discarded the flag — _valid or _ at twelve sites, plus a thirteenth doing the same on a direct fix_unsign in the auth loop. No site in the tree acted on the result, so a frame that failed verification was parsed and applied exactly like an authentic one.
• It now returns Option<Vec<u8>>. A frame that does not verify yields nothing, which is the same information in a form a caller cannot ignore. Thirteen out of thirteen discarding a flag is a signature the type should not have had.
• A failed frame no longer advances the read IV. Undistortion XORs byte positions from that IV, so one damaged frame moved the chain on and every genuine frame after it arrived silently corrupted — a connection that hit one bad frame stayed broken. That half needs no adversary.

What is NOT included

A frame carrying no 8349 tag is still accepted, as the reference client does. #275 asks for the tag to be required whenever a read key is set, and that is the right end state — but whether the gateway ever sends an untagged frame on a keyed connection is not established here, and refusing them on that assumption would drop real traffic to protect against nothing. A warning now makes the case visible, so the question can be settled from logs rather than guessed.

The issue's related finding — that do_srp never checks M2 — is untouched. It needs an M2 function that srp.rs does not have, and belongs in its own change.

Closes #275.

Correction, and an open question

Two reviews reached opposite conclusions on whether a failed frame should advance the read IV, and each was right about the case it considered:

• An injected frame between two authentic ones poisons everything after it if the chain advances, because the IV would be derived from bytes the attacker chose.
• A genuine frame whose signature alone was damaged leaves the connection stuck if it does not, because its body is intact and the derived IV would have been the sender's true next one.

They are indistinguishable at the point of decision, so the question is not which is likelier. new_iv is iv ^ HMAC(key, iv || body) over the received body, and a failed MAC is exactly the statement that this body cannot be vouched for. Advancing therefore takes cryptographic state from unauthenticated input. The chain does not advance.

The cost is real and stated in the code: a genuine frame corrupted in precisely its signature leaves the connection unable to verify what follows. I think the honest resolution is that a channel which has seen a MAC failure should be torn down rather than have its state guessed — but that is a larger change than this one and wants your decision rather than my assumption.

Two other things the reviews found, both fixed:

• The signature was located by searching for 8349= anywhere, which matches the same text inside a field value — a reject reason quoting it, say. That put the body boundary mid-message and made a legitimate frame report invalid: harmless while the verdict was discarded, lost traffic once enforced. Matched with its delimiter at all three sites.
• My caller inventory was wrong. tests/ib_paper_compat/main.rs uses the flag in diagnostic output rather than discarding it, and tests/depth_wire_test.rs destructures the pair — that target compiles at base and did not at my first HEAD. All eight test targets are enumerated and checked now.

Known gap: the thirteenth site, in the auth loop, is a hand copy of the same logic and is not covered — removing its guard leaves the suite green.

Note on #275's threat model: that issue claimed every socket runs inside TLS. It does not — connect_farm runs over plain TCP, so on farm channels this MAC was the only integrity control there was. Corrected on the issue.

Test plan

• cargo test --offline --lib — 805 passed. The 2 failures are config::expiry_tests::{named_zone_converts_with_dst, instant_round_trips_to_wire}, which fail on the base commit too: the host has no legacy timezone files (fixed separately in config: resolve the legacy timezone names IB states its times in (ibx#335) #336).
• cargo check --offline clean on every offline target: --lib, --features python, --bins, --examples, --test control_plane, --test scenarios, --test hot_loop_lifecycle.
• Mutation check, each reverted on its own: returning the frame despite a failed check fails a_frame_that_fails_verification_is_not_returned and a_failed_frame_does_not_advance_the_read_iv; advancing the IV on failure fails the latter; refusing untagged frames fails unsigned_connections_and_untagged_frames_still_pass. All by name.
• The recovery case has a positive control: after a dropped frame, the next genuine frame still verifies — which it would not if the IV had moved.
• Not in this PR: requiring the 8349 tag on a keyed connection, pending log evidence that no legitimate frame arrives without one.

🤖 Generated with Claude Code

[userFRM]

Updated in 9e4166e. Three changes.

The read IV advances again on a failed frame. Holding it back was the wrong side of the trade: the next IV is iv ^ HMAC(key, iv || body), derived from the body rather than the signature, so a frame whose transmitted signature alone is damaged still yields the sender's true next IV — and withholding it desynchronises the following authentic frame. When the body is damaged the derived IV is wrong, but so is the one being held, since the sender advanced using the true body.

The test covering this replayed the same frame rather than a following one signed from the chained IV, so it asserted a recovery it had not demonstrated. It now builds two frames as a sender would.

The signature is matched as a field, not a substring. 8349= appearing inside a field value — a reject reason quoting it — put the body boundary mid-message and made a legitimate signed frame report invalid. Harmless while every caller discarded the verdict; a dropped frame once they do not. Matched with its leading delimiter at all three sites.

Every test target compiles. The caller census in the first message counted src/ only: tests/ib_paper_compat/main.rs uses the flag in diagnostic output and tests/depth_wire_test.rs destructures the pair. All eight targets are enumerated and checked now, and ib_paper_compat is back to its base error count of 14.

806 pass plus the two config::expiry_tests timezone failures.

Separately: #275's threat model states that every socket runs inside TLS. That is not the case — connect_farm runs over plain TCP — and I have corrected the issue.

[userFRM]

The read-IV question here has two defensible answers, and the branch has now taken both, so the reasoning is written where the decision is made rather than asserted in a message.

• An injected frame between two authentic ones poisons everything after it if the chain advances, because the IV would be derived from bytes the sender never wrote.
• A genuine frame whose signature alone was damaged leaves the connection stuck if it does not, because its body is intact and the derived IV would have been the sender's true next one.

Both hold. They are also indistinguishable at the point of decision, so the tiebreak is not which is likelier: new_iv is iv ^ HMAC(key, iv || body) over the received body, and a failed MAC is exactly the statement that this body cannot be vouched for. Advancing would take cryptographic state from unauthenticated input and let one injected frame steer the receiver's chain. The chain does not advance, and the code says what that costs.

Neither answer is fully satisfying. A channel that has seen a MAC failure is not one whose state can be inferred, and tearing the connection down rather than picking an IV is probably the right end state — a larger change than this, and a decision rather than an assumption, so it is flagged rather than taken.

The other two changes: the signature is matched as a delimited field, so a legitimate frame quoting 8349= in a value is no longer dropped; and every one of the eight test targets compiles, including depth_wire_test, which compiles at base.

Known gap: the thirteenth site in the auth loop is a hand copy of the same logic, and removing its guard leaves the suite green.

806 pass plus the two config::expiry_tests timezone failures. Advancing the IV on a failed frame fails an_injected_frame_does_not_poison_the_authentic_ones_around_it by name.

[userFRM]

Updated in 5c7c94f. Two changes, both about the doc rather than the behaviour.

The /// doc on unsign now argues the reason the code has. It still said withholding the IV prevents a damaged frame desynchronising the chain. Under the policy this branch settled on, that case is withholding's cost, which the comment twenty lines below states correctly — so the two argued opposite reasons for the same lines, and the doc is the one rustdoc renders.

The pre-check is pinned. Reverting it to the un-anchored needle passed every test. The uncovered case is an unsigned frame quoting the tag in a field value: the anchored needle correctly declines to verify it, while the bare one routes it into verification, finds no signature field, and drops a legitimate frame. The signed-frame test cannot catch that, since its pre-check passes under either needle.

Two further confirmations worth recording: the needle change is byte-identical on well-formed frames, because the undistortion XOR range can never reach the signature tag bytes, so the pre-check and the post-undistortion field search cannot disagree; and ib_paper_compat matches base by line-identical error sets rather than by count.

807 pass plus the two config::expiry_tests timezone failures. All eight test targets compile, plus --lib, --features python, --bins and --examples.

Still open: the auth-loop copy of this logic has no coverage — its policy has changed twice with no test impact. And a channel that has seen a MAC failure arguably wants tearing down rather than having its IV guessed; that is a design decision rather than something to assume.