Merge pull request #63653 from WanLinghao/token_expiry_limit

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add limit to the TokenRequest expiration time

**What this PR does / why we need it**:
A new API TokenRequest has been implemented.It improves current serviceaccount model from many ways.
This patch adds limit to TokenRequest expiration time.


**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #63575

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

cmd/kube-apiserver/app/server.go

@@ -326,8 +326,12 @@ func CreateKubeAPIServerConfig(
 		return
 	}
 
-	var issuer serviceaccount.TokenGenerator
+	var (
-	var apiAudiences []string
+		issuer        serviceaccount.TokenGenerator
+		apiAudiences  []string
+		maxExpiration time.Duration
+	)
+
 	if s.ServiceAccountSigningKeyFile != "" ||
 		s.Authentication.ServiceAccounts.Issuer != "" ||
 		len(s.Authentication.ServiceAccounts.APIAudiences) > 0 {
@@ -347,8 +351,19 @@ func CreateKubeAPIServerConfig(
 			lastErr = fmt.Errorf("failed to parse service-account-issuer-key-file: %v", err)
 			return
 		}
+		if s.Authentication.ServiceAccounts.MaxExpiration != 0 {
+			lowBound := time.Hour
+			upBound := time.Duration(1<<32) * time.Second
+			if s.Authentication.ServiceAccounts.MaxExpiration < lowBound ||
+				s.Authentication.ServiceAccounts.MaxExpiration > upBound {
+				lastErr = fmt.Errorf("the serviceaccount max expiration is out of range, must be between 1 hour to 2^32 seconds")
+				return
+			}
+		}
+
 		issuer = serviceaccount.JWTTokenGenerator(s.Authentication.ServiceAccounts.Issuer, sk)
 		apiAudiences = s.Authentication.ServiceAccounts.APIAudiences
+		maxExpiration = s.Authentication.ServiceAccounts.MaxExpiration
 	}
 
 	config = &master.Config{
@@ -382,8 +397,9 @@ func CreateKubeAPIServerConfig(
 			EndpointReconcilerType: reconcilers.Type(s.EndpointReconcilerType),
 			MasterCount:            s.MasterCount,
 
-			ServiceAccountIssuer:       issuer,
+			ServiceAccountIssuer:        issuer,
-			ServiceAccountAPIAudiences: apiAudiences,
+			ServiceAccountAPIAudiences:  apiAudiences,
+			ServiceAccountMaxExpiration: maxExpiration,
 		},
 	}
 

pkg/kubeapiserver/options/authentication.go

