From 883832cb90f3792f2e5a02416635b962b331db08 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Mon, 10 Jul 2017 18:34:02 +0200 Subject: [PATCH] Use constants for representation of the SCCs restrictiveness. --- pkg/security/scc/byrestrictions.go | 81 +++++++++++++++++-------- pkg/security/scc/byrestrictions_test.go | 62 +++++++++---------- 2 files changed, 84 insertions(+), 59 deletions(-) diff --git a/pkg/security/scc/byrestrictions.go b/pkg/security/scc/byrestrictions.go index 871c7c25cbae..6d9663bc468f 100644 --- a/pkg/security/scc/byrestrictions.go +++ b/pkg/security/scc/byrestrictions.go @@ -1,6 +1,8 @@ package scc import ( + "github.com/golang/glog" + securityapi "github.com/openshift/origin/pkg/security/apis/security" ) @@ -15,38 +17,65 @@ func (s ByRestrictions) Less(i, j int) bool { return pointValue(s[i]) < pointValue(s[j]) } +// The following constants define the weight of the restrictions and used for +// calculating the points of the particular SCC. The lower the number, the more +// restrictive SCC is. Make sure that weak restrictions are always valued +// higher than the combination of the strong restrictions. + +type points int + +const ( + privilegedPoints points = 20 + + hostVolumePoints points = 10 + nonTrivialVolumePoints points = 5 + + runAsAnyUserPoints points = 4 + runAsNonRootPoints points = 3 + runAsRangePoints points = 2 + runAsUserPoints points = 1 + + noPoints points = 0 +) + // 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 pointValue(constraint *securityapi.SecurityContextConstraints) int { - points := 0 +func pointValue(constraint *securityapi.SecurityContextConstraints) points { + totalPoints := noPoints - // make sure these are always valued higher than the combination of the highest strategies if constraint.AllowPrivilegedContainer { - points += 20 + totalPoints += privilegedPoints } // add points based on volume requests - points += volumePointValue(constraint) - - // strategies in order of least restrictive to most restrictive - switch constraint.SELinuxContext.Type { - case securityapi.SELinuxStrategyRunAsAny: - points += 4 - case securityapi.SELinuxStrategyMustRunAs: - points += 1 + totalPoints += volumePointValue(constraint) + + // the map contains points for both RunAsUser and SELinuxContext + // strategies by taking advantage that they have identical strategy names + strategiesPoints := map[string]points{ + string(securityapi.RunAsUserStrategyRunAsAny): runAsAnyUserPoints, + string(securityapi.RunAsUserStrategyMustRunAsNonRoot): runAsNonRootPoints, + string(securityapi.RunAsUserStrategyMustRunAsRange): runAsRangePoints, + string(securityapi.RunAsUserStrategyMustRunAs): runAsUserPoints, } - switch constraint.RunAsUser.Type { - case securityapi.RunAsUserStrategyRunAsAny: - points += 4 - case securityapi.RunAsUserStrategyMustRunAsNonRoot: - points += 3 - case securityapi.RunAsUserStrategyMustRunAsRange: - points += 2 - case securityapi.RunAsUserStrategyMustRunAs: - points += 1 + strategyType := string(constraint.SELinuxContext.Type) + points, found := strategiesPoints[strategyType] + if found { + totalPoints += points + } else { + glog.Warningf("SELinuxContext type %q has no point value, this may cause issues in sorting SCCs by restriction", strategyType) } - return points + + strategyType = string(constraint.RunAsUser.Type) + points, found = strategiesPoints[strategyType] + if found { + totalPoints += points + } else { + glog.Warningf("RunAsUser type %q has no point value, this may cause issues in sorting SCCs by restriction", strategyType) + } + + return totalPoints } // volumePointValue returns a score based on the volumes allowed by the SCC. @@ -54,7 +83,7 @@ func pointValue(constraint *securityapi.SecurityContextConstraints) int { // than Secret, ConfigMap, EmptyDir, DownwardAPI, Projected, and None will result in // a score of 5. If the SCC only allows these trivial types, it will have a // score of 0. -func volumePointValue(scc *securityapi.SecurityContextConstraints) int { +func volumePointValue(scc *securityapi.SecurityContextConstraints) points { hasHostVolume := false hasNonTrivialVolume := false for _, v := range scc.Volumes { @@ -75,10 +104,10 @@ func volumePointValue(scc *securityapi.SecurityContextConstraints) int { } if hasHostVolume { - return 10 + return hostVolumePoints } if hasNonTrivialVolume { - return 5 + return nonTrivialVolumePoints } - return 0 + return noPoints } diff --git a/pkg/security/scc/byrestrictions_test.go b/pkg/security/scc/byrestrictions_test.go index 07dbd3957cde..46296fc5b875 100644 --- a/pkg/security/scc/byrestrictions_test.go +++ b/pkg/security/scc/byrestrictions_test.go @@ -8,7 +8,8 @@ import ( func TestPointValue(t *testing.T) { newSCC := func(priv bool, seLinuxStrategy securityapi.SELinuxContextStrategyType, userStrategy securityapi.RunAsUserStrategyType) *securityapi.SecurityContextConstraints { - scc := &securityapi.SecurityContextConstraints{ + return &securityapi.SecurityContextConstraints{ + AllowPrivilegedContainer: priv, SELinuxContext: securityapi.SELinuxContextStrategyOptions{ Type: seLinuxStrategy, }, @@ -16,26 +17,19 @@ func TestPointValue(t *testing.T) { Type: userStrategy, }, } - if priv { - scc.AllowPrivilegedContainer = true - } - - return scc } - seLinuxStrategies := map[securityapi.SELinuxContextStrategyType]int{ - securityapi.SELinuxStrategyRunAsAny: 4, - securityapi.SELinuxStrategyMustRunAs: 1, + seLinuxStrategies := map[securityapi.SELinuxContextStrategyType]points{ + securityapi.SELinuxStrategyRunAsAny: runAsAnyUserPoints, + securityapi.SELinuxStrategyMustRunAs: runAsUserPoints, } - userStrategies := map[securityapi.RunAsUserStrategyType]int{ - securityapi.RunAsUserStrategyRunAsAny: 4, - securityapi.RunAsUserStrategyMustRunAsNonRoot: 3, - securityapi.RunAsUserStrategyMustRunAsRange: 2, - securityapi.RunAsUserStrategyMustRunAs: 1, + userStrategies := map[securityapi.RunAsUserStrategyType]points{ + securityapi.RunAsUserStrategyRunAsAny: runAsAnyUserPoints, + securityapi.RunAsUserStrategyMustRunAsNonRoot: runAsNonRootPoints, + securityapi.RunAsUserStrategyMustRunAsRange: runAsRangePoints, + securityapi.RunAsUserStrategyMustRunAs: runAsUserPoints, } - privilegedPoints := 20 - // run through all combos of user strategy + seLinux strategy + priv for userStrategy, userStrategyPoints := range userStrategies { for seLinuxStrategy, seLinuxStrategyPoints := range seLinuxStrategies { @@ -61,7 +55,9 @@ func TestPointValue(t *testing.T) { scc := newSCC(false, securityapi.SELinuxStrategyMustRunAs, securityapi.RunAsUserStrategyMustRunAs) scc.Volumes = []securityapi.FSType{securityapi.FSTypeHostPath} actualPoints := pointValue(scc) - if actualPoints != 12 { //1 (SELinux) + 1 (User) + 10 (host path volume) + // SELinux + User + host path volume + expectedPoints := runAsUserPoints + runAsUserPoints + hostVolumePoints + if actualPoints != expectedPoints { t.Errorf("volume score was not added to the scc point value correctly!") } } @@ -90,79 +86,79 @@ func TestVolumePointValue(t *testing.T) { tests := map[string]struct { scc *securityapi.SecurityContextConstraints - expectedPoints int + expectedPoints points }{ "all volumes": { scc: allowAllSCC, - expectedPoints: 10, + expectedPoints: hostVolumePoints, }, "host volume": { scc: newSCC(true, false, false), - expectedPoints: 10, + expectedPoints: hostVolumePoints, }, "host volume and non trivial volumes": { scc: newSCC(true, true, false), - expectedPoints: 10, + expectedPoints: hostVolumePoints, }, "host volume, non trivial, and trivial": { scc: newSCC(true, true, true), - expectedPoints: 10, + expectedPoints: hostVolumePoints, }, "non trivial": { scc: newSCC(false, true, false), - expectedPoints: 5, + expectedPoints: nonTrivialVolumePoints, }, "non trivial and trivial": { scc: newSCC(false, true, true), - expectedPoints: 5, + expectedPoints: nonTrivialVolumePoints, }, "trivial": { scc: newSCC(false, false, true), - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - secret": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSTypeSecret}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - configMap": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSTypeConfigMap}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - emptyDir": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSTypeEmptyDir}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - downwardAPI": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSTypeDownwardAPI}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - projected": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSProjected}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "trivial - none": { scc: &securityapi.SecurityContextConstraints{ Volumes: []securityapi.FSType{securityapi.FSTypeNone}, }, - expectedPoints: 0, + expectedPoints: noPoints, }, "no volumes allowed": { scc: newSCC(false, false, false), - expectedPoints: 0, + expectedPoints: noPoints, }, "nil volumes": { scc: nilVolumeSCC, - expectedPoints: 0, + expectedPoints: noPoints, }, } for k, v := range tests {