Skip to content

Commit

Permalink
SecurityContextConstraints: pass effective runAsNonRoot and runAsUser…
Browse files Browse the repository at this point in the history
… to user validation interface.
  • Loading branch information
php-coder committed Dec 18, 2017
1 parent b5a8497 commit 3d4c343
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 65 deletions.
2 changes: 1 addition & 1 deletion pkg/security/securitycontextconstraints/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
}

sc := container.SecurityContext
allErrs = append(allErrs, s.runAsUserStrategy.Validate(pod, container)...)
allErrs = append(allErrs, s.runAsUserStrategy.Validate(fldPath.Child("securityContext"), pod, container, sc.RunAsNonRoot, sc.RunAsUser)...)
allErrs = append(allErrs, s.seLinuxStrategy.Validate(fldPath.Child("seLinuxOptions"), pod, container, sc.SELinuxOptions)...)
allErrs = append(allErrs, s.seccompStrategy.ValidateContainer(pod, container)...)

Expand Down
2 changes: 1 addition & 1 deletion pkg/security/securitycontextconstraints/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
"failUserSCC": {
pod: failUserPod,
scc: failUserSCC,
expectedError: "does not match required UID",
expectedError: "runAsUser: Invalid value",
},
"failSELinuxSCC": {
pod: failSELinuxPod,
Expand Down
20 changes: 7 additions & 13 deletions pkg/security/securitycontextconstraints/user/mustrunas.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,18 @@ func (s *mustRunAs) Generate(pod *api.Pod, container *api.Container) (*int64, er
}

// Validate ensures that the specified values fall within the range of the strategy.
func (s *mustRunAs) Validate(pod *api.Pod, container *api.Container) field.ErrorList {
func (s *mustRunAs) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList {
allErrs := field.ErrorList{}

securityContextPath := field.NewPath("securityContext")
if container.SecurityContext == nil {
detail := fmt.Sprintf("unable to validate nil security context for container %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath, container.SecurityContext, detail))
return allErrs
}
if container.SecurityContext.RunAsUser == nil {
detail := fmt.Sprintf("unable to validate nil runAsUser for container %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), container.SecurityContext.RunAsUser, detail))
if runAsUser == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("runAsUser"), ""))
return allErrs
}

if *s.opts.UID != *container.SecurityContext.RunAsUser {
detail := fmt.Sprintf("UID on container %s does not match required UID. Found %d, wanted %d", container.Name, *container.SecurityContext.RunAsUser, *s.opts.UID)
allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), *container.SecurityContext.RunAsUser, detail))
if *s.opts.UID != *runAsUser {
detail := fmt.Sprintf("must be: %v", *s.opts.UID)
allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *runAsUser, detail))
return allErrs
}

return allErrs
Expand Down
21 changes: 14 additions & 7 deletions pkg/security/securitycontextconstraints/user/mustrunas_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package user

