From 1d37a4cf9a053cb0c56fe440ea83d5d38f37e25d Mon Sep 17 00:00:00 2001 From: mjamiv Date: Mon, 25 May 2026 21:43:34 +0000 Subject: [PATCH] fix(sandbox): trust exact declared private endpoints --- crates/openshell-policy/src/merge.rs | 113 +++++++- .../data/sandbox-policy.rego | 41 +++ .../src/mechanistic_mapper.rs | 15 +- crates/openshell-sandbox/src/opa.rs | 188 ++++++++++++- crates/openshell-sandbox/src/policy_local.rs | 19 +- crates/openshell-sandbox/src/proxy.rs | 259 +++++++++++++++++- docs/reference/policy-schema.mdx | 2 +- docs/sandboxes/policy-advisor.mdx | 2 +- docs/security/best-practices.mdx | 8 +- 9 files changed, 631 insertions(+), 16 deletions(-) diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index c01445b11..5ce4b3020 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -723,6 +723,12 @@ fn expand_access_preset(protocol: &str, access: &str) -> Option> { fn append_unique_binaries(existing: &mut Vec, incoming: &[NetworkBinary]) { let mut seen: HashSet = existing.iter().map(|binary| binary.path.clone()).collect(); for binary in incoming { + if let Some(existing_binary) = existing.iter_mut().find(|item| item.path == binary.path) { + if !is_advisor_proposed_binary(binary) { + mark_user_declared_binary(existing_binary); + } + continue; + } if seen.insert(binary.path.clone()) { existing.push(binary.clone()); } @@ -778,8 +784,30 @@ fn dedup_strings(values: &mut Vec) { } fn dedup_binaries(values: &mut Vec) { - let mut seen = HashSet::new(); - values.retain(|binary| seen.insert(binary.path.clone())); + let mut deduped: Vec = Vec::with_capacity(values.len()); + for binary in std::mem::take(values) { + if let Some(existing) = deduped.iter_mut().find(|item| item.path == binary.path) { + if !is_advisor_proposed_binary(&binary) { + mark_user_declared_binary(existing); + } + } else { + deduped.push(binary); + } + } + *values = deduped; +} + +fn is_advisor_proposed_binary(binary: &NetworkBinary) -> bool { + #[allow(deprecated)] + let advisor_proposed = binary.harness; + advisor_proposed +} + +fn mark_user_declared_binary(binary: &mut NetworkBinary) { + #[allow(deprecated)] + { + binary.harness = false; + } } fn dedup_l7_rules(values: &mut Vec) { @@ -878,6 +906,18 @@ mod tests { } } + fn advisor_binary(path: &str) -> NetworkBinary { + let mut binary = NetworkBinary { + path: path.to_string(), + ..Default::default() + }; + #[allow(deprecated)] + { + binary.harness = true; + } + binary + } + fn rest_rule(method: &str, path: &str) -> L7Rule { L7Rule { allow: Some(L7Allow { @@ -949,6 +989,75 @@ mod tests { assert_eq!(rule.binaries.len(), 2); } + #[test] + fn add_rule_user_binary_clears_advisor_marker_for_same_path() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "existing".to_string(), + NetworkPolicyRule { + name: "existing".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![advisor_binary("/usr/bin/curl")], + }, + ); + + let incoming = NetworkPolicyRule { + name: "incoming".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![NetworkBinary { + path: "/usr/bin/curl".to_string(), + ..Default::default() + }], + }; + + let result = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + let rule = &result.policy.network_policies["existing"]; + assert_eq!(rule.binaries.len(), 1); + #[allow(deprecated)] + { + assert!(!rule.binaries[0].harness); + } + } + + #[test] + fn add_rule_duplicate_binaries_prefer_user_declared_marker() { + let incoming = NetworkPolicyRule { + name: "incoming".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![ + advisor_binary("/usr/bin/curl"), + NetworkBinary { + path: "/usr/bin/curl".to_string(), + ..Default::default() + }, + ], + }; + + let result = merge_policy( + restrictive_default_policy(), + &[PolicyMergeOp::AddRule { + rule_name: "github".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + let rule = &result.policy.network_policies["github"]; + assert_eq!(rule.binaries.len(), 1); + #[allow(deprecated)] + { + assert!(!rule.binaries[0].harness); + } + } + #[test] fn add_rule_merges_websocket_credential_rewrite_flag() { let mut policy = restrictive_default_policy(); diff --git a/crates/openshell-sandbox/data/sandbox-policy.rego b/crates/openshell-sandbox/data/sandbox-policy.rego index 0fa1e6be7..185c23c08 100644 --- a/crates/openshell-sandbox/data/sandbox-policy.rego +++ b/crates/openshell-sandbox/data/sandbox-policy.rego @@ -152,6 +152,32 @@ binary_allowed(policy, exec) if { glob.match(b.path, ["/"], p) } +user_declared_binary_allowed(policy, exec) if { + some b + b := policy.binaries[_] + not object.get(b, "advisor_proposed", false) + not contains(b.path, "*") + b.path == exec.path +} + +user_declared_binary_allowed(policy, exec) if { + some b + b := policy.binaries[_] + not object.get(b, "advisor_proposed", false) + not contains(b.path, "*") + ancestor := exec.ancestors[_] + b.path == ancestor +} + +user_declared_binary_allowed(policy, exec) if { + some b in policy.binaries + not object.get(b, "advisor_proposed", false) + contains(b.path, "*") + all_paths := array.concat([exec.path], exec.ancestors) + some p in all_paths + glob.match(b.path, ["/"], p) +} + # --- Network action (allow / deny) --- # # These rules are mutually exclusive by construction: @@ -638,6 +664,21 @@ matched_endpoint_config := _matching_endpoint_configs[0] if { count(_matching_endpoint_configs) > 0 } +_policy_has_exact_declared_endpoint(policy) if { + some ep + ep := policy.endpoints[_] + not contains(ep.host, "*") + lower(ep.host) == lower(input.network.host) + ep.ports[_] == input.network.port +} + +exact_declared_endpoint_host if { + some pname + policy := data.network_policies[pname] + user_declared_binary_allowed(policy, input.exec) + _policy_has_exact_declared_endpoint(policy) +} + # Hosted endpoint: exact host match + port in ports list. endpoint_matches_request(ep, network) if { not contains(ep.host, "*") diff --git a/crates/openshell-sandbox/src/mechanistic_mapper.rs b/crates/openshell-sandbox/src/mechanistic_mapper.rs index 521c882a0..d61abba9a 100644 --- a/crates/openshell-sandbox/src/mechanistic_mapper.rs +++ b/crates/openshell-sandbox/src/mechanistic_mapper.rs @@ -143,10 +143,17 @@ pub fn generate_proposals(summaries: &[DenialSummary]) -> Vec { let binaries: Vec = if binary.is_empty() { vec![] } else { - vec![NetworkBinary { + let mut proposal_binary = NetworkBinary { path: binary.clone(), ..Default::default() - }] + }; + // The deprecated harness bit is ignored by policy YAML, but OPA + // maps it to advisor_proposed to preserve the SSRF two-step flow. + #[allow(deprecated)] + { + proposal_binary.harness = true; + } + vec![proposal_binary] }; let proposed_rule = NetworkPolicyRule { @@ -500,6 +507,10 @@ mod tests { assert_eq!(rule.endpoints[0].port, 443); assert_eq!(rule.binaries.len(), 1); assert_eq!(rule.binaries[0].path, "/usr/bin/curl"); + #[allow(deprecated)] + { + assert!(rule.binaries[0].harness); + } // No L7 fields when no samples provided. assert!(rule.endpoints[0].protocol.is_empty()); diff --git a/crates/openshell-sandbox/src/opa.rs b/crates/openshell-sandbox/src/opa.rs index b49875b78..60063463e 100644 --- a/crates/openshell-sandbox/src/opa.rs +++ b/crates/openshell-sandbox/src/opa.rs @@ -556,6 +556,52 @@ impl OpaEngine { .unwrap_or_default()) } + /// Return true when the matched endpoint is an exact declared hostname. + /// + /// This intentionally excludes wildcard and hostless endpoints. The proxy + /// uses this as a narrow signal that the operator explicitly declared the + /// destination hostname, which can safely skip the default private-IP SSRF + /// denial while preserving separate handling for `allowed_ips` and advisor + /// proposals. + pub fn query_exact_declared_endpoint_host(&self, input: &NetworkInput) -> Result { + let ancestor_strs: Vec = input + .ancestors + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(); + let cmdline_strs: Vec = input + .cmdline_paths + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(); + let input_json = serde_json::json!({ + "exec": { + "path": input.binary_path.to_string_lossy(), + "ancestors": ancestor_strs, + "cmdline_paths": cmdline_strs, + }, + "network": { + "host": input.host, + "port": input.port, + } + }); + + let mut engine = self + .engine + .lock() + .map_err(|_| miette::miette!("OPA engine lock poisoned"))?; + + engine + .set_input_json(&input_json.to_string()) + .map_err(|e| miette::miette!("{e}"))?; + + let val = engine + .eval_rule("data.openshell.sandbox.exact_declared_endpoint_host".into()) + .map_err(|e| miette::miette!("{e}"))?; + + Ok(val == regorus::Value::from(true)) + } + /// Clone the inner regorus engine for per-tunnel L7 evaluation. /// /// With the `arc` feature enabled, this shares compiled policy via Arc @@ -1097,9 +1143,20 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St .binaries .iter() .flat_map(|b| { - let mut entries = vec![serde_json::json!({"path": &b.path})]; + // The deprecated harness bit is ignored by policy YAML, but + // advisor-generated proposals use it as internal provenance. + #[allow(deprecated)] + let advisor_proposed = b.harness; + let binary_entry = |path: &str| { + let mut entry = serde_json::json!({"path": path}); + if advisor_proposed { + entry["advisor_proposed"] = true.into(); + } + entry + }; + let mut entries = vec![binary_entry(&b.path)]; if let Some(resolved) = resolve_binary_in_container(&b.path, entrypoint_pid) { - entries.push(serde_json::json!({"path": resolved})); + entries.push(binary_entry(&resolved)); } entries }) @@ -3435,6 +3492,13 @@ network_policies: - { host: api.github.com, port: 443 } binaries: - { path: /usr/bin/curl } + # Wildcard host endpoint should not count as an exact declared hostname. + wildcard_api: + name: wildcard_api + endpoints: + - { host: "*.corp.net", port: 443 } + binaries: + - { path: /usr/bin/curl } filesystem_policy: include_workdir: true read_only: [] @@ -3536,6 +3600,126 @@ process: assert!(ips.is_empty(), "Mode 1 should return no allowed_ips"); } + #[test] + fn exact_declared_endpoint_host_true_for_l4_host_only() { + let engine = allowed_ips_engine(); + let input = NetworkInput { + host: "api.github.com".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + assert!(engine.query_endpoint_config(&input).unwrap().is_none()); + assert!(engine.query_exact_declared_endpoint_host(&input).unwrap()); + } + + #[test] + fn exact_declared_endpoint_host_true_for_host_with_allowed_ips() { + let engine = allowed_ips_engine(); + let input = NetworkInput { + host: "my-service.corp.net".into(), + port: 8080, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + assert!(engine.query_exact_declared_endpoint_host(&input).unwrap()); + } + + #[test] + fn exact_declared_endpoint_host_false_for_hostless_allowed_ips() { + let engine = allowed_ips_engine(); + let input = NetworkInput { + host: "anything.example.com".into(), + port: 9443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + assert!(!engine.query_exact_declared_endpoint_host(&input).unwrap()); + } + + #[test] + fn exact_declared_endpoint_host_false_for_wildcard_host() { + let engine = allowed_ips_engine(); + let input = NetworkInput { + host: "api.corp.net".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + let decision = engine.evaluate_network(&input).unwrap(); + assert!(decision.allowed, "wildcard endpoint should still allow"); + assert!(!engine.query_exact_declared_endpoint_host(&input).unwrap()); + } + + #[test] + fn exact_declared_endpoint_host_false_for_advisor_proposed_binary() { + let mut network_policies = std::collections::HashMap::new(); + let mut proposal_binary = NetworkBinary { + path: "/usr/bin/curl".to_string(), + ..Default::default() + }; + #[allow(deprecated)] + { + proposal_binary.harness = true; + } + network_policies.insert( + "allow_mcp_internal_corp_example_com_8443".to_string(), + NetworkPolicyRule { + name: "allow_mcp_internal_corp_example_com_8443".to_string(), + endpoints: vec![NetworkEndpoint { + host: "mcp-internal.corp.example.com".to_string(), + port: 8443, + ..Default::default() + }], + binaries: vec![proposal_binary], + }, + ); + let proto = ProtoSandboxPolicy { + version: 1, + filesystem: Some(ProtoFs { + include_workdir: true, + read_only: vec![], + read_write: vec![], + }), + landlock: Some(openshell_core::proto::LandlockPolicy { + compatibility: "best_effort".to_string(), + }), + process: Some(ProtoProc { + run_as_user: "sandbox".to_string(), + run_as_group: "sandbox".to_string(), + }), + network_policies, + }; + let engine = OpaEngine::from_proto(&proto).expect("engine from proto"); + let input = NetworkInput { + host: "mcp-internal.corp.example.com".into(), + port: 8443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + + let decision = engine.evaluate_network(&input).unwrap(); + assert!( + decision.allowed, + "advisor proposal should still allow at OPA L4" + ); + assert!(!engine.query_exact_declared_endpoint_host(&input).unwrap()); + } + #[test] fn allowed_ips_mode3_wrong_port_denied() { let engine = allowed_ips_engine(); diff --git a/crates/openshell-sandbox/src/policy_local.rs b/crates/openshell-sandbox/src/policy_local.rs index 657fd760f..d88fcf493 100644 --- a/crates/openshell-sandbox/src/policy_local.rs +++ b/crates/openshell-sandbox/src/policy_local.rs @@ -1009,9 +1009,18 @@ fn network_rule_from_json( let binaries = rule .binaries .into_iter() - .map(|binary| NetworkBinary { - path: binary.path, - ..Default::default() + .map(|binary| { + let mut proposal_binary = NetworkBinary { + path: binary.path, + ..Default::default() + }; + // The deprecated harness bit is ignored by policy YAML, but OPA + // maps it to advisor_proposed to preserve the SSRF two-step flow. + #[allow(deprecated)] + { + proposal_binary.harness = true; + } + proposal_binary }) .collect(); @@ -1339,6 +1348,10 @@ mod tests { assert_eq!(rule.endpoints[0].port, 443); assert_eq!(rule.endpoints[0].ports, vec![443]); assert_eq!(rule.endpoints[0].protocol, "rest"); + #[allow(deprecated)] + { + assert!(rule.binaries[0].harness); + } assert_eq!( rule.endpoints[0].rules[0].allow.as_ref().unwrap().path, "/user/repos" diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index 037ecfc78..9bd23e367 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -559,10 +559,14 @@ async fn handle_tcp_connection( // allowlist instead of blanket-blocking all private IPs. // When the policy host is already a literal IP address, treat it as // implicitly allowed — the user explicitly declared the destination. + // Exact declared hostnames also skip the private-IP blanket block below, + // while keeping loopback/link-local/unspecified addresses denied. let mut raw_allowed_ips = query_allowed_ips(&opa_engine, &decision, &host_lc, port); if raw_allowed_ips.is_empty() { raw_allowed_ips = implicit_allowed_ips_for_ip_host(&host); } + let exact_declared_endpoint_host = + query_exact_declared_endpoint_host(&opa_engine, &decision, &host_lc, port); // Defense-in-depth: resolve DNS and reject connections to internal IPs. let dns_connect_start = std::time::Instant::now(); @@ -727,6 +731,61 @@ async fn handle_tcp_connection( return Ok(()); } } + } else if exact_declared_endpoint_host { + // Exact declared hostname mode: the operator explicitly allowed this + // host:port, so private IP resolution is permitted without duplicating + // the resolved IP in allowed_ips. Always-blocked addresses and + // control-plane ports remain denied. + match resolve_and_check_declared_endpoint(&host, port, sandbox_entrypoint_pid).await { + Ok(addrs) => TcpStream::connect(addrs.as_slice()) + .await + .into_diagnostic()?, + Err(reason) => { + { + let event = NetworkActivityBuilder::new(crate::ocsf_ctx()) + .activity(ActivityId::Open) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .dst_endpoint(Endpoint::from_domain(&host_lc, port)) + .src_endpoint_addr(peer_addr.ip(), peer_addr.port()) + .actor_process( + Process::from_bypass(&binary_str, &pid_str, &ancestors_str) + .with_cmd_line(&cmdline_str), + ) + .firewall_rule("-", "ssrf") + .message(format!( + "CONNECT blocked: declared endpoint check failed for {host_lc}:{port}" + )) + .status_detail(&reason) + .build(); + ocsf_emit!(event); + } + emit_denial( + &denial_tx, + &host_lc, + port, + &binary_str, + &decision, + &reason, + "ssrf", + ); + respond( + &mut client, + &build_json_error_response( + 403, + "Forbidden", + "ssrf_denied", + &format!( + "CONNECT {host_lc}:{port} blocked: declared endpoint check failed" + ), + ), + ) + .await?; + return Ok(()); + } + } } else { // Default: reject all internal IPs (loopback, RFC 1918, link-local). match resolve_and_reject_internal(&host, port, sandbox_entrypoint_pid).await { @@ -2230,6 +2289,36 @@ fn validate_allowed_ips_for_resolved_addrs( Ok(()) } +fn validate_declared_endpoint_resolved_addrs( + host: &str, + port: u16, + addrs: &[SocketAddr], +) -> std::result::Result<(), String> { + if addrs.is_empty() { + return Err(format!( + "DNS resolution returned no addresses for {}", + normalize_host_lookup_key(host) + )); + } + + if BLOCKED_CONTROL_PLANE_PORTS.contains(&port) { + return Err(format!( + "port {port} is a blocked control-plane port, connection rejected" + )); + } + + for addr in addrs { + if is_always_blocked_ip(addr.ip()) { + return Err(format!( + "{host} resolves to always-blocked address {}, connection rejected", + addr.ip() + )); + } + } + + Ok(()) +} + /// Resolve a host:port using sandbox `/etc/hosts` first (when available), then /// reject if any resolved address is internal. /// @@ -2264,6 +2353,21 @@ async fn resolve_and_check_allowed_ips( Ok(addrs) } +/// Resolve a host:port that was explicitly declared by hostname in policy. +/// +/// Exact declared hostnames are the operator's trust signal, so RFC1918 and +/// other private ranges are allowed without a duplicated `allowed_ips` entry. +/// Loopback, link-local, unspecified, and control-plane ports remain blocked. +async fn resolve_and_check_declared_endpoint( + host: &str, + port: u16, + entrypoint_pid: u32, +) -> std::result::Result, String> { + let addrs = resolve_socket_addrs(host, port, entrypoint_pid).await?; + validate_declared_endpoint_resolved_addrs(host, port, &addrs)?; + Ok(addrs) +} + /// Minimum CIDR prefix length before logging a breadth warning. /// CIDRs broader than /16 (65,536+ addresses) may unintentionally expose /// control-plane services on the same network. @@ -2388,6 +2492,46 @@ fn query_allowed_ips( } } +/// Query whether the matched endpoint was declared as this exact hostname. +fn query_exact_declared_endpoint_host( + engine: &OpaEngine, + decision: &ConnectDecision, + host: &str, + port: u16, +) -> bool { + let has_policy = match &decision.action { + NetworkAction::Allow { matched_policy } => matched_policy.is_some(), + NetworkAction::Deny { .. } => false, + }; + if !has_policy { + return false; + } + + let input = crate::opa::NetworkInput { + host: host.to_string(), + port, + binary_path: decision.binary.clone().unwrap_or_default(), + binary_sha256: String::new(), + ancestors: decision.ancestors.clone(), + cmdline_paths: decision.cmdline_paths.clone(), + }; + + match engine.query_exact_declared_endpoint_host(&input) { + Ok(is_exact_declared) => is_exact_declared, + Err(e) => { + let event = NetworkActivityBuilder::new(crate::ocsf_ctx()) + .activity(ActivityId::Fail) + .severity(SeverityId::Low) + .status(StatusId::Failure) + .dst_endpoint(Endpoint::from_domain(host, port)) + .message(format!("Failed to query exact declared endpoint host: {e}")) + .build(); + ocsf_emit!(event); + false + } + } +} + /// Canonicalize the request-target for inference pattern detection. /// /// Falls back to the raw path on canonicalization error: the request is then @@ -3215,13 +3359,17 @@ async fn handle_forward_proxy( // tiers and validate only against the trusted gateway IP. // - If allowed_ips is set: validate resolved IPs against the allowlist // (this is the SSRF override for private IP destinations). - // - If allowed_ips is empty: reject internal IPs, allow public IPs through. + // - If the endpoint is an exact declared hostname: allow private IPs, + // but still reject always-blocked addresses and control-plane ports. + // - Otherwise: reject internal IPs, allow public IPs through. // When the policy host is already a literal IP address, treat it as // implicitly allowed — the user explicitly declared the destination. let mut raw_allowed_ips = query_allowed_ips(&opa_engine, &decision, &host_lc, port); if raw_allowed_ips.is_empty() { raw_allowed_ips = implicit_allowed_ips_for_ip_host(&host); } + let exact_declared_endpoint_host = + query_exact_declared_endpoint_host(&opa_engine, &decision, &host_lc, port); // The trusted-gateway branch is the first path; reading it before the // allowed_ips and default branches matches the policy decision narrative. @@ -3389,6 +3537,62 @@ async fn handle_forward_proxy( return Ok(()); } } + } else if exact_declared_endpoint_host { + // Exact declared hostname mode mirrors CONNECT: private resolved + // addresses are allowed for this operator-declared host:port, while + // always-blocked addresses and control-plane ports remain denied. + match resolve_and_check_declared_endpoint(&host, port, sandbox_entrypoint_pid).await { + Ok(addrs) => addrs, + Err(reason) => { + { + let event = HttpActivityBuilder::new(crate::ocsf_ctx()) + .activity(ActivityId::Other) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .http_request(HttpRequest::new( + method, + OcsfUrl::new("http", &host_lc, &path, port), + )) + .dst_endpoint(Endpoint::from_domain(&host_lc, port)) + .src_endpoint(Endpoint::from_ip(peer_addr.ip(), peer_addr.port())) + .actor_process( + Process::from_bypass(&binary_str, &pid_str, &ancestors_str) + .with_cmd_line(&cmdline_str), + ) + .firewall_rule(policy_str, "ssrf") + .message(format!( + "FORWARD blocked: declared endpoint check failed for {host_lc}:{port}" + )) + .status_detail(&reason) + .build(); + ocsf_emit!(event); + } + emit_denial_simple( + denial_tx, + &host_lc, + port, + &binary_str, + &decision, + &reason, + "ssrf", + ); + respond( + client, + &build_json_error_response( + 403, + "Forbidden", + "ssrf_denied", + &format!( + "{method} {host_lc}:{port} blocked: declared endpoint check failed" + ), + ), + ) + .await?; + return Ok(()); + } + } } else { // No allowed_ips: reject internal IPs, allow public IPs through. match resolve_and_reject_internal(&host, port, sandbox_entrypoint_pid).await { @@ -4411,6 +4615,59 @@ network_policies: ); } + #[test] + fn test_declared_endpoint_private_hosts_file_resolution_allowed() { + let addrs = resolve_from_hosts_file_contents( + "192.168.1.105 searxng.local\n", + "searxng.local", + 8080, + ); + + assert!(validate_declared_endpoint_resolved_addrs("searxng.local", 8080, &addrs).is_ok()); + } + + #[test] + fn test_declared_endpoint_loopback_stays_blocked() { + let addrs = + resolve_from_hosts_file_contents("127.0.0.1 loopback.local\n", "loopback.local", 80); + + let err = + validate_declared_endpoint_resolved_addrs("loopback.local", 80, &addrs).unwrap_err(); + assert!( + err.contains("always-blocked"), + "expected loopback to stay blocked: {err}" + ); + } + + #[test] + fn test_declared_endpoint_link_local_stays_blocked() { + let addrs = resolve_from_hosts_file_contents( + "169.254.169.254 metadata.local\n", + "metadata.local", + 80, + ); + + let err = + validate_declared_endpoint_resolved_addrs("metadata.local", 80, &addrs).unwrap_err(); + assert!( + err.contains("always-blocked"), + "expected link-local to stay blocked: {err}" + ); + } + + #[test] + fn test_declared_endpoint_blocks_control_plane_ports() { + let addrs = + resolve_from_hosts_file_contents("10.0.0.5 kube-api.local\n", "kube-api.local", 6443); + + let err = + validate_declared_endpoint_resolved_addrs("kube-api.local", 6443, &addrs).unwrap_err(); + assert!( + err.contains("blocked control-plane port"), + "expected control-plane port to stay blocked: {err}" + ); + } + #[test] fn test_resolve_from_hosts_file_contents_always_blocked_ip_stays_blocked() { let addrs = diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index b87e17676..59f72c9f7 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -161,7 +161,7 @@ Each endpoint defines a reachable destination and optional inspection rules. | `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. | | `rules` | list of rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. | | `deny_rules` | list of deny rule objects | No | L7 deny rules that block specific requests even when allowed by `access` or `rules`. Deny rules take precedence over allow rules. | -| `allowed_ips` | list of string | No | CIDR or IP allowlist for SSRF override. Entries overlapping loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), or unspecified (`0.0.0.0`) are rejected at load time. | +| `allowed_ips` | list of string | No | CIDR or IP allowlist for SSRF override. Exact user-declared hostname endpoints may resolve to RFC 1918 private addresses without this field, but wildcard, hostless, and policy-advisor-proposed endpoints still require `allowed_ips` for private resolved IPs. Entries overlapping loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), or unspecified (`0.0.0.0`) are rejected at load time. | | `allow_encoded_slash` | bool | No | When `true`, L7 request parsing preserves `%2F` inside path segments instead of rejecting it. Use this for registries and APIs such as npm scoped packages (`/@scope%2Fname`). Defaults to `false`. | | `websocket_credential_rewrite` | bool | No | When `true` on a `protocol: rest` or `protocol: websocket` endpoint, OpenShell rewrites credential placeholders in client-to-server WebSocket text messages after an allowed HTTP `101` upgrade. Binary frames are relayed but not rewritten. Defaults to `false`. | | `request_body_credential_rewrite` | bool | No | When `true` on a `protocol: rest` endpoint, OpenShell rewrites credential placeholders in UTF-8 `application/json`, `application/x-www-form-urlencoded`, and `text/*` request bodies before forwarding upstream. The proxy buffers at most 256 KiB and updates `Content-Length` after rewriting. Defaults to `false`. | diff --git a/docs/sandboxes/policy-advisor.mdx b/docs/sandboxes/policy-advisor.mdx index d129e5e5a..676018408 100644 --- a/docs/sandboxes/policy-advisor.mdx +++ b/docs/sandboxes/policy-advisor.mdx @@ -116,7 +116,7 @@ For REST APIs, prefer L7 rules over broad L4 access. A good proposal allows one The current `policy.local` JSON shape covers L4 endpoints and REST method or path rules. Use [Customize Sandbox Policies](/sandboxes/policies) or [Policy Schema Reference](/reference/policy-schema) for policy fields that are not part of the agent-authored proposal surface, such as WebSocket credential rewrite, GraphQL operation matching, endpoint path scoping, and provider-owned policy bundles. -Policy advisor proposals do not add `allowed_ips` automatically. If a hostname resolves to an internal or private address, OpenShell's SSRF protections still block the connection until a developer explicitly adds the required `allowed_ips` entry. +Policy advisor proposals do not add `allowed_ips` automatically. If an advisor-proposed hostname resolves to an internal or private address, OpenShell's SSRF protections still block the connection until a developer explicitly adds the required `allowed_ips` entry. Exact hostname trust for user-declared policy endpoints does not apply to advisor-generated proposal binaries. ## Review Proposals diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 6473d63d2..9ee85b0d8 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -127,14 +127,14 @@ This enables credential injection and L7 inspection without explicit configurati ### SSRF Protection -After OPA policy allows a connection, the proxy resolves DNS and rejects connections where the resolved IP is internal (loopback, link-local, or RFC 1918 private). +After OPA policy allows a connection, the proxy resolves DNS and rejects undeclared internal destinations. | Aspect | Detail | |---|---| -| Default | The proxy blocks all private IPs. Loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), and unspecified (`0.0.0.0`) addresses are always blocked and cannot be overridden with `allowed_ips`. | -| What you can change | Add `allowed_ips` (CIDR notation) to an endpoint to permit connections to specific private IP ranges. On Linux, the proxy consults the sandbox's `/etc/hosts` before DNS, so Kubernetes `hostAliases` can make LAN-only hostnames resolvable. Policies with `allowed_ips` entries that overlap loopback, link-local, or unspecified addresses fail to load with a clear validation error. | +| Default | The proxy blocks private IPs for undeclared, wildcard, hostless, and policy-advisor-proposed endpoints. Exact hostnames declared in user policy may resolve to private RFC 1918 addresses. Loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), and unspecified (`0.0.0.0`) addresses are always blocked and cannot be overridden with `allowed_ips`. | +| What you can change | Declare a known internal service as an exact `host` endpoint, or add `allowed_ips` (CIDR notation) to an endpoint to permit specific private IP ranges for wildcard or hostless policies. On Linux, the proxy consults the sandbox's `/etc/hosts` before DNS, so Kubernetes `hostAliases` can make LAN-only hostnames resolvable. Policies with `allowed_ips` entries that overlap loopback, link-local, or unspecified addresses fail to load with a clear validation error. | | Risk if relaxed | Without SSRF protection, a misconfigured policy could allow the agent to reach cloud metadata services (`169.254.169.254`), internal databases, or other infrastructure endpoints through DNS rebinding. | -| Recommendation | Use `allowed_ips` only for known internal services. Scope the CIDR as narrowly as possible (for example, `10.0.5.20/32` for a single host). Loopback, link-local, and unspecified addresses are always blocked regardless of `allowed_ips`. `hostAliases` change resolution, not authorization: `host: searxng.local` with `/etc/hosts` mapping to `192.168.1.105` still needs `allowed_ips: ["192.168.1.105/32"]`. The policy advisor does not propose rules for always-blocked destinations. | +| Recommendation | Prefer exact hostname endpoints for stable internal services. Use `allowed_ips` when you need hostless or wildcard authorization, and scope the CIDR as narrowly as possible (for example, `10.0.5.20/32` for a single host). Loopback, link-local, and unspecified addresses are always blocked regardless of `allowed_ips`. `hostAliases` change resolution, not authorization: `host: searxng.local` with `/etc/hosts` mapping to `192.168.1.105` is trusted only when that exact hostname is declared by user policy. The policy advisor does not propose rules for always-blocked destinations and still requires a separate `allowed_ips` edit for private-IP endpoints. | ### Operator Approval