diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 27e5b80dd9..056b33fead 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -46,6 +46,9 @@ pub const DEFAULT_DOCKER_NETWORK_NAME: &str = "openshell-docker"; /// Default domain used for browser-facing sandbox service URLs. pub const DEFAULT_SERVICE_ROUTING_DOMAIN: &str = "openshell.localhost"; +/// Default per-sandbox cap for each relay budget (SSH and service forwards). +pub const DEFAULT_MAX_CONNECTIONS_PER_SANDBOX: u32 = 20; + /// Gateway delivery path for supervisor configuration. /// /// `Push` sends configuration over the supervisor session to supervisors that @@ -312,6 +315,14 @@ pub struct Config { /// TTL for SSH session tokens, in seconds. 0 disables expiry. pub ssh_session_ttl_secs: u64, + /// Maximum concurrent SSH relays per sandbox. Must be at least 1. + pub max_ssh_connections_per_sandbox: u32, + + /// Maximum concurrent service forward relays per sandbox. Must be at + /// least 1. Kept apart from the SSH budget so open forwards cannot block + /// SSH access. + pub max_service_connections_per_sandbox: u32, + /// Absolute image preparation and initial supervisor startup budget for new /// sandbox attempts, in seconds. Must be between 1 and 86400, inclusive. pub image_preparation_timeout_seconds: u32, @@ -1011,6 +1022,8 @@ impl Config { credential_drivers: Vec::new(), default_credential_driver: None, ssh_session_ttl_secs: default_ssh_session_ttl_secs(), + max_ssh_connections_per_sandbox: DEFAULT_MAX_CONNECTIONS_PER_SANDBOX, + max_service_connections_per_sandbox: DEFAULT_MAX_CONNECTIONS_PER_SANDBOX, image_preparation_timeout_seconds: 1800, grpc_rate_limit_requests: None, grpc_rate_limit_window_secs: None, diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 9b16b53914..09dc346ed2 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -628,6 +628,19 @@ fn prepare_server_config_with_drivers( config = config.with_ssh_session_ttl_secs(ttl); } + if let Some(gateway_file) = file.as_ref().map(|f| &f.openshell.gateway) { + apply_connection_cap( + "max_ssh_connections_per_sandbox", + gateway_file.max_ssh_connections_per_sandbox, + &mut config.max_ssh_connections_per_sandbox, + )?; + apply_connection_cap( + "max_service_connections_per_sandbox", + gateway_file.max_service_connections_per_sandbox, + &mut config.max_service_connections_per_sandbox, + )?; + } + if let Some(mode) = file .as_ref() .and_then(|f| f.openshell.gateway.policy_validation_failure_mode) @@ -1484,6 +1497,17 @@ fn validate_grpc_rate_limit_args(requests: Option, window_seconds: Option, cap: &mut u32) -> Result<()> { + if let Some(value) = value { + if value == 0 { + return Err(miette::miette!("{name} must be at least 1")); + } + *cap = value; + } + Ok(()) +} + fn normalize_compute_driver_socket_args(args: &mut RunArgs) -> Result<()> { let Some(socket) = args.compute_driver_socket.as_ref() else { return Ok(()); @@ -4364,6 +4388,63 @@ mem_mib = "not-a-number" }); } + #[test] + fn server_config_sets_and_validates_per_sandbox_connection_caps() { + let state = tempfile::tempdir().unwrap(); + let tls = tempfile::tempdir().unwrap(); + Environment::new() + .set("XDG_STATE_HOME", state.path().to_str().unwrap()) + .set("OPENSHELL_LOCAL_TLS_DIR", tls.path().to_str().unwrap()) + .run(|| { + let state = std::path::PathBuf::from(std::env::var_os("XDG_STATE_HOME").unwrap()); + let config_path = state.join("gateway.toml"); + let prepare = |gateway: &str| { + std::fs::write( + &config_path, + format!("[openshell]\nversion = 2\n[openshell.gateway]\n{gateway}\n"), + ) + .unwrap(); + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--config", + config_path.to_str().unwrap(), + "--db-url", + "sqlite::memory:", + "--compute-driver", + "docker", + "--disable-tls", + ]); + super::prepare_server_config(&mut args, &matches) + }; + + let defaults = prepare("").expect("defaults"); + assert_eq!(defaults.config.max_ssh_connections_per_sandbox, 20); + assert_eq!(defaults.config.max_service_connections_per_sandbox, 20); + + let set = prepare( + "max_ssh_connections_per_sandbox = 5\nmax_service_connections_per_sandbox = 50", + ) + .expect("caps are applied"); + assert_eq!(set.config.max_ssh_connections_per_sandbox, 5); + assert_eq!(set.config.max_service_connections_per_sandbox, 50); + + for key in [ + "max_ssh_connections_per_sandbox", + "max_service_connections_per_sandbox", + ] { + let Err(error) = prepare(&format!("{key} = 0")) else { + panic!("a zero cap must be rejected"); + }; + assert!( + error + .to_string() + .contains(&format!("{key} must be at least 1")), + "{error}" + ); + } + }); + } + #[test] fn explicit_launch_signing_config_ignores_partial_local_bundle() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-server/src/config_file.rs b/crates/openshell-server/src/config_file.rs index cee598a992..e0891c33ac 100644 --- a/crates/openshell-server/src/config_file.rs +++ b/crates/openshell-server/src/config_file.rs @@ -140,6 +140,13 @@ pub struct GatewayFileSection { // ── Sandbox / SSH ──────────────────────────────────────────────────── #[serde(default)] pub ssh_session_ttl_secs: Option, + /// Per-sandbox cap on concurrent SSH relays; omit to keep the default of 20. + #[serde(default)] + pub max_ssh_connections_per_sandbox: Option, + /// Per-sandbox cap on concurrent service forward relays; omit to keep the + /// default of 20. + #[serde(default)] + pub max_service_connections_per_sandbox: Option, /// Absolute preparation budget for new attempts; existing deadlines persist. #[serde(default)] pub image_preparation_timeout_seconds: Option, diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 77a54a2bc8..967b0e453c 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -2814,9 +2814,48 @@ pub(super) async fn handle_forward_tcp( Ok(Response::new(stream)) } +/// The per-sandbox relay budget a forward stream draws from. SSH relays and +/// service forwards are counted apart so open forwards cannot starve SSH. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum RelayBudget { + Ssh, + Service, +} + +impl RelayBudget { + fn of(init: &TcpForwardInit) -> Self { + match init.target { + Some(tcp_forward_init::Target::Tcp(_)) => Self::Service, + _ => Self::Ssh, + } + } + + fn counts(self, state: &ServerState) -> &std::sync::Mutex> { + match self { + Self::Ssh => &state.ssh_connections_by_sandbox, + Self::Service => &state.service_connections_by_sandbox, + } + } + + fn limit(self, config: &openshell_core::Config) -> u32 { + match self { + Self::Ssh => config.max_ssh_connections_per_sandbox, + Self::Service => config.max_service_connections_per_sandbox, + } + } + + const fn exhausted_message(self) -> &'static str { + match self { + Self::Ssh => "sandbox SSH connection limit reached", + Self::Service => "sandbox service forward connection limit reached", + } + } +} + struct ForwardConnectionGuard { state: Arc, token: Option, + budget: RelayBudget, sandbox_id: String, } @@ -2825,7 +2864,7 @@ impl Drop for ForwardConnectionGuard { if let Some(token) = self.token.as_deref() { decrement_ssh_connection_count(&self.state.ssh_connections_by_token, token); } - decrement_ssh_connection_count(&self.state.ssh_connections_by_sandbox, &self.sandbox_id); + decrement_ssh_connection_count(self.budget.counts(&self.state), &self.sandbox_id); } } @@ -2834,7 +2873,8 @@ impl Drop for ForwardConnectionGuard { /// `target.tcp` streams are authorized by the caller's gateway principal, which /// `handle_forward_tcp` has already checked against the sandbox's workspace, so /// they carry no session token and touch no store: the only per-connection -/// state is the in-memory per-sandbox connection count. `target.ssh` streams +/// state is the in-memory per-sandbox connection count, kept in a budget apart +/// from SSH relays (see [`RelayBudget`]). `target.ssh` streams /// keep requiring the `CreateSshSession` token because the process that opens /// them is an ssh `ProxyCommand` that holds nothing else. A token supplied with /// a TCP target (older clients) is still validated and counted per token. @@ -2845,9 +2885,10 @@ async fn acquire_forward_connection_guard( sandbox: &Sandbox, ) -> Result { let sandbox_id = sandbox.object_id().to_string(); + let budget = RelayBudget::of(init); let token = init.authorization_token.trim(); if token.is_empty() { - if !matches!(init.target, Some(tcp_forward_init::Target::Tcp(_))) { + if budget == RelayBudget::Ssh { return Err(crate::auth::audit::reject_credential( Some(principal), AuthRejection::new( @@ -2857,10 +2898,11 @@ async fn acquire_forward_connection_guard( ), )); } - acquire_sandbox_connection_slot(&state.ssh_connections_by_sandbox, &sandbox_id)?; + acquire_sandbox_connection_slot(state, budget, &sandbox_id)?; return Ok(ForwardConnectionGuard { state: state.clone(), token: None, + budget, sandbox_id, }); } @@ -2869,16 +2911,12 @@ async fn acquire_forward_connection_guard( .await .map_err(|rejection| crate::auth::audit::reject_credential(Some(principal), rejection))?; crate::auth::audit::emit_ssh_session_logon(&state.logons, principal, token); - acquire_ssh_connection_slots( - &state.ssh_connections_by_token, - &state.ssh_connections_by_sandbox, - token, - &sandbox_id, - )?; + acquire_ssh_connection_slots(state, budget, token, &sandbox_id)?; Ok(ForwardConnectionGuard { state: state.clone(), token: Some(token.to_string()), + budget, sandbox_id, }) } @@ -2942,16 +2980,15 @@ async fn validate_ssh_forward_token( } const MAX_CONNECTIONS_PER_TOKEN: u32 = 3; -const MAX_CONNECTIONS_PER_SANDBOX: u32 = 20; fn acquire_ssh_connection_slots( - token_counts: &std::sync::Mutex>, - sandbox_counts: &std::sync::Mutex>, + state: &ServerState, + budget: RelayBudget, token: &str, sandbox_id: &str, ) -> Result<(), Status> { { - let mut counts = token_counts.lock().unwrap(); + let mut counts = state.ssh_connections_by_token.lock().unwrap(); let count = counts.entry(token.to_string()).or_insert(0); if *count >= MAX_CONNECTIONS_PER_TOKEN { return Err(Status::resource_exhausted( @@ -2961,26 +2998,24 @@ fn acquire_ssh_connection_slots( *count += 1; } - if let Err(status) = acquire_sandbox_connection_slot(sandbox_counts, sandbox_id) { - decrement_ssh_connection_count(token_counts, token); + if let Err(status) = acquire_sandbox_connection_slot(state, budget, sandbox_id) { + decrement_ssh_connection_count(&state.ssh_connections_by_token, token); return Err(status); } Ok(()) } -/// Reserve one of the per-sandbox relay slots shared by SSH sessions and -/// service forwards. +/// Reserve one of the sandbox's relay slots in the given budget. fn acquire_sandbox_connection_slot( - sandbox_counts: &std::sync::Mutex>, + state: &ServerState, + budget: RelayBudget, sandbox_id: &str, ) -> Result<(), Status> { - let mut counts = sandbox_counts.lock().unwrap(); + let mut counts = budget.counts(state).lock().unwrap(); let count = counts.entry(sandbox_id.to_string()).or_insert(0); - if *count >= MAX_CONNECTIONS_PER_SANDBOX { - return Err(Status::resource_exhausted( - "sandbox SSH connection limit reached", - )); + if *count >= budget.limit(&state.config) { + return Err(Status::resource_exhausted(budget.exhausted_message())); } *count += 1; Ok(()) @@ -4584,9 +4619,9 @@ mod tests { }) } - fn sandbox_connection_count(state: &ServerState, sandbox_id: &str) -> u32 { - state - .ssh_connections_by_sandbox + fn sandbox_connection_count(state: &ServerState, budget: RelayBudget, sandbox_id: &str) -> u32 { + budget + .counts(state) .lock() .unwrap() .get(sandbox_id) @@ -4613,10 +4648,13 @@ mod tests { "no per-token accounting without a token" ); assert!(state.ssh_connections_by_token.lock().unwrap().is_empty()); - assert_eq!(sandbox_connection_count(&state, sandbox.object_id()), 1); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), + 1 + ); drop(guard); assert_eq!( - sandbox_connection_count(&state, sandbox.object_id()), + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), 0, "dropping the guard releases the sandbox slot" ); @@ -4711,7 +4749,10 @@ mod tests { err.message(), "authorization_token is required for SSH targets" ); - assert_eq!(sandbox_connection_count(&state, sandbox.object_id()), 0); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Ssh, sandbox.object_id()), + 0 + ); assert_ssh_token_failure(&events, "missing_credential", ""); } @@ -4730,7 +4771,10 @@ mod tests { assert_eq!(err.code(), tonic::Code::Unauthenticated); assert_eq!(err.message(), "SSH session token not found"); - assert_eq!(sandbox_connection_count(&state, sandbox.object_id()), 0); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), + 0 + ); assert_ssh_token_failure(&events, "invalid_credential", "canary-ssh-token-unknown"); } @@ -4844,13 +4888,14 @@ mod tests { } #[tokio::test] - async fn tcp_forwards_without_token_share_the_per_sandbox_connection_cap() { + async fn tcp_forwards_are_capped_per_sandbox() { let state = test_server_state().await; let sandbox = test_sandbox("fwd", Vec::new()); let init = forward_init(loopback_tcp_target(), ""); + let cap = openshell_core::config::DEFAULT_MAX_CONNECTIONS_PER_SANDBOX; let mut guards = Vec::new(); - for _ in 0..MAX_CONNECTIONS_PER_SANDBOX { + for _ in 0..cap { guards.push( acquire_forward_connection_guard(&state, &forward_caller(), &init, &sandbox) .await @@ -4862,10 +4907,13 @@ mod tests { .err() .expect("cap reached"); assert_eq!(err.code(), tonic::Code::ResourceExhausted); - assert_eq!(err.message(), "sandbox SSH connection limit reached"); assert_eq!( - sandbox_connection_count(&state, sandbox.object_id()), - MAX_CONNECTIONS_PER_SANDBOX + err.message(), + "sandbox service forward connection limit reached" + ); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), + cap ); guards.pop(); @@ -4874,7 +4922,65 @@ mod tests { .expect("slot freed by a closed connection"); drop(guard); drop(guards); - assert_eq!(sandbox_connection_count(&state, sandbox.object_id()), 0); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), + 0 + ); + } + + #[tokio::test] + async fn ssh_and_service_budgets_are_independent() { + let mut state = test_server_state().await; + { + let config = &mut Arc::get_mut(&mut state).expect("sole state owner").config; + config.max_ssh_connections_per_sandbox = 1; + config.max_service_connections_per_sandbox = 2; + } + let sandbox = test_sandbox("fwd", Vec::new()); + store_ssh_session(&state, "canary-ssh-token-a", "sandbox-fwd", false, 0).await; + store_ssh_session(&state, "canary-ssh-token-b", "sandbox-fwd", false, 0).await; + let tcp = forward_init(loopback_tcp_target(), ""); + let ssh = |token: &str| { + forward_init( + tcp_forward_init::Target::Ssh(SshRelayTarget::default()), + token, + ) + }; + let acquire = |init: TcpForwardInit| { + let state = state.clone(); + let sandbox = sandbox.clone(); + async move { + acquire_forward_connection_guard(&state, &forward_caller(), &init, &sandbox).await + } + }; + + let _forwards = [ + acquire(tcp.clone()).await.expect("first forward"), + acquire(tcp.clone()).await.expect("second forward"), + ]; + let err = acquire(tcp).await.err().expect("forward budget spent"); + assert_eq!( + err.message(), + "sandbox service forward connection limit reached" + ); + + let _ssh = acquire(ssh("canary-ssh-token-a")) + .await + .expect("open forwards leave the SSH budget alone"); + let err = acquire(ssh("canary-ssh-token-b")) + .await + .err() + .expect("SSH budget spent"); + assert_eq!(err.code(), tonic::Code::ResourceExhausted); + assert_eq!(err.message(), "sandbox SSH connection limit reached"); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Ssh, sandbox.object_id()), + 1 + ); + assert_eq!( + sandbox_connection_count(&state, RelayBudget::Service, sandbox.object_id()), + 2 + ); } #[test] diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 06bbf42736..235f2c5f93 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -340,6 +340,9 @@ pub struct ServerState { /// Active SSH tunnel connection counts per sandbox id. pub ssh_connections_by_sandbox: Mutex>, + /// Active service forward connection counts per sandbox id. + pub service_connections_by_sandbox: Mutex>, + /// Registry of active supervisor sessions and pending relay channels. /// /// Stored as `Arc` so compiled compute drivers can be constructed before @@ -508,6 +511,7 @@ impl ServerState { telemetry: telemetry::TelemetryState::new(), ssh_connections_by_token: Mutex::new(HashMap::new()), ssh_connections_by_sandbox: Mutex::new(HashMap::new()), + service_connections_by_sandbox: Mutex::new(HashMap::new()), supervisor_sessions, gateway_shutting_down: AtomicBool::new(false), config_delivery, diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index ef49f8ec86..4b60287c44 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -272,6 +272,11 @@ credential_drivers = ["kubernetes-secrets"] ssh_session_ttl_secs = 3600 +# Per-sandbox caps on concurrent relays. SSH sessions and service forwards +# have separate budgets. Each defaults to 20 and must be at least 1. +max_ssh_connections_per_sandbox = 20 +max_service_connections_per_sandbox = 20 + # Absolute image preparation and initial supervisor startup budget. # Applies to new attempts only; allowed values are 1 through 86400 seconds. image_preparation_timeout_seconds = 1800 diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 10a17c0cea..9cc2325d95 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -273,12 +273,12 @@ Gateway transport uses TLS, with client certificate checks available where the d SSH connections to sandboxes travel through the gateway over the sandbox supervisor's authenticated control path. Each SSH connect call also carries a short-lived session token scoped to a specific sandbox. The sandbox never exposes an SSH port on the network. Its SSH daemon listens on a local Unix socket that only the sandbox's own supervisor process can reach. -Service forwards (`openshell forward`) use the same relay path but carry no session token. The gateway authorizes each forwarded connection against the caller's gateway identity and workspace membership, and the per-sandbox connection limit applies. +Service forwards (`openshell forward`) use the same relay path but carry no session token. The gateway authorizes each forwarded connection against the caller's gateway identity and workspace membership, and the per-sandbox service forward limit applies. | Aspect | Detail | |---|---| -| Default | Session tokens expire after 24 hours. Concurrent connections are limited to 3 per token and 20 per sandbox. | -| What you can change | Configure `ssh_session_ttl_secs`. Set to 0 for no expiry. | +| Default | Session tokens expire after 24 hours. Concurrent connections are limited to 3 per token, 20 SSH relays per sandbox, and 20 service forwards per sandbox. | +| What you can change | Configure `ssh_session_ttl_secs` (set to 0 for no expiry), `max_ssh_connections_per_sandbox`, and `max_service_connections_per_sandbox`. | | Risk if relaxed | Longer TTLs or no expiry increase the window for stolen token reuse. Higher connection limits increase the blast radius of a compromised token. | | Recommendation | Keep the 24-hour default. Monitor connection counts through the TUI. |