Skip to content

Commit

Permalink
Ensure cattle token secret has unique name
Browse files Browse the repository at this point in the history
The "cattle" service account on a downstream cluster is the account that
Rancher uses to connect as an admin to the downstream cluster. Without
this change, if the cattle service account's token is deleted, the
cluster agent will regenerate it identically. This is a problem because
it makes rotation of the token nontrivial.

We can't craft the JWT ourselves or influence what claims are included
in it, that is done within kubernetes. The only way to change the
resulting JWT is to change the values kubernetes uses for claims. The
only option is to make the secret name unique[1]. All other claims come
from the service account, which we do not want to have to change in
order to rotate the token.

This change addresses the problem by using GenerateName when creating
the secret so that it will be unique every time. However, since the name
is no longer predictable, this causes problems when Rancher tries to
look up the token. We now need to look up the name of the secret from
the service account object. A further complication is that for
kubernetes 1.24, the secret name is no longer stored on the service
account, so now we set it explicitly. An extra benefit of this approach
is that we no longer create multiple tokens for service accounts on k8s
<1.24, since creating the token is skipped if it is found on the service
account.

This change refactors any code that was creating a service account token
to use the serviceaccounttoken.EnsureSecretForServiceAccount function in
order to be consistent everywhere. The function is updated to use a
backoff routine instead of an infinite loop to check the state of the
secret. It is flexible enough to use controller caches for callers with
access to that, and falls back to regular clients for remote callers
such as the agent.

See also the change that introduced this functionality in Rancher[2].

[1] https://github.com/kubernetes/kubernetes/blob/v1.25.2/pkg/serviceaccount/legacy.go#L39
[2] #38113
  • Loading branch information
cmurphy authored and KevinJoiner committed Jan 24, 2023
1 parent 65b8d61 commit a30a596
Show file tree
Hide file tree
Showing 12 changed files with 249 additions and 165 deletions.
4 changes: 2 additions & 2 deletions pkg/agent/cluster/cluster.go
Expand Up @@ -74,9 +74,9 @@ func getTokenFromAPI() ([]byte, []byte, error) {
}
return secret.Data[coreV1.ServiceAccountRootCAKey], secret.Data[coreV1.ServiceAccountTokenKey], nil
}
secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.Background(), k8s, sa)
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, k8s, sa)
if err != nil {
return nil, nil, fmt.Errorf("failed to create secret for service account %s/%s: %w", namespace.System, "cattle", err)
return nil, nil, fmt.Errorf("failed to ensure secret for service account %s/%s: %w", namespace.System, "cattle", err)
}
return []byte(cm.Data["ca.crt"]), []byte(secret.Data[coreV1.ServiceAccountTokenKey]), nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/norman/customization/monitor/handler_utils.go
Expand Up @@ -187,9 +187,9 @@ func getAuthToken(userContext *config.UserContext, appName, namespace string) (s
return "", fmt.Errorf("get service account %s:%s for monitor failed, %v", namespace, appName, err)
}

secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.TODO(), userContext.K8sClient, sa)
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, userContext.K8sClient, sa)
if err != nil {
return "", fmt.Errorf("get secret from service account %s:%s for monitor failed: %v", namespace, appName, err)
return "", fmt.Errorf("ensure secret from service account %s:%s for monitor failed: %w", namespace, appName, err)
}

return string(secret.Data[v1.ServiceAccountTokenKey]), nil
Expand Down
36 changes: 9 additions & 27 deletions pkg/controllers/dashboard/apiservice/apiservice.go
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"crypto/sha256"
"encoding/base64"
"fmt"

v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
mgmtcontrollers "github.com/rancher/rancher/pkg/generated/controllers/management.cattle.io/v3"
Expand All @@ -16,10 +17,12 @@ import (
"github.com/rancher/wrangler/pkg/name"
"github.com/rancher/wrangler/pkg/relatedresource"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierror "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
)

