Keats / jsonwebtoken

JWT lib in rust
MIT License
1.67k stars 266 forks source link

jsonwebtoken::Validation could help users implement the spec more correctly #329

Closed grahamc closed 11 months ago

grahamc commented 1 year ago

RFC7519 has this to say about the aud field:

Each principal intended to process the JWT MUST identify itself with a value in the audience claim. If the principal processing the claim does not identify itself with a value in the "aud" claim when this claim is present, then the JWT MUST be rejected.

I'm sure many users aren't using this field, and are opening themselves up to vulnerabilities of jwts intended for one use and reused for another. What if the API was adjusted to be something like this, to help users figure out they should be paying attention here?

Validation::new(Algorithm::HS256, Some(&["me"]);

This is different from the other fields which are optionally set, since those are generally configured to be relatively secure out of the box.

Keats commented 1 year ago

We will error if the aud in the token is set but not in the validation so I don't think it opens up vulnerabilities: https://github.com/Keats/jsonwebtoken/blob/master/src/validation.rs#L265-L276

The issue with having it in new is that not every token uses audiences so you end up with various &[] a bit everywhere..

grahamc commented 12 months ago

We will error if the aud in the token is set but not in the validation

This isn't right: it only errors if a user explicitly sets the validation's aud to Some(&[]). We made this mistake and didn't catch it until I was spelunking the RFC for something unrelated. Does this seem like an error to you?

Keats commented 12 months ago

Ah yes, it's missing the None branch in the match which is probably a bug