Skip to content

Commit c6f3087

Browse files
authored
fix(sandbox): relay WebSocket frames after HTTP 101 Switching Protocols (#718)
Detect 101 Switching Protocols in relay_response() and switch to raw bidirectional TCP relay instead of re-entering the HTTP parsing loop. Previously, is_bodiless_response() treated 101 as a generic 1xx informational response, forwarding only the headers and returning to the HTTP parsing loop. After a 101, subsequent bytes are upgraded protocol frames (e.g. WebSocket), not HTTP — causing the relay to block or silently drop all post-upgrade traffic. Changes: - Add RelayOutcome enum (Reusable/Consumed/Upgraded) replacing bool return type across L7Provider::relay trait and all relay functions - Detect 101 before generic 1xx handler in relay_response(), capture overflow bytes, return RelayOutcome::Upgraded - Validate client sent Upgrade + Connection: Upgrade headers before accepting 101 (rejects unsolicited upgrades from non-compliant upstream servers) - Extract shared handle_upgrade() helper used by both relay_rest() and relay_passthrough_with_credentials() - Add l7_decision=allow_upgrade audit log annotation for upgrades - Add unit tests for 101 overflow capture, unsolicited 101 rejection, and client_requested_upgrade header validation - Add integration test: WebSocket echo through L7Provider::relay Fixes: #652 Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
1 parent 1c659c1 commit c6f3087

File tree

6 files changed

+712
-56
lines changed

6 files changed

+712
-56
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/openshell-sandbox/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ uuid = { version = "1", features = ["v4"] }
8181
[dev-dependencies]
8282
tempfile = "3"
8383
temp-env = "0.3"
84+
tokio-tungstenite = { workspace = true }
85+
futures = { workspace = true }
8486

8587
[lints]
8688
workspace = true

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,22 @@ use std::collections::HashMap;
1414
use std::future::Future;
1515
use tokio::io::{AsyncRead, AsyncWrite};
1616

17+
/// Outcome of relaying a single HTTP request/response pair.
18+
#[derive(Debug)]
19+
pub enum RelayOutcome {
20+
/// Connection is reusable for further HTTP requests (keep-alive).
21+
Reusable,
22+
/// Connection was consumed (e.g. read-until-EOF or `Connection: close`).
23+
Consumed,
24+
/// Server responded with 101 Switching Protocols.
25+
/// The connection has been upgraded (e.g. to WebSocket) and must be
26+
/// relayed as raw bidirectional TCP from this point forward.
27+
/// Contains any overflow bytes read from upstream past the 101 response
28+
/// headers that belong to the upgraded protocol. The 101 headers
29+
/// themselves have already been forwarded to the client.
30+
Upgraded { overflow: Vec<u8> },
31+
}
32+
1733
/// Body framing for HTTP requests/responses.
1834
#[derive(Debug, Clone, Copy)]
1935
pub enum BodyLength {
@@ -57,14 +73,15 @@ pub trait L7Provider: Send + Sync {
5773

5874
/// Forward an allowed request to upstream and relay the response back.
5975
///
60-
/// Returns `true` if the upstream connection is reusable (keep-alive),
61-
/// `false` if it was consumed (e.g. read-until-EOF or `Connection: close`).
76+
/// Returns a [`RelayOutcome`] indicating whether the connection is
77+
/// reusable (keep-alive), consumed, or has been upgraded (101 Switching
78+
/// Protocols) and must be relayed as raw bidirectional TCP.
6279
fn relay<C, U>(
6380
&self,
6481
req: &L7Request,
6582
client: &mut C,
6683
upstream: &mut U,
67-
) -> impl Future<Output = Result<bool>> + Send
84+
) -> impl Future<Output = Result<RelayOutcome>> + Send
6885
where
6986
C: AsyncRead + AsyncWrite + Unpin + Send,
7087
U: AsyncRead + AsyncWrite + Unpin + Send;

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

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//! Parses each request within the tunnel, evaluates it against OPA policy,
88
//! and either forwards or denies the request.
99
10-
use crate::l7::provider::L7Provider;
10+
use crate::l7::provider::{L7Provider, RelayOutcome};
1111
use crate::l7::{EnforcementMode, L7EndpointConfig, L7Protocol, L7RequestInfo};
1212
use crate::secrets::{self, SecretResolver};
1313
use miette::{IntoDiagnostic, Result, miette};
@@ -68,6 +68,40 @@ where
6868
}
6969
}
7070

