WIP: BoundServiceAccountTokenVolume + service-ca.crt experiment

pkg/features/kube_features.go

@@ -786,7 +786,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	StorageObjectInUseProtection:                   {Default: true, PreRelease: featuregate.GA},
 	SupportPodPidsLimit:                            {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23
 	SupportNodePidsLimit:                           {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23
-	BoundServiceAccountTokenVolume:                 {Default: false, PreRelease: featuregate.Beta},                   // TODO(auth): investigate the impact of enabling this feature (https://bugzilla.redhat.com/show_bug.cgi?id=1946479)
+	BoundServiceAccountTokenVolume:                 {Default: true, PreRelease: featuregate.Beta},
 	ServiceAccountIssuerDiscovery:                  {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22
 	CRIContainerLogRotation:                        {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22
 	CSIMigration:                                   {Default: true, PreRelease: featuregate.Beta},

pkg/security/podsecuritypolicy/provider_test.go

@@ -1444,7 +1444,7 @@ func TestValidateProjectedVolume(t *testing.T) {
 		{
 			desc:                  "deny if secret is not allowed",
 			allowedFSTypes:        []policy.FSType{policy.EmptyDir},
-			projectedVolumeSource: serviceaccount.TokenVolumeSource(),
+			projectedVolumeSource: serviceaccount.TokenVolumeSource("service-account-token-secret"),
 			wantAllow:             false,
 		},
 		{
@@ -1471,7 +1471,7 @@ func TestValidateProjectedVolume(t *testing.T) {
 		{
 			desc:                  "allow if secret is allowed and the projected volume sources equals to the ones injected by service account admission plugin",
 			allowedFSTypes:        []policy.FSType{policy.Secret},
-			projectedVolumeSource: serviceaccount.TokenVolumeSource(),
+			projectedVolumeSource: serviceaccount.TokenVolumeSource("service-account-token-secret"),
 			wantAllow:             true,
 		},
 	}

pkg/security/podsecuritypolicy/util/util.go

@@ -23,6 +23,7 @@ import (
 	policy "k8s.io/api/policy/v1beta1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	controllerserviceaccount "k8s.io/kubernetes/pkg/controller/serviceaccount"
 )
 
 const (
@@ -253,7 +254,7 @@ func EqualStringSlices(a, b []string) bool {
 func IsOnlyServiceAccountTokenSources(v *api.ProjectedVolumeSource) bool {
 	for _, s := range v.Sources {
 		// reject any projected source that does not match any of our expected source types
-		if s.ServiceAccountToken == nil && s.ConfigMap == nil && s.DownwardAPI == nil {
+		if s.ServiceAccountToken == nil && s.ConfigMap == nil && s.DownwardAPI == nil && s.Secret == nil {
 			return false
 		}
 		if t := s.ServiceAccountToken; t != nil && (t.Path != "token" || t.Audience != "") {
@@ -271,6 +272,15 @@ func IsOnlyServiceAccountTokenSources(v *api.ProjectedVolumeSource) bool {
 				}
 			}
 		}
+
+		if s.Secret != nil { // FIXME? We don’t check the secret projection’s LocalObjectReference because the caller is allowed to use Secret volume type directly anyway
+			for _, d := range s.Secret.Items {
+				if d.Path != controllerserviceaccount.ServiceServingCASecretKey {
+					return false
+				}
+			}
+
+		}
 	}
 	return true
 }

pkg/security/podsecuritypolicy/util/util_test.go

@@ -285,6 +285,19 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 			},
 		},
 	}
+	secret := api.VolumeProjection{
+		Secret: &api.SecretProjection{
+			LocalObjectReference: api.LocalObjectReference{
+				Name: "some-secret-that-is-allowed-anyway",
+			},
+			Items: []api.KeyToPath{
+				{
+					Key:  "service-ca.crt",
+					Path: "service-ca.crt",
+				},
+			},
+		},
+	}
 
 	tests := []struct {
 		desc   string
@@ -300,6 +313,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 						ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds,
 					}},
 					configMap,
+					secret,
 					downwardAPI,
 				},
 			},
@@ -314,6 +328,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 						ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds,
 					}},
 					configMap,
+					secret,
 					downwardAPI,
 				},
 			},
@@ -336,6 +351,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 							},
 						},
 					},
+					secret,
 					downwardAPI,
 				},
 			},