@@ -73,10 +73,11 @@ type PasswordFileAuthenticationOptions struct {
 }
 
 type ServiceAccountAuthenticationOptions struct {
-	KeyFiles     []string
+	KeyFiles      []string
-	Lookup       bool
+	Lookup        bool
-	Issuer       string
+	Issuer        string
-	APIAudiences []string
+	APIAudiences  []string
+	MaxExpiration time.Duration
 }
 
 type TokenFileAuthenticationOptions struct {
@@ -260,6 +261,10 @@ func (s *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) {
 		fs.StringSliceVar(&s.ServiceAccounts.APIAudiences, "service-account-api-audiences", s.ServiceAccounts.APIAudiences, ""+
 			"Identifiers of the API. The service account token authenticator will validate that "+
 			"tokens used against the API are bound to at least one of these audiences.")
+
+		fs.DurationVar(&s.ServiceAccounts.MaxExpiration, "service-account-max-token-expiration", s.ServiceAccounts.MaxExpiration, ""+
+			"The maximum validity duration of a token created by the service account token issuer. If an otherwise valid "+
+			"TokenRequest with a validity duration larger than this value is requested, a token will be issued with a validity duration of this value.")
 	}
 
 	if s.TokenFile != nil {

pkg/master/master.go

@@ -163,8 +163,9 @@ type ExtraConfig struct {
 	// Selects which reconciler to use
 	EndpointReconcilerType reconcilers.Type
 
-	ServiceAccountIssuer       serviceaccount.TokenGenerator
+	ServiceAccountIssuer        serviceaccount.TokenGenerator
-	ServiceAccountAPIAudiences []string
+	ServiceAccountAPIAudiences  []string
+	ServiceAccountMaxExpiration time.Duration
 }
 
 type Config struct {
@@ -318,15 +319,16 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 	// install legacy rest storage
 	if c.ExtraConfig.APIResourceConfigSource.VersionEnabled(apiv1.SchemeGroupVersion) {
 		legacyRESTStorageProvider := corerest.LegacyRESTStorageProvider{
-			StorageFactory:             c.ExtraConfig.StorageFactory,
+			StorageFactory:              c.ExtraConfig.StorageFactory,
-			ProxyTransport:             c.ExtraConfig.ProxyTransport,
+			ProxyTransport:              c.ExtraConfig.ProxyTransport,
-			KubeletClientConfig:        c.ExtraConfig.KubeletClientConfig,
+			KubeletClientConfig:         c.ExtraConfig.KubeletClientConfig,
-			EventTTL:                   c.ExtraConfig.EventTTL,
+			EventTTL:                    c.ExtraConfig.EventTTL,
-			ServiceIPRange:             c.ExtraConfig.ServiceIPRange,
+			ServiceIPRange:              c.ExtraConfig.ServiceIPRange,
-			ServiceNodePortRange:       c.ExtraConfig.ServiceNodePortRange,
+			ServiceNodePortRange:        c.ExtraConfig.ServiceNodePortRange,
-			LoopbackClientConfig:       c.GenericConfig.LoopbackClientConfig,
+			LoopbackClientConfig:        c.GenericConfig.LoopbackClientConfig,
-			ServiceAccountIssuer:       c.ExtraConfig.ServiceAccountIssuer,
+			ServiceAccountIssuer:        c.ExtraConfig.ServiceAccountIssuer,
-			ServiceAccountAPIAudiences: c.ExtraConfig.ServiceAccountAPIAudiences,
+			ServiceAccountAPIAudiences:  c.ExtraConfig.ServiceAccountAPIAudiences,
+			ServiceAccountMaxExpiration: c.ExtraConfig.ServiceAccountMaxExpiration,
 		}
 		m.InstallLegacyAPI(&c, c.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider)
 	}

pkg/registry/core/rest/storage_core.go

@@ -79,8 +79,9 @@ type LegacyRESTStorageProvider struct {
 	ServiceIPRange       net.IPNet
 	ServiceNodePortRange utilnet.PortRange
 
-	ServiceAccountIssuer       serviceaccount.TokenGenerator
+	ServiceAccountIssuer        serviceaccount.TokenGenerator
-	ServiceAccountAPIAudiences []string
+	ServiceAccountAPIAudiences  []string
+	ServiceAccountMaxExpiration time.Duration
 
 	LoopbackClientConfig *restclient.Config
 }
@@ -141,9 +142,9 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi
 
 	var serviceAccountStorage *serviceaccountstore.REST
 	if c.ServiceAccountIssuer != nil && utilfeature.DefaultFeatureGate.Enabled(features.TokenRequest) {
-		serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.ServiceAccountAPIAudiences, podStorage.Pod.Store, secretStorage.Store)
+		serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.ServiceAccountAPIAudiences, c.ServiceAccountMaxExpiration, podStorage.Pod.Store, secretStorage.Store)
 	} else {
-		serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, nil, nil)
+		serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 0, nil, nil)
 	}
 
 	serviceRESTStorage, serviceStatusStorage := servicestore.NewGenericREST(restOptionsGetter)

pkg/registry/core/serviceaccount/storage/storage.go

