Merge pull request #52799 from php-coder/psp_selinux_categories

Automatic merge from submit-queue (batch tested with PRs 58756, 58758, 58725, 52799, 58534). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

PSP: improve parsing and validation of SELinux levels

**What this PR does / why we need it**:
At this moment, when we're comparing SELinux levels of PSP and container, we compare them as strings. We don't take into account that categories in a level may be specified in a different order (for example, `s0:c0,c6` is the same as `s0:c6,c0`).

This PR improves handling of SELinux levels by doing logical comparison.

**Special notes for your reviewer**:
Here is the issue in OpenShift tracker from @pweil-: https://github.com/openshift/origin/issues/15627
Relate PR to fixing this in OpenShift: https://github.com/openshift/origin/pull/16432

**Release note**:
```release-note
NONE
```

PTAL @pweil- 
CC @simo5

pkg/security/podsecuritypolicy/selinux/BUILD

@@ -18,6 +18,7 @@ go_library(
     deps = [
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/extensions:go_default_library",
+        "//pkg/security/podsecuritypolicy/util:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],
 )

pkg/security/podsecuritypolicy/selinux/mustrunas.go

@@ -18,10 +18,13 @@ package selinux
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
 )
 
 type mustRunAs struct {
@@ -55,7 +58,7 @@ func (s *mustRunAs) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container,
 		allErrs = append(allErrs, field.Required(fldPath, ""))
 		return allErrs
 	}
-	if seLinux.Level != s.opts.SELinuxOptions.Level {
+	if !equalLevels(s.opts.SELinuxOptions.Level, seLinux.Level) {
 		detail := fmt.Sprintf("must be %s", s.opts.SELinuxOptions.Level)
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("level"), seLinux.Level, detail))
 	}
@@ -74,3 +77,44 @@ func (s *mustRunAs) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container,
 
 	return allErrs
 }
+
+// equalLevels compares SELinux levels for equality.
+func equalLevels(expected, actual string) bool {
+	if expected == actual {
+		return true
+	}
+	// "s0:c6,c0" => [ "s0", "c6,c0" ]
+	expectedParts := strings.SplitN(expected, ":", 2)
+	actualParts := strings.SplitN(actual, ":", 2)
+
+	// both SELinux levels must be in a format "sX:cY"
+	if len(expectedParts) != 2 || len(actualParts) != 2 {
+		return false
+	}
+
+	if !equalSensitivity(expectedParts[0], actualParts[0]) {
+		return false
+	}
+
+	if !equalCategories(expectedParts[1], actualParts[1]) {
+		return false
+	}
+
+	return true
+}
+
+// equalSensitivity compares sensitivities of the SELinux levels for equality.
+func equalSensitivity(expected, actual string) bool {
+	return expected == actual
+}
+
+// equalCategories compares categories of the SELinux levels for equality.
+func equalCategories(expected, actual string) bool {
+	expectedCategories := strings.Split(expected, ",")
+	actualCategories := strings.Split(actual, ",")
+
+	sort.Strings(expectedCategories)
+	sort.Strings(actualCategories)
+
+	return util.EqualStringSlices(expectedCategories, actualCategories)
+}

pkg/security/podsecuritypolicy/selinux/mustrunas_test.go

@@ -76,61 +76,81 @@ func TestMustRunAsValidate(t *testing.T) {
 		return &api.SELinuxOptions{
 			User:  "user",
 			Role:  "role",
-			Level: "level",
+			Level: "s0:c0,c6",
 			Type:  "type",
 		}
 	}
 
+	newValidOptsWithLevel := func(level string) *api.SELinuxOptions {
+		opts := newValidOpts()
+		opts.Level = level
+		return opts
+	}
+
 	role := newValidOpts()
 	role.Role = "invalid"
 
 	user := newValidOpts()
 	user.User = "invalid"
 
-	level := newValidOpts()
-	level.Level = "invalid"
-
 	seType := newValidOpts()
 	seType.Type = "invalid"
 
