Bound protocol message sizes before allocating - #108
Conversation
recv_msg allocated the full message buffer from the peer-controlled length prefix before reading any of the body, so a TLS-authenticated (but not yet attested) peer could demand a 4 GiB zeroed allocation with a 4-byte prefix, and hold it by leaving the connection open — repeatable across connections for memory exhaustion. Authenticated-but-unattested is exactly the adversary class the attestation layer exists to screen out, so the framing layer must not trust it with allocation sizes. Bound messages at a named MAX_MSG_SIZE (1 MiB), with compile-time asserts that the bound admits the largest legitimate messages (the hubpacked measurement log and attestation). Oversized lengths are rejected as a new Error::MessageTooLarge before any allocation. No interoperability change: every legitimate protocol message is far below the bound.
andrewjstone
left a comment
andrewjstone
left a comment
There was a problem hiding this comment.
I'm quite surprised I didn't add max message size code here. That's definitely something I try to always do. Nice catch Claude!
| const MAX_MSG_SIZE: usize = 1024 * 1024; | ||
|
|
||
| // The bound must admit every legitimate protocol message. | ||
| const _: () = assert!(dice_verifier::Log::MAX_SIZE <= MAX_MSG_SIZE); |
There was a problem hiding this comment.
Never seen this style of static assertion before. I usually use https://docs.rs/static_assertions/latest/static_assertions/ for this. I like this though!
There was a problem hiding this comment.
I would have reached for that too :) I think this is even what happens inside of static_assertions? De gustibus, imo.
| /// legitimate protocol message: the largest are the hubpacked measurement | ||
| /// log and attestation, whose bounds are asserted below; cert chains and | ||
| /// nonces are far smaller. | ||
| const MAX_MSG_SIZE: usize = 1024 * 1024; |
There was a problem hiding this comment.
This should be large enough but I really don't want to hit this in production if we have too many reconfigurations before garbage collection is implemented for old secrets.. Maybe 10MiB instead?
There was a problem hiding this comment.
This is a good point! I'd trust your judgement about what to fill in here.
2 participants
CAVEAT LECTOR: This PR was generated by Claude Fable 5.
Problem
recv_msgallocates the full message buffer from the peer-controlled length prefix before reading any of the body, so a TLS-authenticated (but not yet attested) peer can demand a 4 GiB zeroed allocation with a 4-byte prefix, and hold it by leaving the connection open, repeatably across connections, for memory exhaustion.I posit that authenticated-but-unattested is the adversary class the attestation layer exists to screen out (e.g. a compromised sled with legitimate certs), so the framing layer must not trust it with allocation sizes.
Fix
Bound messages at a named
MAX_MSG_SIZE(1 MiB) checked before allocation, rejected as a newError::MessageTooLarge { len, max }. Compile-time asserts prove the bound admits the largest legitimate messages (the hubpacked measurementLogandAttestation); cert chains and nonces are far smaller. No interoperability change.Testing
New regression test
oversized_message_rejected: an authenticated client sends au32::MAXlength prefix with no body; the server must reject on the prefix alone.Verified on illumos:
cargo test7/7, CI-style clippy clean.Relationships
Independent of #107 and #109; the three can land in any order.