selinux: Add a permissive boolean

Similarly to how seccomp profiles allow setting the default action to LOG, let's allow the selinux profiles to be set to permissive mode. This way, all calls would have still be allowed, but would appear in audit.log. This is useful in case the user is just iterating on a policy and wants to deploy a first version of the policy in the wild without causing AVC denials.
kubernetes-sigs · Nov 10, 2022 · e87e953 · e87e953
1 parent a8a6908
commit e87e953
Show file tree

Hide file tree

Showing 13 changed files with 242 additions and 0 deletions.
diff --git a/api/selinuxprofile/v1alpha2/selinuxprofile_types.go b/api/selinuxprofile/v1alpha2/selinuxprofile_types.go
@@ -57,6 +57,11 @@ type SelinuxProfileSpec struct {
 	// +optional
 	// +kubebuilder:default={{kind:"System",name:"container"}}
 	Inherit []PolicyRef `json:"inherit,omitempty"`
+	// Permissive, when true will cause the SELinux profile to only
+	// log violations instead of enforcing them.
+	// +optional
+	// +kubebuilder:default=false
+	Permissive bool `json:"permissive,omitempty"`
 	// Defines the allow policy for the profile
 	Allow Allow `json:"allow,omitempty"`
 }

diff --git a/bundle/manifests/security-profiles-operator.x-k8s.io_selinuxprofiles.yaml b/bundle/manifests/security-profiles-operator.x-k8s.io_selinuxprofiles.yaml
@@ -77,6 +77,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/base-crds/crds/selinuxpolicy.yaml b/deploy/base-crds/crds/selinuxpolicy.yaml
@@ -178,6 +178,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/helm/crds/crds.yaml b/deploy/helm/crds/crds.yaml
@@ -1854,6 +1854,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/namespace-operator.yaml b/deploy/namespace-operator.yaml
@@ -1854,6 +1854,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/openshift-dev.yaml b/deploy/openshift-dev.yaml
@@ -1854,6 +1854,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/operator.yaml b/deploy/operator.yaml
@@ -1854,6 +1854,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/deploy/webhook-operator.yaml b/deploy/webhook-operator.yaml
@@ -1925,6 +1925,11 @@ spec:
                   - name
                   type: object
                 type: array
+              permissive:
+                default: false
+                description: Permissive, when true will cause the SELinux profile
+                  to only log violations instead of enforcing them.
+                type: boolean
             type: object
           status:
             description: SelinuxProfileStatus defines the observed state of SelinuxProfile.

