From b8833ceee3c7ea49dfac0a18a4050508ebdee605 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Fri, 12 Apr 2024 09:57:19 -0700 Subject: [PATCH] invalidate JWT with single non-empty string aud on empty bound audiences (#295) --- CHANGELOG.md | 3 ++ go.mod | 3 +- go.sum | 2 - path_login.go | 17 +++++++-- path_login_test.go | 92 +++++++++++++++++++++++++--------------------- path_oidc_test.go | 4 +- path_role.go | 2 +- 7 files changed, 71 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 353ce19f..b816343d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Unreleased +IMPROVEMENTS: +* Invalidate JWT with single non-empty string aud on empty bound audiences https://github.com/hashicorp/vault-plugin-auth-jwt/pull/295 + ## v0.20.2 IMPROVEMENTS: diff --git a/go.mod b/go.mod index d3030250..0580a591 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/hashicorp/vault-plugin-auth-jwt go 1.21 require ( + github.com/go-jose/go-jose/v3 v3.0.3 github.com/go-test/deep v1.1.0 github.com/hashicorp/cap v0.6.0 github.com/hashicorp/errwrap v1.1.0 @@ -22,7 +23,6 @@ require ( golang.org/x/oauth2 v0.18.0 golang.org/x/sync v0.6.0 google.golang.org/api v0.163.0 - gopkg.in/go-jose/go-jose.v2 v2.6.3 ) require ( @@ -41,7 +41,6 @@ require ( github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/fatih/color v1.16.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/go-jose/go-jose/v3 v3.0.3 // indirect github.com/go-jose/go-jose/v4 v4.0.1 // indirect github.com/go-logr/logr v1.4.1 // indirect github.com/go-logr/stdr v1.2.2 // indirect diff --git a/go.sum b/go.sum index 8a6ad417..ae89c069 100644 --- a/go.sum +++ b/go.sum @@ -460,8 +460,6 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU= gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/go-jose/go-jose.v2 v2.6.3 h1:nt80fvSDlhKWQgSWyHyy5CfmlQr+asih51R8PTWNKKs= -gopkg.in/go-jose/go-jose.v2 v2.6.3/go.mod h1:zzZDPkNNw/c9IE7Z9jr11mBZQhKQTMzoEEIoEdZlFBI= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/path_login.go b/path_login.go index d562d01e..e346abd1 100644 --- a/path_login.go +++ b/path_login.go @@ -157,9 +157,20 @@ func (b *jwtAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d // If there are no bound audiences for the role, then the existence of any audience // in the audience claim should result in an error. - aud, ok := getClaim(b.Logger(), allClaims, "aud").([]interface{}) - if ok && len(aud) > 0 && len(role.BoundAudiences) == 0 { - return logical.ErrorResponse("audience claim found in JWT but no audiences bound to the role"), nil + // "aud" value may be a single case-sensitive string if JWT has one audience + // See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.3 + if len(role.BoundAudiences) == 0 { + audClaim := getClaim(b.Logger(), allClaims, "aud") + switch aud := audClaim.(type) { + case []interface{}: + if len(aud) > 0 { + return logical.ErrorResponse("audience claim found in JWT but no audiences bound to the role"), nil + } + case string: + if aud != "" { + return logical.ErrorResponse("audience claim found in JWT but no audiences bound to the role"), nil + } + } } alias, groupAliases, err := b.createIdentity(ctx, allClaims, roleName, role, nil) diff --git a/path_login_test.go b/path_login_test.go index 47161f04..84ecd83c 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -17,13 +17,13 @@ import ( "testing" "time" + "github.com/go-jose/go-jose/v3" + sqjwt "github.com/go-jose/go-jose/v3/jwt" "github.com/go-test/deep" "github.com/hashicorp/cap/jwt" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" - "gopkg.in/go-jose/go-jose.v2" - sqjwt "gopkg.in/go-jose/go-jose.v2/jwt" ) type H map[string]interface{} @@ -270,7 +270,7 @@ func testLogin_JWT(t *testing.T, jwks bool) { } } - // Test missing audience + // Test bound audiences unset, claims "aud" set { cfg := testConfig{ @@ -278,50 +278,58 @@ func testLogin_JWT(t *testing.T, jwks bool) { } b, storage := setupBackend(t, cfg) - cl := sqjwt.Claims{ - Subject: "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients", - Issuer: "https://team-vault.auth0.com/", - NotBefore: sqjwt.NewNumericDate(time.Now().Add(-5 * time.Second)), - Audience: sqjwt.Audience{"https://vault.plugin.auth.jwt.test"}, - } + nonZeroAudCnts := []int{1, 3} + for _, nonZeroAudCnt := range nonZeroAudCnts { + aud := sqjwt.Audience{} + for i := 0; i < nonZeroAudCnt; i++ { + aud = append(aud, fmt.Sprintf("https://vault.plugin.auth.jwt.test%d", i)) + } - privateCl := struct { - User string `json:"https://vault/user"` - Groups []string `json:"https://vault/groups"` - }{ - "jeff", - []string{"foo", "bar"}, - } + cl := sqjwt.Claims{ + Subject: "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients", + Issuer: "https://team-vault.auth0.com/", + NotBefore: sqjwt.NewNumericDate(time.Now().Add(-5 * time.Second)), + Audience: aud, + } - jwtData, _ := getTestJWT(t, ecdsaPrivKey, cl, privateCl) + privateCl := struct { + User string `json:"https://vault/user"` + Groups []string `json:"https://vault/groups"` + }{ + "jeff", + []string{"foo", "bar"}, + } - data := map[string]interface{}{ - "role": "plugin-test", - "jwt": jwtData, - } + jwtData, _ := getTestJWT(t, ecdsaPrivKey, cl, privateCl) - req := &logical.Request{ - Operation: logical.UpdateOperation, - Path: "login", - Storage: storage, - Data: data, - Connection: &logical.Connection{ - RemoteAddr: "127.0.0.1", - }, - } + data := map[string]interface{}{ + "role": "plugin-test", + "jwt": jwtData, + } - resp, err := b.HandleRequest(context.Background(), req) - if err != nil { - t.Fatal(err) - } - if resp == nil { - t.Fatal("got nil response") - } - if !resp.IsError() { - t.Fatal("expected error") - } - if !strings.Contains(resp.Error().Error(), "no audiences bound to the role") { - t.Fatalf("unexpected error: %v", resp.Error()) + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "login", + Storage: storage, + Data: data, + Connection: &logical.Connection{ + RemoteAddr: "127.0.0.1", + }, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("got nil response") + } + if !resp.IsError() { + t.Fatal("expected error") + } + if !strings.Contains(resp.Error().Error(), "no audiences bound to the role") { + t.Fatalf("unexpected error: %v", resp.Error()) + } } } diff --git a/path_oidc_test.go b/path_oidc_test.go index 76c88c79..2b66a1ac 100644 --- a/path_oidc_test.go +++ b/path_oidc_test.go @@ -21,13 +21,13 @@ import ( "testing" "time" + "github.com/go-jose/go-jose/v3" + "github.com/go-jose/go-jose/v3/jwt" "github.com/hashicorp/cap/oidc" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/go-jose/go-jose.v2" - "gopkg.in/go-jose/go-jose.v2/jwt" ) func TestOIDC_AuthURL(t *testing.T) { diff --git a/path_role.go b/path_role.go index 822c6b6b..188f74d6 100644 --- a/path_role.go +++ b/path_role.go @@ -10,12 +10,12 @@ import ( "strings" "time" + "github.com/go-jose/go-jose/v3/jwt" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/tokenutil" "github.com/hashicorp/vault/sdk/logical" - "gopkg.in/go-jose/go-jose.v2/jwt" ) var reservedMetadata = []string{"role"}