type handler struct {
Expand All @@ -32,6 +35,7 @@ type handler struct {
apiServices mgmtcontrollers.APIServiceCache
services corev1controllers.ServiceCache
embedded bool
k8s kubernetes.Interface
ctx context.Context
}

Expand All @@ -46,6 +50,7 @@ func Register(ctx context.Context, context *wrangler.Context, embedded bool) {
apiServices: context.Mgmt.APIService().Cache(),
services: context.Core.Service().Cache(),
embedded: embedded,
k8s: context.K8s,
ctx: ctx,
}

Expand Down Expand Up @@ -142,35 +147,12 @@ func (h *handler) getToken(sa *corev1.ServiceAccount) (string, error) {
return "", err
}

// create a secret-based token for the service account
sName := serviceaccounttoken.ServiceAccountSecretName(sa)
secret, err := h.secretsCache.Get(sa.Namespace, sName)
// create a secret-based token for the service account if one does not exist
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(h.ctx, h.secretsCache.Get, h.k8s, sa)
if err != nil {
if !apierror.IsNotFound(err) {
return "", err
}
sc := serviceaccounttoken.SecretTemplate(sa)
secret, err = h.secretsClient.Create(sc)
if err != nil {
if !apierror.IsAlreadyExists(err) {
return "", err
}
secret, err = h.secretsCache.Get(sa.Namespace, sName)
if err != nil {
if !apierror.IsNotFound(err) {
return "", err
}
secret, err = h.secretsClient.Get(sa.Namespace, sName, metav1.GetOptions{})
if err != nil {
return "", err
}
}
}
}
token := secret.Data["token"]
if len(token) == 0 {
return "", nil
return "", fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err)
}
token := secret.Data[v1.ServiceAccountTokenKey]

hash := sha256.Sum256(token)
return base64.StdEncoding.EncodeToString(hash[:]), nil
Expand Down
21 changes: 5 additions & 16 deletions pkg/controllers/provisioningv2/rke2/bootstrap/controller.go
Expand Up @@ -25,6 +25,7 @@ import (
apierror "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
capi "sigs.k8s.io/cluster-api/api/v1beta1"
)

Expand All @@ -41,6 +42,7 @@ type handler struct {
deploymentCache appcontrollers.DeploymentCache
rkeControlPlanes rkecontroller.RKEControlPlaneCache
rkeBootstrapClient rkecontroller.RKEBootstrapClient
k8s kubernetes.Interface
}

func Register(ctx context.Context, clients *wrangler.Context) {
Expand All @@ -53,6 +55,7 @@ func Register(ctx context.Context, clients *wrangler.Context) {
deploymentCache: clients.Apps.Deployment().Cache(),
rkeControlPlanes: clients.RKE.RKEControlPlane().Cache(),
rkeBootstrapClient: clients.RKE.RKEBootstrap(),
k8s: clients.K8s,
}
rkecontroller.RegisterRKEBootstrapGeneratingHandler(ctx,
clients.RKE.RKEBootstrap(),
Expand Down Expand Up @@ -102,23 +105,9 @@ func (h *handler) getBootstrapSecret(namespace, name string, envVars []corev1.En
return nil, err

}
sName := serviceaccounttoken.ServiceAccountSecretName(sa)
secret, err := h.secretCache.Get(sa.Namespace, sName)
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), h.secretCache.Get, h.k8s, sa)
if err != nil {
if !apierror.IsNotFound(err) {
return nil, err
}
sc := serviceaccounttoken.SecretTemplate(sa)
secret, err = h.secretClient.Create(sc)
if err != nil {
if !apierror.IsAlreadyExists(err) {
return nil, err
}
secret, err = h.secretClient.Get(sa.Namespace, sName, metav1.GetOptions{})
if err != nil {
return nil, err
}
}
return nil, err
}
hash := sha256.Sum256(secret.Data["token"])

Expand Down
Expand Up @@ -19,6 +19,7 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/kubernetes/fake"
"sigs.k8s.io/cluster-api/api/v1beta1"
)

Expand Down Expand Up @@ -135,6 +136,7 @@ func Test_getBootstrapSecret(t *testing.T) {
secretCache: getSecretCacheMock(tt.args.namespaceName, tt.args.secretName),
deploymentCache: getDeploymentCacheMock(),
machineCache: getMachineCacheMock(tt.args.namespaceName, tt.args.os),
k8s: fake.NewSimpleClientset(),
}