+	validOpts := newValidOpts()
+
 	tests := map[string]struct {
-		seLinux     *api.SELinuxOptions
+		podSeLinux  *api.SELinuxOptions
+		pspSeLinux  *api.SELinuxOptions
 		expectedMsg string
 	}{
 		"invalid role": {
-			seLinux:     role,
+			podSeLinux:  role,
+			pspSeLinux:  validOpts,
 			expectedMsg: "role: Invalid value",
 		},
 		"invalid user": {
-			seLinux:     user,
+			podSeLinux:  user,
+			pspSeLinux:  validOpts,
 			expectedMsg: "user: Invalid value",
 		},
-		"invalid level": {
+		"levels are not equal": {
-			seLinux:     level,
+			podSeLinux:  newValidOptsWithLevel("s0"),
+			pspSeLinux:  newValidOptsWithLevel("s0:c1,c2"),
 			expectedMsg: "level: Invalid value",
 		},
-		"invalid type": {
+		"levels differ by sensitivity": {
-			seLinux:     seType,
+			podSeLinux:  newValidOptsWithLevel("s0:c6"),
-			expectedMsg: "type: Invalid value",
+			pspSeLinux:  newValidOptsWithLevel("s1:c6"),
+			expectedMsg: "level: Invalid value",
+		},
+		"levels differ by categories": {
+			podSeLinux:  newValidOptsWithLevel("s0:c0,c8"),
+			pspSeLinux:  newValidOptsWithLevel("s0:c1,c7"),
+			expectedMsg: "level: Invalid value",
 		},
 		"valid": {
-			seLinux:     newValidOpts(),
+			podSeLinux:  validOpts,
+			pspSeLinux:  validOpts,
+			expectedMsg: "",
+		},
+		"valid with different order of categories": {
+			podSeLinux:  newValidOptsWithLevel("s0:c6,c0"),
+			pspSeLinux:  validOpts,
 			expectedMsg: "",
 		},
 	}
 
-	opts := &extensions.SELinuxStrategyOptions{
-		SELinuxOptions: newValidOpts(),
-	}
-
 	for name, tc := range tests {
+		opts := &extensions.SELinuxStrategyOptions{
+			SELinuxOptions: tc.pspSeLinux,
+		}
 		mustRunAs, err := NewMustRunAs(opts)
 		if err != nil {
 			t.Errorf("unexpected error initializing NewMustRunAs for testcase %s: %#v", name, err)
 			continue
 		}
 
-		errs := mustRunAs.Validate(nil, nil, nil, tc.seLinux)
+		errs := mustRunAs.Validate(nil, nil, nil, tc.podSeLinux)
 		//should've passed but didn't
 		if len(tc.expectedMsg) == 0 && len(errs) > 0 {
 			t.Errorf("%s expected no errors but received %v", name, errs)

pkg/security/podsecuritypolicy/util/util.go

@@ -222,3 +222,17 @@ func hasPathPrefix(s, pathPrefix string) bool {
 
 	return false
 }
+
+// EqualStringSlices compares string slices for equality. Slices are equal when
+// their sizes and elements on similar positions are equal.
+func EqualStringSlices(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := 0; i < len(a); i++ {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}

pkg/security/podsecuritypolicy/util/util_test.go

@@ -194,3 +194,38 @@ func TestAllowsHostVolumePath(t *testing.T) {
 		}
 	}
 }
+
+func TestEqualStringSlices(t *testing.T) {
+	tests := map[string]struct {
+		arg1           []string
+		arg2           []string
+		expectedResult bool
+	}{
+		"nil equals to nil": {
+			arg1:           nil,
+			arg2:           nil,
+			expectedResult: true,
+		},
+		"equal by size": {
+			arg1:           []string{"1", "1"},
+			arg2:           []string{"1", "1"},
+			expectedResult: true,
+		},
+		"not equal by size": {
+			arg1:           []string{"1"},
+			arg2:           []string{"1", "1"},
+			expectedResult: false,
+		},
+		"not equal by elements": {
+			arg1:           []string{"1", "1"},
+			arg2:           []string{"1", "2"},
+			expectedResult: false,
+		},
+	}
+
+	for k, v := range tests {
+		if result := EqualStringSlices(v.arg1, v.arg2); result != v.expectedResult {
+			t.Errorf("%s expected to return %t but got %t", k, v.expectedResult, result)
+		}
+	}
+}