Skip to content

fix(oauth2): cap exchange lifetime to subject and app cred exp - #1459

Draft
millionsouls wants to merge 6 commits into
openstack-experimental:mainfrom
millionsouls:expiry
Draft

millionsouls wants to merge 6 commits into
openstack-experimental:mainfrom
millionsouls:expiry

Conversation

@millionsouls

@millionsouls millionsouls commented Oct 8, 2026 •

Copy link
Copy Markdown

Summary

#1353
Caps token exchange lifetime based on subject and app credential lifetime.

Referencing checklist from issue:

  1. crates/core/src/oauth2_client/token_exchange.rs:188:203 Exposes expires_at to claim builder.
  2. crates/core/src/oauth2_client/token_exchange.rs:205:211 Computes min time based on reported cap value in expires_in. Unsure if this should be skipped or rejected if an expiry is absent as I believe open stack does offer non-expiring tokens (?).
  3. crates/core/src/oauth2_client/token_exchange.rs:205:211 crates/keystone/src/api/v4/oauth2/token/token_exchange.rs:117:122 invalid_grant rejection when lifetime <= 0
  4. crates/core/src/oauth2_client/token_exchange.rs:484 general unit tests
  5. tests/integration/src/oauth2_token_exchange.rs:201 unit tests for short lived app credential
  6. doc/src/adr/0026-oauth2-oidc-provider.md:1555

Test plan

cargo test -p openstack-keystone-core oauth2_client::token_exchange::tests Expiry rules
cargo test -p openstack-keystone api::v4::oauth2::token::token_exchange::tests Endpoint and response
cargo check -p openstack-keystone --message-format=short Compiling
cargo fmt --check Format

All pre-commit run --all-files passed
cargo clippy -p openstack-keystone --fix --allow-dirty passed
Could not run cargo test and cargo nextest run as tss-esapi-sys couldn't run on macOS. Ran cargo test -p openstack-keystone-core token_exchange and cargo test -p openstack-keystone token_exchange: passed

Security review checklist

Required if this diff touches authentication, scope, delegation, tokens,
credentials, EC2, trusts, application credentials, or OPA policy input.

Delete this section entirely if it doesn't apply. See
doc/src/contributor/security-model.md
§7 for the full context behind each item.

  • Does any delegation/authorization decision read the scope
    (project_id, ScopeInfo) where it should read the chain
    (authentication_context(), delegation object)? (I1/I2)
  • New delegation-sensitive policy rule? Is the fact it needs projected
    onto Credentials from the chain, and does the rule anchor on
    delegated_project_id + carry the scope-drift tripwire? (I2/I3)
  • New scope shape or redemption path for a delegated auth? Are effective
    roles still bounded by the delegation? Added a test that a restricted
    delegation cannot exceed its roles via the new path? (I4)
  • New ScopeInfo variant or auth method? Updated
    validate_scope_boundaries(), calculate_effective_roles(),
    fully_resolved(), and Credentials::try_from — all without adding a
    wildcard _ => arm? (I5, Gate J)
  • New lookup by a client-derivable key (like sha256(access))? Does it
    re-assert type/shape after fetch? (I6)
  • Does any new data reaching OPA include secrets/decrypted blobs? (I7,
    Gate I)
  • New list/collection endpoint? Does it re-check each item with the
    per-item read policy? (I8)
  • Same endpoint: can an unprivileged caller request the whole collection
    and leave the per-item pass to sort it out? Is the value the
    collection policy gates on the same one the driver actually filters
    by? (I8a)
  • Does the change let a narrow auth method be broadened by a
    request-supplied scope? (I5)
  • Are there negative tests proving the escape is blocked, not just
    positive tests proving the happy path works?
  • Does the test drive ValidatedSecurityContext::new_for_scope()
    end-to-end, not just the inner helper (calculate_effective_roles,
    validate_scope_boundaries) in isolation?

Comment thread crates/core/src/oauth2_client/token_exchange.rs Outdated
Comment thread crates/keystone/src/api/v4/oauth2/token/token_exchange.rs Outdated
Comment thread crates/keystone/src/api/v4/oauth2/token/token_exchange.rs
@millionsouls millionsouls changed the title caps token-exchange lifetime to subject and app credential expiry bug(oauth2): cap exchange lifetime to subject and app cred exp Oct 9, 2026
@millionsouls
millionsouls force-pushed the expiry branch 2 times, most recently from 51d1d31 to 59a6f08 Compare October 14, 2026 04:13
@millionsouls

Copy link
Copy Markdown
Author

I think I messed up on committing my changes with merging the new commits from main. Attempting to fix it, ignore checks for now (apologies).

@millionsouls
millionsouls force-pushed the expiry branch 3 times, most recently from 040c714 to a3b358c Compare October 14, 2026 22:44
MissingSubjectTokenExpiry,

/// The credential-chain expiry is not later than the new token's issue time.
/// Subject token/ App Cred has either already expired or expires at/before the new token's issue time
MissingSubjectTokenExpiry,

/// The credential-chain expiry is not later than the new token's issue time.
/// Subject token/ App Cred has either already expired or expires at/before the new token's issue time
@millionsouls millionsouls changed the title bug(oauth2): cap exchange lifetime to subject and app cred exp fix(oauth2): cap exchange lifetime to subject and app cred exp Oct 17, 2026
Prevents an exchanged token from outliving the credential chain it represents.
Claim builder now caps the new expiry at the earliest of the config
access token expiry, the validated subject token's expiry, and the app
cred expiry.

Assisted-by: Claude <noreply@anthropic.com>

Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
MissingSubjectTokenExpiry and NonPosititiveLifetime errors log seperately
Modified error descriptions to be more specific for these case.
Cases will be logged as warnings, while unexpected error(s) will
be logged as full errors.

Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Three rejections tests through the HTTP endpoint for already
expired app cred token, expiring now app cred token, and expiring now
subject token

Assisted-by: Claude <noreply@anthropic.com>

Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Modified derive_delegation_context to return app credential expiration along
with delegation_context. build_token_exchange_claim does not have
to recheck cred expiration again.

Adjusted existing tests to account for additional paramter field.

Assisted-by: Claude <noreply@anthropic.com>

Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Rename NonPositiveTime to CredentialChainExpired to better reflect
the error; subject token or app cred has expired now, or will
expire before the new token's expiration.

Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
Signed-off-by: Kevin <36448536+millionsouls@users.noreply.github.com>
//! reflects the presented credential's own delegation, per security.md I2
//! (the delegation boundary is the credential's own immutable project, not
//! the request's current scope).
use openstack_keystone_api_types::webauthn::credential_protection_policy;
//! reflects the presented credential's own delegation, per security.md I2
//! (the delegation boundary is the credential's own immutable project, not
//! the request's current scope).
use openstack_keystone_api_types::webauthn::credential_protection_policy;

This branch has not been deployed

No deployments
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.

3 participants