71+
/// Handle an upgraded connection (101 Switching Protocols).
72+
///
73+
/// Forwards any overflow bytes from the upgrade response to the client, then
74+
/// switches to raw bidirectional TCP copy for the upgraded protocol (WebSocket,
75+
/// HTTP/2, etc.). L7 policy enforcement does not apply after the upgrade —
76+
/// the initial HTTP request was already evaluated.
77+
async fn handle_upgrade<C, U>(
78+
client: &mut C,
79+
upstream: &mut U,
80+
overflow: Vec<u8>,
81+
host: &str,
82+
port: u16,
83+
) -> Result<()>
84+
where
85+
C: AsyncRead + AsyncWrite + Unpin + Send,
86+
U: AsyncRead + AsyncWrite + Unpin + Send,
87+
{
88+
info!(
89+
host = %host,
90+
port = port,
91+
overflow_bytes = overflow.len(),
92+
"101 Switching Protocols — switching to raw bidirectional relay \
93+
(L7 enforcement no longer active)"
94+
);
95+
if !overflow.is_empty() {
96+
client.write_all(&overflow).await.into_diagnostic()?;
97+
client.flush().await.into_diagnostic()?;
98+
}
99+
tokio::io::copy_bidirectional(client, upstream)
100+
.await
101+
.into_diagnostic()?;
102+
Ok(())
103+
}
104+
71105
/// REST relay loop: parse request -> evaluate -> allow/deny -> relay response -> repeat.
72106
async fn relay_rest<C, U>(
73107
config: &L7EndpointConfig,
@@ -137,10 +171,24 @@ where
137171
// Evaluate L7 policy via Rego (using redacted target)
138172
let (allowed, reason) = evaluate_l7_request(engine, ctx, &request_info)?;
139173

140-
let decision_str = match (allowed, config.enforcement) {
141-
(true, _) => "allow",
142-
(false, EnforcementMode::Audit) => "audit",
143-
(false, EnforcementMode::Enforce) => "deny",
174+
// Check if this is an upgrade request for logging purposes.
175+
let header_end = req
176+
.raw_header
177+
.windows(4)
178+
.position(|w| w == b"\r\n\r\n")
179+
.map_or(req.raw_header.len(), |p| p + 4);
180+
let is_upgrade_request = {
181+
let h = String::from_utf8_lossy(&req.raw_header[..header_end]);
182+
h.lines()
183+
.skip(1)
184+
.any(|l| l.to_ascii_lowercase().starts_with("upgrade:"))
185+
};
186+
187+
let decision_str = match (allowed, config.enforcement, is_upgrade_request) {
188+
(true, _, true) => "allow_upgrade",
189+
(true, _, false) => "allow",
190+
(false, EnforcementMode::Audit, _) => "audit",
191+
(false, EnforcementMode::Enforce, _) => "deny",
144192
};
145193

146194
// Log every L7 decision (using redacted target — never log real secrets)
@@ -162,20 +210,26 @@ where
162210

163211
if allowed || config.enforcement == EnforcementMode::Audit {
164212
// Forward request to upstream and relay response
165-
let reusable = crate::l7::rest::relay_http_request_with_resolver(
213+
let outcome = crate::l7::rest::relay_http_request_with_resolver(
166214
&req,
167215
client,
168216
upstream,
169217
ctx.secret_resolver.as_deref(),
170218
)
171219
.await?;
172-
if !reusable {
173-
debug!(
174-
host = %ctx.host,
175-
port = ctx.port,
176-
"Upstream connection not reusable, closing L7 relay"
177-
);
178-
return Ok(());
220+
match outcome {
221+
RelayOutcome::Reusable => {} // continue loop
222+
RelayOutcome::Consumed => {
223+
debug!(
224+
host = %ctx.host,
225+
port = ctx.port,
226+
"Upstream connection not reusable, closing L7 relay"
227+
);
228+
return Ok(());
229+
}
230+
RelayOutcome::Upgraded { overflow } => {
231+
return handle_upgrade(client, upstream, overflow, &ctx.host, ctx.port).await;
232+
}
179233
}
180234
} else {
181235
// Enforce mode: deny with 403 and close connection (use redacted target)
@@ -334,12 +388,16 @@ where
334388
// Forward request with credential rewriting and relay the response.
335389
// relay_http_request_with_resolver handles both directions: it sends
336390
// the request upstream and reads the response back to the client.
337-
let reusable =
391+
let outcome =
338392
crate::l7::rest::relay_http_request_with_resolver(&req, client, upstream, resolver)
339393
.await?;
340394

341-
if !reusable {
342-
break;
395+
match outcome {
396+
RelayOutcome::Reusable => {} // continue loop
397+
RelayOutcome::Consumed => break,
398+
RelayOutcome::Upgraded { overflow } => {
399+
return handle_upgrade(client, upstream, overflow, &ctx.host, ctx.port).await;
400+
}
343401
}
344402
}
345403

0 commit comments

Comments
 (0)