diff --git a/architecture/security-policy.md b/architecture/security-policy.md index 44898d70..8c41e5b9 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -716,7 +716,7 @@ If any condition fails, the proxy returns `403 Forbidden`. 7. Rewrites the request: absolute-form → origin-form (`GET /path HTTP/1.1`), strips hop-by-hop headers, adds `Via: 1.1 openshell-sandbox` and `Connection: close` 8. Forwards the rewritten request, then relays bidirectionally using `tokio::io::copy_bidirectional` (supports chunked transfer, SSE streams, and other long-lived responses with no idle timeout) -**V1 simplifications**: Forward proxy v1 injects `Connection: close` (no keep-alive) and does not perform L7 inspection on the forwarded traffic. Every forward proxy connection handles exactly one request-response exchange. +**V1 simplifications**: Forward proxy v1 injects `Connection: close` (no keep-alive). Every forward proxy connection handles exactly one request-response exchange. When an endpoint has L7 rules configured, the forward proxy evaluates the single request's method and path against L7 policy before forwarding. **Implementation**: See `crates/openshell-sandbox/src/proxy.rs` -- `handle_forward_proxy()`, `parse_proxy_uri()`, `rewrite_forward_request()`. diff --git a/crates/openshell-sandbox/src/l7/relay.rs b/crates/openshell-sandbox/src/l7/relay.rs index 940e7f94..88f9ce8a 100644 --- a/crates/openshell-sandbox/src/l7/relay.rs +++ b/crates/openshell-sandbox/src/l7/relay.rs @@ -180,7 +180,7 @@ fn is_benign_connection_error(err: &miette::Report) -> bool { /// Evaluate an L7 request against the OPA engine. /// /// Returns `(allowed, deny_reason)`. -fn evaluate_l7_request( +pub fn evaluate_l7_request( engine: &Mutex, ctx: &L7EvalContext, request: &L7RequestInfo, diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index 6f3eaf9b..088ec46a 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -1803,10 +1803,62 @@ async fn handle_forward_proxy( }; let policy_str = matched_policy.as_deref().unwrap_or("-"); - // 4b. Reject if the endpoint has L7 config — the forward proxy path does - // not perform per-request method/path inspection, so L7-configured - // endpoints must go through the CONNECT tunnel where inspection happens. - if query_l7_config(&opa_engine, &decision, &host_lc, port).is_some() { + // 4b. If the endpoint has L7 config, evaluate the request against + // L7 policy. The forward proxy handles exactly one request per + // connection (Connection: close), so a single evaluation suffices. + if let Some(l7_config) = query_l7_config(&opa_engine, &decision, &host_lc, port) { + let tunnel_engine = opa_engine.clone_engine_for_tunnel().unwrap_or_else(|e| { + warn!( + error = %e, + "Failed to clone OPA engine for forward L7" + ); + regorus::Engine::new() + }); + let engine_mutex = std::sync::Mutex::new(tunnel_engine); + + let l7_ctx = crate::l7::relay::L7EvalContext { + host: host_lc.clone(), + port, + policy_name: matched_policy.clone().unwrap_or_default(), + binary_path: decision + .binary + .as_ref() + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default(), + ancestors: decision + .ancestors + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(), + cmdline_paths: decision + .cmdline_paths + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(), + secret_resolver: secret_resolver.clone(), + }; + + let request_info = crate::l7::L7RequestInfo { + action: method.to_string(), + target: path.clone(), + }; + + let (allowed, reason) = + crate::l7::relay::evaluate_l7_request(&engine_mutex, &l7_ctx, &request_info) + .unwrap_or_else(|e| { + warn!( + error = %e, + "L7 eval failed, denying request" + ); + (false, format!("L7 evaluation error: {e}")) + }); + + let decision_str = match (allowed, l7_config.enforcement) { + (true, _) => "allow", + (false, crate::l7::EnforcementMode::Audit) => "audit", + (false, crate::l7::EnforcementMode::Enforce) => "deny", + }; + info!( dst_host = %host_lc, dst_port = port, @@ -1814,21 +1866,28 @@ async fn handle_forward_proxy( path = %path, binary = %binary_str, policy = %policy_str, - action = "deny", - reason = "endpoint has L7 rules; use CONNECT", - "FORWARD", + l7_protocol = "rest", + l7_decision = decision_str, + l7_deny_reason = %reason, + "FORWARD_L7", ); - emit_denial_simple( - denial_tx, - &host_lc, - port, - &binary_str, - &decision, - "endpoint has L7 rules configured; forward proxy bypasses L7 inspection — use CONNECT", - "forward-l7-bypass", - ); - respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?; - return Ok(()); + + let effectively_denied = + !allowed && l7_config.enforcement == crate::l7::EnforcementMode::Enforce; + + if effectively_denied { + emit_denial_simple( + denial_tx, + &host_lc, + port, + &binary_str, + &decision, + &reason, + "forward-l7-deny", + ); + respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?; + return Ok(()); + } } // 5. DNS resolution + SSRF defence (mirrors the CONNECT path logic). diff --git a/e2e/rust/tests/forward_proxy_l7_bypass.rs b/e2e/rust/tests/forward_proxy_l7_bypass.rs index fb75176c..89ecf2cd 100644 --- a/e2e/rust/tests/forward_proxy_l7_bypass.rs +++ b/e2e/rust/tests/forward_proxy_l7_bypass.rs @@ -1,10 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Regression test: the forward proxy path must reject requests to endpoints -//! that have L7 rules configured. Before the fix, plain `http://` requests -//! (which use the forward proxy, not CONNECT) bypassed per-request method/path -//! enforcement entirely. +//! Regression tests: the forward proxy path must evaluate L7 rules for +//! endpoints that have them configured. Allowed requests (e.g. GET on a +//! read-only endpoint) should succeed; denied requests (e.g. POST) should +//! receive 403. #![cfg(feature = "e2e")] @@ -145,6 +145,7 @@ network_policies: - host: host.openshell.internal port: {port} protocol: rest + enforcement: enforce allowed_ips: - "172.0.0.0/8" rules: @@ -164,24 +165,21 @@ network_policies: Ok(file) } -/// The forward proxy path (plain http:// via HTTP_PROXY) must return 403 for -/// endpoints with L7 rules, forcing clients through the CONNECT tunnel where -/// per-request method/path inspection actually happens. +/// GET /allowed should succeed — the L7 policy explicitly allows it. #[tokio::test] -async fn forward_proxy_rejects_l7_configured_endpoint() { +async fn forward_proxy_allows_l7_permitted_request() { let server = DockerServer::start() .await .expect("start docker test server"); - let policy = write_policy_with_l7_rules(server.port).expect("write custom policy"); + let policy = + write_policy_with_l7_rules(server.port) + .expect("write custom policy"); let policy_path = policy .path() .to_str() .expect("temp policy path should be utf-8") .to_string(); - // Python script that tries a plain http:// request (forward proxy path). - // HTTP_PROXY is set automatically by the sandbox, so urllib will use the - // forward proxy for http:// URLs (not CONNECT). let script = format!( r#" import urllib.request, urllib.error, json, sys @@ -208,10 +206,60 @@ except Exception as e: .await .expect("sandbox create"); - // The forward proxy should return 403 because the endpoint has L7 rules. + // L7 policy allows GET /allowed — should succeed. + assert!( + guard.create_output.contains("\"status\": 200"), + "expected 200 for L7-allowed GET, got:\n{}", + guard.create_output + ); +} + +/// POST /allowed should be denied — the L7 policy only allows GET. +#[tokio::test] +async fn forward_proxy_denies_l7_blocked_request() { + let server = DockerServer::start() + .await + .expect("start docker test server"); + let policy = + write_policy_with_l7_rules(server.port) + .expect("write custom policy"); + let policy_path = policy + .path() + .to_str() + .expect("temp policy path should be utf-8") + .to_string(); + + let script = format!( + r#" +import urllib.request, urllib.error, json, sys +url = "http://host.openshell.internal:{port}/allowed" +req = urllib.request.Request(url, data=b"test", method="POST") +try: + resp = urllib.request.urlopen(req, timeout=15) + print(json.dumps({{"status": resp.status, "error": None}})) +except urllib.error.HTTPError as e: + print(json.dumps({{"status": e.code, "error": str(e)}})) +except Exception as e: + print(json.dumps({{"status": -1, "error": str(e)}})) +"#, + port = server.port, + ); + + let guard = SandboxGuard::create(&[ + "--policy", + &policy_path, + "--", + "python3", + "-c", + &script, + ]) + .await + .expect("sandbox create"); + + // L7 policy denies POST — should return 403. assert!( guard.create_output.contains("\"status\": 403"), - "expected 403 from forward proxy for L7-configured endpoint, got:\n{}", + "expected 403 for L7-denied POST, got:\n{}", guard.create_output ); }