From d75c0f0e21af8229ed3147e9a798441221c03574 Mon Sep 17 00:00:00 2001 From: Cao Shufeng Date: Fri, 27 Oct 2017 10:01:01 +0800 Subject: [PATCH] [advanced audit]add a policy wide omitStage --- .../k8s.io/apiserver/pkg/apis/audit/types.go | 9 +- .../pkg/apis/audit/v1alpha1/types.go | 9 +- .../apiserver/pkg/apis/audit/v1beta1/types.go | 9 +- .../pkg/apis/audit/validation/validation.go | 1 + .../apis/audit/validation/validation_test.go | 22 +- .../apiserver/pkg/audit/policy/checker.go | 19 +- .../pkg/audit/policy/checker_test.go | 222 +++++++++++++----- 7 files changed, 223 insertions(+), 68 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go index 00b5c1dc07b..2b318d5754d 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go @@ -153,6 +153,11 @@ type Policy struct { // The default audit level is None, but can be overridden by a catch-all rule at the end of the list. // PolicyRules are strictly ordered. Rules []PolicyRule + + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified per rule in which case the union of both are omitted. + // +optional + OmitStages []Stage } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -208,8 +213,10 @@ type PolicyRule struct { // +optional NonResourceURLs []string - // OmitStages specify events generated in which stages will not be emitted to backend. + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified policy wide in which case the union of both are omitted. // An empty list means no restrictions will apply. + // +optional OmitStages []Stage } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go index ba00bf733e7..21f10c78d48 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go @@ -160,6 +160,11 @@ type Policy struct { // The default audit level is None, but can be overridden by a catch-all rule at the end of the list. // PolicyRules are strictly ordered. Rules []PolicyRule `json:"rules" protobuf:"bytes,2,rep,name=rules"` + + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified per rule in which case the union of both are omitted. + // +optional + OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,3,rep,name=omitStages"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -215,8 +220,10 @@ type PolicyRule struct { // +optional NonResourceURLs []string `json:"nonResourceURLs,omitempty" protobuf:"bytes,7,rep,name=nonResourceURLs"` - // OmitStages specify events generated in which stages will not be emitted to backend. + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified policy wide in which case the union of both are omitted. // An empty list means no restrictions will apply. + // +optional OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,8,rep,name=omitStages"` } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go index b76170f4492..259599c050e 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go @@ -156,6 +156,11 @@ type Policy struct { // The default audit level is None, but can be overridden by a catch-all rule at the end of the list. // PolicyRules are strictly ordered. Rules []PolicyRule `json:"rules" protobuf:"bytes,2,rep,name=rules"` + + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified per rule in which case the union of both are omitted. + // +optional + OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,3,rep,name=omitStages"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -211,8 +216,10 @@ type PolicyRule struct { // +optional NonResourceURLs []string `json:"nonResourceURLs,omitempty" protobuf:"bytes,7,rep,name=nonResourceURLs"` - // OmitStages specify events generated in which stages will not be emitted to backend. + // OmitStages is a list of stages for which no events are created. Note that this can also + // be specified policy wide in which case the union of both are omitted. // An empty list means no restrictions will apply. + // +optional OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,8,rep,name=omitStages"` } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go index 6520a763948..f80aba01ffe 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go @@ -26,6 +26,7 @@ import ( func ValidatePolicy(policy *audit.Policy) field.ErrorList { var allErrs field.ErrorList + allErrs = append(allErrs, validateOmitStages(policy.OmitStages, field.NewPath("omitStages"))...) rulePath := field.NewPath("rules") for i, rule := range policy.Rules { allErrs = append(allErrs, validatePolicyRule(rule, rulePath.Index(i))...) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go index 53d60782a40..a25d5dfcaa6 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go @@ -54,7 +54,9 @@ func TestValidatePolicy(t *testing.T) { for _, rule := range validRules { successCases = append(successCases, audit.Policy{Rules: []audit.PolicyRule{rule}}) } - successCases = append(successCases, audit.Policy{}) // Empty policy is valid. + successCases = append(successCases, audit.Policy{}) // Empty policy is valid. + successCases = append(successCases, audit.Policy{OmitStages: []audit.Stage{ // Policy with omitStages + audit.Stage("RequestReceived")}}) successCases = append(successCases, audit.Policy{Rules: validRules}) // Multiple rules. for i, policy := range successCases { @@ -113,7 +115,7 @@ func TestValidatePolicy(t *testing.T) { Resources: []audit.GroupResources{{ResourceNames: []string{"leader"}}}, Namespaces: []string{"kube-system"}, }, - { // invalid omitStages + { // invalid omitStages in rule Level: audit.LevelMetadata, OmitStages: []audit.Stage{ audit.Stage("foo"), @@ -124,7 +126,21 @@ func TestValidatePolicy(t *testing.T) { for _, rule := range invalidRules { errorCases = append(errorCases, audit.Policy{Rules: []audit.PolicyRule{rule}}) } - errorCases = append(errorCases, audit.Policy{Rules: append(validRules, audit.PolicyRule{})}) // Multiple rules. + + // Multiple rules. + errorCases = append(errorCases, audit.Policy{Rules: append(validRules, audit.PolicyRule{})}) + + // invalid omitStages in policy + policy := audit.Policy{OmitStages: []audit.Stage{ + audit.Stage("foo"), + }, + Rules: []audit.PolicyRule{ + { + Level: audit.LevelMetadata, + }, + }, + } + errorCases = append(errorCases, policy) for i, policy := range errorCases { if errs := ValidatePolicy(&policy); len(errs) == 0 { diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go index b92ffe26b68..3259013ad72 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -36,9 +36,26 @@ type Checker interface { // NewChecker creates a new policy checker. func NewChecker(policy *audit.Policy) Checker { + for i, rule := range policy.Rules { + policy.Rules[i].OmitStages = unionStages(policy.OmitStages, rule.OmitStages) + } return &policyChecker{*policy} } +func unionStages(stageLists ...[]audit.Stage) []audit.Stage { + m := make(map[audit.Stage]bool) + for _, sl := range stageLists { + for _, s := range sl { + m[s] = true + } + } + result := make([]audit.Stage, 0, len(m)) + for key := range m { + result = append(result, key) + } + return result +} + // FakeChecker creates a checker that returns a constant level for all requests (for testing). func FakeChecker(level audit.Level, stage []audit.Stage) Checker { return &fakeChecker{level, stage} @@ -54,7 +71,7 @@ func (p *policyChecker) LevelAndStages(attrs authorizer.Attributes) (audit.Level return rule.Level, rule.OmitStages } } - return DefaultAuditLevel, nil + return DefaultAuditLevel, p.OmitStages } // Check whether the rule matches the request attrs. diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go index d6cc5f09171..0f323436a92 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go @@ -28,12 +28,12 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" ) -func TestChecker(t *testing.T) { - tim := &user.DefaultInfo{ +var ( + tim = &user.DefaultInfo{ Name: "tim@k8s.io", Groups: []string{"humans", "developers"}, } - attrs := map[string]authorizer.Attributes{ + attrs = map[string]authorizer.Attributes{ "namespaced": &authorizer.AttributesRecord{ User: tim, Verb: "get", @@ -75,7 +75,7 @@ func TestChecker(t *testing.T) { }, } - rules := map[string]audit.PolicyRule{ + rules = map[string]audit.PolicyRule{ "default": { Level: audit.LevelMetadata, }, @@ -151,65 +151,165 @@ func TestChecker(t *testing.T) { }, }, } +) - test := func(req string, expLevel audit.Level, expOmitStages []audit.Stage, ruleNames ...string) { - policy := audit.Policy{} - for _, rule := range ruleNames { - require.Contains(t, rules, rule) - policy.Rules = append(policy.Rules, rules[rule]) +func test(t *testing.T, req string, expLevel audit.Level, policyStages, expOmitStages []audit.Stage, ruleNames ...string) { + policy := audit.Policy{OmitStages: policyStages} + for _, rule := range ruleNames { + require.Contains(t, rules, rule) + policy.Rules = append(policy.Rules, rules[rule]) + } + require.Contains(t, attrs, req) + actualLevel, actualOmitStages := NewChecker(&policy).LevelAndStages(attrs[req]) + assert.Equal(t, expLevel, actualLevel, "request:%s rules:%s", req, strings.Join(ruleNames, ",")) + assert.True(t, stageEqual(expOmitStages, actualOmitStages), "request:%s rules:%s, expected stages: %v, actual stages: %v", + req, strings.Join(ruleNames, ","), expOmitStages, actualOmitStages) +} + +func testAuditLevel(t *testing.T, stages []audit.Stage) { + test(t, "namespaced", audit.LevelMetadata, stages, stages, "default") + test(t, "namespaced", audit.LevelNone, stages, stages, "create") + test(t, "namespaced", audit.LevelMetadata, stages, stages, "tims") + test(t, "namespaced", audit.LevelMetadata, stages, stages, "humans") + test(t, "namespaced", audit.LevelNone, stages, stages, "serviceAccounts") + test(t, "namespaced", audit.LevelRequestResponse, stages, stages, "getPods") + test(t, "namespaced", audit.LevelNone, stages, stages, "getClusterRoles") + test(t, "namespaced", audit.LevelNone, stages, stages, "getLogs") + test(t, "namespaced", audit.LevelNone, stages, stages, "getMetrics") + test(t, "namespaced", audit.LevelMetadata, stages, stages, "getMetrics", "serviceAccounts", "default") + test(t, "namespaced", audit.LevelRequestResponse, stages, stages, "getMetrics", "getPods", "default") + test(t, "namespaced", audit.LevelRequestResponse, stages, stages, "getPodLogs", "getPods") + + test(t, "cluster", audit.LevelMetadata, stages, stages, "default") + test(t, "cluster", audit.LevelNone, stages, stages, "create") + test(t, "cluster", audit.LevelMetadata, stages, stages, "tims") + test(t, "cluster", audit.LevelMetadata, stages, stages, "humans") + test(t, "cluster", audit.LevelNone, stages, stages, "serviceAccounts") + test(t, "cluster", audit.LevelNone, stages, stages, "getPods") + test(t, "cluster", audit.LevelRequestResponse, stages, stages, "getClusterRoles") + test(t, "cluster", audit.LevelRequest, stages, stages, "clusterRoleEdit", "getClusterRoles") + test(t, "cluster", audit.LevelNone, stages, stages, "getLogs") + test(t, "cluster", audit.LevelNone, stages, stages, "getMetrics") + test(t, "cluster", audit.LevelMetadata, stages, stages, "getMetrics", "serviceAccounts", "default") + test(t, "cluster", audit.LevelRequestResponse, stages, stages, "getMetrics", "getClusterRoles", "default") + test(t, "cluster", audit.LevelNone, stages, stages, "getPodLogs", "getPods") + + test(t, "nonResource", audit.LevelMetadata, stages, stages, "default") + test(t, "nonResource", audit.LevelNone, stages, stages, "create") + test(t, "nonResource", audit.LevelMetadata, stages, stages, "tims") + test(t, "nonResource", audit.LevelMetadata, stages, stages, "humans") + test(t, "nonResource", audit.LevelNone, stages, stages, "serviceAccounts") + test(t, "nonResource", audit.LevelNone, stages, stages, "getPods") + test(t, "nonResource", audit.LevelNone, stages, stages, "getClusterRoles") + test(t, "nonResource", audit.LevelRequestResponse, stages, stages, "getLogs") + test(t, "nonResource", audit.LevelNone, stages, stages, "getMetrics") + test(t, "nonResource", audit.LevelMetadata, stages, stages, "getMetrics", "serviceAccounts", "default") + test(t, "nonResource", audit.LevelRequestResponse, stages, stages, "getLogs", "getClusterRoles", "default") + test(t, "nonResource", audit.LevelNone, stages, stages, "getPodLogs", "getPods") + + test(t, "subresource", audit.LevelRequest, stages, stages, "getPodLogs", "getPods") + +} + +func TestChecker(t *testing.T) { + testAuditLevel(t, nil) + + // test omitStages pre rule + test(t, "namespaced", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") + test(t, "namespaced", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") + test(t, "cluster", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") + test(t, "cluster", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") + test(t, "nonResource", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") + test(t, "nonResource", audit.LevelRequest, nil, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") +} + +func TestCheckerPolicyOmitStages(t *testing.T) { + policyStages := []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted} + testAuditLevel(t, policyStages) + + // test omitStages policy wide + test(t, "namespaced", audit.LevelRequest, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted}, "omit RequestReceived", "getPods", "default") + test(t, "namespaced", audit.LevelRequest, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") + test(t, "cluster", audit.LevelRequest, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted}, "omit RequestReceived", "getPods", "default") + test(t, "cluster", audit.LevelRequest, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") + test(t, "nonResource", audit.LevelMetadata, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted}, "default", "omit RequestReceived", "getPods") + test(t, "nonResource", audit.LevelRequest, policyStages, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") +} + +// stageEqual returns true if s1 and s2 are super set of each other +func stageEqual(s1, s2 []audit.Stage) bool { + m1 := make(map[audit.Stage]bool) + m2 := make(map[audit.Stage]bool) + for _, s := range s1 { + m1[s] = true + } + for _, s := range s2 { + m2[s] = true + } + if len(m1) != len(m2) { + return false + } + for key, value := range m1 { + if m2[key] != value { + return false } - require.Contains(t, attrs, req) - actualLevel, actualOmitStages := NewChecker(&policy).LevelAndStages(attrs[req]) - assert.Equal(t, expLevel, actualLevel, "request:%s rules:%s", req, strings.Join(ruleNames, ",")) - assert.Equal(t, expOmitStages, actualOmitStages, "request:%s rules:%s", req, strings.Join(ruleNames, ",")) + } + return true +} + +func TestUnionStages(t *testing.T) { + var testCases = []struct { + s1, s2, exp []audit.Stage + }{ + { + []audit.Stage{}, + []audit.Stage{}, + []audit.Stage{}, + }, + { + []audit.Stage{audit.StageRequestReceived}, + []audit.Stage{}, + []audit.Stage{audit.StageRequestReceived}, + }, + { + []audit.Stage{audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived}, + }, + { + []audit.Stage{audit.StageRequestReceived}, + []audit.Stage{audit.StageResponseStarted}, + []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted}, + }, + { + []audit.Stage{audit.StageRequestReceived, audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived, audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived}, + }, + { + []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted}, + []audit.Stage{audit.StagePanic, audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StagePanic}, + }, + { + nil, + []audit.Stage{audit.StageRequestReceived}, + []audit.Stage{audit.StageRequestReceived}, + }, } - test("namespaced", audit.LevelMetadata, nil, "default") - test("namespaced", audit.LevelNone, nil, "create") - test("namespaced", audit.LevelMetadata, nil, "tims") - test("namespaced", audit.LevelMetadata, nil, "humans") - test("namespaced", audit.LevelNone, nil, "serviceAccounts") - test("namespaced", audit.LevelRequestResponse, nil, "getPods") - test("namespaced", audit.LevelNone, nil, "getClusterRoles") - test("namespaced", audit.LevelNone, nil, "getLogs") - test("namespaced", audit.LevelNone, nil, "getMetrics") - test("namespaced", audit.LevelMetadata, nil, "getMetrics", "serviceAccounts", "default") - test("namespaced", audit.LevelRequestResponse, nil, "getMetrics", "getPods", "default") - test("namespaced", audit.LevelRequestResponse, nil, "getPodLogs", "getPods") - test("namespaced", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") - test("namespaced", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") - - test("cluster", audit.LevelMetadata, nil, "default") - test("cluster", audit.LevelNone, nil, "create") - test("cluster", audit.LevelMetadata, nil, "tims") - test("cluster", audit.LevelMetadata, nil, "humans") - test("cluster", audit.LevelNone, nil, "serviceAccounts") - test("cluster", audit.LevelNone, nil, "getPods") - test("cluster", audit.LevelRequestResponse, nil, "getClusterRoles") - test("cluster", audit.LevelRequest, nil, "clusterRoleEdit", "getClusterRoles") - test("cluster", audit.LevelNone, nil, "getLogs") - test("cluster", audit.LevelNone, nil, "getMetrics") - test("cluster", audit.LevelMetadata, nil, "getMetrics", "serviceAccounts", "default") - test("cluster", audit.LevelRequestResponse, nil, "getMetrics", "getClusterRoles", "default") - test("cluster", audit.LevelNone, nil, "getPodLogs", "getPods") - test("cluster", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") - test("cluster", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") - - test("nonResource", audit.LevelMetadata, nil, "default") - test("nonResource", audit.LevelNone, nil, "create") - test("nonResource", audit.LevelMetadata, nil, "tims") - test("nonResource", audit.LevelMetadata, nil, "humans") - test("nonResource", audit.LevelNone, nil, "serviceAccounts") - test("nonResource", audit.LevelNone, nil, "getPods") - test("nonResource", audit.LevelNone, nil, "getClusterRoles") - test("nonResource", audit.LevelRequestResponse, nil, "getLogs") - test("nonResource", audit.LevelNone, nil, "getMetrics") - test("nonResource", audit.LevelMetadata, nil, "getMetrics", "serviceAccounts", "default") - test("nonResource", audit.LevelRequestResponse, nil, "getLogs", "getClusterRoles", "default") - test("nonResource", audit.LevelNone, nil, "getPodLogs", "getPods") - test("nonResource", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived}, "omit RequestReceived", "getPods", "default") - test("nonResource", audit.LevelRequest, []audit.Stage{audit.StageRequestReceived, audit.StageResponseStarted, audit.StageResponseComplete}, "only audit panic", "getPods", "default") - - test("subresource", audit.LevelRequest, nil, "getPodLogs", "getPods") - test("subresource", audit.LevelRequest, nil, "getPods", "getPodLogs") + for _, tc := range testCases { + result := unionStages(tc.s1, tc.s2) + assert.Len(t, result, len(tc.exp)) + for _, expStage := range tc.exp { + ok := false + for _, resultStage := range result { + if resultStage == expStage { + ok = true + break + } + } + assert.True(t, ok) + } + } }