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
This commit is contained in:
3 files changed
+83
-9
No files matched your search
@@ -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<Limits> {
|
||||
Ok(Limits::default()
|
||||
@@ -80,7 +85,7 @@ pub(crate) fn quic_client_limits() -> eyre::Result<Limits> {
|
||||
/// separately configured outbound connector.
|
||||
pub(crate) fn quic_server_limits() -> eyre::Result<Limits> {
|
||||
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)?
|
||||
|
||||
@@ -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<SendStream, LengthDelimitedCodec>;
|
||||
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<Semaphore>,
|
||||
) -> 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 [
|
||||
|
||||
Reference in new issue
Block a user