Skip to content

Commit 3933ddb

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request kubernetes#33821 from sttts/sttts-sysctl-psp-fixes
Automatic merge from submit-queue Improve sysctl psp tests <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, read our contributor guidelines https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md and developer guide https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md 2. If you want *faster* PR reviews, read how: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md 3. Follow the instructions for writing a release note: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes --> **What this PR does / why we need it**: **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: <!-- Steps to write your release note: 1. Use the release-note-* labels to set the release note state (if you have access) 2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. --> ```release-note ```
2 parents 64d2b12 + e6acc08 commit 3933ddb

File tree

7 files changed

+111
-17
lines changed

7 files changed

+111
-17
lines changed

‎pkg/api/helpers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ func SysctlsFromPodAnnotation(annotation string) ([]Sysctl, error) {
577577
sysctls := make([]Sysctl, len(kvs))
578578
for i, kv := range kvs {
579579
cs := strings.Split(kv, "=")
580-
if len(cs) != 2 {
580+
if len(cs) != 2 || len(cs[0]) == 0 {
581581
return nil, fmt.Errorf("sysctl %q not of the format sysctl_name=value", kv)
582582
}
583583
sysctls[i].Name = cs[0]

‎pkg/api/helpers_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,14 @@ func TestSysctlsFromPodAnnotation(t *testing.T) {
508508
annotation: "foo.bar",
509509
expectErr: true,
510510
},
511+
{
512+
annotation: "=123",
513+
expectErr: true,
514+
},
515+
{
516+
annotation: "foo.bar=",
517+
expectValue: []Sysctl{{Name: "foo.bar", Value: ""}},
518+
},
511519
{
512520
annotation: "foo.bar=42",
513521
expectValue: []Sysctl{{Name: "foo.bar", Value: "42"}},

‎pkg/security/podsecuritypolicy/factory.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,7 @@ func (f *simpleStrategyFactory) CreateStrategies(psp *extensions.PodSecurityPoli
7979
errs = append(errs, err)
8080
}
8181
}
82-
sysctlsStrat, err := createSysctlsStrategy(unsafeSysctls)
83-
if err != nil {
84-
errs = append(errs, err)
85-
}
82+
sysctlsStrat := createSysctlsStrategy(unsafeSysctls)
8683

8784
if len(errs) > 0 {
8885
return nil, errors.NewAggregate(errs)
@@ -162,6 +159,6 @@ func createCapabilitiesStrategy(defaultAddCaps, requiredDropCaps, allowedCaps []
162159
}
163160

164161
// createSysctlsStrategy creates a new unsafe sysctls strategy.
165-
func createSysctlsStrategy(sysctlsPatterns []string) (sysctl.SysctlsStrategy, error) {
162+
func createSysctlsStrategy(sysctlsPatterns []string) sysctl.SysctlsStrategy {
166163
return sysctl.NewMustMatchPatterns(sysctlsPatterns)
167164
}

‎pkg/security/podsecuritypolicy/provider_test.go

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
226226
},
227227
}
228228

229+
failOtherSysctlsAllowedPSP := defaultPSP()
230+
failOtherSysctlsAllowedPSP.Annotations[extensions.SysctlsPodSecurityPolicyAnnotationKey] = "bar,abc"
231+
232+
failNoSysctlAllowedPSP := defaultPSP()
233+
failNoSysctlAllowedPSP.Annotations[extensions.SysctlsPodSecurityPolicyAnnotationKey] = ""
234+
235+
failSafeSysctlFooPod := defaultPod()
236+
failSafeSysctlFooPod.Annotations[api.SysctlsPodAnnotationKey] = "foo=1"
237+
238+
failUnsafeSysctlFooPod := defaultPod()
239+
failUnsafeSysctlFooPod.Annotations[api.UnsafeSysctlsPodAnnotationKey] = "foo=1"
240+
229241
errorCases := map[string]struct {
230242
pod *api.Pod
231243
psp *extensions.PodSecurityPolicy
@@ -281,6 +293,26 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
281293
psp: defaultPSP(),
282294
expectedError: "hostPath volumes are not allowed to be used",
283295
},
296+
"failSafeSysctlFooPod with failNoSysctlAllowedSCC": {
297+
pod: failSafeSysctlFooPod,
298+
psp: failNoSysctlAllowedPSP,
299+
expectedError: "sysctls are not allowed",
300+
},
301+
"failUnsafeSysctlFooPod with failNoSysctlAllowedSCC": {
302+
pod: failUnsafeSysctlFooPod,
303+
psp: failNoSysctlAllowedPSP,
304+
expectedError: "sysctls are not allowed",
305+
},
306+
"failSafeSysctlFooPod with failOtherSysctlsAllowedSCC": {
307+
pod: failSafeSysctlFooPod,
308+
psp: failOtherSysctlsAllowedPSP,
309+
expectedError: "sysctl \"foo\" is not allowed",
310+
},
311+
"failUnsafeSysctlFooPod with failOtherSysctlsAllowedSCC": {
312+
pod: failUnsafeSysctlFooPod,
313+
psp: failOtherSysctlsAllowedPSP,
314+
expectedError: "sysctl \"foo\" is not allowed",
315+
},
284316
}
285317
for k, v := range errorCases {
286318
provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
@@ -471,6 +503,15 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
471503
Level: "level",
472504
}
473505

506+
sysctlAllowFooPSP := defaultPSP()
507+
sysctlAllowFooPSP.Annotations[extensions.SysctlsPodSecurityPolicyAnnotationKey] = "foo"
508+
509+
safeSysctlFooPod := defaultPod()
510+
safeSysctlFooPod.Annotations[api.SysctlsPodAnnotationKey] = "foo=1"
511+
512+
unsafeSysctlFooPod := defaultPod()
513+
unsafeSysctlFooPod.Annotations[api.UnsafeSysctlsPodAnnotationKey] = "foo=1"
514+
474515
errorCases := map[string]struct {
475516
pod *api.Pod
476517
psp *extensions.PodSecurityPolicy
@@ -499,6 +540,22 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
499540
pod: seLinuxPod,
500541
psp: seLinuxPSP,
501542
},
543+
"pass sysctl specific profile with safe sysctl": {
544+
pod: safeSysctlFooPod,
545+
psp: sysctlAllowFooPSP,
546+
},
547+
"pass sysctl specific profile with unsafe sysctl": {
548+
pod: unsafeSysctlFooPod,
549+
psp: sysctlAllowFooPSP,
550+
},
551+
"pass empty profile with safe sysctl": {
552+
pod: safeSysctlFooPod,
553+
psp: defaultPSP(),
554+
},
555+
"pass empty profile with unsafe sysctl": {
556+
pod: unsafeSysctlFooPod,
557+
psp: defaultPSP(),
558+
},
502559
}
503560

504561
for k, v := range errorCases {
@@ -755,7 +812,8 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) {
755812
func defaultPSP() *extensions.PodSecurityPolicy {
756813
return &extensions.PodSecurityPolicy{
757814
ObjectMeta: api.ObjectMeta{
758-
Name: "psp-sa",
815+
Name: "psp-sa",
816+
Annotations: map[string]string{},
759817
},
760818
Spec: extensions.PodSecurityPolicySpec{
761819
RunAsUser: extensions.RunAsUserStrategyOptions{
@@ -777,6 +835,9 @@ func defaultPSP() *extensions.PodSecurityPolicy {
777835
func defaultPod() *api.Pod {
778836
var notPriv bool = false
779837
return &api.Pod{
838+
ObjectMeta: api.ObjectMeta{
839+
Annotations: map[string]string{},
840+
},
780841
Spec: api.PodSpec{
781842
SecurityContext: &api.PodSecurityContext{
782843
// fill in for test cases

‎pkg/security/podsecuritypolicy/sysctl/mustmatchpatterns.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ var (
3535
defaultSysctlsPatterns = []string{"*"}
3636
)
3737

38-
// NewMustMatchPatterns creates a new mustMatchPattern strategy that will provide validation.
38+
// NewMustMatchPatterns creates a new mustMatchPatterns strategy that will provide validation.
3939
// Passing nil means the default pattern, passing an empty list means to disallow all sysctls.
40-
func NewMustMatchPatterns(patterns []string) (SysctlsStrategy, error) {
40+
func NewMustMatchPatterns(patterns []string) SysctlsStrategy {
4141
if patterns == nil {
4242
patterns = defaultSysctlsPatterns
4343
}
4444
return &mustMatchPatterns{
4545
patterns: patterns,
46-
}, nil
46+
}
4747
}
4848

4949
// Validate ensures that the specified values fall within the range of the strategy.

‎pkg/security/podsecuritypolicy/sysctl/mustmatchpatterns_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,7 @@ func TestValidate(t *testing.T) {
5858
}
5959

6060
for k, v := range tests {
61-
strategy, err := NewMustMatchPatterns(v.patterns)
62-
if err != nil {
63-
t.Errorf("%s failed: %v", k, err)
64-
continue
65-
}
61+
strategy := NewMustMatchPatterns(v.patterns)
6662

6763
pod := &api.Pod{}
6864
errs := strategy.Validate(pod)

‎plugin/pkg/admission/security/podsecuritypolicy/admission_test.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,38 @@ func TestAdmitSysctls(t *testing.T) {
11061106
psps: []*extensions.PodSecurityPolicy{emptySysctls},
11071107
shouldPass: false,
11081108
},
1109+
"pod with unsafe sysctls a, b request disallowed under aSysctls SCC": {
1110+
pod: podWithSysctls([]string{}, []string{"a", "b"}),
1111+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1112+
shouldPass: false,
1113+
},
1114+
"pod with unsafe sysctls b request disallowed under aSysctls SCC": {
1115+
pod: podWithSysctls([]string{}, []string{"b"}),
1116+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1117+
shouldPass: false,
1118+
},
1119+
"pod with unsafe sysctls a request allowed under aSysctls SCC": {
1120+
pod: podWithSysctls([]string{}, []string{"a"}),
1121+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1122+
shouldPass: true,
1123+
expectedPSP: aSysctl.Name,
1124+
},
1125+
"pod with safe sysctls a, b request disallowed under aSysctls SCC": {
1126+
pod: podWithSysctls([]string{"a", "b"}, []string{}),
1127+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1128+
shouldPass: false,
1129+
},
1130+
"pod with safe sysctls b request disallowed under aSysctls SCC": {
1131+
pod: podWithSysctls([]string{"b"}, []string{}),
1132+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1133+
shouldPass: false,
1134+
},
1135+
"pod with safe sysctls a request allowed under aSysctls SCC": {
1136+
pod: podWithSysctls([]string{"a"}, []string{}),
1137+
psps: []*extensions.PodSecurityPolicy{aSysctl},
1138+
shouldPass: true,
1139+
expectedPSP: aSysctl.Name,
1140+
},
11091141
"pod with unsafe sysctls request disallowed under emptySysctls PSP": {
11101142
pod: podWithSysctls([]string{}, []string{"a", "b"}),
11111143
psps: []*extensions.PodSecurityPolicy{emptySysctls},
@@ -1117,12 +1149,12 @@ func TestAdmitSysctls(t *testing.T) {
11171149
shouldPass: true,
11181150
expectedPSP: mixedSysctls.Name,
11191151
},
1120-
"pod with not-matching unsafe sysctls request allowed under mixedSysctls PSP": {
1152+
"pod with not-matching unsafe sysctls request disallowed under mixedSysctls PSP": {
11211153
pod: podWithSysctls([]string{"a.b", "b.c", "c", "d.e.f"}, []string{"e"}),
11221154
psps: []*extensions.PodSecurityPolicy{mixedSysctls},
11231155
shouldPass: false,
11241156
},
1125-
"pod with not-matching safe sysctls request allowed under mixedSysctls PSP": {
1157+
"pod with not-matching safe sysctls request disallowed under mixedSysctls PSP": {
11261158
pod: podWithSysctls([]string{"a.b", "b.c", "c", "d.e.f", "e"}, []string{}),
11271159
psps: []*extensions.PodSecurityPolicy{mixedSysctls},
11281160
shouldPass: false,

0 commit comments

Comments
 (0)