Token Validate step should not use algorithm provided in the jwt header

[mlegore]

This implementation is insecure, specifically the verification step can be circumvented easily. Because you use the algorithm specified in the header to verify, and attacker could just send you a JWT with algorithm=None, and your verification method would return true. Instead, JsonWebToken.validate should take the algorithm expected and verify using that algorithm. The algorithm should never be allowed to be chosen by the untrusted user.

[jasongoodwin]

I don't beleive it's vulnerable as described but let me double check - haven't looked at this in a while. The user can also verify the algorithm.

…

On Feb 24, 2017 12:58 AM, "mlegore" ***@***.***> wrote: This implementation is insecure, specifically the verification step can be circumvented easily. Because you use the algorithm specified in the header to verify, and attacker could just send you a JWT with algorithm=None, and your verification method would return true. Instead, JsonWebToken.validate should take the algorithm expected and verify using that algorithm. The algorithm should never be allowed to be chosen by the untrusted user. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#31>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACtqLOuIzGgozgcgJlv9i5KZelzH_qAQks5rfnGdgaJpZM4MK2wu> .

[andyt10]

I remember having a play around with this because I thought the same thing, then realised that because of the way the JWT string is split in JsonWebToken.scala

jwt.split("\\.") match {
  case Array(providedHeader, providedClaims, providedSignature)

we cannot pass in an empty string as the signature "", and match that case of tuple3.

in which case we fail:
case _ ⇒ false

If we pass in a bogus value with header 'None', it still doesn't seem to work. JsonWebSignature.scala compares 'None' to empty array so bogus signature != empty array.

`def apply(algorithm: Algorithm, data: String, key: String = null): Array[Byte] = {
algorithm match {

    case HS256 ⇒ HmacSha("HmacSHA256", data, key)
    case HS384 ⇒ HmacSha("HmacSHA384", data, key)
    case HS512 ⇒ HmacSha("HmacSHA512", data, key)
    case NONE  ⇒ Array.empty[Byte]

`

To me this means the library doesn't even support signature of 'None'.

Am I correct or crazy?

:)

[rbscott]

That is correct. We are using this library and were happy to find we were not vulnerable to the none algorithm, but didn't fully understand why until we stepped through the library code.

Is there a plan to add support for the none algorithm? I am not sure where I stand on this, since not having support for none, probably increased security, but the spec clearly outlines support for it.

If support is added, it would be helpful be able to validate the algorithm and signature at the same time.