Possibility to force signature algorithm on parse

[danilobuerger]

Is it possible to force the signature algorithm on parse? Meaning that the jwt is only considered valid if it is signed with a particular algorithm?

If not, is it possible to disallow the NONE algorithm or otherwise check the algorithm and make sure that the jwt is signed on parse?

[jcarvalho]

You can always use a SigningKeyResolver, so that you can decide to whether to provide the key or not, depending on the provided algorithm.

[danilobuerger]

But will that get called if there is no Signature? When Algorithm is NONE?

[jcarvalho]

That will be called after the token is parsed, but before its signature (and also expiration I believe) is validated, so you can already determine if the algorithm is NONE, and return a null key.

You can find more about it here.

[danilobuerger]

Reading DefaultJwtParser it seems that the SigningKeyResolver will only be called if base64UrlEncodedDigest != null. So what if that is null? How can i check that?

[danilobuerger]

I think i found the solution here: https://stormpath.com/blog/jwt-java-create-verify/ Looks like we can use parseClaimsJws which does not allow Algorithm NONE and must have a valid signature.

[louoso]

+1 I feel the parser should enforce a signature by default if a key is provided.

[lhazlewood]

This is documented in the Readme:

Known Type convenience parse methods

If, unlike above, you are confident of the compact string format and know which type of JWT or JWS it will produce, you can just use one of the 4 new convenience parsing methods to get exactly the type of JWT or JWS you know exists.

Is this issue resolved, then, by just improving the documentation?

[louoso]

First, great work and thank you for writing this.

Second, now that I go back and look at the readme again, it is clear that you purposefully used parseClaimsJws in the first example:

assert Jwts.parser().setSigningKey(key).parseClaimsJws(s).getBody().getSubject().equals("Joe");

My eyes somehow skipped right over that though, and went to this:

try {

    Jwts.parser().setSigningKey(key).parse(compactJwt);

    //OK, we can trust this JWT

} catch (SignatureException e) {

    //don't trust the JWT!
}

So maybe just change that second example to also use parseClaimsJws?

It could just be me, but I can't understand why "algo: none" even exists. Seems like it is just asking for trouble.

Thanks again!

[lhazlewood]

Thanks for the feedback! This would definitely clean up the docs to reflect the most common use case - I think that'd be helpful.

And yeah, NONE is asking for trouble, but we support it because it is in the JWT spec - we're kind of required to support it.

[lhazlewood]

I just updated the Readme in Master - hopefully that helps!

[danilobuerger]

Yes, @louoso explained it perfectly, that second example got me too! Maybe even go further highlighting in the README that parseClaimsJws should be considered the default except you know what you are doing and that it excludes JWT (vs. JWS) and Algo NONE.

Algo NONE is just dangerous. The library (especially being in the "crypto" sector) should make it easy for people to just do the right thing. Obviously the full standard should be supported. Maybe using Algo NONE or JWT (vs. JWS) could be enabled with a Config Option first, else Assert would throw?

[lhazlewood]

I definitely agree about 'doing the right thing' automatically - it's a tactic we take in Apache Shiro (another project I work on), and it's a big reason why Shiro is so nice to use.

I'll leave this issue open as a marker to clean up the documentation and maybe even make API changes as necessary to make this more obvious. Suggestions welcome!

[danilobuerger]

Maybe the new requireXXX (0.6) methods from JwtParser could be extended to the header?

[jahlborn]

After reading this article https://auth0.com/blog/2015/03/31/critical-vulnerabilities-in-json-web-token-libraries/ i believe that the parseClaimsJws() method should take a signature algorithm as a second argument (or provide an overloaded method which allows this).

[lhazlewood]

@jahlborn Not necessary. This has been asked and answered previously: #20

[jahlborn]

Ah, i see, you protect against it as long as you use a Key based type because they are strongly typed in java. cool.

[lhazlewood]

Indeed :)