Merge pull request kubernetes#125177 from liggitt/dynamic-public-key

Move public key serviceaccount getter to interface, filter by key id

Author: k8s-ci-robot

pkg/controlplane/apiserver/config.go

@@ -94,9 +94,9 @@ type Extra struct {
 	ExtendExpiration            bool
 
 	// ServiceAccountIssuerDiscovery
-	ServiceAccountIssuerURL  string
-	ServiceAccountJWKSURI    string
-	ServiceAccountPublicKeys []interface{}
+	ServiceAccountIssuerURL        string
+	ServiceAccountJWKSURI          string
+	ServiceAccountPublicKeysGetter serviceaccount.PublicKeysGetter
 
 	SystemNamespaces []string
 
@@ -368,18 +368,24 @@ func CreateConfig(
 		return nil, nil, fmt.Errorf("failed to apply admission: %w", err)
 	}
 
-	// Load and set the public keys.
-	var pubKeys []interface{}
-	for _, f := range opts.Authentication.ServiceAccounts.KeyFiles {
-		keys, err := keyutil.PublicKeysFromFile(f)
+	if len(opts.Authentication.ServiceAccounts.KeyFiles) > 0 {
+		// Load and set the public keys.
+		var pubKeys []interface{}
+		for _, f := range opts.Authentication.ServiceAccounts.KeyFiles {
+			keys, err := keyutil.PublicKeysFromFile(f)
+			if err != nil {
+				return nil, nil, fmt.Errorf("failed to parse key file %q: %w", f, err)
+			}
+			pubKeys = append(pubKeys, keys...)
+		}
+		keysGetter, err := serviceaccount.StaticPublicKeysGetter(pubKeys)
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to parse key file %q: %w", f, err)
+			return nil, nil, fmt.Errorf("failed to set up public service account keys: %w", err)
 		}
-		pubKeys = append(pubKeys, keys...)
+		config.ServiceAccountPublicKeysGetter = keysGetter
 	}
 	config.ServiceAccountIssuerURL = opts.Authentication.ServiceAccounts.Issuers[0]
 	config.ServiceAccountJWKSURI = opts.Authentication.ServiceAccounts.JWKSURI
-	config.ServiceAccountPublicKeys = pubKeys
 
 	return config, genericInitializers, nil
 }

pkg/controlplane/apiserver/server.go

@@ -93,13 +93,11 @@ func (c completedConfig) New(name string, delegationTarget genericapiserver.Dele
 		routes.Logs{}.Install(generic.Handler.GoRestfulContainer)
 	}
 
