dtls: reject malformed handshake record tails#139
Conversation
|
This looks almost exactly like that other case where we decided to not consider packets all or nothing. Is this really guarding for a real concern? |
|
The reason I think this is different from the broader all-or-nothing datagram question is that this is scoped to one Handshake record, not the whole UDP datagram. If the record header says the fragment contains N bytes of handshake messages, then accepting only a valid prefix and ignoring a malformed trailing handshake means we are accepting a record whose declared handshake payload was not actually parseable. For plaintext this is mostly strict framing hygiene. For protected records it matters more, because the record has authenticated and replay/bookkeeping may advance. I think replay state should only be committed after the decrypted record payload is fully accepted, otherwise a malformed authenticated record can partially mutate handshake/replay state and make recovery depend on retransmission edge cases. The fallout from not doing this is limited: mostly permissive acceptance of a malformed record, and in protected-record edge cases possibly replay/handshake state advancing for a record that later has to be treated as malformed. |
|
Thanks — but I don't think the protected-record part holds up, and that's the part that carried the security argument.
That means the only record that hits the new gap — authenticates fine, but the decrypted payload fails to parse as handshakes — has to come from the genuine peer, since forging an authenticated record needs the key. That's a buggy peer, not an attacker. And the "consume the retransmission slot for a clean record" concern doesn't match DTLS framing: retransmissions don't reuse the record sequence number. The handshake I think we are largely in the same bucket as the all-or-nothing-datagram case. I'm inclined to pass on this unless there's a concrete case where an authenticated-but-unparseable record from the real peer actually wedges recovery. |
Handshake record parsing could accept a valid leading handshake while ignoring a
malformed trailing handshake in the same record. That can leave receive/replay
state committed for a record whose full handshake payload was not valid.
This requires exact same-record handshake parsing for DTLS 1.2 and DTLS 1.3,
defers replay commit until protected records decrypt and parse cleanly, and
discards malformed buffered protected handshakes so they cannot pin recovery.
This intentionally does not try to solve broader multi-record/datagram replay
transactionality.
Line delta:
Validation:
cargo fmt --checkgit diff --checkgit diff --check upstream/main...HEAD/home/ronen/.codex/skills/dimpl/scripts/check-snowflake-local.pl upstream/maincargo test --all-targets --features rcgencargo clippy --all-targets --features rcgen -- -D warningscargo test --no-default-features --features rust-cryptocargo clippy --no-default-features --features rust-crypto -- -D warningscargo test --doc --features rcgen