Skip to content
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

JWT issuer check does not account for encrypted JWTs #591

Closed
zandbelt opened this issue Jan 18, 2019 · 3 comments
Closed

JWT issuer check does not account for encrypted JWTs #591

zandbelt opened this issue Jan 18, 2019 · 3 comments
Assignees
Labels
Milestone

Comments

@zandbelt
Copy link
Contributor

zandbelt commented Jan 18, 2019

The changes in 43a03ab do not seem to account for encrypted JWTs where the id_token iss value can only be checked after decrypting the token. Rather than:

  1. do basic claim checks
  2. verify id_token"

the process should be changed to:

  1. decrypt id_token
  2. do basic claim checks
  3. verify id_token signature

I believe that in its turn calls for splitting the from_jwt method on the IdToken class into separate decryption and verification methods.

The alternative is to rollback this change...

@panva
Copy link

panva commented Jan 18, 2019

Didn't familiarize myself with the changes but an operation where iss, aud and other claims are replicated into the unencrypted part of the JWT in order to support the required operation should be taken into account and processed accordingly.

https://tools.ietf.org/html/rfc7519#section-5.3

In some applications using encrypted JWTs, it is useful to have an
unencrypted representation of some claims. This might be used, for
instance, in application processing rules to determine whether and
how to process the JWT before it is decrypted.

This specification allows claims present in the JWT Claims Set to be
replicated as Header Parameters in a JWT that is a JWE, as needed by
the application. If such replicated claims are present, the
application receiving them SHOULD verify that their values are
identical, unless the application defines other specific processing
rules for these claims. It is the responsibility of the application
to ensure that only claims that are safe to be transmitted in an
unencrypted manner are replicated as Header Parameter values in the
JWT.

Section 10.4.1 of this specification registers the "iss" (issuer),
"sub" (subject), and "aud" (audience) Header Parameter names for the
purpose of providing unencrypted replicas of these claims in
encrypted JWTs for applications that need them. Other specifications
MAY similarly register other names that are registered Claim Names as
Header Parameter names, as needed.

@zandbelt
Copy link
Contributor Author

zandbelt commented Jan 18, 2019

Fair point: the process would then become:

  1. check if the JWT header contains basic claims and validate those
  2. decrypt id_token
  3. verify basic claims if not done already via claims in the header
  4. verify signature

@panva
Copy link

panva commented Jan 18, 2019

2.5 verify claims present in both the unencrypted header and encrypted payload are the same ;)

@schlenk schlenk added the bug label Jan 18, 2019
@tpazderka tpazderka self-assigned this Jan 18, 2019
@tpazderka tpazderka added this to the 0.15.1 milestone Jan 21, 2019
tpazderka added a commit that referenced this issue Jan 21, 2019
tpazderka added a commit that referenced this issue Jan 21, 2019
tpazderka added a commit that referenced this issue Jan 21, 2019
andrewkrug pushed a commit to mozilla-iam/pyoidc that referenced this issue Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants