Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using more than 1 bound_audiences does not validate a claim that has only 1 audience #30

Closed
hmalphettes opened this issue Mar 13, 2019 · 2 comments
Assignees

Comments

@hmalphettes
Copy link

I am trying to configure a single JWTRole that accepts claims where the value of aud can be any value that belongs to a fixed list.

When I configure a jwt role with 2 bound_audiences, if the claim contains a single one of the 2 values, Vault will reject it.

I was hoping to use the bound_audiences claim for that based on the documentation:

`bound_audiences` `(array: <required>)` - List of `aud` claims to match against. Any match is sufficient.

In my setup, the identity of the principal is declared via the authoritative 'aud' claim:

	// Configure vault
	config := &vault.CoreConfig{
		CredentialBackends: map[string]logical.Factory{
			"jwt": vaultjwt.Factory,
		},
	}
	core, _, rootToken := vault.TestCoreUnsealedWithConfig(tb, config)
	ln, addr = http.TestServer(tb, core)        
	client, _ := makeVaultClient(ln, addr)
	opts := &api.EnableAuthOptions{Type: "jwt"}
	err = client.Sys().EnableAuthWithOptions("jwt", opts)
	if err != nil {
		return err
	}
	payload := map[string]interface{}{
		"jwt_validation_pubkeys": pubKey,
	}
	_, _ = client.Logical().Write("auth/jwt/config", payload)
	payload := map[string]interface{}{
		"bound_audiences": []string{
			"INFRA/master/deploy",
			"ADMIN/master/deploy"},
		"user_claim":      "requestor",
		"policies":        string[]{"default","test_policy"},
		"ttl":             "5m",
	}
	_, _ = client.Logical().Write("auth/jwt/role/testrole", payload)
	
	// Login with a jwt token:
	ts := int64(time.Now().Unix())
	claims := &jwt.MapClaims{
		"aud":       []string{"INFRA/master/deploy"},
		"sub":       "anything",
		"foo":       "bar",
		"iat":       ts,
		"exp":       ts + 300,
		"requestor": "unitTester",
	}
	token := jwt.NewWithClaims(jwt.SigningMethodRS512, claims)
	tokenString, err := token.SignedString(key)
	if err != nil {
		return nil, err
	}
	// now sign in (...)
URL: PUT http://127.0.0.1:53937/v1/auth/jwt_vx/login
Code: 400. Errors:
* error validating claims: square/go-jose/jwt: validation failed, invalid audience claim (aud)"

So at the moment: if any of the bound_audiences are not found in the JWT token, the validation fails.

Could you let me know if I am grossly misinterpreting the documentation?

Many thanks!


Longer story - in case we are after being able to create a single JWT Role from JWT Tokens that claims may differ.

On stackoverflow I can find others interpreting the JWT spec in the same manner - a single match is enough: https://stackoverflow.com/a/41237822
with the caveat that the behavior is optional and application specific.

Unfortunately the JWT library sides is validating the opposite:
https://github.com/SermoDigital/jose/blob/master/jwt/eq.go#L23

// ValidAudience returns true iff:
// 	- a and b are strings and a == b
// 	- a is string, b is []string and a is in b
// 	- a is []string, b is []string and all of a is in b
// 	- a is []string, b is string and len(a) == 1 and a[0] == b
func ValidAudience(a, b interface{}) bool {

where a is the expected audience and b the token passed for validation:

https://github.com/SermoDigital/jose/blob/master/jwt/jwt.go#L72:

	if aud, ok := v.Expected.Audience(); ok {
		aud2, ok := j.Claims().Audience()
		if !ok || !ValidAudience(aud, aud2) {
			return ErrInvalidAUDClaim
		}
	}

The unit test is also quite clear that their interpretation of a valid aud claim is the reverse of what we have in mind:
https://github.com/SermoDigital/jose/blob/master/jwt/eq_test.go#L19

The other JWT implementation for golang also adopt the same behavior:
https://github.com/square/go-jose/blob/v2/jwt/validation.go#L89

@kalafut kalafut self-assigned this Mar 13, 2019
@kalafut
Copy link
Contributor

kalafut commented Mar 13, 2019

This is an interesting issue, and an area where the two types of validation (OIDC vs. provided keys) are inconsistent, because of the use of the specific library function in the latter. Based on rereading the RFC and some other discussion of it, I think our documented approach (i.e. any match is sufficient) is reasonable. In fact Auth0, who is using the Square library (square/go-jose#151), handles the check our way in their own JS library: https://github.com/auth0/node-jsonwebtoken/blob/master/verify.js#L160 🤷‍♂️

I'll review this and get back to you.

@hmalphettes
Copy link
Author

@kalafut many thanks again. Apologies for the lengthy tale and thanks for going along with it.

I do think your documented approach is reasonable.
The validation enforced by the SermoDigital (exact matches and more) is already well covered by other claims like sub.
In the OIDC world where aud is the client_id, it obviously does not make sense either to impose to each client to know in advance the client_ids of all the other clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants