From f16470c3f1126a97a657aeb6265516a7ea250359 Mon Sep 17 00:00:00 2001 From: WanLinghao Date: Thu, 10 May 2018 14:29:43 +0800 Subject: [PATCH] This patch adds limit to the TokenRequest expiration time. It constrains a TokenRequest's expiration time to avoid extreme value which could harm the cluster. --- cmd/kube-apiserver/app/server.go | 24 +++- pkg/kubeapiserver/options/authentication.go | 13 ++- pkg/master/master.go | 24 ++-- pkg/registry/core/rest/storage_core.go | 9 +- .../core/serviceaccount/storage/storage.go | 15 ++- .../serviceaccount/storage/storage_test.go | 2 +- .../core/serviceaccount/storage/token.go | 17 ++- test/integration/auth/svcaccttoken_test.go | 106 ++++++++++++++++++ 8 files changed, 175 insertions(+), 35 deletions(-) diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index b936c9ece2a..b44e1abad1a 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -326,8 +326,12 @@ func CreateKubeAPIServerConfig( return } - var issuer serviceaccount.TokenGenerator - var apiAudiences []string + var ( + 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, - ServiceAccountAPIAudiences: apiAudiences, + ServiceAccountIssuer: issuer, + ServiceAccountAPIAudiences: apiAudiences, + ServiceAccountMaxExpiration: maxExpiration, }, } diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go index 8b794de8a09..465c9c661ae 100644 --- a/pkg/kubeapiserver/options/authentication.go +++ b/pkg/kubeapiserver/options/authentication.go @@ -73,10 +73,11 @@ type PasswordFileAuthenticationOptions struct { } type ServiceAccountAuthenticationOptions struct { - KeyFiles []string - Lookup bool - Issuer string - APIAudiences []string + KeyFiles []string + Lookup bool + Issuer 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 { diff --git a/pkg/master/master.go b/pkg/master/master.go index 240f631a863..01d6a327c6e 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -163,8 +163,9 @@ type ExtraConfig struct { // Selects which reconciler to use EndpointReconcilerType reconcilers.Type - ServiceAccountIssuer serviceaccount.TokenGenerator - ServiceAccountAPIAudiences []string + ServiceAccountIssuer serviceaccount.TokenGenerator + 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, - ProxyTransport: c.ExtraConfig.ProxyTransport, - KubeletClientConfig: c.ExtraConfig.KubeletClientConfig, - EventTTL: c.ExtraConfig.EventTTL, - ServiceIPRange: c.ExtraConfig.ServiceIPRange, - ServiceNodePortRange: c.ExtraConfig.ServiceNodePortRange, - LoopbackClientConfig: c.GenericConfig.LoopbackClientConfig, - ServiceAccountIssuer: c.ExtraConfig.ServiceAccountIssuer, - ServiceAccountAPIAudiences: c.ExtraConfig.ServiceAccountAPIAudiences, + StorageFactory: c.ExtraConfig.StorageFactory, + ProxyTransport: c.ExtraConfig.ProxyTransport, + KubeletClientConfig: c.ExtraConfig.KubeletClientConfig, + EventTTL: c.ExtraConfig.EventTTL, + ServiceIPRange: c.ExtraConfig.ServiceIPRange, + ServiceNodePortRange: c.ExtraConfig.ServiceNodePortRange, + LoopbackClientConfig: c.GenericConfig.LoopbackClientConfig, + ServiceAccountIssuer: c.ExtraConfig.ServiceAccountIssuer, + ServiceAccountAPIAudiences: c.ExtraConfig.ServiceAccountAPIAudiences, + ServiceAccountMaxExpiration: c.ExtraConfig.ServiceAccountMaxExpiration, } m.InstallLegacyAPI(&c, c.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider) } diff --git a/pkg/registry/core/rest/storage_core.go b/pkg/registry/core/rest/storage_core.go index 41e21f995f1..45cf9592fb4 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -79,8 +79,9 @@ type LegacyRESTStorageProvider struct { ServiceIPRange net.IPNet ServiceNodePortRange utilnet.PortRange - ServiceAccountIssuer serviceaccount.TokenGenerator - ServiceAccountAPIAudiences []string + ServiceAccountIssuer serviceaccount.TokenGenerator + 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) diff --git a/pkg/registry/core/serviceaccount/storage/storage.go b/pkg/registry/core/serviceaccount/storage/storage.go index 53f28e2ba49..9fe2319dc8c 100644 --- a/pkg/registry/core/serviceaccount/storage/storage.go +++ b/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, - pods: podStorage, - secrets: secretStorage, - issuer: issuer, - auds: auds, + svcaccts: store, + pods: podStorage, + secrets: secretStorage, + issuer: issuer, + auds: auds, + maxExpirationSeconds: int64(max.Seconds()), } } diff --git a/pkg/registry/core/serviceaccount/storage/storage_test.go b/pkg/registry/core/serviceaccount/storage/storage_test.go index d25ee8d62c9..02e0e0a0ece 100644 --- a/pkg/registry/core/serviceaccount/storage/storage_test.go +++ b/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 { diff --git a/pkg/registry/core/serviceaccount/storage/token.go b/pkg/registry/core/serviceaccount/storage/token.go index e1a94f6e50f..23cc43f1566 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/pkg/registry/core/serviceaccount/storage/token.go @@ -39,11 +39,12 @@ func (r *TokenREST) New() runtime.Object { } type TokenREST struct { - svcaccts getter - pods getter - secrets getter - issuer token.TokenGenerator - auds []string + svcaccts getter + pods getter + secrets getter + issuer token.TokenGenerator + 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 { diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go index ac7ba22edae..e6fa7b31366 100644 --- a/test/integration/auth/svcaccttoken_test.go +++ b/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{}