From 3965e2544c88c44438206fe21e9789ae6ca8a8a9 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Wed, 2 Sep 2026 22:27:39 +0200 Subject: [PATCH] fix(peer): bound inbound request frames at 64 KiB instead of 8 MiB Security audit findings NET-03 and Codex #6 ("control-frame prefixes can reserve about 512 MiB across concurrent decoders"). Both directions of the control plane shared MAX_CONTROL_FRAME_BYTES (8 MiB). That size exists for responses: a HelloSnapshot with 4096 library games and a maximal Call-to-Play author slice legitimately approaches it. Requests are tiny; the largest possible GetGameFileChunk with a 255-byte game ID and a 900-byte catalog path is under 2 KiB. Yet every anonymous inbound stream was decoded with an 8 MiB LengthDelimitedCodec, and tokio-util reserves the declared frame length as soon as the 4-byte prefix arrives. With 64 global control-stream permits a LAN host could make a responder reserve ~512 MiB by sending nothing but length prefixes. Changes: - lanspread-proto gains MAX_REQUEST_FRAME_BYTES (64 KiB). Request encode/decode enforce it in addition to the shared bound; Response keeps the 8 MiB allowance. - The server-side stream handler decodes inbound frames with a request-sized codec. The response writer is unchanged. - The server QUIC limits shrink the per-stream receive window to one request frame and size the connection window so every one of the 32 allowed streams can hold its allowance (2 MiB per connection instead of 8 MiB per stream). Client-side decoders (network.rs, discovery Hello pulls) still use the 8 MiB bound because they read responses from identity-pinned peers. Test plan: `just test` (proto tests assert the exact limits and that a maximal request encodes far below the bound; stream tests assert the inbound codec uses the request bound). Manual: three peer-cli containers still exchange snapshots and complete downloads. Claude-Session: https://claude.ai/code/session_017C3Nbgwpdm3YNwZhhFLHwg --- crates/lanspread-peer/src/quic_runtime.rs | 15 +++-- crates/lanspread-peer/src/services/stream.rs | 19 ++++++- crates/lanspread-proto/src/lib.rs | 58 +++++++++++++++++++- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/crates/lanspread-peer/src/quic_runtime.rs b/crates/lanspread-peer/src/quic_runtime.rs index 232df18..90d1527 100644 --- a/crates/lanspread-peer/src/quic_runtime.rs +++ b/crates/lanspread-peer/src/quic_runtime.rs @@ -15,7 +15,7 @@ use std::{ task::{Context, Poll}, }; -use lanspread_proto::{MAX_CONTROL_FRAME_BYTES, PeerEndpoint}; +use lanspread_proto::{MAX_CONTROL_FRAME_BYTES, MAX_REQUEST_FRAME_BYTES, PeerEndpoint}; use s2n_quic::{ Client as QuicClient, Connection, @@ -54,9 +54,14 @@ use crate::{ /// either direction. The server independently enforces the same application /// control-stream task bound before spawning work. pub(crate) const MAX_OPEN_BIDIRECTIONAL_STREAMS: u64 = 32; -/// One server connection exposes only enough aggregate receive credit for one -/// maximal length-delimited control frame at a time. -pub(crate) const SERVER_CONTROL_RECEIVE_WINDOW: u64 = MAX_CONTROL_FRAME_BYTES as u64 + 4; +/// Per-stream receive credit on a server connection: exactly one maximal +/// length-delimited request frame. Inbound public streams carry one request; +/// snapshot-sized frames only flow outbound. +pub(crate) const SERVER_CONTROL_RECEIVE_WINDOW: u64 = MAX_REQUEST_FRAME_BYTES as u64 + 4; +/// Connection-level receive credit on a server connection: every concurrently +/// open request stream may hold its full per-stream allowance. +pub(crate) const SERVER_CONNECTION_RECEIVE_WINDOW: u64 = + SERVER_CONTROL_RECEIVE_WINDOW * MAX_OPEN_BIDIRECTIONAL_STREAMS; pub(crate) fn quic_client_limits() -> eyre::Result { Ok(Limits::default() @@ -80,7 +85,7 @@ pub(crate) fn quic_client_limits() -> eyre::Result { /// separately configured outbound connector. pub(crate) fn quic_server_limits() -> eyre::Result { Ok(Limits::default() - .with_data_window(SERVER_CONTROL_RECEIVE_WINDOW)? + .with_data_window(SERVER_CONNECTION_RECEIVE_WINDOW)? .with_bidirectional_local_data_window(0)? .with_bidirectional_remote_data_window(SERVER_CONTROL_RECEIVE_WINDOW)? .with_unidirectional_data_window(0)? diff --git a/crates/lanspread-peer/src/services/stream.rs b/crates/lanspread-peer/src/services/stream.rs index 12cef1b..2428c84 100644 --- a/crates/lanspread-peer/src/services/stream.rs +++ b/crates/lanspread-peer/src/services/stream.rs @@ -7,6 +7,7 @@ use lanspread_proto::{ ControlErrorCode, ControlMessage, MAX_CONTROL_FRAME_BYTES, + MAX_REQUEST_FRAME_BYTES, Request, Response, }; @@ -34,12 +35,22 @@ type ResponseWriter = FramedWrite; const INBOUND_CONTROL_FRAME_TIMEOUT: Duration = Duration::from_secs(10); const OUTBOUND_CONTROL_IO_TIMEOUT: Duration = Duration::from_secs(10); +/// Response-side codec: snapshots may approach the full control-frame bound. fn control_codec() -> LengthDelimitedCodec { LengthDelimitedCodec::builder() .max_frame_length(MAX_CONTROL_FRAME_BYTES) .new_codec() } +/// Request-side codec for anonymous inbound streams. The length-delimited +/// decoder reserves the declared frame length up front, so the public +/// responder only ever grants the small request allowance per stream. +fn request_codec() -> LengthDelimitedCodec { + LengthDelimitedCodec::builder() + .max_frame_length(MAX_REQUEST_FRAME_BYTES) + .new_codec() +} + /// Reads exactly one bounded request frame, requires request-side EOF, sends at /// most one control response, and then closes the stream. Raw transfer requests /// consume the response side after the same single control-frame admission. @@ -52,7 +63,7 @@ pub(super) async fn handle_peer_stream( bulk_transfer_permits: Arc, ) -> eyre::Result<()> { let (rx, tx) = stream.split(); - let mut framed_rx = FramedRead::new(rx, control_codec()); + let mut framed_rx = FramedRead::new(rx, request_codec()); let mut framed_tx = FramedWrite::new(tx, control_codec()); log::trace!("{remote_addr:?} peer stream opened"); @@ -420,6 +431,12 @@ mod tests { assert_eq!(codec.max_frame_length(), MAX_CONTROL_FRAME_BYTES); } + #[test] + fn inbound_request_decoders_use_the_small_request_bound() { + let codec = request_codec(); + assert_eq!(codec.max_frame_length(), MAX_REQUEST_FRAME_BYTES); + } + #[test] fn trailing_frame_outcomes_are_never_accepted_as_eof() { for outcome in [ diff --git a/crates/lanspread-proto/src/lib.rs b/crates/lanspread-proto/src/lib.rs index d709d61..e3f8a7c 100644 --- a/crates/lanspread-proto/src/lib.rs +++ b/crates/lanspread-proto/src/lib.rs @@ -10,7 +10,13 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de::DeserializeOwn pub const PROTOCOL_VERSION: u32 = 8; pub const ALPN_PROTOCOL: &[u8] = b"lanspread/8"; +/// Upper bound for one control-plane frame in either direction. Responses +/// (peer-state snapshots) can legitimately approach this size. pub const MAX_CONTROL_FRAME_BYTES: usize = 8 * 1024 * 1024; +/// Upper bound for one inbound `Request` frame. Every request variant is a +/// few kilobytes at most, so public responders decode requests with this much +/// smaller allowance instead of reserving a snapshot-sized buffer per stream. +pub const MAX_REQUEST_FRAME_BYTES: usize = 64 * 1024; pub const MAX_STREAM_INSTALL_FRAME_BYTES: usize = 8 * 1024 * 1024; pub const MAX_LIBRARY_GAMES: usize = 4_096; pub const MAX_GAME_ID_BYTES: usize = 255; @@ -713,11 +719,14 @@ trait ValidateControlMessage { impl ControlMessage for Request { fn decode(bytes: Bytes) -> Result { + check_request_frame_length(bytes.len())?; decode_control_message(&bytes) } fn encode(&self) -> Result { - encode_control_message(self) + let bytes = encode_control_message(self)?; + check_request_frame_length(bytes.len())?; + Ok(bytes) } } @@ -772,6 +781,16 @@ const fn check_control_frame_length(length: usize) -> Result<(), ControlCodecErr Ok(()) } +const fn check_request_frame_length(length: usize) -> Result<(), ControlCodecError> { + if length > MAX_REQUEST_FRAME_BYTES { + return Err(ControlCodecError::FrameTooLarge { + actual: length, + maximum: MAX_REQUEST_FRAME_BYTES, + }); + } + Ok(()) +} + impl ValidateControlMessage for Request { fn validate_control(&self) -> Result<(), ControlValidationError> { match self { @@ -1180,9 +1199,33 @@ mod tests { assert_eq!(PROTOCOL_VERSION, 8); assert_eq!(ALPN_PROTOCOL, b"lanspread/8"); assert_eq!(MAX_CONTROL_FRAME_BYTES, 8 * 1024 * 1024); + assert_eq!(MAX_REQUEST_FRAME_BYTES, 64 * 1024); assert_eq!(MAX_CALL_TO_PLAY_AUTHOR_SNAPSHOT_BYTES, 4 * 1024 * 1024); } + #[test] + fn maximal_requests_fit_the_request_frame_bound() { + use lanspread_db::content_manifest::{MAX_CATALOG_COMPONENT_BYTES, MAX_CATALOG_PATH_BYTES}; + + let directory = "d".repeat(MAX_CATALOG_COMPONENT_BYTES); + let relative_path = format!( + "{directory}/{directory}/{directory}/{}", + "f".repeat(MAX_CATALOG_PATH_BYTES - 3 * (MAX_CATALOG_COMPONENT_BYTES + 1)) + ); + assert_eq!(relative_path.len(), MAX_CATALOG_PATH_BYTES); + let request = Request::GetGameFileChunk { + game_id: "g".repeat(MAX_GAME_ID_BYTES), + content_id: content(0xff), + relative_path: CanonicalCatalogPath::new(relative_path) + .expect("maximal catalog path should be canonical"), + offset: u64::MAX, + length: u64::MAX, + }; + let encoded = request.encode().expect("maximal request must encode"); + assert!(encoded.len() < MAX_REQUEST_FRAME_BYTES / 8); + Request::decode(encoded).expect("maximal request must decode"); + } + #[test] fn fixed_hex_ids_are_canonical_and_distinct() { let runtime = session(0xab); @@ -1673,11 +1716,20 @@ mod tests { #[test] fn control_frame_limit_and_single_document_are_enforced() { assert!(matches!( - Request::decode(Bytes::from(vec![b' '; MAX_CONTROL_FRAME_BYTES])), + Request::decode(Bytes::from(vec![b' '; MAX_REQUEST_FRAME_BYTES])), Err(ControlCodecError::Decode(_)) )); assert!(matches!( - Request::decode(Bytes::from(vec![b' '; MAX_CONTROL_FRAME_BYTES + 1])), + Request::decode(Bytes::from(vec![b' '; MAX_REQUEST_FRAME_BYTES + 1])), + Err(ControlCodecError::FrameTooLarge { actual, maximum }) + if actual == MAX_REQUEST_FRAME_BYTES + 1 && maximum == MAX_REQUEST_FRAME_BYTES + )); + assert!(matches!( + Response::decode(Bytes::from(vec![b' '; MAX_CONTROL_FRAME_BYTES])), + Err(ControlCodecError::Decode(_)) + )); + assert!(matches!( + Response::decode(Bytes::from(vec![b' '; MAX_CONTROL_FRAME_BYTES + 1])), Err(ControlCodecError::FrameTooLarge { actual, maximum }) if actual == MAX_CONTROL_FRAME_BYTES + 1 && maximum == MAX_CONTROL_FRAME_BYTES ));