Merge pull request #58647 from oracle/for/upstream/master/hostpath-psp-readonly

api/openapi-spec/swagger.json

@@ -81376,6 +81376,10 @@
      "pathPrefix": {
       "description": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`",
       "type": "string"
+     },
+     "readOnly": {
+      "description": "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.",
+      "type": "boolean"
      }
     }
    },
@@ -82920,6 +82924,10 @@
      "pathPrefix": {
       "description": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`",
       "type": "string"
+     },
+     "readOnly": {
+      "description": "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.",
+      "type": "boolean"
      }
     }
    },

api/swagger-spec/extensions_v1beta1.json

@@ -10517,6 +10517,10 @@
      "pathPrefix": {
       "type": "string",
       "description": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`"
+     },
+     "readOnly": {
+      "type": "boolean",
+      "description": "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly."
      }
     }
    },

api/swagger-spec/policy_v1beta1.json

@@ -2650,6 +2650,10 @@
      "pathPrefix": {
       "type": "string",
       "description": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`"
+     },
+     "readOnly": {
+      "type": "boolean",
+      "description": "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly."
      }
     }
    },

docs/api-reference/extensions/v1beta1/definitions.html

@@ -450,6 +450,13 @@ Examples: <code>/foo</code> would allow <code>/foo</code>, <code>/foo/</code> an
 <td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
 <td class="tableblock halign-left valign-top"></td>
 </tr>
+<tr>
+<td class="tableblock halign-left valign-top"><p class="tableblock">readOnly</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">boolean</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
+</tr>
 </tbody>
 </table>
 

docs/api-reference/policy/v1beta1/definitions.html

@@ -420,6 +420,13 @@ Examples: <code>/foo</code> would allow <code>/foo</code>, <code>/foo/</code> an
 <td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
 <td class="tableblock halign-left valign-top"></td>
 </tr>
+<tr>
+<td class="tableblock halign-left valign-top"><p class="tableblock">readOnly</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">boolean</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
+</tr>
 </tbody>
 </table>
 

pkg/apis/extensions/v1beta1/zz_generated.conversion.go

@@ -168,6 +168,7 @@ func Convert_policy_AllowedFlexVolume_To_v1beta1_AllowedFlexVolume(in *policy.Al
 
 func autoConvert_v1beta1_AllowedHostPath_To_policy_AllowedHostPath(in *v1beta1.AllowedHostPath, out *policy.AllowedHostPath, s conversion.Scope) error {
 	out.PathPrefix = in.PathPrefix
+	out.ReadOnly = in.ReadOnly
 	return nil
 }
 
@@ -178,6 +179,7 @@ func Convert_v1beta1_AllowedHostPath_To_policy_AllowedHostPath(in *v1beta1.Allow
 
 func autoConvert_policy_AllowedHostPath_To_v1beta1_AllowedHostPath(in *policy.AllowedHostPath, out *v1beta1.AllowedHostPath, s conversion.Scope) error {
 	out.PathPrefix = in.PathPrefix
+	out.ReadOnly = in.ReadOnly
 	return nil
 }
 

pkg/apis/policy/types.go

@@ -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

pkg/apis/policy/v1beta1/zz_generated.conversion.go

@@ -98,6 +98,7 @@ func Convert_policy_AllowedFlexVolume_To_v1beta1_AllowedFlexVolume(in *policy.Al
 
 func autoConvert_v1beta1_AllowedHostPath_To_policy_AllowedHostPath(in *v1beta1.AllowedHostPath, out *policy.AllowedHostPath, s conversion.Scope) error {
 	out.PathPrefix = in.PathPrefix
+	out.ReadOnly = in.ReadOnly
 	return nil
 }
 
@@ -108,6 +109,7 @@ func Convert_v1beta1_AllowedHostPath_To_policy_AllowedHostPath(in *v1beta1.Allow
 
 func autoConvert_policy_AllowedHostPath_To_v1beta1_AllowedHostPath(in *policy.AllowedHostPath, out *v1beta1.AllowedHostPath, s conversion.Scope) error {
 	out.PathPrefix = in.PathPrefix
+	out.ReadOnly = in.ReadOnly
 	return nil
 }
 

pkg/security/podsecuritypolicy/provider.go

@@ -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"),
+								)
+							}
+						}
+					}
 				}
 			}
 