import (
"fmt"
"strings"
"testing"

api "k8s.io/kubernetes/pkg/apis/core"
Expand Down Expand Up @@ -58,19 +60,24 @@ func TestMustRunAsValidate(t *testing.T) {
if err != nil {
t.Fatalf("unexpected error initializing NewMustRunAs %v", err)
}
container := &api.Container{
SecurityContext: &api.SecurityContext{
RunAsUser: &badUID,
},

errs := mustRunAs.Validate(nil, nil, nil, nil, nil)
expectedMessage := "runAsUser: Required value"
if len(errs) == 0 {
t.Errorf("expected errors from nil runAsUser but got none")
} else if !strings.Contains(errs[0].Error(), expectedMessage) {
t.Errorf("expected error to contain %q but it did not: %v", expectedMessage, errs)
}

errs := mustRunAs.Validate(nil, container)
errs = mustRunAs.Validate(nil, nil, nil, nil, &badUID)
expectedMessage = fmt.Sprintf("runAsUser: Invalid value: %d: must be: %d", badUID, uid)
if len(errs) == 0 {
t.Errorf("expected errors from mismatch uid but got none")
} else if !strings.Contains(errs[0].Error(), expectedMessage) {
t.Errorf("expected error to contain %q but it did not: %v", expectedMessage, errs)
}

container.SecurityContext.RunAsUser = &uid
errs = mustRunAs.Validate(nil, container)
errs = mustRunAs.Validate(nil, nil, nil, nil, &uid)
if len(errs) != 0 {
t.Errorf("expected no errors from matching uid but got %v", errs)
}
Expand Down
30 changes: 11 additions & 19 deletions pkg/security/securitycontextconstraints/user/mustrunasrange.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import (
securityapi "github.com/openshift/origin/pkg/security/apis/security"
)

// mustRunAs implements the RunAsUserSecurityContextConstraintsStrategy interface
// mustRunAsRange implements the RunAsUserSecurityContextConstraintsStrategy interface
type mustRunAsRange struct {
opts *securityapi.RunAsUserStrategyOptions
}

// NewMustRunAs provides a strategy that requires the container to run as a specific UID in a range.
var _ RunAsUserSecurityContextConstraintsStrategy = &mustRunAsRange{}

// NewMustRunAsRange provides a strategy that requires the container to run as a specific UID in a range.
func NewMustRunAsRange(options *securityapi.RunAsUserStrategyOptions) (RunAsUserSecurityContextConstraintsStrategy, error) {
if options == nil {
return nil, fmt.Errorf("MustRunAsRange requires run as user options")
Expand All @@ -36,28 +38,18 @@ func (s *mustRunAsRange) Generate(pod *api.Pod, container *api.Container) (*int6
}

// Validate ensures that the specified values fall within the range of the strategy.
func (s *mustRunAsRange) Validate(pod *api.Pod, container *api.Container) field.ErrorList {
func (s *mustRunAsRange) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList {
allErrs := field.ErrorList{}

securityContextPath := field.NewPath("securityContext")
if container.SecurityContext == nil {
detail := fmt.Sprintf("unable to validate nil security context for container %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath, container.SecurityContext, detail))
return allErrs
}
if container.SecurityContext.RunAsUser == nil {
detail := fmt.Sprintf("unable to validate nil RunAsUser for container %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), container.SecurityContext.RunAsUser, detail))
if runAsUser == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("runAsUser"), ""))
return allErrs
}

if *container.SecurityContext.RunAsUser < *s.opts.UIDRangeMin || *container.SecurityContext.RunAsUser > *s.opts.UIDRangeMax {
detail := fmt.Sprintf("UID on container %s does not match required range. Found %d, required min: %d max: %d",
container.Name,
*container.SecurityContext.RunAsUser,
*s.opts.UIDRangeMin,
*s.opts.UIDRangeMax)
allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), *container.SecurityContext.RunAsUser, detail))
if *runAsUser < *s.opts.UIDRangeMin || *runAsUser > *s.opts.UIDRangeMax {
detail := fmt.Sprintf("must be in the ranges: [%v, %v]", *s.opts.UIDRangeMin, *s.opts.UIDRangeMax)
allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *runAsUser, detail))
return allErrs
}

return allErrs
Expand Down
19 changes: 9 additions & 10 deletions pkg/security/securitycontextconstraints/user/nonroot.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package user

import (
"fmt"

"k8s.io/apimachinery/pkg/util/validation/field"
api "k8s.io/kubernetes/pkg/apis/core"

Expand All @@ -27,17 +25,18 @@ func (s *nonRoot) Generate(pod *api.Pod, container *api.Container) (*int64, erro
// of this will pass if either the UID is not set, assuming that the image will provided the UID
// or if the UID is set it is not root. In order to work properly this assumes that the kubelet
// will populate an
func (s *nonRoot) Validate(pod *api.Pod, container *api.Container) field.ErrorList {
func (s *nonRoot) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList {
allErrs := field.ErrorList{}
securityContextPath := field.NewPath("securityContext")
if container.SecurityContext == nil {
detail := fmt.Sprintf("unable to validate nil security context for container %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath, container.SecurityContext, detail))
if runAsNonRoot == nil && runAsUser == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("runAsNonRoot"), "must be true"))
return allErrs
}
if runAsNonRoot != nil && *runAsNonRoot == false {
allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsNonRoot"), *runAsNonRoot, "must be true"))
return allErrs
}
if container.SecurityContext.RunAsUser != nil && *container.SecurityContext.RunAsUser == 0 {
detail := fmt.Sprintf("running with the root UID is forbidden by the security context constraints %s", container.Name)
allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), *container.SecurityContext.RunAsUser, detail))
if runAsUser != nil && *runAsUser == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *runAsUser, "running with the root UID is forbidden"))
return allErrs
}
return allErrs
Expand Down
37 changes: 26 additions & 11 deletions pkg/security/securitycontextconstraints/user/nonroot_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package user

import (
"strings"
"testing"

api "k8s.io/kubernetes/pkg/apis/core"
Expand Down Expand Up @@ -38,27 +39,41 @@ func TestNonRootValidate(t *testing.T) {
var badUID int64 = 0
s, err := NewRunAsNonRoot(&securityapi.RunAsUserStrategyOptions{})
if err != nil {
t.Fatalf("unexpected error initializing NewMustRunAs %v", err)
}
container := &api.Container{
SecurityContext: &api.SecurityContext{
RunAsUser: &badUID,
},
t.Fatalf("unexpected error initializing NewRunAsNonRoot %v", err)
}

errs := s.Validate(nil, container)
errs := s.Validate(nil, nil, nil, nil, &badUID)
expectedMessage := "runAsUser: Invalid value: 0: running with the root UID is forbidden"
if len(errs) == 0 {
t.Errorf("expected errors from root uid but got none")
} else if !strings.Contains(errs[0].Error(), expectedMessage) {
t.Errorf("expected error to contain %q but it did not: %v", expectedMessage, errs)
}

errs = s.Validate(nil, nil, nil, nil, nil)
expectedMessage = "runAsNonRoot: Required value: must be true"
if len(errs) == 0 {
t.Errorf("expected error when neither runAsUser nor runAsNonRoot are specified but got none")
} else if !strings.Contains(errs[0].Error(), expectedMessage) {
t.Errorf("expected error to contain %q but it did not: %v", expectedMessage, errs)
}

no := false
errs = s.Validate(nil, nil, nil, &no, nil)
expectedMessage = "runAsNonRoot: Invalid value: false: must be true"
if len(errs) == 0 {
t.Errorf("expected error when runAsNonRoot is false but got none")
} else if !strings.Contains(errs[0].Error(), expectedMessage) {
t.Errorf("expected error to contain %q but it did not: %v", expectedMessage, errs)
}

container.SecurityContext.RunAsUser = &uid
errs = s.Validate(nil, container)
errs = s.Validate(nil, nil, nil, nil, &uid)
if len(errs) != 0 {
t.Errorf("expected no errors from non-root uid but got %v", errs)
}

container.SecurityContext.RunAsUser = nil
errs = s.Validate(nil, container)
yes := true
errs = s.Validate(nil, nil, nil, &yes, nil)
if len(errs) != 0 {
t.Errorf("expected no errors from nil uid but got %v", errs)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/securitycontextconstraints/user/runasany.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ func (s *runAsAny) Generate(pod *api.Pod, container *api.Container) (*int64, err
}

// Validate ensures that the specified values fall within the range of the strategy.
func (s *runAsAny) Validate(pod *api.Pod, container *api.Container) field.ErrorList {
func (s *runAsAny) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList {
return field.ErrorList{}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestRunAsAnyValidate(t *testing.T) {
if err != nil {
t.Fatalf("unexpected error initializing NewRunAsAny %v", err)
}
errs := s.Validate(nil, nil)
errs := s.Validate(nil, nil, nil, nil, nil)
if len(errs) != 0 {
t.Errorf("unexpected errors validating with ")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/securitycontextconstraints/user/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ type RunAsUserSecurityContextConstraintsStrategy interface {
// Generate creates the uid based on policy rules.
Generate(pod *api.Pod, container *api.Container) (*int64, error)
// Validate ensures that the specified values fall within the range of the strategy.
Validate(pod *api.Pod, container *api.Container) field.ErrorList
Validate(fldPath *field.Path, pod *api.Pod, container *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList
}

0 comments on commit 3d4c343

Please sign in to comment.