orchestracities/boost

JWT validation false positives

Closed this issue · 9 comments

Validation works for the most part but there are some gotchas. Specifically, to jwt-go (the lib we use under the bonnet for validation) a field with a 0 value is the same as the field not being there! So e.g. this token (pseudo code):

{ alg: RS256 }.{ exp: 0 }.valid-rs256-signature

passes validation even though it expired at the beginning of the epoch!

Instead of delegating validation to jwt-go, consider rolling out your own. Specifically, for the following JWT registered claims: exp ("expires at"), iat ("issued at"), nbf ("not before").

Note to self. If rolling out custom validation, be extra careful about security implications!!!

For example, checking the signature algo ∂ (HMAC, RSA, ...) declared in the JWT alg header is critical, since jwt-go will check the token according to ∂. Specifically, we could say that in (very!) abstract terms,jwt-go implements a signature verification function

F: Algo x JWT -> Bool 

so that for a JWT token t

t's signature is valid <=> F(∂, t) = T   where ∂ = t.alg

Because of this, an attacker could use our public key K as an HS256 shared secret to sign a forged token u:

header = { alg: HS256, ... }
payload = { ... }
signature = hs256-sign(K, header + payload)

Since in this case F(HS256, u) = T, we'd be fooled into thinking u got signed with the private key paired to K!!!

For a better explanation of the problem, see e.g.

@chicco785

the behaviour may be correct

Quite plainly, I think it's a bug in jwt-go, courtesy of Go's weak type system. But I think you're talking about the scope of our validation? i.e. we shouldn't validate any registered claims? But then who or what's going to check the token hasn't expired? Can Orion do that? If the adapter returns success to the Mixer, then the message gets passed on to Orion...

However, we could implement a custom policy resource like they do in the IBM adapter so that you could check e.g. the exp field using an Istio config expression.

orion can't do that

yep, just I just wanted to double-check with you, thanks.

simply saying that "validation" as jwt validation is one thing (and i suppose the library does what's needed)

As far as signature verification goes, jwt-go can verify the signature correctly as long as you check the JWT alg field yourself---see my earlier comment about it. If you don't do that, verification goes out of the window.

However, jwt-go can also check some of the registered claims, notably the exp field, but like I said there's a bug in the lib whereby a token with a valid signature and an exp field set to 0 is considered valid.

then you need to apply some logic based on the content of the token to finalise the authorisation...the fact that a jwt token is valid, does not mean you are authorised to do something with that token.

Exactly, so let's stick to validation here :-) To me, RSA token validation has the following meaning:

  1. Check the token is well-formed.
  2. Verify the payload hasn't been tampered with.
  3. Identify payload author.
  4. Verify the payload is current, if possible.

RSA signature verification gives us (2) and (3) whereas the additional checks on the exp and nbf fields give us (4). I think we both agree the adapter should do (1), (2) and (3), but perhaps you're not too keen on (4). Should we drop (4) then? Why though?

agreed, cool bananas!

A little late, but I agree. Especially if we also think of future options of maybe caching JWT?
We will have other Access Right functions with XACML. In that context checking for iss/nbf <= now() <= exp.

simply we can do (4) outside the library

Well, we actually have to do it now. In fact I've just unearthed a nasty bug in jwt-go whereby a token with a non-numeric exp field is considered valid! And again, this has to do with Go's weak type system. A token with a payload of e.g.

{ exp: "put null any non numeric value you like here" }

passes validation. Fortunately, we won't cache it since our code actually does the right thing and considers it expired but since we rely on jwt-go to tell us if the token is useable for the current call, we give the Mixer the green light.

So we have to replace jwt-go validation of (4) with our own. Here's a nasty scenario to convince you we should. Say the server that issues the token has a silly serialization bug whereby the exp field value becomes a string

{ exp: "12345678" }

Everytime the client passes it on to us we okay it so potentially the client could use it inadvertently for a long time past the server's intended expiry date. This makes it more likely to be hijacked and if an attacker gets hold of one of those tokens, they'll be able to use it forever!