Trusted publishing: prevent OIDC credential re-use

[woodruffw]

Pointed out by @sethmlarson and @di: an OIDC credential can currently be re-used to issue multiple temporary API tokens, for as long as its expiry window remains open. This results in two potential weaknesses:

1. Disclosure: a user may (incorrectly) believe that the OIDC credential is "spent" after being exchanged, and disclose or log it somewhere public.
2. Exfiltration: an attacker who isn't able to access the temporary API token or OIDC issuance directly may be able to access the extant OIDC credential, and issue a new temporary API token from it.

(2) is unlikely on GitHub, since any context in which the attacker has access to the OIDC credential is also likely one in which they can mint a fresh credential. But this may be still relevant for other supported OIDC IdPs.

Both of these can be addressed by enforcing one-time use of the OIDC credential within Warehouse:

1. Require that all supported OIDC IdPs include the jti claim in their JWTs.
2. Enforce per-provider uniqueness of jtis: if the same JTI is seen again, the JWT it belongs to should be rejected and token exchange should fail.

For (1), both GitLab and GitHub support jti. Google Cloud supports nonce, which is subtly different (jti is server controlled, nonce is user-controlled). I'm not 100% sure what ActiveState since their docs don't mention any claims explicitly 🙂

For (2), we probably need either a single new table with a unique constraint on (provider-name, jti) or a new table per-provider with a unique constraint on jti. With both, the table will probably have unbounded growth unless we also periodically delete it (on the same cadence as our self-expiring API tokens, maybe?)

[di]

For (2), we probably need either a single new table with a unique constraint on (provider-name, jti) or a new table per-provider with a unique constraint on jti. With both, the table will probably have unbounded growth unless we also periodically delete it (on the same cadence as our self-expiring API tokens, maybe?)

If we make this (provider-name, jti, exp), we could just periodically prune all tokens past their expiration date without worrying about provider-specific token lifetime lengths.

[woodruffw]

Google Cloud supports nonce, which is subtly different (jti is server controlled, nonce is user-controlled)

Just double-checked, and Google Cloud does not support either jti or nonce in the OIDC credential retrieval flow that we use.

@th3coop could you provide an example of the claim set that ActiveState offers in its JWTs? I'm curious if your claims include a jti we could use 🙂

[woodruffw]

I've asked @DarkaMaul to look at this!

[th3coop]

Hey morning @woodruffw, sounds like our docs aren't as navigable as they should be. The supported claims (though not worded as such) are here: https://docs.activestate.com/platform/user/oidc/#understanding-the-token We also have our .well-known/openid-configuration

So we support nonce by passing it along in the OIDC JWT if it's present in the API request to create said token.

We do not currently support jti.

[th3coop]

For (2); I've done a similar entry tracking, validation and expiry backed by Redis before.

Pros:

• Don't need sub process for periodic key clean up. Key removal is automatic when setting the TTL
• Don't need to touch the DB Schema

Caveats:

• Requires that you add Redis to the warehouse env if it's not present already
• Depending on scale needs, might hit issues with data consistency if you need more than 1 Redis instance.
  • You all will know the numbers better than me but I suspect that for package publishing, 1 Redis instance will be enough.
• The restriction is no longer baked into the model and therefore clear to everyone
  • I'm not sure that you couldn't do this still with the right abstractions and Redis

Just an idea. No sweat if not used/totally ignored :D.

Don't hesitate to ping me if you need more info about the ActiveState stuff or if you'll need ActiveState to put up a PR or anything else I can assist with.

[woodruffw]

Got it, thanks @th3coop! I swear I've seen that page before, but for some reason I couldn't find it when searching.

We do not currently support jti.

Not to snipe you into adding this, but how difficult do you think it would be to add to your claims? 16 bytes of CSPRNG should be more than sufficient.

Re: Redis: that is probably workable! We definitely already have a Redis service in Warehouse's deployment, and it already provides JWKS caching/rotation for the verification phase.

I don't have a super strong intuition for whether that would be better or worse than a new DB table, though -- the table would be "cheap" here in the sense that it would have no relations, but still heavier weight than Redis. OTOH, with Redis, I think we'd need to keep a potentially unbounded number of JWTs resident in RAM, right?

(Technically the number of JWKS is also unbounded, but we currently pull those from trusted parties, i.e. the OIDC IdPs we trust.)

[th3coop]

The hard/longest part of the task will be me second guessing myself on picking the correct algorithm 😄. I'd probably just slap a UUID in there since we use them for everything already. Other than that, it would not be hard for us to add to out OIDC JWT tokens.

What kind of timeline are you all hoping for here?

Re: Redis: When you say "potentially unbounded number" you mean that there could be any number of values stored at any given time with no guarantee that you have enough space, correct? Then definitely yeah. You need estimates on how many requests you expect to get over the expiry time of a these OIDC tokens, which should be quite short compared to out JWTs. Would you be able to scale vertically, adding memory, if volume spiked for some reason?

