Skip to content

Commit f4a67c6

Browse files
committed
perf(server): drop per-connection session tokens for TCP forwards
`openshell forward service` minted an SSH session token before every forwarded TCP connection and revoked it afterwards: two store commits per connection. The token added nothing on that path. `ForwardTcp` already authenticates the caller and authorizes it against the sandbox's workspace on every stream before it looks at the token, the relay to the supervisor is opened with the sandbox id and target only, and the token is never forwarded, audited, or visible to the target service. The mechanism exists for `openshell sandbox ssh`, where the process that opens the stream is an ssh ProxyCommand holding nothing but the token. Reusing it per TCP connection put a store write on the connect path and serialized concurrent forwards on commit latency: #3494 measured the symptom, and #3543 made the commits cheaper, but each one still holds SQLite's writer lock for an fsync, so connection setup under a burst stayed linear in the number of concurrent connections. Let `target.tcp` streams omit `authorization_token`. The gateway admits them on the already-authorized principal, counts them against the same per-sandbox connection cap, and touches no store. `target.ssh` streams keep requiring the token. A token supplied with a TCP target is still validated and counted per token, so an older CLI against a new gateway is unchanged. The CLI stops minting and revoking a session per forwarded connection; against a gateway that predates this change it recognizes the `authorization_token is required` rejection once and falls back to per-connection tokens for the rest of that forward. Tests cover token-less TCP admission and slot release, SSH targets still rejected without a token, a supplied token still validated, and the per-sandbox cap for token-less forwards. CLI integration tests run `service_forward_tcp` against a mock gateway: token-less inits echo data with no CreateSshSession or RevokeSshSession call, and a gateway that rejects the empty token is detected once, after which every connection in that forward mints and revokes its own token. Architecture and security docs describe which targets carry a token, and the per-token connection limit now reads 3, matching the gateway. Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
1 parent 4ce767f commit f4a67c6

5 files changed

Lines changed: 545 additions & 74 deletions

File tree

‎architecture/gateway.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,10 +1132,15 @@ controls through an extended interface.
11321132

11331133
`ForwardTcp` is the client-facing byte stream for SSH and service forwarding.
11341134
The first frame is a `TcpForwardInit` that carries the workspace-scoped sandbox
1135-
name, an authorization token from `CreateSshSession`, and an explicit target:
1136-
`target.ssh` for the sandbox SSH socket or `target.tcp` for a loopback service
1137-
inside the sandbox. The gateway validates the token and sandbox readiness,
1138-
sends a targeted `RelayOpen` to the supervisor, then bridges
1135+
name and an explicit target: `target.ssh` for the sandbox SSH socket or
1136+
`target.tcp` for a loopback service inside the sandbox. The gateway authorizes
1137+
the caller's principal against the sandbox's workspace on every stream. SSH
1138+
targets additionally require the `authorization_token` issued by
1139+
`CreateSshSession`, because the process that opens them is an ssh
1140+
`ProxyCommand` that holds nothing else. TCP targets carry no token, so a
1141+
service forward costs no store access per connection and only the in-memory
1142+
per-sandbox connection cap applies. The gateway then checks sandbox readiness,
1143+
sends a targeted `RelayOpen` to the supervisor, and bridges
11391144
`TcpForwardFrame::Data` to `RelayFrame::Data` until either side closes.
11401145

11411146
Browser service URLs use the same supervisor relay path after host-based

‎crates/openshell-cli/src/run.rs‎

