Skip to content

Commit 280d026

Browse files
stlazstlaz
committed
Promote {allowedUnsafe,forbidden}Sysctls from annotations to fields
1 parent 0658af0 commit 280d026

File tree

6 files changed

+237
-24
lines changed

6 files changed

+237
-24
lines changed

‎pkg/oc/cli/describe/describer.go

+6
Original file line numberDiff line numberDiff line change
@@ -1879,6 +1879,8 @@ func describeSecurityContextConstraints(scc *securityapi.SecurityContextConstrai
18791879
fmt.Fprintf(out, " Allowed Seccomp Profiles:\t%s\n", stringOrNone(strings.Join(scc.SeccompProfiles, ",")))
18801880
fmt.Fprintf(out, " Allowed Volume Types:\t%s\n", fsTypeToString(scc.Volumes))
18811881
fmt.Fprintf(out, " Allowed Flexvolumes:\t%s\n", flexVolumesToString(scc.AllowedFlexVolumes))
1882+
fmt.Fprintf(out, " Allowed Unsafe Sysctls:\t%s\n", sysctlsToString(scc.AllowedUnsafeSysctls))
1883+
fmt.Fprintf(out, " Forbidden Sysctls:\t%s\n", sysctlsToString(scc.ForbiddenSysctls))
18821884
fmt.Fprintf(out, " Allow Host Network:\t%t\n", scc.AllowHostNetwork)
18831885
fmt.Fprintf(out, " Allow Host Ports:\t%t\n", scc.AllowHostPorts)
18841886
fmt.Fprintf(out, " Allow Host PID:\t%t\n", scc.AllowHostPID)
@@ -1954,6 +1956,10 @@ func flexVolumesToString(flexVolumes []securityapi.AllowedFlexVolume) string {
19541956
return stringOrDefaultValue(strings.Join(volumes, ","), "<all>")
19551957
}
19561958

1959+
func sysctlsToString(sysctls []string) string {
1960+
return stringOrNone(strings.Join(sysctls, ","))
1961+
}
1962+
19571963
func idRangeToString(ranges []securityapi.IDRange) string {
19581964
formattedString := ""
19591965
if ranges != nil {

‎pkg/security/apis/security/types.go

+20
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,26 @@ type SecurityContextConstraints struct {
8484
Users []string
8585
// The groups that have permission to use this security context constraints
8686
Groups []string
87+
88+
// AllowedUnsafeSysctls is a list of explicitly allowed unsafe sysctls, defaults to none.
89+
// Each entry is either a plain sysctl name or ends in "*" in which case it is considered
90+
// as a prefix of allowed sysctls. Single * means all unsafe sysctls are allowed.
91+
// Kubelet has to whitelist all allowed unsafe sysctls explicitly to avoid rejection.
92+
//
93+
// Examples:
94+
// e.g. "foo/*" allows "foo/bar", "foo/baz", etc.
95+
// e.g. "foo.*" allows "foo.bar", "foo.baz", etc.
96+
// +optional
97+
AllowedUnsafeSysctls []string
98+
// ForbiddenSysctls is a list of explicitly forbidden sysctls, defaults to none.
99+
// Each entry is either a plain sysctl name or ends in "*" in which case it is considered
100+
// as a prefix of forbidden sysctls. Single * means all sysctls are forbidden.
101+
//
102+
// Examples:
103+
// e.g. "foo/*" forbids "foo/bar", "foo/baz", etc.
104+
// e.g. "foo.*" forbids "foo.bar", "foo.baz", etc.
105+
// +optional
106+
ForbiddenSysctls []string
87107
}
88108

89109
// FS Type gives strong typing to different file systems that are used by volumes.

‎pkg/security/apis/security/validation/validation.go

+93
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package validation
22

33
import (
44
"fmt"
5+
"regexp"
56
"strings"
67

78
"k8s.io/apimachinery/pkg/api/validation"
@@ -110,6 +111,98 @@ func ValidateSecurityContextConstraints(scc *securityapi.SecurityContextConstrai
110111
}
111112
}
112113

114+
allowedUnsafeSysctlsPath := field.NewPath("allowedUnsafeSysctls")
115+
forbiddenSysctlsPath := field.NewPath("forbiddenSysctls")
116+
allErrs = append(allErrs, validateSCCSysctls(allowedUnsafeSysctlsPath, scc.AllowedUnsafeSysctls)...)
117+
allErrs = append(allErrs, validateSCCSysctls(forbiddenSysctlsPath, scc.ForbiddenSysctls)...)
118+
allErrs = append(allErrs, validatePodSecurityPolicySysctlListsDoNotOverlap(allowedUnsafeSysctlsPath, forbiddenSysctlsPath, scc.AllowedUnsafeSysctls, scc.ForbiddenSysctls)...)
119+
120+
return allErrs
121+
}
122+
123+
const sysctlPatternSegmentFmt string = "([a-z0-9][-_a-z0-9]*)?[a-z0-9*]"
124+
const SysctlPatternFmt string = "(" + kapivalidation.SysctlSegmentFmt + "\\.)*" + sysctlPatternSegmentFmt
125+
126+
var sysctlPatternRegexp = regexp.MustCompile("^" + SysctlPatternFmt + "$")
127+
128+
func IsValidSysctlPattern(name string) bool {
129+
if len(name) > kapivalidation.SysctlMaxLength {
130+
return false
131+
}
132+
return sysctlPatternRegexp.MatchString(name)
133+
}
134+
135+
// validatePodSecurityPolicySysctlListsDoNotOverlap validates the values in forbiddenSysctls and allowedSysctls fields do not overlap.
136+
func validatePodSecurityPolicySysctlListsDoNotOverlap(allowedSysctlsFldPath, forbiddenSysctlsFldPath *field.Path, allowedUnsafeSysctls, forbiddenSysctls []string) field.ErrorList {
137+
allErrs := field.ErrorList{}
138+
for i, allowedSysctl := range allowedUnsafeSysctls {
139+
isAllowedSysctlPattern := false
140+
allowedSysctlPrefix := ""
141+
if strings.HasSuffix(allowedSysctl, "*") {
142+
isAllowedSysctlPattern = true
143+
allowedSysctlPrefix = strings.TrimSuffix(allowedSysctl, "*")
144+
}
145+
for j, forbiddenSysctl := range forbiddenSysctls {
146+
isForbiddenSysctlPattern := false
147+
forbiddenSysctlPrefix := ""
148+
if strings.HasSuffix(forbiddenSysctl, "*") {
149+
isForbiddenSysctlPattern = true
150+
forbiddenSysctlPrefix = strings.TrimSuffix(forbiddenSysctl, "*")
151+
}
152+
switch {
153+
case isAllowedSysctlPattern && isForbiddenSysctlPattern:
154+
if strings.HasPrefix(allowedSysctlPrefix, forbiddenSysctlPrefix) {
155+
allErrs = append(allErrs, field.Invalid(allowedSysctlsFldPath.Index(i), allowedUnsafeSysctls[i], fmt.Sprintf("sysctl overlaps with %v", forbiddenSysctl)))
156+
} else if strings.HasPrefix(forbiddenSysctlPrefix, allowedSysctlPrefix) {
157+
allErrs = append(allErrs, field.Invalid(forbiddenSysctlsFldPath.Index(j), forbiddenSysctls[j], fmt.Sprintf("sysctl overlaps with %v", allowedSysctl)))
158+
}
159+
case isAllowedSysctlPattern:
160+
if strings.HasPrefix(forbiddenSysctl, allowedSysctlPrefix) {
161+
allErrs = append(allErrs, field.Invalid(forbiddenSysctlsFldPath.Index(j), forbiddenSysctls[j], fmt.Sprintf("sysctl overlaps with %v", allowedSysctl)))
162+
}
163+
case isForbiddenSysctlPattern:
164+
if strings.HasPrefix(allowedSysctl, forbiddenSysctlPrefix) {
165+
allErrs = append(allErrs, field.Invalid(allowedSysctlsFldPath.Index(i), allowedUnsafeSysctls[i], fmt.Sprintf("sysctl overlaps with %v", forbiddenSysctl)))
166+
}
167+
default:
168+
if allowedSysctl == forbiddenSysctl {
169+
allErrs = append(allErrs, field.Invalid(allowedSysctlsFldPath.Index(i), allowedUnsafeSysctls[i], fmt.Sprintf("sysctl overlaps with %v", forbiddenSysctl)))
170+
}
171+
}
172+
}
173+
}
174+
return allErrs
175+
}
176+
177+
// validatePodSecurityPolicySysctls validates the sysctls fields of PodSecurityPolicy.
178+
func validateSCCSysctls(fldPath *field.Path, sysctls []string) field.ErrorList {
179+
allErrs := field.ErrorList{}
180+
181+
if len(sysctls) == 0 {
182+
return allErrs
183+
}
184+
185+
coversAll := false
186+
for i, s := range sysctls {
187+
if len(s) == 0 {
188+
allErrs = append(allErrs, field.Invalid(fldPath.Index(i), sysctls[i], fmt.Sprintf("empty sysctl not allowed")))
189+
} else if !IsValidSysctlPattern(string(s)) {
190+
allErrs = append(
191+
allErrs,
192+
field.Invalid(fldPath.Index(i), sysctls[i], fmt.Sprintf("must have at most %d characters and match regex %s",
193+
kapivalidation.SysctlMaxLength,
194+
SysctlPatternFmt,
195+
)),
196+
)
197+
} else if s[0] == '*' {
198+
coversAll = true
199+
}
200+
}
201+
202+
if coversAll && len(sysctls) > 1 {
203+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("items"), fmt.Sprintf("if '*' is present, must not specify other sysctls")))
204+
}
205+
113206
return allErrs
114207
}
115208

