Skip to content

Commit

Permalink
fix(k8sPSPHostFilesystem): null-check on volumes (#595)
Browse files Browse the repository at this point in the history
A K8sNativeValidation implementation of this template was added in #547.

When testing it, I found that a Pod lacking the `volumes` field would
yield a null-pointer style error on the CEL expression:

```
unexpected number of violations: got 1 violations but want none: got messages [expression '(has(request.operation) && request.operation == "UPDATE") || size(variables.badHostPaths) == 0' resulted in error: composited variable "badHostPaths" fails to evaluate: composited variable "volumes" fails to evaluate: no such key: volumes]
```

This PR adds a `has(` check to prevent that null pointer, and adds a
suite test case that fails without the code change.

Signed-off-by: juliankatz <[email protected]>
  • Loading branch information
julianKatz authored Sep 4, 2024
1 parent 598df74 commit 799d77b
Show file tree
Hide file tree
Showing 18 changed files with 460 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
version: 1.1.1
name: k8spsphostfilesystem
displayName: Host Filesystem
createdAt: "2024-09-04T20:18:45Z"
description: Controls usage of the host filesystem. Corresponds to the `allowedHostPaths` field in a PodSecurityPolicy. For more information, see https://kubernetes.io/docs/concepts/policy/pod-security-policy/#volumes-and-file-systems
digest: 8181746b3471a38dd2536d511f05b8b49a6170fa99731637515e62ee58909e12
license: Apache-2.0
homeURL: https://open-policy-agent.github.io/gatekeeper-library/website/host-filesystem
keywords:
- gatekeeper
- open-policy-agent
- policies
readme: |-
# Host Filesystem
Controls usage of the host filesystem. Corresponds to the `allowedHostPaths` field in a PodSecurityPolicy. For more information, see https://kubernetes.io/docs/concepts/policy/pod-security-policy/#volumes-and-file-systems
install: |-
### Usage
```shell
kubectl apply -f https://raw.githubusercontent.com/open-policy-agent/gatekeeper-library/master/artifacthub/library/pod-security-policy/host-filesystem/1.1.1/template.yaml
```
provider:
name: Gatekeeper Library
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
resources:
- template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sPSPHostFilesystem
metadata:
name: no-host-paths
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-no-volumes
spec:
containers:
- name: nginx
image: nginx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sPSPHostFilesystem
metadata:
name: psp-host-filesystem
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
parameters:
allowedHostPaths:
- readOnly: true
pathPrefix: "/foo"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-host-filesystem
spec:
ephemeralContainers:
- name: nginx
image: nginx
volumeMounts:
- mountPath: /cache
name: cache-volume
readOnly: true
volumes:
- name: cache-volume
hostPath:
path: /tmp # directory location on host
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-host-filesystem
spec:
containers:
- name: nginx
image: nginx
volumeMounts:
- mountPath: /cache
name: cache-volume
readOnly: true
volumes:
- name: cache-volume
hostPath:
path: /foo/bar
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-host-filesystem
spec:
containers:
- name: nginx
image: nginx
volumeMounts:
- mountPath: /cache
name: cache-volume
readOnly: true
volumes:
- name: cache-volume
hostPath:
path: /tmp # directory location on host
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
kind: AdmissionReview
apiVersion: admission.k8s.io/v1beta1
request:
operation: "UPDATE"
object:
apiVersion: v1
kind: Pod
metadata:
name: nginx-host-filesystem
labels:
app: nginx-host-filesystem-disallowed
spec:
containers:
- name: nginx
image: nginx
volumeMounts:
- mountPath: /cache
name: cache-volume
readOnly: true
volumes:
- name: cache-volume
hostPath:
path: /tmp # directory location on host
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
metadata:
name: host-filesystem
tests:
- name: host-filesystem
template: template.yaml
constraint: samples/psp-host-filesystem/constraint.yaml
cases:
- name: example-disallowed
object: samples/psp-host-filesystem/example_disallowed.yaml
assertions:
- violations: yes
- name: example-allowed
object: samples/psp-host-filesystem/example_allowed.yaml
assertions:
- violations: no
- name: disallowed-ephemeral
object: samples/psp-host-filesystem/disallowed_ephemeral.yaml
assertions:
- violations: yes
- name: update
object: samples/psp-host-filesystem/update.yaml
assertions:
- violations: no
- name: no-host-paths
template: template.yaml
constraint: samples/no-host-paths/constraint.yaml
cases:
- name: previously-allowed-path-disallowed
object: samples/psp-host-filesystem/example_allowed.yaml
assertions:
- violations: yes
- name: no-volumes-is-allowed
object: samples/no-host-paths/example_allowed_no_volumes.yaml
assertions:
- violations: no
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
apiVersion: templates.gatekeeper.sh/v1
kind: ConstraintTemplate
metadata:
name: k8spsphostfilesystem
annotations:
metadata.gatekeeper.sh/title: "Host Filesystem"
metadata.gatekeeper.sh/version: 1.1.1
description: >-
Controls usage of the host filesystem. Corresponds to the
`allowedHostPaths` field in a PodSecurityPolicy. For more information,
see
https://kubernetes.io/docs/concepts/policy/pod-security-policy/#volumes-and-file-systems
spec:
crd:
spec:
names:
kind: K8sPSPHostFilesystem
validation:
# Schema for the `parameters` field
openAPIV3Schema:
type: object
description: >-
Controls usage of the host filesystem. Corresponds to the
`allowedHostPaths` field in a PodSecurityPolicy. For more information,
see
https://kubernetes.io/docs/concepts/policy/pod-security-policy/#volumes-and-file-systems
properties:
allowedHostPaths:
type: array
description: "An array of hostpath objects, representing paths and read/write configuration."
items:
type: object
properties:
pathPrefix:
type: string
description: "The path prefix that the host volume must match."
readOnly:
type: boolean
description: "when set to true, any container volumeMounts matching the pathPrefix must include `readOnly: true`."
targets:
- target: admission.k8s.gatekeeper.sh
code:
- engine: K8sNativeValidation
source:
variables:
- name: containers
expression: 'has(variables.anyObject.spec.containers) ? variables.anyObject.spec.containers : []'
- name: initContainers
expression: 'has(variables.anyObject.spec.initContainers) ? variables.anyObject.spec.initContainers : []'
- name: ephemeralContainers
expression: 'has(variables.anyObject.spec.ephemeralContainers) ? variables.anyObject.spec.ephemeralContainers : []'
- name: allContainers
expression: 'variables.containers + variables.initContainers + variables.ephemeralContainers'
- name: allowedPaths
expression: |
!has(variables.params.allowedHostPaths) ? [] : variables.params.allowedHostPaths
- name: volumes
expression: |
!has(variables.anyObject.spec.volumes) ? [] : variables.anyObject.spec.volumes.filter(volume, has(volume.hostPath))
- name: badHostPaths
expression: |
variables.volumes.filter(volume,
(size(variables.allowedPaths) == 0) ||
!(variables.allowedPaths.exists(allowedPath,
volume.hostPath.path.startsWith(allowedPath.pathPrefix) && (
(!has(allowedPath.readOnly) || !(allowedPath.readOnly)) ||
(has(allowedPath.readOnly) && allowedPath.readOnly && !variables.allContainers.exists(c,
c.volumeMounts.exists(m, m.name == volume.name && (!has(m.readOnly) || !m.readOnly)))))))
).map(volume, "{ hostPath: { path : " + volume.hostPath.path + " }, name: " + volume.name + "}").map(volume, "HostPath volume " + volume + " is not allowed, pod: " + object.metadata.name + ". Allowed path: " + variables.allowedPaths.map(path, path.pathPrefix + ", readOnly: " + (path.readOnly ? "true" : "false") + "}").join(", "))
validations:
- expression: '(has(request.operation) && request.operation == "UPDATE") || size(variables.badHostPaths) == 0'
messageExpression: 'variables.badHostPaths.join("\n")'
- engine: Rego
source:
rego: |
package k8spsphostfilesystem
import data.lib.exclude_update.is_update
violation[{"msg": msg, "details": {}}] {
# spec.volumes field is immutable.
not is_update(input.review)
volume := input_hostpath_volumes[_]
allowedPaths := get_allowed_paths(input)
input_hostpath_violation(allowedPaths, volume)
msg := sprintf("HostPath volume %v is not allowed, pod: %v. Allowed path: %v", [volume, input.review.object.metadata.name, allowedPaths])
}
input_hostpath_violation(allowedPaths, _) {
# An empty list means all host paths are blocked
allowedPaths == []
}
input_hostpath_violation(allowedPaths, volume) {
not input_hostpath_allowed(allowedPaths, volume)
}
get_allowed_paths(arg) = out {
not arg.parameters
out = []
}
get_allowed_paths(arg) = out {
not arg.parameters.allowedHostPaths
out = []
}
get_allowed_paths(arg) = out {
out = arg.parameters.allowedHostPaths
}
input_hostpath_allowed(allowedPaths, volume) {
allowedHostPath := allowedPaths[_]
path_matches(allowedHostPath.pathPrefix, volume.hostPath.path)
not allowedHostPath.readOnly == true
}
input_hostpath_allowed(allowedPaths, volume) {
allowedHostPath := allowedPaths[_]
path_matches(allowedHostPath.pathPrefix, volume.hostPath.path)
allowedHostPath.readOnly
not writeable_input_volume_mounts(volume.name)
}
writeable_input_volume_mounts(volume_name) {
container := input_containers[_]
mount := container.volumeMounts[_]
mount.name == volume_name
not mount.readOnly
}
# This allows "/foo", "/foo/", "/foo/bar" etc., but
# disallows "/fool", "/etc/foo" etc.
path_matches(prefix, path) {
a := path_array(prefix)
b := path_array(path)
prefix_matches(a, b)
}
path_array(p) = out {
p != "/"
out := split(trim(p, "/"), "/")
}
# This handles the special case for "/", since
# split(trim("/", "/"), "/") == [""]
path_array("/") = []
prefix_matches(a, b) {
count(a) <= count(b)
not any_not_equal_upto(a, b, count(a))
}
any_not_equal_upto(a, b, n) {
a[i] != b[i]
i < n
}
input_hostpath_volumes[v] {
v := input.review.object.spec.volumes[_]
has_field(v, "hostPath")
}
# has_field returns whether an object has a field
has_field(object, field) = true {
object[field]
}
input_containers[c] {
c := input.review.object.spec.containers[_]
}
input_containers[c] {
c := input.review.object.spec.initContainers[_]
}
input_containers[c] {
c := input.review.object.spec.ephemeralContainers[_]
}
libs:
- |
package lib.exclude_update
is_update(review) {
review.operation == "UPDATE"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sPSPHostFilesystem
metadata:
name: no-host-paths
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-no-volumes
spec:
containers:
- name: nginx
image: nginx
12 changes: 12 additions & 0 deletions library/pod-security-policy/host-filesystem/suite.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,15 @@ tests:
object: samples/psp-host-filesystem/update.yaml
assertions:
- violations: no
- name: no-host-paths
template: template.yaml
constraint: samples/no-host-paths/constraint.yaml
cases:
- name: previously-allowed-path-disallowed
object: samples/psp-host-filesystem/example_allowed.yaml
assertions:
- violations: yes
- name: no-volumes-is-allowed
object: samples/no-host-paths/example_allowed_no_volumes.yaml
assertions:
- violations: no
Loading

0 comments on commit 799d77b

Please sign in to comment.