Non-HMAC audit values (hashicorp#4033)

* Add non-hmac request keys

* Update comment

* Initial audit request keys implementation

* Add audit_non_hmac_response_keys

* Move where req.NonHMACKeys gets set

* Minor refactor

* Add params to auth tune endpoints

* Sync cache on loadCredentials

* Explicitly unset req.NonHMACKeys

* Do not error if entry is nil

* Add tests

* docs: Add params to api sections

* Refactor audit.Backend and Formatter interfaces, update audit broker methods

* Add audit_broker.go

* Fix method call params in audit backends

* Remove fields from logical.Request and logical.Response, pass keys via LogInput

* Use data.GetOk to allow unsetting existing values

* Remove debug lines

* Add test for unsetting values

* Address review feedback

* Initialize values in FormatRequest and FormatResponse using input values

* Update docs

* Use strutil.StrListContains

* Use strutil.StrListContains

Author: calvn

api/sys_mounts.go

@@ -129,10 +129,12 @@ type MountInput struct {
 }
 
 type MountConfigInput struct {
-	DefaultLeaseTTL string `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"`
-	MaxLeaseTTL     string `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"`
-	ForceNoCache    bool   `json:"force_no_cache" structs:"force_no_cache" mapstructure:"force_no_cache"`
-	PluginName      string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
+	DefaultLeaseTTL          string   `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"`
+	MaxLeaseTTL              string   `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"`
+	ForceNoCache             bool     `json:"force_no_cache" structs:"force_no_cache" mapstructure:"force_no_cache"`
+	PluginName               string   `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
+	AuditNonHMACRequestKeys  []string `json:"audit_non_hmac_request_keys,omitempty" structs:"audit_non_hmac_request_keys" mapstructure:"audit_non_hmac_request_keys"`
+	AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" structs:"audit_non_hmac_response_keys" mapstructure:"audit_non_hmac_response_keys"`
 }
 
 type MountOutput struct {
@@ -145,8 +147,10 @@ type MountOutput struct {
 }
 
 type MountConfigOutput struct {
-	DefaultLeaseTTL int    `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"`
-	MaxLeaseTTL     int    `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"`
-	ForceNoCache    bool   `json:"force_no_cache" structs:"force_no_cache" mapstructure:"force_no_cache"`
-	PluginName      string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
+	DefaultLeaseTTL          int      `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"`
+	MaxLeaseTTL              int      `json:"max_lease_ttl" structs:"max_lease_ttl" mapstructure:"max_lease_ttl"`
+	ForceNoCache             bool     `json:"force_no_cache" structs:"force_no_cache" mapstructure:"force_no_cache"`
+	PluginName               string   `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
+	AuditNonHMACRequestKeys  []string `json:"audit_non_hmac_request_keys,omitempty" structs:"audit_non_hmac_request_keys" mapstructure:"audit_non_hmac_request_keys"`
+	AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" structs:"audit_non_hmac_response_keys" mapstructure:"audit_non_hmac_response_keys"`
 }

audit/audit.go

@@ -16,13 +16,13 @@ type Backend interface {
 	// request is authorized but before the request is executed. The arguments
 	// MUST not be modified in anyway. They should be deep copied if this is
 	// a possibility.
-	LogRequest(context.Context, *logical.Auth, *logical.Request, error) error
+	LogRequest(context.Context, *LogInput) error
 
 	// LogResponse is used to synchronously log a response. This is done after
 	// the request is processed but before the response is sent. The arguments
 	// MUST not be modified in anyway. They should be deep copied if this is
 	// a possibility.
-	LogResponse(context.Context, *logical.Auth, *logical.Request, *logical.Response, error) error
+	LogResponse(context.Context, *LogInput) error
 
 	// GetHash is used to return the given data with the backend's hash,
 	// so that a caller can determine if a value in the audit log matches
@@ -36,6 +36,18 @@ type Backend interface {
 	Invalidate(context.Context)
 }
 
+// LogInput contains the input parameters passed into LogRequest and LogResponse
+type LogInput struct {
+	Auth                *logical.Auth
+	Request             *logical.Request
+	Response            *logical.Response
+	OuterErr            error
+	NonHMACReqDataKeys  []string
+	NonHMACRespDataKeys []string
+}
+
+// BackendConfig contains configuration parameters used in the factory func to
+// instantiate audit backends
 type BackendConfig struct {
 	// The view to store the salt
 	SaltView logical.Storage

audit/format.go

@@ -25,14 +25,10 @@ type AuditFormatter struct {
 	AuditFormatWriter
 }
 
-func (f *AuditFormatter) FormatRequest(
-	w io.Writer,
-	config FormatterConfig,
-	auth *logical.Auth,
-	req *logical.Request,
-	inErr error) error {
-
-	if req == nil {
+var _ Formatter = (*AuditFormatter)(nil)
+
+func (f *AuditFormatter) FormatRequest(w io.Writer, config FormatterConfig, in *LogInput) error {
+	if in == nil || in.Request == nil {
 		return fmt.Errorf("request to request-audit a nil request")
 	}
 
@@ -49,28 +45,31 @@ func (f *AuditFormatter) FormatRequest(
 		return errwrap.Wrapf("error fetching salt: {{err}}", err)
 	}
 
+	// Set these to the input values at first
+	auth := in.Auth
+	req := in.Request
+
 	if !config.Raw {
 		// Before we copy the structure we must nil out some data
 		// otherwise we will cause reflection to panic and die
-		if req.Connection != nil && req.Connection.ConnState != nil {
-			origReq := req
-			origState := req.Connection.ConnState
-			req.Connection.ConnState = nil
+		if in.Request.Connection != nil && in.Request.Connection.ConnState != nil {
+			origState := in.Request.Connection.ConnState
+			in.Request.Connection.ConnState = nil
 			defer func() {
-				origReq.Connection.ConnState = origState
+				in.Request.Connection.ConnState = origState
 			}()
 		}
 
 		// Copy the auth structure
-		if auth != nil {
-			cp, err := copystructure.Copy(auth)
+		if in.Auth != nil {
+			cp, err := copystructure.Copy(in.Auth)
 			if err != nil {
 				return err
 			}
 			auth = cp.(*logical.Auth)
 		}
 
-		cp, err := copystructure.Copy(req)
+		cp, err := copystructure.Copy(in.Request)
 		if err != nil {
 			return err
 		}
@@ -83,7 +82,7 @@ func (f *AuditFormatter) FormatRequest(
 			if !config.HMACAccessor && auth.Accessor != "" {
 				authAccessor = auth.Accessor
 			}
-			if err := Hash(salt, auth); err != nil {
+			if err := Hash(salt, auth, nil); err != nil {
 				return err
 			}
 			if authAccessor != "" {
@@ -96,7 +95,7 @@ func (f *AuditFormatter) FormatRequest(
 		if !config.HMACAccessor && req != nil && req.ClientTokenAccessor != "" {
 			clientTokenAccessor = req.ClientTokenAccessor
 		}
-		if err := Hash(salt, req); err != nil {
+		if err := Hash(salt, req, in.NonHMACReqDataKeys); err != nil {
 			return err
 		}
 		if clientTokenAccessor != "" {
@@ -109,8 +108,8 @@ func (f *AuditFormatter) FormatRequest(
 		auth = new(logical.Auth)
 	}
 	var errString string
-	if inErr != nil {
-		errString = inErr.Error()
+	if in.OuterErr != nil {
+		errString = in.OuterErr.Error()
 	}
 
 	reqEntry := &AuditRequestEntry{
@@ -152,15 +151,8 @@ func (f *AuditFormatter) FormatRequest(
 	return f.AuditFormatWriter.WriteRequest(w, reqEntry)
 }
 
-func (f *AuditFormatter) FormatResponse(
-	w io.Writer,
-	config FormatterConfig,
-	auth *logical.Auth,
-	req *logical.Request,
-	resp *logical.Response,
-	inErr error) error {
-
-	if req == nil {
+func (f *AuditFormatter) FormatResponse(w io.Writer, config FormatterConfig, in *LogInput) error {
+	if in == nil || in.Request == nil {
 		return fmt.Errorf("request to response-audit a nil request")
 	}
 
@@ -177,35 +169,39 @@ func (f *AuditFormatter) FormatResponse(
 		return errwrap.Wrapf("error fetching salt: {{err}}", err)
 	}
 
+	// Set these to the input values at first
+	auth := in.Auth
+	req := in.Request
+	resp := in.Response
+
 	if !config.Raw {
 		// Before we copy the structure we must nil out some data
 		// otherwise we will cause reflection to panic and die
-		if req.Connection != nil && req.Connection.ConnState != nil {
-			origReq := req
-			origState := req.Connection.ConnState
-			req.Connection.ConnState = nil
+		if in.Request.Connection != nil && in.Request.Connection.ConnState != nil {
+			origState := in.Request.Connection.ConnState
+			in.Request.Connection.ConnState = nil
 			defer func() {
-				origReq.Connection.ConnState = origState
+				in.Request.Connection.ConnState = origState
 			}()
 		}
 
 		// Copy the auth structure
-		if auth != nil {
-			cp, err := copystructure.Copy(auth)
+		if in.Auth != nil {
+			cp, err := copystructure.Copy(in.Auth)
 			if err != nil {
 				return err
 			}
 			auth = cp.(*logical.Auth)
 		}
 
-		cp, err := copystructure.Copy(req)
+		cp, err := copystructure.Copy(in.Request)
 		if err != nil {
 			return err
 		}
 		req = cp.(*logical.Request)
 
-		if resp != nil {
-			cp, err := copystructure.Copy(resp)
+		if in.Response != nil {
+			cp, err := copystructure.Copy(in.Response)
 			if err != nil {
 				return err
 			}
@@ -220,7 +216,7 @@ func (f *AuditFormatter) FormatResponse(
 			if !config.HMACAccessor && auth.Accessor != "" {
 				accessor = auth.Accessor
 			}
-			if err := Hash(salt, auth); err != nil {
+			if err := Hash(salt, auth, nil); err != nil {
 				return err
 			}
 			if accessor != "" {
@@ -233,7 +229,7 @@ func (f *AuditFormatter) FormatResponse(
 		if !config.HMACAccessor && req != nil && req.ClientTokenAccessor != "" {
 			clientTokenAccessor = req.ClientTokenAccessor
 		}
-		if err := Hash(salt, req); err != nil {
+		if err := Hash(salt, req, in.NonHMACReqDataKeys); err != nil {
 			return err
 		}
 		if clientTokenAccessor != "" {
@@ -250,7 +246,7 @@ func (f *AuditFormatter) FormatResponse(
 				wrappedAccessor = resp.WrapInfo.WrappedAccessor
 				wrappingAccessor = resp.WrapInfo.Accessor
 			}
-			if err := Hash(salt, resp); err != nil {
+			if err := Hash(salt, resp, in.NonHMACRespDataKeys); err != nil {
 				return err
 			}
 			if accessor != "" {
@@ -273,8 +269,8 @@ func (f *AuditFormatter) FormatResponse(
 		resp = new(logical.Response)
 	}
 	var errString string
-	if inErr != nil {
-		errString = inErr.Error()
+	if in.OuterErr != nil {
+		errString = in.OuterErr.Error()
 	}
 
 	var respAuth *AuditAuth
@@ -358,7 +354,7 @@ func (f *AuditFormatter) FormatResponse(
 	return f.AuditFormatWriter.WriteResponse(w, respEntry)
 }
 
-// AuditRequest is the structure of a request audit log entry in Audit.
+// AuditRequestEntry is the structure of a request audit log entry in Audit.
 type AuditRequestEntry struct {
 	Time    string       `json:"time,omitempty"`
 	Type    string       `json:"type"`

audit/format_json_test.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 
 	"fmt"
+
 	"github.com/hashicorp/vault/helper/jsonutil"
 	"github.com/hashicorp/vault/helper/salt"
 	"github.com/hashicorp/vault/logical"
@@ -84,7 +85,12 @@ func TestFormatJSON_formatRequest(t *testing.T) {
 		config := FormatterConfig{
 			HMACAccessor: false,
 		}
-		if err := formatter.FormatRequest(&buf, config, tc.Auth, tc.Req, tc.Err); err != nil {
+		in := &LogInput{
+			Auth:     tc.Auth,
+			Request:  tc.Req,
+			OuterErr: tc.Err,
+		}
+		if err := formatter.FormatRequest(&buf, config, in); err != nil {
 			t.Fatalf("bad: %s\nerr: %s", name, err)
 		}
 

audit/format_jsonx_test.go

@@ -89,7 +89,12 @@ func TestFormatJSONx_formatRequest(t *testing.T) {
 			OmitTime:     true,
 			HMACAccessor: false,
 		}
-		if err := formatter.FormatRequest(&buf, config, tc.Auth, tc.Req, tc.Err); err != nil {
+		in := &LogInput{
+			Auth:     tc.Auth,
+			Request:  tc.Req,
+			OuterErr: tc.Err,
+		}
+		if err := formatter.FormatRequest(&buf, config, in); err != nil {
 			t.Fatalf("bad: %s\nerr: %s", name, err)
 		}
 

audit/format_test.go

@@ -40,10 +40,14 @@ func TestFormatRequestErrors(t *testing.T) {
 		AuditFormatWriter: &noopFormatWriter{},
 	}
 
-	if err := formatter.FormatRequest(ioutil.Discard, config, nil, nil, nil); err == nil {
+	if err := formatter.FormatRequest(ioutil.Discard, config, &LogInput{}); err == nil {
 		t.Fatal("expected error due to nil request")
 	}
-	if err := formatter.FormatRequest(nil, config, nil, &logical.Request{}, nil); err == nil {
+
+	in := &LogInput{
+		Request: &logical.Request{},
+	}
+	if err := formatter.FormatRequest(nil, config, in); err == nil {
 		t.Fatal("expected error due to nil writer")
 	}
 }
@@ -54,10 +58,14 @@ func TestFormatResponseErrors(t *testing.T) {
 		AuditFormatWriter: &noopFormatWriter{},
 	}
 
-	if err := formatter.FormatResponse(ioutil.Discard, config, nil, nil, nil, nil); err == nil {
+	if err := formatter.FormatResponse(ioutil.Discard, config, &LogInput{}); err == nil {
 		t.Fatal("expected error due to nil request")
 	}
-	if err := formatter.FormatResponse(nil, config, nil, &logical.Request{}, nil, nil); err == nil {
+
+	in := &LogInput{
+		Request: &logical.Request{},
+	}
+	if err := formatter.FormatResponse(nil, config, in); err == nil {
 		t.Fatal("expected error due to nil writer")
 	}
 }

audit/formatter.go

@@ -2,8 +2,6 @@ package audit
 
 import (
 	"io"
-
-	"github.com/hashicorp/vault/logical"
 )
 
 // Formatter is an interface that is responsible for formating a
@@ -12,8 +10,8 @@ import (
 //
 // It is recommended that you pass data through Hash prior to formatting it.
 type Formatter interface {
-	FormatRequest(io.Writer, FormatterConfig, *logical.Auth, *logical.Request, error) error
-	FormatResponse(io.Writer, FormatterConfig, *logical.Auth, *logical.Request, *logical.Response, error) error
+	FormatRequest(io.Writer, FormatterConfig, *LogInput) error
+	FormatResponse(io.Writer, FormatterConfig, *LogInput) error
 }
 
 type FormatterConfig struct {