Skip to content

feat(provider): support for SPIFFE backed token exchange - #1970

Open
grs wants to merge 6 commits into
NVIDIA:mainfrom
grs:token-exchange
Open

feat(provider): support for SPIFFE backed token exchange#1970
grs wants to merge 6 commits into
NVIDIA:mainfrom
grs:token-exchange

Conversation

@grs

@grs grs commented Jun 22, 2026

Copy link
Copy Markdown

Summary

This PR adds the ability to specify token_exchange instead of a simple token_grant in a provider profile, with the goal of automatically obtaining a token that combines the agents identity with that of the user on whose behalf it is acting.

As with the existing token grants, the token exchange is initiated by the supervisor proxy. In the case of an exchange however it requires a subject token that has the supervisor clients id in the audience. To get this, a two stage exchange is required.

The supervisor first requests a token from the gateway. The gateway does the first exchange, using the identified credential in a provider as the subject and requesting the audience needed for the supervisor (which is based on the supervisors SPIFFE ID). The token from that exchange is then returned to the supervisor which uses it as the subject for a further exchange. The resulting token than has the subject of the original provider credential but the sandbox (as identified by its SPIFFE ID) as the authorized party.

An option was also added to the CLI for provider create|update, allowing the current OIDC token to be stored.

Related Issue

Fixes #1987

Changes

  • Added token_exchange as a provider token grant mode alongside client_credentials.
  • Extended provider profiles/protobuf with subject-token metadata, requested token type, and SPIFFE JWT-SVID settings.
  • Added gateway-side subject-token exchange support for sandbox-scoped dynamic credentials.
  • Added shared SPIFFE/JWT-SVID helpers and gateway SPIFFE Workload API Helm wiring.
  • Updated CLI option to store current OIDC token.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)
  • Manual tests of token exchange, including with updated token, as well as of the original simple token grant.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@grs
grs force-pushed the token-exchange branch from 7a603a0 to b2adf35 Compare June 23, 2026 20:25
@grs grs changed the title feat(provider): support for token exchange feat(provider): support for SPIFFE backed token exchange Jun 23, 2026
@grs
grs marked this pull request as ready for review June 23, 2026 20:27
@grs
grs requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners June 23, 2026 20:27
@grs
grs force-pushed the token-exchange branch from b2adf35 to 3552f53 Compare June 24, 2026 06:35
@grs
grs force-pushed the token-exchange branch 3 times, most recently from 697b181 to 0378b29 Compare July 9, 2026 08:03
@grs
grs force-pushed the token-exchange branch from 0378b29 to 72c753f Compare July 13, 2026 14:39
@grs

grs commented Jul 14, 2026

Copy link
Copy Markdown
Author

@mrunalp @TaylorMutch just fyi, I have verified that this works with both currently supported supervisor topologies.

@afourniernv

Copy link
Copy Markdown

Context from the Okta side: I have a live-tested RFC 8693 OBO implementation in #1681. That path exchanges the logged-in user subject token at Okta using the Okta client ID/secret, audience, and scope.

Before I rework it around this PR: is #1970 intended to support that kind of Okta token endpoint/client-secret exchange, or does the new two-stage flow require the IdP to accept SPIFFE JWT-SVID client assertions?

If the latter, I will need to validate/configure that separately before wiring OBO into the NemoClaw reference.

@grs

grs commented Jul 22, 2026

Copy link
Copy Markdown
Author

@afourniernv At present it does indeed require the IdP to accept SPIFFE JWT-SVIDs. That could certainly be expanded, but I do think that the intention for this path (i.e. the current dynamic token grant path) is for it to use some 'platform provided' credential that is (or can be) scoped to the sandbox.

If the user is providing a client secret for the exchange through a provider, then in my view the approach in your PR seems like the simplest/cleanest. In that case the gateway can handle it in much the same way as a token refresh (as your PR shows).

@grs
grs force-pushed the token-exchange branch 3 times, most recently from 11f2a20 to 85ebd89 Compare July 24, 2026 21:18
@grs

grs commented Jul 24, 2026

Copy link
Copy Markdown
Author

/ok

@grs
grs force-pushed the token-exchange branch 2 times, most recently from e154f4d to 2123487 Compare July 29, 2026 16:24
///
/// When set, the gateway can fetch its own JWT-SVID for provider token exchange
/// client assertions.
pub const GATEWAY_SPIFFE_WORKLOAD_API_SOCKET: &str = "OPENSHELL_GATEWAY_SPIFFE_WORKLOAD_API_SOCKET";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this module's doc comment describes it as "the shared protocol between the compute drivers (which set the variables when launching a sandbox container/VM) and the sandbox supervisor process (which reads them on startup)."

