diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index 6856a3adb8a..cda49838d6a 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -212,6 +212,25 @@ func (s *simpleProvider) ValidatePodSecurityContext(pod *api.Pod, fldPath *field allErrs = append(allErrs, s.strategies.SysctlsStrategy.Validate(pod)...) + // TODO(timstclair): ValidatePodSecurityContext should be renamed to ValidatePod since its scope + // is not limited to the PodSecurityContext. + if len(pod.Spec.Volumes) > 0 && !psputil.PSPAllowsAllVolumes(s.psp) { + allowedVolumes := psputil.FSTypeToStringSet(s.psp.Spec.Volumes) + for i, v := range pod.Spec.Volumes { + fsType, err := psputil.GetVolumeFSType(v) + if err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "volumes").Index(i), string(fsType), err.Error())) + continue + } + + if !allowedVolumes.Has(string(fsType)) { + 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)))) + } + } + } + return allErrs } @@ -235,23 +254,6 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container)...) - if len(pod.Spec.Volumes) > 0 && !psputil.PSPAllowsAllVolumes(s.psp) { - allowedVolumes := psputil.FSTypeToStringSet(s.psp.Spec.Volumes) - for i, v := range pod.Spec.Volumes { - fsType, err := psputil.GetVolumeFSType(v) - if err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("volumes").Index(i), string(fsType), err.Error())) - continue - } - - if !allowedVolumes.Has(string(fsType)) { - allErrs = append(allErrs, field.Invalid( - fldPath.Child("volumes").Index(i), string(fsType), - fmt.Sprintf("%s volumes are not allowed to be used", string(fsType)))) - } - } - } - if !s.psp.Spec.HostNetwork && pod.Spec.SecurityContext.HostNetwork { allErrs = append(allErrs, field.Invalid(fldPath.Child("hostNetwork"), pod.Spec.SecurityContext.HostNetwork, "Host network is not allowed to be used")) } diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index da146b87e36..3529cd46ddd 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -216,6 +216,16 @@ func TestValidatePodSecurityContextFailures(t *testing.T) { Level: "bar", } + failHostDirPod := defaultPod() + failHostDirPod.Spec.Volumes = []api.Volume{ + { + Name: "bad volume", + VolumeSource: api.VolumeSource{ + HostPath: &api.HostPathVolumeSource{}, + }, + }, + } + errorCases := map[string]struct { pod *api.Pod psp *extensions.PodSecurityPolicy @@ -266,6 +276,11 @@ func TestValidatePodSecurityContextFailures(t *testing.T) { psp: failSELinuxPSP, expectedError: "does not match required level. Found bar, wanted foo", }, + "failHostDirPSP": { + pod: failHostDirPod, + psp: defaultPSP(), + expectedError: "hostPath volumes are not allowed to be used", + }, } for k, v := range errorCases { provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) @@ -325,16 +340,6 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) { Add: []api.Capability{"foo"}, } - failHostDirPod := defaultPod() - failHostDirPod.Spec.Volumes = []api.Volume{ - { - Name: "bad volume", - VolumeSource: api.VolumeSource{ - HostPath: &api.HostPathVolumeSource{}, - }, - }, - } - failHostPortPod := defaultPod() failHostPortPod.Spec.Containers[0].Ports = []api.ContainerPort{{HostPort: 1}} @@ -380,11 +385,6 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) { psp: defaultPSP(), expectedError: "capability may not be added", }, - "failHostDirPSP": { - pod: failHostDirPod, - psp: defaultPSP(), - expectedError: "hostPath volumes are not allowed to be used", - }, "failHostPortPSP": { pod: failHostPortPod, psp: defaultPSP(), @@ -832,7 +832,7 @@ func TestValidateAllowedVolumes(t *testing.T) { } // expect a denial for this PSP and test the error message to ensure it's related to the volumesource - errs := provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath("")) + errs := provider.ValidatePodSecurityContext(pod, field.NewPath("")) if len(errs) != 1 { t.Errorf("expected exactly 1 error for %s but got %v", fieldVal.Name, errs) } else { @@ -843,14 +843,14 @@ func TestValidateAllowedVolumes(t *testing.T) { // now add the fstype directly to the psp and it should validate psp.Spec.Volumes = []extensions.FSType{fsType} - errs = provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath("")) + errs = provider.ValidatePodSecurityContext(pod, field.NewPath("")) if len(errs) != 0 { t.Errorf("directly allowing volume expected no errors for %s but got %v", fieldVal.Name, errs) } // now change the psp to allow any volumes and the pod should still validate psp.Spec.Volumes = []extensions.FSType{extensions.All} - errs = provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath("")) + errs = provider.ValidatePodSecurityContext(pod, field.NewPath("")) if len(errs) != 0 { t.Errorf("wildcard volume expected no errors for %s but got %v", fieldVal.Name, errs) }