Skip to content

Commit ea90ab9

Browse files
committed
fix(auth): skip renewal for non-expiring sandbox JWTs
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
1 parent a00ea31 commit ea90ab9

3 files changed

Lines changed: 22 additions & 14 deletions

File tree

‎architecture/gateway.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,8 @@ successor removes that retry path across every gateway replica. Short
361361
that has not yet been refreshed. Omitting `gateway_jwt.ttl_secs` selects
362362
non-expiring launch-scoped gateway and Sandbox Protocol tokens for local
363363
single-player Docker, Podman, and VM gateways; both token profiles carry
364-
`exp = 0`. Typed extension JWTs retain a 900-second default when the field is
364+
`exp = 0`, and supervisors skip periodic renewal of those session tokens.
365+
Typed extension JWTs retain a 900-second default when the field is
365366
omitted. Kubernetes and other shared deployments should set a positive TTL.
366367
Explicit zero is rejected.
367368

‎crates/openshell-core/src/grpc_client.rs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,10 @@ async fn refresh_token_loop(
447447
) {
448448
let mut client = OpenShellClient::new(channel);
449449
loop {
450-
let sleep = compute_refresh_delay(&slot);
450+
let Some(sleep) = compute_refresh_delay(&slot) else {
451+
debug!("gateway sandbox JWT does not expire; stopping periodic renewal");
452+
return;
453+
};
451454
tokio::time::sleep(sleep).await;
452455
match client
453456
.refresh_sandbox_token(RefreshSandboxTokenRequest {
@@ -653,9 +656,10 @@ async fn refresh_extension_credentials_with_client(
653656

654657
/// Compute the next refresh delay: 80 % of the time remaining until the
655658
/// current token's `exp`, plus up to 10 % jitter, with a small lower bound
656-
/// for already-expired tokens and capped at 12 h. If the token can't be parsed
657-
/// (for example, an opaque bootstrap bearer), default to 6 h.
658-
fn compute_refresh_delay(slot: &TokenSlot) -> Duration {
659+
/// for already-expired tokens and capped at 12 h. An `exp` of zero denotes a
660+
/// non-expiring session credential and needs no periodic refresh. If the token
661+
/// can't be parsed (for example, an opaque bootstrap bearer), default to 6 h.
662+
fn compute_refresh_delay(slot: &TokenSlot) -> Option<Duration> {
659663
let token = slot
660664
.read()
661665
.ok()
@@ -668,7 +672,11 @@ fn compute_refresh_delay(slot: &TokenSlot) -> Duration {
668672
.map_or(0, |d| d.as_millis()),
669673
)
670674
.unwrap_or(i64::MAX);
671-
let mut delay_ms = parse_jwt_exp_ms(bearer).map_or(21_600_000, |exp| {
675+
let expires_at = parse_jwt_exp_ms(bearer);
676+
if expires_at == Some(0) {
677+
return None;
678+
}
679+
let mut delay_ms = expires_at.map_or(21_600_000, |exp| {
672680
let remaining_ms = exp - now_ms;
673681
if remaining_ms <= 0 {
674682
1_000
@@ -681,7 +689,7 @@ fn compute_refresh_delay(slot: &TokenSlot) -> Duration {
681689
let jitter_pct = (token.len() % 10) as u64;
682690
let jitter_ms = (u64::try_from(delay_ms).unwrap_or(0) * jitter_pct) / 100;
683691
delay_ms = delay_ms.saturating_add(i64::try_from(jitter_ms).unwrap_or(0));
684-
Duration::from_millis(u64::try_from(delay_ms).unwrap_or(0))
692+
Some(Duration::from_millis(u64::try_from(delay_ms).unwrap_or(0)))
685693
}
686694

687695
/// Decode the `exp` claim from a JWT without verifying its signature.
@@ -794,7 +802,7 @@ mod auth_tests {
794802
let token = format!("h.{payload}.s");
795803
let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap();
796804
let slot: TokenSlot = Arc::new(RwLock::new(bearer));
797-
let delay = compute_refresh_delay(&slot);
805+
let delay = compute_refresh_delay(&slot).expect("expiring token needs refresh");
798806
// 800 s baseline + up to 10 % jitter → 800..=880 s, with some slack
799807
// for the 1-second resolution of the exp claim.
800808
let secs = delay.as_secs();
@@ -815,19 +823,18 @@ mod auth_tests {
815823
let token = format!("h.{payload}.s");
816824
let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap();
817825
let slot: TokenSlot = Arc::new(RwLock::new(bearer));
818-
let delay = compute_refresh_delay(&slot);
826+
let delay = compute_refresh_delay(&slot).expect("expired token needs refresh");
819827
assert!((1..60).contains(&delay.as_secs()));
820828
}
821829

822830
#[test]
823-
fn compute_refresh_delay_treats_exp_zero_as_expired() {
831+
fn compute_refresh_delay_skips_non_expiring_token() {
824832
use base64::Engine as _;
825833
let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(r#"{"exp":0}"#);
826834
let token = format!("h.{payload}.s");
827835
let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap();
828836
let slot: TokenSlot = Arc::new(RwLock::new(bearer));
829-
let delay = compute_refresh_delay(&slot);
830-
assert!((1..60).contains(&delay.as_secs()));
837+
assert_eq!(compute_refresh_delay(&slot), None);
831838
}
832839

833840
#[test]
@@ -843,7 +850,7 @@ mod auth_tests {
843850
let token = format!("h.{payload}.s");
844851
let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap();
845852
let slot: TokenSlot = Arc::new(RwLock::new(bearer));
846-
let delay = compute_refresh_delay(&slot);
853+
let delay = compute_refresh_delay(&slot).expect("expiring token needs refresh");
847854
assert!(
848855
delay.as_secs() < 30,
849856
"expected refresh before 30s expiry, got {delay:?}",

‎docs/reference/gateway-config.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ The client-certificate handshake policy is derived and has no `require_client_au
270270

271271
`[openshell.gateway] policy_validation_failure_mode` controls what sandbox supervisors do when a complete candidate policy fails runtime validation. The default, `fail_closed`, deactivates the previous network policy, closes relays pinned to it, and denies new egress until a valid generation loads. `retain_last_valid` leaves the previous valid generation active. Both modes reject the candidate atomically; startup keeps the workload unstarted until the effective policy and matching provider configuration pass admission. A rejected startup exposes `ConfigurationInvalid` and remains available for policy/provider repair in either mode. Gateway mutation paths that can preflight a known effective scope reject invalid candidates before persistence and leave the active policy unchanged regardless of this setting. Changing the value requires restarting the gateway so it can reload `gateway.toml` and distribute the new posture to sandbox supervisors.
272272

273-
`[openshell.gateway.gateway_jwt] ttl_secs` controls generation-bound gateway-facing and Sandbox Protocol credentials minted for a sandbox session, plus typed extension JWTs. Omit it for non-expiring local sandbox session credentials: both session tokens carry `exp = 0`, and refresh responses omit their expiration timestamps. Typed extension JWTs retain a 900-second default when the field is omitted. Use omission only for local single-player Docker, Podman, or VM gateways. Explicit `0` is invalid. Kubernetes and other shared deployments should set a positive TTL; Helm renders `3600` seconds by default, and the gateway logs a warning when a Kubernetes gateway omits the field.
273+
`[openshell.gateway.gateway_jwt] ttl_secs` controls generation-bound gateway-facing and Sandbox Protocol credentials minted for a sandbox session, plus typed extension JWTs. Omit it for non-expiring local sandbox session credentials: both session tokens carry `exp = 0`, supervisors skip periodic session-token renewal, and refresh responses omit their expiration timestamps. Typed extension JWTs retain a 900-second default when the field is omitted. Use omission only for local single-player Docker, Podman, or VM gateways. Explicit `0` is invalid. Kubernetes and other shared deployments should set a positive TTL; Helm renders `3600` seconds by default, and the gateway logs a warning when a Kubernetes gateway omits the field.
274274

275275
`[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.
276276

0 commit comments

Comments
 (0)