-	// Metadata and keys are expected to only change across restarts at present,
-	// so we just marshal immediately and serve the cached JSON bytes.
-	md, err := serviceaccount.NewOpenIDMetadata(
+	md, err := serviceaccount.NewOpenIDMetadataProvider(
 		c.ServiceAccountIssuerURL,
 		c.ServiceAccountJWKSURI,
 		c.Generic.ExternalAddress,
-		c.ServiceAccountPublicKeys,
+		c.ServiceAccountPublicKeysGetter,
 	)
 	if err != nil {
 		// If there was an error, skip installing the endpoints and log the
@@ -120,8 +118,7 @@ func (c completedConfig) New(name string, delegationTarget genericapiserver.Dele
 			klog.Info(msg)
 		}
 	} else {
-		routes.NewOpenIDMetadataServer(md.ConfigJSON, md.PublicKeysetJSON).
-			Install(generic.Handler.GoRestfulContainer)
+		routes.NewOpenIDMetadataServer(md).Install(generic.Handler.GoRestfulContainer)
 	}
 
 	s := &Server{

pkg/kubeapiserver/authenticator/config.go

@@ -62,7 +62,6 @@ type Config struct {
 	AuthenticationConfig        *apiserver.AuthenticationConfiguration
 	AuthenticationConfigData    string
 	OIDCSigningAlgs             []string
-	ServiceAccountKeyFiles      []string
 	ServiceAccountLookup        bool
 	ServiceAccountIssuers       []string
 	APIAudiences                authenticator.Audiences
@@ -79,7 +78,9 @@ type Config struct {
 
 	RequestHeaderConfig *authenticatorfactory.RequestHeaderConfig
 
-	// TODO, this is the only non-serializable part of the entire config.  Factor it out into a clientconfig
+	// ServiceAccountPublicKeysGetter returns public keys for verifying service account tokens.
+	ServiceAccountPublicKeysGetter serviceaccount.PublicKeysGetter
+	// ServiceAccountTokenGetter fetches API objects used to verify bound objects in service account token claims.
 	ServiceAccountTokenGetter   serviceaccount.ServiceAccountTokenGetter
 	SecretsWriter               typedv1core.SecretsGetter
 	BootstrapTokenAuthenticator authenticator.Token
@@ -127,15 +128,15 @@ func (config Config) New(serverLifecycle context.Context) (authenticator.Request
 		}
 		tokenAuthenticators = append(tokenAuthenticators, authenticator.WrapAudienceAgnosticToken(config.APIAudiences, tokenAuth))
 	}
-	if len(config.ServiceAccountKeyFiles) > 0 {
-		serviceAccountAuth, err := newLegacyServiceAccountAuthenticator(config.ServiceAccountKeyFiles, config.ServiceAccountLookup, config.APIAudiences, config.ServiceAccountTokenGetter, config.SecretsWriter)
+	if config.ServiceAccountPublicKeysGetter != nil {
+		serviceAccountAuth, err := newLegacyServiceAccountAuthenticator(config.ServiceAccountPublicKeysGetter, config.ServiceAccountLookup, config.APIAudiences, config.ServiceAccountTokenGetter, config.SecretsWriter)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
 		tokenAuthenticators = append(tokenAuthenticators, serviceAccountAuth)
 	}
 	if len(config.ServiceAccountIssuers) > 0 {
-		serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountIssuers, config.ServiceAccountKeyFiles, config.APIAudiences, config.ServiceAccountTokenGetter)
+		serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountIssuers, config.ServiceAccountPublicKeysGetter, config.APIAudiences, config.ServiceAccountTokenGetter)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
@@ -338,36 +339,25 @@ func newAuthenticatorFromTokenFile(tokenAuthFile string) (authenticator.Token, e
 }
 
 // newLegacyServiceAccountAuthenticator returns an authenticator.Token or an error
-func newLegacyServiceAccountAuthenticator(keyfiles []string, lookup bool, apiAudiences authenticator.Audiences, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter, secretsWriter typedv1core.SecretsGetter) (authenticator.Token, error) {
-	allPublicKeys := []interface{}{}
-	for _, keyfile := range keyfiles {
-		publicKeys, err := keyutil.PublicKeysFromFile(keyfile)
-		if err != nil {
-			return nil, err
-		}
-		allPublicKeys = append(allPublicKeys, publicKeys...)
+func newLegacyServiceAccountAuthenticator(publicKeysGetter serviceaccount.PublicKeysGetter, lookup bool, apiAudiences authenticator.Audiences, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter, secretsWriter typedv1core.SecretsGetter) (authenticator.Token, error) {
+	if publicKeysGetter == nil {
+		return nil, fmt.Errorf("no public key getter provided")
 	}
 	validator, err := serviceaccount.NewLegacyValidator(lookup, serviceAccountGetter, secretsWriter)
 	if err != nil {
 		return nil, fmt.Errorf("while creating legacy validator, err: %w", err)
 	}
 
-	tokenAuthenticator := serviceaccount.JWTTokenAuthenticator([]string{serviceaccount.LegacyIssuer}, allPublicKeys, apiAudiences, validator)
+	tokenAuthenticator := serviceaccount.JWTTokenAuthenticator([]string{serviceaccount.LegacyIssuer}, publicKeysGetter, apiAudiences, validator)
 	return tokenAuthenticator, nil
 }
 
 // newServiceAccountAuthenticator returns an authenticator.Token or an error
-func newServiceAccountAuthenticator(issuers []string, keyfiles []string, apiAudiences authenticator.Audiences, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Token, error) {
-	allPublicKeys := []interface{}{}
-	for _, keyfile := range keyfiles {
-		publicKeys, err := keyutil.PublicKeysFromFile(keyfile)
-		if err != nil {
-			return nil, err
-		}
-		allPublicKeys = append(allPublicKeys, publicKeys...)
+func newServiceAccountAuthenticator(issuers []string, publicKeysGetter serviceaccount.PublicKeysGetter, apiAudiences authenticator.Audiences, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Token, error) {
+	if publicKeysGetter == nil {
+		return nil, fmt.Errorf("no public key getter provided")
 	}
-
-	tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(issuers, allPublicKeys, apiAudiences, serviceaccount.NewValidator(serviceAccountGetter))
+	tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(issuers, publicKeysGetter, apiAudiences, serviceaccount.NewValidator(serviceAccountGetter))
 	return tokenAuthenticator, nil
 }
 

pkg/kubeapiserver/options/authentication.go

@@ -47,13 +47,15 @@ import (
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	v1listers "k8s.io/client-go/listers/core/v1"
+	"k8s.io/client-go/util/keyutil"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/klog/v2"
 	openapicommon "k8s.io/kube-openapi/pkg/common"
 	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/features"
 	kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
 	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util/filesystem"
 	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/bootstrap"
 	"k8s.io/utils/pointer"
@@ -559,7 +561,21 @@ func (o *BuiltInAuthenticationOptions) ToAuthenticationConfig() (kubeauthenticat
 		if len(o.ServiceAccounts.Issuers) != 0 && len(o.APIAudiences) == 0 {
 			ret.APIAudiences = authenticator.Audiences(o.ServiceAccounts.Issuers)
 		}
-		ret.ServiceAccountKeyFiles = o.ServiceAccounts.KeyFiles
+		if len(o.ServiceAccounts.KeyFiles) > 0 {
+			allPublicKeys := []interface{}{}
+			for _, keyfile := range o.ServiceAccounts.KeyFiles {
+				publicKeys, err := keyutil.PublicKeysFromFile(keyfile)
+				if err != nil {
+					return kubeauthenticator.Config{}, err
+				}
+				allPublicKeys = append(allPublicKeys, publicKeys...)
+			}
+			keysGetter, err := serviceaccount.StaticPublicKeysGetter(allPublicKeys)
+			if err != nil {
+				return kubeauthenticator.Config{}, fmt.Errorf("failed to set up public service account keys: %w", err)
+			}
+			ret.ServiceAccountPublicKeysGetter = keysGetter
+		}
 		ret.ServiceAccountIssuers = o.ServiceAccounts.Issuers
 		ret.ServiceAccountLookup = o.ServiceAccounts.Lookup
 	}

pkg/routes/openidmetadata.go

@@ -17,6 +17,7 @@ limitations under the License.
 package routes
 
 import (
+	"fmt"
 	"net/http"
 
 	restful "github.com/emicklei/go-restful/v3"
@@ -34,26 +35,23 @@ const (
 	// cacheControl is the value of the Cache-Control header. Overrides the
 	// global `private, no-cache` setting.
 	headerCacheControl = "Cache-Control"
-	cacheControl       = "public, max-age=3600" // 1 hour
+
+	cacheControlTemplate = "public, max-age=%d"
 
 	// mimeJWKS is the content type of the keyset response
 	mimeJWKS = "application/jwk-set+json"
 )
 
 // OpenIDMetadataServer is an HTTP server for metadata of the KSA token issuer.
 type OpenIDMetadataServer struct {
-	configJSON []byte
-	keysetJSON []byte
+	provider serviceaccount.OpenIDMetadataProvider
 }
 
 // NewOpenIDMetadataServer creates a new OpenIDMetadataServer.
 // The issuer is the OIDC issuer; keys are the keys that may be used to sign
 // KSA tokens.
-func NewOpenIDMetadataServer(configJSON, keysetJSON []byte) *OpenIDMetadataServer {
-	return &OpenIDMetadataServer{
-		configJSON: configJSON,
-		keysetJSON: keysetJSON,
-	}
+func NewOpenIDMetadataServer(provider serviceaccount.OpenIDMetadataProvider) *OpenIDMetadataServer {
+	return &OpenIDMetadataServer{provider: provider}
 }
 
 // Install adds this server to the request router c.
@@ -95,19 +93,21 @@ func fromStandard(h http.HandlerFunc) restful.RouteFunction {
 }
 
 func (s *OpenIDMetadataServer) serveConfiguration(w http.ResponseWriter, req *http.Request) {
+	configJSON, maxAge := s.provider.GetConfigJSON()
 	w.Header().Set(restful.HEADER_ContentType, restful.MIME_JSON)
-	w.Header().Set(headerCacheControl, cacheControl)
-	if _, err := w.Write(s.configJSON); err != nil {
+	w.Header().Set(headerCacheControl, fmt.Sprintf(cacheControlTemplate, maxAge))
+	if _, err := w.Write(configJSON); err != nil {
 		klog.Errorf("failed to write service account issuer metadata response: %v", err)
 		return
 	}
 }
 
 func (s *OpenIDMetadataServer) serveKeys(w http.ResponseWriter, req *http.Request) {
+	keysetJSON, maxAge := s.provider.GetKeysetJSON()
 	// Per RFC7517 : https://tools.ietf.org/html/rfc7517#section-8.5.1
 	w.Header().Set(restful.HEADER_ContentType, mimeJWKS)
-	w.Header().Set(headerCacheControl, cacheControl)
-	if _, err := w.Write(s.keysetJSON); err != nil {
+	w.Header().Set(headerCacheControl, fmt.Sprintf(cacheControlTemplate, maxAge))
+	if _, err := w.Write(keysetJSON); err != nil {
 		klog.Errorf("failed to write service account issuer JWKS response: %v", err)
 		return
 	}

pkg/serviceaccount/jwt.go

@@ -225,22 +225,97 @@ func (j *jwtTokenGenerator) GenerateToken(claims *jwt.Claims, privateClaims inte
 // JWTTokenAuthenticator authenticates tokens as JWT tokens produced by JWTTokenGenerator
 // Token signatures are verified using each of the given public keys until one works (allowing key rotation)
 // If lookup is true, the service account and secret referenced as claims inside the token are retrieved and verified with the provided ServiceAccountTokenGetter
-func JWTTokenAuthenticator[PrivateClaims any](issuers []string, keys []interface{}, implicitAuds authenticator.Audiences, validator Validator[PrivateClaims]) authenticator.Token {
+func JWTTokenAuthenticator[PrivateClaims any](issuers []string, publicKeysGetter PublicKeysGetter, implicitAuds authenticator.Audiences, validator Validator[PrivateClaims]) authenticator.Token {
 	issuersMap := make(map[string]bool)
 	for _, issuer := range issuers {
 		issuersMap[issuer] = true
 	}
 	return &jwtTokenAuthenticator[PrivateClaims]{
 		issuers:      issuersMap,
-		keys:         keys,
+		keysGetter:   publicKeysGetter,
 		implicitAuds: implicitAuds,
 		validator:    validator,
 	}
 }
 
+// Listener is an interface to use to notify interested parties of a change.
+type Listener interface {
+	// Enqueue should be called when an input may have changed
+	Enqueue()
+}
+
+// PublicKeysGetter returns public keys for a given key id.
+type PublicKeysGetter interface {
+	// AddListener is adds a listener to be notified of potential input changes.
+	// This is a noop on static providers.
+	AddListener(listener Listener)
+
+	// GetCacheAgeMaxSeconds returns the seconds a call to GetPublicKeys() can be cached for.
+	// If the results of GetPublicKeys() can be dynamic, this means a new key must be included in the results
+	// for at least this long before it is used to sign new tokens.
+	GetCacheAgeMaxSeconds() int
+
+	// GetPublicKeys returns public keys to use for verifying a token with the given key id.
+	// keyIDHint may be empty if the token did not have a kid header, or if all public keys are desired.
+	GetPublicKeys(keyIDHint string) []PublicKey
+}
+
+type PublicKey struct {
+	KeyID     string
+	PublicKey interface{}
+}
+
+type staticPublicKeysGetter struct {
+	allPublicKeys  []PublicKey
+	publicKeysByID map[string][]PublicKey
+}
+
+// StaticPublicKeysGetter constructs an implementation of PublicKeysGetter
+// which returns all public keys when key id is unspecified, and returns
+// the public keys matching the keyIDFromPublicKey-derived key id when
+// a key id is specified.
+func StaticPublicKeysGetter(keys []interface{}) (PublicKeysGetter, error) {
+	allPublicKeys := []PublicKey{}
+	publicKeysByID := map[string][]PublicKey{}
+	for _, key := range keys {
+		if privateKey, isPrivateKey := key.(publicKeyGetter); isPrivateKey {
+			// This is a private key. Extract its public key.
+			key = privateKey.Public()
+		}
+
+		keyID, err := keyIDFromPublicKey(key)
+		if err != nil {
+			return nil, err
+		}
+		pk := PublicKey{PublicKey: key, KeyID: keyID}
+		publicKeysByID[keyID] = append(publicKeysByID[keyID], pk)
+		allPublicKeys = append(allPublicKeys, pk)
+	}
+	return &staticPublicKeysGetter{
+		allPublicKeys:  allPublicKeys,
+		publicKeysByID: publicKeysByID,
+	}, nil
+}
+
+func (s staticPublicKeysGetter) AddListener(listener Listener) {
+	// no-op, static key content never changes
+}
+
+func (s staticPublicKeysGetter) GetCacheAgeMaxSeconds() int {
+	// hard-coded to match cache max-age set in OIDC discovery
+	return 3600
+}
+
+func (s staticPublicKeysGetter) GetPublicKeys(keyID string) []PublicKey {
+	if len(keyID) == 0 {
+		return s.allPublicKeys
+	}
+	return s.publicKeysByID[keyID]
+}
+
 type jwtTokenAuthenticator[PrivateClaims any] struct {
 	issuers      map[string]bool
-	keys         []interface{}
+	keysGetter   PublicKeysGetter
 	validator    Validator[PrivateClaims]
 	implicitAuds authenticator.Audiences
 }
@@ -269,13 +344,25 @@ func (j *jwtTokenAuthenticator[PrivateClaims]) AuthenticateToken(ctx context.Con
 	public := &jwt.Claims{}
 	private := new(PrivateClaims)
 
-	// TODO: Pick the key that has the same key ID as `tok`, if one exists.
+	// Pick the key that has the same key ID as `tok`, if one exists.
+	var kid string
+	for _, header := range tok.Headers {
+		if header.KeyID != "" {
+			kid = header.KeyID
+			break
+		}
+	}
+
 	var (
 		found   bool
 		errlist []error
 	)
-	for _, key := range j.keys {
-		if err := tok.Claims(key, public, private); err != nil {
+	keys := j.keysGetter.GetPublicKeys(kid)
+	if len(keys) == 0 {
+		return nil, false, fmt.Errorf("invalid signature, no keys found")
+	}
+	for _, key := range keys {
+		if err := tok.Claims(key.PublicKey, public, private); err != nil {
 			errlist = append(errlist, err)
 			continue
 		}