Skip to content

Commit 19b8737

Browse files
committed
fix(server): restrict sandbox principal RPC access
1 parent f81e26f commit 19b8737

13 files changed

Lines changed: 317 additions & 69 deletions

File tree

‎architecture/gateway.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ token through `IssueSandboxToken`. Supervisors renew gateway JWTs in memory
5353
before expiry. Older tokens are not server-revoked; deployments bound replay
5454
exposure with short `gateway_jwt.ttl_secs` lifetimes.
5555

56+
Sandbox JWTs are not user credentials. The gRPC router accepts
57+
`Principal::Sandbox` only on the supervisor-to-gateway RPC allowlist
58+
(`ConnectSupervisor`, `RelayStream`, token renewal, config sync, policy status,
59+
log push, and policy-analysis callbacks). Handlers then compare the
60+
authenticated sandbox ID with any sandbox ID or name resolved from the request.
61+
Supervisor control and relay streams require a matching sandbox principal before
62+
the gateway registers the session or bridges relay bytes.
63+
5664
## API Surface
5765

5866
The gateway API is organized around platform objects and operational streams:

‎crates/openshell-server/src/auth/authenticator.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
//! - [`super::oidc::OidcAuthenticator`] — user OIDC Bearer tokens
2020
//! - [`PermissiveUserAuthenticator`] — final-fallback dev-mode catch-all
2121
//! that produces a synthetic user principal when no OIDC is
22-
//! configured. Preserves the pre-PR-1 "no OIDC = open" posture for
22+
//! configured. Preserves the "no OIDC = open" dev posture for
2323
//! singleplayer / helm-dev deployments.
2424
2525
use super::identity::{Identity, IdentityProvider};
@@ -99,9 +99,9 @@ impl std::fmt::Debug for AuthenticatorChain {
9999
/// Final-fallback authenticator that produces a synthetic user principal
100100
/// for any request the earlier authenticators didn't claim. Used only
101101
/// when no user-side authentication is configured (no OIDC, no fronting
102-
/// proxy contract) — the pre-PR-1 gateway accepted such requests with
103-
/// no auth at all; this preserves that posture in a principal-aware
104-
/// way so handlers always see *some* principal in extensions.
102+
/// proxy contract). This preserves the dev-mode open posture in a
103+
/// principal-aware way so handlers always see *some* principal in
104+
/// extensions.
105105
///
106106
/// Producing a User principal (rather than Anonymous) means dev-mode
107107
/// requests pass the per-handler IDOR guard via the User-bypass

‎crates/openshell-server/src/auth/guard.rs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//! already evaluated.
1010
1111
use super::principal::Principal;
12+
use super::principal::SandboxPrincipal;
1213
use tonic::Status;
1314
use tracing::info;
1415

@@ -66,6 +67,32 @@ pub fn enforce_sandbox_scope<T>(
6667
Ok(principal)
6768
}
6869

70+
/// Require a sandbox principal and reject users or anonymous callers.
71+
///
72+
/// Supervisor-only control/data plane RPCs (`ConnectSupervisor`,
73+
/// `RelayStream`) must be presented by the sandbox supervisor itself.
74+
/// User principals intentionally pass [`ensure_sandbox_scope`] for normal
75+
/// CLI/TUI APIs because RBAC is their gate, but they are not valid
76+
/// supervisor identities.
77+
#[allow(clippy::result_large_err)]
78+
pub fn ensure_sandbox_principal_scope(
79+
principal: &Principal,
80+
claimed_sandbox_id: &str,
81+
) -> Result<SandboxPrincipal, Status> {
82+
match principal {
83+
Principal::Sandbox(p) => {
84+
ensure_sandbox_scope(principal, claimed_sandbox_id)?;
85+
Ok(p.clone())
86+
}
87+
Principal::User(_) => Err(Status::permission_denied(
88+
"supervisor RPCs require a sandbox principal",
89+
)),
90+
Principal::Anonymous => Err(Status::unauthenticated(
91+
"supervisor RPCs require an authenticated sandbox principal",
92+
)),
93+
}
94+
}
95+
6996
#[cfg(test)]
7097
mod tests {
7198
use super::*;
@@ -119,6 +146,20 @@ mod tests {
119146
assert_eq!(err.code(), tonic::Code::Unauthenticated);
120147
}
121148

149+
#[test]
150+
fn sandbox_principal_scope_returns_matching_sandbox() {
151+
let principal = sandbox("sbx-1");
152+
let scoped = ensure_sandbox_principal_scope(&principal, "sbx-1").expect("scope OK");
153+
assert_eq!(scoped.sandbox_id, "sbx-1");
154+
}
155+
156+
#[test]
157+
fn sandbox_principal_scope_rejects_users() {
158+
let err = ensure_sandbox_principal_scope(&user("alice"), "sbx-1")
159+
.expect_err("users are not supervisor identities");
160+
assert_eq!(err.code(), tonic::Code::PermissionDenied);
161+
}
162+
122163
#[test]
123164
fn enforce_reads_from_request_extensions() {
124165
let mut req = tonic::Request::new(());

‎crates/openshell-server/src/auth/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ pub mod k8s_sa;
1717
pub mod oidc;
1818
pub mod principal;
1919
pub mod sandbox_jwt;
20+
pub mod sandbox_methods;
2021

2122
pub use http::router;

‎crates/openshell-server/src/auth/oidc.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ impl JwksCache {
356356
///
357357
/// Returns `Ok(None)` when no Bearer header is present, so the chain can fall
358358
/// through to other authenticators (e.g. the gateway-minted sandbox JWT
359-
/// authenticator added in PR 2).
359+
/// authenticator).
360360
pub struct OidcAuthenticator {
361361
cache: Arc<JwksCache>,
362362
}

‎crates/openshell-server/src/auth/principal.rs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub enum Principal {
2626
User(UserPrincipal),
2727
/// Sandbox supervisor authenticated by an identity bound to a specific
2828
/// sandbox UUID. The wrapped `sandbox_id` MUST match any sandbox referenced
29-
/// in the request body for sandbox-class methods (PR-4 guard).
29+
/// in the request body for sandbox-class methods.
3030
Sandbox(#[allow(dead_code)] SandboxPrincipal),
3131
/// Truly unauthenticated caller (health probes, reflection). Sandbox-class
3232
/// and user-class methods reject this variant.
@@ -43,17 +43,14 @@ pub struct UserPrincipal {
4343

4444
/// Sandbox caller — bound to one specific sandbox UUID.
4545
///
46-
/// `sandbox_id` and `source` are consumed by the PR-4 handler guard; until
47-
/// then they only exist in the type so the trait shape is stable across the
48-
/// PR series.
46+
/// `sandbox_id` and `source` are consumed by the router and handler guards.
4947
#[derive(Debug, Clone)]
5048
#[allow(dead_code)]
5149
pub struct SandboxPrincipal {
52-
/// Canonical sandbox UUID. Empty string only for the PR-1 legacy marker;
53-
/// PR 2 onwards always populates this from a verified credential.
50+
/// Canonical sandbox UUID populated from a verified sandbox credential.
5451
pub sandbox_id: String,
55-
/// How this principal was verified — used for audit logs and to gate the
56-
/// PR-4 IDOR check against unverified sources.
52+
/// How this principal was verified — used for audit logs and method-specific
53+
/// authorization checks.
5754
pub source: SandboxIdentitySource,
5855
/// SPIFFE trust domain. Populated when the credential is SPIFFE-shaped;
5956
/// reserved for future per-sandbox cert / SPIRE authenticators.
@@ -63,18 +60,17 @@ pub struct SandboxPrincipal {
6360
/// How a [`SandboxPrincipal`] was authenticated.
6461
///
6562
/// Variant fields are populated by the producing authenticator and consumed
66-
/// by audit logging + the PR-4 IDOR guard. Until PR 4 lands those readers
67-
/// they look unused to the dead-code lint.
63+
/// by audit logging and method-specific authorization checks.
6864
#[derive(Debug, Clone)]
6965
#[allow(dead_code)]
7066
pub enum SandboxIdentitySource {
7167
/// Gateway-minted JWT validated against the gateway's signing key.
7268
/// Produced by [`super::sandbox_jwt::SandboxJwtAuthenticator`].
7369
BootstrapJwt { issuer: String },
74-
/// Per-sandbox client certificate. Reserved for the v2 channel-bound
75-
/// identity follow-up.
70+
/// Per-sandbox client certificate. Reserved for channel-bound sandbox
71+
/// identity.
7672
BootstrapCert { fingerprint: String },
77-
/// SPIRE-issued SVID. Reserved for the SPIFFE/SPIRE follow-up.
73+
/// SPIRE-issued SVID. Reserved for SPIFFE/SPIRE sandbox identity.
7874
SpiffeSvid { spiffe_id: String },
7975
/// K8s `ServiceAccount` token used to bootstrap a gateway-minted JWT
8076
/// via `IssueSandboxToken`. Populated only on that one RPC path.

‎crates/openshell-server/src/auth/sandbox_jwt.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//!
66
//! The gateway signs an Ed25519 JWT for each sandbox at create time and
77
//! the sandbox supervisor presents it as `Authorization: Bearer <jwt>` on
8-
//! every gRPC call (PR 3). This module implements both sides of the
8+
//! supervisor-to-gateway gRPC calls. This module implements both sides of the
99
//! gateway-controlled token:
1010
//! - [`SandboxJwtIssuer`] mints fresh tokens (called from
1111
//! `handle_create_sandbox` and the `IssueSandboxToken` RPC).
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//! Method-level allowlist for sandbox principals.
5+
//!
6+
//! Gateway-minted sandbox JWTs identify a single sandbox supervisor. They
7+
//! must not authorize user-facing or admin APIs. The router rejects sandbox
8+
//! principals for every method outside this supervisor-to-gateway allowlist;
9+
//! handlers still perform same-sandbox checks on request bodies.
10+
11+
/// Methods a `Principal::Sandbox` may invoke.
12+
const ALLOWED_SANDBOX_METHODS: &[&str] = &[
13+
"/openshell.v1.OpenShell/IssueSandboxToken",
14+
"/openshell.v1.OpenShell/RefreshSandboxToken",
15+
"/openshell.v1.OpenShell/ConnectSupervisor",
16+
"/openshell.v1.OpenShell/RelayStream",
17+
"/openshell.v1.OpenShell/GetSandboxConfig",
18+
"/openshell.v1.OpenShell/GetSandboxProviderEnvironment",
19+
"/openshell.v1.OpenShell/UpdateConfig",
20+
"/openshell.v1.OpenShell/ReportPolicyStatus",
21+
"/openshell.v1.OpenShell/PushSandboxLogs",
22+
"/openshell.v1.OpenShell/SubmitPolicyAnalysis",
23+
"/openshell.v1.OpenShell/GetDraftPolicy",
24+
];
25+
26+
pub fn is_sandbox_callable(path: &str) -> bool {
27+
ALLOWED_SANDBOX_METHODS.contains(&path)
28+
}
29+
30+
#[cfg(test)]
31+
mod tests {
32+
use super::*;
33+
34+
#[test]
35+
fn supervisor_callbacks_are_allowed() {
36+
assert!(is_sandbox_callable(
37+
"/openshell.v1.OpenShell/ConnectSupervisor"
38+
));
39+
assert!(is_sandbox_callable("/openshell.v1.OpenShell/RelayStream"));
40+
assert!(is_sandbox_callable(
41+
"/openshell.v1.OpenShell/GetSandboxConfig"
42+
));
43+
}
44+
45+
#[test]
46+
fn user_and_admin_methods_are_not_allowed() {
47+
assert!(!is_sandbox_callable(
48+
"/openshell.v1.OpenShell/ListSandboxes"
49+
));
50+
assert!(!is_sandbox_callable(
51+
"/openshell.v1.OpenShell/DeleteSandbox"
52+
));
53+
assert!(!is_sandbox_callable(
54+
"/openshell.v1.OpenShell/CreateProvider"
55+
));
56+
assert!(!is_sandbox_callable(
57+
"/openshell.v1.OpenShell/ApproveDraftChunk"
58+
));
59+
}
60+
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,10 @@ async fn run_from_args(mut args: RunArgs, matches: ArgMatches) -> Result<()> {
336336
});
337337
}
338338

339-
// PR-2 wires gateway_jwt via the config file only — there's no CLI
340-
// flag yet because the standard deployments (helm chart + RPM init
341-
// script) drop the keypair to a known path and pass that path through
342-
// the TOML. A CLI shortcut can be added if a singleplayer operator
343-
// needs to override.
339+
// `gateway_jwt` is configured through TOML. Standard deployments
340+
// (helm chart + RPM init script) drop the keypair to a known path and
341+
// pass that path through the file. A CLI shortcut can be added if a
342+
// singleplayer operator needs to override it.
344343
if let Some(jwt) = file
345344
.as_ref()
346345
.and_then(|f| f.openshell.gateway.gateway_jwt.clone())

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2901,9 +2901,9 @@ mod tests {
29012901
use std::sync::Arc;
29022902
use tonic::Code;
29032903

2904-
/// Wrap a request with a user `Principal` so handlers' scope guards
2905-
/// (introduced in PR 4) treat the test caller as a CLI user — equivalent
2906-
/// to the pre-PR-4 behavior where all tests effectively ran as user.
2904+
/// Wrap a request with a user `Principal` so handler scope guards treat
2905+
/// the test caller as a CLI user. Most handler tests exercise
2906+
/// user-facing behavior and should not trip sandbox equality checks.
29072907
fn with_user<T>(mut request: Request<T>) -> Request<T> {
29082908
request
29092909
.extensions_mut()
@@ -3000,7 +3000,7 @@ mod tests {
30003000
assert!(!is_sandbox_caller(&req));
30013001
}
30023002

3003-
// ---- PR-4 IDOR guard (issue #1354) ----
3003+
// ---- Sandbox IDOR guard (issue #1354) ----
30043004

30053005
#[tokio::test]
30063006
async fn cross_sandbox_get_sandbox_config_denied() {

0 commit comments

Comments
 (0)