On Keys and Algorithms

[bellebaum]

This is a sort of "Lessons Learned" combined with a few suggestions this project might want to consider.

Story time

I have a server that has to verify and decode JWTs from several sources using several algorithms.
Looking at the README for this gem, it strongly suggests hard-coding a parameter called algorithm for the decode function. Since I do not know the algorithm in advance, I took it from the JWT, and ran some tests on it to ensure that only an expected subset of algorithms are considered valid.

But herein lies the problem: What I should have done is take a look at the code and find that algorithm behaves more like allowed_algorithms and I can specify several of those in an array.
I am not an expert in JWTs. There is no way for me to be sure that I have covered all possible attack vectors related to alg.

Suggestion I

Document the possibility of specifying several algorithms in the README in "Algorithms and Usage", possibly with an example.
Ideally, change the name of the variable algorithm to allowed_algorithms.
You can keep the old variable as an alias to maintain backwards compatibility.

Suggestion II

The article linked in the README also suggests the following:

Most (hopefully all?) implementations now have a basic check to prevent this attack: if a secret key was provided, then token verification will fail for tokens using the none algorithm. This is a good idea, but it doesn't solve the underlying problem: attackers control the choice of algorithm. Let's keep digging.

They also follow this up with an example of how one key can be interpreted as either RSA or HMAC relying on the fact that both were encoded as a string.

Since this library does not have such problems (An RSA key is of type OpenSSL::PKEY, while an HMAC shared secret is not, see also #184 ), we could even go one step further:

One could check the provided key against the expected algorithms. I agree that this is not enough in many situations, but I also agree with the article that it does not hurt.

#400 goes one step further by not requiring algorithm at all if the type of key can be resolved. To me it seems like that person has fallen for the same misunderstanding I have, that one cannot specify multiple algorithms.

Conclusion

There is nothing wrong with this library per se, but a few tweaks could nudge people to intuitively use it more secure way.
Let's have a discussion :)

[anakinj]

I really like the suggestions here. I would even like to go one step further (if possible): not using the alg header for anything more than sorting the allowed_algorithms list.

Now the header is still in a pretty big role when it comes to choosing the mechanism to verify the signature.

[excpt]

I'd welcome that change. In my head it would look like this.

Example pseudo code:

This code should raise an error because the 'HS256' algorithm is not in the allowed list.

allowed_algos = ['HS384', 'HS512']

hmac_secret = 'my$ecretK3y'

token = JWT.encode payload, hmac_secret, 'HS256'

# Should raise an exeption, token algo not in allowed_algos list
decoded_token = JWT.decode token, hmac_secret, true, { algorithms: allowed_algos }

[markdboyd]

I would like to upvote the recommendation to verify the JWT against a list of allowed algorithms. I would even go one step further: the library should require that you specify the allowed algorithms unless you explicitly disable it.

This is how the pyJWT library currently works:

https://github.com/jpadilla/pyjwt/blob/50ecae21c3ecf36f7c2a285e4e59dae1d9391e3d/jwt/api_jwt.py#L146

Ultimately, the algorithm confusion exploit for JWT is pretty significant, so while it is good to warn people of the danger by linking to this article, I think it would be preferable if libraries enforced a safe default that mitigates the exploit altogether

I'm happy to personally contribute the PR for this behavior if the maintainers are interested.

[anakinj]

I'm totally agreeing with you and as I'm trying to figure out the scope of the next major version I think maybe instead of trying to focus on backwards compatibility we should focus on making the use of the gem a bit more secure by enforcing things like this.

In other words, the one validating the token should be 100% in control what algorithms are allowed to be used.

[markdboyd]

@anakinj Thanks! Sorry if I came across as argumentative, I just recently was doing some research into JWT security so I was excited to find this issue and wanted to add my support for the proposal.

Also, I did start looking at the code to see if I could make the necessary changes myself, but Ruby isn't my strongest language. I'll try to take another look when I get a chance.

[anakinj]

Closing this for now I feel that the new JWT::EncodedToken is trying to protect the user by:

• Require signature and claims verification to have happen before accessing the payload is allowed. (Claim requirement coming in 3.1.)
• Always require the algorithm to be given when verifying the signature. Exception are JWKs where the algorithm can be resolved from the key itself. The key is something that is trusted by the application by default, therefore we assume the algorithm is also trusted.