//act
Expand Down Expand Up @@ -248,7 +250,7 @@ func getDeploymentCacheMock() *deploymentCacheMock {

func getSecretCacheMock(namespace, secretName string) *secretCacheMock {
mockSecretCache := new(secretCacheMock)
mockSecretCache.On("Get", namespace, secretName+"-token").Return(&v1.Secret{
mockSecretCache.On("Get", namespace, secretName).Return(&v1.Secret{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Expand Down
43 changes: 4 additions & 39 deletions pkg/controllers/provisioningv2/rke2/common.go
Expand Up @@ -22,9 +22,9 @@ import (
"github.com/rancher/wrangler/pkg/generic"
"github.com/rancher/wrangler/pkg/name"
corev1 "k8s.io/api/core/v1"
apierror "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/kubernetes"
capi "sigs.k8s.io/cluster-api/api/v1beta1"
capierrors "sigs.k8s.io/cluster-api/errors"
)
Expand Down Expand Up @@ -231,52 +231,17 @@ func GetPlanSecretName(planSA *corev1.ServiceAccount) (string, error) {

// GetPlanServiceAccountTokenSecret retrieves the secret that corresponds to the plan service account that is passed in. It will create a secret if one does not
// already exist for the plan service account.
func GetPlanServiceAccountTokenSecret(secretClient corecontrollers.SecretController, planSA *corev1.ServiceAccount) (*corev1.Secret, bool, error) {
func GetPlanServiceAccountTokenSecret(secretClient corecontrollers.SecretController, k8s kubernetes.Interface, planSA *corev1.ServiceAccount) (*corev1.Secret, bool, error) {
if planSA == nil {
return nil, false, fmt.Errorf("planSA was nil")
}
sName := serviceaccounttoken.ServiceAccountSecretName(planSA)
secret, err := secretClient.Cache().Get(planSA.Namespace, sName)
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), secretClient.Cache().Get, k8s, planSA)
if err != nil {
if !apierror.IsNotFound(err) {
return nil, false, err
}
sc := serviceaccounttoken.SecretTemplate(planSA)
secret, err = secretClient.Create(sc)
if err != nil {
if !apierror.IsAlreadyExists(err) {
return nil, false, err
}
secret, err = secretClient.Cache().Get(planSA.Namespace, sName)
if err != nil {
return nil, false, err
}
}
}
// wait for token to be populated
if !PlanServiceAccountTokenReady(planSA, secret) {
return secret, true, fmt.Errorf("planSA %s/%s token secret %s/%s was not ready for consumption yet", planSA.Namespace, planSA.Name, secret.Namespace, secret.Name)
return nil, false, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", planSA.Namespace, planSA.Name, err)
}
return secret, true, nil
}

func PlanServiceAccountTokenReady(planSA *corev1.ServiceAccount, tokenSecret *corev1.Secret) bool {
if planSA == nil || tokenSecret == nil {
return false
}
if tokenSecret.Name != serviceaccounttoken.ServiceAccountSecretName(planSA) {
return false
}
if v, ok := tokenSecret.Data[corev1.ServiceAccountTokenKey]; ok {
if len(v) == 0 {
return false
}
} else {
return false
}
return true
}

func PlanSecretFromBootstrapName(bootstrapName string) string {
return name.SafeConcatName(bootstrapName, "machine", "plan")
}
Expand Down
43 changes: 14 additions & 29 deletions pkg/impersonation/impersonation.go
Expand Up @@ -2,6 +2,7 @@
package impersonation

import (
"context"
"fmt"
"reflect"
"sort"
Expand Down Expand Up @@ -100,28 +101,9 @@ func (i *Impersonator) SetUpImpersonation() (*corev1.ServiceAccount, error) {

// GetToken accepts a service account and returns the service account's token.
func (i *Impersonator) GetToken(sa *corev1.ServiceAccount) (string, error) {
name := serviceaccounttoken.ServiceAccountSecretName(sa)
secret, err := i.clusterContext.Core.Secrets(ImpersonationNamespace).Get(name, metav1.GetOptions{})
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), i.clusterContext.Core.Secrets("").Controller().Lister().Get, i.clusterContext.K8sClient, sa)
if err != nil {
if !apierrors.IsNotFound(err) {
logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName)
return "", fmt.Errorf("error getting secret: %w", err)
}
// create the secret
sc := serviceaccounttoken.SecretTemplate(sa)
secret, err = i.clusterContext.Core.Secrets(ImpersonationNamespace).Create(sc)
if err != nil {
if !apierrors.IsAlreadyExists(err) {
logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName)
return "", fmt.Errorf("error creating secret: %w", err)
}
secret, err = i.clusterContext.Core.Secrets(ImpersonationNamespace).Get(name, metav1.GetOptions{})
if err != nil {
logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName)
logrus.Tracef("impersonation: secrret already exists")
return "", fmt.Errorf("error getting secret: %w", err)
}
}
return "", fmt.Errorf("error getting secret: %w", err)
}
token, ok := secret.Data["token"]
if !ok {
Expand Down Expand Up @@ -156,6 +138,9 @@ func (i *Impersonator) getServiceAccount() (*corev1.ServiceAccount, error) {
func (i *Impersonator) createServiceAccount(role *rbacv1.ClusterRole) (*corev1.ServiceAccount, error) {
name := ImpersonationPrefix + i.user.GetUID()
sa, err := i.clusterContext.Core.ServiceAccounts("").Controller().Lister().Get(ImpersonationNamespace, name)
if err != nil && !apierrors.IsNotFound(err) {
return nil, fmt.Errorf("impersonation: error getting service account [%s:%s]: %w", ImpersonationNamespace, name, err)
}
if apierrors.IsNotFound(err) {
logrus.Debugf("impersonation: creating service account %s", name)
sa, err = i.clusterContext.Core.ServiceAccounts(ImpersonationNamespace).Create(&corev1.ServiceAccount{
Expand All @@ -177,16 +162,16 @@ func (i *Impersonator) createServiceAccount(role *rbacv1.ClusterRole) (*corev1.S
// in case cache isn't synced yet, use raw client
sa, err = i.clusterContext.Core.ServiceAccounts(ImpersonationNamespace).Get(name, metav1.GetOptions{})
}
}
if sa != nil {
// create secret for service account
sc := serviceaccounttoken.SecretTemplate(sa)
_, secretErr := i.clusterContext.Core.Secrets(ImpersonationNamespace).Create(sc)
if secretErr != nil && !apierrors.IsAlreadyExists(secretErr) {
return nil, fmt.Errorf("impersonation: error creating secret for service account %s", name)
if err != nil {
return nil, fmt.Errorf("impersonation: error getting service account [%s:%s]: %w", ImpersonationNamespace, name, err)
}
}
return sa, err
// create secret for service account if it was not automatically generated
_, err = serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), i.clusterContext.Core.Secrets("").Controller().Lister().Get, i.clusterContext.K8sClient, sa)
if err != nil {
return nil, fmt.Errorf("impersonation: error ensuring secret for service account %s: %w", name, err)
}
return sa, nil
}

func (i *Impersonator) createNamespace() error {
Expand Down
25 changes: 7 additions & 18 deletions pkg/kontainer-engine/drivers/util/utils.go
Expand Up @@ -3,9 +3,7 @@ package util
import (
"context"
"fmt"
"time"

errs "github.com/pkg/errors"
"github.com/rancher/rancher/pkg/serviceaccounttoken"
v3 "github.com/rancher/rke/types"
"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -94,23 +92,14 @@ func GenerateServiceAccountToken(clientset kubernetes.Interface) (string, error)
return "", fmt.Errorf("error creating role bindings: %v", err)
}

start := time.Millisecond * 250
for i := 0; i < 5; i++ {
time.Sleep(start)
if serviceAccount, err = clientset.CoreV1().ServiceAccounts(cattleNamespace).Get(context.TODO(), serviceAccount.Name, metav1.GetOptions{}); err != nil {
return "", fmt.Errorf("error getting service account: %v", err)
}
secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.TODO(), clientset, serviceAccount)
if err != nil {
return "", fmt.Errorf("error creating secret for service account: %v", err)
}
if token, ok := secret.Data["token"]; ok {
return string(token), nil
}
start = start * 2
if serviceAccount, err = clientset.CoreV1().ServiceAccounts(cattleNamespace).Get(context.Background(), serviceAccount.Name, metav1.GetOptions{}); err != nil {
return "", fmt.Errorf("error getting service account: %w", err)
}

return "", errs.New("failed to fetch serviceAccountToken")
secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, clientset, serviceAccount)
if err != nil {
return "", fmt.Errorf("error ensuring secret for service account: %w", err)
}
return string(secret.Data["token"]), nil
}

func DeleteLegacyServiceAccountAndRoleBinding(clientset kubernetes.Interface) error {
Expand Down

0 comments on commit a30a596

Please sign in to comment.