diff --git a/installation-usage.md b/installation-usage.md
@@ -26,6 +26,7 @@
     - [Merging per-container profile instances](#merging-per-container-profile-instances)
 - [Create a SELinux Profile](#create-a-selinux-profile)
   - [Apply a SELinux profile to a pod](#apply-a-selinux-profile-to-a-pod)
+  - [Make a SELinux profile permissive](#make-a-selinux-profile-permissive)
   - [Record a SELinux profile](#record-a-selinux-profile)
 - [Restricting to a Single Namespace](#restricting-to-a-single-namespace)
   - [Restricting to a Single Namespace with upstream deployment manifests](#restricting-to-a-single-namespace-with-upstream-deployment-manifests)
@@ -850,6 +851,15 @@ spec:
 
 Note that the SELinux type must exist before creating the workload.
 
+### Make a SELinux profile permissive
+
+Similarly to how a `SeccompProfile` might have a default action `SCMP_ACT_LOG`
+which would merely log violations of the policy, but not actually block the
+container from executing, a `SelinuxProfile` can be marked as "permissive"
+by setting `.spec.permissive` to `true`. This mode might be useful e.g. when
+the policy is known or suspected to be incomplete and you'd prefer to just
+watch for subsequent AVC denials after deploying the policy.
+
 ### Record a SELinux profile
 
 Please refer to the seccomp recording documentation, recording a SELinux

diff --git a/internal/pkg/translator/obj2cil.go b/internal/pkg/translator/obj2cil.go
@@ -26,6 +26,10 @@ import (
 	selxv1alpha2 "sigs.k8s.io/security-profiles-operator/api/selinuxprofile/v1alpha2"
 )
 
+const (
+	typePermissive = "(typepermissive process)"
+)
+
 func Object2CIL(
 	systemInherits []string,
 	objInherits []selxv1alpha2.SelinuxProfileObject,
@@ -39,6 +43,10 @@ func Object2CIL(
 	for _, inherit := range objInherits {
 		cilbuilder.WriteString(getCILInheritline(inherit.GetPolicyUsage()))
 	}
+	if sp.Spec.Permissive {
+		cilbuilder.WriteString(typePermissive)
+		cilbuilder.WriteString("\n")
+	}
 	for ttype, tclassMap := range sp.Spec.Allow {
 		for tclass, perms := range tclassMap {
 			cilbuilder.WriteString(getCILAllowLine(sp, ttype, tclass, perms))

diff --git a/internal/pkg/translator/obj2cil_test.go b/internal/pkg/translator/obj2cil_test.go
@@ -190,6 +190,75 @@ func TestObject2CIL(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Test errorlogger translation with permissive mode",
+			profile: &selxv1alpha2.SelinuxProfile{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo-permissive",
+					Namespace: "bar",
+				},
+				Spec: selxv1alpha2.SelinuxProfileSpec{
+					Permissive: true,
+					Inherit: []selxv1alpha2.PolicyRef{
+						{
+							Name: "container",
+						},
+					},
+					Allow: selxv1alpha2.Allow{
+						"var_log_t": {
+							"dir": []string{
+								"open",
+								"read",
+								"getattr",
+								"lock",
+								"search",
+								"ioctl",
+								"add_name",
+								"remove_name",
+								"write",
+							},
+							"file": []string{
+								"getattr",
+								"read",
+								"write",
+								"append",
+								"ioctl",
+								"lock",
+								"map",
+								"open",
+								"create",
+							},
+							"sock_file": []string{
+								"getattr",
+								"read",
+								"write",
+								"append",
+								"open",
+							},
+						},
+					},
+				},
+			},
+			wantMatches: []string{
+				"\\(block foo-permissive_bar",
+				"\\(blockinherit container\\)",
+				"\\(typepermissive process\\)",
+				// We match on several lines since we don't care about the order
+				"\\(allow process var_log_t \\( dir \\(.*open.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( dir \\(.*read.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( dir \\(.*remove_name.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( dir \\(.*write.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( file \\(.*getattr.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( file \\(.*map.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( file \\(.*create.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( sock_file \\(.*getattr.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( sock_file \\(.*append.*\\)\\)\\)\n",
+				"\\(allow process var_log_t \\( sock_file \\(.*open.*\\)\\)\\)\n",
+			},
+			inheritsys: []string{
+				"container",
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt

diff --git a/test/e2e_test.go b/test/e2e_test.go
@@ -133,6 +133,11 @@ func (e *e2e) TestSecurityProfilesOperator() {
 		e.testCaseSelinuxProfileBindingNsNotEnabled()
 	})
 
+	e.Run("cluster-wide: Selinux: Verify the policy can be marked as permissive", func() {
+		e.testCaseSelinuxIncompletePolicy()
+		e.testCaseSelinuxIncompletePermissivePolicy()
+	})
+
 	e.Run("cluster-wide: Selinux: Verify SELinux profile recording logs", func() {
 		e.testCaseProfileRecordingStaticPodSELinuxLogs()
 		e.testCaseProfileRecordingMultiContainerSELinuxLogs()

diff --git a/test/tc_selinux_base_usage_test.go b/test/tc_selinux_base_usage_test.go
@@ -61,6 +61,42 @@ spec:
         - open
 `
 
+	// this is the equivalent of errorloggerPolicy but with several calls removed. The idea is to
+	// ensure that the workload will fail if the policy in incomplete. Allows setting the permissive
+	// flag as needed.
+	errorloggerIncompletePolFmt = `
+apiVersion: security-profiles-operator.x-k8s.io/v1alpha2
+kind: SelinuxProfile
+metadata:
+  name: errorlogger-incomplete-%s
+spec:
+  permissive: %s
+  allow:
+    var_log_t:
+      dir:
+        - getattr
+        - lock
+        - search
+        - ioctl
+        - add_name
+        - remove_name
+        - write
+      file:
+        - getattr
+        - read
+        - ioctl
+        - lock
+        - map
+        - open
+        - create
+      sock_file:
+        - getattr
+        - read
+        - write
+        - append
+        - open
+`
+
 	//nolint:lll // full yaml
 	podWithPolicyFmt = `
 apiVersion: v1
@@ -131,6 +167,80 @@ func (e *e2e) testCaseSelinuxBaseUsage(nodes []string) {
 	e.assertSelinuxPolicyIsRemoved(nodes, rawPolicyName, maxNodeIterations, sleepBetweenIterations)
 }
 
+func (e *e2e) testCaseSelinuxIncompletePolicy() {
+	e.selinuxOnlyTestCase()
+	enforcingProfileName := "errorlogger-incomplete-enforcing"
+
+	e.logf("The 'errorlogger' workload should error out with a wrong policy")
+
+	e.logf("creating incomplete policy")
+	removeFn := e.writeAndCreate(
+		fmt.Sprintf(errorloggerIncompletePolFmt, "enforcing", "false"),
+		"errorlogger-policy-incomplete-enforcing.yml")
+	defer removeFn()
+
+	// Let's wait for the policy to be processed
+	e.kubectl("wait", "--timeout", defaultSelinuxOpTimeout,
+		"--for", "condition=ready", "selinuxprofile", enforcingProfileName)
+
+	e.logf("creating workload - it should become ready, but fail")
+	podWithPolicy := fmt.Sprintf(podWithPolicyFmt, e.getSELinuxPolicyUsage(enforcingProfileName))
+	e.writeAndCreate(podWithPolicy, "pod-w-incomplete-policy.yml")
+
+	// note: this would have been much nicer with kubectl wait --jsonpath, but I found it racy incase the status
+	// doesn't exist yet. So we're using a loop instead.
+	var exitCode string
+	for i := 0; i < 10; i++ {
+		exitCode = e.kubectl("get", "pods", "errorlogger",
+			"-o", "jsonpath={.status.containerStatuses[0].state.terminated.exitCode}")
+		if exitCode == "1" {
+			break
+		}
+		time.Sleep(2 * time.Second)
+	}
+	if exitCode != "1" {
+		e.Fail("The pod should have failed, but it didn't")
+	}
+
+	log := e.kubectl("logs", "errorlogger", "-c", "errorlogger")
+	e.Contains(log, "Permission denied")
+
+	e.logf("removing workload")
+	e.kubectl("delete", "pod", "errorlogger")
+
+	e.logf("removing policy")
+	e.kubectl("delete", "selinuxprofile", enforcingProfileName)
+}
+
+func (e *e2e) testCaseSelinuxIncompletePermissivePolicy() {
+	e.selinuxOnlyTestCase()
+	permissiveProfileName := "errorlogger-incomplete-permissive"
+
+	e.logf("The 'errorlogger' workload should run fine with a wrong policy in permissive mode")
+
+	e.logf("creating incomplete policy")
+	removeFn := e.writeAndCreate(
+		fmt.Sprintf(errorloggerIncompletePolFmt, "permissive", "true"),
+		"errorlogger-policy-incomplete-permissive.yml")
+	defer removeFn()
+
+	// Let's wait for the policy to be processed
+	e.kubectl("wait", "--timeout", defaultSelinuxOpTimeout,
+		"--for", "condition=ready", "selinuxprofile", permissiveProfileName)
+
+	e.logf("creating workload - it should become ready, but fail")
+	podWithPolicy := fmt.Sprintf(podWithPolicyFmt, e.getSELinuxPolicyUsage(permissiveProfileName))
+	e.writeAndCreate(podWithPolicy, "pod-w-incomplete-permissive-policy.yml")
+
+	e.waitFor("condition=ready", "pod", "errorlogger")
+
+	e.logf("removing workload")
+	e.kubectl("delete", "pod", "errorlogger")
+
+	e.logf("removing policy")
+	e.kubectl("delete", "selinuxprofile", permissiveProfileName)
+}
+
 func (e *e2e) assertSelinuxPolicyIsInstalled(nodes []string, policy string, nodeIterations int, sleep time.Duration) {
 	for i := 0; i < nodeIterations; i++ {
 		var missingPolName string