Skip to content

Commit 0914f3f

Browse files
authored
fix(sandbox): log L7 parse denials (#1072)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent 2472474 commit 0914f3f

3 files changed

Lines changed: 93 additions & 11 deletions

File tree

architecture/sandbox.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,7 @@ Implements `L7Provider` for HTTP/1.1:
12271227

12281228
`relay_with_inspection()` in `crates/openshell-sandbox/src/l7/relay.rs` is the main relay loop:
12291229

1230-
1. Parse one HTTP request from client via the provider
1230+
1. Parse one HTTP request from client via the provider. Parser and path-canonicalization failures close the connection and emit a denied OCSF network event with the rejection reason in `status_detail`.
12311231
2. Resolve credential placeholders in the request target via `rewrite_target_for_eval()`. OPA receives the redacted path (`[CREDENTIAL]` markers); the resolved path goes only to upstream. If resolution fails, return HTTP 500 and close the connection.
12321232
3. Build L7 input JSON with `request.method`, the **redacted** `request.path`, `request.query_params`, plus the CONNECT-level context (host, port, binary, ancestors, cmdline)
12331233
4. Evaluate `data.openshell.sandbox.allow_request` and `data.openshell.sandbox.request_deny_reason`
@@ -1580,7 +1580,7 @@ The sandbox uses `miette` for error reporting and `thiserror` for typed errors.
15801580
| Credential injection: resolved value contains CR/LF/null | Placeholder treated as unresolvable, fail-closed |
15811581
| Credential injection: path credential contains traversal/separator | HTTP 500, connection closed (fail-closed) |
15821582
| Credential injection: percent-encoded placeholder bypass attempt | HTTP 500, connection closed (fail-closed) |
1583-
| L7 parse error | Close the connection |
1583+
| L7 parse or path-canonicalization error | Emit denied OCSF network event with `status_detail`, close the connection |
15841584
| SSH socket bind failure | Fatal -- reported through the readiness channel and aborts startup |
15851585
| SSH server accept failure | Async task error logged, main process unaffected |
15861586
| Supervisor session: connect failure | Emit `session_failed` OCSF event, sleep with exponential backoff (1s -> 30s) and reconnect |

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

Lines changed: 90 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::secrets::{self, SecretResolver};
1313
use miette::{IntoDiagnostic, Result, miette};
1414
use openshell_ocsf::{
1515
ActionId, ActivityId, DispositionId, Endpoint, HttpActivityBuilder, HttpRequest,
16-
NetworkActivityBuilder, SeverityId, Url as OcsfUrl, ocsf_emit,
16+
NetworkActivityBuilder, SeverityId, StatusId, Url as OcsfUrl, ocsf_emit,
1717
};
1818
use std::sync::{Arc, Mutex};
1919
use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt};
@@ -37,6 +37,50 @@ pub struct L7EvalContext {
3737
pub(crate) secret_resolver: Option<Arc<SecretResolver>>,
3838
}
3939

