diff --git a/api/swagger-spec/api-v1.json b/api/swagger-spec/api-v1.json index 60717ba4c79f..55a84bc6b71a 100644 --- a/api/swagger-spec/api-v1.json +++ b/api/swagger-spec/api-v1.json @@ -14741,6 +14741,7 @@ "v1.SecurityContextConstraints": { "id": "v1.SecurityContextConstraints", "required": [ + "priority", "allowPrivilegedContainer", "allowedCapabilities", "allowHostDirVolumePlugin", @@ -14761,6 +14762,11 @@ "metadata": { "$ref": "v1.ObjectMeta" }, + "priority": { + "type": "integer", + "format": "int32", + "description": "determines which SCC is used when multiple SCCs allow a particular pod; higher priority SCCs are preferred" + }, "allowPrivilegedContainer": { "type": "boolean", "description": "allow containers to run as privileged" diff --git a/pkg/cmd/server/bootstrappolicy/securitycontextconstraints.go b/pkg/cmd/server/bootstrappolicy/securitycontextconstraints.go index fbe3e0568818..c17dcb7db54d 100644 --- a/pkg/cmd/server/bootstrappolicy/securitycontextconstraints.go +++ b/pkg/cmd/server/bootstrappolicy/securitycontextconstraints.go @@ -246,6 +246,8 @@ func GetBootstrapSecurityContextConstraints(sccNameToAdditionalGroups map[string SupplementalGroups: kapi.SupplementalGroupsStrategyOptions{ Type: kapi.SupplementalGroupsStrategyRunAsAny, }, + // prefer the anyuid SCC over ones that force a uid + Priority: 1, }, } diff --git a/pkg/security/admission/admission.go b/pkg/security/admission/admission.go index 6c3d075ed87a..5123ae779dd4 100644 --- a/pkg/security/admission/admission.go +++ b/pkg/security/admission/admission.go @@ -127,7 +127,7 @@ func (c *constraint) Admit(a kadmission.Attributes) error { // remove duplicate constraints and sort matchedConstraints = deduplicateSecurityContextConstraints(matchedConstraints) - sort.Sort(ByRestrictions(matchedConstraints)) + sort.Sort(ByPriority(matchedConstraints)) providers, errs := c.createProvidersFromConstraints(a.GetNamespace(), matchedConstraints) logProviders(pod, providers, errs) diff --git a/pkg/security/admission/admission_test.go b/pkg/security/admission/admission_test.go index bdc555ef77b3..94d85b1cd9f8 100644 --- a/pkg/security/admission/admission_test.go +++ b/pkg/security/admission/admission_test.go @@ -68,9 +68,11 @@ func TestAdmit(t *testing.T) { Type: kapi.SupplementalGroupsStrategyMustRunAs, }, Groups: []string{"system:serviceaccounts"}, + // give this scc priority since it is what we want to validate with + SortPriority: 1, } // create scc that has specific requirements that shouldn't match but is permissioned to - // service accounts to test exact matches + // service accounts to test that we're matching first against the higher priority SCC. var exactUID int64 = 999 saExactSCC := &kapi.SecurityContextConstraints{ ObjectMeta: kapi.ObjectMeta{ diff --git a/pkg/security/admission/bypriority.go b/pkg/security/admission/bypriority.go new file mode 100644 index 000000000000..880f2377578a --- /dev/null +++ b/pkg/security/admission/bypriority.go @@ -0,0 +1,44 @@ +package admission + +import ( + kapi "k8s.io/kubernetes/pkg/api" +) + +// ByRestrictions is a helper to sort SCCs based on priority. If priorities are equal +// a string compare of the name is used. +type ByPriority []*kapi.SecurityContextConstraints + +func (s ByPriority) Len() int { + return len(s) +} +func (s ByPriority) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s ByPriority) Less(i, j int) bool { + iSCC := s[i] + jSCC := s[j] + + // a higher priority is considered "less" so that it moves to the front of the line + if iSCC.Priority > jSCC.Priority { + return true + } + + if iSCC.Priority < jSCC.Priority { + return false + } + + // they are equal, let's try point values + iRestrictionScore := pointValue(iSCC) + jRestrictionScore := pointValue(jSCC) + + // a lower restriction score is considered "less" so that it moves to the front of the line + // (the greater the score, the more lax the SCC is) + if iRestrictionScore < jRestrictionScore { + return true + } + + if iRestrictionScore > jRestrictionScore { + return false + } + + // they are still equal, sort by name + return iSCC.Name < jSCC.Name +} diff --git a/pkg/security/admission/bypriority_test.go b/pkg/security/admission/bypriority_test.go new file mode 100644 index 000000000000..ccc9935897a3 --- /dev/null +++ b/pkg/security/admission/bypriority_test.go @@ -0,0 +1,86 @@ +package admission + +import ( + "sort" + "testing" + + kapi "k8s.io/kubernetes/pkg/api" +) + +func TestByPriority(t *testing.T) { + sccs := []*kapi.SecurityContextConstraints{testSCC("one", 1), testSCC("two", 2), testSCC("three", 3), testSCC("negative", -1), testSCC("super", 100)} + expected := []string{"super", "three", "two", "one", "negative"} + + sort.Sort(ByPriority(sccs)) + + for i, scc := range sccs { + if scc.Name != expected[i] { + t.Errorf("sort by priority found %s at element %d but expected %s", scc.Name, i, expected[i]) + } + } +} + +func TestByPrioritiesScore(t *testing.T) { + privilegedSCC := testSCC("privileged", 1) + privilegedSCC.AllowPrivilegedContainer = true + + nonPriviledSCC := testSCC("nonprivileged", 1) + + hostDirSCC := testSCC("hostdir", 1) + hostDirSCC.AllowHostDirVolumePlugin = true + + sccs := []*kapi.SecurityContextConstraints{nonPriviledSCC, privilegedSCC, hostDirSCC} + // with equal priorities expect that the SCCs will be sorted with hold behavior based on their score, + // most restrictive first + expected := []string{"nonprivileged", "hostdir", "privileged"} + + sort.Sort(ByPriority(sccs)) + + for i, scc := range sccs { + if scc.Name != expected[i] { + t.Errorf("sort by score found %s at element %d but expected %s", scc.Name, i, expected[i]) + } + } +} + +func TestByPrioritiesName(t *testing.T) { + sccs := []*kapi.SecurityContextConstraints{testSCC("e", 1), testSCC("d", 1), testSCC("a", 1), testSCC("c", 1), testSCC("b", 1)} + // expect that with equal priorities AND an equal point value that SCCs are sorted by name + expected := []string{"a", "b", "c", "d", "e"} + + sort.Sort(ByPriority(sccs)) + + for i, scc := range sccs { + if scc.Name != expected[i] { + t.Errorf("sort by priority found %s at element %d but expected %s", scc.Name, i, expected[i]) + } + } +} + +func TestByPrioritiesMixedSCCs(t *testing.T) { + privilegedSCC := testSCC("privileged", 1) + privilegedSCC.AllowPrivilegedContainer = true + + nonPriviledSCC := testSCC("nonprivileged", 1) + + sccs := []*kapi.SecurityContextConstraints{testSCC("priorityB", 5), testSCC("priorityA", 5), testSCC("super", 100), privilegedSCC, nonPriviledSCC} + // highest priority first, equal priority and equal score sorted by name, equal priority and non-equal score sorted most restrictive to least. + expected := []string{"super", "priorityA", "priorityB", "nonprivileged", "privileged"} + + sort.Sort(ByPriority(sccs)) + + for i, scc := range sccs { + if scc.Name != expected[i] { + t.Errorf("sort by priority found %s at element %d but expected %s", scc.Name, i, expected[i]) + } + } +} + +func testSCC(name string, priority int) *kapi.SecurityContextConstraints { + return &kapi.SecurityContextConstraints{ + ObjectMeta: kapi.ObjectMeta{ + Name: name, + }, + Priority: priority, + } +} diff --git a/pkg/security/admission/byrestrictions.go b/pkg/security/admission/byrestrictions.go index 1e925db261f0..c914d3d91236 100644 --- a/pkg/security/admission/byrestrictions.go +++ b/pkg/security/admission/byrestrictions.go @@ -12,12 +12,12 @@ func (s ByRestrictions) Len() int { } func (s ByRestrictions) Swap(i, j int) { s[i], s[j] = s[j], s[i] } func (s ByRestrictions) Less(i, j int) bool { - return s.pointValue(s[i]) < s.pointValue(s[j]) + return pointValue(s[i]) < pointValue(s[j]) } // pointValue places a value on the SCC based on the settings of the SCC that can be used // to determine how restrictive it is. The lower the number, the more restrictive it is. -func (s ByRestrictions) pointValue(constraint *kapi.SecurityContextConstraints) int { +func pointValue(constraint *kapi.SecurityContextConstraints) int { points := 0 // make sure these are always valued higher than the combination of the highest strategies