From 7b0f8ae07c1938f829d7e6cfb67f49d29452c43e Mon Sep 17 00:00:00 2001 From: Taylor Mutch Date: Tue, 27 Oct 2026 02:33:15 +0000 Subject: [PATCH] fix(gateway): require authentication or explicit development opt-in (#4821) * fix(gateway): require authentication or explicit development opt-in Signed-off-by: Taylor Mutch * fix(gateway): reject implicit unauthenticated admin requests Signed-off-by: Taylor Mutch --------- Signed-off-by: Taylor Mutch --- crates/openshell-server/src/cli.rs | 116 ++++++++++++++++-- .../src/grpc/provider_credentials_tests.rs | 16 ++- crates/openshell-server/src/multiplex.rs | 89 ++++++++++++-- docs/how-it-works/gateways/configuration.mdx | 11 ++ skills/debug-openshell-cluster/SKILL.md | 1 + 5 files changed, 205 insertions(+), 28 deletions(-) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 5b2ce01c1..b6afb10ec 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -659,12 +659,34 @@ fn prepare_server_config_with_drivers( }) } +/// Reject an implicit development gateway before starting any listeners or services. +fn validate_user_auth(config: &openshell_core::Config) -> Result<()> { + if !config.auth.allow_unauthenticated_users + && !config.mtls_auth.enabled + && !config.mtls_auth.operator_enabled + && config.oidc.is_none() + && config.gateway_jwt.is_none() + { + return Err(miette::miette!( + "gateway has no user authentication configured\n\n\ + No OIDC issuer, mTLS user auth, operator auth, or gateway_jwt is set. \ + Authenticated RPCs require credentials unless unauthenticated users are explicitly allowed.\n\n\ + Configure [openshell.gateway.oidc], [openshell.gateway.mtls_auth], or \ + [openshell.gateway.gateway_jwt], or explicitly opt in for trusted local development:\n\n\ + [openshell.gateway.auth]\n\ + allow_unauthenticated_users = true" + )); + } + Ok(()) +} + async fn run_from_args( mut args: RunArgs, matches: ArgMatches, compute_drivers: ComputeDriverRegistry, ) -> Result<()> { let prepared = prepare_server_config_with_drivers(&mut args, &matches, &compute_drivers)?; + validate_user_auth(&prepared.config)?; // Initialize OCSF identity before tracing can emit gateway events. let gateway_identity = crate::gateway_ocsf::GatewayIdentity { @@ -758,18 +780,6 @@ async fn run_from_args( ); } - if !prepared.config.auth.allow_unauthenticated_users - && !prepared.config.mtls_auth.enabled - && !prepared.config.mtls_auth.operator_enabled - && !has_oidc - && prepared.config.gateway_jwt.is_none() - { - warn!( - "Neither mTLS user auth nor OIDC nor sandbox JWT auth is configured — \ - the gateway has no authentication mechanism" - ); - } - info!(bind = %prepared.config.bind_address, "Starting OpenShell server"); let result = Box::pin(run_server(prepared, tracing_log_bus, compute_drivers)).await; @@ -1420,6 +1430,88 @@ mod tests { use std::net::{IpAddr, Ipv4Addr}; use std::sync::atomic::{AtomicUsize, Ordering}; + #[test] + fn user_auth_requires_a_mechanism_or_explicit_opt_in() { + // Cover each mechanism on its own and in combination, with and without + // the development opt-in. Only the entirely unconfigured case fails. + for mechanisms in 0..32 { + let mut config = openshell_core::Config::new(None); + config.auth.allow_unauthenticated_users = mechanisms & 1 != 0; + config.mtls_auth.enabled = mechanisms & 2 != 0; + config.mtls_auth.operator_enabled = mechanisms & 4 != 0; + if mechanisms & 8 != 0 { + config.oidc = Some( + toml::from_str("issuer = 'https://idp.example.com'\naudience = 'openshell'") + .unwrap(), + ); + } + if mechanisms & 16 != 0 { + config.gateway_jwt = Some(toml::from_str( + "signing_key_path = 'key.pem'\npublic_key_path = 'public.pem'\nkid_path = 'kid'" + ).unwrap()); + } + assert_eq!( + super::validate_user_auth(&config).is_ok(), + mechanisms != 0, + "authentication combination {mechanisms}" + ); + } + } + + #[test] + fn startup_rejects_missing_user_auth_before_starting_services() { + for auth in [ + "", + "[openshell.gateway.auth]\nallow_unauthenticated_users = false\n", + ] { + let home = tempfile::tempdir().unwrap(); + let config_path = home.path().join("gateway.toml"); + std::fs::write(&config_path, format!("[openshell]\nversion = 2\n{auth}")).unwrap(); + Environment::new() + .set("XDG_CONFIG_HOME", home.path().to_str().unwrap()) + .set("XDG_STATE_HOME", home.path().to_str().unwrap()) + .set("OPENSHELL_LOCAL_TLS_DIR", home.path().join("tls")) + .remove("OPENSHELL_GATEWAY_CONFIG") + .remove("OPENSHELL_DRIVERS") + .run(|| { + let (args, matches) = parse_with_args(&[ + "openshell-gateway", + "--config", + config_path.to_str().unwrap(), + "--disable-tls", + "--db-url", + "sqlite::memory:", + "--compute-driver", + "test", + ]); + // The factory panics if startup reaches driver construction. + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let error = runtime + .block_on(super::run_from_args( + args, + matches, + test_registry("test", true), + )) + .unwrap_err() + .to_string(); + for expected in [ + "gateway has no user authentication configured", + "OIDC", + "mTLS", + "operator auth", + "gateway_jwt", + "[openshell.gateway.auth]", + "allow_unauthenticated_users = true", + ] { + assert!(error.contains(expected), "missing {expected:?} in {error}"); + } + }); + } + } + static REGISTRY_DETECTION_CALLS: AtomicUsize = AtomicUsize::new(0); static REJECTING_VALIDATION_CALLS: AtomicUsize = AtomicUsize::new(0); diff --git a/crates/openshell-server/src/grpc/provider_credentials_tests.rs b/crates/openshell-server/src/grpc/provider_credentials_tests.rs index c602e2846..66a4578b6 100644 --- a/crates/openshell-server/src/grpc/provider_credentials_tests.rs +++ b/crates/openshell-server/src/grpc/provider_credentials_tests.rs @@ -722,7 +722,8 @@ async fn real_tls_operator_boundary_and_admin_capabilities() { MultiplexService, extract_peer_certificate_sha256, extract_peer_identity, }; use openshell_core::proto::{ - GetCurrentUserRequest, ListWorkspacesRequest, open_shell_client::OpenShellClient, + GetCurrentUserRequest, ListSandboxesRequest, ListWorkspacesRequest, + open_shell_client::OpenShellClient, }; use tonic::transport::{ClientTlsConfig, Endpoint}; @@ -730,6 +731,7 @@ async fn real_tls_operator_boundary_and_admin_capabilities() { let config = &mut Arc::get_mut(&mut state).unwrap().config; config.mtls_auth.operator_enabled = true; config.mtls_auth.enabled = false; + config.auth.allow_unauthenticated_users = false; let dir = tempfile::tempdir().unwrap(); let (ca, ca_key) = crate::tls_test_utils::generate_test_certs_with_ca(dir.path()); let acceptor = crate::TlsAcceptor::from_files( @@ -800,10 +802,17 @@ async fn real_tls_operator_boundary_and_admin_capabilities() { .await .unwrap(); let mut client = OpenShellClient::new(channel); + let sandboxes = client + .list_sandboxes(ListSandboxesRequest { + workspace_scope: Some(workspace_selector("default")), + ..Default::default() + }) + .await; let response = client .get_provider_credentials(request(&["STATIC_KEY"])) .await; if role == Some("operator") { + sandboxes.unwrap(); assert_eq!( response.unwrap().into_inner().credentials["STATIC_KEY"].value, "static-secret" @@ -846,7 +855,8 @@ async fn real_tls_operator_boundary_and_admin_capabilities() { Code::Unauthenticated ); } else { - assert_eq!(response.unwrap_err().code(), Code::PermissionDenied); + assert_eq!(sandboxes.unwrap_err().code(), Code::Unauthenticated); + assert_eq!(response.unwrap_err().code(), Code::Unauthenticated); let mut spoof = Request::new(request(&["STATIC_KEY"])); spoof .metadata_mut() @@ -857,7 +867,7 @@ async fn real_tls_operator_boundary_and_admin_capabilities() { .await .unwrap_err() .code(), - Code::PermissionDenied + Code::Unauthenticated ); } } diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index baf9b53ef..491384cc4 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -1062,9 +1062,8 @@ where /// local developer user when `auth.allow_unauthenticated_users` is explicitly /// enabled. /// -/// When neither OIDC nor sandbox credentials are configured (a barebones -/// dev gateway), the chain is left as `None` so the router short-circuits -/// to pass-through unless mTLS or local unauthenticated users are enabled. +/// When no bearer authenticators are configured, the chain is left as `None`. +/// The router still requires an mTLS principal or the explicit development opt-in. fn build_authenticator_chain(state: &ServerState) -> Option { let mut authenticators: Vec> = Vec::new(); if let Some(peer) = state.peer_authenticator.clone() { @@ -1100,8 +1099,8 @@ fn build_authenticator_chain(state: &ServerState) -> Option /// - Strip any external `x-openshell-auth-source` marker first (so callers /// cannot spoof a sandbox identity). /// - Health probes / reflection bypass the chain entirely. -/// - When no chain is configured (OIDC not configured), forward without -/// authentication — preserves today's pass-through behavior. +/// - When no chain is configured, require an mTLS principal or the explicit +/// development opt-in. /// - Otherwise, run the chain. The first match produces a `Principal`. /// `Principal::User` is gated by the RBAC `AuthzPolicy`. /// `Principal::Sandbox` is gated by a supervisor-method allowlist, then @@ -1297,10 +1296,15 @@ where } else if allow_unauthenticated_users { unauthenticated_dev_user_principal() } else { - // No auth configured — dev / fronting-proxy deployments. - // Inject a local-dev principal so downstream handlers that - // call extract_principal() always find one. - unauthenticated_dev_user_principal() + audit::emit_authentication_failure( + &path, + &origin(req.headers()), + None, + AuthFailureReason::MissingCredential, + ); + return Ok(status_response(tonic::Status::unauthenticated( + "authentication required", + ))); }; let origin = origin(req.headers()); audit::emit_logon( @@ -3520,6 +3524,51 @@ mod tests { } } + #[tokio::test] + async fn operator_only_requires_operator_certificate() { + for (peer_identity, operator) in [ + (None, None), + (Some(mtls_identity("ordinary-user")), None), + ( + Some(operator_principal().identity), + Some(operator_principal()), + ), + ] { + let capture = OcsfCapture::default(); + let _guard = capture.install(); + let authenticated = operator.is_some(); + let (recorder, seen) = PrincipalRecorder::new(); + // Operator-only authentication has no bearer chain and disables + // both user mTLS authentication and the development opt-in. + let mut router = AuthGrpcRouter::with_peer_identity( + recorder, + None, + None, + peer_identity, + false, + false, + ) + .with_operator(operator); + let path = "/openshell.v1.OpenShell/ListSandboxes"; + + let response = router.call(empty_request(path)).await.unwrap(); + + if authenticated { + assert!(grpc_status(&response).is_none()); + assert!(matches!( + *seen.lock().unwrap(), + Some(Principal::Operator(_)) + )); + } else { + assert_eq!(grpc_status(&response).as_deref(), Some("16")); + assert!(seen.lock().unwrap().is_none()); + let events = capture.json_events(); + assert_eq!(events.len(), 1, "{events:?}"); + assert_router_authentication_failure(&events[0], path, "missing_credential", 2); + } + } + } + #[tokio::test] async fn operator_is_admin_but_not_supervisor_peer_or_unknown() { for (method, permitted) in [ @@ -3659,12 +3708,25 @@ mod tests { .unwrap(); assert_eq!(grpc_status(&response).as_deref(), Some("7")); } - let (recorder, _) = PrincipalRecorder::new(); - let response = AuthGrpcRouter::new(recorder, None, None) + // Without a bearer chain, the development opt-in still grants no + // access to the assignment stream. Otherwise authentication fails + // before method authorization. + for (allow_unauthenticated_users, expected_status) in [(true, "7"), (false, "16")] { + let (recorder, seen) = PrincipalRecorder::new(); + let response = AuthGrpcRouter::with_peer_identity( + recorder, + None, + None, + None, + false, + allow_unauthenticated_users, + ) .call(empty_request(attach)) .await .unwrap(); - assert_eq!(grpc_status(&response).as_deref(), Some("7")); + assert_eq!(grpc_status(&response).as_deref(), Some(expected_status)); + assert!(seen.lock().unwrap().is_none()); + } } #[tokio::test] @@ -3695,7 +3757,8 @@ mod tests { assert_eq!(grpc_status(&response).as_deref(), Some("7")); } let (recorder, _) = PrincipalRecorder::new(); - let mut router = AuthGrpcRouter::new(recorder, None, None); + let mut router = + AuthGrpcRouter::with_peer_identity(recorder, None, None, None, false, true); let response = router .call(empty_request( "/openshell.v1.OpenShell/GetProviderCredentials", diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 23d92098d..05b46242d 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -325,6 +325,10 @@ certificate with an `Authorization` header. See [Authentication](/how-it-works/gateways/authentication#mtls-operators) for the certificate trust boundary. +With operator authentication enabled and user mTLS disabled, callers without a +verified operator certificate receive `Unauthenticated` unless another configured +authentication mechanism accepts them or `allow_unauthenticated_users = true`. + The client-certificate handshake policy has no `require_client_auth` TOML field. Client certificates are optional at the transport layer; gateway RPC authorization enforces the configured user or sandbox identity. `[openshell.gateway.tls]` supports optional SNI-based dual-certificate mode for deployments that need separate internal and external server certificates. Set `external_cert_path` and `external_key_path` to point at the external (e.g. ACME/publicly-trusted) certificate and key. List the hostnames that should be served with the external certificate in `external_server_names`. Connections whose TLS SNI hostname matches one of those names receive the external certificate; all other connections (including those with no SNI) receive the primary internal certificate from `cert_path`/`key_path`. Both fields must be set together — providing only one is a configuration error. On Kubernetes with the Helm chart, the external certificate is managed automatically when `certManager.serverIssuerRef.name` is set; the chart populates these fields from the cert-manager-issued external server certificate. @@ -339,6 +343,13 @@ The gateway rejects partial or invalid signing bundles at startup, including pri External compute drivers declare this requirement by adding `openshell.compute.launch-authentication` to their extension metadata's `required_capabilities`. This requires a gateway that understands the launch-signing preflight and supplies `SandboxLaunchAuthentication` on creation. Drivers that use another launch mechanism can omit the requirement. The separate `supports_sandbox_authentication` capability describes the `AuthenticateSandbox` RPC and does not require launch signing. +The gateway refuses to start before binding any ports when no OIDC issuer, mTLS user authentication, operator authentication, or `gateway_jwt` is configured and `allow_unauthenticated_users` is unset or false. Configure an authentication mechanism, or explicitly opt in for trusted local development: + +```toml +[openshell.gateway.auth] +allow_unauthenticated_users = true +``` + `[openshell.gateway.auth] allow_unauthenticated_users = true` is an unsafe local-development and trusted-proxy escape hatch. It accepts user-facing CLI/API calls without OIDC or mTLS credentials while sandbox supervisors still authenticate with gateway-minted sandbox JWTs. Leave it false for shared and production gateways. ## Database Pool diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index e74e1b1c4..6d000a5b8 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -70,6 +70,7 @@ openshell --gateway-endpoint status Common findings: - `No active gateway`: register one with `openshell gateway add `. +- `gateway has no user authentication configured`: configure OIDC, mTLS user or operator authentication, or gateway JWTs using the published gateway configuration reference. For trusted local development, explicitly set `[openshell.gateway.auth] allow_unauthenticated_users = true`; the gateway otherwise refuses to bind any ports. - Connection refused: gateway process is not running, service exposure is wrong, or a port-forward/proxy is not active. - TLS/certificate errors: the endpoint scheme or trust chain is wrong, a CLI mTLS bundle does not match the gateway CA, a supervisor is missing the gateway CA, or TLS termination does not match the gateway listener. Workloads and supervisors should not contain a user TLS client certificate or private key. - Fresh snap installations select `gateway-mode=user`, but the user gateway is globally disabled by default. `install.sh` enables `snap.openshell.user-gateway.service` only for its target user; direct snap installations must select one user and run `systemctl --user enable --now snap.openshell.user-gateway.service` from that user's session. Do not enable it for multiple concurrently logged-in users: every instance binds to `127.0.0.1:17670`, so later instances fail with an address-in-use error. Older snap revisions could leave the unit globally enabled; remove that state with `sudo systemctl disable --global snap.openshell.user-gateway.service`, then disable and stop the service from every other affected user's session. The global command does not stop running user services. Setting `gateway-mode=user` disables the system gateway without changing any user's service. To change the selected user, disable and stop the current user's unit before enabling the new user's unit. Upgraded installations retain their system-owned state through `openshell.system-gateway`. Existing HTTPS registrations continue to work after the service rename. Follow the published snap installation steps to enroll a new user, replace an old HTTP registration, or change modes safely through the `disable` maintenance state. If `gateway-mode=disable`, no local snap gateway is expected to run.