VAULT-23121: Audit - Empty fields are HMAC and appear in audit logs

audit/entry_formatter_test.go

@@ -15,6 +15,7 @@
 	"github.com/hashicorp/eventlogger"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-sockaddr"
+	"github.com/hashicorp/vault/builtin/logical/transit"
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/internal/observability/event"
 	"github.com/hashicorp/vault/sdk/helper/jsonutil"
@@ -640,6 +641,62 @@
 	}
 }
 
+// TestEntryFormatter_FormatResponse_DefaultValueHMACIgnored tests that we ignore
+// 'default' ("") string values for fields when formatting a response using HMAC.
+// The reason for the test is, so we can allow the JSON struct tags to operate on
+// types as expected and be omitted if they were empty rather than HMAC'd in the
+// audit log with the same value over and over again.
+// We also want the behavior of the output to be consistent whether using 'raw'
+// output or HMAC.
+func TestEntryFormatter_FormatResponse_DefaultValueHMACIgnored(t *testing.T) {
+	const hmacPrefix = "hmac-sha256:"
+	ss := newStaticSalt(t)
+	cfg, err := NewFormatterConfig()
+	require.NoError(t, err)
+	l := hclog.NewNullLogger()
+	f, err := NewEntryFormatter("test", cfg, ss, l)
+	require.NoError(t, err)
+
+	input := &logical.LogInput{
+		Request: &logical.Request{ID: "123"},
+		Response: &logical.Response{
+			Data: map[string]any{
+				"batch_results": []transit.EncryptBatchResponseItem{
+					{
+						Ciphertext: "juan1",
+						KeyVersion: 1,
+						Error:      "",
+						Reference:  "qwerty1",
+					},
+					{
+						Ciphertext: "juan2",
+						KeyVersion: 2,
+						Error:      "this is an error",
+						Reference:  "qwerty2",
+					},
+				},
+			},
+		},
+	}
+
+	ctx := namespace.RootContext(context.Background())
+
+	entry, err := f.FormatResponse(ctx, input)
+	require.NoError(t, err)
+	require.NotNil(t, entry)
+	d := entry.Response.Data["batch_results"]
+	results, ok := d.([]transit.EncryptBatchResponseItem)
+	require.True(t, ok)
+	require.Len(t, results, 2)
+
+	juan1 := results[0]
+	juan2 := results[1]
+	require.Equal(t, "", juan1.Error)
+	require.NotEqual(t, "", juan2.Error)
+	require.Contains(t, juan2.Error, hmacPrefix)
+	require.Greater(t, len(juan2.Error), len(hmacPrefix))
+}
+
 // TestEntryFormatter_Process_JSON ensures that the JSON output we get matches what
 // we expect for the specified LogInput.
 func TestEntryFormatter_Process_JSON(t *testing.T) {

changelog/24901.txt

@@ -0,0 +1,4 @@
+```release-note:bug
+audit: Empty string fields no longer have HMAC applied, allowing them to be omitted from the audit log when marked as such in code.
+This change aligns the behavior of audit logs to match the log structure when using the `log_raw` option.
+```

sdk/helper/salt/salt.go

@@ -13,7 +13,7 @@ import (
 	"hash"
 
 	"github.com/hashicorp/errwrap"
-	uuid "github.com/hashicorp/go-uuid"
+	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
@@ -150,8 +150,14 @@ func (s *Salt) GetHMAC(data string) string {
 }
 
 // GetIdentifiedHMAC is used to apply a salt and hash function to data to make
-// sure it is not reversible, with an additional HMAC, and ID prepended
+// sure it is not reversible, with an additional HMAC, and ID prepended.
+// Default value ("") string input will be ignored and returned as-is.
 func (s *Salt) GetIdentifiedHMAC(data string) string {
+	// Do not HMAC an empty value.
+	if data == "" {
+		return data
+	}
+
 	return s.config.HMACType + ":" + s.GetHMAC(data)
 }
 

sdk/helper/salt/salt_test.go

@@ -9,8 +9,9 @@ import (
 	"crypto/sha256"
 	"testing"
 
-	uuid "github.com/hashicorp/go-uuid"
+	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSalt(t *testing.T) {
@@ -89,3 +90,15 @@ func TestSaltID(t *testing.T) {
 		t.Fatalf("mismatch")
 	}
 }
+
+// TestSalt_GetHMAC_Ignored ensures that when attempting to HMAC a default value
+// we don't return a HMAC, and instead just return the default (empty string).
+// This means that when we write audit entries and encode the request/response to
+// JSON we can successfully ignore empty fields if they've been marked as such via
+// the use of struct tags.
+func TestSalt_GetHMAC_Ignored(t *testing.T) {
+	s, err := NewSalt(context.Background(), nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "", s.GetIdentifiedHMAC(""))
+	require.NotEqual(t, "", s.GetIdentifiedHMAC("juan"))
+}