@@ -17,6 +17,8 @@ limitations under the License.
 package storage
 
 import (
+	"time"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
@@ -35,7 +37,7 @@ type REST struct {
 }
 
 // NewREST returns a RESTStorage object that will work against service accounts.
-func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds []string, podStorage, secretStorage *genericregistry.Store) *REST {
+func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds []string, max time.Duration, podStorage, secretStorage *genericregistry.Store) *REST {
 	store := &genericregistry.Store{
 		NewFunc:                  func() runtime.Object { return &api.ServiceAccount{} },
 		NewListFunc:              func() runtime.Object { return &api.ServiceAccountList{} },
@@ -56,11 +58,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator,
 	var trest *TokenREST
 	if issuer != nil && podStorage != nil && secretStorage != nil {
 		trest = &TokenREST{
-			svcaccts: store,
+			svcaccts:             store,
-			pods:     podStorage,
+			pods:                 podStorage,
-			secrets:  secretStorage,
+			secrets:              secretStorage,
-			issuer:   issuer,
+			issuer:               issuer,
-			auds:     auds,
+			auds:                 auds,
+			maxExpirationSeconds: int64(max.Seconds()),
 		}
 	}
 

pkg/registry/core/serviceaccount/storage/storage_test.go

@@ -38,7 +38,7 @@ func newStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) {
 		DeleteCollectionWorkers: 1,
 		ResourcePrefix:          "serviceaccounts",
 	}
-	return NewREST(restOptions, nil, nil, nil, nil), server
+	return NewREST(restOptions, nil, nil, 0, nil, nil), server
 }
 
 func validNewServiceAccount(name string) *api.ServiceAccount {

pkg/registry/core/serviceaccount/storage/token.go

@@ -39,11 +39,12 @@ func (r *TokenREST) New() runtime.Object {
 }
 
 type TokenREST struct {
-	svcaccts getter
+	svcaccts             getter
-	pods     getter
+	pods                 getter
-	secrets  getter
+	secrets              getter
-	issuer   token.TokenGenerator
+	issuer               token.TokenGenerator
-	auds     []string
+	auds                 []string
+	maxExpirationSeconds int64
 }
 
 var _ = rest.NamedCreater(&TokenREST{})
@@ -111,6 +112,12 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object,
 	if len(out.Spec.Audiences) == 0 {
 		out.Spec.Audiences = r.auds
 	}
+
+	if r.maxExpirationSeconds > 0 && out.Spec.ExpirationSeconds > r.maxExpirationSeconds {
+		//only positive value is valid
+		out.Spec.ExpirationSeconds = r.maxExpirationSeconds
+	}
+
 	sc, pc := token.Claims(*svcacct, pod, secret, out.Spec.ExpirationSeconds, out.Spec.Audiences)
 	tokdata, err := r.issuer.GenerateToken(sc, pc)
 	if err != nil {

test/integration/auth/svcaccttoken_test.go

@@ -20,6 +20,7 @@ import (
 	"crypto/ecdsa"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -63,6 +64,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 	const iss = "https://foo.bar.example.com"
 	aud := []string{"api"}
 
+	maxExpirationSeconds := int64(60 * 60)
+	maxExpirationDuration, err := time.ParseDuration(fmt.Sprintf("%ds", maxExpirationSeconds))
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
 	gcs := &clientset.Clientset{}
 
 	// Start the server
@@ -77,6 +84,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 	)
 	masterConfig.ExtraConfig.ServiceAccountIssuer = serviceaccount.JWTTokenGenerator(iss, sk)
 	masterConfig.ExtraConfig.ServiceAccountAPIAudiences = aud
+	masterConfig.ExtraConfig.ServiceAccountMaxExpiration = maxExpirationDuration
 
 	master, _, closeFn := framework.RunAMaster(masterConfig)
 	defer closeFn()
@@ -438,6 +446,94 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 
 		doTokenReview(t, cs, treq, true)
 	})