GATEWAY_SPIFFE_WORKLOAD_API_SOCKET doesn't fit that contract. No compute driver sets it, and no supervisor reads it. It's set by the Helm template on the gateway pod and read by the gateway's gRPC handler (fetch_gateway_jwt_svid). Placing it here blurs the module's scope and could confuse someone auditing which env vars drivers must provide.

Consider moving it to the server crate (e.g., a gateway_env module or alongside the handler that reads it).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I agree and have moved that definition to a private server-local constant in crates/openshell-server/src/grpc/provider.rs where it is used.

@grs
grs force-pushed the token-exchange branch from 2123487 to c578c97 Compare July 30, 2026 08:04

@rhuss rhuss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this, Gordon. The two-stage exchange design is solid, and the security model (subject token never leaves gateway, audience-scoped intermediates, full SVID verification via SPIRE JWT bundles) is well thought through.

A few observations beyond the inline comments:

Consider splitting out the demo. The 59-file diff is a lot to review in one pass. The examples/spiffe-token-exchange-demo/ directory (README, demo.sh, K8s manifests, token-issuer, protected-service) is self-contained and could ship as a follow-up PR without blocking the core feature. That would trim roughly 8 files and make the core diff less intimidating for reviewers.

Extract a shared token exchange helper. perform_token_exchange and perform_intermediate_token_exchange share ~80% structure: URL parsing, client assertion type resolution, form POST assembly, error handling, response parsing, and access token validation. The differences are just which form parameters get added. A common helper that takes a pluggable form-params builder would reduce the duplication and ensure future fixes in one path don't miss the other.

Gateway env var placement. I left an inline comment earlier about GATEWAY_SPIFFE_WORKLOAD_API_SOCKET not fitting the sandbox_env.rs module contract (see thread on that file).

Comment thread proto/openshell.proto
// Supervisor JWT-SVID. The gateway verifies this and uses its `sub` claim
// as the requested audience for the intermediate token.
string supervisor_jwt_svid = 4;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supervisor_jwt_svid carries a JWT-SVID credential but is missing the [(openshell.options.v1.secret) = true] annotation. Every other credential-bearing field in this proto uses it (e.g., IssueSandboxTokenResponse.token at line 288, TcpForwardInit.authorization_token at line 819). Without it, secret-scrubbing interceptors won't redact this value in debug logs.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (I merged this into the original commit)

Comment thread proto/openshell.proto

