search

auth0 / node-jws

JSON Web Signatures
http://self-issued.info/docs/draft-ietf-jose-json-web-signature.html
MIT License
709 stars 108 forks source link

adding equals sign into validation regex #25

Closed troystauffer closed 9 years ago

troystauffer commented 9 years ago

[closes #22]

This will allow tokens base64 encoded tokens to be validated as legit. Without the allowance of the equals sign, base64 encoding that requires padding is rejected. The spec appears to allow for base64 encoded tokens, though it encourages base64url encoding (which does not produce padding).

brianloveswords commented 9 years ago

Hi! I'm trying to figure out where in the spec it says base64 (rather than base64url) is allowed – can you point me to the spot? I'm happy to support this if it's supported by the spec!

troystauffer commented 9 years ago

Well, as I re-reviewed the spec to find where I thought I read that, I pretty much found the exact opposite. It spells out in no uncertain terms in Section 6 that padding should not be allowed either in creation or validation of tokens due to URL safety. I'll have to take this up with the issuer of the JWT (Stormpath).

Thanks!

[Edit: It looks like that was an old spec, and while the latest isn't as explicit in it's verbiage about padding, it says that generally no "additional characters" should be allowed.]

omsmith commented 9 years ago

@troystauffer as a note, I don't believe that particular section / wording made it into the actual RFCs (https://tools.ietf.org/html/rfc7519).

The JWS RFC refers to operations as BASE64URL() and "base64url decode", where the example implementation for decoding appears to be lenient to incoming base64 strings.

So, while your JWT provider should still fix their thing, I believe by the RFC, we needn't be so strict here. @brianloveswords ?

EDIT: typo

brianloveswords commented 9 years ago

From my reading, the RFC doesn't explicitly disallow base64 and indeed it does look like their reference implementation is flexible. If you're comfortable with this change @omsmith, I'll allow it!

omsmith commented 9 years ago

As per the change in question, it seems it should be extended to allow base64 in its entirety (+ and / as well), and allow it on all 3 signature parts

brianloveswords commented 9 years ago

@troystauffer, you wanna make the changes that @omsmith mentioned?

omsmith commented 9 years ago

Closing in favour of #30