Skip to content

Limetime management for OIDC JWKS #10620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
woodruffw opened this issue Jan 19, 2022 · 5 comments · Fixed by #10628
Closed

Limetime management for OIDC JWKS #10620

woodruffw opened this issue Jan 19, 2022 · 5 comments · Fixed by #10628
Assignees

Comments

@woodruffw
Copy link
Member

The first step in verifying the OIDC JWTs is verifying their signatures, which means checking them against the OIDC provider's signing key set.

There are a few approaches we could take to acquiring and maintaining the updatedness of those keys:

  1. Bake them into the Warehouse codebase. We could pull the current JWKS blob from the URI referenced in the provider's openid-configuration.
  • Pros: Simple.
  • Cons: More tedious to update in the event of key updates/rotations; requires a PR each time any provider does so. Gaps between the rotation and updates means that Warehouse would probably reject authentic JWTs.
  1. Fetch the JWKS for each provider on Warehouse startup/initialization. Restarts always re-fetch the JWKS.
  • Pros: Simple.
  • Cons: Most of the same cons as (1).
  1. All of (2), but we also include a periodic job that checks each OIDC provider's JWKS on a schedule.
  • Pros: No rotation downtime.
  • Cons: Requires the most code.
@woodruffw
Copy link
Member Author

(3) is probably the right approach. At minimum, we're going to need:

  1. A registry/configuration list of supported OpenID connectors (just GitHub, for the time being), tied to their openid-configuration URLs
  2. A scheduled periodic task (similar to the ones already in Warehouse) for checking the openid-configuration + JWKS URL
  3. A verification service for incoming JWTs

@woodruffw
Copy link
Member Author

WIP branch is here: https://github.com/trailofbits/warehouse/tree/tob-jwk-management

Some outstanding design questions:

  1. How should we store "live" JWKs? They're JSON blobs and they're not sensitive and they rotate, so it doesn't make sense to cram them into the DB. Perhaps Redis?

  2. What should we do for our development environment? Does it make sense to have a DevelopmentJWKService that uses a local RSA keypair + JWK that we generate as part of make build, or should we just run the "full" JWKService?

@woodruffw woodruffw self-assigned this Jan 20, 2022
@woodruffw
Copy link
Member Author

  • How should we store "live" JWKs? They're JSON blobs and they're not sensitive and they rotate, so it doesn't make sense to cram them into the DB. Perhaps Redis?

I went ahead and did this on the branch -- we cache JWKs under the /warehouse/oidc/jwks/{provider} key for each provider.

You can smoke-test this with:

docker-compose run --rm web python -m warehouse oidc update-oidc-jwks
docker-compose run --rm web python -m warehouse oidc list-jwks --provider github

@di
Copy link
Member

di commented Jan 21, 2022

  1. All of (2), but we also include a periodic job that checks each OIDC provider's JWKS on a schedule.

There's the potential for a brief window here where a new public key has been published to the JWKS but we haven't fetched it yet, and as a result signature verification will fail with legitimate signatures. Perhaps it be better to fetch the JWKS whenever we attempt to verify a JWT that has a kid (key ID) that we haven't encountered before?

This behavior works with cold-starts (the first verification with no JWK in cache causes us to reach out and get the entire JWKS) and this ensures we always have the latest keys before failing. To avoid these requests becoming abusive we could only repeat a request for a previously nonexistent kid outside of some window.

@woodruffw
Copy link
Member Author

There's the potential for a brief window here where a new public key has been published to the JWKS but we haven't fetched it yet, and as a result signature verification will fail with legitimate signatures. Perhaps it be better to fetch the JWKS whenever we attempt to verify a JWT that has a kid (key ID) that we haven't encountered before?

That sounds good! And yeah, that provides nice self-starting behavior (much better than having to jumpstart things with a CLI command on each startup.)

We'll want to make sure that we preserve provider-first identification with these changes -- a previously unknown kid shouldn't enable a cache poisoning attack where an attacker tricks us into trusting a different provider for a matching kid.

To avoid these requests becoming abusive we could only repeat a request for a previously nonexistent kid outside of some window.

Sounds good. In that case we'll probably want an additional self-expiring KV in Redis for the cooldown window. A cooldown of even just 60s is probably more than sufficient.

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 a pull request may close this issue.

2 participants