pkg/security/podsecuritypolicy/provider_test.go

@@ -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"
 
@@ -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,
@@ -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()

pkg/security/podsecuritypolicy/util/util.go

@@ -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

pkg/security/podsecuritypolicy/util/util_test.go

@@ -17,10 +17,11 @@ limitations under the License.
 package util
 
 import (
-	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/apis/policy"
 	"reflect"
 	"testing"
+
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/policy"
 )
 
 // TestVolumeSourceFSTypeDrift ensures that for every known type of volume source (by the fields on
@@ -105,41 +106,52 @@ func TestPSPAllowsFSType(t *testing.T) {
 
 func TestAllowsHostVolumePath(t *testing.T) {
 	tests := map[string]struct {
-		psp    *policy.PodSecurityPolicy
-		path   string
-		allows bool
+		psp            *policy.PodSecurityPolicy
+		path           string
+		allows         bool
+		mustBeReadOnly bool
 	}{
 		"nil psp": {
-			psp:    nil,
-			path:   "/test",
-			allows: false,
+			psp:            nil,
+			path:           "/test",
+			allows:         false,
+			mustBeReadOnly: false,
 		},
 		"empty allowed paths": {
-			psp:    &policy.PodSecurityPolicy{},
-			path:   "/test",
-			allows: true,
+			psp:            &policy.PodSecurityPolicy{},
+			path:           "/test",
+			allows:         true,
+			mustBeReadOnly: false,
 		},
 		"non-matching": {
 			psp: &policy.PodSecurityPolicy{
 				Spec: policy.PodSecurityPolicySpec{
 					AllowedHostPaths: []policy.AllowedHostPath{
-						{PathPrefix: "/foo"},
+						{
+							PathPrefix: "/foo",
+							ReadOnly:   true,
+						},
 					},
 				},
 			},
-			path:   "/foobar",
-			allows: false,
+			path:           "/foobar",
+			allows:         false,
+			mustBeReadOnly: false,
 		},
 		"match on direct match": {
 			psp: &policy.PodSecurityPolicy{
 				Spec: policy.PodSecurityPolicySpec{
 					AllowedHostPaths: []policy.AllowedHostPath{
-						{PathPrefix: "/foo"},
+						{
+							PathPrefix: "/foo",
+							ReadOnly:   true,
+						},
 					},
 				},
 			},
-			path:   "/foo",
-			allows: true,
+			path:           "/foo",
+			allows:         true,
+			mustBeReadOnly: true,
 		},
 		"match with trailing slash on host path": {
 			psp: &policy.PodSecurityPolicy{
@@ -149,8 +161,9 @@ func TestAllowsHostVolumePath(t *testing.T) {
 					},
 				},
 			},
-			path:   "/foo/",
-			allows: true,
+			path:           "/foo/",
+			allows:         true,
+			mustBeReadOnly: false,
 		},
 		"match with trailing slash on allowed path": {
 			psp: &policy.PodSecurityPolicy{
@@ -160,19 +173,24 @@ func TestAllowsHostVolumePath(t *testing.T) {
 					},
 				},
 			},
-			path:   "/foo",
-			allows: true,
+			path:           "/foo",
+			allows:         true,
+			mustBeReadOnly: false,
 		},
 		"match child directory": {
 			psp: &policy.PodSecurityPolicy{
 				Spec: policy.PodSecurityPolicySpec{
 					AllowedHostPaths: []policy.AllowedHostPath{
-						{PathPrefix: "/foo/"},
+						{
+							PathPrefix: "/foo/",
+							ReadOnly:   true,
+						},
 					},
 				},
 			},
-			path:   "/foo/bar",
-			allows: true,
+			path:           "/foo/bar",
+			allows:         true,
+			mustBeReadOnly: true,
 		},
 		"non-matching parent directory": {
 			psp: &policy.PodSecurityPolicy{
@@ -182,15 +200,19 @@ func TestAllowsHostVolumePath(t *testing.T) {
 					},
 				},
 			},
-			path:   "/foo",
-			allows: false,
+			path:           "/foo",
+			allows:         false,
+			mustBeReadOnly: false,
 		},
 	}
 
 	for k, v := range tests {
-		allows := AllowsHostVolumePath(v.psp, v.path)
+		allows, mustBeReadOnly := AllowsHostVolumePath(v.psp, v.path)
 		if v.allows != allows {
-			t.Errorf("%s expected %t but got %t", k, v.allows, allows)
+			t.Errorf("allows: %s expected %t but got %t", k, v.allows, allows)
+		}
+		if v.mustBeReadOnly != mustBeReadOnly {
+			t.Errorf("mustBeReadOnly: %s expected %t but got %t", k, v.mustBeReadOnly, mustBeReadOnly)
 		}
 	}
 }

