From eb88daeeae4a53a20579620e1791e30416517223 Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Tue, 17 May 2022 21:18:53 -0700 Subject: [PATCH] Warn when adding PSA labels to exempt namespaces (#109680) --- .../admission/admission.go | 31 +++- .../admission/admission_test.go | 141 +++++++++++++++++- .../pod-security-admission/api/helpers.go | 53 +++++++ .../api/helpers_test.go | 64 ++++++++ 4 files changed, 280 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission.go b/staging/src/k8s.io/pod-security-admission/admission/admission.go index 53ccb9c58fd..2d897a78c77 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission.go +++ b/staging/src/k8s.io/pod-security-admission/admission/admission.go @@ -249,6 +249,13 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes) if len(newErrs) > 0 { return invalidResponse(attrs, newErrs) } + if a.exemptNamespace(attrs.GetNamespace()) { + if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" { + response := allowedResponse() + response.Warnings = append(response.Warnings, warning) + return response + } + } return sharedAllowedResponse case admissionv1.Update: @@ -286,7 +293,12 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes) return sharedAllowedResponse } if a.exemptNamespace(attrs.GetNamespace()) { - return sharedAllowedByNamespaceExemptionResponse + if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" { + response := allowedResponse() + response.Warnings = append(response.Warnings, warning) + return response + } + return sharedAllowedResponse } response := allowedResponse() response.Warnings = a.EvaluatePodsInNamespace(ctx, namespace.Name, newPolicy.Enforce) @@ -334,10 +346,10 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs api.Attributes) *admi if err != nil { klog.ErrorS(err, "failed to fetch pod namespace", "namespace", attrs.GetNamespace()) a.Metrics.RecordError(true, attrs) - return errorResponse(err, &apierrors.NewInternalError(fmt.Errorf("failed to lookup namespace %s", attrs.GetNamespace())).ErrStatus) + return errorResponse(err, &apierrors.NewInternalError(fmt.Errorf("failed to lookup namespace %q", attrs.GetNamespace())).ErrStatus) } nsPolicy, nsPolicyErrs := a.PolicyToEvaluate(namespace.Labels) - if len(nsPolicyErrs) == 0 && nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged { + if len(nsPolicyErrs) == 0 && nsPolicy.FullyPrivileged() { a.Metrics.RecordEvaluation(metrics.DecisionAllow, nsPolicy.Enforce, metrics.ModeEnforce, attrs) return sharedAllowedPrivilegedResponse } @@ -400,7 +412,7 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs api.Attribu a.Metrics.RecordError(true, attrs) response := allowedResponse() response.AuditAnnotations = map[string]string{ - "error": fmt.Sprintf("failed to lookup namespace %s: %v", attrs.GetNamespace(), err), + "error": fmt.Sprintf("failed to lookup namespace %q: %v", attrs.GetNamespace(), err), } return response } @@ -723,3 +735,14 @@ func containsString(needle string, haystack []string) bool { } return false } + +// exemptNamespaceWarning returns a non-empty warning message if the exempt namespace has a +// non-privileged policy and sets pod security labels. +func (a *Admission) exemptNamespaceWarning(exemptNamespace string, policy api.Policy) string { + if policy.FullyPrivileged() || policy.Equivalent(&a.defaultPolicy) { + return "" + } + + return fmt.Sprintf("namespace %q is exempt from Pod Security, and the policy (%s) will be ignored", + exemptNamespace, policy.CompactString()) +} diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go b/staging/src/k8s.io/pod-security-admission/admission/admission_test.go index 11459ca1a0e..6d20569b07c 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go +++ b/staging/src/k8s.io/pod-security-admission/admission/admission_test.go @@ -267,10 +267,20 @@ func TestValidateNamespace(t *testing.T) { }, { name: "create restricted", - newLabels: map[string]string{api.EnforceLevelLabel: string(api.LevelBaseline)}, + newLabels: map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted)}, expectAllowed: true, expectListPods: false, }, + { + name: "create restricted exempt", + newLabels: map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted)}, + exemptNamespaces: []string{"test"}, + expectAllowed: true, + expectListPods: false, + expectWarnings: []string{ + `namespace "test" is exempt from Pod Security, and the policy (enforce=restricted:latest) will be ignored`, + }, + }, { name: "create malformed level", newLabels: map[string]string{api.EnforceLevelLabel: "unknown"}, @@ -346,10 +356,18 @@ func TestValidateNamespace(t *testing.T) { { name: "update exempt to restricted", exemptNamespaces: []string{"test"}, - newLabels: map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted), api.EnforceVersionLabel: "v1.0"}, - oldLabels: map[string]string{}, - expectAllowed: true, - expectListPods: false, + newLabels: map[string]string{ + api.EnforceLevelLabel: string(api.LevelRestricted), + api.EnforceVersionLabel: "v1.0", + api.AuditLevelLabel: string(api.LevelRestricted), + api.WarnLevelLabel: string(api.LevelBaseline), + }, + oldLabels: map[string]string{}, + expectAllowed: true, + expectListPods: false, + expectWarnings: []string{ + `namespace "test" is exempt from Pod Security, and the policy (enforce=restricted:v1.0, audit=restricted:latest, warn=baseline:latest) will be ignored`, + }, }, // update tests that introduce labels errors @@ -520,6 +538,14 @@ func TestValidateNamespace(t *testing.T) { Level: api.LevelPrivileged, Version: api.LatestVersion(), }, + Audit: api.LevelVersion{ + Level: api.LevelPrivileged, + Version: api.LatestVersion(), + }, + Warn: api.LevelVersion{ + Level: api.LevelPrivileged, + Version: api.LatestVersion(), + }, } if tc.defaultPolicy != nil { defaultPolicy = *tc.defaultPolicy @@ -1112,6 +1138,111 @@ func TestPrioritizePods(t *testing.T) { } } +func TestExemptNamespaceWarning(t *testing.T) { + privileged := api.LevelVersion{ + Level: api.LevelPrivileged, + Version: api.LatestVersion(), + } + privilegedPolicy := api.Policy{ + Enforce: privileged, + Audit: privileged, + Warn: privileged, + } + baseline := api.LevelVersion{ + Level: api.LevelBaseline, + Version: api.MajorMinorVersion(1, 23), + } + baselinePolicy := api.Policy{ + Enforce: baseline, + Audit: baseline, + Warn: baseline, + } + tests := []struct { + name string + labels map[string]string + defaultPolicy api.Policy // Defaults to privilegedPolicy if empty. + expectWarning bool + expectWarningContains string + }{{ + name: "empty-case", + expectWarning: false, + }, { + name: "ignore-privileged", + labels: map[string]string{ + api.EnforceLevelLabel: string(api.LevelPrivileged), + api.EnforceVersionLabel: "v1.24", + api.WarnVersionLabel: "v1.25", + }, + expectWarning: false, + }, { + name: "warn-on-enforce", + labels: map[string]string{ + api.EnforceLevelLabel: string(api.LevelBaseline), + }, + expectWarning: true, + expectWarningContains: "(enforce=baseline:latest)", + }, { + name: "warn-on-warn", + labels: map[string]string{ + api.WarnLevelLabel: string(api.LevelBaseline), + }, + expectWarning: true, + expectWarningContains: "(warn=baseline:latest)", + }, { + name: "warn-on-audit", + labels: map[string]string{ + api.AuditLevelLabel: string(api.LevelRestricted), + }, + expectWarning: true, + expectWarningContains: "(audit=restricted:latest)", + }, { + name: "ignore-default-policy", + defaultPolicy: baselinePolicy, + expectWarning: false, + }, { + name: "warn-versions-default-policy", + labels: map[string]string{ + api.WarnVersionLabel: "latest", + }, + defaultPolicy: baselinePolicy, + expectWarning: true, + expectWarningContains: "(enforce=baseline:v1.23, audit=baseline:v1.23, warn=baseline:latest)", + }} + + const ( + sentinelLabelKey = "qqincegidneocgu" + sentinelLabelValue = "vpmxkpcjphxrcpx" + ) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defaultPolicy := test.defaultPolicy + if defaultPolicy == (api.Policy{}) { + defaultPolicy = privilegedPolicy + } + a := &Admission{defaultPolicy: defaultPolicy} + labels := test.labels + if labels == nil { + labels = map[string]string{} + } + labels[sentinelLabelKey] = sentinelLabelValue + policy, err := api.PolicyToEvaluate(labels, defaultPolicy) + require.NoError(t, err.ToAggregate()) + + warning := a.exemptNamespaceWarning(test.name, policy) + if !test.expectWarning { + assert.Empty(t, warning) + return + } + require.NotEmpty(t, warning) + + assert.NotContains(t, warning, sentinelLabelKey, "non-podsecurity label key included") + assert.NotContains(t, warning, sentinelLabelValue, "non-podsecurity label value included") + + assert.Contains(t, warning, test.expectWarningContains) + }) + } +} + type testAttributes struct { api.AttributesRecord diff --git a/staging/src/k8s.io/pod-security-admission/api/helpers.go b/staging/src/k8s.io/pod-security-admission/api/helpers.go index d46670de6c5..67aa83e5f0f 100644 --- a/staging/src/k8s.io/pod-security-admission/api/helpers.go +++ b/staging/src/k8s.io/pod-security-admission/api/helpers.go @@ -144,12 +144,65 @@ func (lv LevelVersion) String() string { return fmt.Sprintf("%s:%s", lv.Level, lv.Version) } +// Equivalent determines whether two LevelVersions are functionally equivalent. LevelVersions are +// considered equivalent if both are privileged, or both levels & versions are equal. +func (lv *LevelVersion) Equivalent(other *LevelVersion) bool { + return (lv.Level == LevelPrivileged && other.Level == LevelPrivileged) || + (lv.Level == other.Level && lv.Version == other.Version) +} + type Policy struct { Enforce LevelVersion Audit LevelVersion Warn LevelVersion } +func (p *Policy) String() string { + return fmt.Sprintf("enforce=%#v, audit=%#v, warn=%#v", p.Enforce, p.Audit, p.Warn) +} + +// CompactString prints a minimalist representation of the policy that excludes any privileged +// levels. +func (p *Policy) CompactString() string { + sb := strings.Builder{} + if p.Enforce.Level != LevelPrivileged { + sb.WriteString("enforce=") + sb.WriteString(p.Enforce.String()) + } + if p.Audit.Level != LevelPrivileged { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString("audit=") + sb.WriteString(p.Audit.String()) + } + if p.Warn.Level != LevelPrivileged { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString("warn=") + sb.WriteString(p.Warn.String()) + } + if sb.Len() == 0 { + // All modes were privileged, just output "privileged". + return string(LevelPrivileged) + } + return sb.String() +} + +// Equivalent determines whether two policies are functionally equivalent. Policies are considered +// equivalent if all 3 modes are considered equivalent. +func (p *Policy) Equivalent(other *Policy) bool { + return p.Enforce.Equivalent(&other.Enforce) && p.Audit.Equivalent(&other.Audit) && p.Warn.Equivalent(&other.Warn) +} + +// FullyPrivileged returns true if all 3 policy modes are privileged. +func (p *Policy) FullyPrivileged() bool { + return p.Enforce.Level == LevelPrivileged && + p.Audit.Level == LevelPrivileged && + p.Warn.Level == LevelPrivileged +} + // PolicyToEvaluate resolves the PodSecurity namespace labels to the policy for that namespace, // falling back to the provided defaults when a label is unspecified. A valid policy is always // returned, even when an error is returned. If labels cannot be parsed correctly, the values of diff --git a/staging/src/k8s.io/pod-security-admission/api/helpers_test.go b/staging/src/k8s.io/pod-security-admission/api/helpers_test.go index b1167ec5bd2..09a0b2a9028 100644 --- a/staging/src/k8s.io/pod-security-admission/api/helpers_test.go +++ b/staging/src/k8s.io/pod-security-admission/api/helpers_test.go @@ -53,3 +53,67 @@ func TestParseVersion(t *testing.T) { }) } } + +func TestLevelVersionEquals(t *testing.T) { + t.Run("a LevelVersion should be equal to itself", func(t *testing.T) { + for _, l := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} { + for _, v := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} { + lv := LevelVersion{l, v} + other := lv + assert.True(t, lv.Equivalent(&other), lv.String()) + } + } + }) + t.Run("different levels should not be equal", func(t *testing.T) { + for _, l1 := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} { + for _, l2 := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} { + if l1 != l2 { + lv1 := LevelVersion{l1, LatestVersion()} + lv2 := LevelVersion{l2, LatestVersion()} + assert.False(t, lv1.Equivalent(&lv2), "%#v != %#v", lv1, lv2) + } + } + } + }) + t.Run("different non-privileged versions should not be equal", func(t *testing.T) { + for _, l := range []Level{LevelBaseline, LevelRestricted} { + for _, v1 := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} { + for _, v2 := range []Version{MajorMinorVersion(1, 16), MajorMinorVersion(1, 13)} { + lv1 := LevelVersion{l, v1} + lv2 := LevelVersion{l, v2} + assert.False(t, lv1.Equivalent(&lv2), "%#v != %#v", lv1, lv2) + } + } + } + }) + t.Run("different privileged versions should be equal", func(t *testing.T) { + for _, v1 := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} { + for _, v2 := range []Version{MajorMinorVersion(1, 16), MajorMinorVersion(1, 13)} { + lv1 := LevelVersion{LevelPrivileged, v1} + lv2 := LevelVersion{LevelPrivileged, v2} + assert.True(t, lv1.Equivalent(&lv2), "%#v == %#v", lv1, lv2) + } + } + }) +} + +func TestPolicyEquals(t *testing.T) { + privileged := Policy{ + Enforce: LevelVersion{LevelPrivileged, LatestVersion()}, + Audit: LevelVersion{LevelPrivileged, LatestVersion()}, + Warn: LevelVersion{LevelPrivileged, LatestVersion()}, + } + require.True(t, privileged.FullyPrivileged()) + + privileged2 := privileged + privileged2.Enforce.Version = MajorMinorVersion(1, 20) + require.True(t, privileged2.FullyPrivileged()) + + baseline := privileged + baseline.Audit.Level = LevelBaseline + require.False(t, baseline.FullyPrivileged()) + + assert.True(t, privileged.Equivalent(&privileged2), "ignore privileged versions") + assert.True(t, baseline.Equivalent(&baseline), "baseline policy equals itself") + assert.False(t, privileged.Equivalent(&baseline), "privileged != baseline") +}