message ExchangeProviderSubjectTokenResponse {
string access_token = 1;
int64 expires_in = 2;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue: access_token should have [(openshell.options.v1.secret) = true] for consistency with every other token field in the proto.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (I merged this into the original commit as above)

pub async fn obtain_provider_token(request: ObtainProviderTokenRequest<'_>) -> Result<String> {
let grant_type = ProviderCredentialTokenGrantType::try_from(request.grant_type)
.unwrap_or(ProviderCredentialTokenGrantType::ClientCredentials);
obtain_provider_token_with_grant(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a newer gateway sends an unrecognized grant_type enum variant, this silently falls back to ClientCredentials instead of reporting the misconfiguration. This could mask a broken token exchange setup where the profile says token_exchange but the proto value doesn't round-trip correctly.

Consider logging a warning on the fallback, or returning an error for unknown values (keeping the Unspecified -> ClientCredentials mapping as-is).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (also merged this into the original commit)

@rhuss

rhuss commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Additional observations (lower severity)

A few more things I noticed while reviewing. None of these are blocking, but worth considering:

Possible double-skew on intermediate token cache expiry
In provider.rs intermediate_token_cache_expires_at_ms, the 30-second skew constant (INTERMEDIATE_TOKEN_CACHE_EXPIRY_SKEW_SECONDS) is subtracted in seconds from expires_in before multiplying by 1000. Then cap_cache_expiry_ms applies the same skew again, converting to milliseconds. The effective skew could be 60s instead of the intended 30s. Worth verifying whether this is intentional.

perform_token_exchange takes 8 positional &str parameters
Two of them (subject_token_type and requested_token_type) are adjacent and easily swappable. A field swap compiles but produces incorrect OAuth2 requests. The PR already introduces ObtainProviderTokenRequest for the public API; a similar struct here would be consistent and prevent this class of bug.

No unit test for perform_token_exchange form parameters
perform_token_grant has tests that capture the outbound HTTP form POST and assert each parameter. The new perform_token_exchange has no equivalent. Given this is an RFC 8693 exchange, a test verifying the form fields (grant_type, subject_token, subject_token_type, requested_token_type) would prevent regressions in parameter assembly.

Cache eviction is non-deterministic
When the intermediate token cache in provider.rs is full, it evicts the first key from HashMap::keys(). Iteration order is non-deterministic, so this could evict a fresh entry while keeping one about to expire. An LRU or "evict closest to expiry" strategy would be more predictable.

SELinux label z on SPIFFE socket mount
In container.rs, the SPIFFE socket bind mount uses lowercase z, which relabels the host directory with a shared SELinux label. This could affect other consumers of the same SPIRE agent socket (the agent itself, other containers). The existing TLS cert mounts work because those are per-sandbox files, not a shared socket. Consider Z (private label) or omitting the label.

parse_unverified_jwt_svid_claims missing security doc note
In spiffe.rs, the function name correctly says "unverified," but there's no doc comment warning callers they must separately verify the JWT signature before trusting the claims. A /// # Security note would prevent future misuse.

Helm values.yaml comment scope
The comment on server.providerTokenGrants.spiffe.enabled says "gateway and sandbox pods" but this value only controls the gateway pod mount. Sandbox-side SPIFFE is configured separately through the driver config. Minor clarification.

@rhuss

rhuss commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

@TaylorMutch @mrunalp @johntmyers, this PR could use your eyes. I went through the implementation in detail (agent-assisted) and I think it's ready for maintainer review.

The design is non-intrusive by construction. Token exchange is a new grant_type enum value alongside the existing client_credentials. The L7 injection layer doesn't change at all because both paths delegate through the same TokenGrantResolver trait. If a provider profile doesn't declare token_exchange, or SPIFFE infrastructure isn't present, the feature is completely inert. All three conditions must align: SPIRE running, driver config pointing to the socket, and a profile requesting it. Nothing changes for environments without SPIFFE.

I reviewed the driver integration (agent-assisted) and the abstraction boundaries are clean. The supervisor logic is fully driver-agnostic. It reads an env var and doesn't know whether it's running under Kubernetes, Podman, or anything else. Each driver opts in independently (K8s via CSI volume, Podman via bind mount). Docker and VM drivers simply don't set the env var and everything works as before. (but nothing prevents those drivers to add support for SPIFFE later, too).

The Podman support is a real bonus for developer workflow. You can run SPIRE locally, test the full token exchange on your laptop, then deploy the same provider profiles to a K8s cluster with proper per-pod attestation. The e2e test validates this path end-to-end.

Strategically, this is the foundation for agent delegation chains, proving both the user's identity and which specific sandbox is acting on their behalf. That capability becomes essential once multi-agent scenarios need real identity propagation.

The PR is 59 files but I think the demo (examples/spiffe-token-exchange-demo/) can be split out into a follow-up to reduce the core review surface. Tests are thorough (unit + e2e).

would be great to come to a decission whether we could add this functionality to the beta roadmap as well. Our customers are asking actively for this feature, so there is a real-world value backing for this, too.

validate_access_token(&intermediate.access_token)?;
let intermediate_subject_token_type =
final_exchange_subject_token_type(request.requested_token_type);
perform_token_exchange(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: intermediate_subject_token_type is derived from request.requested_token_type (the output type the caller wants), but the intermediate token from Stage 1 is always an access token (that's what the gateway returns). In the Stage 2 exchange, the subject_token_type should describe what the intermediate token is, not what the final output should be.

If requested_token_type differs from the default access token type, this would send an incorrect subject_token_type in the RFC 8693 form POST. Should this use DEFAULT_TOKEN_TYPE (or the token_type from the gateway's ExchangeProviderSubjectTokenResponse) instead?

The final_exchange_subject_token_type wrapper uses effective_token_type which falls back to DEFAULT_TOKEN_TYPE when empty, so in practice it may work today if requested_token_type is always empty or matches. But the semantic coupling seems fragile.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^ is a coderabbit finding, @grs I hope it makes sense to you (but could be a false alarm, too). I found it interesting enough to share this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rhuss! I agree and have made this fix along with the oauth refactor, with a renamed constant to make it clearer.

grs added 6 commits July 30, 2026 22:30
…nt credentials as OAuth grant_type

Signed-off-by: Gordon Sim <gsim@redhat.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
…ervisor

Signed-off-by: Gordon Sim <gsim@redhat.com>
…cache

Signed-off-by: Gordon Sim <gsim@redhat.com>
@grs
grs force-pushed the token-exchange branch from c578c97 to d9a0e16 Compare July 30, 2026 21:51
@grs

grs commented Jul 30, 2026

Copy link
Copy Markdown
Author

Thank you very much for the great review @rhuss!

Consider splitting out the demo. The 59-file diff is a lot to review in one pass. The examples/spiffe-token-exchange-demo/ directory (README, demo.sh, K8s manifests, token-issuer, protected-service) is self-contained and could ship as a follow-up PR without blocking the core feature. That would trim roughly 8 files and make the core diff less intimidating for reviewers.

I can certainly do that, but at present I kept it as a separate commit on the branch as it is a useful smoke test for anyone trying things out. If you review commit-by-commit you can skip that one.

Extract a shared token exchange helper. perform_token_exchange and perform_intermediate_token_exchange share ~80% structure: URL parsing, client assertion type resolution, form POST assembly, error handling, response parsing, and access token validation. The differences are just which form parameters get added. A common helper that takes a pluggable form-params builder would reduce the duplication and ensure future fixes in one path don't miss the other.

Agreed. I have made this change as an additional commit and included the original token grant in the refactor as well, as there is a reasonable amount of duplication there also.

Gateway env var placement. I left an inline comment earlier about GATEWAY_SPIFFE_WORKLOAD_API_SOCKET not fitting the sandbox_env.rs module contract (see thread on that file).

Fixed earlier.

@grs

grs commented Jul 30, 2026

Copy link
Copy Markdown
Author

Additional observations (lower severity)

A few more things I noticed while reviewing. None of these are blocking, but worth considering:

Possible double-skew on intermediate token cache expiry In provider.rs intermediate_token_cache_expires_at_ms, the 30-second skew constant (INTERMEDIATE_TOKEN_CACHE_EXPIRY_SKEW_SECONDS) is subtracted in seconds from expires_in before multiplying by 1000. Then cap_cache_expiry_ms applies the same skew again, converting to milliseconds. The effective skew could be 60s instead of the intended 30s. Worth verifying whether this is intentional.

The allowance is applied to the tokens own expiry first, then to the expiry of other tokens (the subject token and the jwt-svid), with the minimum taken between these. So I believe it is correct as is at present, but challenge me further if you disagree or I am misunderstanding your point.

perform_token_exchange takes 8 positional &str parameters Two of them (subject_token_type and requested_token_type) are adjacent and easily swappable. A field swap compiles but produces incorrect OAuth2 requests. The PR already introduces ObtainProviderTokenRequest for the public API; a similar struct here would be consistent and prevent this class of bug.

Agreed, I have done this in the new oauth refactor commit.

No unit test for perform_token_exchange form parameters perform_token_grant has tests that capture the outbound HTTP form POST and assert each parameter. The new perform_token_exchange has no equivalent. Given this is an RFC 8693 exchange, a test verifying the form fields (grant_type, subject_token, subject_token_type, requested_token_type) would prevent regressions in parameter assembly.

This is also covered in the oauth refactor.

Cache eviction is non-deterministic When the intermediate token cache in provider.rs is full, it evicts the first key from HashMap::keys(). Iteration order is non-deterministic, so this could evict a fresh entry while keeping one about to expire. An LRU or "evict closest to expiry" strategy would be more predictable.

I have added a new commit for this that evicts the closest to expiry which is more intuitive.

SELinux label z on SPIFFE socket mount In container.rs, the SPIFFE socket bind mount uses lowercase z, which relabels the host directory with a shared SELinux label. This could affect other consumers of the same SPIRE agent socket (the agent itself, other containers). The existing TLS cert mounts work because those are per-sandbox files, not a shared socket. Consider Z (private label) or omitting the label.

Agreed, I have removed the label and merged that fix into the podman support commit.

parse_unverified_jwt_svid_claims missing security doc note In spiffe.rs, the function name correctly says "unverified," but there's no doc comment warning callers they must separately verify the JWT signature before trusting the claims. A /// # Security note would prevent future misuse.

Fixed (merged into the first commit)

Helm values.yaml comment scope The comment on server.providerTokenGrants.spiffe.enabled says "gateway and sandbox pods" but this value only controls the gateway pod mount. Sandbox-side SPIFFE is configured separately through the driver config. Minor clarification.

I think the comment is correct. That value is used to create the driver config. So from the users perspective its one switch to enable spiffe on an install, that will then affect the gateway and the driver config (and through that the supervisors).

Thanks again for all the great feedback @rhuss !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support user-subject dynamic token grants for sandbox agents

3 participants