diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 87aa529e6a..d2df51dfdb 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -326,18 +326,25 @@ func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunnele } var issuer serviceaccount.TokenGenerator - if s.ServiceAccountSigningKeyFile != "" || s.Authentication.ServiceAccounts.Issuer != "" { + var apiAudiences []string + if s.ServiceAccountSigningKeyFile != "" || + s.Authentication.ServiceAccounts.Issuer != "" || + len(s.Authentication.ServiceAccounts.APIAudiences) > 0 { if !utilfeature.DefaultFeatureGate.Enabled(features.TokenRequest) { return nil, nil, nil, nil, nil, fmt.Errorf("the TokenRequest feature is not enabled but --service-account-signing-key-file and/or --service-account-issuer-id flags were passed") } - if s.ServiceAccountSigningKeyFile == "" || s.Authentication.ServiceAccounts.Issuer == "" { - return nil, nil, nil, nil, nil, fmt.Errorf("service-account-signing-key-file and service-account-issuer should be specified together") + if s.ServiceAccountSigningKeyFile == "" || + s.Authentication.ServiceAccounts.Issuer == "" || + len(s.Authentication.ServiceAccounts.APIAudiences) == 0 || + len(s.Authentication.ServiceAccounts.KeyFiles) == 0 { + return nil, nil, nil, nil, nil, fmt.Errorf("service-account-signing-key-file, service-account-issuer, service-account-api-audiences and service-account-key-file should be specified together") } sk, err := certutil.PrivateKeyFromFile(s.ServiceAccountSigningKeyFile) if err != nil { return nil, nil, nil, nil, nil, fmt.Errorf("failed to parse service-account-issuer-key-file: %v", err) } issuer = serviceaccount.JWTTokenGenerator(s.Authentication.ServiceAccounts.Issuer, sk) + apiAudiences = s.Authentication.ServiceAccounts.APIAudiences } config := &master.Config{ @@ -371,7 +378,9 @@ func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunnele EndpointReconcilerType: reconcilers.Type(s.EndpointReconcilerType), MasterCount: s.MasterCount, - ServiceAccountIssuer: issuer, + + ServiceAccountIssuer: issuer, + ServiceAccountAPIAudiences: apiAudiences, }, } @@ -672,12 +681,19 @@ func defaultOptions(s *options.ServerRunOptions) error { s.Authentication.ApplyAuthorization(s.Authorization) - // Default to the private server key for service account token signing - if len(s.Authentication.ServiceAccounts.KeyFiles) == 0 && s.SecureServing.ServerCert.CertKey.KeyFile != "" { - if kubeauthenticator.IsValidServiceAccountKeyFile(s.SecureServing.ServerCert.CertKey.KeyFile) { - s.Authentication.ServiceAccounts.KeyFiles = []string{s.SecureServing.ServerCert.CertKey.KeyFile} - } else { - glog.Warning("No TLS key provided, service account token authentication disabled") + // Use (ServiceAccountSigningKeyFile != "") as a proxy to the user enabling + // TokenRequest functionality. This defaulting was convenient, but messed up + // a lot of people when they rotated their serving cert with no idea it was + // connected to their service account keys. We are taking this oppurtunity to + // remove this problematic defaulting. + if s.ServiceAccountSigningKeyFile == "" { + // Default to the private server key for service account token signing + if len(s.Authentication.ServiceAccounts.KeyFiles) == 0 && s.SecureServing.ServerCert.CertKey.KeyFile != "" { + if kubeauthenticator.IsValidServiceAccountKeyFile(s.SecureServing.ServerCert.CertKey.KeyFile) { + s.Authentication.ServiceAccounts.KeyFiles = []string{s.SecureServing.ServerCert.CertKey.KeyFile} + } else { + glog.Warning("No TLS key provided, service account token authentication disabled") + } } } diff --git a/pkg/controller/serviceaccount/tokengetter.go b/pkg/controller/serviceaccount/tokengetter.go index 2243d0f00b..ed86489639 100644 --- a/pkg/controller/serviceaccount/tokengetter.go +++ b/pkg/controller/serviceaccount/tokengetter.go @@ -35,9 +35,15 @@ type clientGetter struct { func NewGetterFromClient(c clientset.Interface) serviceaccount.ServiceAccountTokenGetter { return clientGetter{c} } + func (c clientGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) { return c.client.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) } + +func (c clientGetter) GetPod(namespace, name string) (*v1.Pod, error) { + return c.client.CoreV1().Pods(namespace).Get(name, metav1.GetOptions{}) +} + func (c clientGetter) GetSecret(namespace, name string) (*v1.Secret, error) { return c.client.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) } diff --git a/pkg/kubeapiserver/authenticator/BUILD b/pkg/kubeapiserver/authenticator/BUILD index 8830b311bf..31bd91614f 100644 --- a/pkg/kubeapiserver/authenticator/BUILD +++ b/pkg/kubeapiserver/authenticator/BUILD @@ -10,6 +10,7 @@ go_library( srcs = ["config.go"], importpath = "k8s.io/kubernetes/pkg/kubeapiserver/authenticator", deps = [ + "//pkg/features:go_default_library", "//pkg/serviceaccount:go_default_library", "//vendor/github.com/go-openapi/spec:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library", @@ -24,6 +25,7 @@ go_library( "//vendor/k8s.io/apiserver/pkg/authentication/token/cache:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/token/tokenfile:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/token/union:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/apiserver/plugin/pkg/authenticator/password/passwordfile:go_default_library", "//vendor/k8s.io/apiserver/plugin/pkg/authenticator/request/basicauth:go_default_library", "//vendor/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc:go_default_library", diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index f9e90da0eb..05c4c377aa 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -33,11 +33,13 @@ import ( tokencache "k8s.io/apiserver/pkg/authentication/token/cache" "k8s.io/apiserver/pkg/authentication/token/tokenfile" tokenunion "k8s.io/apiserver/pkg/authentication/token/union" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/plugin/pkg/authenticator/password/passwordfile" "k8s.io/apiserver/plugin/pkg/authenticator/request/basicauth" "k8s.io/apiserver/plugin/pkg/authenticator/token/oidc" "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook" certutil "k8s.io/client-go/util/cert" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/serviceaccount" _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -59,6 +61,8 @@ type AuthenticatorConfig struct { OIDCSigningAlgs []string ServiceAccountKeyFiles []string ServiceAccountLookup bool + ServiceAccountIssuer string + ServiceAccountAPIAudiences []string WebhookTokenAuthnConfigFile string WebhookTokenAuthnCacheTTL time.Duration @@ -123,7 +127,14 @@ func (config AuthenticatorConfig) New() (authenticator.Request, *spec.SecurityDe tokenAuthenticators = append(tokenAuthenticators, tokenAuth) } if len(config.ServiceAccountKeyFiles) > 0 { - serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountKeyFiles, config.ServiceAccountLookup, config.ServiceAccountTokenGetter) + serviceAccountAuth, err := newLegacyServiceAccountAuthenticator(config.ServiceAccountKeyFiles, config.ServiceAccountLookup, config.ServiceAccountTokenGetter) + if err != nil { + return nil, nil, err + } + tokenAuthenticators = append(tokenAuthenticators, serviceAccountAuth) + } + if utilfeature.DefaultFeatureGate.Enabled(features.TokenRequest) && config.ServiceAccountIssuer != "" { + serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountIssuer, config.ServiceAccountAPIAudiences, config.ServiceAccountKeyFiles, config.ServiceAccountTokenGetter) if err != nil { return nil, nil, err } @@ -267,8 +278,8 @@ func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClai return tokenAuthenticator, nil } -// newServiceAccountAuthenticator returns an authenticator.Token or an error -func newServiceAccountAuthenticator(keyfiles []string, lookup bool, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Token, error) { +// newLegacyServiceAccountAuthenticator returns an authenticator.Token or an error +func newLegacyServiceAccountAuthenticator(keyfiles []string, lookup bool, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Token, error) { allPublicKeys := []interface{}{} for _, keyfile := range keyfiles { publicKeys, err := certutil.PublicKeysFromFile(keyfile) @@ -282,6 +293,21 @@ func newServiceAccountAuthenticator(keyfiles []string, lookup bool, serviceAccou return tokenAuthenticator, nil } +// newServiceAccountAuthenticator returns an authenticator.Token or an error +func newServiceAccountAuthenticator(iss string, audiences []string, keyfiles []string, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Token, error) { + allPublicKeys := []interface{}{} + for _, keyfile := range keyfiles { + publicKeys, err := certutil.PublicKeysFromFile(keyfile) + if err != nil { + return nil, err + } + allPublicKeys = append(allPublicKeys, publicKeys...) + } + + tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(iss, allPublicKeys, serviceaccount.NewValidator(audiences, serviceAccountGetter)) + return tokenAuthenticator, nil +} + // newAuthenticatorFromClientCAFile returns an authenticator.Request or an error func newAuthenticatorFromClientCAFile(clientCAFile string) (authenticator.Request, error) { roots, err := certutil.NewPool(clientCAFile) diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go index c24bbacac6..1a71fb1773 100644 --- a/pkg/kubeapiserver/options/authentication.go +++ b/pkg/kubeapiserver/options/authentication.go @@ -70,9 +70,10 @@ type PasswordFileAuthenticationOptions struct { } type ServiceAccountAuthenticationOptions struct { - KeyFiles []string - Lookup bool - Issuer string + KeyFiles []string + Lookup bool + Issuer string + APIAudiences []string } type TokenFileAuthenticationOptions struct { @@ -236,8 +237,10 @@ func (s *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) { if s.ServiceAccounts != nil { fs.StringArrayVar(&s.ServiceAccounts.KeyFiles, "service-account-key-file", s.ServiceAccounts.KeyFiles, ""+ "File containing PEM-encoded x509 RSA or ECDSA private or public keys, used to verify "+ - "ServiceAccount tokens. If unspecified, --tls-private-key-file is used. "+ - "The specified file can contain multiple keys, and the flag can be specified multiple times with different files.") + "ServiceAccount tokens. The specified file can contain multiple keys, and the flag can "+ + "be specified multiple times with different files. If unspecified, "+ + "--tls-private-key-file is used. Must be specified when "+ + "--service-account-signing-key is provided") fs.BoolVar(&s.ServiceAccounts.Lookup, "service-account-lookup", s.ServiceAccounts.Lookup, "If true, validate ServiceAccount tokens exist in etcd as part of authentication.") @@ -245,6 +248,10 @@ func (s *BuiltInAuthenticationOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.ServiceAccounts.Issuer, "service-account-issuer", s.ServiceAccounts.Issuer, ""+ "Identifier of the service account token issuer. The issuer will assert this identifier "+ "in \"iss\" claim of issued tokens. This value is a string or URI.") + + 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.") } if s.TokenFile != nil { @@ -303,6 +310,8 @@ func (s *BuiltInAuthenticationOptions) ToAuthenticationConfig() authenticator.Au if s.ServiceAccounts != nil { ret.ServiceAccountKeyFiles = s.ServiceAccounts.KeyFiles ret.ServiceAccountLookup = s.ServiceAccounts.Lookup + ret.ServiceAccountIssuer = s.ServiceAccounts.Issuer + ret.ServiceAccountAPIAudiences = s.ServiceAccounts.APIAudiences } if s.TokenFile != nil { diff --git a/pkg/master/master.go b/pkg/master/master.go index c071649697..d90dadfda6 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -157,7 +157,8 @@ type ExtraConfig struct { // Selects which reconciler to use EndpointReconcilerType reconcilers.Type - ServiceAccountIssuer serviceaccount.TokenGenerator + ServiceAccountIssuer serviceaccount.TokenGenerator + ServiceAccountAPIAudiences []string } type Config struct { @@ -314,14 +315,15 @@ 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, + 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, } 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 c937e4f287..d085022aae 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -79,7 +79,8 @@ type LegacyRESTStorageProvider struct { ServiceIPRange net.IPNet ServiceNodePortRange utilnet.PortRange - ServiceAccountIssuer serviceaccount.TokenGenerator + ServiceAccountIssuer serviceaccount.TokenGenerator + ServiceAccountAPIAudiences []string LoopbackClientConfig *restclient.Config } @@ -140,9 +141,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, podStorage.Pod.Store, secretStorage.Store) + serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.ServiceAccountAPIAudiences, podStorage.Pod.Store, secretStorage.Store) } else { - serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, nil) + serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 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 ae47cae068..a10f99ee60 100644 --- a/pkg/registry/core/serviceaccount/storage/storage.go +++ b/pkg/registry/core/serviceaccount/storage/storage.go @@ -32,7 +32,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against service accounts. -func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, podStorage, secretStorage *genericregistry.Store) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds []string, podStorage, secretStorage *genericregistry.Store) *REST { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ServiceAccount{} }, NewListFunc: func() runtime.Object { return &api.ServiceAccountList{} }, @@ -52,9 +52,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, if issuer != nil && podStorage != nil && secretStorage != nil { trest = &TokenREST{ svcaccts: store, - issuer: issuer, pods: podStorage, secrets: secretStorage, + issuer: issuer, + auds: auds, } } diff --git a/pkg/registry/core/serviceaccount/storage/storage_test.go b/pkg/registry/core/serviceaccount/storage/storage_test.go index 1a41a9efcb..d25ee8d62c 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), server + return NewREST(restOptions, nil, nil, 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 0079a51cc7..276681bd79 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/pkg/registry/core/serviceaccount/storage/token.go @@ -41,6 +41,7 @@ type TokenREST struct { pods getter secrets getter issuer token.TokenGenerator + auds []string } var _ = rest.NamedCreater(&TokenREST{}) @@ -92,6 +93,9 @@ func (r *TokenREST) Create(ctx genericapirequest.Context, name string, obj runti return nil, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, ref.Name, fmt.Errorf("the UID in the bound object reference (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", ref.UID, uid)) } } + if len(out.Spec.Audiences) == 0 { + out.Spec.Audiences = r.auds + } 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/pkg/serviceaccount/claims.go b/pkg/serviceaccount/claims.go index 084af2dfaa..c3ed29c4c2 100644 --- a/pkg/serviceaccount/claims.go +++ b/pkg/serviceaccount/claims.go @@ -17,8 +17,11 @@ limitations under the License. package serviceaccount import ( + "errors" + "fmt" "time" + "github.com/golang/glog" apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/kubernetes/pkg/apis/core" @@ -76,3 +79,108 @@ func Claims(sa core.ServiceAccount, pod *core.Pod, secret *core.Secret, expirati } return sc, pc } + +func NewValidator(audiences []string, getter ServiceAccountTokenGetter) Validator { + return &validator{ + auds: audiences, + getter: getter, + } +} + +type validator struct { + auds []string + getter ServiceAccountTokenGetter +} + +var _ = Validator(&validator{}) + +func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{}) (string, string, string, error) { + private, ok := privateObj.(*privateClaims) + if !ok { + glog.Errorf("jwt validator expected private claim of type *privateClaims but got: %T", privateObj) + return "", "", "", errors.New("Token could not be validated.") + } + err := public.Validate(jwt.Expected{ + Time: now(), + }) + switch { + case err == nil: + case err == jwt.ErrExpired: + return "", "", "", errors.New("Token has expired.") + default: + glog.Errorf("unexpected validation error: %T", err) + return "", "", "", errors.New("Token could not be validated.") + } + + var audValid bool + + for _, aud := range v.auds { + audValid = public.Audience.Contains(aud) + if audValid { + break + } + } + + if !audValid { + return "", "", "", errors.New("Token is invalid for this audience.") + } + + namespace := private.Kubernetes.Namespace + saref := private.Kubernetes.Svcacct + podref := private.Kubernetes.Pod + secref := private.Kubernetes.Secret + // Make sure service account still exists (name and UID) + serviceAccount, err := v.getter.GetServiceAccount(namespace, saref.Name) + if err != nil { + glog.V(4).Infof("Could not retrieve service account %s/%s: %v", namespace, saref.Name, err) + return "", "", "", err + } + if serviceAccount.DeletionTimestamp != nil { + glog.V(4).Infof("Service account has been deleted %s/%s", namespace, saref.Name) + return "", "", "", fmt.Errorf("ServiceAccount %s/%s has been deleted", namespace, saref.Name) + } + if string(serviceAccount.UID) != saref.UID { + glog.V(4).Infof("Service account UID no longer matches %s/%s: %q != %q", namespace, saref.Name, string(serviceAccount.UID), saref.UID) + return "", "", "", fmt.Errorf("ServiceAccount UID (%s) does not match claim (%s)", serviceAccount.UID, saref.UID) + } + + if secref != nil { + // Make sure token hasn't been invalidated by deletion of the secret + secret, err := v.getter.GetSecret(namespace, secref.Name) + if err != nil { + glog.V(4).Infof("Could not retrieve bound secret %s/%s for service account %s/%s: %v", namespace, secref.Name, namespace, saref.Name, err) + return "", "", "", errors.New("Token has been invalidated") + } + if secret.DeletionTimestamp != nil { + glog.V(4).Infof("Bound secret is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secref.Name, namespace, saref.Name) + return "", "", "", errors.New("Token has been invalidated") + } + if string(secref.UID) != secref.UID { + glog.V(4).Infof("Secret UID no longer matches %s/%s: %q != %q", namespace, secref.Name, string(serviceAccount.UID), secref.UID) + return "", "", "", fmt.Errorf("Secret UID (%s) does not match claim (%s)", secret.UID, secref.UID) + } + } + + if podref != nil { + // Make sure token hasn't been invalidated by deletion of the pod + pod, err := v.getter.GetPod(namespace, podref.Name) + if err != nil { + glog.V(4).Infof("Could not retrieve bound secret %s/%s for service account %s/%s: %v", namespace, podref.Name, namespace, saref.Name, err) + return "", "", "", errors.New("Token has been invalidated") + } + if pod.DeletionTimestamp != nil { + glog.V(4).Infof("Bound pod is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, podref.Name, namespace, saref.Name) + return "", "", "", errors.New("Token has been invalidated") + } + if string(podref.UID) != podref.UID { + glog.V(4).Infof("Pod UID no longer matches %s/%s: %q != %q", namespace, podref.Name, string(serviceAccount.UID), podref.UID) + return "", "", "", fmt.Errorf("Pod UID (%s) does not match claim (%s)", pod.UID, podref.UID) + } + } + + return private.Kubernetes.Namespace, private.Kubernetes.Svcacct.Name, private.Kubernetes.Svcacct.UID, nil +} + +func (v *validator) NewPrivateClaims() interface{} { + return &privateClaims{} +} diff --git a/pkg/serviceaccount/jwt.go b/pkg/serviceaccount/jwt.go index 4371d98717..01c369a31f 100644 --- a/pkg/serviceaccount/jwt.go +++ b/pkg/serviceaccount/jwt.go @@ -38,6 +38,7 @@ import ( // ServiceAccountTokenGetter defines functions to retrieve a named service account and secret type ServiceAccountTokenGetter interface { GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) + GetPod(namespace, name string) (*v1.Pod, error) GetSecret(namespace, name string) (*v1.Secret, error) } diff --git a/test/integration/auth/BUILD b/test/integration/auth/BUILD index 23bf4fef6c..34331a7025 100644 --- a/test/integration/auth/BUILD +++ b/test/integration/auth/BUILD @@ -31,6 +31,7 @@ go_test( "//pkg/auth/nodeidentifier:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/informers/informers_generated/internalversion:go_default_library", + "//pkg/controller/serviceaccount:go_default_library", "//pkg/features:go_default_library", "//pkg/kubeapiserver/authorizer:go_default_library", "//pkg/master:go_default_library", diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go index 259f64fd61..fe8421b954 100644 --- a/test/integration/auth/svcaccttoken_test.go +++ b/test/integration/auth/svcaccttoken_test.go @@ -17,20 +17,25 @@ limitations under the License. package auth import ( + "crypto/ecdsa" "encoding/base64" "encoding/json" "strings" "testing" + "time" authenticationv1 "k8s.io/api/authentication/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/authentication/request/bearertoken" "k8s.io/apiserver/pkg/authorization/authorizerfactory" utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" clientset "k8s.io/client-go/kubernetes" + externalclientset "k8s.io/client-go/kubernetes" certutil "k8s.io/client-go/util/cert" + serviceaccountgetter "k8s.io/kubernetes/pkg/controller/serviceaccount" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/serviceaccount" "k8s.io/kubernetes/test/integration/framework" @@ -50,11 +55,25 @@ func TestServiceAccountTokenCreate(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + pk := sk.(*ecdsa.PrivateKey).PublicKey + + const iss = "https://foo.bar.example.com" + aud := []string{"api"} + + gcs := &clientset.Clientset{} // Start the server masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.GenericConfig.Authorization.Authorizer = authorizerfactory.NewAlwaysAllowAuthorizer() - masterConfig.ExtraConfig.ServiceAccountIssuer = serviceaccount.JWTTokenGenerator("https://foo.bar.example.com", sk) + masterConfig.GenericConfig.Authentication.Authenticator = bearertoken.New( + serviceaccount.JWTTokenAuthenticator( + iss, + []interface{}{&pk}, + serviceaccount.NewValidator(aud, serviceaccountgetter.NewGetterFromClient(gcs)), + ), + ) + masterConfig.ExtraConfig.ServiceAccountIssuer = serviceaccount.JWTTokenGenerator(iss, sk) + masterConfig.ExtraConfig.ServiceAccountAPIAudiences = aud master, _, closeFn := framework.RunAMaster(masterConfig) defer closeFn() @@ -63,6 +82,7 @@ func TestServiceAccountTokenCreate(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + *gcs = *cs var ( sa = &v1.ServiceAccount{ @@ -114,8 +134,8 @@ func TestServiceAccountTokenCreate(t *testing.T) { if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err == nil { t.Fatalf("expected err creating token for nonexistant svcacct but got: %#v", resp) } - sa, del := createDeleteSvcAcct(t, cs, sa) - defer del() + sa, delSvcAcct := createDeleteSvcAcct(t, cs, sa) + defer delSvcAcct() treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq) if err != nil { @@ -128,6 +148,10 @@ func TestServiceAccountTokenCreate(t *testing.T) { checkPayload(t, treq.Status.Token, "null", "kubernetes.io", "secret") checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace") checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name") + + doTokenReview(t, cs, treq, false) + delSvcAcct() + doTokenReview(t, cs, treq, true) }) t.Run("bound to service account and pod", func(t *testing.T) { @@ -152,8 +176,8 @@ func TestServiceAccountTokenCreate(t *testing.T) { if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err == nil { t.Fatalf("expected err creating token bound to nonexistant pod but got: %#v", resp) } - pod, del := createDeletePod(t, cs, pod) - defer del() + pod, delPod := createDeletePod(t, cs, pod) + defer delPod() // right uid treq.Spec.BoundObjectRef.UID = pod.UID @@ -178,6 +202,10 @@ func TestServiceAccountTokenCreate(t *testing.T) { checkPayload(t, treq.Status.Token, "null", "kubernetes.io", "secret") checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace") checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name") + + doTokenReview(t, cs, treq, false) + delPod() + doTokenReview(t, cs, treq, true) }) t.Run("bound to service account and secret", func(t *testing.T) { @@ -203,8 +231,8 @@ func TestServiceAccountTokenCreate(t *testing.T) { if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err == nil { t.Fatalf("expected err creating token bound to nonexistant secret but got: %#v", resp) } - secret, del := createDeleteSecret(t, cs, secret) - defer del() + secret, delSecret := createDeleteSecret(t, cs, secret) + defer delSecret() // right uid treq.Spec.BoundObjectRef.UID = secret.UID @@ -229,6 +257,10 @@ func TestServiceAccountTokenCreate(t *testing.T) { 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") + + doTokenReview(t, cs, treq, false) + delSecret() + doTokenReview(t, cs, treq, true) }) t.Run("bound to service account and pod running as different service account", func(t *testing.T) { @@ -253,6 +285,85 @@ func TestServiceAccountTokenCreate(t *testing.T) { t.Fatalf("expected err but got: %#v", resp) } }) + + t.Run("expired token", func(t *testing.T) { + treq := &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"api"}, + ExpirationSeconds: &one, + }, + } + + sa, del := createDeleteSvcAcct(t, cs, sa) + defer del() + + treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq) + if err != nil { + t.Fatalf("err: %v", err) + } + + doTokenReview(t, cs, treq, false) + time.Sleep(63 * time.Second) + doTokenReview(t, cs, treq, true) + }) + + t.Run("a token without an api audience is invalid", func(t *testing.T) { + treq := &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"not-the-api"}, + }, + } + + sa, del := createDeleteSvcAcct(t, cs, sa) + defer del() + + treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq) + if err != nil { + t.Fatalf("err: %v", err) + } + + doTokenReview(t, cs, treq, true) + }) + + t.Run("a tokenrequest without an audience is valid against the api", func(t *testing.T) { + treq := &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{}, + } + + sa, del := createDeleteSvcAcct(t, cs, sa) + defer del() + + treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq) + if err != nil { + t.Fatalf("err: %v", err) + } + + checkPayload(t, treq.Status.Token, `["api"]`, "aud") + + doTokenReview(t, cs, treq, false) + }) +} + +func doTokenReview(t *testing.T, cs externalclientset.Interface, treq *authenticationv1.TokenRequest, expectErr bool) { + t.Helper() + trev, err := cs.AuthenticationV1().TokenReviews().Create(&authenticationv1.TokenReview{ + Spec: authenticationv1.TokenReviewSpec{ + Token: treq.Status.Token, + }, + }) + if err != nil { + t.Fatalf("err: %v", err) + } + t.Logf("status: %+v", trev.Status) + if (trev.Status.Error != "") && !expectErr { + t.Fatalf("expected no error but got: %v", trev.Status.Error) + } + if (trev.Status.Error == "") && expectErr { + t.Fatalf("expected error but got: %+v", trev.Status) + } + if !trev.Status.Authenticated && !expectErr { + t.Fatal("expected token to be authenticated but it wasn't") + } } func checkPayload(t *testing.T, tok string, want string, parts ...string) { @@ -299,8 +410,13 @@ func createDeleteSvcAcct(t *testing.T, cs clientset.Interface, sa *v1.ServiceAcc if err != nil { t.Fatalf("err: %v", err) } + done := false return sa, func() { t.Helper() + if done { + return + } + done = true if err := cs.CoreV1().ServiceAccounts(sa.Namespace).Delete(sa.Name, nil); err != nil { t.Fatalf("err: %v", err) } @@ -313,8 +429,13 @@ func createDeletePod(t *testing.T, cs clientset.Interface, pod *v1.Pod) (*v1.Pod if err != nil { t.Fatalf("err: %v", err) } + done := false return pod, func() { t.Helper() + if done { + return + } + done = true if err := cs.CoreV1().Pods(pod.Namespace).Delete(pod.Name, nil); err != nil { t.Fatalf("err: %v", err) } @@ -327,8 +448,13 @@ func createDeleteSecret(t *testing.T, cs clientset.Interface, sec *v1.Secret) (* if err != nil { t.Fatalf("err: %v", err) } + done := false return sec, func() { t.Helper() + if done { + return + } + done = true if err := cs.CoreV1().Secrets(sec.Namespace).Delete(sec.Name, nil); err != nil { t.Fatalf("err: %v", err) }