square / go-jose

An implementation of JOSE standards (JWE, JWS, JWT) in Go
1.98k stars 278 forks source link

go-jose.v2 Expected audience length must equal claim audience? #151

Closed kukicado closed 7 years ago

kukicado commented 7 years ago

Hello,

At Auth0, are using the go-jose.v2 package to validate the aud claims in a JWT against the defined audience of our API. Being a SaaS, we have a use case where the JWT may contain multiple audiences, that our Golang application may not know about.

For example, here is a valid JWT that has an additional audience for getting the userinfo from our API

{
  "iss": "https://adobot.auth0.com/",
  "sub": "auth0|57be8eb207732b164b356bc3",
  "aud": [
    "golang-sample",
    "https://adobot.auth0.com/userinfo"
  ],
  "azp": "E7v0bIDB2bM4ICfIgbWPbe6J6T54hsiT",
  "exp": 1496797023,
  "iat": 1496789823,
  "scope": "openid read:messages"
}

In our Golang application, the only audience we have defined is golang-sample, so when we use the go-jose.v2 library to validate the audience we get the error "square/go-jose/jwt: validation failed, invalid audience claim (aud)".

This happens because in the following code (https://github.com/square/go-jose/blob/v2/jwt/validation.go#L73):

if len(e.Audience) != len(c.Audience) {
    return ErrInvalidAudience
}

e.Audience = [golang-sample]

while

c.Audience = [golang-sample, https://adobot.auth0.com/userinfo]

If we remove the code above, the validation passes.

I would be happy to submit a PR that removes this check, but I am curious to know if there is a particular reason the check exists in the first place? I think it makes sense to remove the check as the code that follow it loops over the array of expected claims and checks to see if it exists in the JWT - I see no reason they must be the same length.

Please let me know what you think and if you would be ok accepting such a PR.

csstaub commented 7 years ago

Yeah, this seems like a bug.

From RFC 7519:

4.1.3.  "aud" (Audience) Claim

   The "aud" (audience) claim identifies the recipients that the JWT is
   intended for.  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.  In the general case, the "aud" value is an array of case-
   sensitive strings, each containing a StringOrURI value.  In the
   special case when the JWT has one audience, the "aud" value MAY be a
   single case-sensitive string containing a StringOrURI value.  The
   interpretation of audience values is generally application specific.
   Use of this claim is OPTIONAL.

Would be happy to accept a PR to fix this. Square requires a CLA for contributions, please see CONTRIBUTING.md for a link to that. If you can't sign a CLA for some reason I can also fix this bug myself but it might take me a few days to get around to it.

kukicado commented 7 years ago

Awesome thanks! I just created a PR for it.

Please let me know if you need anything else. :)