From 613a71271768df45f0558cd0828068ae792e0856 Mon Sep 17 00:00:00 2001 From: Shihang Zhang Date: Wed, 10 Jun 2020 13:35:17 -0700 Subject: [PATCH] default to add projected fstype in psp when boundedserviceaccounttoken is enabled --- pkg/security/podsecuritypolicy/BUILD | 2 + pkg/security/podsecuritypolicy/provider.go | 15 +- .../podsecuritypolicy/provider_test.go | 61 +++++ pkg/security/podsecuritypolicy/util/BUILD | 1 + pkg/security/podsecuritypolicy/util/util.go | 25 ++ .../podsecuritypolicy/util/util_test.go | 218 ++++++++++++++++++ .../pkg/admission/serviceaccount/admission.go | 77 ++++--- 7 files changed, 362 insertions(+), 37 deletions(-) diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD index 15a1df6789a..eb60ea12175 100644 --- a/pkg/security/podsecuritypolicy/BUILD +++ b/pkg/security/podsecuritypolicy/BUILD @@ -32,6 +32,7 @@ go_library( "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", ], @@ -48,6 +49,7 @@ go_test( "//pkg/security/apparmor:go_default_library", "//pkg/security/podsecuritypolicy/seccomp:go_default_library", "//pkg/security/podsecuritypolicy/util:go_default_library", + "//plugin/pkg/admission/serviceaccount:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index a995346d589..babad966abe 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1beta1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" podutil "k8s.io/kubernetes/pkg/api/pod" @@ -259,7 +260,7 @@ func (s *simpleProvider) validatePodVolumes(pod *api.Pod) field.ErrorList { continue } - if !allowsAllVolumeTypes && !allowedVolumes.Has(string(fsType)) { + if !allowsAllVolumeTypes && !allowsVolumeType(allowedVolumes, fsType, v) { allErrs = append(allErrs, field.Invalid( field.NewPath("spec", "volumes").Index(i), string(fsType), fmt.Sprintf("%s volumes are not allowed to be used", string(fsType)))) @@ -449,3 +450,15 @@ func validateRuntimeClassName(actual *string, validNames []string) field.ErrorLi } return field.ErrorList{field.Invalid(field.NewPath("spec", "runtimeClassName"), *actual, "")} } + +func allowsVolumeType(allowedVolumes sets.String, fsType policy.FSType, volume api.Volume) bool { + if allowedVolumes.Has(string(fsType)) { + return true + } + + // if secret volume is allowed, all the projected volume sources that projected service account token volumes expose are allowed, regardless of psp. + if allowedVolumes.Has(string(policy.Secret)) && fsType == policy.Projected && psputil.IsOnlyServiceAccountTokenSources(volume.VolumeSource.Projected) { + return true + } + return false +} diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index f4e89f7ebb1..3dbef70e05a 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -37,6 +37,7 @@ import ( "k8s.io/kubernetes/pkg/security/apparmor" "k8s.io/kubernetes/pkg/security/podsecuritypolicy/seccomp" psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util" + "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount" "k8s.io/utils/pointer" ) @@ -1373,6 +1374,66 @@ func TestValidateAllowedVolumes(t *testing.T) { } } +func TestValidateProjectedVolume(t *testing.T) { + pod := defaultPod() + psp := defaultPSP() + provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "error creating provider") + + tests := []struct { + desc string + allowedFSTypes []policy.FSType + projectedVolumeSource *api.ProjectedVolumeSource + wantAllow bool + }{ + { + desc: "deny if secret is not allowed", + allowedFSTypes: []policy.FSType{policy.EmptyDir}, + projectedVolumeSource: serviceaccount.TokenVolumeSource(), + wantAllow: false, + }, + { + desc: "deny if the projected volume has volume source other than the ones in projected volume injected by service account token admission plugin", + allowedFSTypes: []policy.FSType{policy.Secret}, + projectedVolumeSource: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + { + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo-ca.crt", + }, + Items: []api.KeyToPath{ + { + Key: "ca.crt", + Path: "ca.crt", + }, + }, + }, + }, + }}, + wantAllow: false, + }, + { + 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(), + wantAllow: true, + }, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + pod.Spec.Volumes = []api.Volume{{VolumeSource: api.VolumeSource{Projected: test.projectedVolumeSource}}} + psp.Spec.Volumes = test.allowedFSTypes + errs := provider.ValidatePod(pod) + if test.wantAllow { + assert.Empty(t, errs, "projected volumes are allowed if secret volumes is allowed and BoundServiceAccountTokenVolume is enabled") + } else { + assert.Contains(t, errs.ToAggregate().Error(), fmt.Sprintf("projected volumes are not allowed to be used"), "did not find the expected error") + } + }) + } +} + func TestAllowPrivilegeEscalation(t *testing.T) { ptr := pointer.BoolPtr tests := []struct { diff --git a/pkg/security/podsecuritypolicy/util/BUILD b/pkg/security/podsecuritypolicy/util/BUILD index 7e4100e0095..64421e26912 100644 --- a/pkg/security/podsecuritypolicy/util/BUILD +++ b/pkg/security/podsecuritypolicy/util/BUILD @@ -26,6 +26,7 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core:go_default_library", + "//pkg/serviceaccount:go_default_library", "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", ], ) diff --git a/pkg/security/podsecuritypolicy/util/util.go b/pkg/security/podsecuritypolicy/util/util.go index 3620adca7a0..6f0b3706d33 100644 --- a/pkg/security/podsecuritypolicy/util/util.go +++ b/pkg/security/podsecuritypolicy/util/util.go @@ -242,3 +242,28 @@ func EqualStringSlices(a, b []string) bool { } return true } + +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 { + return false + } + if t := s.ServiceAccountToken; t != nil && (t.Path != "token" || t.Audience != "") { + return false + } + + if s.ConfigMap != nil && s.ConfigMap.LocalObjectReference.Name != "kube-root-ca.crt" { + return false + } + + if s.DownwardAPI != nil { + for _, d := range s.DownwardAPI.Items { + if d.Path != "namespace" || d.FieldRef == nil || d.FieldRef.APIVersion != "v1" || d.FieldRef.FieldPath != "metadata.namespace" { + return false + } + } + } + } + return true +} diff --git a/pkg/security/podsecuritypolicy/util/util_test.go b/pkg/security/podsecuritypolicy/util/util_test.go index f495e65e1a7..ceef7dc0dc9 100644 --- a/pkg/security/podsecuritypolicy/util/util_test.go +++ b/pkg/security/podsecuritypolicy/util/util_test.go @@ -22,6 +22,7 @@ import ( policy "k8s.io/api/policy/v1beta1" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/serviceaccount" ) // TestVolumeSourceFSTypeDrift ensures that for every known type of volume source (by the fields on @@ -251,3 +252,220 @@ func TestEqualStringSlices(t *testing.T) { } } } + +func TestIsOnlyServiceAccountTokenSources(t *testing.T) { + serviceAccountToken := api.VolumeProjection{ + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "token", + ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds, + }} + configMap := api.VolumeProjection{ + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{ + Name: "kube-root-ca.crt", + }, + Items: []api.KeyToPath{ + { + Key: "ca.crt", + Path: "ca.crt", + }, + }, + }, + } + downwardAPI := api.VolumeProjection{ + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "namespace", + FieldRef: &api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, + }, + } + + tests := []struct { + desc string + volume *api.ProjectedVolumeSource + want bool + }{ + { + desc: "deny if ServiceAccountToken has wrong path", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "notatoken", + ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds, + }}, + configMap, + downwardAPI, + }, + }, + }, + { + desc: "deny if ServiceAccountToken has wrong audience", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "token", + Audience: "not api server", + ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds, + }}, + configMap, + downwardAPI, + }, + }, + }, + { + desc: "deny if CondigMap has wrong LocalObjectReference.Name", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + serviceAccountToken, + { + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo-ca.crt", + }, + Items: []api.KeyToPath{ + { + Key: "ca.crt", + Path: "ca.crt", + }, + }, + }, + }, + downwardAPI, + }, + }, + }, + { + desc: "deny if DownwardAPI has wrong path", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + serviceAccountToken, + configMap, + { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "foo", + FieldRef: &api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "deny if DownwardAPI has nil field ref", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + serviceAccountToken, + configMap, + { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "namespace", + }, + }, + }, + }, + }, + }, + }, + { + desc: "deny if DownwardAPI has wrong api version", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + serviceAccountToken, + configMap, + { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "namespace", + FieldRef: &api.ObjectFieldSelector{ + APIVersion: "v1beta1", + FieldPath: "metadata.namespace", + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "deny if DownwardAPI has wrong field path", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + serviceAccountToken, + configMap, + { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "namespace", + FieldRef: &api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.foo", + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "deny if Secret exists", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + { + Secret: &api.SecretProjection{}, + }, + configMap, + downwardAPI, + serviceAccountToken, + }, + }, + }, + { + desc: "deny if none of ServiceAccountToken, ConfigMap and DownwardAPI exist", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + {}, + }, + }, + }, + { + desc: "allow if any of ServiceAccountToken, ConfigMap and DownwardAPI matches", + volume: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + configMap, + downwardAPI, + serviceAccountToken, + }, + }, + want: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + if got := IsOnlyServiceAccountTokenSources(test.volume); got != test.want { + t.Errorf("IsOnlyServiceAccountTokenSources(%+v) = %v, want %v", test.volume, got, test.want) + } + }) + } +} diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 19f633c747f..2b85ddd3f18 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -525,42 +525,7 @@ func (s *Plugin) createVolume(tokenVolumeName, secretName string) api.Volume { return api.Volume{ Name: tokenVolumeName, VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "token", - ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds, - }, - }, - { - 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", - }, - }, - }, - }, - }, - }, - }, + Projected: TokenVolumeSource(), }, } } @@ -573,3 +538,43 @@ 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{ + Sources: []api.VolumeProjection{ + { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "token", + ExpirationSeconds: serviceaccount.WarnOnlyBoundTokenExpirationSeconds, + }, + }, + { + 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", + }, + }, + }, + }, + }, + }, + } +}