40+
#[derive(Debug, Clone, Copy)]
41+
enum ParseRejectionMode {
42+
L7Endpoint,
43+
Passthrough,
44+
}
45+
46+
fn parse_rejection_detail(error: &str, mode: ParseRejectionMode) -> String {
47+
if error.contains("encoded '/' (%2F)") {
48+
match mode {
49+
ParseRejectionMode::L7Endpoint => format!(
50+
"{error}; set allow_encoded_slash: true on this endpoint if the upstream requires encoded slashes"
51+
),
52+
ParseRejectionMode::Passthrough => format!(
53+
"{error}; passthrough credential relay uses strict path parsing, so configure this endpoint with protocol: rest and allow_encoded_slash: true for encoded-slash APIs, or use tls: skip if HTTP parsing is not needed"
54+
),
55+
}
56+
} else {
57+
error.to_string()
58+
}
59+
}
60+
61+
fn emit_parse_rejection(ctx: &L7EvalContext, detail: &str, engine_type: &str) {
62+
let policy_name = if ctx.policy_name.is_empty() {
63+
"-"
64+
} else {
65+
&ctx.policy_name
66+
};
67+
let event = NetworkActivityBuilder::new(crate::ocsf_ctx())
68+
.activity(ActivityId::Open)
69+
.action(ActionId::Denied)
70+
.disposition(DispositionId::Blocked)
71+
.severity(SeverityId::Medium)
72+
.status(StatusId::Failure)
73+
.dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port))
74+
.firewall_rule(policy_name, engine_type)
75+
.message(format!(
76+
"HTTP request rejected before policy evaluation for {}:{}",
77+
ctx.host, ctx.port
78+
))
79+
.status_detail(detail)
80+
.build();
81+
ocsf_emit!(event);
82+
}
83+
4084
/// Run protocol-aware L7 inspection on a tunnel.
4185
///
4286
/// This replaces `copy_bidirectional` for L7-enabled endpoints.
@@ -150,13 +194,9 @@ where
150194
"L7 connection closed"
151195
);
152196
} else {
153-
let event = NetworkActivityBuilder::new(crate::ocsf_ctx())
154-
.activity(ActivityId::Fail)
155-
.severity(SeverityId::Low)
156-
.dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port))
157-
.message(format!("HTTP parse error in L7 relay: {e}"))
158-
.build();
159-
ocsf_emit!(event);
197+
let detail =
198+
parse_rejection_detail(&e.to_string(), ParseRejectionMode::L7Endpoint);
199+
emit_parse_rejection(ctx, &detail, "l7");
160200
}
161201
return Ok(()); // Close connection on parse error
162202
}
@@ -394,7 +434,10 @@ where
394434
if is_benign_connection_error(&e) {
395435
break;
396436
}
397-
return Err(e);
437+
let detail =
438+
parse_rejection_detail(&e.to_string(), ParseRejectionMode::Passthrough);
439+
emit_parse_rejection(ctx, &detail, "http-parser");
440+
return Ok(());
398441
}
399442
};
400443

@@ -468,3 +511,41 @@ where
468511

469512
Ok(())
470513
}
514+
515+
#[cfg(test)]
516+
mod tests {
517+
use super::*;
518+
519+
#[test]
520+
fn parse_rejection_detail_adds_l7_hint_for_encoded_slash() {
521+
let detail = parse_rejection_detail(
522+
"HTTP request-target rejected: request-target contains an encoded '/' (%2F) which is not allowed on this endpoint",
523+
ParseRejectionMode::L7Endpoint,
524+
);
525+
526+
assert!(detail.contains("allow_encoded_slash: true"));
527+
assert!(detail.contains("upstream requires encoded slashes"));
528+
}
529+
530+
#[test]
531+
fn parse_rejection_detail_adds_passthrough_hint_for_encoded_slash() {
532+
let detail = parse_rejection_detail(
533+
"HTTP request-target rejected: request-target contains an encoded '/' (%2F) which is not allowed on this endpoint",
534+
ParseRejectionMode::Passthrough,
535+
);
536+
537+
assert!(detail.contains("protocol: rest"));
538+
assert!(detail.contains("allow_encoded_slash: true"));
539+
assert!(detail.contains("tls: skip"));
540+
}
541+
542+
#[test]
543+
fn parse_rejection_detail_preserves_other_errors() {
544+
let error = "HTTP headers contain invalid UTF-8";
545+
546+
assert_eq!(
547+
parse_rejection_detail(error, ParseRejectionMode::L7Endpoint),
548+
error
549+
);
550+
}
551+
}

docs/observability/logging.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ Common reason phrases emitted by the sandbox include:
169169
| `resolves to <ip> which is not in allowed_ips, connection rejected` | The destination resolved to an IP outside the policy's `allowed_ips` allowlist. |
170170
| `DNS resolution failed for <host>:<port>` | The proxy could not resolve the destination. |
171171
| `port <n> is a blocked control-plane port, connection rejected` | The destination port matches a control-plane port (etcd, Kubernetes API, kubelet) and is always blocked. |
172+
| `request-target contains an encoded '/' (%2F)` | The L7 HTTP parser rejected an encoded slash. Configure `allow_encoded_slash: true` on a REST endpoint when the upstream requires encoded slashes. |
172173
| `l7 deny` | An L7 policy rule denied the request. |
173174

174175
Invalid `allowed_ips` entries and entries that overlap always-blocked ranges are rejected at policy-load time, so they never reach the runtime denial path. The phrases above come from the proxy's per-CONNECT `allowed_ips` and SSRF checks, not from policy validation.

0 commit comments

Comments
 (0)