Initialize the AuditEvent with the AuditContext (#113611)

* Initialize the AuditEvent with the AuditContext

* Squash: Address PR feedback

* Squash: address PR feedback
This commit is contained in:
Tim Allclair 2023-07-03 09:16:51 -07:00 committed by GitHub
parent b1d43aa1dd
commit 2b03f04ce5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 138 additions and 149 deletions

View File

@ -142,10 +142,10 @@ func TestWithAudit(t *testing.T) {
}
for tcName, tc := range testCases {
var handler Interface = fakeHandler{tc.admit, tc.admitAnnotations, tc.validate, tc.validateAnnotations, tc.handles}
ae := &auditinternal.Event{Level: auditinternal.LevelMetadata}
ctx := audit.WithAuditContext(context.Background())
ac := audit.AuditContextFrom(ctx)
ac.Event = ae
ae := &ac.Event
ae.Level = auditinternal.LevelMetadata
auditHandler := WithAudit(handler)
a := attributes()
@ -188,7 +188,7 @@ func TestWithAuditConcurrency(t *testing.T) {
var handler Interface = fakeHandler{admitAnnotations: admitAnnotations, handles: true}
ctx := audit.WithAuditContext(context.Background())
ac := audit.AuditContextFrom(ctx)
ac.Event = &auditinternal.Event{Level: auditinternal.LevelMetadata}
ac.Event.Level = auditinternal.LevelMetadata
auditHandler := WithAudit(handler)
a := attributes()

View File

@ -39,21 +39,18 @@ type AuditContext struct {
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 *auditinternal.Event
// the API audit log.
Event auditinternal.Event
// annotations holds audit annotations that are recorded before the event has been initialized.
// This is represented as a slice rather than a map to preserve order.
annotations []annotation
// annotationMutex guards annotations AND event.Annotations
// annotationMutex guards event.Annotations
annotationMutex sync.Mutex
// auditID is the Audit ID associated with this request.
auditID types.UID
}
type annotation struct {
key, value string
// Enabled checks whether auditing is enabled for this audit context.
func (ac *AuditContext) Enabled() bool {
// Note: An unset Level should be considered Enabled, so that request data (e.g. annotations)
// can still be captured before the audit policy is evaluated.
return ac != nil && ac.RequestAuditConfig.Level != auditinternal.LevelNone
}
// AddAuditAnnotation sets the audit annotation for the given key, value pair.
@ -65,8 +62,7 @@ type annotation struct {
// prefer AddAuditAnnotation over LogAnnotation to avoid dropping annotations.
func AddAuditAnnotation(ctx context.Context, key, value string) {
ac := AuditContextFrom(ctx)
if ac == nil {
// auditing is not enabled
if !ac.Enabled() {
return
}
@ -81,8 +77,7 @@ func AddAuditAnnotation(ctx context.Context, key, value string) {
// keysAndValues are the key-value pairs to add, and must have an even number of items.
func AddAuditAnnotations(ctx context.Context, keysAndValues ...string) {
ac := AuditContextFrom(ctx)
if ac == nil {
// auditing is not enabled
if !ac.Enabled() {
return
}
@ -101,8 +96,7 @@ func AddAuditAnnotations(ctx context.Context, keysAndValues ...string) {
// restrictions on when this can be called.
func AddAuditAnnotationsMap(ctx context.Context, annotations map[string]string) {
ac := AuditContextFrom(ctx)
if ac == nil {
// auditing is not enabled
if !ac.Enabled() {
return
}
@ -114,38 +108,10 @@ func AddAuditAnnotationsMap(ctx context.Context, annotations map[string]string)
}
}
// addAuditAnnotationLocked is the shared code for recording an audit annotation. This method should
// only be called while the auditAnnotationsMutex is locked.
// addAuditAnnotationLocked records the audit annotation on the event.
func addAuditAnnotationLocked(ac *AuditContext, key, value string) {
if ac.Event != nil {
logAnnotation(ac.Event, key, value)
} else {
ac.annotations = append(ac.annotations, annotation{key: key, value: value})
}
}
ae := &ac.Event
// This is private to prevent reads/write to the slice from outside of this package.
// The audit event should be directly read to get access to the annotations.
func addAuditAnnotationsFrom(ctx context.Context, ev *auditinternal.Event) {
ac := AuditContextFrom(ctx)
if ac == nil {
// auditing is not enabled
return
}
ac.annotationMutex.Lock()
defer ac.annotationMutex.Unlock()
for _, kv := range ac.annotations {
logAnnotation(ev, kv.key, kv.value)
}
}
// LogAnnotation fills in the Annotations according to the key value pair.
func logAnnotation(ae *auditinternal.Event, key, value string) {
if ae == nil || ae.Level.Less(auditinternal.LevelMetadata) {
return
}
if ae.Annotations == nil {
ae.Annotations = make(map[string]string)
}
@ -167,8 +133,8 @@ func WithAuditContext(parent context.Context) context.Context {
// 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
if ac := AuditContextFrom(ctx); ac.Enabled() {
return &ac.Event
}
return nil
}
@ -187,20 +153,16 @@ func WithAuditID(ctx context.Context, auditID types.UID) {
if auditID == "" {
return
}
ac := AuditContextFrom(ctx)
if ac == nil {
return
}
ac.auditID = auditID
if ac.Event != nil {
if ac := AuditContextFrom(ctx); ac != nil {
ac.Event.AuditID = auditID
}
}
// AuditIDFrom returns the value of the audit ID from the request context.
// AuditIDFrom returns the value of the audit ID from the request context, along with whether
// auditing is enabled.
func AuditIDFrom(ctx context.Context) (types.UID, bool) {
if ac := AuditContextFrom(ctx); ac != nil {
return ac.auditID, ac.auditID != ""
return ac.Event.AuditID, true
}
return "", false
}

View File

@ -27,32 +27,53 @@ import (
"github.com/stretchr/testify/assert"
)
func TestEnabled(t *testing.T) {
tests := []struct {
name string
ctx *AuditContext
expectEnabled bool
}{{
name: "nil context",
expectEnabled: false,
}, {
name: "empty context",
ctx: &AuditContext{},
expectEnabled: true, // An AuditContext should be considered enabled before the level is set
}, {
name: "level None",
ctx: &AuditContext{RequestAuditConfig: RequestAuditConfig{Level: auditinternal.LevelNone}},
expectEnabled: false,
}, {
name: "level Metadata",
ctx: &AuditContext{RequestAuditConfig: RequestAuditConfig{Level: auditinternal.LevelMetadata}},
expectEnabled: true,
}, {
name: "level RequestResponse",
ctx: &AuditContext{RequestAuditConfig: RequestAuditConfig{Level: auditinternal.LevelRequestResponse}},
expectEnabled: true,
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.expectEnabled, test.ctx.Enabled())
})
}
}
func TestAddAuditAnnotation(t *testing.T) {
const (
annotationKeyTemplate = "test-annotation-%d"
annotationValue = "test-annotation-value"
annotationExtraValue = "test-existing-annotation"
numAnnotations = 10
)
expectAnnotations := func(t *testing.T, annotations map[string]string) {
assert.Len(t, annotations, numAnnotations)
}
noopValidator := func(_ *testing.T, _ context.Context) {}
preEventValidator := func(t *testing.T, ctx context.Context) {
ev := auditinternal.Event{
Level: auditinternal.LevelMetadata,
}
addAuditAnnotationsFrom(ctx, &ev)
expectAnnotations(t, ev.Annotations)
}
postEventValidator := func(t *testing.T, ctx context.Context) {
ev := AuditEventFrom(ctx)
expectAnnotations(t, ev.Annotations)
}
postEventEmptyValidator := func(t *testing.T, ctx context.Context) {
ev := AuditEventFrom(ctx)
assert.Empty(t, ev.Annotations)
}
ctxWithAnnotation := withAuditContextAndLevel(context.Background(), auditinternal.LevelMetadata)
AddAuditAnnotation(ctxWithAnnotation, fmt.Sprintf(annotationKeyTemplate, 0), annotationExtraValue)
tests := []struct {
description string
@ -61,19 +82,39 @@ func TestAddAuditAnnotation(t *testing.T) {
}{{
description: "no audit",
ctx: context.Background(),
validator: noopValidator,
validator: func(_ *testing.T, _ context.Context) {},
}, {
description: "empty audit context",
ctx: WithAuditContext(context.Background()),
validator: preEventValidator,
description: "context initialized, policy not evaluated",
// Audit context is initialized, but the policy has not yet been evaluated (no level).
// Annotations should be retained.
ctx: WithAuditContext(context.Background()),
validator: func(t *testing.T, ctx context.Context) {
ev := AuditContextFrom(ctx).Event
expectAnnotations(t, ev.Annotations)
},
}, {
description: "with metadata level",
ctx: withAuditContextAndLevel(context.Background(), auditinternal.LevelMetadata),
validator: postEventValidator,
validator: func(t *testing.T, ctx context.Context) {
ev := AuditContextFrom(ctx).Event
expectAnnotations(t, ev.Annotations)
},
}, {
description: "with none level",
ctx: withAuditContextAndLevel(context.Background(), auditinternal.LevelNone),
validator: postEventEmptyValidator,
validator: func(t *testing.T, ctx context.Context) {
ev := AuditContextFrom(ctx).Event
assert.Empty(t, ev.Annotations)
},
}, {
description: "with overlapping annotations",
ctx: ctxWithAnnotation,
validator: func(t *testing.T, ctx context.Context) {
ev := AuditContextFrom(ctx).Event
expectAnnotations(t, ev.Annotations)
// Verify that the pre-existing annotation is not overwritten.
assert.Equal(t, annotationExtraValue, ev.Annotations[fmt.Sprintf(annotationKeyTemplate, 0)])
},
}}
for _, test := range tests {
@ -93,25 +134,32 @@ func TestAddAuditAnnotation(t *testing.T) {
}
}
func TestLogAnnotation(t *testing.T) {
ev := &auditinternal.Event{
Level: auditinternal.LevelMetadata,
AuditID: "fake id",
}
logAnnotation(ev, "foo", "bar")
logAnnotation(ev, "foo", "baz")
assert.Equal(t, "bar", ev.Annotations["foo"], "audit annotation should not be overwritten.")
func TestAuditAnnotationsWithAuditLoggingSetup(t *testing.T) {
// No audit context data in the request context
ctx := context.Background()
AddAuditAnnotation(ctx, "nil", "0")
logAnnotation(ev, "qux", "")
logAnnotation(ev, "qux", "baz")
assert.Equal(t, "", ev.Annotations["qux"], "audit annotation should not be overwritten.")
// initialize audit context, policy not evaluated yet
ctx = WithAuditContext(ctx)
AddAuditAnnotation(ctx, "before-evaluation", "1")
// policy evaluated, audit logging enabled
if ac := AuditContextFrom(ctx); ac != nil {
ac.RequestAuditConfig.Level = auditinternal.LevelMetadata
}
AddAuditAnnotation(ctx, "after-evaluation", "2")
expected := map[string]string{
"before-evaluation": "1",
"after-evaluation": "2",
}
actual := AuditContextFrom(ctx).Event.Annotations
assert.Equal(t, expected, actual)
}
func withAuditContextAndLevel(ctx context.Context, l auditinternal.Level) context.Context {
ctx = WithAuditContext(ctx)
ac := AuditContextFrom(ctx)
ac.Event = &auditinternal.Event{
Level: l,
}
ac.RequestAuditConfig.Level = l
return ctx
}

View File

@ -28,14 +28,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilnet "k8s.io/apimachinery/pkg/util/net"
auditinternal "k8s.io/apiserver/pkg/apis/audit"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/klog/v2"
"github.com/google/uuid"
)
const (
@ -43,20 +40,18 @@ const (
userAgentTruncateSuffix = "...TRUNCATED"
)
func NewEventFromRequest(req *http.Request, requestReceivedTimestamp time.Time, level auditinternal.Level, attribs authorizer.Attributes) (*auditinternal.Event, error) {
ev := &auditinternal.Event{
RequestReceivedTimestamp: metav1.NewMicroTime(requestReceivedTimestamp),
Verb: attribs.GetVerb(),
RequestURI: req.URL.RequestURI(),
UserAgent: maybeTruncateUserAgent(req),
Level: level,
func LogRequestMetadata(ctx context.Context, req *http.Request, requestReceivedTimestamp time.Time, level auditinternal.Level, attribs authorizer.Attributes) {
ac := AuditContextFrom(ctx)
if !ac.Enabled() {
return
}
ev := &ac.Event
auditID, found := AuditIDFrom(req.Context())
if !found {
auditID = types.UID(uuid.New().String())
}
ev.AuditID = auditID
ev.RequestReceivedTimestamp = metav1.NewMicroTime(requestReceivedTimestamp)
ev.Verb = attribs.GetVerb()
ev.RequestURI = req.URL.RequestURI()
ev.UserAgent = maybeTruncateUserAgent(req)
ev.Level = level
ips := utilnet.SourceIPs(req)
ev.SourceIPs = make([]string, len(ips))
@ -84,10 +79,6 @@ func NewEventFromRequest(req *http.Request, requestReceivedTimestamp time.Time,
APIVersion: attribs.GetAPIVersion(),
}
}
addAuditAnnotationsFrom(req.Context(), ev)
return ev, nil
}
// LogImpersonatedUser fills in the impersonated user attributes into an audit event.

View File

@ -197,15 +197,14 @@ func (a *cachedTokenAuthenticator) doAuthenticateToken(ctx context.Context, toke
recorder := &recorder{}
ctx = warning.WithWarningRecorder(ctx, recorder)
// 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 = audit.WithAuditContext(ctx)
ac := audit.AuditContextFrom(ctx)
ac.Event = ev
// 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.
ac.Event.Level = auditinternal.LevelMetadata
record.resp, record.ok, record.err = a.authenticator.AuthenticateToken(ctx, token)
record.annotations = ev.Annotations
record.annotations = ac.Event.Annotations
record.warnings = recorder.extractWarnings()
if !a.cacheErrs && record.err != nil {

View File

@ -547,7 +547,7 @@ func (s *singleBenchmark) bench(b *testing.B) {
func withAudit(ctx context.Context) context.Context {
ctx = audit.WithAuditContext(ctx)
ac := audit.AuditContextFrom(ctx)
ac.Event = &auditinternal.Event{Level: auditinternal.LevelMetadata}
ac.Event.Level = auditinternal.LevelMetadata
return ctx
}

View File

@ -51,11 +51,11 @@ func WithAudit(handler http.Handler, sink audit.Sink, policy audit.PolicyRuleEva
return
}
if ac == nil || ac.Event == nil {
if !ac.Enabled() {
handler.ServeHTTP(w, req)
return
}
ev := ac.Event
ev := &ac.Event
ctx := req.Context()
omitStages := ac.RequestAuditConfig.OmitStages
@ -124,7 +124,7 @@ func evaluatePolicyAndCreateAuditEvent(req *http.Request, policy audit.PolicyRul
ctx := req.Context()
ac := audit.AuditContextFrom(ctx)
if ac == nil {
// Auditing not enabled.
// Auditing not configured.
return nil, nil
}
@ -145,12 +145,7 @@ func evaluatePolicyAndCreateAuditEvent(req *http.Request, policy audit.PolicyRul
if !ok {
requestReceivedTimestamp = time.Now()
}
ev, err := audit.NewEventFromRequest(req, requestReceivedTimestamp, rac.Level, attribs)
if err != nil {
return nil, fmt.Errorf("failed to complete audit event from request: %v", err)
}
ac.Event = ev
audit.LogRequestMetadata(ctx, req, requestReceivedTimestamp, rac.Level, attribs)
return ac, nil
}

View File

@ -849,7 +849,7 @@ func withTestContext(req *http.Request, user user.Info, ae *auditinternal.Event)
}
if ae != nil {
ac := audit.AuditContextFrom(ctx)
ac.Event = ae
ac.Event = *ae
}
if info, err := newTestRequestInfoResolver().NewRequestInfo(req); err == nil {
ctx = request.WithRequestInfo(ctx, info)

View File

@ -43,11 +43,11 @@ func WithFailedAuthenticationAudit(failedHandler http.Handler, sink audit.Sink,
return
}
if ac == nil || ac.Event == nil {
if !ac.Enabled() {
failedHandler.ServeHTTP(w, req)
return
}
ev := ac.Event
ev := &ac.Event
ev.ResponseStatus = &metav1.Status{}
ev.ResponseStatus.Message = getAuthMethods(req)

View File

@ -29,6 +29,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/authorization/authorizer"
)
@ -172,16 +173,16 @@ func TestAuditAnnotation(t *testing.T) {
scheme := runtime.NewScheme()
negotiatedSerializer := serializer.NewCodecFactory(scheme).WithoutConversion()
for k, tc := range testcases {
audit := &auditinternal.Event{Level: auditinternal.LevelMetadata}
handler := WithAuthorization(&fakeHTTPHandler{}, tc.authorizer, negotiatedSerializer)
// TODO: fake audit injector
req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil)
req = withTestContext(req, nil, audit)
req = withTestContext(req, nil, &auditinternal.Event{Level: auditinternal.LevelMetadata})
ae := audit.AuditEventFrom(req.Context())
req.RemoteAddr = "127.0.0.1"
handler.ServeHTTP(httptest.NewRecorder(), req)
assert.Equal(t, tc.decisionAnnotation, audit.Annotations[decisionAnnotationKey], k+": unexpected decision annotation")
assert.Equal(t, tc.reasonAnnotation, audit.Annotations[reasonAnnotationKey], k+": unexpected reason annotation")
assert.Equal(t, tc.decisionAnnotation, ae.Annotations[decisionAnnotationKey], k+": unexpected decision annotation")
assert.Equal(t, tc.reasonAnnotation, ae.Annotations[reasonAnnotationKey], k+": unexpected reason annotation")
}
}

View File

@ -115,11 +115,11 @@ func withFailedRequestAudit(failedHandler http.Handler, statusErr *apierrors.Sta
return
}
if ac == nil || ac.Event == nil {
if !ac.Enabled() {
failedHandler.ServeHTTP(w, req)
return
}
ev := ac.Event
ev := &ac.Event
ev.ResponseStatus = &metav1.Status{}
ev.Stage = auditinternal.StageResponseStarted

View File

@ -66,9 +66,7 @@ func TestDeleteResourceAuditLogRequestObject(t *testing.T) {
ctx := audit.WithAuditContext(context.TODO())
ac := audit.AuditContextFrom(ctx)
ac.Event = &auditapis.Event{
Level: auditapis.LevelRequestResponse,
}
ac.Event.Level = auditapis.LevelRequestResponse
policy := metav1.DeletePropagationBackground
deleteOption := &metav1.DeleteOptions{

View File

@ -285,11 +285,6 @@ func TestAuthenticationAuditAnnotationsDefaultChain(t *testing.T) {
// confirm that we can set an audit annotation in a handler before WithAudit
audit.AddAuditAnnotation(req.Context(), "pandas", "are awesome")
// confirm that trying to use the audit event directly would never work
if ae := audit.AuditEventFrom(req.Context()); ae != nil {
t.Errorf("expected nil audit event, got %v", ae)
}
return &authenticator.Response{User: &user.DefaultInfo{}}, true, nil
})
backend := &testBackend{}

View File

@ -247,7 +247,7 @@ func TestCheckForHostnameError(t *testing.T) {
}
req = req.WithContext(audit.WithAuditContext(req.Context()))
auditCtx := audit.AuditContextFrom(req.Context())
auditCtx.Event = &auditapi.Event{Level: auditapi.LevelMetadata}
auditCtx.Event.Level = auditapi.LevelMetadata
_, err = client.Transport.RoundTrip(req)
@ -390,7 +390,7 @@ func TestCheckForInsecureAlgorithmError(t *testing.T) {
}
req = req.WithContext(audit.WithAuditContext(req.Context()))
auditCtx := audit.AuditContextFrom(req.Context())
auditCtx.Event = &auditapi.Event{Level: auditapi.LevelMetadata}
auditCtx.Event.Level = auditapi.LevelMetadata
// can't use tlsServer.Client() as it contains the server certificate
// in tls.Config.Certificates. The signatures are, however, only checked