Skip to content

Commit 94fbb64

Browse files
fix(proxy): add L7 inspection to forward proxy path (#666)
* refactor(l7): export evaluate_l7_request for cross-module use Make evaluate_l7_request() public so the forward proxy path can evaluate individual requests against L7 policy without going through the full relay_with_inspection() loop. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> * fix(proxy): add L7 inspection to forward proxy path The forward proxy previously rejected all requests to endpoints with L7 rules (blanket 403), forcing clients through the CONNECT tunnel. This meant policies like read-only (allow GET, block POST) had no effect on plain http:// requests through the forward proxy. Replace the blanket rejection with actual L7 evaluation: - Query L7 config for the endpoint (same as before) - Clone the OPA engine and evaluate the request method/path - Allow if L7 policy permits, deny with 403 if enforcement is enforce - Audit mode: log but allow (matching CONNECT path behavior) - Fail-closed: deny on evaluation errors The forward proxy uses Connection: close (one request per connection), so a single evaluation suffices — no relay loop needed. Update e2e tests to validate the new behavior: - GET /allowed → 200 (L7 policy allows) - POST /allowed → 403 (L7 policy denies, enforcement: enforce) Update security-policy.md to reflect the new forward proxy L7 behavior. Closes #643 Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> * style(proxy): apply cargo fmt formatting Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> --------- Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> Co-authored-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
1 parent 0ac1fbd commit 94fbb64

4 files changed

Lines changed: 141 additions & 34 deletions

File tree

architecture/security-policy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ If any condition fails, the proxy returns `403 Forbidden`.
716716
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`
717717
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)
718718

719-
**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.
719+
**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.
720720

721721
**Implementation**: See `crates/openshell-sandbox/src/proxy.rs` -- `handle_forward_proxy()`, `parse_proxy_uri()`, `rewrite_forward_request()`.
722722

crates/openshell-sandbox/src/l7/relay.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ fn is_benign_connection_error(err: &miette::Report) -> bool {
180180
/// Evaluate an L7 request against the OPA engine.
181181
///
182182
/// Returns `(allowed, deny_reason)`.
183-
fn evaluate_l7_request(
183+
pub fn evaluate_l7_request(
184184
engine: &Mutex<regorus::Engine>,
185185
ctx: &L7EvalContext,
186186
request: &L7RequestInfo,

crates/openshell-sandbox/src/proxy.rs

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,32 +1803,91 @@ async fn handle_forward_proxy(
18031803
};
18041804
let policy_str = matched_policy.as_deref().unwrap_or("-");
18051805

1806-
// 4b. Reject if the endpoint has L7 config — the forward proxy path does
1807-
// not perform per-request method/path inspection, so L7-configured
1808-
// endpoints must go through the CONNECT tunnel where inspection happens.
1809-
if query_l7_config(&opa_engine, &decision, &host_lc, port).is_some() {
1806+
// 4b. If the endpoint has L7 config, evaluate the request against
1807+
// L7 policy. The forward proxy handles exactly one request per
1808+
// connection (Connection: close), so a single evaluation suffices.
1809+
if let Some(l7_config) = query_l7_config(&opa_engine, &decision, &host_lc, port) {
1810+
let tunnel_engine = opa_engine.clone_engine_for_tunnel().unwrap_or_else(|e| {
1811+
warn!(
1812+
error = %e,
1813+
"Failed to clone OPA engine for forward L7"
1814+
);
1815+
regorus::Engine::new()
1816+
});
1817+
let engine_mutex = std::sync::Mutex::new(tunnel_engine);
1818+
1819+
let l7_ctx = crate::l7::relay::L7EvalContext {
1820+
host: host_lc.clone(),
1821+
port,
1822+
policy_name: matched_policy.clone().unwrap_or_default(),
1823+
binary_path: decision
1824+
.binary
1825+
.as_ref()
1826+
.map(|p| p.to_string_lossy().into_owned())
1827+
.unwrap_or_default(),
1828+
ancestors: decision
1829+
.ancestors
1830+
.iter()
1831+
.map(|p| p.to_string_lossy().into_owned())
1832+
.collect(),
1833+
cmdline_paths: decision
1834+
.cmdline_paths
1835+
.iter()
1836+
.map(|p| p.to_string_lossy().into_owned())
1837+
.collect(),
1838+
secret_resolver: secret_resolver.clone(),
1839+
};
1840+
1841+
let request_info = crate::l7::L7RequestInfo {
1842+
action: method.to_string(),
1843+
target: path.clone(),
1844+
};
1845+
1846+
let (allowed, reason) =
1847+
crate::l7::relay::evaluate_l7_request(&engine_mutex, &l7_ctx, &request_info)
1848+
.unwrap_or_else(|e| {
1849+
warn!(
1850+
error = %e,
1851+
"L7 eval failed, denying request"
1852+
);
1853+
(false, format!("L7 evaluation error: {e}"))
1854+
});
1855+
1856+
let decision_str = match (allowed, l7_config.enforcement) {
1857+
(true, _) => "allow",
1858+
(false, crate::l7::EnforcementMode::Audit) => "audit",
1859+
(false, crate::l7::EnforcementMode::Enforce) => "deny",
1860+
};
1861+
18101862
info!(
18111863
dst_host = %host_lc,
18121864
dst_port = port,
18131865
method = %method,
18141866
path = %path,
18151867
binary = %binary_str,
18161868
policy = %policy_str,
1817-
action = "deny",
1818-
reason = "endpoint has L7 rules; use CONNECT",
1819-
"FORWARD",
1869+
l7_protocol = "rest",
1870+
l7_decision = decision_str,
1871+
l7_deny_reason = %reason,
1872+
"FORWARD_L7",
18201873
);
1821-
emit_denial_simple(
1822-
denial_tx,
1823-
&host_lc,
1824-
port,
1825-
&binary_str,
1826-
&decision,
1827-
"endpoint has L7 rules configured; forward proxy bypasses L7 inspection — use CONNECT",
1828-
"forward-l7-bypass",
1829-
);
1830-
respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
1831-
return Ok(());
1874+
1875+
let effectively_denied =
1876+
!allowed && l7_config.enforcement == crate::l7::EnforcementMode::Enforce;
1877+
1878+
if effectively_denied {
1879+
emit_denial_simple(
1880+
denial_tx,
1881+
&host_lc,
1882+
port,
1883+
&binary_str,
1884+
&decision,
1885+
&reason,
1886+
"forward-l7-deny",
1887+
);
1888+
respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
1889+
return Ok(());
1890+
}
18321891
}
18331892

18341893
// 5. DNS resolution + SSRF defence (mirrors the CONNECT path logic).

e2e/rust/tests/forward_proxy_l7_bypass.rs

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
//! Regression test: the forward proxy path must reject requests to endpoints
5-
//! that have L7 rules configured. Before the fix, plain `http://` requests
6-
//! (which use the forward proxy, not CONNECT) bypassed per-request method/path
7-
//! enforcement entirely.
4+
//! Regression tests: the forward proxy path must evaluate L7 rules for
5+
//! endpoints that have them configured. Allowed requests (e.g. GET on a
6+
//! read-only endpoint) should succeed; denied requests (e.g. POST) should
7+
//! receive 403.
88
99
#![cfg(feature = "e2e")]
1010

@@ -145,6 +145,7 @@ network_policies:
145145
- host: host.openshell.internal
146146
port: {port}
147147
protocol: rest
148+
enforcement: enforce
148149
allowed_ips:
149150
- "172.0.0.0/8"
150151
rules:
@@ -164,24 +165,21 @@ network_policies:
164165
Ok(file)
165166
}
166167

