diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index ebd47e2251..014ded65a4 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -5,15 +5,51 @@ use std::path::Path; const LOCAL_NO_PROXY: &str = "127.0.0.1,localhost,::1"; +/// Build the NO_PROXY value by combining localhost entries with any hosts +/// listed in `OPENSHELL_DIRECT_TCP_HOSTS` / `OPENSHELL_DIRECT_TCP_ENDPOINTS`. +/// Those hosts have iptables ACCEPT rules for direct TCP (set up by netns), +/// so HTTP clients must also skip the proxy to avoid TLS termination issues +/// with non-Node binaries (e.g. Rust/rustls programs that cannot trust the +/// egress proxy CA). +fn build_no_proxy() -> String { + let mut no_proxy = LOCAL_NO_PROXY.to_owned(); + let mut push_host = |raw: &str| { + let host = raw.trim(); + if host.is_empty() { + return; + } + no_proxy.push(','); + no_proxy.push_str(host); + }; + if let Ok(hosts) = std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { + for host in hosts.split(',') { + push_host(host); + } + } + if let Ok(endpoints) = std::env::var("OPENSHELL_DIRECT_TCP_ENDPOINTS") { + for entry in endpoints.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + continue; + } + let host = entry.rsplit_once(':').map(|(h, _)| h).unwrap_or(entry); + let host = host.trim().trim_start_matches('[').trim_end_matches(']'); + push_host(host); + } + } + no_proxy +} + pub(crate) fn proxy_env_vars(proxy_url: &str) -> [(&'static str, String); 9] { + let no_proxy = build_no_proxy(); [ ("ALL_PROXY", proxy_url.to_owned()), ("HTTP_PROXY", proxy_url.to_owned()), ("HTTPS_PROXY", proxy_url.to_owned()), - ("NO_PROXY", LOCAL_NO_PROXY.to_owned()), + ("NO_PROXY", no_proxy.clone()), ("http_proxy", proxy_url.to_owned()), ("https_proxy", proxy_url.to_owned()), - ("no_proxy", LOCAL_NO_PROXY.to_owned()), + ("no_proxy", no_proxy), ("grpc_proxy", proxy_url.to_owned()), // Node.js only honors HTTP(S)_PROXY for built-in fetch/http clients when // proxy support is explicitly enabled at process startup. @@ -35,6 +71,18 @@ pub(crate) fn tls_env_vars( ] } +/// Serializes tests that read or write `OPENSHELL_DIRECT_TCP_*`. +/// +/// `std::env::set_var` / `remove_var` are unsafe because a concurrent read or +/// write of the environment is undefined behavior. Tests run in parallel by +/// default, and the tests in this module and in `sandbox::linux::netns` touch +/// the same variables, so every such test holds this lock for its whole body. +#[cfg(test)] +pub(crate) fn lock_direct_tcp_env() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + #[cfg(test)] mod tests { use super::*; @@ -43,6 +91,11 @@ mod tests { #[test] fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() { + let _env = lock_direct_tcp_env(); + // Ensure no leftover env from other tests affects NO_PROXY + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + let mut cmd = Command::new("/usr/bin/env"); cmd.stdin(Stdio::null()) .stdout(Stdio::piped()) @@ -61,6 +114,49 @@ mod tests { assert!(stdout.contains("no_proxy=127.0.0.1,localhost,::1")); } + #[test] + fn no_proxy_includes_direct_tcp_hosts() { + let _env = lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var( + "OPENSHELL_DIRECT_TCP_HOSTS", + "oauth2.googleapis.com,gmail.googleapis.com", + ) }; + + let no_proxy = build_no_proxy(); + assert_eq!( + no_proxy, + "127.0.0.1,localhost,::1,oauth2.googleapis.com,gmail.googleapis.com" + ); + + // Clean up + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + } + + #[test] + fn no_proxy_includes_direct_tcp_endpoints() { + let _env = lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", + ) }; + + let no_proxy = build_no_proxy(); + assert_eq!( + no_proxy, + "127.0.0.1,localhost,::1,10.0.1.215,10.0.1.215,db.internal" + ); + + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + } + #[test] fn apply_tls_env_sets_node_and_bundle_paths() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index f9e8fb4c54..fe67ce8483 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -1149,8 +1149,14 @@ const PROXY_BASELINE_READ_ONLY: &[&str] = &[ ]; /// Minimum read-write paths required for a proxy-mode sandbox child process: -/// user working directory and temporary files. -const PROXY_BASELINE_READ_WRITE: &[&str] = &["/sandbox", "/tmp"]; +/// user working directory, temporary files, and PTY devices. +/// +/// `/dev/ptmx` and `/dev/pts`: VS Code Remote-SSH launches its server under the +/// sandbox policy, and the server later allocates PTYs for the integrated +/// terminal via `node-pty`. Landlock blocks device-file opens unless they are +/// explicitly whitelisted, so PTY allocation fails with `EACCES` unless both +/// the PTY multiplexer and the slave PTY directory are writable. +const PROXY_BASELINE_READ_WRITE: &[&str] = &["/sandbox", "/tmp", "/dev/ptmx", "/dev/pts"]; /// GPU read-only paths. /// @@ -1388,10 +1394,45 @@ mod baseline_tests { } #[test] - fn baseline_read_write_always_includes_sandbox_and_tmp() { + fn baseline_read_write_includes_core_runtime_and_pty_paths() { let (_ro, rw) = baseline_enrichment_paths(); assert!(rw.contains(&"/sandbox".to_string())); assert!(rw.contains(&"/tmp".to_string())); + assert!(rw.contains(&"/dev/ptmx".to_string())); + assert!(rw.contains(&"/dev/pts".to_string())); + } + + #[test] + fn enrich_proto_baseline_paths_adds_pty_paths_for_proxy_mode() { + let mut policy = openshell_core::proto::SandboxPolicy::default(); + policy.network_policies.insert( + "test".to_string(), + openshell_core::proto::NetworkPolicyRule::default(), + ); + + let modified = enrich_proto_baseline_paths(&mut policy); + assert!(modified, "proxy-mode policy should be enriched"); + + let fs = policy + .filesystem + .as_ref() + .expect("filesystem policy should be created during enrichment"); + assert!( + fs.read_write.iter().any(|p| p == "/sandbox"), + "proxy baseline should include /sandbox" + ); + assert!( + fs.read_write.iter().any(|p| p == "/tmp"), + "proxy baseline should include /tmp" + ); + assert!( + fs.read_write.iter().any(|p| p == "/dev/ptmx"), + "proxy baseline should include /dev/ptmx" + ); + assert!( + fs.read_write.iter().any(|p| p == "/dev/pts"), + "proxy baseline should include /dev/pts" + ); } #[test] @@ -1406,6 +1447,15 @@ mod baseline_tests { ); } + #[test] + fn runtime_device_paths_are_not_prepared_for_chown() { + assert!(is_runtime_device_path(std::path::Path::new("/dev/ptmx"))); + assert!(is_runtime_device_path(std::path::Path::new("/dev/pts"))); + assert!(is_runtime_device_path(std::path::Path::new("/proc"))); + assert!(!is_runtime_device_path(std::path::Path::new("/sandbox"))); + assert!(!is_runtime_device_path(std::path::Path::new("/tmp"))); + } + #[test] fn no_duplicate_paths_in_baseline() { let (ro, rw) = baseline_enrichment_paths(); @@ -1750,11 +1800,25 @@ fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { // (e.g. /dev/null) are legitimate read_write entries and must be allowed. if let Ok(meta) = std::fs::symlink_metadata(path) { if meta.file_type().is_symlink() { + if is_runtime_device_path(path) { + debug!( + path = %path.display(), + "Skipping ownership change on runtime device symlink" + ); + continue; + } return Err(miette::miette!( "read_write path '{}' is a symlink — refusing to chown (potential privilege escalation)", path.display() )); } + if is_runtime_device_path(path) { + debug!( + path = %path.display(), + "Skipping ownership change on runtime device path" + ); + continue; + } } else { debug!(path = %path.display(), "Creating read_write directory"); std::fs::create_dir_all(path).into_diagnostic()?; @@ -1767,6 +1831,13 @@ fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { Ok(()) } +#[cfg(unix)] +fn is_runtime_device_path(path: &std::path::Path) -> bool { + path.starts_with(std::path::Path::new("/dev")) + || path.starts_with(std::path::Path::new("/proc")) + || path.starts_with(std::path::Path::new("/sys")) +} + #[cfg(not(unix))] fn prepare_filesystem(_policy: &SandboxPolicy) -> Result<()> { Ok(()) diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index b52cc60b90..8cd561a524 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -1916,10 +1916,15 @@ fn rewrite_forward_request( output.extend_from_slice(&raw[header_end..used]); } - // Fail-closed: scan for any remaining unresolved placeholders + // Fail-closed: scan for any remaining unresolved placeholders or + // provider-shaped aliases, including the percent-decoded request line. if secret_resolver.is_some() { let output_str = String::from_utf8_lossy(&output); - if output_str.contains(crate::secrets::PLACEHOLDER_PREFIX_PUBLIC) { + let request_line = output_str.split("\r\n").next().unwrap_or(""); + if crate::secrets::request_line_has_credential_marker(request_line) { + return Err(crate::secrets::UnresolvedPlaceholderError { location: "path" }); + } + if crate::secrets::contains_credential_marker(&output_str) { return Err(crate::secrets::UnresolvedPlaceholderError { location: "header" }); } } @@ -3170,6 +3175,59 @@ mod tests { assert!(!result_str.contains("openshell:resolve:env:ANTHROPIC_API_KEY")); } + fn github_alias_resolver() -> Option { + SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "github_pat_REAL".to_string())] + .into_iter() + .collect(), + ) + .1 + } + + #[test] + fn test_rewrite_resolves_provider_alias_auth_header() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN\r\n\r\n"; + let result = rewrite_forward_request(raw, raw.len(), "/p", resolver.as_ref()) + .expect("should succeed"); + let result_str = String::from_utf8_lossy(&result); + assert!(result_str.contains("Authorization: token github_pat_REAL")); + assert!(!result_str.contains("OPENSHELL-RESOLVE-ENV-")); + } + + #[test] + fn test_rewrite_rejects_unknown_provider_alias() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN\r\n\r\n"; + let err = rewrite_forward_request(raw, raw.len(), "/p", resolver.as_ref()) + .expect_err("unresolved alias must fail closed"); + assert_eq!(err.location, "header"); + } + + #[test] + fn test_rewrite_rejects_provider_alias_in_path() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\n\r\n"; + assert!( + rewrite_forward_request( + raw, + raw.len(), + "/p?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", + resolver.as_ref(), + ) + .is_err() + ); + assert!( + rewrite_forward_request( + raw, + raw.len(), + "/p?t=github_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN", + resolver.as_ref(), + ) + .is_err() + ); + } + // --- Forward proxy SSRF defence tests --- // // The forward proxy handler uses the same SSRF logic as the CONNECT path: diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index 37d11f0c3e..3d315cf299 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -19,6 +19,128 @@ const SUBNET_PREFIX: &str = "10.200.0"; const HOST_IP_SUFFIX: u8 = 1; const SANDBOX_IP_SUFFIX: u8 = 2; +/// Parse the `OPENSHELL_DIRECT_TCP_HOSTS` environment variable into a list of +/// hostnames. Returns an empty vec if the variable is unset or empty. +fn parse_direct_tcp_hosts() -> Vec { + let hosts = match std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { + Ok(val) if !val.is_empty() => val, + _ => return Vec::new(), + }; + hosts + .split(',') + .map(|h| h.trim().to_owned()) + .filter(|h| !h.is_empty()) + .collect() +} + +/// A single `host:port` endpoint for direct TCP bypass. +#[derive(Debug, Clone, PartialEq, Eq)] +struct DirectTcpEndpoint { + host: String, + port: u16, +} + +/// Parse `OPENSHELL_DIRECT_TCP_ENDPOINTS` (comma-separated `host:port` pairs). +/// +/// Each entry bypasses the egress proxy with per-endpoint iptables ACCEPT on +/// the sandbox side and MASQUERADE + FORWARD on the host side. Use for arbitrary +/// TCP ports beyond 443 (postgres 5432, redis 6379, smtp 1025, etc.) that the +/// egress proxy rejects or that raw-TCP clients need. +/// +/// `host` may be an IPv4 literal or a hostname. Hostnames are resolved at pod +/// startup via the cluster resolver. +fn parse_direct_tcp_endpoints() -> Vec { + let raw = match std::env::var("OPENSHELL_DIRECT_TCP_ENDPOINTS") { + Ok(val) if !val.is_empty() => val, + _ => return Vec::new(), + }; + let mut out = Vec::new(); + for entry in raw.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + continue; + } + let Some((host, port)) = entry.rsplit_once(':') else { + warn!(entry = %entry, "OPENSHELL_DIRECT_TCP_ENDPOINTS entry missing ':port'"); + continue; + }; + let host = host.trim().trim_start_matches('[').trim_end_matches(']'); + let port: u16 = match port.trim().parse() { + Ok(p) => p, + Err(e) => { + warn!(entry = %entry, error = %e, "Invalid port in OPENSHELL_DIRECT_TCP_ENDPOINTS"); + continue; + } + }; + if host.is_empty() { + continue; + } + out.push(DirectTcpEndpoint { host: host.to_owned(), port }); + } + out +} + +/// Resolve a direct-TCP endpoint to one or more IPv4 addresses. +/// +/// IPv4 literals pass through unchanged. Hostnames are resolved via the system +/// resolver (running in the pod netns where cluster DNS works). IPv6 addresses +/// are dropped — sandbox netns rules are IPv4-only. +fn resolve_endpoint_ipv4s(ep: &DirectTcpEndpoint) -> Vec { + if let Ok(addr) = ep.host.parse::() { + return vec![addr]; + } + match std::net::ToSocketAddrs::to_socket_addrs(&(ep.host.as_str(), ep.port)) { + Ok(iter) => { + let mut seen = Vec::new(); + for sa in iter { + if let std::net::IpAddr::V4(ip) = sa.ip() { + if !seen.contains(&ip) { + seen.push(ip); + } + } + } + if seen.is_empty() { + warn!(host = %ep.host, "No IPv4 address resolved for direct-TCP endpoint"); + } + seen + } + Err(e) => { + warn!(host = %ep.host, error = %e, "Failed to resolve direct-TCP endpoint"); + Vec::new() + } + } +} + +/// Resolve the cluster DNS server IP for the iptables ACCEPT rule. +/// +/// Priority: +/// 1. `OPENSHELL_DNS_SERVER` environment variable (operator override) +/// 2. First `nameserver` entry in `/etc/resolv.conf` +/// +/// Returns `None` if neither source provides a valid IP, in which case +/// no DNS ACCEPT rule will be installed and UDP DNS remains blocked. +fn resolve_dns_server() -> Option { + if let Ok(val) = std::env::var("OPENSHELL_DNS_SERVER") { + if let Ok(addr) = val.parse::() { + return Some(addr); + } + warn!(value = %val, "OPENSHELL_DNS_SERVER is not a valid IP address, ignoring"); + } + + if let Ok(contents) = std::fs::read_to_string("/etc/resolv.conf") { + for line in contents.lines() { + let line = line.trim(); + if let Some(rest) = line.strip_prefix("nameserver") { + if let Ok(addr) = rest.trim().parse::() { + return Some(addr); + } + } + } + } + + None +} + /// Handle to a network namespace with veth pair. /// /// The namespace and veth interfaces are automatically cleaned up on drop. @@ -315,6 +437,98 @@ impl NetworkNamespace { } } + // Enable IP forwarding and NAT on the host side of the veth for DNS. + if let Some(dns_ip) = resolve_dns_server() { + let dns_ip_str = dns_ip.to_string(); + let sandbox_ip_str = self.sandbox_ip.to_string(); + + let forwarding_path = format!( + "/proc/sys/net/ipv4/conf/{}/forwarding", + self.veth_host + ); + if let Err(e) = std::fs::write(&forwarding_path, "1") { + warn!( + error = %e, + path = %forwarding_path, + "Failed to enable IP forwarding on host veth (DNS may not work)" + ); + } + let _ = std::fs::write("/proc/sys/net/ipv4/ip_forward", "1"); + + let dns_cidr = format!("{dns_ip_str}/32"); + let sandbox_cidr = format!("{sandbox_ip_str}/32"); + let _ = Command::new(&iptables_path) + .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-d", &dns_cidr, "-p", "udp", "--dport", "53", "-j", "MASQUERADE"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-d", &dns_cidr, "-p", "udp", "--dport", "53", "-j", "ACCEPT"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-m", "state", "--state", "ESTABLISHED,RELATED", "-j", "ACCEPT"]) + .output(); + + info!( + dns_server = %dns_ip_str, + veth = %self.veth_host, + "Enabled DNS forwarding from sandbox to cluster nameserver" + ); + } + + // Host-side forwarding for direct TCP 443 (OPENSHELL_DIRECT_TCP_HOSTS). + let direct_tcp_hosts = parse_direct_tcp_hosts(); + if !direct_tcp_hosts.is_empty() { + let sandbox_cidr = format!("{}/32", self.sandbox_ip); + let _ = Command::new(&iptables_path) + .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-p", "tcp", "--dport", "443", "-j", "MASQUERADE"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT"]) + .output(); + info!( + hosts = direct_tcp_hosts.len(), + "Enabled broad TCP 443 forwarding for OPENSHELL_DIRECT_TCP_HOSTS" + ); + } + + // Host-side forwarding for OPENSHELL_DIRECT_TCP_ENDPOINTS (host:port + // pairs). Per-endpoint MASQUERADE + FORWARD on the specific dest IP and + // port so the sandbox can reach services on the pod host (e.g. postgres + // 5432, redis 6379) without going through the egress proxy — which + // blocks well-known DB ports and does not handle raw TCP protocols. + let direct_tcp_endpoints = parse_direct_tcp_endpoints(); + if !direct_tcp_endpoints.is_empty() { + let sandbox_cidr = format!("{}/32", self.sandbox_ip); + let mut installed = 0usize; + for ep in &direct_tcp_endpoints { + let port_str = ep.port.to_string(); + for ip in resolve_endpoint_ipv4s(ep) { + let ip_cidr = format!("{ip}/32"); + let _ = Command::new(&iptables_path) + .args([ + "-t", "nat", "-A", "POSTROUTING", + "-s", &sandbox_cidr, "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "MASQUERADE", + ]) + .output(); + let _ = Command::new(&iptables_path) + .args([ + "-A", "FORWARD", + "-s", &sandbox_cidr, "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "ACCEPT", + ]) + .output(); + installed += 1; + } + } + info!( + endpoints = direct_tcp_endpoints.len(), + rules = installed, + "Enabled direct TCP forwarding for OPENSHELL_DIRECT_TCP_ENDPOINTS" + ); + } + openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(crate::ocsf_ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -416,6 +630,73 @@ impl NetworkNamespace { .build()); } + // Rule 4.5: ACCEPT all TCP 443 when OPENSHELL_DIRECT_TCP_HOSTS is set. + // + // Some binaries (e.g. Rust/rustls programs like `gws`) cannot trust the + // egress proxy's TLS-terminating CA and need direct TCP 443 connections. + // Rather than tracking per-IP rules (which break when DNS round-robin + // returns new IPs), we ACCEPT all outbound TCP 443 from the sandbox. + // + // Security: applications still use HTTPS_PROXY for hosts not in NO_PROXY. + // This rule only affects the iptables layer — it means processes that + // intentionally bypass the proxy env vars can reach any HTTPS endpoint + // directly, which is an acceptable trade-off given the proxy cannot + // inspect TLS content anyway (HTTP CONNECT tunnel). + if !parse_direct_tcp_hosts().is_empty() { + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &["-A", "OUTPUT", "-p", "tcp", "--dport", "443", "-j", "ACCEPT"], + ) { + warn!(error = %e, "Failed to install TCP 443 ACCEPT rule"); + } else { + info!("Installed broad TCP 443 ACCEPT rule for OPENSHELL_DIRECT_TCP_HOSTS"); + } + } + + // Rule 4.6: ACCEPT per-endpoint direct TCP for OPENSHELL_DIRECT_TCP_ENDPOINTS. + // + // Unlike DIRECT_TCP_HOSTS (broad TCP 443), endpoints are dest-IP + dport + // pairs — required for non-HTTPS services the proxy can't/won't handle: + // postgres/redis wire protocols, and ports the proxy explicitly blocks + // (e.g. 5432/6379 hardcoded). + let endpoints = parse_direct_tcp_endpoints(); + if !endpoints.is_empty() { + let mut accepted = 0usize; + for ep in &endpoints { + let port_str = ep.port.to_string(); + for ip in resolve_endpoint_ipv4s(ep) { + let ip_cidr = format!("{ip}/32"); + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &[ + "-A", "OUTPUT", + "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "ACCEPT", + ], + ) { + warn!( + error = %e, + ip = %ip, + port = ep.port, + "Failed to install direct TCP endpoint ACCEPT rule" + ); + } else { + accepted += 1; + } + } + } + if accepted > 0 { + info!( + rules = accepted, + endpoints = endpoints.len(), + "Installed direct TCP endpoint ACCEPT rules for OPENSHELL_DIRECT_TCP_ENDPOINTS" + ); + } + } + // Rule 5: REJECT TCP bypass attempts (fast-fail) run_iptables_netns( &self.name, @@ -432,6 +713,33 @@ impl NetworkNamespace { ], )?; + // Rule 5.5: ACCEPT DNS (UDP port 53) to the cluster nameserver. + // + // Some libraries (e.g. Node.js `ws`, used by @slack/socket-mode) + // resolve hostnames directly via the system resolver, bypassing + // HTTP_PROXY / HTTPS_PROXY. Allow UDP DNS to the nameserver + // configured in /etc/resolv.conf so that resolution succeeds + // without opening a broad UDP hole. + if let Some(dns_ip) = resolve_dns_server() { + let dns_ip_cidr = format!("{dns_ip}/32"); + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &[ + "-A", "OUTPUT", "-d", &dns_ip_cidr, "-p", "udp", "--dport", "53", "-j", + "ACCEPT", + ], + ) { + warn!( + error = %e, + dns_server = %dns_ip, + "Failed to install DNS ACCEPT rule (non-fatal, UDP DNS will be rejected)" + ); + } else { + info!(dns_server = %dns_ip, "Installed DNS ACCEPT rule for UDP port 53"); + } + } + // Rule 6: LOG UDP bypass attempts (rate-limited, covers DNS bypass) if let Err(e) = run_iptables_netns( &self.name, @@ -842,6 +1150,85 @@ mod tests { // These tests require root and network namespace support // Run with: sudo cargo test -- --ignored + #[test] + fn test_parse_direct_tcp_hosts() { + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var( + "OPENSHELL_DIRECT_TCP_HOSTS", + "oauth2.googleapis.com, gmail.googleapis.com , ", + ) }; + let hosts = parse_direct_tcp_hosts(); + assert_eq!(hosts, vec!["oauth2.googleapis.com", "gmail.googleapis.com"]); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + } + + #[test] + fn test_parse_direct_tcp_hosts_empty() { + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + assert!(parse_direct_tcp_hosts().is_empty()); + + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_HOSTS", "") }; + assert!(parse_direct_tcp_hosts().is_empty()); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + } + + #[test] + fn test_parse_direct_tcp_endpoints_basic() { + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", + ) }; + let eps = parse_direct_tcp_endpoints(); + assert_eq!( + eps, + vec![ + DirectTcpEndpoint { host: "10.0.1.215".into(), port: 5432 }, + DirectTcpEndpoint { host: "10.0.1.215".into(), port: 6379 }, + DirectTcpEndpoint { host: "db.internal".into(), port: 1025 }, + ] + ); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + } + + #[test] + fn test_parse_direct_tcp_endpoints_invalid_entries_skipped() { + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "host-no-port, :5432, host:abc, good.internal:8025", + ) }; + let eps = parse_direct_tcp_endpoints(); + assert_eq!( + eps, + vec![DirectTcpEndpoint { host: "good.internal".into(), port: 8025 }] + ); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + } + + #[test] + fn test_parse_direct_tcp_endpoints_empty() { + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + assert!(parse_direct_tcp_endpoints().is_empty()); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_ENDPOINTS", "") }; + assert!(parse_direct_tcp_endpoints().is_empty()); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + } + #[test] #[ignore = "requires root privileges"] fn test_create_and_drop_namespace() { diff --git a/crates/openshell-sandbox/src/secrets.rs b/crates/openshell-sandbox/src/secrets.rs index a27537c910..dfb9c5c756 100644 --- a/crates/openshell-sandbox/src/secrets.rs +++ b/crates/openshell-sandbox/src/secrets.rs @@ -7,8 +7,16 @@ use std::fmt; const PLACEHOLDER_PREFIX: &str = "openshell:resolve:env:"; -/// Public access to the placeholder prefix for fail-closed scanning in other modules. -pub(crate) const PLACEHOLDER_PREFIX_PUBLIC: &str = PLACEHOLDER_PREFIX; + +/// Marker of a provider-shaped alias: `OPENSHELL-RESOLVE-ENV-`. +/// +/// Some clients validate the credential's shape before sending it (e.g. gh's +/// `--attach` only accepts `github_pat_` / `ghp_` / ... tokens) and cannot carry +/// the canonical `openshell:resolve:env:KEY` placeholder. They send an alias +/// such as `github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN` instead, which the +/// proxy resolves to the same secret as the canonical placeholder. +/// Ported from NVIDIA/OpenShell#1286 (header and request-line scope only). +const PROVIDER_ALIAS_MARKER: &str = "OPENSHELL-RESOLVE-ENV-"; /// Characters that are valid in an env var key name (used to extract /// placeholder boundaries within concatenated strings like path segments). @@ -16,6 +24,45 @@ fn is_env_key_char(b: u8) -> bool { b.is_ascii_alphanumeric() || b == b'_' } +/// Characters allowed in the shape prefix of a provider-shaped alias +/// (RFC 3986 unreserved characters). +fn is_alias_token_char(b: u8) -> bool { + b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-' | b'.' | b'~') +} + +/// Extract the env key from a whole-token provider-shaped alias. +/// +/// The token must be `OPENSHELL-RESOLVE-ENV-` where the prefix is +/// non-empty and made of unreserved characters, and the key runs to the end of +/// the token. Anything else is not an alias. +fn alias_env_key(token: &str) -> Option<&str> { + let marker_start = token.find(PROVIDER_ALIAS_MARKER)?; + if marker_start == 0 { + return None; + } + if !token[..marker_start].bytes().all(is_alias_token_char) { + return None; + } + let key_start = marker_start + PROVIDER_ALIAS_MARKER.len(); + let key_end = token[key_start..] + .bytes() + .position(|b| !is_env_key_char(b)) + .map_or(token.len(), |p| key_start + p); + (key_end == token.len() && key_end > key_start).then_some(&token[key_start..key_end]) +} + +/// True if the text still carries a credential placeholder or alias marker. +/// Used by the fail-closed scans after rewriting. +pub(crate) fn contains_credential_marker(text: &str) -> bool { + text.contains(PLACEHOLDER_PREFIX) || text.contains(PROVIDER_ALIAS_MARKER) +} + +/// True if a request line carries a placeholder or alias marker in its raw or +/// percent-decoded form (F5 — encoded placeholder bypass). +pub(crate) fn request_line_has_credential_marker(line: &str) -> bool { + contains_credential_marker(line) || contains_credential_marker(&percent_decode(line)) +} + // --------------------------------------------------------------------------- // Error and result types // --------------------------------------------------------------------------- @@ -91,7 +138,15 @@ impl SecretResolver { /// Returns `None` if the placeholder is unknown or the resolved value /// contains prohibited control characters (CRLF, null byte). pub(crate) fn resolve_placeholder(&self, value: &str) -> Option<&str> { - let secret = self.by_placeholder.get(value).map(String::as_str)?; + let secret = match self.by_placeholder.get(value) { + Some(secret) => secret.as_str(), + None => { + let key = alias_env_key(value)?; + self.by_placeholder + .get(&placeholder_for_env_key(key)) + .map(String::as_str)? + } + }; match validate_resolved_secret(secret) { Ok(s) => Some(s), Err(reason) => { @@ -610,14 +665,14 @@ pub(crate) fn rewrite_http_header_block( // Fail-closed scan: check for any remaining unresolved placeholders // in both raw form and percent-decoded form of the output header block. let output_header = String::from_utf8_lossy(&output[..output.len().min(header_end + 256)]); - if output_header.contains(PLACEHOLDER_PREFIX) { + if contains_credential_marker(&output_header) { return Err(UnresolvedPlaceholderError { location: "header" }); } // Also check percent-decoded form of the request line (F5 — encoded placeholder bypass) let rewritten_rl = output_header.split("\r\n").next().unwrap_or(""); let decoded_rl = percent_decode(rewritten_rl); - if decoded_rl.contains(PLACEHOLDER_PREFIX) { + if contains_credential_marker(&decoded_rl) { return Err(UnresolvedPlaceholderError { location: "path" }); } @@ -646,6 +701,13 @@ pub(crate) fn rewrite_target_for_eval( target: &str, resolver: &SecretResolver, ) -> Result { + // Provider-shaped aliases are resolved in headers only. An alias in the + // target would never be resolved, so fail closed before it reaches OPA or + // the upstream (raw and percent-decoded forms). + if target.contains(PROVIDER_ALIAS_MARKER) || percent_decode(target).contains(PROVIDER_ALIAS_MARKER) { + return Err(UnresolvedPlaceholderError { location: "path" }); + } + if !target.contains(PLACEHOLDER_PREFIX) { // Also check percent-decoded form let decoded = percent_decode(target); @@ -1473,4 +1535,150 @@ mod tests { assert_eq!(result.resolved, "/bottok123/method?key=key456"); assert_eq!(result.redacted, "/bot[CREDENTIAL]/method?key=[CREDENTIAL]"); } + + // === Provider-shaped aliases (ported from NVIDIA/OpenShell#1286) === + // + // A client that validates the credential's shape before sending (e.g. gh's + // `--attach`, which only accepts `github_pat_` / `ghp_` / ... tokens) cannot + // carry the canonical `openshell:resolve:env:KEY` placeholder. Such a client + // sends `OPENSHELL-RESOLVE-ENV-KEY` instead, and the proxy must + // resolve it to the same secret as the canonical placeholder. + + fn github_resolver() -> SecretResolver { + let (_, resolver) = SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "github_pat_REAL".to_string())] + .into_iter() + .collect(), + ); + resolver.expect("resolver") + } + + #[test] + fn provider_alias_resolves_to_the_canonical_secret() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + Some("github_pat_REAL") + ); + } + + #[test] + fn provider_alias_in_token_scheme_header_is_rewritten() { + let resolver = github_resolver(); + assert_eq!( + rewrite_header_line( + "Authorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", + &resolver, + ), + "Authorization: token github_pat_REAL" + ); + } + + #[test] + fn provider_alias_accepts_unreserved_prefix_characters() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("a.b~c-d_e9OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + Some("github_pat_REAL") + ); + } + + #[test] + fn provider_alias_with_unknown_key_is_not_resolved() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_requires_a_non_empty_prefix() { + let resolver = github_resolver(); + assert_eq!(resolver.resolve_placeholder("OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), None); + } + + #[test] + fn provider_alias_rejects_prefix_with_reserved_characters() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("a/b:OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_must_end_at_the_key() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN!x"), + None + ); + assert_eq!(resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-"), None); + } + + #[test] + fn provider_alias_resolution_rejects_crlf_secret() { + let (_, resolver) = SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "bad\r\nX-Injected: 1".to_string())] + .into_iter() + .collect(), + ); + let resolver = resolver.expect("resolver"); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_header_block_round_trip() { + let resolver = github_resolver(); + let raw = b"POST /user-attachments/assets?name=a.webm HTTP/1.1\r\nHost: uploads.github.com\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN\r\nContent-Length: 4\r\n\r\nbody"; + let result = rewrite_http_header_block(raw, Some(&resolver)).expect("should resolve"); + let rewritten = String::from_utf8(result.rewritten).expect("utf8"); + assert!(rewritten.contains("Authorization: token github_pat_REAL\r\n")); + assert!(!rewritten.contains("OPENSHELL-RESOLVE-ENV-")); + assert!(rewritten.ends_with("\r\n\r\nbody")); + } + + #[test] + fn unresolved_provider_alias_in_header_fails_closed() { + let resolver = github_resolver(); + let raw = b"GET /user HTTP/1.1\r\nHost: api.github.com\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN\r\n\r\n"; + let err = rewrite_http_header_block(raw, Some(&resolver)).expect_err("must fail closed"); + assert_eq!(err.location, "header"); + } + + #[test] + fn provider_alias_in_request_line_fails_closed() { + let resolver = github_resolver(); + let raw = b"GET /x?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN HTTP/1.1\r\nHost: api.github.com\r\n\r\n"; + assert!(rewrite_http_header_block(raw, Some(&resolver)).is_err()); + let encoded = b"GET /x%3Ft%3Dgithub_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN HTTP/1.1\r\nHost: api.github.com\r\n\r\n"; + assert!(rewrite_http_header_block(encoded, Some(&resolver)).is_err()); + } + + #[test] + fn rewrite_target_for_eval_rejects_provider_alias() { + // Aliases are resolved in headers only. One in the request target is + // never resolved, so it must fail closed instead of reaching OPA and upstream. + let resolver = github_resolver(); + let err = rewrite_target_for_eval("/x?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", &resolver) + .expect_err("alias in target must fail closed"); + assert_eq!(err.location, "path"); + assert!( + rewrite_target_for_eval("/x?t=github_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN", &resolver) + .is_err() + ); + } + + #[test] + fn canonical_placeholder_still_resolves_after_alias_support() { + let resolver = github_resolver(); + assert_eq!( + rewrite_header_line("Authorization: token openshell:resolve:env:GITHUB_TOKEN", &resolver), + "Authorization: token github_pat_REAL" + ); + } }