staging/src/k8s.io/api/extensions/v1beta1/generated.proto

@@ -51,6 +51,10 @@ message AllowedHostPath {
   // `/foo` would allow `/foo`, `/foo/` and `/foo/bar`
   // `/foo` would not allow `/food` or `/etc/foo`
   optional string pathPrefix = 1;
+
+  // when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.
+  // +optional
+  optional bool readOnly = 2;
 }
 
 message CustomMetricCurrentStatus {

staging/src/k8s.io/api/extensions/v1beta1/types.go

@@ -960,6 +960,10 @@ type AllowedHostPath struct {
 	// `/foo` would allow `/foo`, `/foo/` and `/foo/bar`
 	// `/foo` would not allow `/food` or `/etc/foo`
 	PathPrefix string `json:"pathPrefix,omitempty" protobuf:"bytes,1,rep,name=pathPrefix"`
+
+	// when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.
+	// +optional
+	ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,2,opt,name=readOnly"`
 }
 
 // FSType gives strong typing to different file systems that are used by volumes.

staging/src/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.go

@@ -39,6 +39,7 @@ func (AllowedFlexVolume) SwaggerDoc() map[string]string {
 var map_AllowedHostPath = map[string]string{
 	"":           "AllowedHostPath defines the host volume conditions that will be enabled by a policy for pods to use. It requires the path prefix to be defined. Deprecated: use AllowedHostPath from policy API Group instead.",
 	"pathPrefix": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`",
+	"readOnly":   "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.",
 }
 
 func (AllowedHostPath) SwaggerDoc() map[string]string {

staging/src/k8s.io/api/policy/v1beta1/generated.proto

@@ -47,6 +47,10 @@ message AllowedHostPath {
   // `/foo` would allow `/foo`, `/foo/` and `/foo/bar`
   // `/foo` would not allow `/food` or `/etc/foo`
   optional string pathPrefix = 1;
+
+  // when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.
+  // +optional
+  optional bool readOnly = 2;
 }
 
 // Eviction evicts a pod from its node subject to certain policies and safety constraints.

staging/src/k8s.io/api/policy/v1beta1/types.go

@@ -214,6 +214,10 @@ type AllowedHostPath struct {
 	// `/foo` would allow `/foo`, `/foo/` and `/foo/bar`
 	// `/foo` would not allow `/food` or `/etc/foo`
 	PathPrefix string `json:"pathPrefix,omitempty" protobuf:"bytes,1,rep,name=pathPrefix"`
+
+	// when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.
+	// +optional
+	ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,2,opt,name=readOnly"`
 }
 
 // FSType gives strong typing to different file systems that are used by volumes.

staging/src/k8s.io/api/policy/v1beta1/types_swagger_doc_generated.go

@@ -39,6 +39,7 @@ func (AllowedFlexVolume) SwaggerDoc() map[string]string {
 var map_AllowedHostPath = map[string]string{
 	"":           "AllowedHostPath defines the host volume conditions that will be enabled by a policy for pods to use. It requires the path prefix to be defined.",
 	"pathPrefix": "pathPrefix is the path prefix that the host volume must match. It does not support `*`. Trailing slashes are trimmed when validating the path prefix with a host path.\n\nExamples: `/foo` would allow `/foo`, `/foo/` and `/foo/bar` `/foo` would not allow `/food` or `/etc/foo`",
+	"readOnly":   "when set to true, will allow host volumes matching the pathPrefix only if all volume mounts are readOnly.",
 }
 
 func (AllowedHostPath) SwaggerDoc() map[string]string {