‎pkg/security/apis/security/validation/validation_test.go

+47
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package validation
22

33
import (
4+
"fmt"
45
"testing"
56

67
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -102,6 +103,20 @@ func TestValidateSecurityContextConstraints(t *testing.T) {
102103
nonEmptyFlexVolumes := validSCC()
103104
nonEmptyFlexVolumes.AllowedFlexVolumes = []securityapi.AllowedFlexVolume{{Driver: "example/driver"}}
104105

106+
invalidAllowedUnsafeSysctlPattern := validSCC()
107+
invalidAllowedUnsafeSysctlPattern.AllowedUnsafeSysctls = []string{"a.*.b"}
108+
109+
invalidForbiddenSysctlPattern := validSCC()
110+
invalidForbiddenSysctlPattern.ForbiddenSysctls = []string{"a.*.b"}
111+
112+
invalidOverlappingSysctls := validSCC()
113+
invalidOverlappingSysctls.ForbiddenSysctls = []string{"kernel.*", "net.ipv4.ip_local_port_range"}
114+
invalidOverlappingSysctls.AllowedUnsafeSysctls = []string{"kernel.shmmax", "net.ipv4.ip_local_port_range"}
115+
116+
invalidDuplicatedSysctls := validSCC()
117+
invalidDuplicatedSysctls.ForbiddenSysctls = []string{"net.ipv4.ip_local_port_range"}
118+
invalidDuplicatedSysctls.AllowedUnsafeSysctls = []string{"net.ipv4.ip_local_port_range"}
119+
105120
errorCases := map[string]struct {
106121
scc *securityapi.SecurityContextConstraints
107122
errorType field.ErrorType
@@ -202,6 +217,26 @@ func TestValidateSecurityContextConstraints(t *testing.T) {
202217
errorType: field.ErrorTypeInvalid,
203218
errorDetail: "volumes does not include 'flexVolume' or '*', so no flex volumes are allowed",
204219
},
220+
"invalid allowed unsafe sysctl pattern": {
221+
scc: invalidAllowedUnsafeSysctlPattern,
222+
errorType: field.ErrorTypeInvalid,
223+
errorDetail: fmt.Sprintf("must have at most 253 characters and match regex %s", SysctlPatternFmt),
224+
},
225+
"invalid forbidden sysctl pattern": {
226+
scc: invalidForbiddenSysctlPattern,
227+
errorType: field.ErrorTypeInvalid,
228+
errorDetail: fmt.Sprintf("must have at most 253 characters and match regex %s", SysctlPatternFmt),
229+
},
230+
"invalid overlapping sysctl pattern": {
231+
scc: invalidOverlappingSysctls,
232+
errorType: field.ErrorTypeInvalid,
233+
errorDetail: fmt.Sprintf("sysctl overlaps with %s", invalidOverlappingSysctls.ForbiddenSysctls[0]),
234+
},
235+
"invalid duplicated sysctls": {
236+
scc: invalidDuplicatedSysctls,
237+
errorType: field.ErrorTypeInvalid,
238+
errorDetail: fmt.Sprintf("sysctl overlaps with %s", invalidDuplicatedSysctls.AllowedUnsafeSysctls[0]),
239+
},
205240
}
206241

207242
for k, v := range errorCases {
@@ -244,6 +279,12 @@ func TestValidateSecurityContextConstraints(t *testing.T) {
244279
{Driver: "example/driver2"},
245280
}
246281

282+
withForbiddenSysctl := validSCC()
283+
withForbiddenSysctl.ForbiddenSysctls = []string{"net.*"}
284+
285+
withAllowedUnsafeSysctl := validSCC()
286+
withAllowedUnsafeSysctl.AllowedUnsafeSysctls = []string{"net.ipv4.tcp_max_syn_backlog"}
287+
247288
successCases := map[string]struct {
248289
scc *securityapi.SecurityContextConstraints
249290
}{
@@ -268,6 +309,12 @@ func TestValidateSecurityContextConstraints(t *testing.T) {
268309
"allow white-listed flexVolume when all volumes are allowed": {
269310
scc: flexvolumeWhenAllVolumesAllowed,
270311
},
312+
"with network sysctls forbidden": {
313+
scc: withForbiddenSysctl,
314+
},
315+
"with unsafe net.ipv4.tcp_max_syn_backlog sysctl allowed": {
316+
scc: withAllowedUnsafeSysctl,
317+
},
271318
}
272319

273320
for k, v := range successCases {

‎pkg/security/securitycontextconstraints/provider.go

+2-23
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package securitycontextconstraints
22

33
import (
44
"fmt"
5-
"strings"
65

76
"github.com/openshift/origin/pkg/security/securitycontextconstraints/capabilities"
87
"github.com/openshift/origin/pkg/security/securitycontextconstraints/group"
@@ -77,15 +76,7 @@ func NewSimpleProvider(scc *securityapi.SecurityContextConstraints) (SecurityCon
7776
return nil, err
7877
}
7978

80-
var unsafeSysctls []string
81-
if ann, found := scc.Annotations[SysctlsPodSecurityPolicyAnnotationKey]; found {
82-
var err error
83-
unsafeSysctls, err = SysctlsFromPodSecurityPolicyAnnotation(ann)
84-
if err != nil {
85-
return nil, err
86-
}
87-
}
88-
sysctlsStrat, err := createSysctlsStrategy(sysctl.SafeSysctlWhitelist(), unsafeSysctls, []string{})
79+
sysctlsStrat, err := createSysctlsStrategy(sysctl.SafeSysctlWhitelist(), scc.AllowedUnsafeSysctls, scc.ForbiddenSysctls)
8980
if err != nil {
9081
return nil, err
9182
}
@@ -409,19 +400,7 @@ func createSeccompStrategy(allowedProfiles []string) (seccomp.SeccompStrategy, e
409400
return seccomp.NewWithSeccompProfile(allowedProfiles)
410401
}
411402

412-
// createSysctlsStrategy creates a new unsafe sysctls strategy.
403+
// createSysctlsStrategy creates a new sysctls strategy
413404
func createSysctlsStrategy(safeWhitelist, allowedUnsafeSysctls, forbiddenSysctls []string) (sysctl.SysctlsStrategy, error) {
414405
return sysctl.NewMustMatchPatterns(safeWhitelist, allowedUnsafeSysctls, forbiddenSysctls), nil
415406
}
416-
417-
// TODO promote like kube did
418-
const SysctlsPodSecurityPolicyAnnotationKey string = "security.alpha.kubernetes.io/sysctls"
419-
420-
// TODO promote like kube did
421-
func SysctlsFromPodSecurityPolicyAnnotation(annotation string) ([]string, error) {
422-
if len(annotation) == 0 {
423-
return []string{}, nil
424-
}
425-
426-
return strings.Split(annotation, ","), nil
427-
}

‎pkg/security/securitycontextconstraints/provider_test.go

+69-1
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,31 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
199199
},
200200
}
201201

202+
failSysctlDisallowedSCC := defaultSCC()
203+
failSysctlDisallowedSCC.ForbiddenSysctls = []string{"kernel.shm_rmid_forced"}
204+
205+
failNoSafeSysctlAllowedSCC := defaultSCC()
206+
failNoSafeSysctlAllowedSCC.ForbiddenSysctls = []string{"*"}
207+
208+
failAllUnsafeSysctlsSCC := defaultSCC()
209+
failAllUnsafeSysctlsSCC.AllowedUnsafeSysctls = []string{}
210+
211+
failSafeSysctlKernelPod := defaultPod()
212+
failSafeSysctlKernelPod.Spec.SecurityContext.Sysctls = []api.Sysctl{
213+
{
214+
Name: "kernel.shm_rmid_forced",
215+
Value: "1",
216+
},
217+
}
218+
219+
failUnsafeSysctlPod := defaultPod()
220+
failUnsafeSysctlPod.Spec.SecurityContext.Sysctls = []api.Sysctl{
221+
{
222+
Name: "kernel.sem",
223+
Value: "32000",
224+
},
225+
}
226+
202227
errorCases := map[string]struct {
203228
pod *api.Pod
204229
scc *securityapi.SecurityContextConstraints
@@ -274,6 +299,21 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
274299
scc: allowFlexVolumesSCC(false, true),
275300
expectedError: "Flexvolume driver is not allowed to be used",
276301
},
302+
"failSafeSysctlKernelPod with failNoSafeSysctlAllowedSCC": {
303+
pod: failSafeSysctlKernelPod,
304+
scc: failNoSafeSysctlAllowedSCC,
305+
expectedError: "sysctl \"kernel.shm_rmid_forced\" is not allowed",
306+
},
307+
"failSafeSysctlKernelPod with failSysctlDisallowedSCC": {
308+
pod: failSafeSysctlKernelPod,
309+
scc: failSysctlDisallowedSCC,
310+
expectedError: "sysctl \"kernel.shm_rmid_forced\" is not allowed",
311+
},
312+
"failUnsafeSysctlPod with failAllUnsafeSysctlsSCC": {
313+
pod: failUnsafeSysctlPod,
314+
scc: failAllUnsafeSysctlsSCC,
315+
expectedError: "unsafe sysctl \"kernel.sem\" is not allowed",
316+
},
277317
}
278318
for k, v := range errorCases {
279319
provider, err := NewSimpleProvider(v.scc)
@@ -494,6 +534,26 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
494534
},
495535
}
496536

537+
sysctlAllowAllSCC := defaultSCC()
538+
sysctlAllowAllSCC.ForbiddenSysctls = []string{}
539+
sysctlAllowAllSCC.AllowedUnsafeSysctls = []string{"*"}
540+
541+
safeSysctlKernelPod := defaultPod()
542+
safeSysctlKernelPod.Spec.SecurityContext.Sysctls = []api.Sysctl{
543+
{
544+
Name: "kernel.shm_rmid_forced",
545+
Value: "1",
546+
},
547+
}
548+
549+
unsafeSysctlKernelPod := defaultPod()
550+
unsafeSysctlKernelPod.Spec.SecurityContext.Sysctls = []api.Sysctl{
551+
{
552+
Name: "kernel.sem",
553+
Value: "32000",
554+
},
555+
}
556+
497557
successCases := map[string]struct {
498558
pod *api.Pod
499559
scc *securityapi.SecurityContextConstraints
@@ -554,6 +614,14 @@ func TestValidatePodSecurityContextSuccess(t *testing.T) {
554614
pod: flexVolumePod,
555615
scc: allowFlexVolumesSCC(true, false),
556616
},
617+
"pass sysctl specific profile with safe kernel sysctl": {
618+
pod: safeSysctlKernelPod,
619+
scc: sysctlAllowAllSCC,
620+
},
621+
"pass sysctl specific profile with unsafe kernel sysctl": {
622+
pod: unsafeSysctlKernelPod,
623+
scc: sysctlAllowAllSCC,
624+
},
557625
}
558626

559627
for k, v := range successCases {
@@ -836,7 +904,7 @@ func defaultPod() *api.Pod {
836904
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}},
837905
Spec: api.PodSpec{
838906
SecurityContext: &api.PodSecurityContext{
839-
// fill in for test cases
907+
// fill in for test cases
840908
},
841909
Containers: []api.Container{
842910
{

0 commit comments

Comments
 (0)