Lines changed: 132 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,6 +2000,9 @@ pub async fn service_forward_tcp(
20002000
let sandbox_name = name.to_string();
20012001
let sandbox_workspace = workspace.to_string();
20022002
let (fatal_tx, mut fatal_rx) = tokio::sync::mpsc::channel::<String>(1);
2003+
// Set once this forward learns that the gateway predates principal-authorized
2004+
// TCP forwards and still requires a `CreateSshSession` token per connection.
2005+
let legacy_session_tokens = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
20032006
let mut health_check = tokio::time::interval(Duration::from_secs(2));
20042007
health_check.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
20052008
loop {
@@ -2023,21 +2026,8 @@ pub async fn service_forward_tcp(
20232026
let target_host = target_host.to_string();
20242027
let service_id = format!("service-forward:{name}:{target_host}:{target_port}");
20252028
let fatal_tx = fatal_tx.clone();
2029+
let legacy_session_tokens = legacy_session_tokens.clone();
20262030
tokio::spawn(async move {
2027-
let token = match create_forward_session_token(
2028-
&mut client,
2029-
&sandbox_name,
2030-
&sandbox_workspace,
2031-
).await {
2032-
Ok(token) => token,
2033-
Err(err) => {
2034-
tracing::warn!(peer = %peer, error = %err, "service forward session creation failed");
2035-
if err.fatal {
2036-
let _ = fatal_tx.send(err.message).await;
2037-
}
2038-
return;
2039-
}
2040-
};
20412031
if let Err(err) = forward_one_tcp_connection(
20422032
&mut client,
20432033
socket,
@@ -2046,7 +2036,7 @@ pub async fn service_forward_tcp(
20462036
target_host,
20472037
target_port,
20482038
service_id,
2049-
token.clone(),
2039+
legacy_session_tokens,
20502040
)
20512041
.await
20522042
{
@@ -2055,9 +2045,6 @@ pub async fn service_forward_tcp(
20552045
let _ = fatal_tx.send(err.message).await;
20562046
}
20572047
}
2058-
let _ = client
2059-
.revoke_ssh_session(RevokeSshSessionRequest { allow_missing: true, token })
2060-
.await;
20612048
});
20622049
}
20632050
}
@@ -2081,6 +2068,14 @@ async fn create_forward_session_token(
20812068
Ok(response.into_inner().token)
20822069
}
20832070

2071+
/// Older gateways reject a token-less `ForwardTcp` init with this
2072+
/// `Unauthenticated` status; newer ones authorize TCP targets on the caller's
2073+
/// principal and only demand a token for SSH targets.
2074+
fn forward_requires_session_token(status: &Status) -> bool {
2075+
status.code() == Code::Unauthenticated
2076+
&& status.message().contains("authorization_token is required")
2077+
}
2078+
20842079
async fn fetch_ready_sandbox_for_forward(
20852080
client: &mut crate::tls::GrpcClient,
20862081
name: &str,
@@ -2182,37 +2177,120 @@ async fn forward_one_tcp_connection(
21822177
target_host: String,
21832178
target_port: u16,
21842179
service_id: String,
2185-
authorization_token: String,
2180+
legacy_session_tokens: std::sync::Arc<std::sync::atomic::AtomicBool>,
21862181
) -> std::result::Result<(), ForwardTcpConnectionError> {
2187-
use tokio::io::{AsyncReadExt, AsyncWriteExt};
2182+
let mut init = TcpForwardInit {
2183+
sandbox: sandbox_name.clone(),
2184+
workspace: workspace.clone(),
2185+
service_id,
2186+
target: Some(tcp_forward_init::Target::Tcp(TcpRelayTarget {
2187+
host: target_host,
2188+
port: u32::from(target_port),
2189+
})),
2190+
// The gateway authorizes TCP forwards on this client's credentials for
2191+
// every stream, so no per-connection session token is minted unless the
2192+
// gateway turns out to predate that (`forward_requires_session_token`),
2193+
// which this forward remembers in `legacy_session_tokens`.
2194+
authorization_token: String::new(),
2195+
};
2196+
2197+
let mut session_token = None;
2198+
if legacy_session_tokens.load(std::sync::atomic::Ordering::Relaxed) {
2199+
match create_forward_session_token(client, &sandbox_name, &workspace).await {
2200+
Ok(token) => {
2201+
init.authorization_token.clone_from(&token);
2202+
session_token = Some(token);
2203+
}
2204+
Err(err) => {
2205+
drain_and_shutdown_local_socket(socket).await;
2206+
return Err(err);
2207+
}
2208+
}
2209+
}
2210+
2211+
let opened = match open_forward_tcp_stream(client, init.clone()).await {
2212+
Ok(opened) => opened,
2213+
Err(status) if session_token.is_none() && forward_requires_session_token(&status) => {
2214+
tracing::info!(
2215+
"gateway requires an SSH session token per forwarded connection; \
2216+
minting one per connection for the rest of this forward"
2217+
);
2218+
legacy_session_tokens.store(true, std::sync::atomic::Ordering::Relaxed);
2219+
let token = match create_forward_session_token(client, &sandbox_name, &workspace).await
2220+
{
2221+
Ok(token) => token,
2222+
Err(err) => {
2223+
drain_and_shutdown_local_socket(socket).await;
2224+
return Err(err);
2225+
}
2226+
};
2227+
init.authorization_token.clone_from(&token);
2228+
session_token = Some(token);
2229+
match open_forward_tcp_stream(client, init).await {
2230+
Ok(opened) => opened,
2231+
Err(status) => {
2232+
drain_and_shutdown_local_socket(socket).await;
2233+
revoke_forward_session_token(client, session_token).await;
2234+
return Err(ForwardTcpConnectionError::from_status(status));
2235+
}
2236+
}
2237+
}
2238+
Err(status) => {
2239+
drain_and_shutdown_local_socket(socket).await;
2240+
revoke_forward_session_token(client, session_token).await;
2241+
return Err(ForwardTcpConnectionError::from_status(status));
2242+
}
2243+
};
2244+
2245+
let result = bridge_local_socket_to_forward_stream(socket, opened).await;
2246+
revoke_forward_session_token(client, session_token).await;
2247+
result
2248+
}
2249+
2250+
/// An open `ForwardTcp` stream: the sender for local-to-gateway frames and the
2251+
/// gateway-to-local response stream.
2252+
type OpenForwardTcpStream = (
2253+
tokio::sync::mpsc::Sender<TcpForwardFrame>,
2254+
tonic::Streaming<TcpForwardFrame>,
2255+
);
2256+
2257+
async fn open_forward_tcp_stream(
2258+
client: &mut crate::tls::GrpcClient,
2259+
init: TcpForwardInit,
2260+
) -> std::result::Result<OpenForwardTcpStream, Status> {
21882261
use tokio_stream::wrappers::ReceiverStream;
21892262

21902263
let (tx, rx) = tokio::sync::mpsc::channel::<TcpForwardFrame>(16);
21912264
tx.send(TcpForwardFrame {
21922265
payload: Some(openshell_core::proto::tcp_forward_frame::Payload::Init(
2193-
TcpForwardInit {
2194-
sandbox: sandbox_name,
2195-
workspace: workspace.clone(),
2196-
service_id,
2197-
target: Some(tcp_forward_init::Target::Tcp(TcpRelayTarget {
2198-
host: target_host,
2199-
port: u32::from(target_port),
2200-
})),
2201-
authorization_token,
2202-
},
2266+
init,
22032267
)),
22042268
})
22052269
.await
2206-
.map_err(|_| ForwardTcpConnectionError::transient("failed to initialize forward stream"))?;
2270+
.map_err(|_| Status::internal("failed to initialize forward stream"))?;
2271+
let response = client
2272+
.forward_tcp(ReceiverStream::new(rx))
2273+
.await?
2274+
.into_inner();
2275+
Ok((tx, response))
2276+
}
22072277

2208-
let mut response = match client.forward_tcp(ReceiverStream::new(rx)).await {
2209-
Ok(response) => response.into_inner(),
2210-
Err(status) => {
2211-
let err = ForwardTcpConnectionError::from_status(status);
2212-
drain_and_shutdown_local_socket(socket).await;
2213-
return Err(err);
2214-
}
2215-
};
2278+
async fn revoke_forward_session_token(client: &mut crate::tls::GrpcClient, token: Option<String>) {
2279+
if let Some(token) = token {
2280+
let _ = client
2281+
.revoke_ssh_session(RevokeSshSessionRequest {
2282+
allow_missing: true,
2283+
token,
2284+
})
2285+
.await;
2286+
}
2287+
}
2288+
2289+
async fn bridge_local_socket_to_forward_stream(
2290+
socket: tokio::net::TcpStream,
2291+
(tx, mut response): OpenForwardTcpStream,
2292+
) -> std::result::Result<(), ForwardTcpConnectionError> {
2293+
use tokio::io::{AsyncReadExt, AsyncWriteExt};
22162294

22172295
let (mut local_read, mut local_write) = socket.into_split();
22182296

@@ -6402,8 +6480,8 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String
64026480
mod tests {
64036481
use super::{
64046482
PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, format_endpoint,
6405-
format_log_line, git_sync_files, has_main_process_result, parse_cli_setting_value,
6406-
parse_credential_expiry_cli_value, parse_driver_config_json,
6483+
format_log_line, forward_requires_session_token, git_sync_files, has_main_process_result,
6484+
parse_cli_setting_value, parse_credential_expiry_cli_value, parse_driver_config_json,
64076485
parse_secret_material_env_pairs, policy_revision_list_json, policy_revision_to_json,
64086486
proto_execution_timeout, provisioning_timeout_message, ready_false_condition_message,
64096487
resolve_from, rootfs_tar_sources_supported_for_gateway, sandbox_should_persist,
@@ -8052,4 +8130,17 @@ mod tests {
80528130
let log = log_line("OCSF", "ocsf", message, "sandbox", &[]);
80538131
assert!(format_log_line(&log).ends_with(message));
80548132
}
8133+
8134+
#[test]
8135+
fn forward_requires_session_token_matches_only_the_legacy_gateway_error() {
8136+
assert!(forward_requires_session_token(&Status::unauthenticated(
8137+
"authorization_token is required for ForwardTcp"
8138+
)));
8139+
assert!(!forward_requires_session_token(&Status::unauthenticated(
8140+
"SSH session token not found"
8141+
)));
8142+
assert!(!forward_requires_session_token(&Status::permission_denied(
8143+
"authorization_token is required for ForwardTcp"
8144+
)));
8145+
}
80558146
}

0 commit comments

Comments
 (0)