search

BTBurke / caddy-jwt

JWT middleware for the Caddy server
MIT License
113 stars 40 forks source link

Added ECDSA (ES256, ES384, ES512) support (transparent) #31

Closed blaubaer closed 6 years ago

blaubaer commented 6 years ago

...and separated key loading and parsing.

hairyhenderson commented 6 years ago

Wow, what timing! I was just trying to verify an ES256-signed token and wondering why it didn't work... 😂 Thanks for submitting this!

Hopefully @BTBurke can merge soon - in the meantime, I'll try this out 😉

hairyhenderson commented 6 years ago

@blaubaer @BTBurke I've just tested this change out, and I can confirm it works for ES256 👍

BTBurke commented 6 years ago

@blaubaer Thanks for the work on this. I really appreciate the contribution. There are a lot of changes in here so I'll have to take a closer look this weekend.

BTBurke commented 6 years ago

Sorry for the slow response on this. I'm in the middle of moving right now so it may take me a bit to look into this fully since there are quite a few changes here.

BTBurke commented 6 years ago

I fixed a few of these and went ahead and merged it. This is available in v3.6.0 available via the Caddy build server or by building from source.

Thanks for the contribution!

hairyhenderson commented 6 years ago

[...] no longer throws an error when people try to mix and match HMAC with public key authentication [...]

😨

I fixed a few of these and went ahead and merged it.

@BTBurke just to be clear - does the middleware now prevent HMAC and asymmetric validation on the same endpoint?

BTBurke commented 6 years ago

Yes, that's always been the way it works. You can't use HMAC and public key on the same protected path. You can use different algorithms on different paths. It's specifically related to this vulnerability: https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

The idea is to prevent people from shooting themselves in the foot by eliminating some common problems in configuring this securely. If people have such an advanced use case that they need to accept tokens signed by multiple algorithms, then they should move that validation into their app.

hairyhenderson commented 6 years ago

@BTBurke thanks! I'm aware of the vulnerability and wanted to make sure the behaviour hadn't changed 😁

blaubaer commented 6 years ago

Hi @BTBurke!

Thanks for reviewing it. I was simply not aware about https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/ and changed this because I was thinking that this behavior was just because of the way how the code stored the values before. Thank you very much for clarifying. 😄

And yes.... If I refactor things... filename => envPubKey ... that's why I always love if someone reviews my code because things like this could happens the best developers. 😈

Cheers