+
+	t.Run("a token request within expiration time", func(t *testing.T) {
+		normalExpirationTime := maxExpirationSeconds - 10*60
+		treq := &authenticationv1.TokenRequest{
+			Spec: authenticationv1.TokenRequestSpec{
+				Audiences:         []string{"api"},
+				ExpirationSeconds: &normalExpirationTime,
+				BoundObjectRef: &authenticationv1.BoundObjectReference{
+					Kind:       "Secret",
+					APIVersion: "v1",
+					Name:       secret.Name,
+					UID:        secret.UID,
+				},
+			},
+		}
+
+		sa, del := createDeleteSvcAcct(t, cs, sa)
+		defer del()
+
+		originalSecret, originalDelSecret := createDeleteSecret(t, cs, secret)
+		defer originalDelSecret()
+
+		treq.Spec.BoundObjectRef.UID = originalSecret.UID
+		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
+		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
+		checkPayload(t, treq.Status.Token, `null`, "kubernetes.io", "pod")
+		checkPayload(t, treq.Status.Token, `"test-secret"`, "kubernetes.io", "secret", "name")
+		checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace")
+		checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name")
+		checkExpiration(t, treq, normalExpirationTime)
+
+		doTokenReview(t, cs, treq, false)
+		originalDelSecret()
+		doTokenReview(t, cs, treq, true)
+
+		_, recreateDelSecret := createDeleteSecret(t, cs, secret)
+		defer recreateDelSecret()
+
+		doTokenReview(t, cs, treq, true)
+	})
+
+	t.Run("a token request with out-of-range expiration", func(t *testing.T) {
+		tooLongExpirationTime := maxExpirationSeconds + 10*60
+		treq := &authenticationv1.TokenRequest{
+			Spec: authenticationv1.TokenRequestSpec{
+				Audiences:         []string{"api"},
+				ExpirationSeconds: &tooLongExpirationTime,
+				BoundObjectRef: &authenticationv1.BoundObjectReference{
+					Kind:       "Secret",
+					APIVersion: "v1",
+					Name:       secret.Name,
+					UID:        secret.UID,
+				},
+			},
+		}
+
+		sa, del := createDeleteSvcAcct(t, cs, sa)
+		defer del()
+
+		originalSecret, originalDelSecret := createDeleteSecret(t, cs, secret)
+		defer originalDelSecret()
+
+		treq.Spec.BoundObjectRef.UID = originalSecret.UID
+		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
+		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
+		checkPayload(t, treq.Status.Token, `null`, "kubernetes.io", "pod")
+		checkPayload(t, treq.Status.Token, `"test-secret"`, "kubernetes.io", "secret", "name")
+		checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace")
+		checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name")
+		checkExpiration(t, treq, maxExpirationSeconds)
+
+		doTokenReview(t, cs, treq, false)
+		originalDelSecret()
+		doTokenReview(t, cs, treq, true)
+
+		_, recreateDelSecret := createDeleteSecret(t, cs, secret)
+		defer recreateDelSecret()
+
+		doTokenReview(t, cs, treq, true)
+	})
 }
 
 func doTokenReview(t *testing.T, cs externalclientset.Interface, treq *authenticationv1.TokenRequest, expectErr bool) {
@@ -470,6 +566,16 @@ func checkPayload(t *testing.T, tok string, want string, parts ...string) {
 	}
 }
 
+func checkExpiration(t *testing.T, treq *authenticationv1.TokenRequest, expectedExpiration int64) {
+	t.Helper()
+	if treq.Spec.ExpirationSeconds == nil {
+		t.Errorf("unexpected nil expiration seconds.")
+	}
+	if *treq.Spec.ExpirationSeconds != expectedExpiration {
+		t.Errorf("unexpected expiration seconds.\nsaw:\t%d\nwant:\t%d", treq.Spec.ExpirationSeconds, expectedExpiration)
+	}
+}
+
 func getSubObject(t *testing.T, b string, parts ...string) string {
 	t.Helper()
 	var obj interface{}