From 682726410672deff3be1656faecd850bc8bfe594 Mon Sep 17 00:00:00 2001 From: derailed Date: Tue, 7 Jan 2020 20:40:34 -0700 Subject: [PATCH] fix #53 --- change_logs/release_v0.6.1.md | 25 +++++++++ internal/issues/codes.go | 5 +- internal/issues/codes_test.go | 2 +- internal/sanitize/pod.go | 58 ++++++++++++++++++- internal/sanitize/pod_test.go | 102 ++++++++++++++++++++++++++++++++++ 5 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 change_logs/release_v0.6.1.md diff --git a/change_logs/release_v0.6.1.md b/change_logs/release_v0.6.1.md new file mode 100644 index 00000000..67c8fd7e --- /dev/null +++ b/change_logs/release_v0.6.1.md @@ -0,0 +1,25 @@ + + +# 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) + +--- + +  © 2019 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0) diff --git a/internal/issues/codes.go b/internal/issues/codes.go index 89bc3976..06edc876 100644 --- a/internal/issues/codes.go +++ b/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 diff --git a/internal/issues/codes_test.go b/internal/issues/codes_test.go index cf1f39c3..d0029df7 100644 --- a/internal/issues/codes_test.go +++ b/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) } diff --git a/internal/sanitize/pod.go b/internal/sanitize/pod.go index d6ee08cb..63e5c8d1 100644 --- a/internal/sanitize/pod.go +++ b/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 { diff --git a/internal/sanitize/pod_test.go b/internal/sanitize/pod_test.go index 08e0918a..07976686 100644 --- a/internal/sanitize/pod_test.go +++ b/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, + }, + } +}