diff --git a/crates/openshell-sandbox/src/l7/inference.rs b/crates/openshell-sandbox/src/l7/inference.rs index 59dafdab..140213f0 100644 --- a/crates/openshell-sandbox/src/l7/inference.rs +++ b/crates/openshell-sandbox/src/l7/inference.rs @@ -96,6 +96,8 @@ pub enum ParseResult { Complete(ParsedHttpRequest, usize), /// Headers are incomplete — caller should read more data. Incomplete, + /// The request is malformed and must be rejected (e.g., duplicate Content-Length). + Invalid(String), } /// Try to parse an HTTP/1.1 request from raw bytes. @@ -125,6 +127,7 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { let mut headers = Vec::new(); let mut content_length: usize = 0; + let mut has_content_length = false; let mut is_chunked = false; for line in lines { if line.is_empty() { @@ -134,7 +137,21 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { let name = name.trim().to_string(); let value = value.trim().to_string(); if name.eq_ignore_ascii_case("content-length") { - content_length = value.parse().unwrap_or(0); + let new_len: usize = match value.parse() { + Ok(v) => v, + Err(_) => { + return ParseResult::Invalid(format!( + "invalid Content-Length value: {value}" + )); + } + }; + if has_content_length && new_len != content_length { + return ParseResult::Invalid(format!( + "duplicate Content-Length headers with differing values ({content_length} vs {new_len})" + )); + } + content_length = new_len; + has_content_length = true; } if name.eq_ignore_ascii_case("transfer-encoding") && value @@ -552,4 +569,43 @@ mod tests { }; assert_eq!(parsed.body.len(), 100); } + + // ---- SEC: Content-Length validation ---- + + #[test] + fn reject_differing_duplicate_content_length() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(reason) if reason.contains("differing values") + )); + } + + #[test] + fn accept_identical_duplicate_content_length() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\nContent-Length: 5\r\n\r\nhello"; + let ParseResult::Complete(parsed, _) = try_parse_http_request(request) else { + panic!("expected Complete for identical duplicate CL"); + }; + assert_eq!(parsed.body, b"hello"); + } + + #[test] + fn reject_non_numeric_content_length() { + let request = + b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(reason) if reason.contains("invalid Content-Length") + )); + } + + #[test] + fn reject_two_non_numeric_content_lengths() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\nContent-Length: def\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(_) + )); + } } diff --git a/crates/openshell-sandbox/src/l7/rest.rs b/crates/openshell-sandbox/src/l7/rest.rs index ebb34957..f47f01bd 100644 --- a/crates/openshell-sandbox/src/l7/rest.rs +++ b/crates/openshell-sandbox/src/l7/rest.rs @@ -242,14 +242,22 @@ fn parse_body_length(headers: &str) -> Result { let lower = line.to_ascii_lowercase(); if lower.starts_with("transfer-encoding:") { let val = lower.split_once(':').map_or("", |(_, v)| v.trim()); - if val.contains("chunked") { + if val.split(',').any(|enc| enc.trim() == "chunked") { has_te_chunked = true; } } - if lower.starts_with("content-length:") - && let Some(val) = lower.split_once(':').map(|(_, v)| v.trim()) - && let Ok(len) = val.parse::() - { + if lower.starts_with("content-length:") { + let val = lower.split_once(':').map_or("", |(_, v)| v.trim()); + let len: u64 = val + .parse() + .map_err(|_| miette!("Request contains invalid Content-Length value"))?; + if let Some(prev) = cl_value { + if prev != len { + return Err(miette!( + "Request contains multiple Content-Length headers with differing values ({prev} vs {len})" + )); + } + } cl_value = Some(len); } } @@ -702,6 +710,59 @@ mod tests { ); } + /// SEC: Reject differing duplicate Content-Length headers. + #[test] + fn reject_differing_duplicate_content_length() { + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject differing duplicate Content-Length" + ); + } + + /// SEC: Accept identical duplicate Content-Length headers. + #[test] + fn accept_identical_duplicate_content_length() { + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: 42\r\n\r\n"; + match parse_body_length(headers).unwrap() { + BodyLength::ContentLength(42) => {} + other => panic!("Expected ContentLength(42), got {other:?}"), + } + } + + /// SEC: Reject non-numeric Content-Length values. + #[test] + fn reject_non_numeric_content_length() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject non-numeric Content-Length" + ); + } + + /// SEC: Reject when second Content-Length is non-numeric (bypass test). + #[test] + fn reject_valid_then_invalid_content_length() { + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: abc\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject when any Content-Length is non-numeric" + ); + } + + /// SEC: Transfer-Encoding substring match must not match partial tokens. + #[test] + fn te_substring_not_chunked() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunkedx\r\n\r\n"; + match parse_body_length(headers).unwrap() { + BodyLength::None => {} + other => panic!("Expected None for non-matching TE, got {other:?}"), + } + } + /// SEC-009: Bare LF in headers enables header injection. #[tokio::test] async fn reject_bare_lf_in_headers() { diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index 1f38a2cc..6f3eaf9b 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -943,6 +943,12 @@ async fn handle_inference_interception( buf.resize((buf.len() * 2).min(MAX_INFERENCE_BUF), 0); } } + ParseResult::Invalid(reason) => { + warn!(reason = %reason, "rejecting malformed inference request"); + let response = format_http_response(400, &[], b"Bad Request"); + write_all(&mut tls_client, &response).await?; + return Ok(InferenceOutcome::Denied { reason }); + } } }