From 8be823b0b0270e1b979b3d4c6e683e1daa0f2e01 Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Mon, 20 Sep 2021 17:43:16 -0400 Subject: [PATCH 1/2] apiserver: store (event, evaluated policy) pair in request context --- .../src/k8s.io/apiserver/pkg/audit/context.go | 34 ++++++++++++++++++- .../k8s.io/apiserver/pkg/audit/evaluator.go | 17 ++++++++++ .../token/cache/cached_token_authenticator.go | 3 +- .../cache/cached_token_authenticator_test.go | 7 ++-- .../apiserver/pkg/endpoints/filters/audit.go | 5 ++- .../pkg/endpoints/filters/audit_test.go | 9 +++-- .../pkg/endpoints/filters/authorization.go | 2 +- .../pkg/endpoints/filters/impersonation.go | 2 +- .../filters/request_deadline_test.go | 3 +- .../pkg/endpoints/handlers/create.go | 2 +- .../pkg/endpoints/handlers/delete.go | 8 ++--- .../apiserver/pkg/endpoints/handlers/patch.go | 2 +- .../handlers/responsewriters/writers.go | 2 +- .../apiserver/pkg/endpoints/handlers/rest.go | 3 +- .../pkg/endpoints/handlers/update.go | 2 +- .../pkg/endpoints/request/context.go | 15 -------- .../apiserver/pkg/server/config_test.go | 4 +-- 17 files changed, 81 insertions(+), 39 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/audit/context.go b/staging/src/k8s.io/apiserver/pkg/audit/context.go index 3d616bbd4cc..eada7add9a8 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/context.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/context.go @@ -19,6 +19,7 @@ package audit import ( "context" + auditinternal "k8s.io/apiserver/pkg/apis/audit" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" ) @@ -27,7 +28,15 @@ type key int const ( // auditAnnotationsKey is the context key for the audit annotations. + // TODO: it's wasteful to store the audit annotations under a separate key, we + // copy the request context twice for audit purposes. We should move the audit + // annotations under AuditContext so we can get rid of the additional request + // context copy. auditAnnotationsKey key = iota + + // auditKey is the context key for storing the audit event that is being + // captured and the evaluated policy that applies to the given request. + auditKey ) // annotations = *[]annotation instead of a map to preserve order of insertions @@ -59,7 +68,7 @@ func WithAuditAnnotations(parent context.Context) context.Context { // prefer AddAuditAnnotation over LogAnnotation to avoid dropping annotations. func AddAuditAnnotation(ctx context.Context, key, value string) { // use the audit event directly if we have it - if ae := genericapirequest.AuditEventFrom(ctx); ae != nil { + if ae := AuditEventFrom(ctx); ae != nil { LogAnnotation(ae, key, value) return } @@ -82,3 +91,26 @@ func auditAnnotationsFrom(ctx context.Context) []annotation { return *annotations } + +// WithAuditContext returns a new context that stores the pair of the audit +// configuration object that applies to the given request and +// the audit event that is going to be written to the API audit log. +func WithAuditContext(parent context.Context, ev *AuditContext) context.Context { + return genericapirequest.WithValue(parent, auditKey, ev) +} + +// AuditEventFrom returns the audit event struct on the ctx +func AuditEventFrom(ctx context.Context) *auditinternal.Event { + if o := AuditContextFrom(ctx); o != nil { + return o.Event + } + return nil +} + +// AuditContextFrom returns the pair of the audit configuration object +// that applies to the given request and the audit event that is going to +// be written to the API audit log. +func AuditContextFrom(ctx context.Context) *AuditContext { + ev, _ := ctx.Value(auditKey).(*AuditContext) + return ev +} diff --git a/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go b/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go index 4319e050dd0..cc82beb83f8 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go @@ -21,6 +21,23 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" ) +// AuditContext is a pair of the audit configuration object that applies to +// a given request and the audit Event object that is being captured. +// It's a convenient placeholder to store both these objects in the request context. +type AuditContext struct { + // RequestAuditConfig is the audit configuration that applies to the request + RequestAuditConfig RequestAuditConfig + + // Event is the audit Event object that is being captured to be written in + // the API audit log. It is set to nil when the request is not being audited. + Event *audit.Event +} + +// RequestAuditConfig is the evaluated audit configuration that is applicable to +// a given request. PolicyRuleEvaluator evaluates the audit policy against the +// authorizer attributes and returns a RequestAuditConfig that applies to the request. +type RequestAuditConfig struct{} + // PolicyRuleEvaluator exposes methods for evaluating the policy rules. type PolicyRuleEvaluator interface { // Check the audit level for a request with the given authorizer attributes. diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator.go b/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator.go index afd990f7184..787d926b4ed 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator.go @@ -36,7 +36,6 @@ import ( auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authentication/authenticator" - "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/klog/v2" "k8s.io/utils/clock" ) @@ -189,7 +188,7 @@ func (a *cachedTokenAuthenticator) doAuthenticateToken(ctx context.Context, toke // since this is shared work between multiple requests, we have no way of knowing if any // particular request supports audit annotations. thus we always attempt to record them. ev := &auditinternal.Event{Level: auditinternal.LevelMetadata} - ctx = request.WithAuditEvent(ctx, ev) + ctx = audit.WithAuditContext(ctx, &audit.AuditContext{Event: ev}) record.resp, record.ok, record.err = a.authenticator.AuthenticateToken(ctx, token) record.annotations = ev.Annotations diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator_test.go b/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator_test.go index 80ef78ec9d1..69379babab8 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/token/cache/cached_token_authenticator_test.go @@ -36,7 +36,6 @@ import ( "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/utils/clock" testingclock "k8s.io/utils/clock/testing" ) @@ -309,7 +308,9 @@ func TestCachedAuditAnnotations(t *testing.T) { if randomChoice { ctx = audit.WithAuditAnnotations(ctx) } else { - ctx = request.WithAuditEvent(ctx, &auditinternal.Event{Level: auditinternal.LevelMetadata}) + ctx = audit.WithAuditContext(ctx, &audit.AuditContext{ + Event: &auditinternal.Event{Level: auditinternal.LevelMetadata}, + }) } _, _, _ = a.AuthenticateToken(ctx, "token") @@ -317,7 +318,7 @@ func TestCachedAuditAnnotations(t *testing.T) { if randomChoice { allAnnotations <- extractAnnotations(ctx) } else { - allAnnotations <- request.AuditEventFrom(ctx).Annotations + allAnnotations <- audit.AuditEventFrom(ctx).Annotations } }() } 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 ecee6600f19..a15a6d9963f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go @@ -140,7 +140,10 @@ func createAuditEventAndAttachToContext(req *http.Request, policy audit.PolicyRu return req, nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err) } - req = req.WithContext(request.WithAuditEvent(ctx, ev)) + req = req.WithContext(audit.WithAuditContext(ctx, &audit.AuditContext{ + RequestAuditConfig: audit.RequestAuditConfig{}, + Event: ev, + })) return req, ev, omitStages, nil } 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 ea7e940a0b7..7c622e9780f 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 @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/audit/policy" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/request" @@ -839,13 +840,15 @@ func TestAuditIDHttpHeader(t *testing.T) { } } -func withTestContext(req *http.Request, user user.Info, audit *auditinternal.Event) *http.Request { +func withTestContext(req *http.Request, user user.Info, ae *auditinternal.Event) *http.Request { ctx := req.Context() if user != nil { ctx = request.WithUser(ctx, user) } - if audit != nil { - ctx = request.WithAuditEvent(ctx, audit) + if ae != nil { + ctx = audit.WithAuditContext(ctx, &audit.AuditContext{ + Event: ae, + }) } if info, err := newTestRequestInfoResolver().NewRequestInfo(req); err == nil { ctx = request.WithRequestInfo(ctx, info) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go index 96834938124..395d9332a1f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go @@ -49,7 +49,7 @@ func WithAuthorization(handler http.Handler, a authorizer.Authorizer, s runtime. } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ctx := req.Context() - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) attributes, err := GetAuthorizerAttributes(ctx) if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go index d82ad68a7c2..1dc1fe4a505 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go @@ -166,7 +166,7 @@ func WithImpersonation(handler http.Handler, a authorizer.Authorizer, s runtime. oldUser, _ := request.UserFrom(ctx) httplog.LogOf(req, w).Addf("%v is acting as %v", oldUser, newUser) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) audit.LogImpersonatedUser(ae, newUser) // clear all the impersonation headers from the request diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline_test.go index 384c78092a5..7806e645c42 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline_test.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/audit/policy" "k8s.io/apiserver/pkg/endpoints/request" testingclock "k8s.io/utils/clock/testing" @@ -410,7 +411,7 @@ func TestWithFailedRequestAudit(t *testing.T) { t.Errorf("expected an http.ResponseWriter of type: %T but got: %T", &auditResponseWriter{}, rwGot) } - auditEventGot := request.AuditEventFrom(requestGot.Context()) + auditEventGot := audit.AuditEventFrom(requestGot.Context()) if auditEventGot == nil { t.Fatal("expected an audit event object but got nil") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index d5f7c414b06..e62a5a594bf 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -141,7 +141,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int } ctx = request.WithNamespace(ctx, namespace) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index 9b2b9b60a2c..5b9c36ecab2 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -67,7 +67,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc defer cancel() ctx = request.WithNamespace(ctx, namespace) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope) @@ -103,7 +103,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc } trace.Step("Decoded delete options") - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) objGV := gvk.GroupVersion() audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer) trace.Step("Recorded the audit event") @@ -189,7 +189,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc defer cancel() ctx = request.WithNamespace(ctx, namespace) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) outputMediaType, _, err := negotiation.NegotiateOutputMediaType(req, scope.Serializer, scope) if err != nil { @@ -250,7 +250,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc return } - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) objGV := gvk.GroupVersion() audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer) } else { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 1abcccaa170..1138bc0aee3 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -123,7 +123,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac } options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("PatchOptions")) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) audit.LogRequestPatch(ae, patchBytes) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go index 16e16c35376..676b32fc8dd 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go @@ -267,7 +267,7 @@ func WriteObjectNegotiated(s runtime.NegotiatedSerializer, restrictions negotiat return } - if ae := request.AuditEventFrom(req.Context()); ae != nil { + if ae := audit.AuditEventFrom(req.Context()); ae != nil { audit.LogResponseObject(ae, object, gv, s) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 4c2e63e10c0..ef43939be07 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -39,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" @@ -187,7 +188,7 @@ func ConnectResource(connecter rest.Connecter, scope *RequestScope, admit admiss } ctx := req.Context() ctx = request.WithNamespace(ctx, namespace) - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) opts, subpath, subpathKey := connecter.NewConnectOptions() diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index e0dbd54a54c..bfd8cb4b514 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -118,7 +118,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa } trace.Step("Conversion done") - ae := request.AuditEventFrom(ctx) + ae := audit.AuditEventFrom(ctx) audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer) admit = admission.WithAudit(admit, ae) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/request/context.go b/staging/src/k8s.io/apiserver/pkg/endpoints/request/context.go index 95166f5c474..8f4e60f5495 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/request/context.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/request/context.go @@ -20,7 +20,6 @@ import ( "context" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" ) @@ -33,9 +32,6 @@ const ( // userKey is the context key for the request user. userKey - - // auditKey is the context key for the audit event. - auditKey ) // NewContext instantiates a base context object for request flows. @@ -80,14 +76,3 @@ func UserFrom(ctx context.Context) (user.Info, bool) { user, ok := ctx.Value(userKey).(user.Info) return user, ok } - -// WithAuditEvent returns set audit event struct. -func WithAuditEvent(parent context.Context, ev *audit.Event) context.Context { - return WithValue(parent, auditKey, ev) -} - -// AuditEventFrom returns the audit event struct on the ctx -func AuditEventFrom(ctx context.Context) *audit.Event { - ev, _ := ctx.Value(auditKey).(*audit.Event) - return ev -} diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index b4be78f5d19..01c3cbafcb8 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -281,7 +281,7 @@ func TestAuthenticationAuditAnnotationsDefaultChain(t *testing.T) { audit.AddAuditAnnotation(req.Context(), "pandas", "are awesome") // confirm that trying to use the audit event directly would never work - if ae := request.AuditEventFrom(req.Context()); ae != nil { + if ae := audit.AuditEventFrom(req.Context()); ae != nil { t.Errorf("expected nil audit event, got %v", ae) } @@ -307,7 +307,7 @@ func TestAuthenticationAuditAnnotationsDefaultChain(t *testing.T) { } // confirm that we have an audit event - ae := request.AuditEventFrom(r.Context()) + ae := audit.AuditEventFrom(r.Context()) if ae == nil { t.Error("unexpected nil audit event") } From a748fdc6775c63b52a1a963e2332ac774890d2a9 Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Mon, 20 Sep 2021 17:44:11 -0400 Subject: [PATCH 2/2] apiserver: refactor PolicyRuleEvaluator to return a struct --- cluster/gce/gci/audit_policy_test.go | 8 ++-- .../k8s.io/apiserver/pkg/audit/evaluator.go | 22 +++++++-- .../apiserver/pkg/audit/policy/checker.go | 26 ++++++++--- .../pkg/audit/policy/checker_test.go | 8 ++-- .../apiserver/pkg/endpoints/filters/audit.go | 45 ++++++++++--------- .../pkg/endpoints/filters/authn_audit.go | 8 +++- .../pkg/endpoints/filters/request_deadline.go | 8 +++- test/integration/examples/webhook_test.go | 11 +++-- 8 files changed, 92 insertions(+), 44 deletions(-) diff --git a/cluster/gce/gci/audit_policy_test.go b/cluster/gce/gci/audit_policy_test.go index bd9a553f7ff..64765a1f41c 100644 --- a/cluster/gce/gci/audit_policy_test.go +++ b/cluster/gce/gci/audit_policy_test.go @@ -231,10 +231,10 @@ func (t *auditTester) expectLevel(expected audit.Level, attrs authorizer.Attribu name := fmt.Sprintf("%s.%s.%s", attrs.GetUser().GetName(), attrs.GetVerb(), obj) evaluator := t.evaluator t.Run(name, func(t *testing.T) { - level, stages := evaluator.LevelAndStages(attrs) - assert.Equal(t, expected, level) - if level != audit.LevelNone { - assert.ElementsMatch(t, stages, []audit.Stage{audit.StageRequestReceived}) + auditConfig := evaluator.EvaluatePolicyRule(attrs) + assert.Equal(t, expected, auditConfig.Level) + if auditConfig.Level != audit.LevelNone { + assert.ElementsMatch(t, auditConfig.OmitStages, []audit.Stage{audit.StageRequestReceived}) } }) } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go b/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go index cc82beb83f8..e8591106aa2 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/evaluator.go @@ -36,10 +36,26 @@ type AuditContext struct { // RequestAuditConfig is the evaluated audit configuration that is applicable to // a given request. PolicyRuleEvaluator evaluates the audit policy against the // authorizer attributes and returns a RequestAuditConfig that applies to the request. -type RequestAuditConfig struct{} +type RequestAuditConfig struct { + // OmitStages is the stages that need to be omitted from being audited. + OmitStages []audit.Stage +} + +// RequestAuditConfigWithLevel includes Level at which the request is being audited. +// PolicyRuleEvaluator evaluates the audit configuration for a request +// against the authorizer attributes and returns an RequestAuditConfigWithLevel +// that applies to the request. +type RequestAuditConfigWithLevel struct { + RequestAuditConfig + + // Level at which the request is being audited at + Level audit.Level +} // PolicyRuleEvaluator exposes methods for evaluating the policy rules. type PolicyRuleEvaluator interface { - // Check the audit level for a request with the given authorizer attributes. - LevelAndStages(authorizer.Attributes) (audit.Level, []audit.Stage) + // EvaluatePolicyRule evaluates the audit policy of the apiserver against + // the given authorizer attributes and returns the audit configuration that + // is applicable to the given equest. + EvaluatePolicyRule(authorizer.Attributes) RequestAuditConfigWithLevel } 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 26ce7825f05..dfad2977b6c 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -61,13 +61,24 @@ type policyRuleEvaluator struct { audit.Policy } -func (p *policyRuleEvaluator) LevelAndStages(attrs authorizer.Attributes) (audit.Level, []audit.Stage) { +func (p *policyRuleEvaluator) EvaluatePolicyRule(attrs authorizer.Attributes) auditinternal.RequestAuditConfigWithLevel { for _, rule := range p.Rules { if ruleMatches(&rule, attrs) { - return rule.Level, rule.OmitStages + return auditinternal.RequestAuditConfigWithLevel{ + Level: rule.Level, + RequestAuditConfig: auditinternal.RequestAuditConfig{ + OmitStages: rule.OmitStages, + }, + } } } - return DefaultAuditLevel, p.OmitStages + + return auditinternal.RequestAuditConfigWithLevel{ + Level: DefaultAuditLevel, + RequestAuditConfig: auditinternal.RequestAuditConfig{ + OmitStages: p.OmitStages, + }, + } } // Check whether the rule matches the request attrs. @@ -210,6 +221,11 @@ type fakePolicyRuleEvaluator struct { stage []audit.Stage } -func (f *fakePolicyRuleEvaluator) LevelAndStages(_ authorizer.Attributes) (audit.Level, []audit.Stage) { - return f.level, f.stage +func (f *fakePolicyRuleEvaluator) EvaluatePolicyRule(_ authorizer.Attributes) auditinternal.RequestAuditConfigWithLevel { + return auditinternal.RequestAuditConfigWithLevel{ + Level: f.level, + RequestAuditConfig: auditinternal.RequestAuditConfig{ + OmitStages: 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 b08a917c7aa..5a250c069c4 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 @@ -185,10 +185,10 @@ func test(t *testing.T, req string, expLevel audit.Level, policyStages, expOmitS policy.Rules = append(policy.Rules, rules[rule]) } require.Contains(t, attrs, req) - actualLevel, actualOmitStages := NewPolicyRuleEvaluator(&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) + auditConfig := NewPolicyRuleEvaluator(&policy).EvaluatePolicyRule(attrs[req]) + assert.Equal(t, expLevel, auditConfig.Level, "request:%s rules:%s", req, strings.Join(ruleNames, ",")) + assert.True(t, stageEqual(expOmitStages, auditConfig.OmitStages), "request:%s rules:%s, expected stages: %v, actual stages: %v", + req, strings.Join(ruleNames, ","), expOmitStages, auditConfig.OmitStages) } func testAuditLevel(t *testing.T, stages []audit.Stage) { 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 a15a6d9963f..fe32ad3632c 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go @@ -43,18 +43,24 @@ func WithAudit(handler http.Handler, sink audit.Sink, policy audit.PolicyRuleEva return handler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy) + auditContext, err := evaluatePolicyAndCreateAuditEvent(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")) return } - ctx := req.Context() - if ev == nil || ctx == nil { + + ev := auditContext.Event + if ev == nil || req.Context() == nil { handler.ServeHTTP(w, req) return } + req = req.WithContext(audit.WithAuditContext(req.Context(), auditContext)) + + ctx := req.Context() + omitStages := auditContext.RequestAuditConfig.OmitStages + ev.Stage = auditinternal.StageRequestReceived if processed := processAuditEvent(ctx, sink, ev, omitStages); !processed { audit.ApiserverAuditDroppedCounter.WithContext(ctx).Inc() @@ -111,41 +117,40 @@ func WithAudit(handler http.Handler, sink audit.Sink, policy audit.PolicyRuleEva }) } -// createAuditEventAndAttachToContext is responsible for creating the audit event -// and attaching it to the appropriate request context. It returns: -// - context with audit event attached to it -// - created audit event +// evaluatePolicyAndCreateAuditEvent is responsible for evaluating the audit +// policy configuration applicable to the request and create a new audit +// event that will be written to the API audit log. // - error if anything bad happened -func createAuditEventAndAttachToContext(req *http.Request, policy audit.PolicyRuleEvaluator) (*http.Request, *auditinternal.Event, []auditinternal.Stage, error) { +func evaluatePolicyAndCreateAuditEvent(req *http.Request, policy audit.PolicyRuleEvaluator) (*audit.AuditContext, error) { ctx := req.Context() attribs, err := GetAuthorizerAttributes(ctx) if err != nil { - return req, nil, nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err) + return nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err) } - level, omitStages := policy.LevelAndStages(attribs) - audit.ObservePolicyLevel(ctx, level) - if level == auditinternal.LevelNone { + ls := policy.EvaluatePolicyRule(attribs) + audit.ObservePolicyLevel(ctx, ls.Level) + if ls.Level == auditinternal.LevelNone { // Don't audit. - return req, nil, nil, nil + return &audit.AuditContext{ + RequestAuditConfig: ls.RequestAuditConfig, + }, nil } requestReceivedTimestamp, ok := request.ReceivedTimestampFrom(ctx) if !ok { requestReceivedTimestamp = time.Now() } - ev, err := audit.NewEventFromRequest(req, requestReceivedTimestamp, level, attribs) + ev, err := audit.NewEventFromRequest(req, requestReceivedTimestamp, ls.Level, attribs) if err != nil { - return req, nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err) + return nil, fmt.Errorf("failed to complete audit event from request: %v", err) } - req = req.WithContext(audit.WithAuditContext(ctx, &audit.AuditContext{ - RequestAuditConfig: audit.RequestAuditConfig{}, + return &audit.AuditContext{ + RequestAuditConfig: ls.RequestAuditConfig, Event: ev, - })) - - return req, ev, omitStages, nil + }, nil } func processAuditEvent(ctx context.Context, sink audit.Sink, ev *auditinternal.Event, omitStages []auditinternal.Stage) bool { 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 550a2f25b29..fc9340b10ed 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 @@ -36,22 +36,26 @@ func WithFailedAuthenticationAudit(failedHandler http.Handler, sink audit.Sink, return failedHandler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy) + a, err := evaluatePolicyAndCreateAuditEvent(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")) return } + + ev := a.Event if ev == nil { failedHandler.ServeHTTP(w, req) return } + req = req.WithContext(audit.WithAuditContext(req.Context(), a)) + ev.ResponseStatus = &metav1.Status{} ev.ResponseStatus.Message = getAuthMethods(req) ev.Stage = auditinternal.StageResponseStarted - rw := decorateResponseWriter(req.Context(), w, ev, sink, omitStages) + rw := decorateResponseWriter(req.Context(), w, ev, sink, a.RequestAuditConfig.OmitStages) failedHandler.ServeHTTP(rw, req) }) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go index 132ffab1cf6..84255d525ac 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go @@ -108,24 +108,28 @@ func withFailedRequestAudit(failedHandler http.Handler, statusErr *apierrors.Sta return failedHandler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy) + a, err := evaluatePolicyAndCreateAuditEvent(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")) return } + + ev := a.Event if ev == nil { failedHandler.ServeHTTP(w, req) return } + req = req.WithContext(audit.WithAuditContext(req.Context(), a)) + ev.ResponseStatus = &metav1.Status{} ev.Stage = auditinternal.StageResponseStarted if statusErr != nil { ev.ResponseStatus.Message = statusErr.Error() } - rw := decorateResponseWriter(req.Context(), w, ev, sink, omitStages) + rw := decorateResponseWriter(req.Context(), w, ev, sink, a.RequestAuditConfig.OmitStages) failedHandler.ServeHTTP(rw, req) }) } diff --git a/test/integration/examples/webhook_test.go b/test/integration/examples/webhook_test.go index 2592902d3d5..1e265986a16 100644 --- a/test/integration/examples/webhook_test.go +++ b/test/integration/examples/webhook_test.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/kubernetes/cmd/kube-apiserver/app/options" "k8s.io/kubernetes/pkg/controlplane" @@ -52,14 +53,16 @@ func TestWebhookLoopback(t *testing.T) { // Hook into audit to watch requests config.GenericConfig.AuditBackend = auditSinkFunc(func(events ...*auditinternal.Event) {}) - config.GenericConfig.AuditPolicyRuleEvaluator = auditPolicyRuleEvaluator(func(attrs authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage) { + config.GenericConfig.AuditPolicyRuleEvaluator = auditPolicyRuleEvaluator(func(attrs authorizer.Attributes) audit.RequestAuditConfigWithLevel { if attrs.GetPath() == webhookPath { if attrs.GetUser().GetName() != "system:apiserver" { t.Errorf("expected user %q, got %q", "system:apiserver", attrs.GetUser().GetName()) } atomic.AddInt32(&called, 1) } - return auditinternal.LevelNone, nil + return audit.RequestAuditConfigWithLevel{ + Level: auditinternal.LevelNone, + } }) }, }) @@ -106,9 +109,9 @@ func TestWebhookLoopback(t *testing.T) { } } -type auditPolicyRuleEvaluator func(authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage) +type auditPolicyRuleEvaluator func(authorizer.Attributes) audit.RequestAuditConfigWithLevel -func (f auditPolicyRuleEvaluator) LevelAndStages(attrs authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage) { +func (f auditPolicyRuleEvaluator) EvaluatePolicyRule(attrs authorizer.Attributes) audit.RequestAuditConfigWithLevel { return f(attrs) }