From a0eeac89378bbc8095b38204776dadb0b37947e5 Mon Sep 17 00:00:00 2001 From: dzerik Date: Mon, 6 Jul 2026 12:56:05 +0300 Subject: [PATCH 1/5] =?UTF-8?q?feat(net):=20credential=20injection=20primi?= =?UTF-8?q?tives=20=E2=80=94=20inject=20an=20auth=20secret=20in=20the=20pr?= =?UTF-8?q?oxy=20(RFC=20#66)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1, Layer 2 (transparent mode): a named secret lives only in the supervisor and is rendered into a matching request's auth header (or query parameter) inside the MITM proxy — after the ACL check, so a denied request never touches the secret — while the sandboxed child never sees the real value. New `credential` module: - `SecretString`: zeroed on drop (volatile writes), redacted `Debug`, and deliberately no `Display`/`ToString`/`Serialize` and unreachable through any `Error` chain, so a stray `format!("{}", ..)` can't re-open the leak. - Sources `env:`/`file:`/`fd:` (a trailing newline is stripped); `literal:` is rejected — it leaks via ps / shell history. `file:`/`fd:` are capped at 64 KiB and `fd:` refuses the std streams (0/1/2) and reads through a dup. - `AuthShape` renderings: `bearer`, `basic:` (base64), `header:` / `apikey:`, `query:`. Injected headers are marked sensitive; a secret that can't render (e.g. CRLF) fails the request rather than forwarding it uncredentialed. - `OnExistingHeader` defaults to `Replace`: the proxy owns the credential, and SDKs (openai, anthropic, …) always send a placeholder `Authorization`, so keeping it would forward the placeholder and never inject. Pass the trailing `add-only` token to keep a value the child set instead. Wiring: `--credential NAME=SOURCE` and `--http-inject "METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]"` flow through the builder to a resolved `Arc>` on the sandbox (never serialized — `#[serde(skip)]`; secrets are re-loaded from source per build, not persisted). `AclService` applies the first matching rule to the outbound request after `http_acl_check` and records the credential *name* — never its value. `--http-inject` requires an HTTP ACL proxy (rejected otherwise); with no CA it warns that only plaintext HTTP is intercepted. A checkpoint is rejected while injection is configured (the non-serialized secrets can't be restored into the image). Env-sourced secrets are stripped from the child's environment post-fork, so the agent can't read the real value straight out of its own env instead of relying on proxy-side injection. Deferred to later slices: `--service openai` (Layer 1), phantom-token env swap, explicit proxy mode, body-level injection, and a structured audit log (the current audit is a by-name stderr line). Tests: unit coverage of `SecretString` redaction, source loading (incl. `literal:`/std-fd rejection and the 64 KiB cap), every `AuthShape`, the `Replace` default vs `add-only`, query encoding, a CRLF secret failing the request, and rule parsing incl. Arc dedup + env-strip collection; integration tests that a credential is injected into the upstream while the child carries none, that a denied request never renders the secret, and that an `env:` credential is scrubbed from the child. HTTPS/MITM injection is exercised by the same scheme-agnostic path but a TLS end-to-end test is a follow-up. --- crates/sandlock-cli/src/main.rs | 2 + .../sandlock-core/src/checkpoint/capture.rs | 12 + crates/sandlock-core/src/context.rs | 6 + crates/sandlock-core/src/credential.rs | 620 ++++++++++++++++++ crates/sandlock-core/src/lib.rs | 1 + crates/sandlock-core/src/sandbox.rs | 15 + crates/sandlock-core/src/sandbox/builder.rs | 73 +++ .../src/transparent_proxy/mod.rs | 5 +- .../src/transparent_proxy/service.rs | 31 + .../tests/integration/test_http_acl.rs | 151 +++++ 10 files changed, 914 insertions(+), 2 deletions(-) create mode 100644 crates/sandlock-core/src/credential.rs diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index ad973bf3..31bdae15 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -461,6 +461,8 @@ async fn run_command(args: RunArgs) -> Result { if !pb.extra_deny_syscalls.is_empty() { builder = builder.extra_deny_syscalls(pb.extra_deny_syscalls.clone()); } for rule in &pb.http_allow { builder = builder.http_allow(rule); } for rule in &pb.http_deny { builder = builder.http_deny(rule); } + for c in &pb.credentials { builder = builder.credential_spec(c); } + for rule in &pb.http_inject { builder = builder.http_inject(rule); } for port in &pb.http_ports { builder = builder.http_port(*port); } if let Some(ref ca) = pb.http_ca { builder = builder.http_ca(ca); } if let Some(ref key) = pb.http_key { builder = builder.http_key(key); } diff --git a/crates/sandlock-core/src/checkpoint/capture.rs b/crates/sandlock-core/src/checkpoint/capture.rs index 3c674e9e..e58f9a82 100644 --- a/crates/sandlock-core/src/checkpoint/capture.rs +++ b/crates/sandlock-core/src/checkpoint/capture.rs @@ -297,6 +297,18 @@ fn parse_fdinfo(pid: i32, fd: i32) -> io::Result<(i32, u64)> { /// Capture a checkpoint from a running, stopped sandbox. /// The sandbox must already be frozen (SIGSTOP'd and fork-held). pub(crate) fn capture(pid: i32, policy: &Sandbox) -> Result { + // Credential-injection rules hold supervisor-only secrets that are + // deliberately not serialized (`#[serde(skip)]`). A checkpoint image would + // therefore restore with no injection rules and send every request + // uncredentialed — reject rather than silently drop them. + if !policy.inject.is_empty() { + return Err(SandlockError::Runtime(SandboxRuntimeError::Child( + "checkpoint is not supported with credential injection (--http-inject); \ + the injected secrets cannot be serialized into the image" + .into(), + ))); + } + // Seize via ptrace (PTRACE_SEIZE + PTRACE_INTERRUPT -- doesn't auto-SIGSTOP) ptrace_seize(pid).map_err(|e| { SandlockError::Runtime(SandboxRuntimeError::Child(format!("ptrace seize: {}", e))) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 9d41ce32..70921f02 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -501,6 +501,12 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { for (key, value) in &sandbox.env { std::env::set_var(key, value); } + // Remove env vars whose value was loaded into the supervisor as a credential + // source, so the agent can't read the real secret straight from its own + // environment (this is the child; the mutation is process-local and post-fork). + for name in &sandbox.inject_env_strip { + std::env::remove_var(name); + } // 13b. GPU device visibility if let Some(ref devices) = sandbox.gpu_devices { diff --git a/crates/sandlock-core/src/credential.rs b/crates/sandlock-core/src/credential.rs new file mode 100644 index 00000000..b67dcde8 --- /dev/null +++ b/crates/sandlock-core/src/credential.rs @@ -0,0 +1,620 @@ +//! Credential injection (RFC #66, Phase 1 — Layer 2 primitives, transparent mode). +//! +//! A named secret lives only in the supervisor. Requests the sandboxed child +//! makes to a matching upstream have the secret rendered into an auth header (or +//! query parameter) inside the MITM proxy — after the ACL check, so a denied +//! request never touches the secret — and the child never sees the real value. +//! +//! This module holds the primitives: [`SecretString`] (zeroed on drop, never +//! printable), the [`AuthShape`] renderings, source loading (`env:`/`file:`/ +//! `fd:`), and [`InjectRule`] (a matcher + auth shape + secret). Higher layers +//! (`--service openai`) and the phantom-token swap build on top of these. + +use std::io::Read; +use std::ptr; +use std::sync::Arc; + +use crate::error::SandboxError; +use crate::http::HttpRule; + +/// A secret held in the supervisor, zeroed on drop. +/// +/// It deliberately does **not** implement `Display`, `ToString`, or +/// `serde::Serialize`, and is not reachable through any `std::error::Error` +/// chain — so a stray `format!("{}", ..)` or a serialized policy cannot re-open +/// the leak the type exists to close. Only [`SecretString::expose`], used at the +/// single point where the value is rendered into an outbound request, reaches +/// the bytes. +pub struct SecretString(Vec); + +impl SecretString { + pub fn new(bytes: Vec) -> Self { + Self(bytes) + } + + /// The raw secret bytes. Call this only to render the value into an outbound + /// header/query at send time — never to log, store, or return it. + fn expose(&self) -> &[u8] { + &self.0 + } +} + +impl Drop for SecretString { + fn drop(&mut self) { + // Volatile writes so the zeroing can't be optimized away. + for b in self.0.iter_mut() { + unsafe { ptr::write_volatile(b as *mut u8, 0) }; + } + } +} + +impl std::fmt::Debug for SecretString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("SecretString()") + } +} + +/// Load a secret into the supervisor from a source spec. `literal:` is +/// intentionally unsupported — an inline value leaks via `ps` and shell history +/// even when used correctly; use `env:`/`file:`/`fd:`. +/// +/// A single trailing newline (`\n` or `\r\n`) is stripped, since files and +/// heredocs commonly add one. +pub fn load_secret(source: &str) -> Result { + let (kind, val) = source.split_once(':').ok_or_else(|| { + SandboxError::Invalid(format!( + "credential source must be env:/file:/fd:<...>, got {source:?}" + )) + })?; + let mut bytes = match kind { + "env" => std::env::var_os(val) + .ok_or_else(|| SandboxError::Invalid(format!("credential env var {val} is not set")))? + .into_encoded_bytes(), + "file" => read_capped( + &mut std::fs::File::open(val) + .map_err(|e| SandboxError::Invalid(format!("credential file {val}: {e}")))?, + &format!("file {val}"), + )?, + "fd" => { + let n: i32 = val + .parse() + .map_err(|_| SandboxError::Invalid(format!("credential fd must be an integer, got {val:?}")))?; + read_fd(n)? + } + "literal" => { + return Err(SandboxError::Invalid( + "credential source 'literal:' is unsupported (it leaks via ps / shell history); \ + use env:/file:/fd:" + .into(), + )) + } + other => return Err(SandboxError::Invalid(format!("unknown credential source {other:?}"))), + }; + if bytes.last() == Some(&b'\n') { + bytes.pop(); + if bytes.last() == Some(&b'\r') { + bytes.pop(); + } + } + if bytes.is_empty() { + return Err(SandboxError::Invalid(format!("credential from {source:?} is empty"))); + } + Ok(SecretString::new(bytes)) +} + +/// Largest secret we read from a `file:`/`fd:` source. Bounds supervisor memory +/// so a hostile/careless `file:/dev/zero` or an endless `fd:` pipe can't OOM it. +const MAX_SECRET_BYTES: u64 = 64 << 10; + +/// Read at most `MAX_SECRET_BYTES` from `r`, erroring if the source is larger. +fn read_capped(r: &mut R, what: &str) -> Result, SandboxError> { + let mut buf = Vec::new(); + // Read one past the cap so an oversized source is detected, not truncated. + r.take(MAX_SECRET_BYTES + 1) + .read_to_end(&mut buf) + .map_err(|e| SandboxError::Invalid(format!("credential {what}: {e}")))?; + if buf.len() as u64 > MAX_SECRET_BYTES { + return Err(SandboxError::Invalid(format!( + "credential {what} exceeds {MAX_SECRET_BYTES} bytes" + ))); + } + Ok(buf) +} + +/// Read a credential from an already-open fd (e.g. a shell `<(...)` process +/// substitution passed as `fd:N`). Reads through a *dup*, so the caller's fd is +/// left open, and refuses the std streams (0/1/2) so a typo like `fd:1` can't +/// close/consume stdout. +fn read_fd(n: i32) -> Result, SandboxError> { + use std::os::fd::FromRawFd; + if n <= 2 { + return Err(SandboxError::Invalid(format!( + "credential fd {n} refers to a standard stream (0/1/2); pass a dedicated fd" + ))); + } + let dup = unsafe { libc::dup(n) }; + if dup < 0 { + return Err(SandboxError::Invalid(format!( + "credential fd {n}: {}", + std::io::Error::last_os_error() + ))); + } + // Owns `dup` (not `n`), so only the dup is closed on drop. + let mut f = unsafe { std::fs::File::from_raw_fd(dup) }; + read_capped(&mut f, &format!("fd {n}")) +} + +/// How the secret is attached to a matching request. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AuthShape { + /// `Authorization: Bearer `. + Bearer, + /// `Authorization: Basic base64(:)`. + Basic { username: String }, + /// A custom header carrying the raw secret, e.g. `x-api-key: `. + Header { name: String }, + /// A query parameter carrying the raw secret, e.g. `?key=`. + /// + /// Less private than the header shapes: unlike an injected header, a query + /// value can't be marked sensitive, so it lands in the upstream's access + /// logs and any `Referer`, and a request-level tracing subscriber would log + /// it. Prefer `bearer`/`header` when the upstream accepts them; use `query` + /// only for APIs that require it, and not in a traced environment. + Query { param: String }, +} + +/// What to do when the target header/param already exists on the child's request. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum OnExistingHeader { + /// Overwrite the value the child already set (default). The proxy owns the + /// credential, and every major SDK (openai, anthropic, …) *requires* an API + /// key to be set and always sends its own `Authorization: Bearer ` + /// — so keeping the child's value would forward the placeholder and never + /// inject. Replacing that one header the rule targets is what makes + /// `--http-inject "* api.openai.com/* bearer openai"` work at all. + #[default] + Replace, + /// Leave the header/param the child already set, injecting only when absent. + /// Opt in with the trailing `add-only` token when the agent legitimately owns + /// the credential and the rule is a fallback. + AddOnly, +} + +/// A credential-injection rule: match a request, then attach `secret` per `auth`. +/// `name` is the credential's declared name, recorded in the audit trail — never +/// the value. +pub struct InjectRule { + pub name: String, + pub matcher: HttpRule, + pub auth: AuthShape, + /// Shared so several rules can reference one credential without re-loading + /// its source (an `fd:` source is consumed on first read). + pub secret: Arc, + pub on_existing: OnExistingHeader, +} + +impl std::fmt::Debug for InjectRule { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InjectRule") + .field("name", &self.name) + .field("matcher", &self.matcher) + .field("auth", &self.auth) + .field("on_existing", &self.on_existing) + .field("secret", &self.secret) // redacted + .finish() + } +} + +impl InjectRule { + /// True if this rule applies to the given request. + pub fn matches(&self, method: &str, host: &str, path: &str) -> bool { + self.matcher.matches(method, host, path) + } + + /// Render the secret into `parts` (header or query), honoring `on_existing`. + /// The rendered header is marked sensitive so downstream logging redacts it. + /// + /// `Err(())` means the secret could not be rendered (e.g. it contains bytes + /// illegal in an HTTP header value) — the caller must reject the request + /// rather than forward it with no (or a partial) credential. A no-op skip + /// under `AddOnly` (the agent already set the header) is `Ok`. + /// + /// Zeroing is best-effort: [`SecretString`] itself is wiped on drop, but the + /// transient buffers built here (the `Bearer `/`Basic ` byte vecs, the base64 + /// string, the query pair) hold a copy of the secret and are dropped without + /// volatile zeroing, so a copy briefly lives in freed heap. + pub fn apply(&self, parts: &mut hyper::http::request::Parts) -> Result<(), ()> { + match &self.auth { + AuthShape::Bearer => { + let mut v = b"Bearer ".to_vec(); + v.extend_from_slice(self.secret.expose()); + self.set_header(parts, "authorization", &v) + } + AuthShape::Basic { username } => { + let mut raw = username.clone().into_bytes(); + raw.push(b':'); + raw.extend_from_slice(self.secret.expose()); + let mut v = b"Basic ".to_vec(); + v.extend_from_slice(base64_encode(&raw).as_bytes()); + self.set_header(parts, "authorization", &v) + } + AuthShape::Header { name } => self.set_header(parts, name, self.secret.expose()), + AuthShape::Query { param } => self.set_query(parts, param), + } + } + + fn set_header( + &self, + parts: &mut hyper::http::request::Parts, + name: &str, + value: &[u8], + ) -> Result<(), ()> { + let hn = hyper::header::HeaderName::from_bytes(name.as_bytes()).map_err(|_| ())?; + if self.on_existing == OnExistingHeader::AddOnly && parts.headers.contains_key(&hn) { + return Ok(()); // agent already set it — leave it, not a failure + } + let mut hv = hyper::header::HeaderValue::from_bytes(value).map_err(|_| ())?; + hv.set_sensitive(true); + parts.headers.insert(hn, hv); + Ok(()) + } + + fn set_query(&self, parts: &mut hyper::http::request::Parts, param: &str) -> Result<(), ()> { + let uri = &parts.uri; + let path = uri.path(); + let existing = uri.query(); + // Honor AddOnly: don't append a param the request already carries. + if self.on_existing == OnExistingHeader::AddOnly { + if let Some(q) = existing { + let prefix = format!("{}=", urlencode_bytes(param.as_bytes())); + if q.split('&').any(|kv| kv.starts_with(&prefix)) { + return Ok(()); + } + } + } + // Percent-encode the raw secret bytes (never lossy-stringify — a binary + // key would be corrupted). + let pair = format!( + "{}={}", + urlencode_bytes(param.as_bytes()), + urlencode_bytes(self.secret.expose()) + ); + let new_pq = match existing { + Some(q) if !q.is_empty() => format!("{path}?{q}&{pair}"), + _ => format!("{path}?{pair}"), + }; + let mut b = hyper::http::uri::Builder::new(); + if let Some(s) = uri.scheme() { + b = b.scheme(s.clone()); + } + if let Some(a) = uri.authority() { + b = b.authority(a.clone()); + } + parts.uri = b.path_and_query(new_pq).build().map_err(|_| ())?; + Ok(()) + } +} + +/// Parse an `AuthShape` from the auth token of an `--http-inject` rule: +/// `bearer` | `basic:` | `header:` | `apikey:` | `query:`. +/// Returns `(shape, credential_name)`. +pub fn parse_auth(spec: &str, credential: &str) -> Result { + let (kind, arg) = match spec.split_once(':') { + Some((k, a)) => (k, Some(a)), + None => (spec, None), + }; + let shape = match (kind, arg) { + ("bearer", None) => AuthShape::Bearer, + ("basic", Some(user)) if !user.is_empty() => AuthShape::Basic { username: user.to_string() }, + // `apikey:
` and `header:` are the same rendering. + ("header" | "apikey", Some(name)) if !name.is_empty() => AuthShape::Header { name: name.to_string() }, + ("query", Some(param)) if !param.is_empty() => AuthShape::Query { param: param.to_string() }, + _ => { + return Err(SandboxError::Invalid(format!( + "invalid auth shape {spec:?} for credential {credential:?} \ + (expected bearer | basic: | header: | apikey: | query:)" + ))) + } + }; + Ok(shape) +} + +/// Resolve `--credential`/`--http-inject` specs into ready-to-apply rules, +/// loading each secret into the supervisor. +/// +/// - `credentials`: `NAME=SOURCE` where SOURCE is `env:`/`file:`/`fd:` (see +/// [`load_secret`]). +/// - `inject`: `METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]`, e.g. +/// `"* api.openai.com/* bearer openai"` or `"GET x.com/* header:x-api-key key add-only"`. +/// AUTHSPEC is `bearer | basic: | header: | apikey: | query:`. +/// The trailing token defaults to `replace` (the proxy overwrites the +/// placeholder auth SDKs always send); pass `add-only` to keep a value the +/// child set. +/// +/// Each rule loads its own secret from the named credential's source, so a +/// credential may back several rules (an `fd:` source is single-use, so it can +/// back only one). Referencing an undeclared credential is an error. +pub fn resolve_inject_rules( + credentials: &[String], + inject: &[String], +) -> Result<(Vec, Vec), SandboxError> { + use std::collections::HashMap; + + let mut sources: HashMap<&str, &str> = HashMap::new(); + for c in credentials { + let (name, source) = c.split_once('=').ok_or_else(|| { + SandboxError::Invalid(format!("--credential must be NAME=SOURCE, got {c:?}")) + })?; + if name.is_empty() { + return Err(SandboxError::Invalid(format!("--credential has empty name: {c:?}"))); + } + if sources.insert(name, source).is_some() { + return Err(SandboxError::Invalid(format!("--credential {name:?} declared twice"))); + } + } + + // Parse every rule first so we know which credentials are actually used. + struct Parsed<'a> { + name: &'a str, + matcher: HttpRule, + auth: AuthShape, + on_existing: OnExistingHeader, + source: &'a str, + } + let mut parsed: Vec = Vec::with_capacity(inject.len()); + for spec in inject { + let toks: Vec<&str> = spec.split_whitespace().collect(); + if toks.len() < 4 { + return Err(SandboxError::Invalid(format!( + "--http-inject must be 'METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]', got {spec:?}" + ))); + } + let matcher = HttpRule::parse(&format!("{} {}", toks[0], toks[1]))?; + let cred = toks[3]; + let auth = parse_auth(toks[2], cred)?; + let on_existing = match toks.get(4) { + // Default Replace: the proxy owns the credential, and SDKs always send + // a placeholder auth header (see OnExistingHeader::Replace). + None | Some(&"replace") => OnExistingHeader::Replace, + Some(&"add-only") => OnExistingHeader::AddOnly, + Some(other) => { + return Err(SandboxError::Invalid(format!( + "--http-inject trailing token must be 'replace', 'add-only', or absent, got {other:?}" + ))) + } + }; + let source = *sources.get(cred).ok_or_else(|| { + SandboxError::Invalid(format!("--http-inject references undeclared credential {cred:?}")) + })?; + parsed.push(Parsed { name: cred, matcher, auth, on_existing, source }); + } + + // Load each referenced credential exactly once (so an `fd:` source is read + // once and several rules can share the secret), and collect the env-var + // names of `env:` sources so the child can be denied them — otherwise the + // agent would just read the value straight from its own environment. + let mut loaded: HashMap<&str, Arc> = HashMap::new(); + let mut env_strip: Vec = Vec::new(); + for p in &parsed { + if !loaded.contains_key(p.name) { + loaded.insert(p.name, Arc::new(load_secret(p.source)?)); + if let Some(var) = p.source.strip_prefix("env:") { + if !env_strip.iter().any(|v| v == var) { + env_strip.push(var.to_string()); + } + } + } + } + + let rules = parsed + .into_iter() + .map(|p| InjectRule { + name: p.name.to_string(), + matcher: p.matcher, + auth: p.auth, + secret: Arc::clone(&loaded[p.name]), + on_existing: p.on_existing, + }) + .collect(); + Ok((rules, env_strip)) +} + +/// Standard base64 (RFC 4648) — small inline encoder to avoid a dependency. +fn base64_encode(input: &[u8]) -> String { + const T: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = String::with_capacity(input.len().div_ceil(3) * 4); + for chunk in input.chunks(3) { + let b0 = chunk[0] as u32; + let b1 = *chunk.get(1).unwrap_or(&0) as u32; + let b2 = *chunk.get(2).unwrap_or(&0) as u32; + let n = (b0 << 16) | (b1 << 8) | b2; + out.push(T[((n >> 18) & 63) as usize] as char); + out.push(T[((n >> 12) & 63) as usize] as char); + out.push(if chunk.len() > 1 { T[((n >> 6) & 63) as usize] as char } else { '=' }); + out.push(if chunk.len() > 2 { T[(n & 63) as usize] as char } else { '=' }); + } + out +} + +/// Percent-encode raw bytes as a query component (encode everything not +/// unreserved, so `&`/`=`/`#`/`%` and any binary byte can't break out). +fn urlencode_bytes(bytes: &[u8]) -> String { + let mut out = String::with_capacity(bytes.len()); + for &b in bytes { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => out.push(b as char), + _ => out.push_str(&format!("%{b:02X}")), + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parts_of(uri: &str, headers: &[(&str, &str)]) -> hyper::http::request::Parts { + let mut b = hyper::Request::builder().uri(uri); + for (k, v) in headers { + b = b.header(*k, *v); + } + b.body(()).unwrap().into_parts().0 + } + + fn rule(auth: AuthShape, secret: &str, on_existing: OnExistingHeader) -> InjectRule { + InjectRule { + name: "test".into(), + matcher: HttpRule::parse("* api.example.com/*").unwrap(), + auth, + secret: Arc::new(SecretString::new(secret.as_bytes().to_vec())), + on_existing, + } + } + + #[test] + fn secret_debug_is_redacted() { + let s = SecretString::new(b"sk-supersecret".to_vec()); + assert_eq!(format!("{s:?}"), "SecretString()"); + // The rule's Debug must not leak the secret either. + let r = rule(AuthShape::Bearer, "sk-supersecret", OnExistingHeader::AddOnly); + assert!(!format!("{r:?}").contains("supersecret")); + } + + #[test] + fn load_secret_rejects_literal_and_unknown() { + assert!(load_secret("literal:sk-x").is_err()); + assert!(load_secret("weird:x").is_err()); + assert!(load_secret("no-colon").is_err()); + } + + #[test] + fn load_secret_env_strips_newline() { + std::env::set_var("SANDLOCK_TEST_CRED", "sk-abc\n"); + let s = load_secret("env:SANDLOCK_TEST_CRED").unwrap(); + assert_eq!(s.expose(), b"sk-abc"); + std::env::remove_var("SANDLOCK_TEST_CRED"); + assert!(load_secret("env:SANDLOCK_TEST_CRED").is_err()); + } + + #[test] + fn bearer_injects_authorization() { + let mut p = parts_of("https://api.example.com/v1/x", &[]); + rule(AuthShape::Bearer, "sk-abc", OnExistingHeader::AddOnly).apply(&mut p).unwrap(); + assert_eq!(p.headers.get("authorization").unwrap(), "Bearer sk-abc"); + assert!(p.headers.get("authorization").unwrap().is_sensitive()); + } + + #[test] + fn basic_injects_base64() { + let mut p = parts_of("https://api.example.com/x", &[]); + rule(AuthShape::Basic { username: "user".into() }, "pass", OnExistingHeader::AddOnly).apply(&mut p).unwrap(); + // base64("user:pass") == "dXNlcjpwYXNz" + assert_eq!(p.headers.get("authorization").unwrap(), "Basic dXNlcjpwYXNz"); + } + + #[test] + fn header_shape_sets_named_header() { + let mut p = parts_of("https://api.example.com/x", &[]); + rule(AuthShape::Header { name: "x-api-key".into() }, "k123", OnExistingHeader::AddOnly).apply(&mut p).unwrap(); + assert_eq!(p.headers.get("x-api-key").unwrap(), "k123"); + } + + #[test] + fn add_only_does_not_overwrite_but_replace_does() { + let mut p = parts_of("https://api.example.com/x", &[("authorization", "Bearer child-set")]); + rule(AuthShape::Bearer, "sk-real", OnExistingHeader::AddOnly).apply(&mut p).unwrap(); + assert_eq!(p.headers.get("authorization").unwrap(), "Bearer child-set"); + + let mut p2 = parts_of("https://api.example.com/x", &[("authorization", "Bearer child-set")]); + rule(AuthShape::Bearer, "sk-real", OnExistingHeader::Replace).apply(&mut p2).unwrap(); + assert_eq!(p2.headers.get("authorization").unwrap(), "Bearer sk-real"); + } + + #[test] + fn query_shape_appends_param() { + let mut p = parts_of("https://api.example.com/v1/x?a=1", &[]); + rule(AuthShape::Query { param: "key".into() }, "s e/cret", OnExistingHeader::AddOnly).apply(&mut p).unwrap(); + assert_eq!(p.uri.query().unwrap(), "a=1&key=s%20e%2Fcret"); + + let mut p2 = parts_of("https://api.example.com/v1/x", &[]); + rule(AuthShape::Query { param: "key".into() }, "abc", OnExistingHeader::AddOnly).apply(&mut p2).unwrap(); + assert_eq!(p2.uri.query().unwrap(), "key=abc"); + } + + #[test] + fn parse_auth_shapes() { + assert_eq!(parse_auth("bearer", "c").unwrap(), AuthShape::Bearer); + assert_eq!(parse_auth("basic:user", "c").unwrap(), AuthShape::Basic { username: "user".into() }); + assert_eq!(parse_auth("header:x-api-key", "c").unwrap(), AuthShape::Header { name: "x-api-key".into() }); + assert_eq!(parse_auth("apikey:x-key", "c").unwrap(), AuthShape::Header { name: "x-key".into() }); + assert_eq!(parse_auth("query:token", "c").unwrap(), AuthShape::Query { param: "token".into() }); + assert!(parse_auth("basic:", "c").is_err()); + assert!(parse_auth("bogus", "c").is_err()); + } + + #[test] + fn apply_fails_on_secret_with_illegal_header_bytes() { + // A secret containing CR/LF can't be a header value — apply must report + // failure (so the caller rejects the request) rather than silently drop. + let mut p = parts_of("https://api.example.com/x", &[]); + let r = rule(AuthShape::Bearer, "sk\r\nx-evil: 1", OnExistingHeader::AddOnly); + assert!(r.apply(&mut p).is_err()); + assert!(p.headers.get("authorization").is_none()); + assert!(p.headers.get("x-evil").is_none()); // no header injection + } + + #[test] + fn query_add_only_does_not_duplicate() { + let mut p = parts_of("https://api.example.com/x?key=child", &[]); + rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::AddOnly) + .apply(&mut p).unwrap(); + assert_eq!(p.uri.query().unwrap(), "key=child"); // untouched + let mut p2 = parts_of("https://api.example.com/x?key=child", &[]); + rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::Replace) + .apply(&mut p2).unwrap(); + assert_eq!(p2.uri.query().unwrap(), "key=child&key=sk-real"); + } + + #[test] + fn resolve_dedups_credentials_and_collects_env_strip() { + std::env::set_var("SANDLOCK_TEST_RESOLVE", "sk-resolve"); + let creds = vec!["a=env:SANDLOCK_TEST_RESOLVE".to_string()]; + let inject = vec![ + "GET x.com/* bearer a".to_string(), + "POST x.com/* header:x-key a".to_string(), // same credential, second rule + ]; + let (rules, env_strip) = resolve_inject_rules(&creds, &inject).unwrap(); + assert_eq!(rules.len(), 2); + // Both rules share one loaded secret (one Arc). + assert!(Arc::ptr_eq(&rules[0].secret, &rules[1].secret)); + assert_eq!(env_strip, vec!["SANDLOCK_TEST_RESOLVE".to_string()]); + std::env::remove_var("SANDLOCK_TEST_RESOLVE"); + } + + #[test] + fn resolve_rejects_undeclared_and_std_fd() { + assert!(resolve_inject_rules(&[], &["GET x/* bearer missing".to_string()]).is_err()); + assert!(load_secret("fd:1").is_err()); // stdout + assert!(load_secret("fd:0").is_err()); // stdin + } + + #[test] + fn resolve_default_is_replace_add_only_is_opt_in() { + std::env::set_var("SANDLOCK_TEST_ONEX", "sk-x"); + let creds = vec!["a=env:SANDLOCK_TEST_ONEX".to_string()]; + // No trailing token → Replace, so an SDK's placeholder Authorization is + // overwritten (the whole point of the feature — regression guard). + let (r, _) = resolve_inject_rules(&creds, &["* x.com/* bearer a".to_string()]).unwrap(); + assert_eq!(r[0].on_existing, OnExistingHeader::Replace); + // Prove it at the apply level: a child-set Authorization is replaced. + let mut p = parts_of("https://x.com/v1", &[("authorization", "Bearer sk-placeholder")]); + r[0].apply(&mut p).unwrap(); + assert_eq!(p.headers.get("authorization").unwrap(), "Bearer sk-x"); + + let (r2, _) = resolve_inject_rules(&creds, &["* x.com/* bearer a add-only".to_string()]).unwrap(); + assert_eq!(r2[0].on_existing, OnExistingHeader::AddOnly); + // Unknown trailing token is rejected. + assert!(resolve_inject_rules(&creds, &["* x.com/* bearer a keep".to_string()]).is_err()); + std::env::remove_var("SANDLOCK_TEST_ONEX"); + } +} diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index b4330768..4b5e886c 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -1,5 +1,6 @@ pub mod error; pub mod http; +pub(crate) mod credential; pub mod sandbox; // formerly `policy`; contains Sandbox + SandboxBuilder + Confinement pub mod profile; pub mod result; diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index ca99f9f5..50b36440 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -145,6 +145,7 @@ impl TryFrom<&Sandbox> for Confinement { if sandbox.allows_sysv_ipc() { unsupported.push("extra_allow_syscalls=[\"sysv_ipc\"]"); } if !sandbox.http_allow.is_empty() { unsupported.push("http_allow"); } if !sandbox.http_deny.is_empty() { unsupported.push("http_deny"); } + if !sandbox.inject.is_empty() { unsupported.push("http_inject"); } if !sandbox.http_ports.is_empty() { unsupported.push("http_ports"); } if sandbox.http_ca.is_some() { unsupported.push("http_ca"); } if sandbox.http_key.is_some() { unsupported.push("http_key"); } @@ -341,6 +342,17 @@ pub struct Sandbox { // HTTP ACL pub http_allow: Vec, pub http_deny: Vec, + /// Credential-injection rules, applied in the MITM proxy after the ACL + /// check. `Arc` so the (non-Clone) secrets flow to the proxy by sharing. + /// Not serialized: the resolved secrets live only in the supervisor and are + /// re-loaded from their sources on each build, never persisted in a policy. + #[serde(skip)] + pub(crate) inject: std::sync::Arc>, + /// `env:` var names to remove from the child's environment (so an env-sourced + /// credential can't be read straight out of the agent's own env). Just names, + /// no secrets — safe to serialize, but tied to `inject` which isn't restored. + #[serde(skip)] + pub(crate) inject_env_strip: Vec, /// TCP ports to intercept for HTTP ACL. Defaults to [80] (plus 443 when /// http_ca is set). Override with `http_ports` to intercept custom ports. pub http_ports: Vec, @@ -479,6 +491,8 @@ impl Clone for Sandbox { net_deny_bind: self.net_deny_bind.clone(), http_allow: self.http_allow.clone(), http_deny: self.http_deny.clone(), + inject: self.inject.clone(), + inject_env_strip: self.inject_env_strip.clone(), http_ports: self.http_ports.clone(), http_ca: self.http_ca.clone(), http_key: self.http_key.clone(), @@ -1489,6 +1503,7 @@ impl Sandbox { let handle = crate::transparent_proxy::spawn_transparent_proxy( self.http_allow.clone(), self.http_deny.clone(), + std::sync::Arc::clone(&self.inject), cert_pem, key_pem, ) diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index 0cdd3b70..60fe3e72 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -59,6 +59,25 @@ pub struct SandboxBuilder { #[cfg_attr(feature = "cli", arg(long = "http-deny", value_name = "RULE"))] pub http_deny: Vec, + /// Named credential loaded into the supervisor: `NAME=SOURCE`, where SOURCE + /// is `env:VAR`, `file:/path`, or `fd:N`. The resolved secret is never handed + /// to the child: an `env:` var is stripped from the child's environment and an + /// `fd:` is read supervisor-side only. A `file:` source, however, is only as + /// private as the path — keep it outside the sandbox's readable paths, or the + /// child can open it directly. + #[cfg_attr(feature = "cli", arg(long = "credential", value_name = "NAME=SOURCE"))] + pub credentials: Vec, + + /// Credential-injection rule (needs an HTTPS MITM proxy, i.e. `--http-ca` / + /// `--http-inject-ca`): `METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]`, + /// where AUTHSPEC is `bearer | basic: | header: | apikey: | + /// query:`. The matching request gets the credential attached in the + /// proxy, after the ACL check. The trailing token defaults to `replace` (the + /// proxy overwrites the placeholder auth SDKs send); pass `add-only` to keep a + /// value the child set. + #[cfg_attr(feature = "cli", arg(long = "http-inject", value_name = "RULE"))] + pub http_inject: Vec, + /// TCP ports to intercept for HTTP ACL (default: 80, plus 443 with --http-ca) #[cfg_attr(feature = "cli", arg(long = "http-port", value_name = "PORT"))] pub http_ports: Vec, @@ -229,6 +248,8 @@ impl Clone for SandboxBuilder { net_deny_bind: self.net_deny_bind.clone(), http_allow: self.http_allow.clone(), http_deny: self.http_deny.clone(), + credentials: self.credentials.clone(), + http_inject: self.http_inject.clone(), http_ports: self.http_ports.clone(), http_ca: self.http_ca.clone(), http_key: self.http_key.clone(), @@ -397,6 +418,24 @@ impl SandboxBuilder { self } + /// Declare a named credential: `name` and a `source` (`env:`/`file:`/`fd:`). + pub fn credential(mut self, name: &str, source: &str) -> Self { + self.credentials.push(format!("{name}={source}")); + self + } + + /// Declare a credential from a raw `NAME=SOURCE` spec (as the CLI parses it). + pub fn credential_spec(mut self, spec: &str) -> Self { + self.credentials.push(spec.to_string()); + self + } + + /// Add a credential-injection rule (see the `--http-inject` field docs). + pub fn http_inject(mut self, rule: &str) -> Self { + self.http_inject.push(rule.to_string()); + self + } + pub fn http_port(mut self, port: u16) -> Self { self.http_ports.push(port); self @@ -680,6 +719,38 @@ impl SandboxBuilder { .map(|s| HttpRule::parse(&s)) .collect::>()?; + // Credential injection happens inside the ACL proxy, so it needs the + // proxy to run at all (i.e. some http rule). Injecting into HTTPS + // additionally needs a CA (--http-ca / --http-inject-ca) to MITM 443; + // without one only plaintext HTTP is intercepted. Reject a rule that + // could never fire (no proxy). + if !self.http_inject.is_empty() && http_allow.is_empty() && http_deny.is_empty() { + return Err(SandboxError::Invalid( + "--http-inject requires an HTTP ACL proxy (--http-allow or --http-deny); \ + HTTPS injection additionally needs --http-ca or --http-inject-ca" + .into(), + )); + } + // Without a CA only port 80 is intercepted, so a rule for an HTTPS host + // (the common case: `bearer openai` → api.openai.com:443) silently never + // fires — the request bypasses the proxy and goes out uncredentialed. We + // can't know a rule's scheme at build time, so warn rather than reject. + if !self.http_inject.is_empty() && self.http_ca.is_none() && self.http_inject_ca.is_empty() { + eprintln!( + "sandlock: warning: --http-inject with no CA (--http-ca/--http-inject-ca) only \ + injects into plaintext HTTP (port 80); requests to HTTPS hosts bypass the proxy \ + and are sent without the credential" + ); + } + // Resolve credentials + injection rules, loading each secret into the + // supervisor. Wrapped in Arc so it flows to the proxy without cloning + // the (deliberately non-Clone) secrets. `inject_env_strip` is the set of + // `env:` var names to remove from the child, so an env-sourced secret + // can't just be read out of the child's own environment. + let (inject_rules, inject_env_strip) = + crate::credential::resolve_inject_rules(&self.credentials, &self.http_inject)?; + let inject = std::sync::Arc::new(inject_rules); + // Default HTTP intercept ports: 80 always, 443 when HTTPS CA is configured. let http_ports = if self.http_ports.is_empty() && (!http_allow.is_empty() || !http_deny.is_empty()) { let mut ports = vec![80]; @@ -744,6 +815,8 @@ impl SandboxBuilder { net_deny_bind, http_allow, http_deny, + inject, + inject_env_strip, http_ports, http_ca: self.http_ca, http_key: self.http_key, diff --git a/crates/sandlock-core/src/transparent_proxy/mod.rs b/crates/sandlock-core/src/transparent_proxy/mod.rs index e3fbc785..4215d59e 100644 --- a/crates/sandlock-core/src/transparent_proxy/mod.rs +++ b/crates/sandlock-core/src/transparent_proxy/mod.rs @@ -49,6 +49,7 @@ fn is_tls_client_hello(first_byte: u8) -> bool { pub(crate) async fn spawn_transparent_proxy( allow: Vec, deny: Vec, + inject: Arc>, ca_cert_pem: Option<&str>, ca_key_pem: Option<&str>, ) -> std::io::Result { @@ -56,7 +57,7 @@ pub(crate) async fn spawn_transparent_proxy( let orig_dest: OrigDestMap = Arc::new(std::sync::RwLock::new(std::collections::HashMap::new())); let forwarder = Forwarder::new()?; - let svc = AclService::new(allow, deny, Arc::clone(&orig_dest), forwarder); + let svc = AclService::new(allow, deny, inject, Arc::clone(&orig_dest), forwarder); let signer = match (ca_cert_pem, ca_key_pem) { (Some(c), Some(k)) => Some(Arc::new(CertSigner::new(c, k)?)), @@ -181,7 +182,7 @@ mod tests { .expect("resolve_ca ok") .expect("ephemeral CA generated"); let allow = vec![crate::http::HttpRule::parse("GET allowed.test/*").expect("rule parses")]; - let handle = super::spawn_transparent_proxy(allow, vec![], Some(&ca.cert_pem), Some(&ca.key_pem)) + let handle = super::spawn_transparent_proxy(allow, vec![], Arc::new(vec![]), Some(&ca.cert_pem), Some(&ca.key_pem)) .await .expect("proxy spawns"); let addr = handle.addr; diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index 4835502d..d8557264 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -14,6 +14,7 @@ use hyper::{Request, Response, StatusCode}; use tokio::sync::Mutex; use super::upstream::{box_incoming, Forwarder}; +use crate::credential::InjectRule; use crate::http::{http_acl_check, HttpRule}; type BoxError = Box; @@ -31,6 +32,7 @@ struct DnsEntry { pub(crate) struct AclService { pub(crate) allow: Arc>, pub(crate) deny: Arc>, + pub(crate) inject: Arc>, pub(crate) orig_dest: OrigDestMap, pub(crate) forwarder: Forwarder, dns_cache: Arc>>, @@ -40,12 +42,14 @@ impl AclService { pub(crate) fn new( allow: Vec, deny: Vec, + inject: Arc>, orig_dest: OrigDestMap, forwarder: Forwarder, ) -> Self { Self { allow: Arc::new(allow), deny: Arc::new(deny), + inject, orig_dest, forwarder, dns_cache: Arc::new(Mutex::new(HashMap::new())), @@ -150,6 +154,33 @@ impl AclService { 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. + for r in self.inject.iter() { + if r.matches(&method, &host, &path) { + if r.apply(&mut parts).is_err() { + // Rendering failed (e.g. the secret has bytes illegal in a + // header) — fail the request rather than forward it with no + // credential, which would look like an auth bug to the caller. + eprintln!( + "sandlock: credential {:?} could not be rendered for {} {}{} — rejecting", + r.name, method, host, path + ); + return text_response( + StatusCode::BAD_GATEWAY, + "Blocked by sandlock: credential could not be applied", + ); + } + eprintln!( + "sandlock: injected credential {:?} for {} {}{}", + r.name, method, host, path + ); + break; + } + } + let out_req = Request::from_parts(parts, box_incoming(body)); match self.forwarder.forward(out_req).await { diff --git a/crates/sandlock-core/tests/integration/test_http_acl.rs b/crates/sandlock-core/tests/integration/test_http_acl.rs index 301d693f..8d4dd2a4 100644 --- a/crates/sandlock-core/tests/integration/test_http_acl.rs +++ b/crates/sandlock-core/tests/integration/test_http_acl.rs @@ -529,3 +529,154 @@ async fn test_http_acl_ipv6_method_filtering() { let _ = std::fs::remove_file(&out_get); let _ = std::fs::remove_file(&out_post); } + +/// Spawn a server that records the `Authorization` header of the one request it +/// receives, for credential-injection tests. +fn spawn_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()); + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 { + break; + } + if line.to_lowercase().starts_with("authorization:") { + *cap.lock().unwrap() = + Some(line.split_once(':').unwrap().1.trim().to_string()); + } + if line == "\r\n" || line == "\n" { + break; + } + } + 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) +} + +/// A credential declared in the supervisor is injected into the outbound request +/// inside the proxy — the child never carries it in env/argv/headers — and +/// reaches the upstream, after the ACL check. +#[tokio::test] +async fn test_credential_injected_into_upstream() { + let out = temp_file("cred-inject"); + let secret_file = temp_file("cred-secret"); + std::fs::write(&secret_file, "sk-phase1-secret\n").unwrap(); + let (port, srv, captured) = spawn_capturing_http_server(); + + let policy = base_policy() + .http_allow("GET 127.0.0.1/*") + .http_port(port) + .credential("api", &format!("file:{}", secret_file.display())) + .http_inject("GET 127.0.0.1/* bearer api") + .build() + .unwrap(); + + let script = http_script(&format!("http://127.0.0.1:{}/data", port), &out); + let result = policy.with_name("test").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"), "child request should succeed, got: {}", content); + + srv.join().unwrap(); + let got = captured.lock().unwrap().clone(); + let _ = std::fs::remove_file(&out); + let _ = std::fs::remove_file(&secret_file); + + // The child sent no Authorization header; the proxy injected the credential + // by value while the child only ever knew its name. + assert_eq!( + got.as_deref(), Some("Bearer sk-phase1-secret"), + "upstream must receive the injected credential (got {got:?})" + ); +} + +/// Deny-path invariant: a request that matches an inject rule but is denied by +/// the ACL must be blocked (403) and the credential must never be rendered — the +/// upstream sees no connection at all. Proves injection sits strictly after the +/// ACL check. +#[tokio::test] +async fn test_denied_request_does_not_inject_credential() { + let out = temp_file("cred-deny"); + let secret_file = temp_file("cred-deny-secret"); + std::fs::write(&secret_file, "sk-must-not-leak\n").unwrap(); + let (port, _srv, captured) = spawn_capturing_http_server(); + + // ACL allows only /allowed; the inject rule matches every path. A GET to + // /secret is denied by the ACL, so injection must not run. + let policy = base_policy() + .http_allow("GET 127.0.0.1/allowed") + .http_port(port) + .credential("api", &format!("file:{}", secret_file.display())) + .http_inject("GET 127.0.0.1/* bearer api") + .build() + .unwrap(); + + let script = http_script(&format!("http://127.0.0.1:{}/secret", port), &out); + let result = policy.with_name("test").run_interactive(&["python3", "-c", &script]) + .await.unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + + let content = std::fs::read_to_string(&out).unwrap_or_default(); + // Give the (never-reached) upstream a moment; it must not have been contacted. + std::thread::sleep(std::time::Duration::from_millis(200)); + let got = captured.lock().unwrap().clone(); + let _ = std::fs::remove_file(&out); + let _ = std::fs::remove_file(&secret_file); + + assert!(content.starts_with("HTTP:403"), "denied request should be 403, got: {}", content); + assert_eq!(got, None, "credential must not reach the upstream on a denied request"); +} + +/// An `env:`-sourced credential must be scrubbed from the child's environment, +/// otherwise the agent could read the real secret straight out of its own env +/// instead of relying on proxy-side injection. The child does no network here — +/// it just reports whether it can still see the variable. +#[tokio::test] +async fn test_env_sourced_credential_stripped_from_child() { + std::env::set_var("SANDLOCK_TEST_SECRET_ENV", "sk-env-secret"); + let out = temp_file("cred-envstrip"); + let (port, _srv) = spawn_http_server(0); // just to obtain a valid intercept port + let policy = base_policy() + .http_allow("GET 127.0.0.1/*") + .http_port(port) + .credential("api", "env:SANDLOCK_TEST_SECRET_ENV") + .http_inject("GET 127.0.0.1/* bearer api") + .build() + .unwrap(); + + let script = format!( + "import os; open('{}', 'w').write(os.environ.get('SANDLOCK_TEST_SECRET_ENV', 'ABSENT'))", + out.display() + ); + let result = policy.with_name("test").run_interactive(&["python3", "-c", &script]) + .await.unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + + let content = std::fs::read_to_string(&out).unwrap_or_default(); + let _ = std::fs::remove_file(&out); + std::env::remove_var("SANDLOCK_TEST_SECRET_ENV"); + assert_eq!( + content, "ABSENT", + "env-sourced credential must be stripped from the child's environment, got {content:?}" + ); +} + +// 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 +// "https" path), and the plaintext tests above exercise it; a TLS end-to-end +// test needs a CA-trust harness and is tracked as a separate follow-up. From 69e87d4458f80dfccd3b6597914a1982324483ae Mon Sep 17 00:00:00 2001 From: dzerik Date: Mon, 6 Jul 2026 22:14:19 +0300 Subject: [PATCH 2/5] fix(net): address review on credential injection (RFC #66) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - context.rs: strip inherited credential env vars *before* applying `sandbox.env`, so a deliberate placeholder (`--env OPENAI_API_KEY=dummy`, which SDKs need set to start) survives while the inherited real secret is still removed. Previously the strip ran after and deleted the placeholder, breaking the canonical `env:` flow before any request. - credential.rs: `Replace` now actually replaces for the query shape — existing occurrences of the target param are dropped before appending, instead of producing a duplicate (`key=child&key=sk-real`) where a framework reading the first occurrence would authenticate against the child's value. The test is corrected to assert replacement, not the duplicate it pinned. - credential.rs: reject ':' in a `basic:` user-id (RFC 7617) — it would shift the `user:pass` boundary and leak part of the secret into the password. - credential.rs: drop the stale `parse_auth` doc line claiming a tuple return. --- crates/sandlock-core/src/context.rs | 9 ++-- crates/sandlock-core/src/credential.rs | 58 ++++++++++++++++++++------ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 70921f02..d4ef7f23 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -498,15 +498,18 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { std::env::remove_var(&key); } } - for (key, value) in &sandbox.env { - std::env::set_var(key, value); - } // Remove env vars whose value was loaded into the supervisor as a credential // source, so the agent can't read the real secret straight from its own // environment (this is the child; the mutation is process-local and post-fork). + // This runs *before* applying `sandbox.env`, so the inherited real secret is + // dropped but a deliberate placeholder the user passes (e.g. + // `--env OPENAI_API_KEY=dummy`, which SDKs need set to start) still survives. for name in &sandbox.inject_env_strip { std::env::remove_var(name); } + for (key, value) in &sandbox.env { + std::env::set_var(key, value); + } // 13b. GPU device visibility if let Some(ref devices) = sandbox.gpu_devices { diff --git a/crates/sandlock-core/src/credential.rs b/crates/sandlock-core/src/credential.rs index b67dcde8..3d7e138d 100644 --- a/crates/sandlock-core/src/credential.rs +++ b/crates/sandlock-core/src/credential.rs @@ -263,10 +263,10 @@ impl InjectRule { let uri = &parts.uri; let path = uri.path(); let existing = uri.query(); + let prefix = format!("{}=", urlencode_bytes(param.as_bytes())); // Honor AddOnly: don't append a param the request already carries. if self.on_existing == OnExistingHeader::AddOnly { if let Some(q) = existing { - let prefix = format!("{}=", urlencode_bytes(param.as_bytes())); if q.split('&').any(|kv| kv.starts_with(&prefix)) { return Ok(()); } @@ -274,13 +274,20 @@ impl InjectRule { } // Percent-encode the raw secret bytes (never lossy-stringify — a binary // key would be corrupted). - let pair = format!( - "{}={}", - urlencode_bytes(param.as_bytes()), - urlencode_bytes(self.secret.expose()) - ); - let new_pq = match existing { - Some(q) if !q.is_empty() => format!("{path}?{q}&{pair}"), + let pair = format!("{}{}", prefix, urlencode_bytes(self.secret.expose())); + // Drop any existing occurrence of this param before appending, so + // `Replace` actually replaces instead of appending a duplicate (most + // frameworks read the first occurrence, so a duplicate would leave the + // child's placeholder winning). For `AddOnly` this only runs when the + // param is absent, so the filter is a no-op there. + let kept: Option = existing.map(|q| { + q.split('&') + .filter(|kv| !kv.is_empty() && !kv.starts_with(&prefix)) + .collect::>() + .join("&") + }); + let new_pq = match kept { + Some(ref k) if !k.is_empty() => format!("{path}?{k}&{pair}"), _ => format!("{path}?{pair}"), }; let mut b = hyper::http::uri::Builder::new(); @@ -297,7 +304,6 @@ impl InjectRule { /// Parse an `AuthShape` from the auth token of an `--http-inject` rule: /// `bearer` | `basic:` | `header:` | `apikey:` | `query:`. -/// Returns `(shape, credential_name)`. pub fn parse_auth(spec: &str, credential: &str) -> Result { let (kind, arg) = match spec.split_once(':') { Some((k, a)) => (k, Some(a)), @@ -305,6 +311,14 @@ pub fn parse_auth(spec: &str, credential: &str) -> Result AuthShape::Bearer, + // RFC 7617 forbids ':' in the user-id: it would shift the `user:pass` + // boundary in the base64 payload (upstream would parse part of the secret + // as the password), so reject it rather than silently mis-encode. + ("basic", Some(user)) if user.contains(':') => { + return Err(SandboxError::Invalid(format!( + "basic auth user-id must not contain ':' (RFC 7617), got {user:?}" + ))) + } ("basic", Some(user)) if !user.is_empty() => AuthShape::Basic { username: user.to_string() }, // `apikey:
` and `header:` are the same rendering. ("header" | "apikey", Some(name)) if !name.is_empty() => AuthShape::Header { name: name.to_string() }, @@ -550,6 +564,10 @@ mod tests { assert_eq!(parse_auth("query:token", "c").unwrap(), AuthShape::Query { param: "token".into() }); assert!(parse_auth("basic:", "c").is_err()); assert!(parse_auth("bogus", "c").is_err()); + // RFC 7617: ':' in the user-id is rejected (would shift the user:pass + // boundary and leak part of the secret into the password field). + assert!(parse_auth("basic:a:b", "c").is_err()); + assert!(matches!(parse_auth("basic:alice", "c"), Ok(AuthShape::Basic { username }) if username == "alice")); } #[test] @@ -564,15 +582,31 @@ mod tests { } #[test] - fn query_add_only_does_not_duplicate() { + fn query_add_only_keeps_child_replace_overwrites() { + // AddOnly: a param the child already carries is left untouched. let mut p = parts_of("https://api.example.com/x?key=child", &[]); rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::AddOnly) .apply(&mut p).unwrap(); - assert_eq!(p.uri.query().unwrap(), "key=child"); // untouched + assert_eq!(p.uri.query().unwrap(), "key=child"); + + // Replace must *replace*, not append a duplicate — otherwise a framework + // reading the first occurrence authenticates against the child's value. let mut p2 = parts_of("https://api.example.com/x?key=child", &[]); rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::Replace) .apply(&mut p2).unwrap(); - assert_eq!(p2.uri.query().unwrap(), "key=child&key=sk-real"); + assert_eq!(p2.uri.query().unwrap(), "key=sk-real"); + + // Replace preserves other params and drops only the target's duplicates. + let mut p3 = parts_of("https://api.example.com/x?a=1&key=old&b=2", &[]); + rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::Replace) + .apply(&mut p3).unwrap(); + assert_eq!(p3.uri.query().unwrap(), "a=1&b=2&key=sk-real"); + + // Replace on a request without the param just appends it. + let mut p4 = parts_of("https://api.example.com/x", &[]); + rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::Replace) + .apply(&mut p4).unwrap(); + assert_eq!(p4.uri.query().unwrap(), "key=sk-real"); } #[test] From 3bd2fe95b32c6ed4a16934671f4a5d848e930908 Mon Sep 17 00:00:00 2001 From: dzerik Date: Sat, 11 Jul 2026 00:04:36 +0300 Subject: [PATCH 3/5] credential-injection: rename --http-inject to --http-auth, warn on cleartext, docs Address review follow-ups: - Rename the CLI flag --http-inject to --http-auth (and the SandboxBuilder::http_auth field/method to match) so it no longer reads as a sibling of --http-inject-ca, which is an unrelated CA-provisioning flag. The CA flags (--http-inject-ca / --http-ca) are untouched. - Warn when a credential is injected over cleartext http. The warning is emitted in the transparent proxy (core), at request time where the scheme is known, so it fires for library/API callers and not just the CLI. It is latched once per run, names only the credential and host (never the secret value), and is a warning only -- the request still proceeds. The existing build-time "no CA set" warning covers a different risk and is kept. - Document --http-auth and credential injection in README.md and docs/sandbox-reference.md. --- README.md | 11 +++ crates/sandlock-cli/src/main.rs | 2 +- .../sandlock-core/src/checkpoint/capture.rs | 2 +- crates/sandlock-core/src/credential.rs | 12 +-- crates/sandlock-core/src/sandbox.rs | 2 +- crates/sandlock-core/src/sandbox/builder.rs | 22 ++--- .../src/transparent_proxy/service.rs | 44 ++++++++++ .../tests/integration/test_http_acl.rs | 6 +- docs/sandbox-reference.md | 80 +++++++++++++++++++ 9 files changed, 158 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 237cfb9e..ce12e6f6 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,17 @@ sandlock run \ --http-ca ca.pem --http-key ca-key.pem \ -r /usr -r /lib -r /etc -- python3 agent.py +# Credential injection: the secret lives in the supervisor, the child never sees +# it. sandlock attaches it in the proxy AFTER the ACL check. Over HTTPS the value +# is encrypted to the upstream; over cleartext HTTP sandlock warns (the secret +# would be exposed on the wire). +sandlock run \ + --http-allow "POST api.openai.com/v1/*" \ + --http-inject-ca /etc/ssl/certs/ca-certificates.crt \ + --credential openai=env:OPENAI_API_KEY \ + --http-auth "POST api.openai.com/* bearer openai" \ + -r /usr -r /lib -r /etc -- python3 agent.py + # Server listening on ports (Landlock --net-allow-bind, separate from --net-allow; # accepts comma-separated ports and lo-hi ranges, repeatable) sandlock run --net-allow-bind 8080,9000-9005 -r /usr -r /lib -r /etc -- python3 server.py diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 31bdae15..b5f2be09 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -462,7 +462,7 @@ async fn run_command(args: RunArgs) -> Result { for rule in &pb.http_allow { builder = builder.http_allow(rule); } for rule in &pb.http_deny { builder = builder.http_deny(rule); } for c in &pb.credentials { builder = builder.credential_spec(c); } - for rule in &pb.http_inject { builder = builder.http_inject(rule); } + for rule in &pb.http_auth { builder = builder.http_auth(rule); } for port in &pb.http_ports { builder = builder.http_port(*port); } if let Some(ref ca) = pb.http_ca { builder = builder.http_ca(ca); } if let Some(ref key) = pb.http_key { builder = builder.http_key(key); } diff --git a/crates/sandlock-core/src/checkpoint/capture.rs b/crates/sandlock-core/src/checkpoint/capture.rs index e58f9a82..34a65a56 100644 --- a/crates/sandlock-core/src/checkpoint/capture.rs +++ b/crates/sandlock-core/src/checkpoint/capture.rs @@ -303,7 +303,7 @@ pub(crate) fn capture(pid: i32, policy: &Sandbox) -> Result` /// — so keeping the child's value would forward the placeholder and never /// inject. Replacing that one header the rule targets is what makes - /// `--http-inject "* api.openai.com/* bearer openai"` work at all. + /// `--http-auth "* api.openai.com/* bearer openai"` work at all. #[default] Replace, /// Leave the header/param the child already set, injecting only when absent. @@ -302,7 +302,7 @@ impl InjectRule { } } -/// Parse an `AuthShape` from the auth token of an `--http-inject` rule: +/// Parse an `AuthShape` from the auth token of an `--http-auth` rule: /// `bearer` | `basic:` | `header:` | `apikey:` | `query:`. pub fn parse_auth(spec: &str, credential: &str) -> Result { let (kind, arg) = match spec.split_once(':') { @@ -333,7 +333,7 @@ pub fn parse_auth(spec: &str, credential: &str) -> Result = spec.split_whitespace().collect(); if toks.len() < 4 { return Err(SandboxError::Invalid(format!( - "--http-inject must be 'METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]', got {spec:?}" + "--http-auth must be 'METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]', got {spec:?}" ))); } let matcher = HttpRule::parse(&format!("{} {}", toks[0], toks[1]))?; @@ -393,12 +393,12 @@ pub fn resolve_inject_rules( Some(&"add-only") => OnExistingHeader::AddOnly, Some(other) => { return Err(SandboxError::Invalid(format!( - "--http-inject trailing token must be 'replace', 'add-only', or absent, got {other:?}" + "--http-auth trailing token must be 'replace', 'add-only', or absent, got {other:?}" ))) } }; let source = *sources.get(cred).ok_or_else(|| { - SandboxError::Invalid(format!("--http-inject references undeclared credential {cred:?}")) + SandboxError::Invalid(format!("--http-auth references undeclared credential {cred:?}")) })?; parsed.push(Parsed { name: cred, matcher, auth, on_existing, source }); } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 50b36440..d7fe6952 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -145,7 +145,7 @@ impl TryFrom<&Sandbox> for Confinement { if sandbox.allows_sysv_ipc() { unsupported.push("extra_allow_syscalls=[\"sysv_ipc\"]"); } if !sandbox.http_allow.is_empty() { unsupported.push("http_allow"); } if !sandbox.http_deny.is_empty() { unsupported.push("http_deny"); } - if !sandbox.inject.is_empty() { unsupported.push("http_inject"); } + if !sandbox.inject.is_empty() { unsupported.push("http_auth"); } if !sandbox.http_ports.is_empty() { unsupported.push("http_ports"); } if sandbox.http_ca.is_some() { unsupported.push("http_ca"); } if sandbox.http_key.is_some() { unsupported.push("http_key"); } diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index 60fe3e72..d4b62ced 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -75,8 +75,8 @@ pub struct SandboxBuilder { /// proxy, after the ACL check. The trailing token defaults to `replace` (the /// proxy overwrites the placeholder auth SDKs send); pass `add-only` to keep a /// value the child set. - #[cfg_attr(feature = "cli", arg(long = "http-inject", value_name = "RULE"))] - pub http_inject: Vec, + #[cfg_attr(feature = "cli", arg(long = "http-auth", value_name = "RULE"))] + pub http_auth: Vec, /// TCP ports to intercept for HTTP ACL (default: 80, plus 443 with --http-ca) #[cfg_attr(feature = "cli", arg(long = "http-port", value_name = "PORT"))] @@ -249,7 +249,7 @@ impl Clone for SandboxBuilder { http_allow: self.http_allow.clone(), http_deny: self.http_deny.clone(), credentials: self.credentials.clone(), - http_inject: self.http_inject.clone(), + http_auth: self.http_auth.clone(), http_ports: self.http_ports.clone(), http_ca: self.http_ca.clone(), http_key: self.http_key.clone(), @@ -430,9 +430,9 @@ impl SandboxBuilder { self } - /// Add a credential-injection rule (see the `--http-inject` field docs). - pub fn http_inject(mut self, rule: &str) -> Self { - self.http_inject.push(rule.to_string()); + /// Add a credential-injection rule (see the `--http-auth` field docs). + pub fn http_auth(mut self, rule: &str) -> Self { + self.http_auth.push(rule.to_string()); self } @@ -724,9 +724,9 @@ impl SandboxBuilder { // additionally needs a CA (--http-ca / --http-inject-ca) to MITM 443; // without one only plaintext HTTP is intercepted. Reject a rule that // could never fire (no proxy). - if !self.http_inject.is_empty() && http_allow.is_empty() && http_deny.is_empty() { + if !self.http_auth.is_empty() && http_allow.is_empty() && http_deny.is_empty() { return Err(SandboxError::Invalid( - "--http-inject requires an HTTP ACL proxy (--http-allow or --http-deny); \ + "--http-auth requires an HTTP ACL proxy (--http-allow or --http-deny); \ HTTPS injection additionally needs --http-ca or --http-inject-ca" .into(), )); @@ -735,9 +735,9 @@ impl SandboxBuilder { // (the common case: `bearer openai` → api.openai.com:443) silently never // fires — the request bypasses the proxy and goes out uncredentialed. We // can't know a rule's scheme at build time, so warn rather than reject. - if !self.http_inject.is_empty() && self.http_ca.is_none() && self.http_inject_ca.is_empty() { + if !self.http_auth.is_empty() && self.http_ca.is_none() && self.http_inject_ca.is_empty() { eprintln!( - "sandlock: warning: --http-inject with no CA (--http-ca/--http-inject-ca) only \ + "sandlock: warning: --http-auth with no CA (--http-ca/--http-inject-ca) only \ injects into plaintext HTTP (port 80); requests to HTTPS hosts bypass the proxy \ and are sent without the credential" ); @@ -748,7 +748,7 @@ impl SandboxBuilder { // `env:` var names to remove from the child, so an env-sourced secret // can't just be read out of the child's own environment. let (inject_rules, inject_env_strip) = - crate::credential::resolve_inject_rules(&self.credentials, &self.http_inject)?; + crate::credential::resolve_inject_rules(&self.credentials, &self.http_auth)?; let inject = std::sync::Arc::new(inject_rules); // Default HTTP intercept ports: 80 always, 443 when HTTPS CA is configured. diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index d8557264..66918502 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use std::net::{IpAddr, SocketAddr}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -36,6 +37,18 @@ pub(crate) struct AclService { pub(crate) orig_dest: OrigDestMap, pub(crate) forwarder: Forwarder, dns_cache: Arc>>, + /// Latched once the first cleartext (`http`) credential injection is warned, + /// so a library/API caller gets the warning once per run instead of per + /// request. See [`first_cleartext_warn`]. + cleartext_warned: Arc, +} + +/// Whether this cleartext injection should emit the one-per-run warning: true the +/// first time a credential is injected over plaintext `http` (and latches `seen`), +/// false afterwards and always false for `https`. Split out so the warn-once +/// contract is unit-tested without capturing the supervisor's stderr. +fn first_cleartext_warn(scheme: &str, seen: &AtomicBool) -> bool { + scheme == "http" && !seen.swap(true, Ordering::Relaxed) } impl AclService { @@ -53,6 +66,7 @@ impl AclService { orig_dest, forwarder, dns_cache: Arc::new(Mutex::new(HashMap::new())), + cleartext_warned: Arc::new(AtomicBool::new(false)), } } @@ -177,6 +191,17 @@ impl AclService { "sandlock: injected credential {:?} for {} {}{}", r.name, method, host, path ); + // Fires for any caller (library/API, not just the CLI) since the + // proxy is in core: the scheme is only known here at request time. + // Once per run — over cleartext the secret is exposed on the wire. + if first_cleartext_warn(scheme, &self.cleartext_warned) { + eprintln!( + "sandlock: warning: credential {:?} injected over cleartext HTTP (no TLS) \ + to {} — the secret is exposed on the wire; prefer an HTTPS host \ + (configure MITM via --http-ca / --http-inject-ca)", + r.name, host + ); + } break; } } @@ -196,3 +221,22 @@ fn text_response(status: StatusCode, msg: &str) -> Response`, `header:`, +`apikey:`, or `query:`; the default `replace` overwrites an existing +credential of that shape, `add-only` leaves a caller-supplied one in place. +Injection requires an HTTP ACL proxy (at least one `http_allow` / `http_deny`); +injecting into an HTTPS host additionally needs `http_ca` / `http_inject_ca` so +the proxy can MITM port 443. Over cleartext HTTP the secret is sent to the +upstream in plaintext, so sandlock emits a one-per-run warning. `--credential` / +`--http-auth` are CLI/builder flags, not `[config]` profile keys. + +Examples, each showing the salient flags of a `sandlock run` invocation: + +```console +# Bearer token from an env var (OpenAI-style). The var is stripped from the +# child, so the agent can't read OPENAI_API_KEY back out of its own environment. +--credential openai=env:OPENAI_API_KEY \ +--http-auth "POST api.openai.com/v1/* bearer openai" + +# Custom header from a mounted secret file (Anthropic-style). `apikey:` is +# an alias of `header:`, and `file:` / `fd:` are alternatives to `env:`. +--credential anthropic=file:/run/secrets/anthropic-key \ +--http-auth "* api.anthropic.com/* header:x-api-key anthropic" + +# HTTP Basic with a fixed user-id. A ':' in the user-id is rejected (RFC 7617): +# it would shift the user:pass boundary and leak part of the secret. +--credential registry=env:REGISTRY_PASSWORD \ +--http-auth "* registry.internal/* basic:deploy registry" + +# Query parameter — the least private shape. The value lands in the upstream's +# access logs and any Referer, so use it only for APIs that require it. +--credential maps=file:/run/secrets/maps-key \ +--http-auth "GET maps.example.com/* query:key maps" + +# One credential backing several hosts. The trailing `add-only` makes the second +# rule a fallback: if the agent already set x-token itself, its value is kept +# (the default, `replace`, would overwrite it — needed for SDKs that always send +# a placeholder auth header). +--credential shared=env:SHARED_TOKEN \ +--http-auth "* a.example.com/* bearer shared" \ +--http-auth "* b.example.com/* header:x-token shared add-only" +``` + +Sandlock has no built-in secret-manager client — instead an external fetcher +materializes the secret into a `file:` or `fd:` source, keeping the value off +`ps`, the shell history, and the child's environment. This composes with any +manager (Vault, AWS/GCP/Azure secret stores, a CSI driver) rather than pinning +one into the supervisor. + +```console +# fd: via process substitution — the secret never touches disk. The fetcher +# writes to fd 3, sandlock reads it through a dup, and the child never sees it. +sandlock run \ + --http-allow "POST api.internal/*" \ + --http-inject-ca /etc/ssl/certs/ca-certificates.crt \ + --credential api=fd:3 \ + --http-auth "POST api.internal/* bearer api" \ + 3< <(vault read -field=token secret/data/api) \ + -r /usr -r /lib -r /etc -- python3 agent.py + +# file: from a mounted secret — e.g. a Vault Agent sidecar or a CSI +# secrets-store driver renders the value onto an in-memory tmpfs. The whole +# file is the secret (a single trailing newline is stripped). +sandlock run \ + --http-allow "* api.internal/*" \ + --http-inject-ca /etc/ssl/certs/ca-certificates.crt \ + --credential api=file:/vault/secrets/api-key \ + --http-auth "* api.internal/* bearer api" \ + -r /usr -r /lib -r /etc -- python3 agent.py +``` + +Dynamic/leased secrets (Vault leases with a TTL, rotating keys) are out of scope: +the secret is loaded once at supervisor start, so a rotated value is picked up +only on the next `sandlock run`. + ## `[determinism]` Knobs that pin sources of non-determinism in the child process. From 00b83b81884e0ce53aea0178af50d3f272c069b0 Mon Sep 17 00:00:00 2001 From: dzerik Date: Sat, 11 Jul 2026 17:37:30 +0300 Subject: [PATCH 4/5] credential: fix cross-origin exfiltration + strip all declared env credentials Review follow-ups: - SECURITY: reject a split-host request in the proxy. handle() keyed the ACL check, host verification, and the credential match on the URI-authority host but rebuilt the outbound request from the Host header, and the upstream client routes by that. A child could send `GET https://allowed/...` with `Host: attacker` to have the secret injected for the allowed host yet forwarded (carrying the credential) to an attacker-chosen one -- cross-origin credential exfiltration plus an egress-ACL bypass. Fail closed when the URI host and the Host header disagree (split_host_rejected), before verify/ACL/inject. - Strip the env var of EVERY declared `env:` credential from the child, not only the ones an --http-auth rule references. A declared-but-unreferenced `--credential X=env:SECRET` was left in the child's environment (readable directly) and could also reach the on-disk checkpoint image via the child's memory. Populating env_strip from all declarations closes both. - Warn when a `file:` credential path sits inside an fs-read grant (the child can read it directly) -- env: is stripped and fd: is dup-read, but file: had no backstop. - apply() now returns Injected/Skipped so an add-only no-op is logged truthfully ("kept caller-supplied credential") instead of a false "injected" audit line. Unit tests: split-host guard, unreferenced-env strip. --- crates/sandlock-core/src/credential.rs | 70 +++++++--- crates/sandlock-core/src/sandbox/builder.rs | 27 ++++ .../src/transparent_proxy/service.rs | 128 ++++++++++++++---- 3 files changed, 181 insertions(+), 44 deletions(-) diff --git a/crates/sandlock-core/src/credential.rs b/crates/sandlock-core/src/credential.rs index 0f53d294..bc9a17d9 100644 --- a/crates/sandlock-core/src/credential.rs +++ b/crates/sandlock-core/src/credential.rs @@ -205,6 +205,18 @@ impl std::fmt::Debug for InjectRule { } } +/// What [`InjectRule::apply`] did on the `Ok` path — so the caller logs a +/// truthful audit line instead of claiming "injected" when an `add-only` rule +/// actually left the caller's own credential in place. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Applied { + /// The secret was rendered into the request. + Injected, + /// `add-only` and the request already carried the target header/param, so + /// the caller's value was kept and no secret was written. + Skipped, +} + impl InjectRule { /// True if this rule applies to the given request. pub fn matches(&self, method: &str, host: &str, path: &str) -> bool { @@ -214,16 +226,17 @@ impl InjectRule { /// Render the secret into `parts` (header or query), honoring `on_existing`. /// The rendered header is marked sensitive so downstream logging redacts it. /// - /// `Err(())` means the secret could not be rendered (e.g. it contains bytes - /// illegal in an HTTP header value) — the caller must reject the request - /// rather than forward it with no (or a partial) credential. A no-op skip - /// under `AddOnly` (the agent already set the header) is `Ok`. + /// `Ok(Injected)` wrote the secret; `Ok(Skipped)` left a caller-supplied + /// value in place under `add-only`. `Err(())` means the secret could not be + /// rendered (e.g. it contains bytes illegal in an HTTP header value) — the + /// caller must reject the request rather than forward it with no (or a + /// partial) credential. /// /// Zeroing is best-effort: [`SecretString`] itself is wiped on drop, but the /// transient buffers built here (the `Bearer `/`Basic ` byte vecs, the base64 /// string, the query pair) hold a copy of the secret and are dropped without /// volatile zeroing, so a copy briefly lives in freed heap. - pub fn apply(&self, parts: &mut hyper::http::request::Parts) -> Result<(), ()> { + pub fn apply(&self, parts: &mut hyper::http::request::Parts) -> Result { match &self.auth { AuthShape::Bearer => { let mut v = b"Bearer ".to_vec(); @@ -248,18 +261,18 @@ impl InjectRule { parts: &mut hyper::http::request::Parts, name: &str, value: &[u8], - ) -> Result<(), ()> { + ) -> Result { let hn = hyper::header::HeaderName::from_bytes(name.as_bytes()).map_err(|_| ())?; if self.on_existing == OnExistingHeader::AddOnly && parts.headers.contains_key(&hn) { - return Ok(()); // agent already set it — leave it, not a failure + return Ok(Applied::Skipped); // agent already set it — leave it, not a failure } let mut hv = hyper::header::HeaderValue::from_bytes(value).map_err(|_| ())?; hv.set_sensitive(true); parts.headers.insert(hn, hv); - Ok(()) + Ok(Applied::Injected) } - fn set_query(&self, parts: &mut hyper::http::request::Parts, param: &str) -> Result<(), ()> { + fn set_query(&self, parts: &mut hyper::http::request::Parts, param: &str) -> Result { let uri = &parts.uri; let path = uri.path(); let existing = uri.query(); @@ -268,7 +281,7 @@ impl InjectRule { if self.on_existing == OnExistingHeader::AddOnly { if let Some(q) = existing { if q.split('&').any(|kv| kv.starts_with(&prefix)) { - return Ok(()); + return Ok(Applied::Skipped); } } } @@ -298,7 +311,7 @@ impl InjectRule { b = b.authority(a.clone()); } parts.uri = b.path_and_query(new_pq).build().map_err(|_| ())?; - Ok(()) + Ok(Applied::Injected) } } @@ -408,14 +421,23 @@ pub fn resolve_inject_rules( // names of `env:` sources so the child can be denied them — otherwise the // agent would just read the value straight from its own environment. let mut loaded: HashMap<&str, Arc> = HashMap::new(); - let mut env_strip: Vec = Vec::new(); for p in &parsed { if !loaded.contains_key(p.name) { loaded.insert(p.name, Arc::new(load_secret(p.source)?)); - if let Some(var) = p.source.strip_prefix("env:") { - if !env_strip.iter().any(|v| v == var) { - env_strip.push(var.to_string()); - } + } + } + + // Strip the env var of EVERY declared `env:` credential from the child — + // including one no `--http-auth` rule references. Declaring + // `--credential X=env:VAR` is the signal that VAR is a secret; stripping + // only the *referenced* ones would leave the child able to read the value + // straight from its own environment whenever the rule was omitted, typo'd, + // or commented out — handing it the exact secret the feature withholds. + let mut env_strip: Vec = Vec::new(); + for source in sources.values() { + if let Some(var) = source.strip_prefix("env:") { + if !env_strip.iter().any(|v| v == var) { + env_strip.push(var.to_string()); } } } @@ -625,6 +647,22 @@ mod tests { std::env::remove_var("SANDLOCK_TEST_RESOLVE"); } + #[test] + fn resolve_strips_declared_but_unreferenced_env_credential() { + // A credential declared via env: but referenced by no --http-auth rule + // must STILL be stripped from the child — otherwise the child reads the + // secret straight from its own environment. The var need not even be set + // (unreferenced credentials are not loaded), so this is pure config. + let creds = vec!["unused=env:SANDLOCK_TEST_UNREF".to_string()]; + let (rules, env_strip) = resolve_inject_rules(&creds, &[]).unwrap(); + assert!(rules.is_empty(), "no rule references the credential"); + assert_eq!( + env_strip, + vec!["SANDLOCK_TEST_UNREF".to_string()], + "the declared env var must be stripped even though it is unused" + ); + } + #[test] fn resolve_rejects_undeclared_and_std_fd() { assert!(resolve_inject_rules(&[], &["GET x/* bearer missing".to_string()]).is_err()); diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index d4b62ced..aa39c2fd 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -742,6 +742,33 @@ impl SandboxBuilder { and are sent without the credential" ); } + // A `file:` credential is the one source with no automatic backstop: + // `env:` vars are stripped from the child and `fd:` is read through a + // dup, but a secret file sitting inside an fs-read grant is `cat`-able by + // the child directly. We can't safely auto-deny (the grant is often a + // broad dir the workload needs), so warn on the overlap, best-effort. + for c in &self.credentials { + let Some(path) = c.split_once('=').and_then(|(_, s)| s.strip_prefix("file:")) else { + continue; + }; + let secret = std::path::Path::new(path); + let secret_abs = secret.canonicalize(); + let secret_ref = secret_abs.as_deref().unwrap_or(secret); + for grant in &self.fs_readable { + let grant_abs = grant.canonicalize(); + let grant_ref = grant_abs.as_deref().unwrap_or(grant.as_path()); + if secret_ref.starts_with(grant_ref) { + eprintln!( + "sandlock: warning: credential file {} is inside the readable grant {} — \ + the sandboxed child can read the secret directly; keep it outside every \ + fs-read grant (or add an fs-deny for it)", + path, + grant.display() + ); + break; + } + } + } // Resolve credentials + injection rules, loading each secret into the // supervisor. Wrapped in Arc so it flows to the proxy without cloning // the (deliberately non-Clone) secrets. `inject_env_strip` is the set of diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index 66918502..8a3cd413 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -51,6 +51,24 @@ fn first_cleartext_warn(scheme: &str, seen: &AtomicBool) -> bool { scheme == "http" && !seen.swap(true, Ordering::Relaxed) } +/// Whether a request must be rejected because its absolute-form URI authority +/// host disagrees (case-insensitive) with its `Host` header — the split that +/// would otherwise let ACL/verify/inject key off one host while the request is +/// forwarded to the other. Origin-form requests (no URI host) and a missing / +/// unparseable Host header are not rejected. Split out so the guard is +/// unit-tested without a live `Incoming` body. +fn split_host_rejected(uri: &hyper::Uri, headers: &hyper::HeaderMap) -> bool { + let Some(uri_host) = uri.host() else { return false }; + match headers + .get("host") + .and_then(|v| v.to_str().ok()) + .map(|h| h.split(':').next().unwrap_or(h)) + { + Some(hdr_host) => !uri_host.eq_ignore_ascii_case(hdr_host), + None => false, + } +} + impl AclService { pub(crate) fn new( allow: Vec, @@ -131,6 +149,23 @@ impl AclService { .unwrap_or_default(); let path = req.uri().path().to_string(); + // Fail closed on a split-host request: `host` above prefers the URI + // authority, but the outbound request is rebuilt from the Host header + // (`host_hdr` below), and the upstream client routes by that. If the two + // disagree, the ACL check, `verify_host`, and the credential match would + // all key off the URI host while the request — carrying the injected + // secret — is forwarded to the Host-header host. A malicious child could + // then send `GET https://allowed.example/… ` with `Host: attacker` and + // exfiltrate the credential cross-origin (and bypass the egress ACL). + // Requiring the two to agree collapses them to one destination; origin- + // form requests (no URI authority) are unaffected. + if split_host_rejected(req.uri(), req.headers()) { + return text_response( + StatusCode::FORBIDDEN, + "Blocked by sandlock: request-target host does not match the Host header", + ); + } + if !self.verify_host(&client_addr, &host).await { if let Ok(mut m) = self.orig_dest.write() { m.remove(&client_addr); @@ -174,34 +209,48 @@ impl AclService { // the deny path above — and only its name is recorded, never the value. for r in self.inject.iter() { if r.matches(&method, &host, &path) { - if r.apply(&mut parts).is_err() { - // Rendering failed (e.g. the secret has bytes illegal in a - // header) — fail the request rather than forward it with no - // credential, which would look like an auth bug to the caller. - eprintln!( - "sandlock: credential {:?} could not be rendered for {} {}{} — rejecting", - r.name, method, host, path - ); - return text_response( - StatusCode::BAD_GATEWAY, - "Blocked by sandlock: credential could not be applied", - ); - } - eprintln!( - "sandlock: injected credential {:?} for {} {}{}", - r.name, method, host, path - ); - // Fires for any caller (library/API, not just the CLI) since the - // proxy is in core: the scheme is only known here at request time. - // Once per run — over cleartext the secret is exposed on the wire. - if first_cleartext_warn(scheme, &self.cleartext_warned) { - eprintln!( - "sandlock: warning: credential {:?} injected over cleartext HTTP (no TLS) \ - to {} — the secret is exposed on the wire; prefer an HTTPS host \ - (configure MITM via --http-ca / --http-inject-ca)", - r.name, host - ); + match r.apply(&mut parts) { + Err(()) => { + // Rendering failed (e.g. the secret has bytes illegal in a + // header) — fail the request rather than forward it with no + // credential, which would look like an auth bug to the caller. + eprintln!( + "sandlock: credential {:?} could not be rendered for {} {}{} — rejecting", + r.name, method, host, path + ); + return text_response( + StatusCode::BAD_GATEWAY, + "Blocked by sandlock: credential could not be applied", + ); + } + Ok(crate::credential::Applied::Skipped) => { + // add-only and the caller already set the target — keep + // theirs and record it truthfully (not as an injection). + eprintln!( + "sandlock: kept caller-supplied credential {:?} for {} {}{} (add-only)", + r.name, method, host, path + ); + } + Ok(crate::credential::Applied::Injected) => { + eprintln!( + "sandlock: injected credential {:?} for {} {}{}", + r.name, method, host, path + ); + // Fires for any caller (library/API, not just the CLI) since the + // proxy is in core: the scheme is only known here at request time. + // Once per run — over cleartext the secret is exposed on the wire. + if first_cleartext_warn(scheme, &self.cleartext_warned) { + eprintln!( + "sandlock: warning: credential {:?} injected over cleartext HTTP (no TLS) \ + to {} — the secret is exposed on the wire; prefer an HTTPS host \ + (configure MITM via --http-ca / --http-inject-ca)", + r.name, host + ); + } + } } + // First match wins whether it injected or deliberately kept the + // caller's value. break; } } @@ -224,7 +273,7 @@ fn text_response(status: StatusCode, msg: &str) -> Response) -> bool { + let uri: hyper::Uri = uri.parse().unwrap(); + let mut headers = hyper::HeaderMap::new(); + if let Some(h) = host { + headers.insert("host", h.parse().unwrap()); + } + split_host_rejected(&uri, &headers) + } + + #[test] + fn split_host_guard_rejects_only_a_real_mismatch() { + // The exfiltration case: absolute-form allowed host, spoofed Host header. + assert!(rejected("http://allowed.example/v1", Some("attacker.example"))); + // Agreement (incl. case-insensitive, and a port on either side) is fine. + assert!(!rejected("http://allowed.example/v1", Some("allowed.example"))); + assert!(!rejected("http://allowed.example/v1", Some("ALLOWED.EXAMPLE"))); + assert!(!rejected("http://allowed.example/v1", Some("allowed.example:8080"))); + assert!(!rejected("http://allowed.example:443/v1", Some("allowed.example"))); + // Origin-form (no URI authority) and a missing Host header don't reject. + assert!(!rejected("/v1", Some("anything.example"))); + assert!(!rejected("http://allowed.example/v1", None)); + } } From c4c3fd6360c6ed245048f505294d6c50c0ca6221 Mon Sep 17 00:00:00 2001 From: dzerik Date: Sat, 11 Jul 2026 18:09:24 +0300 Subject: [PATCH 5/5] credential: harden split-host guard + close file:/query/fd gaps (audit follow-up) A self-audit of the previous fix found #1 incomplete and surfaced more gaps: - The split-host guard compared uri.host() against a split(':') view of the Host header while the request was forwarded using the full re-parsed Host authority, so `Host: allowed:x@attacker` (userinfo) or an origin-form request still routed the injected secret to an attacker host. Replaced with request_authority(): parse the Host header and any URI authority as http::uri::Authority, reject userinfo and a URI-vs-Host mismatch, and drive verify/ACL/inject AND the outbound request from that ONE authority. Also fixes IPv6-literal handling. - The file: overlap warning now covers every grant that exposes the secret -- fs_writable (write access includes read), fs_mount host paths, and the chroot root -- not just fs_readable. - Query Replace/AddOnly now match the param NAME, so a value-less `?key` is treated as the target instead of appending `key&key=secret` (which a first-occurrence-reading upstream would authenticate with the empty value). - Allow fd:0 (stdin), the most portable secret-passing pattern, rejecting only fd:1/fd:2. Reject --http-auth specs with more than 5 tokens. Unit tests: request_authority (userinfo/split/origin-form rejection), value-less query param. --- crates/sandlock-core/src/credential.rs | 47 ++++-- crates/sandlock-core/src/sandbox/builder.rs | 21 ++- .../src/transparent_proxy/service.rs | 143 ++++++++++-------- 3 files changed, 129 insertions(+), 82 deletions(-) diff --git a/crates/sandlock-core/src/credential.rs b/crates/sandlock-core/src/credential.rs index bc9a17d9..325b72ce 100644 --- a/crates/sandlock-core/src/credential.rs +++ b/crates/sandlock-core/src/credential.rs @@ -122,14 +122,16 @@ fn read_capped(r: &mut R, what: &str) -> Result, Sandb } /// Read a credential from an already-open fd (e.g. a shell `<(...)` process -/// substitution passed as `fd:N`). Reads through a *dup*, so the caller's fd is -/// left open, and refuses the std streams (0/1/2) so a typo like `fd:1` can't -/// close/consume stdout. +/// substitution passed as `fd:N`, or the secret piped on stdin as `fd:0`). Reads +/// through a *dup*, so the caller's fd is left open. `fd:0` (stdin) is allowed — +/// it's the most portable secret-passing pattern (`printf %s "$SECRET" | sandlock +/// … --credential k=fd:0`, docker `-i`, systemd credential fds) — but `fd:1`/`fd:2` +/// are refused so a typo can't close/consume stdout/stderr. fn read_fd(n: i32) -> Result, SandboxError> { use std::os::fd::FromRawFd; - if n <= 2 { + if n == 1 || n == 2 { return Err(SandboxError::Invalid(format!( - "credential fd {n} refers to a standard stream (0/1/2); pass a dedicated fd" + "credential fd {n} refers to stdout/stderr; pass a dedicated fd (or fd:0 for stdin)" ))); } let dup = unsafe { libc::dup(n) }; @@ -276,18 +278,24 @@ impl InjectRule { let uri = &parts.uri; let path = uri.path(); let existing = uri.query(); - let prefix = format!("{}=", urlencode_bytes(param.as_bytes())); + let enc = urlencode_bytes(param.as_bytes()); + // Match on the param NAME (the token before `=`), so a value-less `?key` + // — no `=`, which `starts_with("key=")` would miss — is still recognised + // as the target: otherwise Replace would append `key&key=secret`, and a + // first-occurrence-reading upstream would authenticate with the child's + // empty value instead of the injected one. + let is_target = |kv: &str| kv.split('=').next().unwrap_or(kv) == enc; // Honor AddOnly: don't append a param the request already carries. if self.on_existing == OnExistingHeader::AddOnly { if let Some(q) = existing { - if q.split('&').any(|kv| kv.starts_with(&prefix)) { + if q.split('&').any(is_target) { return Ok(Applied::Skipped); } } } // Percent-encode the raw secret bytes (never lossy-stringify — a binary // key would be corrupted). - let pair = format!("{}{}", prefix, urlencode_bytes(self.secret.expose())); + let pair = format!("{}={}", enc, urlencode_bytes(self.secret.expose())); // Drop any existing occurrence of this param before appending, so // `Replace` actually replaces instead of appending a duplicate (most // frameworks read the first occurrence, so a duplicate would leave the @@ -295,7 +303,7 @@ impl InjectRule { // param is absent, so the filter is a no-op there. let kept: Option = existing.map(|q| { q.split('&') - .filter(|kv| !kv.is_empty() && !kv.starts_with(&prefix)) + .filter(|kv| !kv.is_empty() && !is_target(kv)) .collect::>() .join("&") }); @@ -391,7 +399,7 @@ pub fn resolve_inject_rules( let mut parsed: Vec = Vec::with_capacity(inject.len()); for spec in inject { let toks: Vec<&str> = spec.split_whitespace().collect(); - if toks.len() < 4 { + if toks.len() < 4 || toks.len() > 5 { return Err(SandboxError::Invalid(format!( "--http-auth must be 'METHOD HOST/PATH AUTHSPEC CREDNAME [replace|add-only]', got {spec:?}" ))); @@ -631,6 +639,23 @@ mod tests { assert_eq!(p4.uri.query().unwrap(), "key=sk-real"); } + #[test] + fn query_targets_value_less_param() { + // A bare `?key` (no `=`) is still the target: Replace must drop it and + // inject, not append `key&key=secret` (a first-occurrence-reading upstream + // would authenticate with the child's empty value). + let mut p = parts_of("https://api.example.com/x?key", &[]); + rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::Replace) + .apply(&mut p).unwrap(); + assert_eq!(p.uri.query().unwrap(), "key=sk-real"); + + // AddOnly sees the value-less param as present → keep the child's, skip. + let mut p2 = parts_of("https://api.example.com/x?key", &[]); + let r = rule(AuthShape::Query { param: "key".into() }, "sk-real", OnExistingHeader::AddOnly); + assert!(matches!(r.apply(&mut p2), Ok(Applied::Skipped))); + assert_eq!(p2.uri.query().unwrap(), "key"); + } + #[test] fn resolve_dedups_credentials_and_collects_env_strip() { std::env::set_var("SANDLOCK_TEST_RESOLVE", "sk-resolve"); @@ -667,7 +692,7 @@ mod tests { fn resolve_rejects_undeclared_and_std_fd() { assert!(resolve_inject_rules(&[], &["GET x/* bearer missing".to_string()]).is_err()); assert!(load_secret("fd:1").is_err()); // stdout - assert!(load_secret("fd:0").is_err()); // stdin + assert!(load_secret("fd:2").is_err()); // stderr (fd:0/stdin is now allowed) } #[test] diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index aa39c2fd..7a99e03a 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -744,9 +744,12 @@ impl SandboxBuilder { } // A `file:` credential is the one source with no automatic backstop: // `env:` vars are stripped from the child and `fd:` is read through a - // dup, but a secret file sitting inside an fs-read grant is `cat`-able by - // the child directly. We can't safely auto-deny (the grant is often a - // broad dir the workload needs), so warn on the overlap, best-effort. + // dup, but a secret file sitting inside any grant that lets the child + // reach it is `cat`-able directly. We can't safely auto-deny (the grant + // is often a broad dir the workload needs), so warn on the overlap. Every + // exposing grant is covered: read grants, write grants (write access + // includes read), bind-mounted host dirs, and the chroot root (visible + // regardless of Landlock). Best-effort — canonicalize where possible. for c in &self.credentials { let Some(path) = c.split_once('=').and_then(|(_, s)| s.strip_prefix("file:")) else { continue; @@ -754,14 +757,20 @@ impl SandboxBuilder { let secret = std::path::Path::new(path); let secret_abs = secret.canonicalize(); let secret_ref = secret_abs.as_deref().unwrap_or(secret); - for grant in &self.fs_readable { + let grants = self + .fs_readable + .iter() + .chain(self.fs_writable.iter()) + .chain(self.fs_mount.iter().map(|(_, host)| host)) + .chain(self.chroot.as_ref()); + for grant in grants { let grant_abs = grant.canonicalize(); let grant_ref = grant_abs.as_deref().unwrap_or(grant.as_path()); if secret_ref.starts_with(grant_ref) { eprintln!( - "sandlock: warning: credential file {} is inside the readable grant {} — \ + "sandlock: warning: credential file {} is inside the sandbox grant {} — \ the sandboxed child can read the secret directly; keep it outside every \ - fs-read grant (or add an fs-deny for it)", + fs grant (or add an fs-deny for it)", path, grant.display() ); diff --git a/crates/sandlock-core/src/transparent_proxy/service.rs b/crates/sandlock-core/src/transparent_proxy/service.rs index 8a3cd413..e767b4e7 100644 --- a/crates/sandlock-core/src/transparent_proxy/service.rs +++ b/crates/sandlock-core/src/transparent_proxy/service.rs @@ -51,21 +51,40 @@ fn first_cleartext_warn(scheme: &str, seen: &AtomicBool) -> bool { scheme == "http" && !seen.swap(true, Ordering::Relaxed) } -/// Whether a request must be rejected because its absolute-form URI authority -/// host disagrees (case-insensitive) with its `Host` header — the split that -/// would otherwise let ACL/verify/inject key off one host while the request is -/// forwarded to the other. Origin-form requests (no URI host) and a missing / -/// unparseable Host header are not rejected. Split out so the guard is -/// unit-tested without a live `Incoming` body. -fn split_host_rejected(uri: &hyper::Uri, headers: &hyper::HeaderMap) -> bool { - let Some(uri_host) = uri.host() else { return false }; - match headers - .get("host") - .and_then(|v| v.to_str().ok()) - .map(|h| h.split(':').next().unwrap_or(h)) - { - Some(hdr_host) => !uri_host.eq_ignore_ascii_case(hdr_host), - None => false, +/// The single upstream authority for a request, or `Err` if it is missing, +/// malformed, or ambiguous. +/// +/// Both the absolute-form URI authority and the `Host` header are parsed as an +/// [`Authority`] (not split on the first `:`, which mis-handles userinfo and +/// IPv6). Userinfo (`user@host`) is rejected because its `@` lets the real host +/// diverge from a naive `split(':')` view; when both a URI authority and a Host +/// header are present their hosts must agree. The returned authority — never the +/// raw Host header — drives `verify_host`, the ACL, the credential match, AND the +/// outbound request, so the host a credential is matched for is exactly the host +/// it is sent to. Split out so it is unit-tested without a live `Incoming` body. +fn request_authority( + uri: &hyper::Uri, + headers: &hyper::HeaderMap, +) -> Result { + use hyper::http::uri::Authority; + + let hdr_auth: Option = match headers.get("host") { + Some(v) => Some(v.to_str().map_err(|_| ())?.parse::().map_err(|_| ())?), + None => None, + }; + let uri_auth: Option = uri.authority().cloned(); + + let has_userinfo = |a: &Authority| a.as_str().contains('@'); + if hdr_auth.as_ref().is_some_and(has_userinfo) || uri_auth.as_ref().is_some_and(has_userinfo) { + return Err(()); + } + + match (uri_auth, hdr_auth) { + (Some(u), Some(h)) if u.host().eq_ignore_ascii_case(h.host()) => Ok(u), + (Some(_), Some(_)) => Err(()), // split host: URI authority vs Host header + (Some(u), None) => Ok(u), + (None, Some(h)) => Ok(h), + (None, None) => Err(()), // no host at all } } @@ -136,35 +155,27 @@ impl AclService { req: Request, ) -> Response> { let method = req.method().as_str().to_string(); - let host = req - .uri() - .host() - .map(|h| h.to_string()) - .or_else(|| { - req.headers() - .get("host") - .and_then(|v| v.to_str().ok()) - .map(|h| h.split(':').next().unwrap_or(h).to_string()) - }) - .unwrap_or_default(); - let path = req.uri().path().to_string(); - // Fail closed on a split-host request: `host` above prefers the URI - // authority, but the outbound request is rebuilt from the Host header - // (`host_hdr` below), and the upstream client routes by that. If the two - // disagree, the ACL check, `verify_host`, and the credential match would - // all key off the URI host while the request — carrying the injected - // secret — is forwarded to the Host-header host. A malicious child could - // then send `GET https://allowed.example/… ` with `Host: attacker` and - // exfiltrate the credential cross-origin (and bypass the egress ACL). - // Requiring the two to agree collapses them to one destination; origin- - // form requests (no URI authority) are unaffected. - if split_host_rejected(req.uri(), req.headers()) { - return text_response( - StatusCode::FORBIDDEN, - "Blocked by sandlock: request-target host does not match the Host header", - ); - } + // Resolve ONE authority for the whole request (see `request_authority`). + // `verify_host`, the ACL, the credential match, and the outbound request + // are all keyed off this single parsed value, so the host a credential is + // matched for is exactly the host it is forwarded to. This closes the + // split-host exfiltration where a child sends an allowed URI/Host but + // smuggles a different forward host (e.g. via userinfo `a:b@attacker`). + let authority = match request_authority(req.uri(), req.headers()) { + Ok(a) => a, + Err(()) => { + if let Ok(mut m) = self.orig_dest.write() { + m.remove(&client_addr); + } + return text_response( + StatusCode::FORBIDDEN, + "Blocked by sandlock: missing, malformed, or mismatched request host", + ); + } + }; + let host = authority.host().to_string(); + let path = req.uri().path().to_string(); if !self.verify_host(&client_addr, &host).await { if let Ok(mut m) = self.orig_dest.write() { @@ -183,20 +194,16 @@ impl AclService { return text_response(StatusCode::FORBIDDEN, "Blocked by sandlock HTTP ACL policy"); } - // Rebuild an absolute-URI request for the upstream client. - let host_hdr = req - .headers() - .get("host") - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_string()) - .unwrap_or_else(|| host.clone()); + // 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() .path_and_query() .map(|p| p.as_str()) .unwrap_or("/") .to_string(); - let uri: hyper::Uri = match format!("{scheme}://{host_hdr}{pq}").parse() { + let uri: hyper::Uri = match format!("{scheme}://{authority}{pq}").parse() { Ok(u) => u, Err(_) => return text_response(StatusCode::BAD_GATEWAY, "bad upstream URI"), }; @@ -273,7 +280,7 @@ fn text_response(status: StatusCode, msg: &str) -> Response) -> bool { + fn authority_of(uri: &str, host: Option<&str>) -> Result { let uri: hyper::Uri = uri.parse().unwrap(); let mut headers = hyper::HeaderMap::new(); if let Some(h) = host { headers.insert("host", h.parse().unwrap()); } - split_host_rejected(&uri, &headers) + request_authority(&uri, &headers).map(|a| a.to_string()) } #[test] - fn split_host_guard_rejects_only_a_real_mismatch() { - // The exfiltration case: absolute-form allowed host, spoofed Host header. - assert!(rejected("http://allowed.example/v1", Some("attacker.example"))); - // Agreement (incl. case-insensitive, and a port on either side) is fine. - assert!(!rejected("http://allowed.example/v1", Some("allowed.example"))); - assert!(!rejected("http://allowed.example/v1", Some("ALLOWED.EXAMPLE"))); - assert!(!rejected("http://allowed.example/v1", Some("allowed.example:8080"))); - assert!(!rejected("http://allowed.example:443/v1", Some("allowed.example"))); - // Origin-form (no URI authority) and a missing Host header don't reject. - assert!(!rejected("/v1", Some("anything.example"))); - assert!(!rejected("http://allowed.example/v1", None)); + fn request_authority_collapses_to_one_safe_host() { + // Agreement, origin-form, a port, and a case-only difference all resolve. + assert_eq!(authority_of("http://allowed.example/v1", Some("allowed.example")).unwrap(), "allowed.example"); + assert_eq!(authority_of("/v1", Some("allowed.example")).unwrap(), "allowed.example"); + assert_eq!(authority_of("http://allowed.example/v1", Some("ALLOWED.EXAMPLE")).unwrap(), "allowed.example"); + assert_eq!(authority_of("/v1", Some("allowed.example:443")).unwrap(), "allowed.example:443"); + + // The exfiltration vectors are all rejected: + // absolute-form URI host vs a spoofed Host header, + assert!(authority_of("http://allowed.example/v1", Some("attacker.example")).is_err()); + // and userinfo smuggling that hides the real forward host behind `@`, + // in both origin-form and absolute-form. + assert!(authority_of("/v1", Some("allowed.example:x@attacker.example")).is_err()); + assert!(authority_of("http://allowed.example/v1", Some("allowed.example:x@attacker.example")).is_err()); + + // A request with no host at all is rejected (fail closed). + assert!(authority_of("/v1", None).is_err()); } }