Critical vulnerabilities in JSON Web Token

Issue #122 resolved
Alex Soto created an issue

As described in this post https://www.timmclean.net/2015/03/31/jwt-algorithm-confusion.html there is a critical bug in JWT libraries which assumes that the algorithm provided in token is the correct one, but one attacker may change this and the token will be checked using a different algorithm. And this has some implications for example in case of asymetric algorithm or none. Of course this is something that spec should address, but from the point of view of libraries they can provide some tools so developers can mitigate the problem as described in How can libraries fix this? section. Basically it allows developer to do something like verify(string token, string algorithm, string verificationKey).

The mitigation I propose for Nimbus is overloading com.nimbusds.jose.JWSObject.verify method so apart from this call developer also can do com.nimbusds.jose.JWSObject.verify(JWSVerifier verifier, JWSAlgorithm algorithm). Inside this method we can check if alg header field is equals to the one passed by developer. If equals then go ahead, if not then throw a JOSEException.

Of course it is only a mitigation and server side should know a prior which algorithm is going to be used (by setting it statically which is the most case) or by using kid field to query about algorithm.

I can implement this change without any problem.

Comments (7)

  1. Vladimir Dzhuvinov

    So there seem to be two approaches for that:

    1. Let the default accept alg set be empty. That way the users will be forced explicitly to turn on the algs they need.

    2. As @asotobu suggests, add an alg set to the constructor list.

    2 seems my favourite for now :)

  2. Vladimir Dzhuvinov

    Pardon, I now went through your PR and realised you suggest something different :)

    I need to look at how this change would play out in practice, for example with the Connect2id server where reverse ID token verification is done for certain purposes. I want to be sure all possible implications are understood before committing to this. This is so much fun :)

    Cheers,

    Vladimir

  3. Vladimir Dzhuvinov

    Hi guys,

    I mediated yesterday and today on this issue. I wish this gets resolved in a way that makes good sense and is consistent.

    Tweaking the JWSObject class to augment verification does not feel good as it breaks the rule of separating concerns. The purpose of JWSObject (and its sister classes) is to be a representation of JWS objects (and tokens), nothing more. Checking on the "alg" value should be the job of the JWSVerifier. Blurring the line between the two concerns will not be a good engineering decision I think.

    The problem with the current implementation is that while the range of say "HSnnn" algs can be set in the MACVerifier, this is more of a side option, and developers may not be aware of it, or skip it.

    My proposal is to make the Verifier classes immutable, and have them set the range (1..x) of accepted supported "alg" values in the constructor. Moreover, add specific constructors with JWK argument, where their "alg" parameter is used to lock down the permitted algorithm, "kid" the ID, etc. This I think would be the best measure to enforce say verification of HS512 when the client pushes a HS256. Note that the library is not vulnerable to the HS / RS / EC substitution attack due to Java's strong type and the use different classes for each alg type.

    About the need to maintain a pool of MAC verifiers of different HS algs?

    I examined some client side uses, and that is not required.

    With OIDC for example a client registers for a particular algorithm and key, and creates just one verifier for it. This verifier is then fed with any number of ID tokens, with varying subject, etc. But the "alg" and "kid" remain the same.

    Besides, even if verifier reuse can't work for some reason, the state that each verifier has is very lightlweight; the rest is calls to static method / singleton crypto classes behind the JCA facade. So creating many verifiers should not be a performance problem.

    Good night :)

  4. Vladimir Dzhuvinov

    I'm also in favour of gentle RPs. But occasionally keeping things orderly and in line requires one to dig deeper. Such is life :)

    I will try to get this done next week, I've got it all pictured in my head now. Will show you the first bits to get an idea.

    Cheers,

  5. Vladimir Dzhuvinov

    Processing of JOSE objects has been significantly updated, and a framework of kind has been introduced too. All this should become part of a new major 4.0 release, probably out by the end of this month :)

  6. Log in to comment