diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index b4d03fd51dd..01a0538c1fe 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -143,21 +143,23 @@ func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatch allErrs = append(allErrs, field.Required(fldPath, fmt.Sprintf(atLeastOneRequiredErrFmt, fldPath))) return allErrs } - // This stricter validation is because the --oidc-client-id flag option is singular. - // This will be removed when we support multiple audiences with the StructuredAuthenticationConfiguration feature gate. - if len(audiences) > 1 { - allErrs = append(allErrs, field.TooMany(fldPath, len(audiences), 1)) - return allErrs - } + seenAudiences := sets.NewString() for i, audience := range audiences { fldPath := fldPath.Index(i) if len(audience) == 0 { allErrs = append(allErrs, field.Required(fldPath, "audience can't be empty")) } + if seenAudiences.Has(audience) { + allErrs = append(allErrs, field.Duplicate(fldPath, audience)) + } + seenAudiences.Insert(audience) } - if len(audienceMatchPolicy) > 0 && audienceMatchPolicy != api.AudienceMatchPolicyMatchAny { + if len(audiences) > 1 && audienceMatchPolicy != api.AudienceMatchPolicyMatchAny { + allErrs = append(allErrs, field.Invalid(audienceMatchPolicyFldPath, audienceMatchPolicy, "audienceMatchPolicy must be MatchAny for multiple audiences")) + } + if len(audiences) == 1 && (len(audienceMatchPolicy) > 0 && audienceMatchPolicy != api.AudienceMatchPolicyMatchAny) { allErrs = append(allErrs, field.Invalid(audienceMatchPolicyFldPath, audienceMatchPolicy, "audienceMatchPolicy must be empty or MatchAny for single audience")) } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index e3ea3a46684..54cf78e4ae6 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -304,6 +304,23 @@ func TestValidateAudiences(t *testing.T) { matchPolicy: "MatchAny", want: "", }, + { + name: "duplicate audience", + in: []string{"audience", "audience"}, + matchPolicy: "MatchAny", + want: `issuer.audiences[1]: Duplicate value: "audience"`, + }, + { + name: "match policy not set with multiple audiences", + in: []string{"audience1", "audience2"}, + want: `issuer.audienceMatchPolicy: Invalid value: "": audienceMatchPolicy must be MatchAny for multiple audiences`, + }, + { + name: "valid multiple audiences", + in: []string{"audience1", "audience2"}, + matchPolicy: "MatchAny", + want: "", + }, } for _, tt := range testCases { diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go index 1aa2f7abff5..26e6c7335d6 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go @@ -49,6 +49,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/net" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/apis/apiserver" apiservervalidation "k8s.io/apiserver/pkg/apis/apiserver/validation" @@ -99,12 +100,12 @@ type CAContentProvider interface { // initVerifier creates a new ID token verifier for the given configuration and issuer URL. On success, calls setVerifier with the // resulting verifier. -func initVerifier(ctx context.Context, config *oidc.Config, iss string) (*oidc.IDTokenVerifier, error) { +func initVerifier(ctx context.Context, config *oidc.Config, iss string, audiences sets.Set[string]) (*idTokenVerifier, error) { provider, err := oidc.NewProvider(ctx, iss) if err != nil { return nil, fmt.Errorf("init verifier failed: %v", err) } - return provider.Verifier(config), nil + return &idTokenVerifier{provider.Verifier(config), audiences}, nil } // asyncIDTokenVerifier is an ID token verifier that allows async initialization @@ -115,13 +116,13 @@ type asyncIDTokenVerifier struct { // v is the ID token verifier initialized asynchronously. It remains nil // up until it is eventually initialized. // Guarded by m - v *oidc.IDTokenVerifier + v *idTokenVerifier } // newAsyncIDTokenVerifier creates a new asynchronous token verifier. The // verifier is available immediately, but may remain uninitialized for some time // after creation. -func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string) *asyncIDTokenVerifier { +func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string, audiences sets.Set[string]) *asyncIDTokenVerifier { t := &asyncIDTokenVerifier{} sync := make(chan struct{}) @@ -129,7 +130,7 @@ func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string) *a // verifier, or until context canceled. initFn := func() (done bool, err error) { klog.V(4).Infof("oidc authenticator: attempting init: iss=%v", iss) - v, err := initVerifier(ctx, c, iss) + v, err := initVerifier(ctx, c, iss, audiences) if err != nil { klog.Errorf("oidc authenticator: async token verifier for issuer: %q: %v", iss, err) return false, nil @@ -155,7 +156,7 @@ func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string) *a } // verifier returns the underlying ID token verifier, or nil if one is not yet initialized. -func (a *asyncIDTokenVerifier) verifier() *oidc.IDTokenVerifier { +func (a *asyncIDTokenVerifier) verifier() *idTokenVerifier { a.m.Lock() defer a.m.Unlock() return a.v @@ -181,13 +182,20 @@ type Authenticator struct { requiredClaims map[string]string } -func (a *Authenticator) setVerifier(v *oidc.IDTokenVerifier) { +// idTokenVerifier is a wrapper around oidc.IDTokenVerifier. It uses the oidc.IDTokenVerifier +// to verify the raw ID token and then performs audience validation locally. +type idTokenVerifier struct { + verifier *oidc.IDTokenVerifier + audiences sets.Set[string] +} + +func (a *Authenticator) setVerifier(v *idTokenVerifier) { a.verifier.Store(v) } -func (a *Authenticator) idTokenVerifier() (*oidc.IDTokenVerifier, bool) { +func (a *Authenticator) idTokenVerifier() (*idTokenVerifier, bool) { if v := a.verifier.Load(); v != nil { - return v.(*oidc.IDTokenVerifier), true + return v.(*idTokenVerifier), true } return nil, false } @@ -265,16 +273,26 @@ func New(opts Options) (*Authenticator, error) { now = time.Now } + audiences := sets.New[string](opts.JWTAuthenticator.Issuer.Audiences...) verifierConfig := &oidc.Config{ ClientID: opts.JWTAuthenticator.Issuer.Audiences[0], SupportedSigningAlgs: supportedSigningAlgs, Now: now, } + if audiences.Len() > 1 { + verifierConfig.ClientID = "" + // SkipClientIDCheck is set to true because we want to support multiple audiences + // in the authentication configuration. + // The go oidc library does not support validating + // multiple audiences, so we have to skip the client ID check and do it ourselves. + // xref: https://github.com/coreos/go-oidc/issues/397 + verifierConfig.SkipClientIDCheck = true + } var resolver *claimResolver groupsClaim := opts.JWTAuthenticator.ClaimMappings.Groups.Claim if groupsClaim != "" { - resolver = newClaimResolver(groupsClaim, client, verifierConfig) + resolver = newClaimResolver(groupsClaim, client, verifierConfig, audiences) } requiredClaims := make(map[string]string) @@ -294,7 +312,10 @@ func New(opts Options) (*Authenticator, error) { if opts.KeySet != nil { // We already have a key set, synchronously initialize the verifier. - authenticator.setVerifier(oidc.NewVerifier(opts.JWTAuthenticator.Issuer.URL, opts.KeySet, verifierConfig)) + authenticator.setVerifier(&idTokenVerifier{ + oidc.NewVerifier(opts.JWTAuthenticator.Issuer.URL, opts.KeySet, verifierConfig), + audiences, + }) } else { // Asynchronously attempt to initialize the authenticator. This enables // self-hosted providers, providers that run on top of Kubernetes itself. @@ -306,7 +327,7 @@ func New(opts Options) (*Authenticator, error) { } verifier := provider.Verifier(verifierConfig) - authenticator.setVerifier(verifier) + authenticator.setVerifier(&idTokenVerifier{verifier, audiences}) return true, nil }, ctx.Done()) } @@ -374,6 +395,10 @@ type claimResolver struct { // claim is the distributed claim that may be resolved. claim string + // audiences is the set of acceptable audiences the JWT must be issued to. + // At least one of the entries must match the "aud" claim in presented JWTs. + audiences sets.Set[string] + // client is the to use for resolving distributed claims client *http.Client @@ -390,19 +415,25 @@ type claimResolver struct { } // newClaimResolver creates a new resolver for distributed claims. -func newClaimResolver(claim string, client *http.Client, config *oidc.Config) *claimResolver { - return &claimResolver{claim: claim, client: client, config: config, verifierPerIssuer: map[string]*asyncIDTokenVerifier{}} +func newClaimResolver(claim string, client *http.Client, config *oidc.Config, audiences sets.Set[string]) *claimResolver { + return &claimResolver{ + claim: claim, + audiences: audiences, + client: client, + config: config, + verifierPerIssuer: map[string]*asyncIDTokenVerifier{}, + } } // Verifier returns either the verifier for the specified issuer, or error. -func (r *claimResolver) Verifier(iss string) (*oidc.IDTokenVerifier, error) { +func (r *claimResolver) Verifier(iss string) (*idTokenVerifier, error) { r.m.Lock() av := r.verifierPerIssuer[iss] if av == nil { // This lazy init should normally be very quick. // TODO: Make this context cancelable. ctx := oidc.ClientContext(context.Background(), r.client) - av = newAsyncIDTokenVerifier(ctx, r.config, iss) + av = newAsyncIDTokenVerifier(ctx, r.config, iss, r.audiences) r.verifierPerIssuer[iss] = av } r.m.Unlock() @@ -520,6 +551,39 @@ func (r *claimResolver) resolve(ctx context.Context, endpoint endpoint, allClaim return nil } +func (v *idTokenVerifier) Verify(ctx context.Context, rawIDToken string) (*oidc.IDToken, error) { + t, err := v.verifier.Verify(ctx, rawIDToken) + if err != nil { + return nil, err + } + if err := v.verifyAudience(t); err != nil { + return nil, err + } + return t, nil +} + +// verifyAudience verifies the audience field in the ID token matches the expected audience. +// This is added based on https://github.com/coreos/go-oidc/blob/b203e58c24394ddf5e816706a7645f01280245c7/oidc/verify.go#L275-L281 +// with the difference that we allow multiple audiences. +// +// AuthenticationConfiguration has a audienceMatchPolicy field, but the only supported value now is "MatchAny". +// So, The default match behavior is to match at least one of the audiences in the ID token. +func (v *idTokenVerifier) verifyAudience(t *oidc.IDToken) error { + // We validate audience field is not empty in the authentication configuration. + // This check ensures callers of "Verify" using idTokenVerifier are not passing + // an empty audience. + if v.audiences.Len() == 0 { + return fmt.Errorf("oidc: invalid configuration, audiences cannot be empty") + } + for _, aud := range t.Audience { + if v.audiences.Has(aud) { + return nil + } + } + + return fmt.Errorf("oidc: expected audience in %q got %q", sets.List(v.audiences), t.Audience) +} + func (a *Authenticator) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { if !hasCorrectIssuer(a.jwtAuthenticator.Issuer.URL, token) { return nil, false, nil diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go index 63ec87935f3..1cdc2831bee 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -1521,6 +1521,70 @@ func TestToken(t *testing.T) { Name: "jane", }, }, + { + name: "multiple-audiences in authentication config", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"random-client", "my-client"}, + AudienceMatchPolicy: "MatchAny", + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: pointer.String(""), + }, + }, + }, + now: func() time.Time { return now }, + }, + signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256), + pubKeys: []*jose.JSONWebKey{ + loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256), + }, + claims: fmt.Sprintf(`{ + "iss": "https://auth.example.com", + "aud": ["not-my-client", "my-client"], + "azp": "not-my-client", + "username": "jane", + "exp": %d + }`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "jane", + }, + }, + { + name: "multiple-audiences in authentication config, no match", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"random-client", "my-client"}, + AudienceMatchPolicy: "MatchAny", + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: pointer.String(""), + }, + }, + }, + now: func() time.Time { return now }, + }, + signingKey: loadRSAPrivKey(t, "testdata/rsa_1.pem", jose.RS256), + pubKeys: []*jose.JSONWebKey{ + loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256), + }, + claims: fmt.Sprintf(`{ + "iss": "https://auth.example.com", + "aud": ["not-my-client"], + "azp": "not-my-client", + "username": "jane", + "exp": %d + }`, valid.Unix()), + wantErr: `oidc: verify token: oidc: expected audience in ["my-client" "random-client"] got ["not-my-client"]`, + }, { name: "invalid-issuer", options: Options{ diff --git a/test/integration/apiserver/oidc/oidc_test.go b/test/integration/apiserver/oidc/oidc_test.go index 9f7ec3c6a36..a681ccc2ba3 100644 --- a/test/integration/apiserver/oidc/oidc_test.go +++ b/test/integration/apiserver/oidc/oidc_test.go @@ -432,6 +432,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: @@ -475,6 +477,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: @@ -522,6 +526,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: @@ -565,6 +571,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: @@ -621,6 +629,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: @@ -668,6 +678,8 @@ jwt: url: %s audiences: - %s + - another-audience + audienceMatchPolicy: MatchAny certificateAuthority: | %s claimMappings: