From 8be823b0b0270e1b979b3d4c6e683e1daa0f2e01 Mon Sep 17 00:00:00 2001 From: Abu Kashem Date: Mon, 20 Sep 2021 17:43:16 -0400 Subject: [PATCH] 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") }