@@ -346,6 +362,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 				Sources: []api.VolumeProjection{
 					serviceAccountToken,
 					configMap,
+					secret,
 					{
 						DownwardAPI: &api.DownwardAPIProjection{
 							Items: []api.DownwardAPIVolumeFile{
@@ -368,6 +385,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 				Sources: []api.VolumeProjection{
 					serviceAccountToken,
 					configMap,
+					secret,
 					{
 						DownwardAPI: &api.DownwardAPIProjection{
 							Items: []api.DownwardAPIVolumeFile{
@@ -386,6 +404,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 				Sources: []api.VolumeProjection{
 					serviceAccountToken,
 					configMap,
+					secret,
 					{
 						DownwardAPI: &api.DownwardAPIProjection{
 							Items: []api.DownwardAPIVolumeFile{
@@ -408,6 +427,7 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 				Sources: []api.VolumeProjection{
 					serviceAccountToken,
 					configMap,
+					secret,
 					{
 						DownwardAPI: &api.DownwardAPIProjection{
 							Items: []api.DownwardAPIVolumeFile{
@@ -425,33 +445,44 @@ func TestIsOnlyServiceAccountTokenSources(t *testing.T) {
 			},
 		},
 		{
-			desc: "deny if Secret exists",
+			desc: "deny if Secret has wrong field path",
 			volume: &api.ProjectedVolumeSource{
 				Sources: []api.VolumeProjection{
-					{
-						Secret: &api.SecretProjection{},
-					},
 					configMap,
 					downwardAPI,
 					serviceAccountToken,
+					{
+						Secret: &api.SecretProjection{
+							LocalObjectReference: api.LocalObjectReference{
+								Name: "some-secret-that-is-allowed-anyway",
+							},
+							Items: []api.KeyToPath{
+								{
+									Key:  "not-service-ca.crt",
+									Path: "not-service-ca.crt",
+								},
+							},
+						},
+					},
 				},
 			},
 		},
 		{
-			desc: "deny if none of ServiceAccountToken, ConfigMap and DownwardAPI exist",
+			desc: "deny if none of ServiceAccountToken, ConfigMap, DownwardAPI and Secret exist",
 			volume: &api.ProjectedVolumeSource{
 				Sources: []api.VolumeProjection{
 					{},
 				},
 			},
 		},
 		{
-			desc: "allow if any of ServiceAccountToken, ConfigMap and DownwardAPI matches",
+			desc: "allow if any of ServiceAccountToken, ConfigMap, DownwardAPI and Secret matches",
 			volume: &api.ProjectedVolumeSource{
 				Sources: []api.VolumeProjection{
 					configMap,
 					downwardAPI,
 					serviceAccountToken,
+					secret,
 				},
 			},
 			want: true,

plugin/pkg/admission/serviceaccount/admission.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/component-base/featuregate"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	controllerserviceaccount "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 )
@@ -328,29 +329,32 @@ func (s *Plugin) getServiceAccount(namespace string, name string) (*corev1.Servi
 	return nil, errors.NewNotFound(api.Resource("serviceaccount"), name)
 }
 
-// getReferencedServiceAccountToken returns the name of the first referenced secret which is a ServiceAccountToken for the service account
-func (s *Plugin) getReferencedServiceAccountToken(serviceAccount *corev1.ServiceAccount) (string, error) {
+// getReferencedServiceAccountToken returns the name of the first referenced secret which is a ServiceAccountToken for the service account, and whether it contains service-ca.crt.
+func (s *Plugin) getReferencedServiceAccountToken(serviceAccount *corev1.ServiceAccount) (string, bool, error) {
 	if len(serviceAccount.Secrets) == 0 {
-		return "", nil
+		return "", false, nil
 	}
 
 	tokens, err := s.getServiceAccountTokens(serviceAccount)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
 
 	accountTokens := sets.NewString()
+	hasServiceServingCA := map[string]bool{}
 	for _, token := range tokens {
 		accountTokens.Insert(token.Name)
+		_, ok := token.Data[controllerserviceaccount.ServiceServingCASecretKey] // This may be false during bootstrap.
+		hasServiceServingCA[token.Name] = ok
 	}
 	// Prefer secrets in the order they're referenced.
 	for _, secret := range serviceAccount.Secrets {
 		if accountTokens.Has(secret.Name) {
-			return secret.Name, nil
+			return secret.Name, hasServiceServingCA[secret.Name], nil
 		}
 	}
 
-	return "", nil
+	return "", false, nil
 }
 
 // getServiceAccountTokens returns all ServiceAccountToken secrets for the given ServiceAccount
@@ -430,12 +434,10 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount,
 		serviceAccountToken string
 		err                 error
 	)
-	if !s.boundServiceAccountTokenVolume {
-		// Find the name of a referenced ServiceAccountToken secret we can mount
-		serviceAccountToken, err = s.getReferencedServiceAccountToken(serviceAccount)
-		if err != nil {
-			return fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err)
-		}
+	// Find the name of a referenced ServiceAccountToken secret we can mount
+	serviceAccountToken, hasServiceServingCA, err := s.getReferencedServiceAccountToken(serviceAccount)
+	if err != nil {
+		return fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err)
 	}
 	if len(serviceAccountToken) == 0 && !s.boundServiceAccountTokenVolume {
 		// We don't have an API token to mount, so return
@@ -516,17 +518,20 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount,
 
 	// Add the volume if a container needs it
 	if !hasTokenVolume && needsTokenVolume {
-		pod.Spec.Volumes = append(pod.Spec.Volumes, s.createVolume(tokenVolumeName, serviceAccountToken))
+		pod.Spec.Volumes = append(pod.Spec.Volumes, s.createVolume(tokenVolumeName, serviceAccountToken, hasServiceServingCA))
 	}
 	return nil
 }
 
-func (s *Plugin) createVolume(tokenVolumeName, secretName string) api.Volume {
+func (s *Plugin) createVolume(tokenVolumeName, secretName string, hasServiceServingCA bool) api.Volume {
 	if s.boundServiceAccountTokenVolume {
+		if !hasServiceServingCA {
+			secretName = ""
+		}
 		return api.Volume{
 			Name: tokenVolumeName,
 			VolumeSource: api.VolumeSource{
-				Projected: TokenVolumeSource(),
+				Projected: TokenVolumeSource(secretName),
 			},
 		}
 	}
@@ -541,8 +546,8 @@ func (s *Plugin) createVolume(tokenVolumeName, secretName string) api.Volume {
 }
 
 // TokenVolumeSource returns the projected volume source for service account token.
-func TokenVolumeSource() *api.ProjectedVolumeSource {
-	return &api.ProjectedVolumeSource{
+func TokenVolumeSource(serviceAccountTokenSecretName string) *api.ProjectedVolumeSource {
+	res := &api.ProjectedVolumeSource{
 		Sources: []api.VolumeProjection{
 			{
 				ServiceAccountToken: &api.ServiceAccountTokenProjection{
@@ -578,4 +583,20 @@ func TokenVolumeSource() *api.ProjectedVolumeSource {
 			},
 		},
 	}
+	if serviceAccountTokenSecretName != "" {
+		res.Sources = append(res.Sources, api.VolumeProjection{
+			Secret: &api.SecretProjection{
+				LocalObjectReference: api.LocalObjectReference{
+					Name: serviceAccountTokenSecretName,
+				},
+				Items: []api.KeyToPath{
+					{
+						Key:  controllerserviceaccount.ServiceServingCASecretKey,
+						Path: controllerserviceaccount.ServiceServingCASecretKey,
+					},
+				},
+			},
+		})
+	}
+	return res
 }

plugin/pkg/admission/serviceaccount/admission_test.go

@@ -220,6 +220,7 @@ func TestAssignsDefaultServiceAccountAndRejectsMissingAPIToken(t *testing.T) {
 
 func TestAssignsDefaultServiceAccountAndBoundTokenWithNoSecretTokens(t *testing.T) {
 	ns := "myns"
+	serviceAccountTokenSecretName := "some-service-account-token-secret"
 
 	admit := NewServiceAccount()
 	informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc())
@@ -234,6 +235,22 @@ func TestAssignsDefaultServiceAccountAndBoundTokenWithNoSecretTokens(t *testing.
 			Name:      DefaultServiceAccountName,
 			Namespace: ns,
 		},
+		Secrets: []corev1.ObjectReference{
+			{
+				Name: serviceAccountTokenSecretName,
+			},
+		},
+	})
+	informerFactory.Core().V1().Secrets().Informer().GetStore().Add(&corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      serviceAccountTokenSecretName,
+			Namespace: ns,
+			Annotations: map[string]string{
+				corev1.ServiceAccountNameKey: DefaultServiceAccountName,
+			},
+		},
+		Type: corev1.SecretTypeServiceAccountToken,
+		Data: map[string][]byte{"service-ca.crt": []byte("contents")},
 	})
 
 	pod := &api.Pod{
@@ -255,6 +272,7 @@ func TestAssignsDefaultServiceAccountAndBoundTokenWithNoSecretTokens(t *testing.
 					{ServiceAccountToken: &api.ServiceAccountTokenProjection{ExpirationSeconds: 3607, Path: "token"}},
 					{ConfigMap: &api.ConfigMapProjection{LocalObjectReference: api.LocalObjectReference{Name: "kube-root-ca.crt"}, Items: []api.KeyToPath{{Key: "ca.crt", Path: "ca.crt"}}}},
 					{DownwardAPI: &api.DownwardAPIProjection{Items: []api.DownwardAPIVolumeFile{{Path: "namespace", FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.namespace"}}}}},
+					{Secret: &api.SecretProjection{LocalObjectReference: api.LocalObjectReference{Name: serviceAccountTokenSecretName}, Items: []api.KeyToPath{{Key: "service-ca.crt", Path: "service-ca.crt"}}}},
 				},
 			},
 		},
@@ -351,7 +369,7 @@ func TestAutomountsAPIToken(t *testing.T) {
 			tokenName = generatedVolumeName
 		}
 
-		expectedVolume := admit.createVolume(tokenName, tokenName)
+		expectedVolume := admit.createVolume(tokenName, tokenName, false)
 		expectedVolumeMount := api.VolumeMount{
 			Name:      tokenName,
 			ReadOnly:  true,