Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for enforcing read only host paths in PSPs. #58647

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions api/openapi-spec/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions api/swagger-spec/extensions_v1beta1.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions api/swagger-spec/policy_v1beta1.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions docs/api-reference/extensions/v1beta1/definitions.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions docs/api-reference/policy/v1beta1/definitions.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/extensions/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/apis/policy/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,9 @@ type AllowedHostPath struct {
// `/foo` would allow `/foo`, `/foo/` and `/foo/bar`
// `/foo` would not allow `/food` or `/etc/foo`
PathPrefix string

// when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.
ReadOnly bool
}

// HostPortRange defines a range of host ports that will be enabled by a policy
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/policy/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion pkg/security/podsecuritypolicy/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,33 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod, fldPath *field.Path) field.Er
}

if fsType == policy.HostPath {
if !psputil.AllowsHostVolumePath(s.psp, v.HostPath.Path) {
allows, mustBeReadOnly := psputil.AllowsHostVolumePath(s.psp, v.HostPath.Path)
if !allows {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "volumes").Index(i).Child("hostPath", "pathPrefix"), v.HostPath.Path,
fmt.Sprintf("is not allowed to be used")))
} else if mustBeReadOnly {
// Ensure all the VolumeMounts that use this volume are read-only
for i, c := range pod.Spec.InitContainers {
for j, cv := range c.VolumeMounts {
if cv.Name == v.Name && !cv.ReadOnly {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "initContainers").Index(i).Child("volumeMounts").Index(j).Child("readOnly"),
cv.ReadOnly, "must be read-only"),
)
}
}
}
for i, c := range pod.Spec.Containers {
for j, cv := range c.VolumeMounts {
if cv.Name == v.Name && !cv.ReadOnly {
allErrs = append(allErrs, field.Invalid(
field.NewPath("spec", "containers").Index(i).Child("volumeMounts").Index(j).Child("readOnly"),
cv.ReadOnly, "must be read-only"),
)
}
}
}
}
}

Expand Down
91 changes: 88 additions & 3 deletions pkg/security/podsecuritypolicy/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,32 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
{PathPrefix: "/foo/bar"},
}

failHostPathReadOnlyPod := defaultPod()
failHostPathReadOnlyPod.Spec.Containers[0].VolumeMounts = []api.VolumeMount{
{
Name: "bad volume",
ReadOnly: false,
},
}
failHostPathReadOnlyPod.Spec.Volumes = []api.Volume{
{
Name: "bad volume",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/foo",
},
},
},
}
failHostPathReadOnlyPSP := defaultPSP()
failHostPathReadOnlyPSP.Spec.Volumes = []policy.FSType{policy.HostPath}
failHostPathReadOnlyPSP.Spec.AllowedHostPaths = []policy.AllowedHostPath{
{
PathPrefix: "/foo",
ReadOnly: true,
},
}

failOtherSysctlsAllowedPSP := defaultPSP()
failOtherSysctlsAllowedPSP.Annotations[policy.SysctlsPodSecurityPolicyAnnotationKey] = "bar,abc"

Expand Down Expand Up @@ -328,6 +354,11 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
psp: failHostPathDirPSP,
expectedError: "is not allowed to be used",
},
"failHostPathReadOnlyPSP": {
pod: failHostPathReadOnlyPod,
psp: failHostPathReadOnlyPSP,
expectedError: "must be read-only",
},
"failSafeSysctlFooPod with failNoSysctlAllowedSCC": {
pod: failSafeSysctlFooPod,
psp: failNoSysctlAllowedPSP,
Expand Down Expand Up @@ -598,28 +629,82 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
Level: "level",
}

hostPathDirPodVolumeMounts := []api.VolumeMount{
{
Name: "writeable /foo/bar",
ReadOnly: false,
},
{
Name: "read only /foo/bar/baz",
ReadOnly: true,
},
{
Name: "parent read only volume",
ReadOnly: true,
},
{
Name: "read only child volume",
ReadOnly: true,
},
}

hostPathDirPod := defaultPod()
hostPathDirPod.Spec.InitContainers = []api.Container{
{
Name: defaultContainerName,
VolumeMounts: hostPathDirPodVolumeMounts,
},
}

hostPathDirPod.Spec.Containers[0].VolumeMounts = hostPathDirPodVolumeMounts
hostPathDirPod.Spec.Volumes = []api.Volume{
{
Name: "good volume",
Name: "writeable /foo/bar",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/foo/bar",
},
},
},
{
Name: "read only /foo/bar/baz",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/foo/bar/baz",
},
},
},
{
Name: "parent read only volume",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/foo/",
},
},
},
{
Name: "read only child volume",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/foo/readonly/child",
},
},
},
}

hostPathDirPSP := defaultPSP()
hostPathDirPSP.Spec.Volumes = []policy.FSType{policy.HostPath}
hostPathDirPSP.Spec.AllowedHostPaths = []policy.AllowedHostPath{
{PathPrefix: "/foo/bar"},
// overlapping test case where child is different than parent directory.
{PathPrefix: "/foo/bar/baz", ReadOnly: true},
{PathPrefix: "/foo", ReadOnly: true},
{PathPrefix: "/foo/bar", ReadOnly: false},
}

hostPathDirAsterisksPSP := defaultPSP()
hostPathDirAsterisksPSP.Spec.Volumes = []policy.FSType{policy.All}
hostPathDirAsterisksPSP.Spec.AllowedHostPaths = []policy.AllowedHostPath{
{PathPrefix: "/foo/bar"},
{PathPrefix: "/foo"},
}

sysctlAllowFooPSP := defaultPSP()
Expand Down
14 changes: 9 additions & 5 deletions pkg/security/podsecuritypolicy/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,23 +175,27 @@ func GroupFallsInRange(id int64, rng policy.IDRange) bool {

// AllowsHostVolumePath is a utility for checking if a PSP allows the host volume path.
// This only checks the path. You should still check to make sure the host volume fs type is allowed.
func AllowsHostVolumePath(psp *policy.PodSecurityPolicy, hostPath string) bool {
func AllowsHostVolumePath(psp *policy.PodSecurityPolicy, hostPath string) (pathIsAllowed, mustBeReadOnly bool) {
if psp == nil {
return false
return false, false
}

// If no allowed paths are specified then allow any path
if len(psp.Spec.AllowedHostPaths) == 0 {
return true
return true, false
}

for _, allowedPath := range psp.Spec.AllowedHostPaths {
if hasPathPrefix(hostPath, allowedPath.PathPrefix) {
return true
if !allowedPath.ReadOnly {
return true, allowedPath.ReadOnly
}
pathIsAllowed = true
mustBeReadOnly = true
}
}

return false
return pathIsAllowed, mustBeReadOnly
}

// hasPathPrefix returns true if the string matches pathPrefix exactly, or if is prefixed with pathPrefix at a path segment boundary
Expand Down