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

When using none alg (no signature), should the claims verification be ignored? #323

Open
glasses618 opened this issue Jun 21, 2019 · 4 comments · May be fixed by #352
Open

When using none alg (no signature), should the claims verification be ignored? #323

glasses618 opened this issue Jun 21, 2019 · 4 comments · May be fixed by #352

Comments

@glasses618
Copy link

glasses618 commented Jun 21, 2019

Hi,

When I upgrade from v1.5.6 to v2.2.1, I find that, the 'none' algorithms behaves differently.

In v1.5.6, even use 'none', the exp claim would be verified, but in v2.2.1, it doesn't

the script:

payload = { 'exp' => (Time.now.to_i - 5) }
id_token = JWT.encode(payload, nil, 'none')
JWT.decode(id_token, nil, false)

the output:

  • v1.5.6:
=> JWT::ExpiredSignature: Signature has expired
  • v2.2.1:
=> [{"exp"=>1561080813}, {"alg"=>"none"}]

And I find that this commit 67f4a5a change the behavior.

I want to make sure that, when using none alg (no signature), should the claims verification be ignored?

Thank you!

@hesalx
Copy link

hesalx commented Sep 10, 2019

I think is was just a bug that verify = false still caused verification in the first place.
What you want would look like this (should have been like this even in 1.5.6):
JWT.decode(id_token, nil, true, { verify_exp: true, algorithm: 'none' })
But it does not actually work.

It looks like alg none is only really accounted for in encode but not in decode.

@glasses618
Copy link
Author

I think is was just a bug that verify = false still caused verification in the first place.
What you want would look like this (should have been like this even in 1.5.6):
JWT.decode(id_token, nil, true, { verify_exp: true, algorithm: 'none' })
But it does not actually work.

It looks like alg none is only really accounted for in encode but not in decode.

I think there are two meanings of verification, signature and claims.
What is the parameter 'verify' in decode method designed for, to verify the signature, or to verify claims?

def initialize(jwt, key, verify, options, &keyfinder)

I want to make sure that, is it correct to perform the claim verification on an Unsecured JWT?(https://tools.ietf.org/html/rfc7519#section-6.1)

Thank you!

@hesalx
Copy link

hesalx commented Apr 28, 2020

Thoughts specific to the gem in question

I think there are two meanings of verification, signature and claims.
What is the parameter 'verify' in decode method designed for, to verify the signature, or to verify claims?

I'm not the author of that code to tell what was the intension behind the API's design decisions. Naming things is one of those subjective and hard topics in programming.

This gem has basic validation as per RFC and simply does not cover some rare or peculiar use cases.

Although it's possible the gem could make use of "validation" vs "verification" terminology, but as I point out later, a full-blown validation is likely not the point of the gem and what it does right now it more like a "do what RFC explicitly states" in a single chunk under the "verify" umbrella term.

As best as I can understand the current design of the gem, the API:

JWT.decode(id_token, nil, true, { verify_exp: true, algorithm: 'none' })

would make perfect sense.
verify=true (third argument) stands for "standard set of RFC requirements", while options let you choose specific claims to verify.
Following this design there exists a bug as of now. The verification does not support "alg": "none". Which is perfectly suitable for further verification as per the RFC (it does not make distinction between secured and unsecured tokens when it comes to claims verification). The gem explicitly supports "alg": "none" for token creation and it would only be logical if it could successfully parse back it own output.
Everything beyond that behaviour would be considered application-specific behaviour.

Thoughts about terminology and RFCs

I want to make sure that, is it correct to perform the claim verification on an Unsecured JWT?

Yes, the RFC does not forbid the verification of unsecured tokens and leaves it to the developer to secure the token by the means they deem sufficient. Section 6 clarifies that https://tools.ietf.org/html/rfc7519#section-6

To my mind, whether you trust a JWT or not is a separate question from claims verification.

Signature has nothing to do with the validness of the contents of a message. It is normal to validate submitted html forms but they are not signed. You can verify (be sure) who sent the message (using cryptography) but you have to check separately whether it's not garbage.

RFCs are often very generic. And it's sometimes hard to draw the line between mandatory/standard and application-specific. It's a common issue with RFCs involving auth and crypto. Hardly anyone should blame lib's creators and maintainers in such cases (so please don't).

The RFC puts some requirements on the registered claims while still leaving them in the application domain. It will be a very long and not necessarily fruitful discussion to decide whether a gem should have sophisticated claims validation or should leave it completely to a gem's user.

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

The "exp" (expiration time) claim identifies the expiration time on or after which the JWT MUST NOT be accepted for processing.

What is processing? Does it mean "halt the executing right there"? Or does it mean "don't do with that token what you would usually do" (i.e. grant permission) and you can do something else?
Does this mean I can never look into the contents of an otherwise perfectly valid but expired token to show some nice error message or anything else based on its claims?

I think there are two meanings of verification, signature and claims.

Think about "unverified" and "invalid". What would you want to do with an unverified token and with an invalid token. It's likely that you don't want to touch an unverified token any further. But you may still want to do a special treatment for a verified but invalid token. Here verification is about trust and validation is about relevance and consistency.

This point of view helps me to better architecture the handling of JWT. In one of the cases any trustworthy token received by that particular system had to have valid signature, Issuer and Audience. Otherwise it's not even allowed to be read by other components. Meanwhile there were a plethora of uses cases (access tokens, permission granting, authorization, etc.) with distinct claims validation rules (many of which utilized registered claims). And I had to write a completely separate API on top of this gem to do that.

hesalx added a commit to hesalx/ruby-jwt that referenced this issue Apr 28, 2020
Allows for symmetric support of Unsecured JWT. It now works as any other algorithm for both encoding and decoding.

Fixes jwt#323.
hesalx added a commit to hesalx/ruby-jwt that referenced this issue Apr 28, 2020
Allows for symmetric support of Unsecured JWT. It now works as any other algorithm for both encoding and decoding.

Fixes jwt#323.
@hesalx hesalx linked a pull request Apr 28, 2020 that will close this issue
hesalx added a commit to hesalx/ruby-jwt that referenced this issue Apr 28, 2020
Allows for symmetric support of Unsecured JWT. It now works as any other algorithm for both encoding and decoding.

Fixes jwt#323.
hesalx added a commit to hesalx/ruby-jwt that referenced this issue Apr 28, 2020
Allows for symmetric support of Unsecured JWT. It now works as any other algorithm for both encoding and decoding.

Fixes jwt#323.
@glasses618
Copy link
Author

@hesalx
Thank for your detailed response, especially the RFCs v.s. application (Verification v.s. Validation) part. I benefit a lot from your comment.

I do not mean to blame the author. What I want to confirm is if it is a bug or not.

@anakinj anakinj removed this from the Version 2.3.0 milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants