BTBurke / caddy-jwt

JWT middleware for the Caddy server
MIT License
114 stars 39 forks source link

expiry time not validated? #4

Closed joncfoo closed 8 years ago

joncfoo commented 8 years ago

Hi, I'm not sure if I'm doing something wrong or if this is a bug..

  1. given this Caddyfile:
localhost {
  log stdout
  jwt /protected
}
  1. invoke caddy like so: env JWT_SECRET=testing caddy
  2. visit http://localhost:2015/protected?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOiIxMDAiLCJyb2xlIjoibWVtYmVyIn0.CYkF_TZi1dBR3wbM8UtQtbTQAfy1bzXU__McQPTLm0U
  3. 404 is returned instead of an expected 401

I set the "exp" claim to 100 in the token - it can be viewed/validated at https://jwt.io/

Am I using this incorrectly? I also tried using a valid expiry time in the past, e.g. 1460046313, and still got the 404 instead of a 401.

BTBurke commented 8 years ago

Hi @joncfoo, I added a few tests to the suite to figure out what's going on. The "exp" field needs to be a unix timestamp as an integer. If you try it with an arbitrary string it will still validate the token as long as the signature is valid.

This is a bit of a gotcha in how the IETF standard for JWT is implemented. The expiration time is not a required field so tokens won't fail validation if it either doesn't exist or is malformed.

The thinking behind this is that it's up to the issuer to issue a token with a valid format. An attacker can't forge a new or malformed value in the "exp" field because it would fail signature validation. Personally, I think it would be better to fail validation if the "exp" field exists but doesn't correspond to a valid timestamp. However, I don't think it would be a good idea to change the expected behavior of this library from the standard.

If you try it again with a valid timestamp as an integer it should work. I added another test to the library to verify that it is indeed failing expired tokens and everything looks ok. Little gotchas like this is why I recommend that you use one of the JWT libraries to generate the token when you use this in production.

joncfoo commented 8 years ago

Thanks for the detailed break-down @BTBurke! I do wish the jwt library would fail on invalid input especially if the exp field is invalid but I think in this scenario PEBKAC suffices since it would be near impossible for a client to modify the token.

Thanks!