Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VAULT-26466: audit - include correlation ID headers by default #26777

Merged
merged 4 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions audit/headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
ccapurso marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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
}
Expand Down
54 changes: 50 additions & 4 deletions audit/headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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"]
Expand All @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
4 changes: 4 additions & 0 deletions changelog/26777.txt
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ What is the user impact given the breaking change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a corresponding docs PR that tries to call out the change: #26778.

The Vercel deployment currently lives here: https://vault-n5poe4e19-hashicorp.vercel.app/vault/docs/audit#audit-request-headers but could change if there are any more pushes to that branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the clarification!

are present in the incoming request. By default they are not HMAC'ed (but can be configured to HMAC by Vault Operators).
```
Loading