From 48b0a6812b9da8b4d4d7c8c368dcc77098f72a99 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 2 May 2024 11:25:51 +0100 Subject: [PATCH 1/3] Include correlation ID header by default for audit --- audit/headers.go | 23 ++++++++++++++++++ audit/headers_test.go | 54 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/audit/headers.go b/audit/headers.go index 51d4eddd2864..10aa45661742 100644 --- a/audit/headers.go +++ b/audit/headers.go @@ -163,6 +163,21 @@ func (a *HeadersConfig) Remove(ctx context.Context, header string) error { return nil } +// DefaultHeaders can be used to retrieve the set of default headers that will be +// added to HeadersConfig in order to allow them to appear in audit logs in a raw +// format. If the Vault Operator adds their own setting for any of the defaults, +// their setting will be honored. +func (a *HeadersConfig) DefaultHeaders() map[string]*HeaderSettings { + // Support deprecated 'x-' prefix (https://datatracker.ietf.org/doc/html/rfc6648) + const CorrelationID = "correlation-id" + XCorrelationID := fmt.Sprintf("x-%s", CorrelationID) + + return map[string]*HeaderSettings{ + CorrelationID: {}, + XCorrelationID: {}, + } +} + // Invalidate attempts to refresh the allowed audit headers and their settings. // NOTE: Invalidate will acquire a write lock in order to update the underlying headers. func (a *HeadersConfig) Invalidate(ctx context.Context) error { @@ -192,6 +207,14 @@ func (a *HeadersConfig) Invalidate(ctx context.Context) error { lowerHeaders[strings.ToLower(k)] = v } + // Ensure that we have default headers configured to appear in the audit log. + // Add them if they're missing. + for header, setting := range a.DefaultHeaders() { + if _, ok := lowerHeaders[header]; !ok { + lowerHeaders[header] = setting + } + } + a.headerSettings = lowerHeaders return nil } diff --git a/audit/headers_test.go b/audit/headers_test.go index 2020b28ce100..e4ff34047fc0 100644 --- a/audit/headers_test.go +++ b/audit/headers_test.go @@ -458,7 +458,7 @@ func TestAuditedHeaders_invalidate(t *testing.T) { // Invalidate and check we now see the header we stored err = ahc.Invalidate(context.Background()) require.NoError(t, err) - require.Len(t, ahc.headerSettings, 1) + require.Equal(t, len(ahc.DefaultHeaders())+1, len(ahc.headerSettings)) // (defaults + 1). _, ok := ahc.headerSettings["x-magic-header"] require.True(t, ok) @@ -475,7 +475,7 @@ func TestAuditedHeaders_invalidate(t *testing.T) { // Invalidate and check we now see the header we stored err = ahc.Invalidate(context.Background()) require.NoError(t, err) - require.Len(t, ahc.headerSettings, 2) + require.Equal(t, len(ahc.DefaultHeaders())+2, len(ahc.headerSettings)) // (defaults + 2 new headers) _, ok = ahc.headerSettings["x-magic-header"] require.True(t, ok) _, ok = ahc.headerSettings["x-even-more-magic-header"] @@ -502,7 +502,7 @@ func TestAuditedHeaders_invalidate_nil_view(t *testing.T) { // Invalidate and check we now see the header we stored err = ahc.Invalidate(context.Background()) require.NoError(t, err) - require.Len(t, ahc.headerSettings, 1) + require.Equal(t, len(ahc.DefaultHeaders())+1, len(ahc.headerSettings)) // defaults + 1 _, ok := ahc.headerSettings["x-magic-header"] require.True(t, ok) @@ -516,7 +516,7 @@ func TestAuditedHeaders_invalidate_nil_view(t *testing.T) { // Invalidate should clear out the existing headers without error err = ahc.Invalidate(context.Background()) require.NoError(t, err) - require.Len(t, ahc.headerSettings, 0) + require.Equal(t, len(ahc.DefaultHeaders()), len(ahc.headerSettings)) // defaults } // TestAuditedHeaders_invalidate_bad_data ensures that we correctly error if the @@ -584,3 +584,49 @@ func TestAuditedHeaders_headers(t *testing.T) { require.Equal(t, true, s["juan"].HMAC) require.Equal(t, false, s["john"].HMAC) } + +// TestAuditedHeaders_invalidate_defaults checks that we ensure any 'default' headers +// are present after invalidation, and if they were loaded from storage then they +// do not get overwritten with our defaults. +func TestAuditedHeaders_invalidate_defaults(t *testing.T) { + t.Parallel() + + view := newMockStorage(t) + ahc, err := NewHeadersConfig(view) + require.NoError(t, err) + require.Len(t, ahc.headerSettings, 0) + + // Store some data using the view. + fakeHeaders1 := map[string]*HeaderSettings{"x-magic-header": {}} + fakeBytes1, err := json.Marshal(fakeHeaders1) + require.NoError(t, err) + err = view.Put(context.Background(), &logical.StorageEntry{Key: auditedHeadersEntry, Value: fakeBytes1}) + require.NoError(t, err) + + // Invalidate and check we now see the header we stored + err = ahc.Invalidate(context.Background()) + require.NoError(t, err) + require.Equal(t, len(ahc.DefaultHeaders())+1, len(ahc.headerSettings)) // (defaults + 1 new header) + _, ok := ahc.headerSettings["x-magic-header"] + require.True(t, ok) + s, ok := ahc.headerSettings["x-correlation-id"] + require.True(t, ok) + require.False(t, s.HMAC) + + // Add correlation ID specifically with HMAC and make sure it doesn't get blasted away. + fakeHeaders1 = map[string]*HeaderSettings{"x-magic-header": {}, "X-Correlation-ID": {HMAC: true}} + fakeBytes1, err = json.Marshal(fakeHeaders1) + require.NoError(t, err) + err = view.Put(context.Background(), &logical.StorageEntry{Key: auditedHeadersEntry, Value: fakeBytes1}) + require.NoError(t, err) + + // Invalidate and check we now see the header we stored + err = ahc.Invalidate(context.Background()) + require.NoError(t, err) + require.Equal(t, len(ahc.DefaultHeaders())+1, len(ahc.headerSettings)) // (defaults + 1 new header, 1 is also a default) + _, ok = ahc.headerSettings["x-magic-header"] + require.True(t, ok) + s, ok = ahc.headerSettings["x-correlation-id"] + require.True(t, ok) + require.True(t, s.HMAC) +} From 4a7f568cb2156682bfea6975c32f2fe77974b0cf Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 2 May 2024 11:34:08 +0100 Subject: [PATCH 2/3] changelog --- changelog/26777.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/26777.txt diff --git a/changelog/26777.txt b/changelog/26777.txt new file mode 100644 index 000000000000..1d04c1646624 --- /dev/null +++ b/changelog/26777.txt @@ -0,0 +1,4 @@ +```release-note:change +audit: breaking change - Vault now allows audit logs to contain 'correlation-id' and 'x-correlation-id' headers when they +are present in the incoming request. By default they are not HMAC'ed (but can be configured to HMAC by Vault Operators). +``` From c57ea027cf1f4920ae8f65814825e6749826efd8 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 7 May 2024 16:55:44 +0100 Subject: [PATCH 3/3] casing adjustment --- audit/headers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/audit/headers.go b/audit/headers.go index 10aa45661742..535a61244e3e 100644 --- a/audit/headers.go +++ b/audit/headers.go @@ -169,12 +169,12 @@ func (a *HeadersConfig) Remove(ctx context.Context, header string) error { // their setting will be honored. func (a *HeadersConfig) DefaultHeaders() map[string]*HeaderSettings { // Support deprecated 'x-' prefix (https://datatracker.ietf.org/doc/html/rfc6648) - const CorrelationID = "correlation-id" - XCorrelationID := fmt.Sprintf("x-%s", CorrelationID) + const correlationID = "correlation-id" + xCorrelationID := fmt.Sprintf("x-%s", correlationID) return map[string]*HeaderSettings{ - CorrelationID: {}, - XCorrelationID: {}, + correlationID: {}, + xCorrelationID: {}, } }