From c22a41e879e72ba4c925b06d8aa00e43160a0f86 Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Thu, 29 Aug 2024 17:09:07 -0700 Subject: [PATCH] Set credential-id in userinfo.extra for jwt authenticators if jti claim present Signed-off-by: Anish Ramasekar --- .../core/serviceaccount/storage/token.go | 5 +- pkg/serviceaccount/claims.go | 3 +- .../pkg/authentication/serviceaccount/util.go | 9 -- .../pkg/authentication/token/jwt/jwt.go | 26 ++++ .../pkg/authenticator/token/oidc/oidc.go | 32 ++-- .../pkg/authenticator/token/oidc/oidc_test.go | 146 ++++++++++++++++++ test/integration/apiserver/oidc/oidc_test.go | 54 +++++++ 7 files changed, 254 insertions(+), 21 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/authentication/token/jwt/jwt.go diff --git a/pkg/registry/core/serviceaccount/storage/token.go b/pkg/registry/core/serviceaccount/storage/token.go index 1e802bcb3e5..0c5f0cb1325 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/pkg/registry/core/serviceaccount/storage/token.go @@ -32,6 +32,7 @@ import ( "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/serviceaccount" + authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -203,7 +204,7 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object, } if r.maxExpirationSeconds > 0 && req.Spec.ExpirationSeconds > r.maxExpirationSeconds { - //only positive value is valid + // only positive value is valid warning.AddWarning(ctx, "", fmt.Sprintf("requested expiration of %d seconds shortened to %d seconds", req.Spec.ExpirationSeconds, r.maxExpirationSeconds)) req.Spec.ExpirationSeconds = r.maxExpirationSeconds } @@ -235,7 +236,7 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object, ExpirationTimestamp: metav1.Time{Time: nowTime.Add(time.Duration(out.Spec.ExpirationSeconds) * time.Second)}, } if utilfeature.DefaultFeatureGate.Enabled(features.ServiceAccountTokenJTI) && len(sc.ID) > 0 { - audit.AddAuditAnnotation(ctx, serviceaccount.IssuedCredentialIDAuditAnnotationKey, serviceaccount.CredentialIDForJTI(sc.ID)) + audit.AddAuditAnnotation(ctx, serviceaccount.IssuedCredentialIDAuditAnnotationKey, authenticationtokenjwt.CredentialIDForJTI(sc.ID)) } return out, nil } diff --git a/pkg/serviceaccount/claims.go b/pkg/serviceaccount/claims.go index 64feaca7ed3..1cac8b3b507 100644 --- a/pkg/serviceaccount/claims.go +++ b/pkg/serviceaccount/claims.go @@ -27,6 +27,7 @@ import ( "k8s.io/apiserver/pkg/audit" apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount" + authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/apis/core" @@ -286,6 +287,6 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims, PodUID: podUID, NodeName: nodeName, NodeUID: nodeUID, - CredentialID: apiserverserviceaccount.CredentialIDForJTI(jti), + CredentialID: authenticationtokenjwt.CredentialIDForJTI(jti), }, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go index 949b6092247..dd11efbde5c 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go @@ -163,15 +163,6 @@ func (sa *ServiceAccountInfo) UserInfo() user.Info { return info } -// CredentialIDForJTI converts a given JTI string into a credential identifier for use in a -// users 'extra' info. -func CredentialIDForJTI(jti string) string { - if len(jti) == 0 { - return "" - } - return "JTI=" + jti -} - // IsServiceAccountToken returns true if the secret is a valid api token for the service account func IsServiceAccountToken(secret *v1.Secret, sa *v1.ServiceAccount) bool { if secret.Type != v1.SecretTypeServiceAccountToken { diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/token/jwt/jwt.go b/staging/src/k8s.io/apiserver/pkg/authentication/token/jwt/jwt.go new file mode 100644 index 00000000000..17b38494931 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/authentication/token/jwt/jwt.go @@ -0,0 +1,26 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package jwt + +// CredentialIDForJTI converts a given JTI string into a credential identifier for use in a +// users 'extra' info. +func CredentialIDForJTI(jti string) string { + if len(jti) == 0 { + return "" + } + return "JTI=" + jti +} 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 bdf50d89c3e..226ea22dec5 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 @@ -56,6 +56,7 @@ import ( apiservervalidation "k8s.io/apiserver/pkg/apis/apiserver/validation" "k8s.io/apiserver/pkg/authentication/authenticator" authenticationcel "k8s.io/apiserver/pkg/authentication/cel" + authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt" "k8s.io/apiserver/pkg/authentication/user" certutil "k8s.io/client-go/util/cert" "k8s.io/klog/v2" @@ -726,7 +727,7 @@ func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string) return nil, false, err } - extra, err := a.getExtra(ctx, claimsUnstructured) + extra, err := a.getExtra(ctx, c, claimsUnstructured) if err != nil { return nil, false, err } @@ -914,17 +915,21 @@ func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructu return evalResult.EvalResult.Value().(string), nil } -func (a *jwtAuthenticator) getExtra(ctx context.Context, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) { +func (a *jwtAuthenticator) getExtra(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) { + extra := make(map[string][]string) + + if credentialID := getCredentialID(c); len(credentialID) > 0 { + extra[user.CredentialIDKey] = []string{credentialID} + } + if a.celMapper.Extra == nil { - return nil, nil + return extra, nil } evalResult, err := a.celMapper.Extra.EvalClaimMappings(ctx, claimsUnstructured) if err != nil { return nil, err } - - extra := make(map[string][]string, len(evalResult)) for _, result := range evalResult { extraMapping, ok := result.ExpressionAccessor.(*authenticationcel.ExtraMappingExpression) if !ok { @@ -936,16 +941,25 @@ func (a *jwtAuthenticator) getExtra(ctx context.Context, claimsUnstructured *uns return nil, fmt.Errorf("oidc: error evaluating extra claim expression: %s: %w", extraMapping.Expression, err) } - if len(extraValues) == 0 { - continue + if len(extraValues) > 0 { + extra[extraMapping.Key] = extraValues } - - extra[extraMapping.Key] = extraValues } return extra, nil } +func getCredentialID(c claims) string { + if _, ok := c["jti"]; ok { + var jti string + if err := c.unmarshalClaim("jti", &jti); err == nil { + return authenticationtokenjwt.CredentialIDForJTI(jti) + } + } + + return "" +} + // getClaimJWT gets a distributed claim JWT from url, using the supplied access // token as bearer token. If the access token is "", the authorization header // will not be set. 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 986d79b1056..b8ff716c3a4 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 @@ -3356,6 +3356,152 @@ func TestToken(t *testing.T) { Name: "jane", }, }, + { + name: "credential id set in extra even when no extra claim mappings are defined", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: "claims.username", + }, + }, + }, + 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": "my-client", + "username": "jane", + "exp": %d, + "jti": "1234" + }`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "jane", + Extra: map[string][]string{ + user.CredentialIDKey: {"JTI=1234"}, + }, + }, + }, + { + name: "credential id set in extra when extra claim mappings are defined", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: "claims.username", + }, + Extra: []apiserver.ExtraMapping{ + { + Key: "example.org/foo", + ValueExpression: "claims.foo", + }, + { + Key: "example.org/bar", + ValueExpression: "claims.bar", + }, + }, + }, + }, + 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": "my-client", + "username": "jane", + "exp": %d, + "jti": "1234", + "foo": "bar", + "bar": [ + "baz", + "qux" + ] + }`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "jane", + Extra: map[string][]string{ + user.CredentialIDKey: {"JTI=1234"}, + "example.org/foo": {"bar"}, + "example.org/bar": {"baz", "qux"}, + }, + }, + }, + { + name: "non-string jti claim does not set credential id in extra or error", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: "claims.username", + }, + }, + }, + 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": "my-client", + "username": "jane", + "exp": %d, + "jti": 1234 + }`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "jane", + }, + }, + { + name: "missing jti claim does not set credential id in extra or error", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Expression: "claims.username", + }, + }, + }, + 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": "my-client", + "username": "jane", + "exp": %d + }`, valid.Unix()), + want: &user.DefaultInfo{ + Name: "jane", + }, + }, } var successTestCount, failureTestCount int diff --git a/test/integration/apiserver/oidc/oidc_test.go b/test/integration/apiserver/oidc/oidc_test.go index d8765b80e8c..c4c582051c9 100644 --- a/test/integration/apiserver/oidc/oidc_test.go +++ b/test/integration/apiserver/oidc/oidc_test.go @@ -629,6 +629,7 @@ jwt: "sub": defaultOIDCClaimedUsername, "aud": defaultOIDCClientID, "exp": time.Now().Add(idTokenLifetime).Unix(), + "jti": "0123456789", }, defaultStubAccessToken, defaultStubRefreshToken, @@ -641,6 +642,10 @@ jwt: wantUser: &authenticationv1.UserInfo{ Username: "k8s-john_doe", Groups: []string{"system:authenticated"}, + Extra: map[string]authenticationv1.ExtraValue{ + // validates credential id is set correctly when jti claim is present + "authentication.kubernetes.io/credential-id": {"JTI=0123456789"}, + }, }, }, { @@ -777,6 +782,7 @@ jwt: "aud": defaultOIDCClientID, "exp": time.Now().Add(idTokenLifetime).Unix(), "baz": "qux", + "jti": "0123456789", }, defaultStubAccessToken, defaultStubRefreshToken, @@ -790,6 +796,8 @@ jwt: Username: "k8s-john_doe", Groups: []string{"system:authenticated"}, Extra: map[string]authenticationv1.ExtraValue{ + // validates credential id is set correctly and other extra fields are set + "authentication.kubernetes.io/credential-id": {"JTI=0123456789"}, "example.org/foo": {"bar"}, "example.org/baz": {"qux"}, }, @@ -945,6 +953,52 @@ jwt: UID: "1234", }, }, + { + name: "non-string jti claim doesn't result in authentication error", + authConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + configureInfrastructure: configureTestInfrastructure[*rsa.PrivateKey, *rsa.PublicKey], + configureOIDCServerBehaviour: func(t *testing.T, oidcServer *utilsoidc.TestServer, signingPrivateKey *rsa.PrivateKey) { + idTokenLifetime := time.Second * 1200 + oidcServer.TokenHandler().EXPECT().Token().RunAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT( + t, + signingPrivateKey, + map[string]interface{}{ + "iss": oidcServer.URL(), + "sub": defaultOIDCClaimedUsername, + "aud": defaultOIDCClientID, + "exp": time.Now().Add(idTokenLifetime).Unix(), + "jti": 1234, + }, + defaultStubAccessToken, + defaultStubRefreshToken, + )).Times(1) + }, + configureClient: configureClientFetchingOIDCCredentials, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + wantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + }, } for _, tt := range tests {