From 072214597c16a47ace1988ce6631b638d33845b9 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Wed, 24 Jan 2018 12:02:34 +0100 Subject: [PATCH 1/5] PSP: when comparing categories in SELinux levels, ignore its order. --- .../podsecuritypolicy/selinux/mustrunas.go | 46 ++++++++++++++++++- .../selinux/mustrunas_test.go | 12 ++++- pkg/security/podsecuritypolicy/util/util.go | 14 ++++++ .../podsecuritypolicy/util/util_test.go | 35 ++++++++++++++ 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas.go b/pkg/security/podsecuritypolicy/selinux/mustrunas.go index b2605cd3304..67f23b99508 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas.go +++ b/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) +} diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go index 986fa8adb77..ae83ca66ee0 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go @@ -76,11 +76,17 @@ 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" @@ -117,6 +123,10 @@ func TestMustRunAsValidate(t *testing.T) { seLinux: newValidOpts(), expectedMsg: "", }, + "valid with different order of categories": { + seLinux: newValidOptsWithLevel("s0:c6,c0"), + expectedMsg: "", + }, } opts := &extensions.SELinuxStrategyOptions{ diff --git a/pkg/security/podsecuritypolicy/util/util.go b/pkg/security/podsecuritypolicy/util/util.go index d654f88c77f..d581f5012a0 100644 --- a/pkg/security/podsecuritypolicy/util/util.go +++ b/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 +} diff --git a/pkg/security/podsecuritypolicy/util/util_test.go b/pkg/security/podsecuritypolicy/util/util_test.go index f4910a4970c..bb492b74317 100644 --- a/pkg/security/podsecuritypolicy/util/util_test.go +++ b/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) + } + } +} From 2e55ffbbbf838e877d4102e2375e0c6726a68f03 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Wed, 24 Jan 2018 12:09:39 +0100 Subject: [PATCH 2/5] Update autogenerated files. --- pkg/security/podsecuritypolicy/selinux/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/security/podsecuritypolicy/selinux/BUILD b/pkg/security/podsecuritypolicy/selinux/BUILD index 628436d8163..361e6a8e0df 100644 --- a/pkg/security/podsecuritypolicy/selinux/BUILD +++ b/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", ], ) From 3f261537b10000c65e98d537cc9e634ed88b1609 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Wed, 24 Jan 2018 18:35:48 +0100 Subject: [PATCH 3/5] selinux/mustrunas_test.go(TestMustRunAsValidate): rename a member to make its meaning obvious. --- .../podsecuritypolicy/selinux/mustrunas_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go index ae83ca66ee0..caa0a0438cc 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go @@ -100,31 +100,31 @@ func TestMustRunAsValidate(t *testing.T) { seType.Type = "invalid" tests := map[string]struct { - seLinux *api.SELinuxOptions + podSeLinux *api.SELinuxOptions expectedMsg string }{ "invalid role": { - seLinux: role, + podSeLinux: role, expectedMsg: "role: Invalid value", }, "invalid user": { - seLinux: user, + podSeLinux: user, expectedMsg: "user: Invalid value", }, "invalid level": { - seLinux: level, + podSeLinux: level, expectedMsg: "level: Invalid value", }, "invalid type": { - seLinux: seType, + podSeLinux: seType, expectedMsg: "type: Invalid value", }, "valid": { - seLinux: newValidOpts(), + podSeLinux: newValidOpts(), expectedMsg: "", }, "valid with different order of categories": { - seLinux: newValidOptsWithLevel("s0:c6,c0"), + podSeLinux: newValidOptsWithLevel("s0:c6,c0"), expectedMsg: "", }, } @@ -140,7 +140,7 @@ func TestMustRunAsValidate(t *testing.T) { 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) From 26fb4ed5fcd9c1ac9e427ac6e0c34d0aa01125e0 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Wed, 24 Jan 2018 18:40:07 +0100 Subject: [PATCH 4/5] selinux/mustrunas_test.go(TestMustRunAsValidate): make PSP SeLinux options configurable. --- .../selinux/mustrunas_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go index caa0a0438cc..0eca54bdc49 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go @@ -99,41 +99,49 @@ func TestMustRunAsValidate(t *testing.T) { seType := newValidOpts() seType.Type = "invalid" + validOpts := newValidOpts() + tests := map[string]struct { podSeLinux *api.SELinuxOptions + pspSeLinux *api.SELinuxOptions expectedMsg string }{ "invalid role": { podSeLinux: role, + pspSeLinux: validOpts, expectedMsg: "role: Invalid value", }, "invalid user": { podSeLinux: user, + pspSeLinux: validOpts, expectedMsg: "user: Invalid value", }, "invalid level": { podSeLinux: level, + pspSeLinux: validOpts, expectedMsg: "level: Invalid value", }, "invalid type": { podSeLinux: seType, + pspSeLinux: validOpts, expectedMsg: "type: Invalid value", }, "valid": { - podSeLinux: 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) From 09333b3a5d6b0f59272708a9e95bbe4b1493030d Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Wed, 24 Jan 2018 18:52:11 +0100 Subject: [PATCH 5/5] selinux/mustrunas_test.go(TestMustRunAsValidate): add more test cases to improve code coverage. --- .../selinux/mustrunas_test.go | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go index 0eca54bdc49..4c57df73797 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go @@ -93,9 +93,6 @@ func TestMustRunAsValidate(t *testing.T) { user := newValidOpts() user.User = "invalid" - level := newValidOpts() - level.Level = "invalid" - seType := newValidOpts() seType.Type = "invalid" @@ -116,15 +113,20 @@ func TestMustRunAsValidate(t *testing.T) { pspSeLinux: validOpts, expectedMsg: "user: Invalid value", }, - "invalid level": { - podSeLinux: level, - pspSeLinux: validOpts, + "levels are not equal": { + podSeLinux: newValidOptsWithLevel("s0"), + pspSeLinux: newValidOptsWithLevel("s0:c1,c2"), expectedMsg: "level: Invalid value", }, - "invalid type": { - podSeLinux: seType, - pspSeLinux: validOpts, - expectedMsg: "type: Invalid value", + "levels differ by sensitivity": { + podSeLinux: newValidOptsWithLevel("s0:c6"), + 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": { podSeLinux: validOpts,