[woodruffw]

The hard/longest part of the task will be me second guessing myself on picking the correct algorithm 😄. I'd probably just slap a UUID in there since we use them for everything already. Other than that, it would not be hard for us to add to out OIDC JWT tokens.

UUIDv4 has 122 bits of entropy when generated correctly, so this would be more than sufficient 🙂

What kind of timeline are you all hoping for here?

No hard timeline for this, IMO -- the top level feature would be a good one for PyPI to have, but it's currently not breaking anything and can be rolled out incrementally for each IdP. So it'd be great to have ActiveState support jti, but a lack of support wouldn't prevent implementation from rolling forwards here.

Re: Redis: When you say "potentially unbounded number" you mean that there could be any number of values stored at any given time with no guarantee that you have enough space, correct?

Yep. That part was mostly me thinking out loud -- I suspect in practice this wouldn't be a huge issue, but AFAIK there aren't many other places where user interaction with PyPI can cause unbounded Redis usage, so I figured it was worth calling out.

[th3coop]

That all makes sense. I don't see problem getting this added to our OIDC token pretty quickly when the time comes. Keep my posted and please let me know if I can help in other ways, if I can, I will.

[DarkaMaul]

Hi - I'm taking over this issue.

For the moment, the plan is as follows:

• Create a oidc_jti_tokens table with :

  • oidc_provider_name
  • jwt_token_identifier : use an unique constraint on this column
  • expiration :

• Create a task to periodically clean this table (daily) if expiration is after current timestamp : delete_expired_jwt_tokens

• Before minting a token, in verify_claims, if the publisher is an OIDCPublisher, check that the current JTI is not already present in the oidc_jti_tokens table. If not, bail out with an error :

  • code: « invalid-reuse-token »
  • description: « valid token, but already used »

• If all claims checks are successful, add the current jti to the oidc_jti_tokens table with the expiration date set to the exp of the JWT.

• Add tests for all this

Let me know if you have any feedback on how to improve this.

[di]

Create a oidc_jti_tokens table with : Create a task to periodically clean this table (daily) if expiration is after current timestamp : delete_expired_jwt_tokens

Instead of this, I like @th3coop's idea here:

For (2); I've done a similar entry tracking, validation and expiry backed by Redis before.

E.g., using the EXPIRE command to let redis automatically expire these keys.

[DarkaMaul]

Create a oidc_jti_tokens table with : Create a task to periodically clean this table (daily) if expiration is after current timestamp : delete_expired_jwt_tokens

Instead of this, I like @th3coop's idea here:

For (2); I've done a similar entry tracking, validation and expiry backed by Redis before.

E.g., using the EXPIRE command to let redis automatically expire these keys.

Sure, I'll implement the Redis version first, and we can replace it with a table if needed afterwards.

[th3coop]

If you use the PXAT argument on the SET command when adding the token to Redis, I think you'll be able to pass the OIDC tokens exp field as the expiry time/date. https://redis.io/docs/latest/commands/set/#options

[woodruffw]

I think it should be just EXAT instead of PXAT since we don't have millisecond resolution, but thanks for the pointer to these options!

[offbyone]

I hate to necromancer a closed issue -- I'm happy to create a new one if I'm on the right track! -- but I'm seeing what looks like a failure of GitHub actions' tokens to include a JTI: https://github.com/offbyone/django-svcs/actions/runs/10552758336/job/29233437205

##[debug]Authenticating to https://test.pypi.org/legacy/ via Trusted Publishing
##[debug]Selected Trusted Publishing Exchange Endpoint: https://test.pypi.org/_/Oidc/Mint-Token
Error: Trusted publishing exchange failure: 
Token request failed: the server refused the request for the following reasons:

* `invalid-publisher`: valid token, but no corresponding publisher (Missing claim 'jti')

This generally indicates a trusted publisher configuration error, but could
also indicate an internal error on GitHub or PyPI's part.

This is in reference to test pypi, not primary, so it's possible this is something less critical, but it's biting me as I try to publish a new package for the first time using a trusted publisher.

[facutuesca]

@offbyone Huh that is not something we expected. I tried to reproduce it but couldn't.

Looking at warehouse's code, I don't see any obvious things that would trigger that error other than a missing jti claim. Could you try re-running that workflow without any changes? If it's reproducible, we can try to extract the OIDC token and see what's going wrong.

[offbyone]

Happy to, here ya are: https://github.com/offbyone/django-svcs/actions/runs/10552758336

(if you'd prefer I open a new issue for this, just say the word)

[facutuesca]

(if you'd prefer I open a new issue for this, just say the word)

Yeah go for it, better to have this problem documented in its own issue