diff --git a/cmd/cloud-controller-manager/app/controllermanager.go b/cmd/cloud-controller-manager/app/controllermanager.go index 85c1fc60649..00bc7442ee6 100644 --- a/cmd/cloud-controller-manager/app/controllermanager.go +++ b/cmd/cloud-controller-manager/app/controllermanager.go @@ -135,9 +135,10 @@ func Run(s *options.CloudControllerManagerServer, cloud cloudprovider.Interface) var clientBuilder controller.ControllerClientBuilder if len(s.ServiceAccountKeyFile) > 0 && s.UseServiceAccountCredentials { clientBuilder = controller.SAControllerClientBuilder{ - ClientConfig: restclient.AnonymousClientConfig(kubeconfig), - CoreClient: kubeClient.Core(), - Namespace: "kube-system", + ClientConfig: restclient.AnonymousClientConfig(kubeconfig), + CoreClient: kubeClient.Core(), + AuthenticationClient: kubeClient.Authentication(), + Namespace: "kube-system", } } else { clientBuilder = rootClientBuilder diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index ff4660286f8..26b8efdca0c 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -159,9 +159,10 @@ func Run(s *options.CMServer) error { var clientBuilder controller.ControllerClientBuilder if len(s.ServiceAccountKeyFile) > 0 && s.UseServiceAccountCredentials { clientBuilder = controller.SAControllerClientBuilder{ - ClientConfig: restclient.AnonymousClientConfig(kubeconfig), - CoreClient: kubeClient.Core(), - Namespace: "kube-system", + ClientConfig: restclient.AnonymousClientConfig(kubeconfig), + CoreClient: kubeClient.Core(), + AuthenticationClient: kubeClient.Authentication(), + Namespace: "kube-system", } } else { clientBuilder = rootClientBuilder diff --git a/pkg/controller/BUILD b/pkg/controller/BUILD index df71efeb98f..f410b0f0349 100644 --- a/pkg/controller/BUILD +++ b/pkg/controller/BUILD @@ -22,8 +22,10 @@ go_library( "//pkg/api:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/api/validation:go_default_library", + "//pkg/apis/authentication/v1:go_default_library", "//pkg/apis/extensions/v1beta1:go_default_library", "//pkg/client/clientset_generated/clientset:go_default_library", + "//pkg/client/clientset_generated/clientset/typed/authentication/v1:go_default_library", "//pkg/client/clientset_generated/clientset/typed/core/v1:go_default_library", "//pkg/client/retry:go_default_library", "//pkg/serviceaccount:go_default_library", diff --git a/pkg/controller/client_builder.go b/pkg/controller/client_builder.go index b3d5d44efd7..3472f10ca08 100644 --- a/pkg/controller/client_builder.go +++ b/pkg/controller/client_builder.go @@ -31,7 +31,9 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/v1" + v1authenticationapi "k8s.io/kubernetes/pkg/apis/authentication/v1" "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" + v1authentication "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/authentication/v1" v1core "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/core/v1" "k8s.io/kubernetes/pkg/serviceaccount" @@ -109,6 +111,10 @@ type SAControllerClientBuilder struct { // to construct a controller client CoreClient v1core.CoreV1Interface + // AuthenticationClient is used to check API tokens to make sure they are valid before + // building a controller client from them + AuthenticationClient v1authentication.AuthenticationV1Interface + // Namespace is the namespace used to host the service accounts that will back the // controllers. It must be highly privileged namespace which normal users cannot inspect. Namespace string @@ -117,29 +123,13 @@ type SAControllerClientBuilder struct { // config returns a complete clientConfig for constructing clients. This is separate in anticipation of composition // which means that not all clientsets are known here func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, error) { - clientConfig := restclient.AnonymousClientConfig(b.ClientConfig) - - // we need the SA UID to find a matching SA token - sa, err := b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{}) - if err != nil && !apierrors.IsNotFound(err) { + sa, err := b.getOrCreateServiceAccount(name) + if err != nil { return nil, err - } else if apierrors.IsNotFound(err) { - // check to see if the namespace exists. If it isn't a NotFound, just try to create the SA. - // It'll probably fail, but perhaps that will have a better message. - if _, err := b.CoreClient.Namespaces().Get(b.Namespace, metav1.GetOptions{}); apierrors.IsNotFound(err) { - _, err = b.CoreClient.Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: b.Namespace}}) - if err != nil && !apierrors.IsAlreadyExists(err) { - return nil, err - } - } - - sa, err = b.CoreClient.ServiceAccounts(b.Namespace).Create( - &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: b.Namespace, Name: name}}) - if err != nil { - return nil, err - } } + var clientConfig *restclient.Config + lw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { options.FieldSelector = fields.SelectorFromSet(map[string]string{api.SecretTypeField: string(v1.SecretTypeServiceAccountToken)}).String() @@ -159,14 +149,32 @@ func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, erro return false, fmt.Errorf("error watching") case watch.Added, watch.Modified: - secret := event.Object.(*v1.Secret) - if !serviceaccount.IsServiceAccountToken(secret, sa) || - len(secret.Data[v1.ServiceAccountTokenKey]) == 0 { + secret, ok := event.Object.(*v1.Secret) + if !ok { + return false, fmt.Errorf("unexpected object type: %T", event.Object) + } + if !serviceaccount.IsServiceAccountToken(secret, sa) { return false, nil } - // TODO maybe verify the token is valid - clientConfig.BearerToken = string(secret.Data[v1.ServiceAccountTokenKey]) - restclient.AddUserAgent(clientConfig, apiserverserviceaccount.MakeUsername(b.Namespace, name)) + if len(secret.Data[v1.ServiceAccountTokenKey]) == 0 { + return false, nil + } + validConfig, valid, err := b.getAuthenticatedConfig(sa, string(secret.Data[v1.ServiceAccountTokenKey])) + if err != nil { + glog.Warningf("error validating API token for %s/%s in secret %s: %v", sa.Name, sa.Namespace, secret.Name, err) + // continue watching for good tokens + return false, nil + } + if !valid { + glog.Warningf("secret %s contained an invalid API token for %s/%s", secret.Name, sa.Name, sa.Namespace) + // try to delete the secret containing the invalid token + if err := b.CoreClient.Secrets(secret.Namespace).Delete(secret.Name, &metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { + glog.Warningf("error deleting secret %s containing invalid API token for %s/%s: %v", secret.Name, sa.Name, sa.Namespace, err) + } + // continue watching for good tokens + return false, nil + } + clientConfig = validConfig return true, nil default: @@ -180,6 +188,69 @@ func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, erro return clientConfig, nil } +func (b SAControllerClientBuilder) getOrCreateServiceAccount(name string) (*v1.ServiceAccount, error) { + sa, err := b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{}) + if err == nil { + return sa, nil + } + if !apierrors.IsNotFound(err) { + return nil, err + } + + // Create the namespace if we can't verify it exists. + // Tolerate errors, since we don't know whether this component has namespace creation permissions. + if _, err := b.CoreClient.Namespaces().Get(b.Namespace, metav1.GetOptions{}); err != nil { + b.CoreClient.Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: b.Namespace}}) + } + + // Create the service account + sa, err = b.CoreClient.ServiceAccounts(b.Namespace).Create(&v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: b.Namespace, Name: name}}) + if apierrors.IsAlreadyExists(err) { + // If we're racing to init and someone else already created it, re-fetch + return b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{}) + } + return sa, err +} + +func (b SAControllerClientBuilder) getAuthenticatedConfig(sa *v1.ServiceAccount, token string) (*restclient.Config, bool, error) { + username := apiserverserviceaccount.MakeUsername(sa.Namespace, sa.Name) + + clientConfig := restclient.AnonymousClientConfig(b.ClientConfig) + clientConfig.BearerToken = token + restclient.AddUserAgent(clientConfig, username) + + // Try token review first + tokenReview := &v1authenticationapi.TokenReview{Spec: v1authenticationapi.TokenReviewSpec{Token: token}} + if tokenResult, err := b.AuthenticationClient.TokenReviews().Create(tokenReview); err == nil { + if !tokenResult.Status.Authenticated { + glog.Warningf("Token for %s/%s did not authenticate correctly", sa.Name, sa.Namespace) + return nil, false, nil + } + if tokenResult.Status.User.Username != username { + glog.Warningf("Token for %s/%s authenticated as unexpected username: %s", sa.Name, sa.Namespace, tokenResult.Status.User.Username) + return nil, false, nil + } + glog.V(4).Infof("Verified credential for %s/%s", sa.Name, sa.Namespace) + return clientConfig, true, nil + } + + // If we couldn't run the token review, the API might be disabled or we might not have permission. + // Try to make a request to /apis with the token. If we get a 401 we should consider the token invalid. + clientConfigCopy := *clientConfig + clientConfigCopy.NegotiatedSerializer = api.Codecs + client, err := restclient.UnversionedRESTClientFor(&clientConfigCopy) + if err != nil { + return nil, false, err + } + err = client.Get().AbsPath("/apis").Do().Error() + if apierrors.IsUnauthorized(err) { + glog.Warningf("Token for %s/%s did not authenticate correctly: %v", sa.Name, sa.Namespace, err) + return nil, false, nil + } + + return clientConfig, true, nil +} + func (b SAControllerClientBuilder) ConfigOrDie(name string) *restclient.Config { clientConfig, err := b.Config(name) if err != nil { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index 0605275e324..0694dbf0bfc 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -294,6 +294,8 @@ func ClusterRoles() []rbac.ClusterRole { rbac.NewRule("delete").Groups(legacyGroup).Resources("secrets").RuleOrDie(), rbac.NewRule("get").Groups(legacyGroup).Resources("endpoints", "namespaces", "serviceaccounts").RuleOrDie(), rbac.NewRule("update").Groups(legacyGroup).Resources("endpoints", "serviceaccounts").RuleOrDie(), + // Needed to check API access. These creates are non-mutating + rbac.NewRule("create").Groups(authenticationGroup).Resources("tokenreviews").RuleOrDie(), rbac.NewRule("list", "watch").Groups(legacyGroup).Resources( "configmaps", diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml index c001e8d502d..ca2d48125ba 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml @@ -461,6 +461,12 @@ items: - serviceaccounts verbs: - update + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create - apiGroups: - "" resources: