Merge branch 'issue_53'

change_logs/release_v0.6.1.md

@@ -0,0 +1,25 @@
+<img src="https://raw.githubusercontent.com/derailed/popeye/master/assets/popeye.png" align="right" width="200" height="auto"/>
+
+# Release v0.6.1
+
+## Notes
+
+Thank you so much for your support and suggestions to make Popeye better!!
+
+If you dig this tool, please make some noise on social! [@kitesurfer](https://twitter.com/kitesurfer)
+
+---
+
+## Change Logs
+
+Maintenance release!
+
+---
+
+## Resolved Bugs
+
+* [Issue #53](https://github.com/derailed/popeye/issues/53)
+
+---
+
+<img src="https://raw.githubusercontent.com/derailed/popeye/master/assets/imhotep_logo.png" width="32" height="auto"/>&nbsp; © 2019 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0)

internal/issues/codes.go

@@ -121,7 +121,7 @@ codes:
     message:  Connects to API Server? ServiceAccount token is mounted
     severity: 2
   302:
-    message:  Containers are possibly running as root
+    message:  Pod could be running as root user. Check SecurityContext/image
     severity: 2
   303:
     message: Do you mean it? ServiceAccount is automounting APIServer credentials
@@ -132,6 +132,9 @@ codes:
   305:
     message: References a docker-image "%s" pull secret which does not exist
     severity: 3
+  306:
+    message: Container could be running as root user. Check SecurityContext/Image
+    severity: 2
 
     # -------------------------------------------------------------------------
   # General

internal/issues/codes_test.go

@@ -12,7 +12,7 @@ func TestCodesLoad(t *testing.T) {
 	cc, err := issues.LoadCodes()
 
 	assert.Nil(t, err)
-	assert.Equal(t, 75, len(cc.Glossary))
+	assert.Equal(t, 76, len(cc.Glossary))
 	assert.Equal(t, "No liveness probe", cc.Glossary[103].Message)
 	assert.Equal(t, config.WarnLevel, cc.Glossary[103].Severity)
 }

internal/sanitize/pod.go

@@ -11,6 +11,17 @@ import (
 	mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1"
 )
 
+const (
+	// SecUndefined denotes no root user set
+	SecNonRootUndefined NonRootUser = iota - 1
+	// SecUnset denotes root user
+	SecNonRootUnset = 0
+	// SecSet denotes non root user
+	SecNonRootSet = 1
+)
+
+type NonRootUser int
+
 type (
 	// Pod represents a Pod linter.
 	Pod struct {
@@ -111,11 +122,56 @@ func (p *Pod) checkSecure(ctx context.Context, spec v1.PodSpec) {
 		return
 	}
 
-	if spec.SecurityContext.RunAsNonRoot == nil || !*spec.SecurityContext.RunAsNonRoot {
+	// If pod security ctx is present and we have
+	podSec := hasPodNonRootUser(spec.SecurityContext)
+	var victims int
+	for _, co := range spec.InitContainers {
+		if !checkCOSecurityContext(co) && !podSec {
+			victims++
+			p.AddSubCode(internal.WithGroup(ctx, co.Name), 306)
+		}
+	}
+	for _, co := range spec.Containers {
+		if !checkCOSecurityContext(co) && !podSec {
+			victims++
+			p.AddSubCode(internal.WithGroup(ctx, co.Name), 306)
+		}
+	}
+	if victims > 0 && !podSec {
 		p.AddCode(ctx, 302)
 	}
 }
 
+func checkCOSecurityContext(co v1.Container) bool {
+	return hasCoNonRootUser(co.SecurityContext)
+}
+
+func hasPodNonRootUser(sec *v1.PodSecurityContext) bool {
+	if sec == nil {
+		return false
+	}
+	if sec.RunAsNonRoot != nil {
+		return *sec.RunAsNonRoot
+	}
+	if sec.RunAsUser != nil {
+		return *sec.RunAsUser != 0
+	}
+	return false
+}
+
+func hasCoNonRootUser(sec *v1.SecurityContext) bool {
+	if sec == nil {
+		return false
+	}
+	if sec.RunAsNonRoot != nil {
+		return *sec.RunAsNonRoot
+	}
+	if sec.RunAsUser != nil {
+		return *sec.RunAsUser != 0
+	}
+	return false
+}
+
 func (p *Pod) checkContainers(ctx context.Context, po *v1.Pod) {
 	co := NewContainer(internal.MustExtractFQN(ctx), p)
 	for _, c := range po.Spec.InitContainers {

internal/sanitize/pod_test.go

@@ -3,6 +3,7 @@ package sanitize
 import (
 	"testing"
 
+	"github.com/derailed/popeye/internal"
 	"github.com/derailed/popeye/internal/issues"
 	"github.com/derailed/popeye/pkg/config"
 	"github.com/stretchr/testify/assert"
@@ -12,6 +13,66 @@ import (
 	v1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1"
 )
 
+func TestPodCheckSecure(t *testing.T) {
+	uu := map[string]struct {
+		pod    v1.Pod
+		issues int
+	}{
+		"cool_1": {
+			pod:    makeSecPod(SecNonRootSet, SecNonRootSet, SecNonRootSet, SecNonRootSet),
+			issues: 0,
+		},
+		"cool_2": {
+			pod:    makeSecPod(SecNonRootSet, SecNonRootUnset, SecNonRootUnset, SecNonRootUnset),
+			issues: 0,
+		},
+		"cool_3": {
+			pod:    makeSecPod(SecNonRootUnset, SecNonRootSet, SecNonRootSet, SecNonRootSet),
+			issues: 0,
+		},
+		"cool_4": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootSet, SecNonRootSet, SecNonRootSet),
+			issues: 0,
+		},
+		"cool_5": {
+			pod:    makeSecPod(SecNonRootSet, SecNonRootUndefined, SecNonRootUndefined, SecNonRootUndefined),
+			issues: 0,
+		},
+		"hacked_1": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootUndefined, SecNonRootUndefined, SecNonRootUndefined),
+			issues: 4,
+		},
+		"hacked_2": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootUnset, SecNonRootUndefined, SecNonRootUndefined),
+			issues: 4,
+		},
+		"hacked_3": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootSet, SecNonRootUndefined, SecNonRootUndefined),
+			issues: 3,
+		},
+		"hacked_4": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootUnset, SecNonRootSet, SecNonRootUndefined),
+			issues: 3,
+		},
+		"toast": {
+			pod:    makeSecPod(SecNonRootUndefined, SecNonRootUndefined, SecNonRootUndefined, SecNonRootUndefined),
+			issues: 4,
+		},
+	}
+
+	ctx := makeContext("po")
+	ctx = internal.WithFQN(ctx, "default/p1")
+	for k := range uu {
+		u := uu[k]
+		t.Run(k, func(t *testing.T) {
+			p := NewPod(issues.NewCollector(loadCodes(t), makeConfig(t)), nil)
+
+			p.checkSecure(ctx, u.pod.Spec)
+			assert.Equal(t, u.issues, len(p.Outcome()["default/p1"]))
+		})
+	}
+}
+
 func TestPodSanitize(t *testing.T) {
 	uu := map[string]struct {
 		lister PodMXLister
@@ -275,3 +336,44 @@ func makeCS(n string, opts csOpts) v1.ContainerStatus {
 
 	return cs
 }
+
+func makeSecCO(name string, level NonRootUser) v1.Container {
+	t, f := true, false
+	secCtx := v1.SecurityContext{}
+	switch level {
+	case SecNonRootUnset:
+		secCtx.RunAsNonRoot = &f
+	case SecNonRootSet:
+		secCtx.RunAsNonRoot = &t
+	}
+
+	return v1.Container{Name: name, SecurityContext: &secCtx}
+}
+
+func makeSecPod(pod, init, co1, co2 NonRootUser) v1.Pod {
+	t, f := true, false
+
+	secCtx := v1.PodSecurityContext{}
+	switch pod {
+	case SecNonRootUnset:
+		secCtx.RunAsNonRoot = &f
+	case SecNonRootSet:
+		secCtx.RunAsNonRoot = &t
+	}
+
+	var auto bool
+	return v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p1",
+		},
+		Spec: v1.PodSpec{
+			AutomountServiceAccountToken: &auto,
+			InitContainers:               []v1.Container{makeSecCO("i1", init)},
+			Containers: []v1.Container{
+				makeSecCO("co1", co1),
+				makeSecCO("co2", co2),
+			},
+			SecurityContext: &secCtx,
+		},
+	}
+}