diff --git a/CHANGELOG.md b/CHANGELOG.md index ea375c691e..a9a8f5b6e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ IMPROVEMENTS: BUG FIXES: + * core: Correctly honor non-HMAC request keys when auditing requests [GH-6653] * replication: Fix an issue causing startup problems if a namespace policy wasn't replicated properly * replication: Properly update mount entry cache on a secondary to apply all diff --git a/vault/acl.go b/vault/acl.go index 4105b3583f..1313fa33b5 100644 --- a/vault/acl.go +++ b/vault/acl.go @@ -709,7 +709,14 @@ func valueInParameterList(v interface{}, list []interface{}) bool { func valueInSlice(v interface{}, list []interface{}) bool { for _, el := range list { - if reflect.TypeOf(el).String() == "string" && reflect.TypeOf(v).String() == "string" { + if el == nil || v == nil { + // It doesn't seem possible to set up a nil entry in the list, but it is possible + // to pass in a null entry in the API request being checked. Just in case, + // nil will match nil. + if el == v { + return true + } + } else if reflect.TypeOf(el).String() == "string" && reflect.TypeOf(v).String() == "string" { item := el.(string) val := v.(string) diff --git a/vault/acl_test.go b/vault/acl_test.go index 5a2de1b7fc..a8cb634dd8 100644 --- a/vault/acl_test.go +++ b/vault/acl_test.go @@ -549,6 +549,8 @@ func testACLValuePermissions(t *testing.T, ns *namespace.Namespace) { {"foo/bar", []string{"deny"}, []interface{}{"bad glob"}, false}, {"foo/bar", []string{"deny"}, []interface{}{"good"}, true}, {"foo/bar", []string{"allow"}, []interface{}{"good"}, true}, + {"foo/bar", []string{"deny"}, []interface{}{nil}, true}, + {"foo/bar", []string{"allow"}, []interface{}{nil}, true}, {"foo/baz", []string{"aLLow"}, []interface{}{"good"}, true}, {"foo/baz", []string{"deny"}, []interface{}{"bad"}, false}, {"foo/baz", []string{"deny"}, []interface{}{"good"}, false}, @@ -557,6 +559,7 @@ func testACLValuePermissions(t *testing.T, ns *namespace.Namespace) { {"foo/baz", []string{"deNy", "allow"}, []interface{}{"bad", "good"}, false}, {"foo/baz", []string{"aLLow"}, []interface{}{"bad"}, false}, {"foo/baz", []string{"Neither"}, []interface{}{"bad"}, false}, + {"foo/baz", []string{"allow"}, []interface{}{nil}, false}, {"fizz/buzz", []string{"allow_multi"}, []interface{}{"good"}, true}, {"fizz/buzz", []string{"allow_multi"}, []interface{}{"good1"}, true}, {"fizz/buzz", []string{"allow_multi"}, []interface{}{"good2"}, true}, diff --git a/vault/request_handling.go b/vault/request_handling.go index b2f0a9a202..d3e9bb04a6 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -914,9 +914,17 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re req.Unauthenticated = true - var auth *logical.Auth + var nonHMACReqDataKeys []string + entry := c.router.MatchingMountEntry(ctx, req.Path) + if entry != nil { + // Get and set ignored HMAC'd value. + if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok { + nonHMACReqDataKeys = rawVals.([]string) + } + } // Do an unauth check. This will cause EGP policies to be checked + var auth *logical.Auth var ctErr error auth, _, ctErr = c.checkToken(ctx, req, true) if ctErr == logical.ErrPerfStandbyPleaseForward { @@ -933,15 +941,6 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re errType = logical.ErrInvalidRequest } - var nonHMACReqDataKeys []string - entry := c.router.MatchingMountEntry(ctx, req.Path) - if entry != nil { - // Get and set ignored HMAC'd value. - if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok { - nonHMACReqDataKeys = rawVals.([]string) - } - } - logInput := &audit.LogInput{ Auth: auth, Request: req, @@ -965,8 +964,9 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re // Create an audit trail of the request. Attach auth if it was returned, // e.g. if a token was provided. logInput := &audit.LogInput{ - Auth: auth, - Request: req, + Auth: auth, + Request: req, + NonHMACReqDataKeys: nonHMACReqDataKeys, } if err := c.auditBroker.LogRequest(ctx, logInput, c.auditedHeaders); err != nil { c.logger.Error("failed to audit request", "path", req.Path, "error", err)