From 47ba91450fbe7d9002bfc9d4a48a73256252821f Mon Sep 17 00:00:00 2001 From: Cao Shufeng Date: Mon, 4 Sep 2017 16:53:19 +0800 Subject: [PATCH] Provide a way to omit Event stages in audit policy Updates https://github.com/kubernetes/kubernetes/issues/48561 This provide a way to omit some stages for each audit policy rule. For example: apiVersion: audit.k8s.io/v1beta1 kind: Policy - level: Metadata resources: - group: "rbac.authorization.k8s.io" resources: ["roles"] omitStages: - "RequestReceived" RequestReceived stage will not be emitted to audit backends with previous config. --- .../k8s.io/apiserver/pkg/apis/audit/types.go | 4 + .../pkg/apis/audit/v1alpha1/types.go | 4 + .../apiserver/pkg/apis/audit/v1beta1/types.go | 4 + .../pkg/apis/audit/validation/validation.go | 25 ++++ .../apis/audit/validation/validation_test.go | 11 ++ .../apiserver/pkg/audit/policy/checker.go | 17 +-- .../pkg/audit/policy/checker_test.go | 105 +++++++++------ .../apiserver/pkg/endpoints/apiserver_test.go | 2 +- .../apiserver/pkg/endpoints/filters/audit.go | 47 ++++--- .../pkg/endpoints/filters/audit_test.go | 122 ++++++++++++++++-- .../pkg/endpoints/filters/authn_audit.go | 4 +- .../pkg/endpoints/filters/authn_audit_test.go | 23 +++- 12 files changed, 278 insertions(+), 90 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 f9d37e54caa..7b5f6d81679 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go @@ -207,6 +207,10 @@ type PolicyRule struct { // "/healthz*" - Log all health checks // +optional NonResourceURLs []string + + // OmitStages specify events generated in which stages will not be emitted to backend. + // An empty list means no restrictions will apply. + OmitStages []Stage } // GroupResources represents resource kinds in an API group. 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 9eaaa111136..768a515b0cd 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 @@ -208,6 +208,10 @@ type PolicyRule struct { // "/healthz*" - Log all health checks // +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. + // An empty list means no restrictions will apply. + OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,8,rep,name=omitStages"` } // GroupResources represents resource kinds in an API group. 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 87a95a85efb..be42edffcd5 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 @@ -201,6 +201,10 @@ type PolicyRule struct { // "/healthz*" - Log all health checks // +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. + // An empty list means no restrictions will apply. + OmitStages []Stage `json:"omitStages,omitempty" protobuf:"bytes,8,rep,name=omitStages"` } // GroupResources represents resource kinds in an API group. 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 0db2030433a..6520a763948 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 @@ -38,6 +38,7 @@ func validatePolicyRule(rule audit.PolicyRule, fldPath *field.Path) field.ErrorL allErrs = append(allErrs, validateLevel(rule.Level, fldPath.Child("level"))...) allErrs = append(allErrs, validateNonResourceURLs(rule.NonResourceURLs, fldPath.Child("nonResourceURLs"))...) allErrs = append(allErrs, validateResources(rule.Resources, fldPath.Child("resources"))...) + allErrs = append(allErrs, validateOmitStages(rule.OmitStages, fldPath.Child("omitStages"))...) if len(rule.NonResourceURLs) > 0 { if len(rule.Resources) > 0 || len(rule.Namespaces) > 0 { @@ -55,6 +56,13 @@ var validLevels = []string{ string(audit.LevelRequestResponse), } +var validOmitStages = []string{ + string(audit.StageRequestReceived), + string(audit.StageResponseStarted), + string(audit.StageResponseComplete), + string(audit.StagePanic), +} + func validateLevel(level audit.Level, fldPath *field.Path) field.ErrorList { switch level { case audit.LevelNone, audit.LevelMetadata, audit.LevelRequest, audit.LevelRequestResponse: @@ -104,3 +112,20 @@ func validateResources(groupResources []audit.GroupResources, fldPath *field.Pat } return allErrs } + +func validateOmitStages(omitStages []audit.Stage, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for i, stage := range omitStages { + valid := false + for _, validOmitStage := range validOmitStages { + if string(stage) == validOmitStage { + valid = true + break + } + } + if !valid { + allErrs = append(allErrs, field.Invalid(fldPath.Index(i), string(stage), "allowed stages are "+strings.Join(validOmitStages, ","))) + } + } + return allErrs +} 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 3acb9598bd0..53d60782a40 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 @@ -43,6 +43,11 @@ func TestValidatePolicy(t *testing.T) { "/metrics", "*", }, + }, { // Omit RequestReceived stage + Level: audit.LevelMetadata, + OmitStages: []audit.Stage{ + audit.Stage("RequestReceived"), + }, }, } successCases := []audit.Policy{} @@ -108,6 +113,12 @@ func TestValidatePolicy(t *testing.T) { Resources: []audit.GroupResources{{ResourceNames: []string{"leader"}}}, Namespaces: []string{"kube-system"}, }, + { // invalid omitStages + Level: audit.LevelMetadata, + OmitStages: []audit.Stage{ + audit.Stage("foo"), + }, + }, } errorCases := []audit.Policy{} for _, rule := range invalidRules { 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 6896ab7c579..b92ffe26b68 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -31,7 +31,7 @@ const ( // Checker exposes methods for checking the policy rules. type Checker interface { // Check the audit level for a request with the given authorizer attributes. - Level(authorizer.Attributes) audit.Level + LevelAndStages(authorizer.Attributes) (audit.Level, []audit.Stage) } // NewChecker creates a new policy checker. @@ -40,21 +40,21 @@ func NewChecker(policy *audit.Policy) Checker { } // FakeChecker creates a checker that returns a constant level for all requests (for testing). -func FakeChecker(level audit.Level) Checker { - return &fakeChecker{level} +func FakeChecker(level audit.Level, stage []audit.Stage) Checker { + return &fakeChecker{level, stage} } type policyChecker struct { audit.Policy } -func (p *policyChecker) Level(attrs authorizer.Attributes) audit.Level { +func (p *policyChecker) LevelAndStages(attrs authorizer.Attributes) (audit.Level, []audit.Stage) { for _, rule := range p.Rules { if ruleMatches(&rule, attrs) { - return rule.Level + return rule.Level, rule.OmitStages } } - return DefaultAuditLevel + return DefaultAuditLevel, nil } // Check whether the rule matches the request attrs. @@ -181,8 +181,9 @@ func hasString(slice []string, value string) bool { type fakeChecker struct { level audit.Level + stage []audit.Stage } -func (f *fakeChecker) Level(_ authorizer.Attributes) audit.Level { - return f.level +func (f *fakeChecker) LevelAndStages(_ authorizer.Attributes) (audit.Level, []audit.Stage) { + return f.level, f.stage } 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 015e23beb7e..d6cc5f09171 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 @@ -136,59 +136,80 @@ func TestChecker(t *testing.T) { ResourceNames: []string{"edit"}, }}, }, + "omit RequestReceived": { + Level: audit.LevelRequest, + OmitStages: []audit.Stage{ + audit.StageRequestReceived, + }, + }, + "only audit panic": { + Level: audit.LevelRequest, + OmitStages: []audit.Stage{ + audit.StageRequestReceived, + audit.StageResponseStarted, + audit.StageResponseComplete, + }, + }, } - test := func(req string, expected audit.Level, ruleNames ...string) { + 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]) } require.Contains(t, attrs, req) - actual := NewChecker(&policy).Level(attrs[req]) - assert.Equal(t, expected, actual, "request:%s rules:%s", req, strings.Join(ruleNames, ",")) + 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, ",")) } - test("namespaced", audit.LevelMetadata, "default") - test("namespaced", audit.LevelNone, "create") - test("namespaced", audit.LevelMetadata, "tims") - test("namespaced", audit.LevelMetadata, "humans") - test("namespaced", audit.LevelNone, "serviceAccounts") - test("namespaced", audit.LevelRequestResponse, "getPods") - test("namespaced", audit.LevelNone, "getClusterRoles") - test("namespaced", audit.LevelNone, "getLogs") - test("namespaced", audit.LevelNone, "getMetrics") - test("namespaced", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") - test("namespaced", audit.LevelRequestResponse, "getMetrics", "getPods", "default") - test("namespaced", audit.LevelRequestResponse, "getPodLogs", "getPods") + 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, "default") - test("cluster", audit.LevelNone, "create") - test("cluster", audit.LevelMetadata, "tims") - test("cluster", audit.LevelMetadata, "humans") - test("cluster", audit.LevelNone, "serviceAccounts") - test("cluster", audit.LevelNone, "getPods") - test("cluster", audit.LevelRequestResponse, "getClusterRoles") - test("cluster", audit.LevelRequest, "clusterRoleEdit", "getClusterRoles") - test("cluster", audit.LevelNone, "getLogs") - test("cluster", audit.LevelNone, "getMetrics") - test("cluster", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") - test("cluster", audit.LevelRequestResponse, "getMetrics", "getClusterRoles", "default") - test("cluster", audit.LevelNone, "getPodLogs", "getPods") + 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, "default") - test("nonResource", audit.LevelNone, "create") - test("nonResource", audit.LevelMetadata, "tims") - test("nonResource", audit.LevelMetadata, "humans") - test("nonResource", audit.LevelNone, "serviceAccounts") - test("nonResource", audit.LevelNone, "getPods") - test("nonResource", audit.LevelNone, "getClusterRoles") - test("nonResource", audit.LevelRequestResponse, "getLogs") - test("nonResource", audit.LevelNone, "getMetrics") - test("nonResource", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") - test("nonResource", audit.LevelRequestResponse, "getLogs", "getClusterRoles", "default") - test("nonResource", audit.LevelNone, "getPodLogs", "getPods") + 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, "getPodLogs", "getPods") - test("subresource", audit.LevelRequest, "getPods", "getPodLogs") + test("subresource", audit.LevelRequest, nil, "getPodLogs", "getPods") + test("subresource", audit.LevelRequest, nil, "getPods", "getPodLogs") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 4847a593408..ee553869147 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -331,7 +331,7 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission. } } - handler := genericapifilters.WithAudit(mux, requestContextMapper, auditSink, auditpolicy.FakeChecker(auditinternal.LevelRequestResponse), func(r *http.Request, requestInfo *request.RequestInfo) bool { + handler := genericapifilters.WithAudit(mux, requestContextMapper, auditSink, auditpolicy.FakeChecker(auditinternal.LevelRequestResponse, nil), func(r *http.Request, requestInfo *request.RequestInfo) bool { // simplified long-running check return requestInfo.Verb == "watch" || requestInfo.Verb == "proxy" }) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go index 39813ea98cd..78849c66bb8 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go @@ -42,7 +42,7 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext return handler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx, ev, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) + ctx, ev, omitStages, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) if err != nil { utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err)) responsewriters.InternalError(w, req, errors.New("failed to create audit event")) @@ -54,7 +54,7 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext } ev.Stage = auditinternal.StageRequestReceived - processAuditEvent(sink, ev) + processAuditEvent(sink, ev, omitStages) // intercept the status code var longRunningSink audit.Sink @@ -64,7 +64,7 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext longRunningSink = sink } } - respWriter := decorateResponseWriter(w, ev, longRunningSink) + respWriter := decorateResponseWriter(w, ev, longRunningSink, omitStages) // send audit event when we leave this func, either via a panic or cleanly. In the case of long // running requests, this will be the second audit event. @@ -78,7 +78,7 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext Reason: metav1.StatusReasonInternalError, Message: fmt.Sprintf("APIServer panic'd: %v", r), } - processAuditEvent(sink, ev) + processAuditEvent(sink, ev, omitStages) return } @@ -92,14 +92,14 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext if ev.ResponseStatus == nil && longRunningSink != nil { ev.ResponseStatus = fakedSuccessStatus ev.Stage = auditinternal.StageResponseStarted - processAuditEvent(longRunningSink, ev) + processAuditEvent(longRunningSink, ev, omitStages) } ev.Stage = auditinternal.StageResponseComplete if ev.ResponseStatus == nil { ev.ResponseStatus = fakedSuccessStatus } - processAuditEvent(sink, ev) + processAuditEvent(sink, ev, omitStages) }() handler.ServeHTTP(respWriter, req) }) @@ -110,47 +110,53 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext // - context with audit event attached to it // - created audit event // - error if anything bad happened -func createAuditEventAndAttachToContext(requestContextMapper request.RequestContextMapper, req *http.Request, policy policy.Checker) (request.Context, *auditinternal.Event, error) { +func createAuditEventAndAttachToContext(requestContextMapper request.RequestContextMapper, req *http.Request, policy policy.Checker) (request.Context, *auditinternal.Event, []auditinternal.Stage, error) { ctx, ok := requestContextMapper.Get(req) if !ok { - return nil, nil, fmt.Errorf("no context found for request") + return nil, nil, nil, fmt.Errorf("no context found for request") } attribs, err := GetAuthorizerAttributes(ctx) if err != nil { - return nil, nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err) + return nil, nil, nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err) } - level := policy.Level(attribs) + level, omitStages := policy.LevelAndStages(attribs) audit.ObservePolicyLevel(level) if level == auditinternal.LevelNone { // Don't audit. - return nil, nil, nil + return nil, nil, nil, nil } ev, err := audit.NewEventFromRequest(req, level, attribs) if err != nil { - return nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err) + return nil, nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err) } ctx = request.WithAuditEvent(ctx, ev) if err := requestContextMapper.Update(req, ctx); err != nil { - return nil, nil, fmt.Errorf("failed to attach audit event to context: %v", err) + return nil, nil, nil, fmt.Errorf("failed to attach audit event to context: %v", err) } - return ctx, ev, nil + return ctx, ev, omitStages, nil } -func processAuditEvent(sink audit.Sink, ev *auditinternal.Event) { +func processAuditEvent(sink audit.Sink, ev *auditinternal.Event, omitStages []auditinternal.Stage) { + for _, stage := range omitStages { + if ev.Stage == stage { + return + } + } audit.ObserveEvent() sink.ProcessEvents(ev) } -func decorateResponseWriter(responseWriter http.ResponseWriter, ev *auditinternal.Event, sink audit.Sink) http.ResponseWriter { +func decorateResponseWriter(responseWriter http.ResponseWriter, ev *auditinternal.Event, sink audit.Sink, omitStages []auditinternal.Stage) http.ResponseWriter { delegate := &auditResponseWriter{ ResponseWriter: responseWriter, event: ev, sink: sink, + omitStages: omitStages, } // check if the ResponseWriter we're wrapping is the fancy one we need @@ -170,9 +176,10 @@ var _ http.ResponseWriter = &auditResponseWriter{} // create immediately an event (for long running requests). type auditResponseWriter struct { http.ResponseWriter - event *auditinternal.Event - once sync.Once - sink audit.Sink + event *auditinternal.Event + once sync.Once + sink audit.Sink + omitStages []auditinternal.Stage } func (a *auditResponseWriter) setHttpHeader() { @@ -188,7 +195,7 @@ func (a *auditResponseWriter) processCode(code int) { a.event.Stage = auditinternal.StageResponseStarted if a.sink != nil { - processAuditEvent(a.sink, a.event) + processAuditEvent(a.sink, a.event, a.omitStages) } }) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit_test.go index 852fa916fd9..bfac84f8fa4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit_test.go @@ -98,14 +98,14 @@ func (*fancyResponseWriter) Flush() {} func (*fancyResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { return nil, nil, nil } func TestConstructResponseWriter(t *testing.T) { - actual := decorateResponseWriter(&simpleResponseWriter{}, nil, nil) + actual := decorateResponseWriter(&simpleResponseWriter{}, nil, nil, nil) switch v := actual.(type) { case *auditResponseWriter: default: t.Errorf("Expected auditResponseWriter, got %v", reflect.TypeOf(v)) } - actual = decorateResponseWriter(&fancyResponseWriter{}, nil, nil) + actual = decorateResponseWriter(&fancyResponseWriter{}, nil, nil, nil) switch v := actual.(type) { case *fancyResponseWriterDelegator: default: @@ -115,7 +115,7 @@ func TestConstructResponseWriter(t *testing.T) { func TestDecorateResponseWriterWithoutChannel(t *testing.T) { ev := &auditinternal.Event{} - actual := decorateResponseWriter(&simpleResponseWriter{}, ev, nil) + actual := decorateResponseWriter(&simpleResponseWriter{}, ev, nil, nil) // write status. This will not block because firstEventSentCh is nil actual.WriteHeader(42) @@ -129,7 +129,7 @@ func TestDecorateResponseWriterWithoutChannel(t *testing.T) { func TestDecorateResponseWriterWithImplicitWrite(t *testing.T) { ev := &auditinternal.Event{} - actual := decorateResponseWriter(&simpleResponseWriter{}, ev, nil) + actual := decorateResponseWriter(&simpleResponseWriter{}, ev, nil, nil) // write status. This will not block because firstEventSentCh is nil actual.Write([]byte("foo")) @@ -144,7 +144,7 @@ func TestDecorateResponseWriterWithImplicitWrite(t *testing.T) { func TestDecorateResponseWriterChannel(t *testing.T) { sink := &fakeAuditSink{} ev := &auditinternal.Event{} - actual := decorateResponseWriter(&simpleResponseWriter{}, ev, sink) + actual := decorateResponseWriter(&simpleResponseWriter{}, ev, sink, nil) done := make(chan struct{}) go func() { @@ -203,17 +203,19 @@ func TestAuditLegacy(t *testing.T) { delay := 500 * time.Millisecond for _, test := range []struct { - desc string - path string - verb string - handler func(http.ResponseWriter, *http.Request) - expected []string + desc string + path string + verb string + omitStages []auditinternal.Stage + handler func(http.ResponseWriter, *http.Request) + expected []string }{ // short running requests with read-only verb { "read-only empty", shortRunningPath, "GET", + nil, func(http.ResponseWriter, *http.Request) {}, []string{ readOnlyShortRunningPrefix(auditinternal.StageRequestReceived) + ` response=""`, @@ -224,6 +226,7 @@ func TestAuditLegacy(t *testing.T) { "read-only panic", shortRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -238,6 +241,7 @@ func TestAuditLegacy(t *testing.T) { "writing empty", shortRunningPath, "PUT", + nil, func(http.ResponseWriter, *http.Request) {}, []string{ writingShortRunningPrefix(auditinternal.StageRequestReceived) + ` response=""`, @@ -248,6 +252,7 @@ func TestAuditLegacy(t *testing.T) { "writing sleep", shortRunningPath, "PUT", + nil, func(http.ResponseWriter, *http.Request) { time.Sleep(delay) }, @@ -260,6 +265,7 @@ func TestAuditLegacy(t *testing.T) { "writing 403+write", shortRunningPath, "PUT", + nil, func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(403) w.Write([]byte("foo")) @@ -273,6 +279,7 @@ func TestAuditLegacy(t *testing.T) { "writing panic", shortRunningPath, "PUT", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -285,6 +292,7 @@ func TestAuditLegacy(t *testing.T) { "writing write+panic", shortRunningPath, "PUT", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) panic("kaboom") @@ -300,6 +308,7 @@ func TestAuditLegacy(t *testing.T) { "empty longrunning", longRunningPath, "GET", + nil, func(http.ResponseWriter, *http.Request) {}, []string{ longRunningPrefix(auditinternal.StageRequestReceived) + ` response=""`, @@ -311,6 +320,7 @@ func TestAuditLegacy(t *testing.T) { "sleep longrunning", longRunningPath, "GET", + nil, func(http.ResponseWriter, *http.Request) { time.Sleep(delay) }, @@ -324,6 +334,7 @@ func TestAuditLegacy(t *testing.T) { "sleep+403 longrunning", longRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { time.Sleep(delay) w.WriteHeader(403) @@ -338,6 +349,7 @@ func TestAuditLegacy(t *testing.T) { "write longrunning", longRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) }, @@ -351,6 +363,7 @@ func TestAuditLegacy(t *testing.T) { "403+write longrunning", longRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(403) w.Write([]byte("foo")) @@ -365,6 +378,7 @@ func TestAuditLegacy(t *testing.T) { "panic longrunning", longRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -377,6 +391,7 @@ func TestAuditLegacy(t *testing.T) { "write+panic longrunning", longRunningPath, "GET", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) panic("kaboom") @@ -387,10 +402,33 @@ func TestAuditLegacy(t *testing.T) { longRunningPrefix(auditinternal.StagePanic) + ` response="500"`, }, }, + { + "omit RequestReceived", + shortRunningPath, + "GET", + []auditinternal.Stage{auditinternal.StageRequestReceived}, + func(http.ResponseWriter, *http.Request) {}, + []string{ + readOnlyShortRunningPrefix(auditinternal.StageResponseComplete) + ` response="200"`, + }, + }, + { + "emit painc only", + longRunningPath, + "GET", + []auditinternal.Stage{auditinternal.StageRequestReceived, auditinternal.StageResponseStarted, auditinternal.StageResponseComplete}, + func(w http.ResponseWriter, req *http.Request) { + w.Write([]byte("foo")) + panic("kaboom") + }, + []string{ + longRunningPrefix(auditinternal.StagePanic) + ` response="500"`, + }, + }, } { var buf bytes.Buffer backend := pluginlog.NewBackend(&buf, pluginlog.FormatLegacy, auditv1beta1.SchemeGroupVersion) - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, test.omitStages) handler := WithAudit(http.HandlerFunc(test.handler), &fakeRequestContextMapper{ user: &user.DefaultInfo{Name: "admin"}, }, backend, policyChecker, func(r *http.Request, ri *request.RequestInfo) bool { @@ -440,6 +478,7 @@ func TestAuditJson(t *testing.T) { path string verb string auditID string + omitStages []auditinternal.Stage handler func(http.ResponseWriter, *http.Request) expected []auditv1beta1.Event respHeader bool @@ -450,6 +489,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "GET", "", + nil, func(http.ResponseWriter, *http.Request) {}, []auditv1beta1.Event{ { @@ -471,6 +511,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "GET", uuid.NewRandom().String(), + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) }, @@ -494,6 +535,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -518,6 +560,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "PUT", "", + nil, func(http.ResponseWriter, *http.Request) {}, []auditv1beta1.Event{ { @@ -539,6 +582,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "PUT", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) time.Sleep(delay) @@ -563,6 +607,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "PUT", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(403) w.Write([]byte("foo")) @@ -587,6 +632,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "PUT", "", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -610,6 +656,7 @@ func TestAuditJson(t *testing.T) { shortRunningPath, "PUT", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) panic("kaboom") @@ -635,6 +682,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(http.ResponseWriter, *http.Request) {}, []auditv1beta1.Event{ { @@ -662,6 +710,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", uuid.NewRandom().String(), + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) }, @@ -691,6 +740,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(http.ResponseWriter, *http.Request) { time.Sleep(delay) }, @@ -720,6 +770,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { time.Sleep(delay) w.WriteHeader(403) @@ -750,6 +801,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) }, @@ -779,6 +831,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(403) w.Write([]byte("foo")) @@ -809,6 +862,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { panic("kaboom") }, @@ -832,6 +886,7 @@ func TestAuditJson(t *testing.T) { longRunningPath, "GET", "", + nil, func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("foo")) panic("kaboom") @@ -857,10 +912,49 @@ func TestAuditJson(t *testing.T) { }, true, }, + { + "omit RequestReceived", + shortRunningPath, + "GET", + "", + []auditinternal.Stage{auditinternal.StageRequestReceived}, + func(w http.ResponseWriter, req *http.Request) { + w.Write([]byte("foo")) + }, + []auditv1beta1.Event{ + { + Stage: auditinternal.StageResponseComplete, + Verb: "get", + RequestURI: shortRunningPath, + ResponseStatus: &metav1.Status{Code: 200}, + }, + }, + true, + }, + { + "emit Panic only", + longRunningPath, + "GET", + "", + []auditinternal.Stage{auditinternal.StageRequestReceived, auditinternal.StageResponseStarted, auditinternal.StageResponseComplete}, + func(w http.ResponseWriter, req *http.Request) { + w.Write([]byte("foo")) + panic("kaboom") + }, + []auditv1beta1.Event{ + { + Stage: auditinternal.StagePanic, + Verb: "watch", + RequestURI: longRunningPath, + ResponseStatus: &metav1.Status{Code: 500}, + }, + }, + true, + }, } { var buf bytes.Buffer backend := pluginlog.NewBackend(&buf, pluginlog.FormatJson, auditv1beta1.SchemeGroupVersion) - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, test.omitStages) handler := WithAudit(http.HandlerFunc(test.handler), &fakeRequestContextMapper{ user: &user.DefaultInfo{Name: "admin"}, }, backend, policyChecker, func(r *http.Request, ri *request.RequestInfo) bool { @@ -964,7 +1058,7 @@ func (*fakeRequestContextMapper) Update(req *http.Request, context request.Conte } func TestAuditNoPanicOnNilUser(t *testing.T) { - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, nil) handler := WithAudit(&fakeHTTPHandler{}, &fakeRequestContextMapper{}, &fakeAuditSink{}, policyChecker, nil) req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) req.RemoteAddr = "127.0.0.1" @@ -977,7 +1071,7 @@ func TestAuditLevelNone(t *testing.T) { handler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(200) }) - policyChecker := policy.FakeChecker(auditinternal.LevelNone) + policyChecker := policy.FakeChecker(auditinternal.LevelNone, nil) handler = WithAudit(handler, &fakeRequestContextMapper{ user: &user.DefaultInfo{Name: "admin"}, }, sink, policyChecker, nil) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go index a3c192f79fa..86aca9872c7 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go @@ -38,7 +38,7 @@ func WithFailedAuthenticationAudit(failedHandler http.Handler, requestContextMap return failedHandler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - _, ev, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) + _, ev, omitStages, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) if err != nil { utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err)) responsewriters.InternalError(w, req, errors.New("failed to create audit event")) @@ -53,7 +53,7 @@ func WithFailedAuthenticationAudit(failedHandler http.Handler, requestContextMap ev.ResponseStatus.Message = getAuthMethods(req) ev.Stage = auditinternal.StageResponseStarted - rw := decorateResponseWriter(w, ev, sink) + rw := decorateResponseWriter(w, ev, sink, omitStages) failedHandler.ServeHTTP(rw, req) }) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go index fb9eeebf6b3..a320a977d89 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go @@ -30,7 +30,7 @@ import ( func TestFailedAuthnAudit(t *testing.T) { sink := &fakeAuditSink{} - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, nil) handler := WithFailedAuthenticationAudit( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) @@ -61,7 +61,7 @@ func TestFailedAuthnAudit(t *testing.T) { func TestFailedMultipleAuthnAudit(t *testing.T) { sink := &fakeAuditSink{} - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, nil) handler := WithFailedAuthenticationAudit( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) @@ -93,7 +93,7 @@ func TestFailedMultipleAuthnAudit(t *testing.T) { func TestFailedAuthnAuditWithoutAuthorization(t *testing.T) { sink := &fakeAuditSink{} - policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, nil) handler := WithFailedAuthenticationAudit( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) @@ -120,3 +120,20 @@ func TestFailedAuthnAuditWithoutAuthorization(t *testing.T) { t.Errorf("Unexpected user, expected /api/v1/namespaces/default/pods, got %s", ev.RequestURI) } } + +func TestFailedAuthnAuditOmitted(t *testing.T) { + sink := &fakeAuditSink{} + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse, []auditinternal.Stage{auditinternal.StageResponseStarted}) + handler := WithFailedAuthenticationAudit( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }), + &fakeRequestContextMapper{}, sink, policyChecker) + req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) + req.RemoteAddr = "127.0.0.1" + handler.ServeHTTP(httptest.NewRecorder(), req) + + if len(sink.events) != 0 { + t.Fatalf("Unexpected number of audit events generated, expected 0, got: %d", len(sink.events)) + } +}