frasertweedale / hs-jose

Haskell JOSE and JWT library
http://hackage.haskell.org/package/jose
Apache License 2.0
122 stars 46 forks source link

JWT verification vulnerable to header manipulation #29

Closed edgmnt closed 7 years ago

edgmnt commented 7 years ago

It looks like Crypto.JOSE.JWS.verifySig gets the key type from the unverified header:

verifySig k m (Signature raw h (Types.Base64Octets s)) = 
  verify (param (view jwsHeaderAlg h)) (view jwkMaterial k) tbs s where [...]

And here's what the callee does:

verify JWA.JWS.None _ = \_ s -> Right $ s == ""

An attacker could simply substitute None for the key type in the header and clear out the signature.

Issue #5 might've been closed too soon.

frasertweedale commented 7 years ago

Per https://tools.ietf.org/html/rfc7518#section-3.6, hs-jose does not accept the "none" algorithm by default - applications must explicitly enable it.

Apart from that, there is little else one can do whilst remaining compliant with RFC 7515, which does allow the "alg" parameter to be carried in the unsecured header (see https://tools.ietf.org/html/rfc7515#section-4.1.1).

If it is important for your application, you could check whether the "alg" param was carried in the protected header.

edgmnt commented 7 years ago

I think the user (caller) should explicitly select the algorithm without relying on the "alg" field at all. That field can be used for a simple sanity check instead.

Currently, it seems keys are distinguished by type, in effect selecting the algorithm by the key to some extent. However, what happens if you expect RS512 but an attacker alters "alg" to RS256? Are we sure the verification fails, does a downgrade to RS256 happen or is the truncated key much easier to break?

frasertweedale commented 7 years ago

@edgmnt switching from RS512 to RS256 does not truncate the key; it changes the digest algorithm that will be used. If an attacker changes the content and/or hash algorithm, they have to forge a valid signature for the new digest. Because the key is the same, "downgrading" the hash does not make it easier to forge a signature (because the signature output size is the same, and signatures are uniformly distributed).

BTW, an application can modify the ValidationSettings to accept a single algorithm.

haishengwu-okta commented 7 years ago

sounds like sanity check alg would great. this is what I did in my application. https://github.com/okta/samples-haskell-scotty/blob/master/src/Okta/Samples/JWT.hs#L123-L132

frasertweedale commented 7 years ago

@haishengwu-okta afaict this doesn't prevent any attacks (if I am wrong about that, please elaborate!)

I have previously considered whether it is appropriate to fail verification if the JWK's "alg" field is present and does not match alg of signature. I decided against it because the RFCs do not require or recommend a particular behaviour around this JWK "alg" parameter; all it says is:

The "alg" (algorithm) parameter identifies the algorithm intended for use with the key. (https://tools.ietf.org/html/rfc7517#section-4.4)

Therefore I elected to keep the default behaviour conformant to the RFC.

With jose >= 0.6 you can define an instance of JWKStore for a newtype of JWK that performs the specific filtering you need. I'll have a look at providing some combinators to make it easy for library users to easily define "filtered" versions of existing JWKStore instances. And if I do that, then I can see value in export the "JWK alg matches JWS alg" behaviour but not having it as default behaviour.

frasertweedale commented 7 years ago

@haishengwu-okta I filed a new issue for this feature: https://github.com/frasertweedale/hs-jose/issues/44

haishengwu-okta commented 7 years ago

sounds great. thanks!