167-
/// The forward proxy path (plain http:// via HTTP_PROXY) must return 403 for
168-
/// endpoints with L7 rules, forcing clients through the CONNECT tunnel where
169-
/// per-request method/path inspection actually happens.
168+
/// GET /allowed should succeed — the L7 policy explicitly allows it.
170169
#[tokio::test]
171-
async fn forward_proxy_rejects_l7_configured_endpoint() {
170+
async fn forward_proxy_allows_l7_permitted_request() {
172171
let server = DockerServer::start()
173172
.await
174173
.expect("start docker test server");
175-
let policy = write_policy_with_l7_rules(server.port).expect("write custom policy");
174+
let policy =
175+
write_policy_with_l7_rules(server.port)
176+
.expect("write custom policy");
176177
let policy_path = policy
177178
.path()
178179
.to_str()
179180
.expect("temp policy path should be utf-8")
180181
.to_string();
181182

182-
// Python script that tries a plain http:// request (forward proxy path).
183-
// HTTP_PROXY is set automatically by the sandbox, so urllib will use the
184-
// forward proxy for http:// URLs (not CONNECT).
185183
let script = format!(
186184
r#"
187185
import urllib.request, urllib.error, json, sys
@@ -208,10 +206,60 @@ except Exception as e:
208206
.await
209207
.expect("sandbox create");
210208

211-
// The forward proxy should return 403 because the endpoint has L7 rules.
209+
// L7 policy allows GET /allowed — should succeed.
210+
assert!(
211+
guard.create_output.contains("\"status\": 200"),
212+
"expected 200 for L7-allowed GET, got:\n{}",
213+
guard.create_output
214+
);
215+
}
216+
217+
/// POST /allowed should be denied — the L7 policy only allows GET.
218+
#[tokio::test]
219+
async fn forward_proxy_denies_l7_blocked_request() {
220+
let server = DockerServer::start()
221+
.await
222+
.expect("start docker test server");
223+
let policy =
224+
write_policy_with_l7_rules(server.port)
225+
.expect("write custom policy");
226+
let policy_path = policy
227+
.path()
228+
.to_str()
229+
.expect("temp policy path should be utf-8")
230+
.to_string();
231+
232+
let script = format!(
233+
r#"
234+
import urllib.request, urllib.error, json, sys
235+
url = "http://host.openshell.internal:{port}/allowed"
236+
req = urllib.request.Request(url, data=b"test", method="POST")
237+
try:
238+
resp = urllib.request.urlopen(req, timeout=15)
239+
print(json.dumps({{"status": resp.status, "error": None}}))
240+
except urllib.error.HTTPError as e:
241+
print(json.dumps({{"status": e.code, "error": str(e)}}))
242+
except Exception as e:
243+
print(json.dumps({{"status": -1, "error": str(e)}}))
244+
"#,
245+
port = server.port,
246+
);
247+
248+
let guard = SandboxGuard::create(&[
249+
"--policy",
250+
&policy_path,
251+
"--",
252+
"python3",
253+
"-c",
254+
&script,
255+
])
256+
.await
257+
.expect("sandbox create");
258+
259+
// L7 policy denies POST — should return 403.
212260
assert!(
213261
guard.create_output.contains("\"status\": 403"),
214-
"expected 403 from forward proxy for L7-configured endpoint, got:\n{}",
262+
"expected 403 for L7-denied POST, got:\n{}",
215263
guard.create_output
216264
);
217265
}

0 commit comments

Comments
 (0)