From 8e0cf65310dd9ccfc58d25d1a29ebc1268b9eef1 Mon Sep 17 00:00:00 2001 From: Vladimir Vivien Date: Mon, 22 Apr 2019 18:15:47 -0500 Subject: [PATCH] Enforce pod security policy for CSI inline --- pkg/apis/policy/types.go | 3 +- pkg/apis/policy/validation/validation_test.go | 22 ++++ pkg/kubectl/describe/versioned/describe.go | 13 ++ pkg/security/podsecuritypolicy/BUILD | 3 + pkg/security/podsecuritypolicy/provider.go | 83 ++++++++----- .../podsecuritypolicy/provider_test.go | 112 +++++++++++++++++- .../security/podsecuritypolicy/BUILD | 3 + .../podsecuritypolicy/admission_test.go | 5 + .../k8s.io/api/extensions/v1beta1/types.go | 3 +- .../src/k8s.io/api/policy/v1beta1/types.go | 5 +- 10 files changed, 220 insertions(+), 32 deletions(-) diff --git a/pkg/apis/policy/types.go b/pkg/apis/policy/types.go index fec6281a23..1691d3a0c3 100644 --- a/pkg/apis/policy/types.go +++ b/pkg/apis/policy/types.go @@ -209,7 +209,8 @@ type PodSecurityPolicySpec struct { // +optional AllowedFlexVolumes []AllowedFlexVolume // AllowedCSIDrivers is a whitelist of inline CSI drivers that must be explicitly set to be embedded within a pod spec. - // An empty value means no CSI drivers can run inline within a pod spec. + // An empty value indicates that any CSI driver can be used for inline ephemeral volumes. + // This is an alpha field, and is only honored if the API server enables the CSIInlineVolume feature gate. // +optional AllowedCSIDrivers []AllowedCSIDriver // AllowedUnsafeSysctls is a list of explicitly allowed unsafe sysctls, defaults to none. diff --git a/pkg/apis/policy/validation/validation_test.go b/pkg/apis/policy/validation/validation_test.go index c390097bcc..e521c41849 100644 --- a/pkg/apis/policy/validation/validation_test.go +++ b/pkg/apis/policy/validation/validation_test.go @@ -281,6 +281,10 @@ func TestValidatePodSecurityPolicy(t *testing.T) { invalidProcMount := validPSP() invalidProcMount.Spec.AllowedProcMountTypes = []api.ProcMountType{api.ProcMountType("bogus")} + allowedCSIDriverPSP := validPSP() + allowedCSIDriverPSP.Spec.Volumes = []policy.FSType{policy.CSI} + allowedCSIDriverPSP.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{}} + type testCase struct { psp *policy.PodSecurityPolicy errorType field.ErrorType @@ -447,6 +451,10 @@ func TestValidatePodSecurityPolicy(t *testing.T) { errorType: field.ErrorTypeRequired, errorDetail: "must specify a driver", }, + "CSI policy with empty allowed driver list": { + psp: allowedCSIDriverPSP, + errorType: field.ErrorTypeRequired, + }, "invalid allowedProcMountTypes": { psp: invalidProcMount, errorType: field.ErrorTypeNotSupported, @@ -549,6 +557,14 @@ func TestValidatePodSecurityPolicy(t *testing.T) { validProcMount := validPSP() validProcMount.Spec.AllowedProcMountTypes = []api.ProcMountType{api.DefaultProcMount, api.UnmaskedProcMount} + allowedCSIDriversWithCSIFsType := validPSP() + allowedCSIDriversWithCSIFsType.Spec.Volumes = []policy.FSType{policy.CSI} + allowedCSIDriversWithCSIFsType.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "foo"}} + + allowedCSIDriversWithAllFsTypes := validPSP() + allowedCSIDriversWithAllFsTypes.Spec.Volumes = []policy.FSType{policy.All} + allowedCSIDriversWithAllFsTypes.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "bar"}} + successCases := map[string]struct { psp *policy.PodSecurityPolicy }{ @@ -591,6 +607,12 @@ func TestValidatePodSecurityPolicy(t *testing.T) { "valid allowedProcMountTypes": { psp: validProcMount, }, + "allowed CSI drivers when FSType policy is set to CSI": { + psp: allowedCSIDriversWithCSIFsType, + }, + "allowed CSI drivers when FSType policy is set to All": { + psp: allowedCSIDriversWithAllFsTypes, + }, } for k, v := range successCases { diff --git a/pkg/kubectl/describe/versioned/describe.go b/pkg/kubectl/describe/versioned/describe.go index ca8373d56b..0ad7e070b3 100644 --- a/pkg/kubectl/describe/versioned/describe.go +++ b/pkg/kubectl/describe/versioned/describe.go @@ -3856,6 +3856,11 @@ func describePodSecurityPolicy(psp *policyv1beta1.PodSecurityPolicy) (string, er if len(psp.Spec.AllowedFlexVolumes) > 0 { w.Write(LEVEL_1, "Allowed FlexVolume Types:\t%s\n", flexVolumesToString(psp.Spec.AllowedFlexVolumes)) } + + if len(psp.Spec.AllowedCSIDrivers) > 0 { + w.Write(LEVEL_1, "Allowed CSI Drivers:\t%s\n", csiDriversToString(psp.Spec.AllowedCSIDrivers)) + } + if len(psp.Spec.AllowedUnsafeSysctls) > 0 { w.Write(LEVEL_1, "Allowed Unsafe Sysctls:\t%s\n", sysctlsToString(psp.Spec.AllowedUnsafeSysctls)) } @@ -3921,6 +3926,14 @@ func flexVolumesToString(flexVolumes []policyv1beta1.AllowedFlexVolume) string { return stringOrDefaultValue(strings.Join(volumes, ","), "") } +func csiDriversToString(csiDrivers []policyv1beta1.AllowedCSIDriver) string { + drivers := []string{} + for _, csiDriver := range csiDrivers { + drivers = append(drivers, "driver="+csiDriver.Name) + } + return stringOrDefaultValue(strings.Join(drivers, ","), "") +} + func sysctlsToString(sysctls []string) string { return stringOrNone(strings.Join(sysctls, ",")) } diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD index b4031c320d..fb7715cc5a 100644 --- a/pkg/security/podsecuritypolicy/BUILD +++ b/pkg/security/podsecuritypolicy/BUILD @@ -42,6 +42,7 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", + "//pkg/features:go_default_library", "//pkg/security/apparmor:go_default_library", "//pkg/security/podsecuritypolicy/seccomp:go_default_library", "//pkg/security/podsecuritypolicy/util:go_default_library", @@ -49,6 +50,8 @@ go_test( "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index b67014dd70..fc33afc5dc 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -233,6 +233,28 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList { allErrs = append(allErrs, s.strategies.SysctlsStrategy.Validate(pod)...) + allErrs = append(allErrs, s.validatePodVolumes(pod)...) + + if s.psp.Spec.RuntimeClass != nil { + allErrs = append(allErrs, validateRuntimeClassName(pod.Spec.RuntimeClassName, s.psp.Spec.RuntimeClass.AllowedRuntimeClassNames)...) + } + + fldPath := field.NewPath("spec", "initContainers") + for i := range pod.Spec.InitContainers { + allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.InitContainers[i], fldPath.Index(i))...) + } + + fldPath = field.NewPath("spec", "containers") + for i := range pod.Spec.Containers { + allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.Containers[i], fldPath.Index(i))...) + } + + return allErrs +} + +func (s *simpleProvider) validatePodVolumes(pod *api.Pod) field.ErrorList { + allErrs := field.ErrorList{} + if len(pod.Spec.Volumes) > 0 { allowsAllVolumeTypes := psputil.PSPAllowsAllVolumes(s.psp) allowedVolumes := psputil.FSTypeToStringSet(s.psp.Spec.Volumes) @@ -250,7 +272,8 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList { continue } - if fsType == policy.HostPath { + switch fsType { + case policy.HostPath: allows, mustBeReadOnly := psputil.AllowsHostVolumePath(s.psp, v.HostPath.Path) if !allows { allErrs = append(allErrs, field.Invalid( @@ -279,40 +302,46 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList { } } } - } - if fsType == policy.FlexVolume && len(s.psp.Spec.AllowedFlexVolumes) > 0 { - found := false - driver := v.FlexVolume.Driver - for _, allowedFlexVolume := range s.psp.Spec.AllowedFlexVolumes { - if driver == allowedFlexVolume.Driver { - found = true - break + case policy.FlexVolume: + if len(s.psp.Spec.AllowedFlexVolumes) > 0 { + found := false + driver := v.FlexVolume.Driver + for _, allowedFlexVolume := range s.psp.Spec.AllowedFlexVolumes { + if driver == allowedFlexVolume.Driver { + found = true + break + } + } + if !found { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "volumes").Index(i).Child("driver"), driver, + "Flexvolume driver is not allowed to be used")) } } - if !found { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "volumes").Index(i).Child("driver"), driver, - "Flexvolume driver is not allowed to be used")) + + case policy.CSI: + if utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { + if len(s.psp.Spec.AllowedCSIDrivers) > 0 { + found := false + driver := v.CSI.Driver + for _, allowedCSIDriver := range s.psp.Spec.AllowedCSIDrivers { + if driver == allowedCSIDriver.Name { + found = true + break + } + } + if !found { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "volumes").Index(i).Child("csi", "driver"), driver, + "Inline CSI driver is not allowed to be used")) + } + } } } } } - if s.psp.Spec.RuntimeClass != nil { - allErrs = append(allErrs, validateRuntimeClassName(pod.Spec.RuntimeClassName, s.psp.Spec.RuntimeClass.AllowedRuntimeClassNames)...) - } - - fldPath := field.NewPath("spec", "initContainers") - for i := range pod.Spec.InitContainers { - allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.InitContainers[i], fldPath.Index(i))...) - } - - fldPath = field.NewPath("spec", "containers") - for i := range pod.Spec.Containers { - allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.Containers[i], fldPath.Index(i))...) - } - return allErrs } diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index cfac1cba44..62ce971ebd 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -25,12 +25,15 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/security/apparmor" "k8s.io/kubernetes/pkg/security/podsecuritypolicy/seccomp" psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util" @@ -172,6 +175,8 @@ func TestMutateContainerNonmutating(t *testing.T) { } func TestValidatePodFailures(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + failHostNetworkPod := defaultPod() failHostNetworkPod.Spec.SecurityContext.HostNetwork = true @@ -328,6 +333,18 @@ func TestValidatePodFailures(t *testing.T) { }, } + failCSIDriverPod := defaultPod() + failCSIDriverPod.Spec.Volumes = []api.Volume{ + { + Name: "csi volume pod", + VolumeSource: api.VolumeSource{ + CSI: &api.CSIVolumeSource{ + Driver: "csi.driver.foo", + }, + }, + }, + } + errorCases := map[string]struct { pod *api.Pod psp *policy.PodSecurityPolicy @@ -433,6 +450,40 @@ func TestValidatePodFailures(t *testing.T) { psp: allowFlexVolumesPSP(false, true), expectedError: "Flexvolume driver is not allowed to be used", }, + "CSI policy using disallowed CDI driver": { + pod: failCSIDriverPod, + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.Volumes = []policy.FSType{policy.CSI} + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "csi.driver.disallowed"}} + return psp + }(), + expectedError: "Inline CSI driver is not allowed to be used", + }, + "Using inline CSI driver with no policy specified": { + pod: failCSIDriverPod, + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "csi.driver.foo"}} + return psp + }(), + expectedError: "csi volumes are not allowed to be used", + }, + "policy.All using disallowed CDI driver": { + pod: failCSIDriverPod, + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.Volumes = []policy.FSType{policy.All} + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "csi.driver.disallowed"}} + return psp + }(), + expectedError: "Inline CSI driver is not allowed to be used", + }, + "CSI inline volumes without proper policy set": { + pod: failCSIDriverPod, + psp: defaultPSP(), + expectedError: "csi volumes are not allowed to be used", + }, } for name, test := range errorCases { t.Run(name, func(t *testing.T) { @@ -616,6 +667,8 @@ func TestValidateContainerFailures(t *testing.T) { } func TestValidatePodSuccess(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + hostNetworkPSP := defaultPSP() hostNetworkPSP.Spec.HostNetwork = true hostNetworkPod := defaultPod() @@ -806,6 +859,34 @@ func TestValidatePodSuccess(t *testing.T) { }, } + csiDriverPod := defaultPod() + csiDriverPod.Spec.Volumes = []api.Volume{ + { + Name: "csi inline driver", + VolumeSource: api.VolumeSource{ + CSI: &api.CSIVolumeSource{ + Driver: "foo", + }, + }, + }, + { + Name: "csi inline driver 2", + VolumeSource: api.VolumeSource{ + CSI: &api.CSIVolumeSource{ + Driver: "bar", + }, + }, + }, + { + Name: "csi inline driver 3", + VolumeSource: api.VolumeSource{ + CSI: &api.CSIVolumeSource{ + Driver: "baz", + }, + }, + }, + } + successCases := map[string]struct { pod *api.Pod psp *policy.PodSecurityPolicy @@ -886,6 +967,33 @@ func TestValidatePodSuccess(t *testing.T) { pod: flexVolumePod, psp: allowFlexVolumesPSP(true, false), }, + "CSI policy with no CSI volumes used": { + pod: defaultPod(), + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.Volumes = []policy.FSType{policy.CSI} + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "foo"}, {Name: "bar"}, {Name: "baz"}} + return psp + }(), + }, + "CSI policy with CSI inline volumes used": { + pod: csiDriverPod, + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.Volumes = []policy.FSType{policy.CSI} + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "foo"}, {Name: "bar"}, {Name: "baz"}} + return psp + }(), + }, + "policy.All with CSI inline volumes used": { + pod: csiDriverPod, + psp: func() *policy.PodSecurityPolicy { + psp := defaultPSP() + psp.Spec.Volumes = []policy.FSType{policy.All} + psp.Spec.AllowedCSIDrivers = []policy.AllowedCSIDriver{{Name: "foo"}, {Name: "bar"}, {Name: "baz"}} + return psp + }(), + }, } for name, test := range successCases { @@ -1218,6 +1326,8 @@ func defaultV1Pod() *v1.Pod { // a pod with that type of volume and deny it, accept it explicitly, or accept it with // the FSTypeAll wildcard. func TestValidateAllowedVolumes(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + val := reflect.ValueOf(api.VolumeSource{}) for i := 0; i < val.NumField(); i++ { diff --git a/plugin/pkg/admission/security/podsecuritypolicy/BUILD b/plugin/pkg/admission/security/podsecuritypolicy/BUILD index cd04ef554a..6295537458 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/BUILD +++ b/plugin/pkg/admission/security/podsecuritypolicy/BUILD @@ -41,6 +41,7 @@ go_test( "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", "//pkg/controller:go_default_library", + "//pkg/features:go_default_library", "//pkg/security/apparmor:go_default_library", "//pkg/security/podsecuritypolicy:go_default_library", "//pkg/security/podsecuritypolicy/seccomp:go_default_library", @@ -56,7 +57,9 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", ], diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index 76d8ab5948..ceeb0981d1 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -35,11 +35,14 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/authorization/authorizerfactory" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" kapi "k8s.io/kubernetes/pkg/apis/core" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" "k8s.io/kubernetes/pkg/controller" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/security/apparmor" kpsp "k8s.io/kubernetes/pkg/security/podsecuritypolicy" "k8s.io/kubernetes/pkg/security/podsecuritypolicy/seccomp" @@ -625,6 +628,8 @@ func TestAdmitCaps(t *testing.T) { } func TestAdmitVolumes(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + val := reflect.ValueOf(kapi.VolumeSource{}) for i := 0; i < val.NumField(); i++ { diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types.go b/staging/src/k8s.io/api/extensions/v1beta1/types.go index d58418c7b9..7d802b9446 100644 --- a/staging/src/k8s.io/api/extensions/v1beta1/types.go +++ b/staging/src/k8s.io/api/extensions/v1beta1/types.go @@ -929,7 +929,8 @@ type PodSecurityPolicySpec struct { // +optional AllowedFlexVolumes []AllowedFlexVolume `json:"allowedFlexVolumes,omitempty" protobuf:"bytes,18,rep,name=allowedFlexVolumes"` // AllowedCSIDrivers is a whitelist of inline CSI drivers that must be explicitly set to be embedded within a pod spec. - // An empty value means no CSI drivers can run inline within a pod spec. + // An empty value indicates that any CSI driver can be used for inline ephemeral volumes. + // This is an alpha field, and is only honored if the API server enables the CSIInlineVolume feature gate. // +optional AllowedCSIDrivers []AllowedCSIDriver `json:"allowedCSIDrivers,omitempty" protobuf:"bytes,23,rep,name=allowedCSIDrivers"` // allowedUnsafeSysctls is a list of explicitly allowed unsafe sysctls, defaults to none. diff --git a/staging/src/k8s.io/api/policy/v1beta1/types.go b/staging/src/k8s.io/api/policy/v1beta1/types.go index 6426b294fa..a59df9840d 100644 --- a/staging/src/k8s.io/api/policy/v1beta1/types.go +++ b/staging/src/k8s.io/api/policy/v1beta1/types.go @@ -17,7 +17,7 @@ limitations under the License. package v1beta1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -217,7 +217,8 @@ type PodSecurityPolicySpec struct { // +optional AllowedFlexVolumes []AllowedFlexVolume `json:"allowedFlexVolumes,omitempty" protobuf:"bytes,18,rep,name=allowedFlexVolumes"` // AllowedCSIDrivers is a whitelist of inline CSI drivers that must be explicitly set to be embedded within a pod spec. - // An empty value means no CSI drivers can run inline within a pod spec. + // An empty value indicates that any CSI driver can be used for inline ephemeral volumes. + // This is an alpha field, and is only honored if the API server enables the CSIInlineVolume feature gate. // +optional AllowedCSIDrivers []AllowedCSIDriver `json:"allowedCSIDrivers,omitempty" protobuf:"bytes,23,rep,name=allowedCSIDrivers"` // allowedUnsafeSysctls is a list of explicitly allowed unsafe sysctls, defaults to none.