From 658040b882aa3c255b1e8d4afb5850840f972271 Mon Sep 17 00:00:00 2001 From: Vahagn Date: Sun, 1 Nov 2026 09:48:29 +0400 Subject: [PATCH 1/3] [feat] http: observe request bodies via http_body_fn Add http_body_fn, an optional proxy callback that receives the request body alongside method, host and path. None by default. It runs before the ACL check and caps what the callback sees at 10 MiB. Signed-off-by: Vahagn --- crates/sandlock-core/src/sandbox.rs | 11 +- crates/sandlock-core/src/sandbox/builder.rs | 31 ++- .../src/transparent_proxy/disabled.rs | 1 + .../src/transparent_proxy/mod.rs | 3 + .../src/transparent_proxy/service.rs | 46 +++- .../tests/integration/test_http_acl.rs | 213 ++++++++++++++++++ 6 files changed, 293 insertions(+), 12 deletions(-) diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 8db65cf1..efeaffbb 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -811,6 +811,9 @@ pub struct Sandbox { /// spawned even without ACL rules; every request is logged via this closure. #[serde(skip)] pub(crate) http_log_fn: Option>, + /// Optional callback given the body of every proxied request. + #[serde(skip)] + pub(crate) http_body_fn: Option>, // Resource limits pub max_memory: Option, @@ -974,6 +977,7 @@ impl Clone for Sandbox { http_inject_ca: self.http_inject_ca.clone(), http_ca_out: self.http_ca_out.clone(), http_log_fn: self.http_log_fn.clone(), + http_body_fn: self.http_body_fn.clone(), max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, @@ -2440,7 +2444,11 @@ impl Sandbox { ); let mut ca_inject_pem: Option>> = None; - if !self.http_allow.is_empty() || !self.http_deny.is_empty() || self.http_log_fn.is_some() { + if !self.http_allow.is_empty() + || !self.http_deny.is_empty() + || self.http_log_fn.is_some() + || self.http_body_fn.is_some() + { // Generate an ephemeral CA when injection is requested without BYO. let generate = !self.http_inject_ca.is_empty(); let ca_material = crate::transparent_proxy::resolve_ca( @@ -2474,6 +2482,7 @@ impl Sandbox { cert_pem, key_pem, self.http_log_fn.clone(), + self.http_body_fn.clone(), ) .await .map_err(SandboxRuntimeError::Io)?; diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index 8fbfb975..607d94a1 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -120,6 +120,11 @@ pub struct SandboxBuilder { #[cfg_attr(feature = "cli", clap(skip))] pub http_log_fn: Option>, + /// Optional callback given the body of every request that reaches the + /// proxy, in addition to method/host/path. + #[cfg_attr(feature = "cli", clap(skip))] + pub http_body_fn: Option>, + // max_memory uses a string in the CLI (e.g. "512M"); not directly clap-friendly as ByteSize. #[cfg_attr(feature = "cli", clap(skip))] pub max_memory: Option, @@ -296,6 +301,7 @@ impl Default for SandboxBuilder { http_inject_ca: Vec::new(), http_ca_out: None, http_log_fn: None, + http_body_fn: None, max_memory: None, max_processes: None, max_open_files: None, @@ -363,6 +369,7 @@ impl Clone for SandboxBuilder { http_inject_ca: self.http_inject_ca.clone(), http_ca_out: self.http_ca_out.clone(), http_log_fn: self.http_log_fn.clone(), + http_body_fn: self.http_body_fn.clone(), max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, @@ -615,6 +622,16 @@ impl SandboxBuilder { self } + /// Observe the body of every request that reaches the proxy, with its + /// method, host and path. Independent of [`Self::http_log_fn`]. + pub fn http_body_fn( + mut self, + f: std::sync::Arc, + ) -> Self { + self.http_body_fn = Some(f); + self + } + pub fn max_memory(mut self, size: ByteSize) -> Self { self.max_memory = Some(size); self @@ -1085,6 +1102,7 @@ impl SandboxBuilder { ("--http-auth", !self.http_auth.is_empty()), ("--credential", !self.credentials.is_empty()), ("http_log_fn", self.http_log_fn.is_some()), + ("http_body_fn", self.http_body_fn.is_some()), ]; if let Some((what, _)) = set.iter().find(|(_, on)| *on) { return Err(SandboxError::FeatureDisabled { @@ -1102,12 +1120,16 @@ impl SandboxBuilder { } // --http-inject-ca / --http-ca-out are meaningless without an HTTP ACL - // proxy to do MITM, which only spawns when http rules or a log callback exist. - let has_http_rules = - !self.http_allow.is_empty() || !self.http_deny.is_empty() || self.http_log_fn.is_some(); + // proxy to do MITM, which only spawns when http rules or an observation + // callback exist. + let has_http_rules = !self.http_allow.is_empty() + || !self.http_deny.is_empty() + || self.http_log_fn.is_some() + || self.http_body_fn.is_some(); if !self.http_inject_ca.is_empty() && !has_http_rules { return Err(SandboxError::Invalid( - "--http-inject-ca requires --http-allow or --http-deny".into(), + "--http-inject-ca requires --http-allow, --http-deny, or an observation callback" + .into(), )); } // --http-ca-out needs an actual CA to export (BYO or generated). @@ -1270,6 +1292,7 @@ impl SandboxBuilder { http_inject_ca: self.http_inject_ca, http_ca_out: self.http_ca_out, http_log_fn: self.http_log_fn, + http_body_fn: self.http_body_fn, max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, diff --git a/crates/sandlock-core/src/transparent_proxy/disabled.rs b/crates/sandlock-core/src/transparent_proxy/disabled.rs index b0d92828..ea2b6f96 100644 --- a/crates/sandlock-core/src/transparent_proxy/disabled.rs +++ b/crates/sandlock-core/src/transparent_proxy/disabled.rs @@ -44,6 +44,7 @@ pub(crate) async fn spawn_transparent_proxy( _: Option<&str>, _: Option<&str>, _: Option>, + _: Option>, ) -> io::Result { Err(disabled()) } diff --git a/crates/sandlock-core/src/transparent_proxy/mod.rs b/crates/sandlock-core/src/transparent_proxy/mod.rs index 9796e905..5751521c 100644 --- a/crates/sandlock-core/src/transparent_proxy/mod.rs +++ b/crates/sandlock-core/src/transparent_proxy/mod.rs @@ -53,6 +53,7 @@ pub(crate) async fn spawn_transparent_proxy( ca_cert_pem: Option<&str>, ca_key_pem: Option<&str>, log_fn: Option>, + body_fn: Option>, ) -> std::io::Result { let orig_dest: OrigDestMap = Arc::new(std::sync::RwLock::new(std::collections::HashMap::new())); let forwarder = Forwarder::new()?; @@ -63,6 +64,7 @@ pub(crate) async fn spawn_transparent_proxy( Arc::clone(&orig_dest), forwarder, log_fn, + body_fn, ); let signer = match (ca_cert_pem, ca_key_pem) { @@ -195,6 +197,7 @@ mod tests { Some(&ca.cert_pem), Some(&ca.key_pem), None, + None, ) .await .expect("proxy spawns"); diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index c86c1679..dba227e8 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -44,6 +44,16 @@ pub(crate) struct AclService { /// Optional observation callback for learn mode. Called with (method, host, path) /// for every request after the host is validated, before the ACL check. pub(crate) log_fn: Option>, + /// Optional callback given the full request body, in addition to + /// (method, host, path). + pub(crate) body_fn: Option>, +} + +/// Truncate to the cap handed to `body_fn`, since the untrusted workload could +/// otherwise force an arbitrarily large allocation into the callback's own storage. +fn capped_for_body_fn(body: &[u8]) -> &[u8] { + const MAX_BODY_FN_BYTES: usize = 10 * 1024 * 1024; + &body[..body.len().min(MAX_BODY_FN_BYTES)] } /// Whether this cleartext injection should emit the one-per-run warning: true the @@ -104,6 +114,7 @@ impl AclService { orig_dest: OrigDestMap, forwarder: Forwarder, log_fn: Option>, + body_fn: Option>, ) -> Self { Self { allow: Arc::new(allow), @@ -114,6 +125,7 @@ impl AclService { dns_cache: Arc::new(Mutex::new(HashMap::new())), cleartext_warned: Arc::new(AtomicBool::new(false)), log_fn, + body_fn, } } @@ -204,6 +216,29 @@ impl AclService { f(&method, &host, &path); } + let (mut parts, body) = req.into_parts(); + + // Buffer the body once as `Bytes` (cheap to share) before forwarding. + // A read failure rejects the request: the headers still carry the + // original Content-Length, so forwarding a short body would stall the + // upstream waiting for bytes that never arrive. + let body = match self.body_fn { + Some(ref f) => { + let body_bytes = match body.collect().await { + Ok(collected) => collected.to_bytes(), + Err(_) => { + return text_response( + StatusCode::BAD_GATEWAY, + "sandlock could not read the request body", + ) + } + }; + f(&method, &host, &path, capped_for_body_fn(&body_bytes)); + Full::new(body_bytes).map_err(|e| match e {}).boxed() + } + None => box_incoming(body), + }; + if !http_acl_check(&self.allow, &self.deny, &method, &host, &path) { return text_response(StatusCode::FORBIDDEN, "Blocked by sandlock HTTP ACL policy"); } @@ -211,20 +246,17 @@ impl AclService { // Rebuild an absolute-URI request for the upstream client from the SAME // validated authority — never the raw Host header — so the forward host // equals the host the ACL/credential match ran against. - let pq = req - .uri() + let pq = parts + .uri .path_and_query() .map(|p| p.as_str()) .unwrap_or("/") .to_string(); - let uri: hyper::Uri = match format!("{scheme}://{authority}{pq}").parse() { + parts.uri = match format!("{scheme}://{authority}{pq}").parse() { Ok(u) => u, Err(_) => return text_response(StatusCode::BAD_GATEWAY, "bad upstream URI"), }; - let (mut parts, body) = req.into_parts(); - parts.uri = uri; - // ACL passed: attach a credential if a rule matches. First match wins. // The secret is rendered into the outbound request only here — never on // the deny path above — and only its name is recorded, never the value. @@ -286,7 +318,7 @@ impl AclService { } } - let out_req = Request::from_parts(parts, box_incoming(body)); + let out_req = Request::from_parts(parts, body); match self.forwarder.forward(out_req).await { Ok(resp) => resp, diff --git a/crates/sandlock-core/tests/integration/test_http_acl.rs b/crates/sandlock-core/tests/integration/test_http_acl.rs index 6d22585f..75822694 100644 --- a/crates/sandlock-core/tests/integration/test_http_acl.rs +++ b/crates/sandlock-core/tests/integration/test_http_acl.rs @@ -801,6 +801,219 @@ async fn test_env_sourced_credential_stripped_from_child() { ); } +// ============================================================ +// Observation callbacks: http_log_fn / http_body_fn +// ============================================================ + +fn post_bytes_script(url: &str, body: &str, out: &std::path::Path) -> String { + let body_literal = format!("b'{}'", body); + format!( + concat!( + "import urllib.request, urllib.error\n", + "try:\n", + " req = urllib.request.Request('{url}', method='POST', data={body_literal})\n", + " resp = urllib.request.urlopen(req)\n", + " open('{out}', 'w').write('OK:' + str(resp.status))\n", + "except urllib.error.HTTPError as e:\n", + " open('{out}', 'w').write('HTTP:' + str(e.code))\n", + "except Exception as e:\n", + " open('{out}', 'w').write('ERR:' + str(e))\n", + ), + url = url, + body_literal = body_literal, + out = out.display(), + ) +} + +/// `http_log_fn` observes method/host/path for every request that reaches +/// the proxy, in addition to whatever ACL/forwarding decision it makes. +#[tokio::test] +async fn test_http_log_fn_records_requests() { + let out = temp_file("log-fn-records"); + let (port, srv) = spawn_http_server(1); + + let seen: std::sync::Arc>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + + let policy = base_policy() + .http_allow("GET 127.0.0.1/*") + .http_port(port) + .http_log_fn(std::sync::Arc::new(move |method, host, path| { + seen_cb + .lock() + .unwrap() + .push(format!("{method} {host}{path}")); + })) + .build() + .unwrap(); + + let script = http_script(&format!("http://127.0.0.1:{}/get", port), &out); + let result = policy + .clone() + .run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!(content.starts_with("OK:200"), "got: {}", content); + + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); + + assert_eq!(seen.lock().unwrap().as_slice(), ["GET 127.0.0.1/get"]); +} + +/// Spawn a server that records the body of the one request it receives, for +/// `http_body_fn` tests that need to tell the callback's view and the +/// upstream's view of a request apart. +fn spawn_body_capturing_http_server() -> ( + u16, + thread::JoinHandle<()>, + std::sync::Arc>>>, +) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let captured = std::sync::Arc::new(std::sync::Mutex::new(None)); + let cap = captured.clone(); + let handle = thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut content_length = 0usize; + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 { + break; + } + if line.to_lowercase().starts_with("content-length:") { + content_length = line + .split(':') + .nth(1) + .and_then(|v| v.trim().parse().ok()) + .unwrap_or(0); + } + if line == "\r\n" || line == "\n" { + break; + } + } + let mut body = vec![0u8; content_length]; + if content_length > 0 { + let _ = reader.read_exact(&mut body); + } + *cap.lock().unwrap() = Some(body); + let resp = "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok"; + let _ = stream.write_all(resp.as_bytes()); + let _ = stream.flush(); + } + }); + (port, handle, captured) +} + +/// `http_body_fn` sees exactly the bytes the child sent, and the upstream +/// still receives the same, unmodified bytes. +#[tokio::test] +async fn test_http_body_fn_receives_request_body() { + let out = temp_file("body-fn-receives"); + let (port, srv, upstream_got) = spawn_body_capturing_http_server(); + + let seen: std::sync::Arc)>>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + + let policy = base_policy() + .http_allow("POST 127.0.0.1/*") + .http_port(port) + .http_body_fn(std::sync::Arc::new(move |method, host, path, body| { + seen_cb.lock().unwrap().push(( + method.to_string(), + host.to_string(), + path.to_string(), + body.to_vec(), + )); + })) + .build() + .unwrap(); + + let script = post_bytes_script( + &format!("http://127.0.0.1:{}/upload", port), + "hello-body-fn", + &out, + ); + let result = policy + .clone() + .run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!(content.starts_with("OK:200"), "got: {}", content); + + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); + + let captured = seen.lock().unwrap(); + assert_eq!(captured.len(), 1, "callback should fire exactly once"); + let (method, host, path, body) = &captured[0]; + assert_eq!(method, "POST"); + assert_eq!(host, "127.0.0.1"); + assert_eq!(path, "/upload"); + assert_eq!(body, b"hello-body-fn"); + + assert_eq!( + upstream_got.lock().unwrap().as_deref(), + Some(b"hello-body-fn".as_slice()), + "upstream must still receive the unmodified body" + ); +} + +/// A request the ACL blocks still reaches `http_body_fn`, so the body of an +/// attempt is observable even though it is never forwarded. +#[tokio::test] +async fn test_http_body_fn_observes_denied_request() { + let out = temp_file("body-fn-denied"); + let (port, _srv, _got) = spawn_body_capturing_http_server(); + + let seen: std::sync::Arc>>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + + let policy = base_policy() + .http_allow("POST 127.0.0.1/allowed") + .http_port(port) + .http_body_fn(std::sync::Arc::new(move |_m, _h, _p, body| { + seen_cb.lock().unwrap().push(body.to_vec()); + })) + .build() + .unwrap(); + + let script = post_bytes_script( + &format!("http://127.0.0.1:{}/blocked", port), + "blocked-prompt", + &out, + ); + let result = policy + .clone() + .run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!( + content.starts_with("HTTP:403"), + "expected 403, got: {}", + content + ); + let _ = std::fs::remove_file(&out); + + let captured = seen.lock().unwrap(); + assert_eq!( + captured.len(), + 1, + "body_fn should fire for a denied request" + ); + assert_eq!(captured[0], b"blocked-prompt"); +} + // NOTE (RFC #66 follow-up): injection over the HTTPS/MITM path is not covered // here — this suite has no TLS upstream or ephemeral-CA harness. The rendering // code is scheme-agnostic (service.rs runs the same `apply` after the ACL on the From 5094bf3efd819257f011408a7968638142e0fe60 Mon Sep 17 00:00:00 2001 From: Vahagn Date: Wed, 11 Nov 2026 07:54:58 +0400 Subject: [PATCH 2/3] [feat] http: tee request bodies via a single http_observer Collapse http_log_fn and http_body_fn into http_observer, one optional callback given the request method, host, path, headers and ACL verdict. The callback can return a sink, a closure handed each body chunk as the proxy forwards it. The body is teed rather than collected, so the proxy holds one chunk instead of the whole body. Move learn to the new callback, add a test that a 1 MB body reaches the sink as several frames, and update sandbox-reference.md. Signed-off-by: Vahagn --- Cargo.lock | 1 + crates/sandlock-cli/src/learn.rs | 16 +- crates/sandlock-core/Cargo.toml | 1 + crates/sandlock-core/src/http.rs | 18 ++ crates/sandlock-core/src/sandbox.rs | 20 +- crates/sandlock-core/src/sandbox/builder.rs | 47 ++--- .../src/transparent_proxy/disabled.rs | 3 +- .../src/transparent_proxy/mod.rs | 7 +- .../src/transparent_proxy/service.rs | 103 ++++++----- .../tests/integration/test_http_acl.rs | 173 +++++++++++++----- docs/sandbox-reference.md | 4 +- 11 files changed, 238 insertions(+), 155 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 67e357bf..e1419a89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1409,6 +1409,7 @@ dependencies = [ "flate2", "futures-util", "goblin", + "http", "http-body-util", "hyper", "hyper-rustls", diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 00fa7838..0a0b93ae 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -639,14 +639,14 @@ pub async fn run(args: LearnArgs) -> Result<()> { #[cfg(feature = "http")] { let http_requests_cb = Arc::clone(&observer.http_requests); - let http_log: Arc = - Arc::new(move |method, host, path| { - http_requests_cb - .lock() - .unwrap() - .insert(format!("{method} {host}{path}")); - }); - builder = builder.http_log_fn(http_log); + let http_observer: sandlock_core::http::HttpObserver = Arc::new(move |event| { + http_requests_cb + .lock() + .unwrap() + .insert(format!("{} {}{}", event.method, event.host, event.path)); + None + }); + builder = builder.http_observer(http_observer); } for path in &args.http_inject_ca { builder = builder.http_inject_ca(path); diff --git a/crates/sandlock-core/Cargo.toml b/crates/sandlock-core/Cargo.toml index 21d9275b..59ec28a3 100644 --- a/crates/sandlock-core/Cargo.toml +++ b/crates/sandlock-core/Cargo.toml @@ -16,6 +16,7 @@ syscalls = { version = "0.8", default-features = false } nix = { version = "0.29", features = ["dir", "user"] } tokio = { version = "1.53.3", features = ["rt", "net", "time", "sync", "macros", "io-util", "fs"] } tokio-util = "0.7" +http = "1" serde = { version = "1", features = ["derive"] } thiserror = "2" rand = "0.8" diff --git a/crates/sandlock-core/src/http.rs b/crates/sandlock-core/src/http.rs index 12be3398..551f8915 100644 --- a/crates/sandlock-core/src/http.rs +++ b/crates/sandlock-core/src/http.rs @@ -3,6 +3,24 @@ use serde::{Deserialize, Serialize}; use crate::error::SandboxError; use crate::network::{NetAllow, NetTarget, Protocol}; +/// One request observed by the proxy, borrowed for the duration of the call. +pub struct HttpRequestEvent<'a> { + pub method: &'a str, + /// Validated authority, never the raw `Host` header. + pub host: &'a str, + pub path: &'a str, + pub headers: &'a ::http::HeaderMap, + pub allowed: bool, +} + +/// Receives one request's body as it streams to the upstream. +pub type HttpBodySink = Box; + +/// Observes every request reaching the proxy. `Some` tees that request's body +/// into the sink, `None` leaves it untouched. +pub type HttpObserver = + std::sync::Arc Option + Send + Sync>; + /// An HTTP access control rule. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct HttpRule { diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index efeaffbb..adce8b34 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -807,13 +807,10 @@ pub struct Sandbox { /// Path to write the active MITM CA public cert (PEM) for external trust /// wiring (e.g. NODE_EXTRA_CA_CERTS). Never writes the private key. pub http_ca_out: Option, - /// Optional observation callback for HTTP learn mode. When set the proxy is - /// spawned even without ACL rules; every request is logged via this closure. + /// Optional observation callback. When set the proxy is spawned even + /// without ACL rules. #[serde(skip)] - pub(crate) http_log_fn: Option>, - /// Optional callback given the body of every proxied request. - #[serde(skip)] - pub(crate) http_body_fn: Option>, + pub(crate) http_observer: Option, // Resource limits pub max_memory: Option, @@ -976,8 +973,7 @@ impl Clone for Sandbox { http_key: self.http_key.clone(), http_inject_ca: self.http_inject_ca.clone(), http_ca_out: self.http_ca_out.clone(), - http_log_fn: self.http_log_fn.clone(), - http_body_fn: self.http_body_fn.clone(), + http_observer: self.http_observer.clone(), max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, @@ -2444,10 +2440,7 @@ impl Sandbox { ); let mut ca_inject_pem: Option>> = None; - if !self.http_allow.is_empty() - || !self.http_deny.is_empty() - || self.http_log_fn.is_some() - || self.http_body_fn.is_some() + if !self.http_allow.is_empty() || !self.http_deny.is_empty() || self.http_observer.is_some() { // Generate an ephemeral CA when injection is requested without BYO. let generate = !self.http_inject_ca.is_empty(); @@ -2481,8 +2474,7 @@ impl Sandbox { std::sync::Arc::clone(&self.inject), cert_pem, key_pem, - self.http_log_fn.clone(), - self.http_body_fn.clone(), + self.http_observer.clone(), ) .await .map_err(SandboxRuntimeError::Io)?; diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index 607d94a1..364ecde3 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -115,15 +115,10 @@ pub struct SandboxBuilder { #[cfg_attr(feature = "cli", arg(long = "http-ca-out", value_name = "PATH"))] pub http_ca_out: Option, - /// Optional observation callback for HTTP learn mode. When set, the proxy is - /// spawned even without ACL rules so every request is logged via this closure. + /// Optional observation callback. When set, the proxy is spawned even + /// without ACL rules so every request reaching it is observed. #[cfg_attr(feature = "cli", clap(skip))] - pub http_log_fn: Option>, - - /// Optional callback given the body of every request that reaches the - /// proxy, in addition to method/host/path. - #[cfg_attr(feature = "cli", clap(skip))] - pub http_body_fn: Option>, + pub http_observer: Option, // max_memory uses a string in the CLI (e.g. "512M"); not directly clap-friendly as ByteSize. #[cfg_attr(feature = "cli", clap(skip))] @@ -300,8 +295,7 @@ impl Default for SandboxBuilder { http_key: None, http_inject_ca: Vec::new(), http_ca_out: None, - http_log_fn: None, - http_body_fn: None, + http_observer: None, max_memory: None, max_processes: None, max_open_files: None, @@ -368,8 +362,7 @@ impl Clone for SandboxBuilder { http_key: self.http_key.clone(), http_inject_ca: self.http_inject_ca.clone(), http_ca_out: self.http_ca_out.clone(), - http_log_fn: self.http_log_fn.clone(), - http_body_fn: self.http_body_fn.clone(), + http_observer: self.http_observer.clone(), max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, @@ -614,21 +607,12 @@ impl SandboxBuilder { self } - pub fn http_log_fn( - mut self, - f: std::sync::Arc, - ) -> Self { - self.http_log_fn = Some(f); - self - } - - /// Observe the body of every request that reaches the proxy, with its - /// method, host and path. Independent of [`Self::http_log_fn`]. - pub fn http_body_fn( - mut self, - f: std::sync::Arc, - ) -> Self { - self.http_body_fn = Some(f); + /// Observe every request that reaches the proxy, allowed or denied. + /// + /// The callback returns [`Some`] to receive that request's body as it + /// streams to the upstream, or [`None`] to skip it. + pub fn http_observer(mut self, f: crate::http::HttpObserver) -> Self { + self.http_observer = Some(f); self } @@ -1101,8 +1085,7 @@ impl SandboxBuilder { ("--http-ca-out", self.http_ca_out.is_some()), ("--http-auth", !self.http_auth.is_empty()), ("--credential", !self.credentials.is_empty()), - ("http_log_fn", self.http_log_fn.is_some()), - ("http_body_fn", self.http_body_fn.is_some()), + ("http_observer", self.http_observer.is_some()), ]; if let Some((what, _)) = set.iter().find(|(_, on)| *on) { return Err(SandboxError::FeatureDisabled { @@ -1124,8 +1107,7 @@ impl SandboxBuilder { // callback exist. let has_http_rules = !self.http_allow.is_empty() || !self.http_deny.is_empty() - || self.http_log_fn.is_some() - || self.http_body_fn.is_some(); + || self.http_observer.is_some(); if !self.http_inject_ca.is_empty() && !has_http_rules { return Err(SandboxError::Invalid( "--http-inject-ca requires --http-allow, --http-deny, or an observation callback" @@ -1291,8 +1273,7 @@ impl SandboxBuilder { http_key: self.http_key, http_inject_ca: self.http_inject_ca, http_ca_out: self.http_ca_out, - http_log_fn: self.http_log_fn, - http_body_fn: self.http_body_fn, + http_observer: self.http_observer, max_memory: self.max_memory, max_processes: self.max_processes, max_open_files: self.max_open_files, diff --git a/crates/sandlock-core/src/transparent_proxy/disabled.rs b/crates/sandlock-core/src/transparent_proxy/disabled.rs index ea2b6f96..1f63cc52 100644 --- a/crates/sandlock-core/src/transparent_proxy/disabled.rs +++ b/crates/sandlock-core/src/transparent_proxy/disabled.rs @@ -43,8 +43,7 @@ pub(crate) async fn spawn_transparent_proxy( _: Arc>, _: Option<&str>, _: Option<&str>, - _: Option>, - _: Option>, + _: Option, ) -> io::Result { Err(disabled()) } diff --git a/crates/sandlock-core/src/transparent_proxy/mod.rs b/crates/sandlock-core/src/transparent_proxy/mod.rs index 5751521c..8eaec478 100644 --- a/crates/sandlock-core/src/transparent_proxy/mod.rs +++ b/crates/sandlock-core/src/transparent_proxy/mod.rs @@ -52,8 +52,7 @@ pub(crate) async fn spawn_transparent_proxy( inject: Arc>, ca_cert_pem: Option<&str>, ca_key_pem: Option<&str>, - log_fn: Option>, - body_fn: Option>, + observer: Option, ) -> std::io::Result { let orig_dest: OrigDestMap = Arc::new(std::sync::RwLock::new(std::collections::HashMap::new())); let forwarder = Forwarder::new()?; @@ -63,8 +62,7 @@ pub(crate) async fn spawn_transparent_proxy( inject, Arc::clone(&orig_dest), forwarder, - log_fn, - body_fn, + observer, ); let signer = match (ca_cert_pem, ca_key_pem) { @@ -197,7 +195,6 @@ mod tests { Some(&ca.cert_pem), Some(&ca.key_pem), None, - None, ) .await .expect("proxy spawns"); diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index dba227e8..cdf46f04 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -16,7 +16,7 @@ use tokio::sync::Mutex; use super::upstream::{box_incoming, Forwarder}; use crate::credential::InjectRule; -use crate::http::{http_acl_check, HttpRule}; +use crate::http::{http_acl_check, HttpBodySink, HttpObserver, HttpRequestEvent, HttpRule}; type BoxError = Box; pub(crate) type OrigDestMap = Arc>>; @@ -41,19 +41,48 @@ pub(crate) struct AclService { /// so a library/API caller gets the warning once per run instead of per /// request. See [`first_cleartext_warn`]. cleartext_warned: Arc, - /// Optional observation callback for learn mode. Called with (method, host, path) - /// for every request after the host is validated, before the ACL check. - pub(crate) log_fn: Option>, - /// Optional callback given the full request body, in addition to - /// (method, host, path). - pub(crate) body_fn: Option>, + /// Called once per request, after the ACL has decided it. + pub(crate) observer: Option, } -/// Truncate to the cap handed to `body_fn`, since the untrusted workload could -/// otherwise force an arbitrarily large allocation into the callback's own storage. -fn capped_for_body_fn(body: &[u8]) -> &[u8] { - const MAX_BODY_FN_BYTES: usize = 10 * 1024 * 1024; - &body[..body.len().min(MAX_BODY_FN_BYTES)] +/// Copies each body chunk to a sink on its way to the upstream. Driven by the +/// forward, so nothing is accumulated and an unforwarded body is unread. +struct TeeBody { + inner: B, + sink: HttpBodySink, +} + +impl hyper::body::Body for TeeBody +where + B: hyper::body::Body + Unpin, +{ + type Data = Bytes; + type Error = B::Error; + + fn poll_frame( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll, Self::Error>>> { + let this = self.get_mut(); + match std::pin::Pin::new(&mut this.inner).poll_frame(cx) { + std::task::Poll::Ready(Some(Ok(frame))) => { + if let Some(data) = frame.data_ref() { + (this.sink)(data); + } + std::task::Poll::Ready(Some(Ok(frame))) + } + other => other, + } + } + + /// Forwarded, or the upstream request loses the length and switches to chunked. + fn size_hint(&self) -> hyper::body::SizeHint { + self.inner.size_hint() + } + + fn is_end_stream(&self) -> bool { + self.inner.is_end_stream() + } } /// Whether this cleartext injection should emit the one-per-run warning: true the @@ -113,8 +142,7 @@ impl AclService { inject: Arc>, orig_dest: OrigDestMap, forwarder: Forwarder, - log_fn: Option>, - body_fn: Option>, + observer: Option, ) -> Self { Self { allow: Arc::new(allow), @@ -124,8 +152,7 @@ impl AclService { forwarder, dns_cache: Arc::new(Mutex::new(HashMap::new())), cleartext_warned: Arc::new(AtomicBool::new(false)), - log_fn, - body_fn, + observer, } } @@ -212,37 +239,31 @@ impl AclService { m.remove(&client_addr); } - if let Some(ref f) = self.log_fn { - f(&method, &host, &path); + let allowed = http_acl_check(&self.allow, &self.deny, &method, &host, &path); + + let sink = match self.observer { + Some(ref f) => f(&HttpRequestEvent { + method: &method, + host: &host, + path: &path, + headers: req.headers(), + allowed, + }), + None => None, + }; + + if !allowed { + return text_response(StatusCode::FORBIDDEN, "Blocked by sandlock HTTP ACL policy"); } let (mut parts, body) = req.into_parts(); - - // Buffer the body once as `Bytes` (cheap to share) before forwarding. - // A read failure rejects the request: the headers still carry the - // original Content-Length, so forwarding a short body would stall the - // upstream waiting for bytes that never arrive. - let body = match self.body_fn { - Some(ref f) => { - let body_bytes = match body.collect().await { - Ok(collected) => collected.to_bytes(), - Err(_) => { - return text_response( - StatusCode::BAD_GATEWAY, - "sandlock could not read the request body", - ) - } - }; - f(&method, &host, &path, capped_for_body_fn(&body_bytes)); - Full::new(body_bytes).map_err(|e| match e {}).boxed() - } + let body = match sink { + Some(sink) => TeeBody { inner: body, sink } + .map_err(|e| Box::new(e) as BoxError) + .boxed(), None => box_incoming(body), }; - if !http_acl_check(&self.allow, &self.deny, &method, &host, &path) { - return text_response(StatusCode::FORBIDDEN, "Blocked by sandlock HTTP ACL policy"); - } - // Rebuild an absolute-URI request for the upstream client from the SAME // validated authority — never the raw Host header — so the forward host // equals the host the ACL/credential match ran against. diff --git a/crates/sandlock-core/tests/integration/test_http_acl.rs b/crates/sandlock-core/tests/integration/test_http_acl.rs index 75822694..e73834d7 100644 --- a/crates/sandlock-core/tests/integration/test_http_acl.rs +++ b/crates/sandlock-core/tests/integration/test_http_acl.rs @@ -802,7 +802,7 @@ async fn test_env_sourced_credential_stripped_from_child() { } // ============================================================ -// Observation callbacks: http_log_fn / http_body_fn +// Observation callback: http_observer // ============================================================ fn post_bytes_script(url: &str, body: &str, out: &std::path::Path) -> String { @@ -825,11 +825,10 @@ fn post_bytes_script(url: &str, body: &str, out: &std::path::Path) -> String { ) } -/// `http_log_fn` observes method/host/path for every request that reaches -/// the proxy, in addition to whatever ACL/forwarding decision it makes. +/// An observer returning `None` still sees the request line and headers. #[tokio::test] -async fn test_http_log_fn_records_requests() { - let out = temp_file("log-fn-records"); +async fn test_http_observer_records_requests() { + let out = temp_file("observer-records"); let (port, srv) = spawn_http_server(1); let seen: std::sync::Arc>> = @@ -839,11 +838,13 @@ async fn test_http_log_fn_records_requests() { let policy = base_policy() .http_allow("GET 127.0.0.1/*") .http_port(port) - .http_log_fn(std::sync::Arc::new(move |method, host, path| { - seen_cb - .lock() - .unwrap() - .push(format!("{method} {host}{path}")); + .http_observer(std::sync::Arc::new(move |event| { + let ua = event.headers.contains_key("user-agent"); + seen_cb.lock().unwrap().push(format!( + "{} {}{} allowed={} ua={}", + event.method, event.host, event.path, event.allowed, ua + )); + None })) .build() .unwrap(); @@ -861,12 +862,14 @@ async fn test_http_log_fn_records_requests() { srv.join().unwrap(); let _ = std::fs::remove_file(&out); - assert_eq!(seen.lock().unwrap().as_slice(), ["GET 127.0.0.1/get"]); + assert_eq!( + seen.lock().unwrap().as_slice(), + ["GET 127.0.0.1/get allowed=true ua=true"] + ); } -/// Spawn a server that records the body of the one request it receives, for -/// `http_body_fn` tests that need to tell the callback's view and the -/// upstream's view of a request apart. +/// Records the body of the one request it receives, so a test can tell the +/// sink's view and the upstream's view apart. fn spawn_body_capturing_http_server() -> ( u16, thread::JoinHandle<()>, @@ -909,34 +912,35 @@ fn spawn_body_capturing_http_server() -> ( (port, handle, captured) } -/// `http_body_fn` sees exactly the bytes the child sent, and the upstream -/// still receives the same, unmodified bytes. +/// The sink receives the body, and the upstream receives it unmodified. #[tokio::test] -async fn test_http_body_fn_receives_request_body() { - let out = temp_file("body-fn-receives"); +async fn test_http_observer_sink_receives_body() { + let out = temp_file("observer-sink"); let (port, srv, upstream_got) = spawn_body_capturing_http_server(); - let seen: std::sync::Arc)>>> = + let teed: std::sync::Arc>> = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let seen_cb = seen.clone(); + let teed_cb = teed.clone(); let policy = base_policy() .http_allow("POST 127.0.0.1/*") .http_port(port) - .http_body_fn(std::sync::Arc::new(move |method, host, path, body| { - seen_cb.lock().unwrap().push(( - method.to_string(), - host.to_string(), - path.to_string(), - body.to_vec(), - )); + .http_observer(std::sync::Arc::new(move |event| { + assert_eq!(event.method, "POST"); + assert_eq!(event.host, "127.0.0.1"); + assert_eq!(event.path, "/upload"); + assert!(event.allowed); + let acc = teed_cb.clone(); + Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().extend_from_slice(chunk); + })) })) .build() .unwrap(); let script = post_bytes_script( &format!("http://127.0.0.1:{}/upload", port), - "hello-body-fn", + "hello-observer", &out, ); let result = policy @@ -951,37 +955,40 @@ async fn test_http_body_fn_receives_request_body() { srv.join().unwrap(); let _ = std::fs::remove_file(&out); - let captured = seen.lock().unwrap(); - assert_eq!(captured.len(), 1, "callback should fire exactly once"); - let (method, host, path, body) = &captured[0]; - assert_eq!(method, "POST"); - assert_eq!(host, "127.0.0.1"); - assert_eq!(path, "/upload"); - assert_eq!(body, b"hello-body-fn"); - + assert_eq!( + teed.lock().unwrap().as_slice(), + b"hello-observer", + "the sink must see the body the child sent" + ); assert_eq!( upstream_got.lock().unwrap().as_deref(), - Some(b"hello-body-fn".as_slice()), + Some(b"hello-observer".as_slice()), "upstream must still receive the unmodified body" ); } -/// A request the ACL blocks still reaches `http_body_fn`, so the body of an -/// attempt is observable even though it is never forwarded. +/// A blocked request is reported with `allowed` false, and its body unread. #[tokio::test] -async fn test_http_body_fn_observes_denied_request() { - let out = temp_file("body-fn-denied"); +async fn test_http_observer_reports_denied_request() { + let out = temp_file("observer-denied"); let (port, _srv, _got) = spawn_body_capturing_http_server(); - let seen: std::sync::Arc>>> = + let verdicts: std::sync::Arc>> = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); - let seen_cb = seen.clone(); + let teed: std::sync::Arc>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let verdicts_cb = verdicts.clone(); + let teed_cb = teed.clone(); let policy = base_policy() .http_allow("POST 127.0.0.1/allowed") .http_port(port) - .http_body_fn(std::sync::Arc::new(move |_m, _h, _p, body| { - seen_cb.lock().unwrap().push(body.to_vec()); + .http_observer(std::sync::Arc::new(move |event| { + verdicts_cb.lock().unwrap().push(event.allowed); + let acc = teed_cb.clone(); + Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().extend_from_slice(chunk); + })) })) .build() .unwrap(); @@ -1005,13 +1012,79 @@ async fn test_http_body_fn_observes_denied_request() { ); let _ = std::fs::remove_file(&out); - let captured = seen.lock().unwrap(); assert_eq!( - captured.len(), - 1, - "body_fn should fire for a denied request" + verdicts.lock().unwrap().as_slice(), + [false], + "the denied request must be reported once, with allowed=false" + ); + assert!( + teed.lock().unwrap().is_empty(), + "a denied request is never forwarded, so its body is never read" + ); +} + +/// A large body reaches the sink as several frames, and upstream whole. +#[tokio::test] +async fn test_http_observer_sink_receives_streamed_chunks() { + let out = temp_file("observer-chunks"); + let (port, srv, upstream_got) = spawn_body_capturing_http_server(); + + let chunks: std::sync::Arc>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let chunks_cb = chunks.clone(); + + let policy = base_policy() + .http_allow("POST 127.0.0.1/*") + .http_port(port) + .http_observer(std::sync::Arc::new(move |_event| { + let acc = chunks_cb.clone(); + Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().push(chunk.len()); + })) + })) + .build() + .unwrap(); + + let size = 1_000_000usize; + let script = format!( + concat!( + "import urllib.request, urllib.error\n", + "body = b'x' * {size}\n", + "try:\n", + " req = urllib.request.Request('{url}', method='POST', data=body)\n", + " resp = urllib.request.urlopen(req)\n", + " open('{out}', 'w').write('OK:' + str(resp.status))\n", + "except Exception as e:\n", + " open('{out}', 'w').write('ERR:' + str(e))\n", + ), + size = size, + url = format!("http://127.0.0.1:{}/upload", port), + out = out.display(), + ); + let result = policy + .clone() + .run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!(content.starts_with("OK:200"), "got: {}", content); + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); + + let seen = chunks.lock().unwrap(); + let total: usize = seen.iter().sum(); + assert_eq!(total, size, "sink must see every byte"); + assert!( + seen.len() > 1, + "expected several frames, got {}", + seen.len() + ); + assert_eq!( + upstream_got.lock().unwrap().as_ref().map(|b| b.len()), + Some(size), + "upstream must receive the whole body" ); - assert_eq!(captured[0], b"blocked-prompt"); } // NOTE (RFC #66 follow-up): injection over the HTTPS/MITM path is not covered diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index e91e47db..d9fef2a7 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -132,8 +132,8 @@ Top-level configuration for the supervisor and COW workspace. | ------------ | ----------- | ------------- | ------- | ---------------------------------------------------------------------------------------- | | `http_ca` | `http_ca` | `str \| None` | `None` | PEM CA certificate path for HTTPS MITM. When set, port `443` is added to `http_ports`. | | `http_key` | `http_key` | `str \| None` | `None` | PEM CA private key path. Required whenever `http_ca` is set. | -| `http_inject_ca` | `http_inject_ca` | `list[str]` | `[]` | Trust bundle paths to splice the active MITM CA's public cert into at open time. Without `http_ca`, generates an ephemeral CA (private key in memory only, never on disk) and intercepts port `443`. Requires at least one `http_allow` / `http_deny` rule. | -| `http_ca_out` | `http_ca_out` | `str \| None` | `None` | Writes the active CA's public certificate (PEM) to this path; never the private key. Requires at least one `http_allow` / `http_deny` rule. | +| `http_inject_ca` | `http_inject_ca` | `list[str]` | `[]` | Trust bundle paths to splice the active MITM CA's public cert into at open time. Without `http_ca`, generates an ephemeral CA (private key in memory only, never on disk) and intercepts port `443`. Requires at least one `http_allow` / `http_deny` rule, or an observation callback. | +| `http_ca_out` | `http_ca_out` | `str \| None` | `None` | Writes the active CA's public certificate (PEM) to this path; never the private key. Requires at least one `http_allow` / `http_deny` rule, or an observation callback. | | `fs_storage` | `fs_storage`| `str \| None` | `None` | Separate storage directory for the seccomp COW upper layer / deltas. | | `workdir` | `workdir` | `str \| None` | `None` | COW root directory. Controls which directory COW tracks; does **not** set the child's working directory. | From 287ef279d7d304ff47356b80cc874c954a263267 Mon Sep 17 00:00:00 2001 From: Vahagn Date: Wed, 11 Nov 2026 19:29:04 +0400 Subject: [PATCH 3/3] [feat] http: observe response status and bodies Add a response slot to HttpSinks, next to the request sink the observer already returns. It holds a hook called once the status and headers exist, which may then return a sink to tee that response's body. TeeBody is reused unchanged for that direction. Re-export HeaderMap so an observer can name the type without depending on http, and add a test that the hook sees the status and its sink receives the body. Signed-off-by: Vahagn --- crates/sandlock-cli/src/learn.rs | 2 +- crates/sandlock-core/src/http.rs | 33 ++++++- .../src/transparent_proxy/service.rs | 36 ++++++-- .../tests/integration/test_http_acl.rs | 91 +++++++++++++++++-- 4 files changed, 139 insertions(+), 23 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 0a0b93ae..1af12803 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -644,7 +644,7 @@ pub async fn run(args: LearnArgs) -> Result<()> { .lock() .unwrap() .insert(format!("{} {}{}", event.method, event.host, event.path)); - None + sandlock_core::http::HttpSinks::default() }); builder = builder.http_observer(http_observer); } diff --git a/crates/sandlock-core/src/http.rs b/crates/sandlock-core/src/http.rs index 551f8915..526e061b 100644 --- a/crates/sandlock-core/src/http.rs +++ b/crates/sandlock-core/src/http.rs @@ -3,6 +3,9 @@ use serde::{Deserialize, Serialize}; use crate::error::SandboxError; use crate::network::{NetAllow, NetTarget, Protocol}; +/// Re-exported so an observer can name the type without depending on `http`. +pub use ::http::HeaderMap; + /// One request observed by the proxy, borrowed for the duration of the call. pub struct HttpRequestEvent<'a> { pub method: &'a str, @@ -13,13 +16,33 @@ pub struct HttpRequestEvent<'a> { pub allowed: bool, } -/// Receives one request's body as it streams to the upstream. +/// One upstream response, borrowed for the duration of the call. +pub struct HttpResponseEvent<'a> { + pub status: u16, + pub headers: &'a ::http::HeaderMap, +} + +/// Receives one body as it streams, in either direction. pub type HttpBodySink = Box; -/// Observes every request reaching the proxy. `Some` tees that request's body -/// into the sink, `None` leaves it untouched. -pub type HttpObserver = - std::sync::Arc Option + Send + Sync>; +/// Called once, when the response status and headers exist, which is later than +/// the observer runs. Returns a sink to tee that response's body, or `None`. +pub type HttpResponseHook = Box Option + Send>; + +/// What an observer asks for. Both sinks are chosen while the request is still +/// in scope, so a consumer correlates the two bodies by what it captures. +#[derive(Default)] +pub struct HttpSinks { + /// `Some` tees the request body, `None` leaves it untouched. + pub request: Option, + /// `Some` is called with the upstream response, and may then tee its body. + /// A denied request has no upstream response, so this is never called; the + /// child still receives sandlock's 403. + pub response: Option, +} + +/// Observes every request reaching the proxy. +pub type HttpObserver = std::sync::Arc HttpSinks + Send + Sync>; /// An HTTP access control rule. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index cdf46f04..69ec92ea 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -16,7 +16,10 @@ use tokio::sync::Mutex; use super::upstream::{box_incoming, Forwarder}; use crate::credential::InjectRule; -use crate::http::{http_acl_check, HttpBodySink, HttpObserver, HttpRequestEvent, HttpRule}; +use crate::http::{ + http_acl_check, HttpBodySink, HttpObserver, HttpRequestEvent, HttpResponseEvent, HttpRule, + HttpSinks, +}; type BoxError = Box; pub(crate) type OrigDestMap = Arc>>; @@ -241,7 +244,7 @@ impl AclService { let allowed = http_acl_check(&self.allow, &self.deny, &method, &host, &path); - let sink = match self.observer { + let sinks = match self.observer { Some(ref f) => f(&HttpRequestEvent { method: &method, host: &host, @@ -249,15 +252,20 @@ impl AclService { headers: req.headers(), allowed, }), - None => None, + None => HttpSinks::default(), }; if !allowed { return text_response(StatusCode::FORBIDDEN, "Blocked by sandlock HTTP ACL policy"); } + let HttpSinks { + request: req_sink, + response: resp_hook, + } = sinks; + let (mut parts, body) = req.into_parts(); - let body = match sink { + let body = match req_sink { Some(sink) => TeeBody { inner: body, sink } .map_err(|e| Box::new(e) as BoxError) .boxed(), @@ -341,10 +349,24 @@ impl AclService { let out_req = Request::from_parts(parts, body); - match self.forwarder.forward(out_req).await { + let resp = match self.forwarder.forward(out_req).await { Ok(resp) => resp, - Err(_) => text_response(StatusCode::BAD_GATEWAY, "upstream error"), - } + Err(_) => return text_response(StatusCode::BAD_GATEWAY, "upstream error"), + }; + + let Some(hook) = resp_hook else { + return resp; + }; + let (parts, body) = resp.into_parts(); + let sink = hook(&HttpResponseEvent { + status: parts.status.as_u16(), + headers: &parts.headers, + }); + let body = match sink { + Some(sink) => TeeBody { inner: body, sink }.boxed(), + None => body, + }; + Response::from_parts(parts, body) } } diff --git a/crates/sandlock-core/tests/integration/test_http_acl.rs b/crates/sandlock-core/tests/integration/test_http_acl.rs index e73834d7..c532bf06 100644 --- a/crates/sandlock-core/tests/integration/test_http_acl.rs +++ b/crates/sandlock-core/tests/integration/test_http_acl.rs @@ -844,7 +844,7 @@ async fn test_http_observer_records_requests() { "{} {}{} allowed={} ua={}", event.method, event.host, event.path, event.allowed, ua )); - None + sandlock_core::http::HttpSinks::default() })) .build() .unwrap(); @@ -931,9 +931,12 @@ async fn test_http_observer_sink_receives_body() { assert_eq!(event.path, "/upload"); assert!(event.allowed); let acc = teed_cb.clone(); - Some(Box::new(move |chunk: &[u8]| { - acc.lock().unwrap().extend_from_slice(chunk); - })) + sandlock_core::http::HttpSinks { + request: Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().extend_from_slice(chunk); + })), + response: None, + } })) .build() .unwrap(); @@ -986,9 +989,12 @@ async fn test_http_observer_reports_denied_request() { .http_observer(std::sync::Arc::new(move |event| { verdicts_cb.lock().unwrap().push(event.allowed); let acc = teed_cb.clone(); - Some(Box::new(move |chunk: &[u8]| { - acc.lock().unwrap().extend_from_slice(chunk); - })) + sandlock_core::http::HttpSinks { + request: Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().extend_from_slice(chunk); + })), + response: None, + } })) .build() .unwrap(); @@ -1038,9 +1044,12 @@ async fn test_http_observer_sink_receives_streamed_chunks() { .http_port(port) .http_observer(std::sync::Arc::new(move |_event| { let acc = chunks_cb.clone(); - Some(Box::new(move |chunk: &[u8]| { - acc.lock().unwrap().push(chunk.len()); - })) + sandlock_core::http::HttpSinks { + request: Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().push(chunk.len()); + })), + response: None, + } })) .build() .unwrap(); @@ -1087,6 +1096,68 @@ async fn test_http_observer_sink_receives_streamed_chunks() { ); } +/// The response hook sees the upstream status, and its sink receives the +/// response body. +#[tokio::test] +async fn test_http_observer_response_hook_tees_body() { + let out = temp_file("observer-response"); + let (port, srv, _upstream_got) = spawn_body_capturing_http_server(); + + let status: std::sync::Arc>> = + std::sync::Arc::new(std::sync::Mutex::new(None)); + let resp_body: std::sync::Arc>> = + std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let status_cb = status.clone(); + let resp_body_cb = resp_body.clone(); + + let policy = base_policy() + .http_allow("POST 127.0.0.1/*") + .http_port(port) + .http_observer(std::sync::Arc::new(move |_event| { + let seen_status = status_cb.clone(); + let acc = resp_body_cb.clone(); + sandlock_core::http::HttpSinks { + request: None, + response: Some(Box::new(move |resp| { + *seen_status.lock().unwrap() = Some(resp.status); + Some(Box::new(move |chunk: &[u8]| { + acc.lock().unwrap().extend_from_slice(chunk); + })) + })), + } + })) + .build() + .unwrap(); + + let script = post_bytes_script( + &format!("http://127.0.0.1:{}/upload", port), + "hello-response", + &out, + ); + let result = policy + .clone() + .run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!(content.starts_with("OK:200"), "got: {}", content); + + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); + + assert_eq!( + *status.lock().unwrap(), + Some(200), + "the hook must see the upstream status" + ); + assert_eq!( + resp_body.lock().unwrap().as_slice(), + b"ok", + "the sink must see the response body" + ); +} + // NOTE (RFC #66 follow-up): injection over the HTTPS/MITM path is not covered // here — this suite has no TLS upstream or ephemeral-CA harness. The rendering // code is scheme-agnostic (service.rs runs the same `apply` after the ACL on the