From d35455c87d658bf7607d46aa6093e3c7601578df Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 12:46:10 +0100 Subject: [PATCH 1/9] rewrite audit entry formatting to improve performance --- audit/backend_noop.go | 2 +- audit/entry_formatter.go | 722 ++++++++++++++++------------------ audit/entry_formatter_test.go | 132 ++++--- audit/event.go | 17 + audit/hashstructure.go | 36 +- audit/hashstructure_test.go | 157 ++++---- audit/types.go | 36 +- 7 files changed, 550 insertions(+), 552 deletions(-) diff --git a/audit/backend_noop.go b/audit/backend_noop.go index 3d8862c89161..357d046b6085 100644 --- a/audit/backend_noop.go +++ b/audit/backend_noop.go @@ -244,7 +244,7 @@ func (n *noopWrapper) Process(ctx context.Context, e *eventlogger.Event) (*event // formatted headers that would have made it to the logs via the sink node. // They only appear in requests. if a.Subtype == RequestType { - reqEntry := &RequestEntry{} + reqEntry := &Entry{} err = json.Unmarshal(b, &reqEntry) if err != nil { return nil, fmt.Errorf("unable to parse formatted audit entry data: %w", err) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index ebf53088ef5c..28400cf03a6a 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -5,8 +5,6 @@ package audit import ( "context" - "crypto/tls" - "errors" "fmt" "reflect" "runtime/debug" @@ -22,6 +20,7 @@ import ( "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/logical" "github.com/jefferai/jsonx" + "github.com/mitchellh/copystructure" ) var _ eventlogger.Node = (*entryFormatter)(nil) @@ -100,7 +99,7 @@ func (f *entryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * } if a.Data == nil { - return nil, fmt.Errorf("cannot audit event (%s) with no data: %w", a.Subtype, ErrInvalidParameter) + return nil, fmt.Errorf("cannot audit a '%s' event with no data: %w", a.Subtype, ErrInvalidParameter) } // Handle panics @@ -110,8 +109,13 @@ func (f *entryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * return } + path := "unknown" + if a.Data.Request != nil { + path = a.Data.Request.Path + } + f.logger.Error("panic during logging", - "request_path", a.Data.Request.Path, + "request_path", path, "audit_device_path", f.name, "error", r, "stacktrace", string(debug.Stack())) @@ -120,46 +124,12 @@ func (f *entryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * retErr = multierror.Append(retErr, fmt.Errorf("panic generating audit log: %q", f.name)).ErrorOrNil() }() - // Take a copy of the event data before we modify anything. - data, err := a.Data.Clone() - if err != nil { - return nil, fmt.Errorf("unable to clone audit event data: %w", err) - } - - // If the request is present in the input data, apply header configuration - // regardless. We shouldn't be in a situation where the header formatter isn't - // present as it's required. - if data.Request != nil { - // Ensure that any headers in the request, are formatted as required, and are - // only present if they have been configured to appear in the audit log. - // e.g. via: /sys/config/auditing/request-headers/:name - data.Request.Headers, err = f.config.headerFormatter.ApplyConfig(ctx, data.Request.Headers, f.salter) - if err != nil { - return nil, fmt.Errorf("unable to transform headers for auditing: %w", err) - } - } - - // If the request contains a Server-Side Consistency Token (SSCT), and we - // have an auth response, overwrite the existing client token with the SSCT, - // so that the SSCT appears in the audit log for this entry. - if data.Request != nil && data.Request.InboundSSCToken != "" && data.Auth != nil { - data.Auth.ClientToken = data.Request.InboundSSCToken - } - - // Using 'any' as we have two different types that we can get back from either - // formatRequest or formatResponse, but the JSON encoder doesn't care about types. + // Using 'any' to make exclusion easier, the JSON encoder doesn't care about types. var entry any - - switch a.Subtype { - case RequestType: - entry, err = f.formatRequest(ctx, data, a) - case ResponseType: - entry, err = f.formatResponse(ctx, data, a) - default: - return nil, fmt.Errorf("unknown audit event subtype: %q", a.Subtype) - } + var err error + entry, err = f.createEntry(ctx, a) if err != nil { - return nil, fmt.Errorf("unable to parse %s from audit event: %w", a.Subtype, err) + return nil, err } // If this pipeline has been configured with (Enterprise-only) exclusions then @@ -197,21 +167,12 @@ func (f *entryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * result = append([]byte(f.config.prefix), result...) } - // Copy some properties from the event (and audit event) and store the - // format for the next (sink) node to Process. - a2 := &AuditEvent{ - ID: a.ID, - Version: a.Version, - Subtype: a.Subtype, - Timestamp: a.Timestamp, - Data: data, // Use the cloned data here rather than a pointer to the original. - } - + // Create a new event, so we can store our formatted data without conflict. e2 := &eventlogger.Event{ Type: e.Type, CreatedAt: e.CreatedAt, Formatted: make(map[string][]byte), // we are about to set this ourselves. - Payload: a2, + Payload: a, } e2.FormattedAs(f.config.requiredFormat.String(), result) @@ -219,117 +180,121 @@ func (f *entryFormatter) Process(ctx context.Context, e *eventlogger.Event) (_ * return e2, nil } -// formatRequest attempts to format the specified logical.LogInput into a RequestEntry. -func (f *entryFormatter) formatRequest(ctx context.Context, in *logical.LogInput, provider timeProvider) (*RequestEntry, error) { - switch { - case in == nil || in.Request == nil: - return nil, errors.New("request to request-audit a nil request") - case f.salter == nil: - return nil, errors.New("salt func not configured") +// remoteAddr safely gets the remote address avoiding a nil pointer. +func remoteAddr(req *logical.Request) string { + if req != nil && req.Connection != nil { + return req.Connection.RemoteAddr } + return "" +} - // Set these to the input values at first - auth := in.Auth - req := in.Request - var connState *tls.ConnectionState - if auth == nil { - auth = new(logical.Auth) +// remotePort safely gets the remote port avoiding a nil pointer. +func remotePort(req *logical.Request) int { + if req != nil && req.Connection != nil { + return req.Connection.RemotePort } + return 0 +} - if in.Request.Connection != nil && in.Request.Connection.ConnState != nil { - connState = in.Request.Connection.ConnState +// clientCertSerialNumber attempts the retrieve the serial number of the peer +// certificate from the specified tls.ConnectionState. +func clientCertSerialNumber(req *logical.Request) string { + if req == nil { + return "" } - if !f.config.raw { - var err error - err = hashAuth(ctx, f.salter, auth, f.config.hmacAccessor) - if err != nil { - return nil, err - } + if req.Connection == nil { + return "" + } - err = hashRequest(ctx, f.salter, req, f.config.hmacAccessor, in.NonHMACReqDataKeys) - if err != nil { - return nil, err - } + connState := req.Connection.ConnState + + if connState == nil || len(connState.VerifiedChains) == 0 || len(connState.VerifiedChains[0]) == 0 { + return "" } - var errString string - if in.OuterErr != nil { - errString = in.OuterErr.Error() + return connState.VerifiedChains[0][0].SerialNumber.String() +} + +// parseVaultTokenFromJWT returns a string iff the token was a JWT, and we could +// extract the original token ID from inside +func parseVaultTokenFromJWT(token string) *string { + if strings.Count(token, ".") != 2 { + return nil } - ns, err := namespace.FromContext(ctx) + parsedJWT, err := jwt.ParseSigned(token) if err != nil { - return nil, err + return nil } - reqType := in.Type - if reqType == "" { - reqType = "request" - } - reqEntry := &RequestEntry{ - Type: reqType, - Error: errString, - ForwardedFrom: req.ForwardedFrom, - Auth: &Auth{ - ClientToken: auth.ClientToken, - Accessor: auth.Accessor, - DisplayName: auth.DisplayName, - Policies: auth.Policies, - TokenPolicies: auth.TokenPolicies, - IdentityPolicies: auth.IdentityPolicies, - ExternalNamespacePolicies: auth.ExternalNamespacePolicies, - NoDefaultPolicy: auth.NoDefaultPolicy, - Metadata: auth.Metadata, - EntityID: auth.EntityID, - RemainingUses: req.ClientTokenRemainingUses, - TokenType: auth.TokenType.String(), - TokenTTL: int64(auth.TTL.Seconds()), - }, + var claims jwt.Claims + if err = parsedJWT.UnsafeClaimsWithoutVerification(&claims); err != nil { + return nil + } - Request: &Request{ - ID: req.ID, - ClientID: req.ClientID, - ClientToken: req.ClientToken, - ClientTokenAccessor: req.ClientTokenAccessor, - Operation: req.Operation, - MountPoint: req.MountPoint, - MountType: req.MountType, - MountAccessor: req.MountAccessor, - MountRunningVersion: req.MountRunningVersion(), - MountRunningSha256: req.MountRunningSha256(), - MountIsExternalPlugin: req.MountIsExternalPlugin(), - MountClass: req.MountClass(), - Namespace: &Namespace{ - ID: ns.ID, - Path: ns.Path, - }, - Path: req.Path, - Data: req.Data, - PolicyOverride: req.PolicyOverride, - RemoteAddr: getRemoteAddr(req), - RemotePort: getRemotePort(req), - ReplicationCluster: req.ReplicationCluster, - Headers: req.Headers, - ClientCertificateSerialNumber: getClientCertificateSerialNumber(connState), - }, + return &claims.ID +} + +// newTemporaryEntryFormatter creates a cloned entryFormatter instance with a non-persistent Salter. +func newTemporaryEntryFormatter(n *entryFormatter) *entryFormatter { + return &entryFormatter{ + salter: &nonPersistentSalt{}, + config: n.config, } +} - if req.HTTPRequest != nil && req.HTTPRequest.RequestURI != req.Path { - reqEntry.Request.RequestURI = req.HTTPRequest.RequestURI +// Salt returns a new salt with default configuration and no storage usage, and no error. +func (s *nonPersistentSalt) Salt(_ context.Context) (*salt.Salt, error) { + return salt.NewNonpersistentSalt(), nil +} + +// clone can be used to deep clone the specified type. +func clone[V any](s V) (V, error) { + s2, err := copystructure.Copy(s) + + return s2.(V), err +} + +// newAuth takes a logical.Auth and the number of remaining client token uses +// (which should be supplied from the logical.Request's client token), and creates +// an audit Auth. +// tokenRemainingUses should be the client token remaining uses to include in auth. +// This usually can be found in logical.Request.ClientTokenRemainingUses. +func newAuth(auth *logical.Auth, tokenRemainingUses int) (*Auth, error) { + if auth == nil { + return nil, nil } - if !auth.IssueTime.IsZero() { - reqEntry.Auth.TokenIssueTime = auth.IssueTime.Format(time.RFC3339) + extNSPolicies, err := clone(auth.ExternalNamespacePolicies) + if err != nil { + return nil, fmt.Errorf("unable to clone logical auth: external namespace policies: %w", err) + } + + identityPolicies, err := clone(auth.IdentityPolicies) + if err != nil { + return nil, fmt.Errorf("unable to clone logical auth: identity policies: %w", err) + } + + metadata, err := clone(auth.Metadata) + if err != nil { + return nil, fmt.Errorf("unable to clone logical auth: metadata: %w", err) } + policies, err := clone(auth.Policies) + if err != nil { + return nil, fmt.Errorf("unable to clone logical auth: policies: %w", err) + } + + var policyResults *PolicyResults if auth.PolicyResults != nil { - reqEntry.Auth.PolicyResults = &PolicyResults{ - Allowed: auth.PolicyResults.Allowed, + policyResults = &PolicyResults{ + Allowed: auth.PolicyResults.Allowed, + GrantingPolicies: make([]PolicyInfo, len(auth.PolicyResults.GrantingPolicies)), } for _, p := range auth.PolicyResults.GrantingPolicies { - reqEntry.Auth.PolicyResults.GrantingPolicies = append(reqEntry.Auth.PolicyResults.GrantingPolicies, PolicyInfo{ + policyResults.GrantingPolicies = append(policyResults.GrantingPolicies, PolicyInfo{ Name: p.Name, NamespaceId: p.NamespaceId, NamespacePath: p.NamespacePath, @@ -338,127 +303,176 @@ func (f *entryFormatter) formatRequest(ctx context.Context, in *logical.LogInput } } - if req.WrapInfo != nil { - reqEntry.Request.WrapTTL = int(req.WrapInfo.TTL / time.Second) + tokenPolicies, err := clone(auth.TokenPolicies) + if err != nil { + return nil, fmt.Errorf("unable to clone logical auth: token policies: %w", err) } - if !f.config.omitTime { - // Use the time provider to supply the time for this entry. - reqEntry.Time = provider.formattedTime() + var tokenIssueTime string + if !auth.IssueTime.IsZero() { + tokenIssueTime = auth.IssueTime.Format(time.RFC3339) + } + + return &Auth{ + Accessor: auth.Accessor, + ClientToken: auth.ClientToken, + DisplayName: auth.DisplayName, + EntityCreated: auth.EntityCreated, + EntityID: auth.EntityID, + ExternalNamespacePolicies: extNSPolicies, + IdentityPolicies: identityPolicies, + Metadata: metadata, + NoDefaultPolicy: auth.NoDefaultPolicy, + NumUses: auth.NumUses, + Policies: policies, + PolicyResults: policyResults, + RemainingUses: tokenRemainingUses, + TokenPolicies: tokenPolicies, + TokenIssueTime: tokenIssueTime, + TokenTTL: int64(auth.TTL.Seconds()), + TokenType: auth.TokenType.String(), + }, nil +} + +// newRequest takes a logical.Request and namespace.Namespace, transforms and +// aggregates them into an audit Request. +func newRequest(req *logical.Request, ns *namespace.Namespace) (*Request, error) { + if req == nil { + return nil, nil } - return reqEntry, nil -} + remoteAddr := remoteAddr(req) + remotePort := remotePort(req) + clientCertSerial := clientCertSerialNumber(req) -// formatResponse attempts to format the specified logical.LogInput into a ResponseEntry. -func (f *entryFormatter) formatResponse(ctx context.Context, in *logical.LogInput, provider timeProvider) (*ResponseEntry, error) { - switch { - case f == nil: - return nil, errors.New("formatter is nil") - case in == nil || in.Request == nil: - return nil, errors.New("request to response-audit a nil request") - case f.salter == nil: - return nil, errors.New("salt func not configured") + data, err := clone(req.Data) + if err != nil { + return nil, fmt.Errorf("unable to clone logical request: data: %w", err) } - // Set these to the input values at first - auth, req, resp := in.Auth, in.Request, in.Response - if auth == nil { - auth = new(logical.Auth) + headers, err := clone(req.Headers) + if err != nil { + return nil, fmt.Errorf("unable to clone logical request: headers: %w", err) } + + var reqURI string + if req.HTTPRequest != nil && req.HTTPRequest.RequestURI != req.Path { + reqURI = req.HTTPRequest.RequestURI + } + var wrapTTL int + if req.WrapInfo != nil { + wrapTTL = int(req.WrapInfo.TTL / time.Second) + } + + return &Request{ + ClientCertificateSerialNumber: clientCertSerial, + ClientID: req.ClientID, + ClientToken: req.ClientToken, + ClientTokenAccessor: req.ClientTokenAccessor, + Data: data, + Headers: headers, + ID: req.ID, + MountAccessor: req.MountAccessor, + MountClass: req.MountClass(), + MountIsExternalPlugin: req.MountIsExternalPlugin(), + MountPoint: req.MountPoint, + MountRunningSha256: req.MountRunningSha256(), + MountRunningVersion: req.MountRunningVersion(), + MountType: req.MountType, + Namespace: &Namespace{ + ID: ns.ID, + Path: ns.Path, + }, + Operation: req.Operation, + Path: req.Path, + PolicyOverride: req.PolicyOverride, + RemoteAddr: remoteAddr, + RemotePort: remotePort, + ReplicationCluster: req.ReplicationCluster, + RequestURI: reqURI, + WrapTTL: wrapTTL, + }, nil +} + +// newResponse takes a logical.Response and logical.Request, transforms and +// aggregates them into an audit Response. +// isElisionRequired is used to indicate that response 'Data' should be elided. +func newResponse(resp *logical.Response, req *logical.Request, isElisionRequired bool) (*Response, error) { if resp == nil { - resp = new(logical.Response) + return nil, nil } - var connState *tls.ConnectionState - if in.Request.Connection != nil && in.Request.Connection.ConnState != nil { - connState = in.Request.Connection.ConnState + if req == nil { + // Request should never be nil, even for a response. + return nil, fmt.Errorf("request is nil") + } + + auth, err := newAuth(resp.Auth, req.ClientTokenRemainingUses) + if err != nil { + return nil, fmt.Errorf("unable to convert logical auth response: %w", err) } - elideListResponseData := f.config.elideListResponses && req.Operation == logical.ListOperation + var data map[string]any + if resp.Data != nil { + data = make(map[string]any, len(resp.Data)) - var respData map[string]interface{} - if f.config.raw { - // In the non-raw case, elision of list response data occurs inside hashResponse, to avoid redundant deep - // copies and hashing of data only to elide it later. In the raw case, we need to do it here. - if elideListResponseData && resp.Data != nil { - // Copy the data map before making changes, but we only need to go one level deep in this case - respData = make(map[string]interface{}, len(resp.Data)) + if isElisionRequired { + // Performs the actual elision (ideally for list operations) of response data, + // once surrounding code has determined it should apply to a particular request. + // If the value for a key should not be elided, then it will be cloned. for k, v := range resp.Data { - respData[k] = v + isCloneRequired := true + switch k { + case "keys": + if vSlice, ok := v.([]string); ok { + data[k] = len(vSlice) + isCloneRequired = false + } + case "key_info": + if vMap, ok := v.(map[string]any); ok { + data[k] = len(vMap) + isCloneRequired = false + } + } + + // Clone values if they weren't legitimate keys or key_info. + if isCloneRequired { + v2, err := clone(v) + if err != nil { + return nil, fmt.Errorf("unable to clone response data while eliding: %w", err) + } + data[k] = v2 + } } - - doElideListResponseData(respData) } else { - respData = resp.Data - } - } else { - var err error - err = hashAuth(ctx, f.salter, auth, f.config.hmacAccessor) - if err != nil { - return nil, err - } - - err = hashRequest(ctx, f.salter, req, f.config.hmacAccessor, in.NonHMACReqDataKeys) - if err != nil { - return nil, err - } - - err = hashResponse(ctx, f.salter, resp, f.config.hmacAccessor, in.NonHMACRespDataKeys, elideListResponseData) - if err != nil { - return nil, err + // Deep clone all values, no shortcuts here. + data, err = clone(resp.Data) + if err != nil { + return nil, fmt.Errorf("unable to clone response data: %w", err) + } } - - respData = resp.Data } - var errString string - if in.OuterErr != nil { - errString = in.OuterErr.Error() - } - - ns, err := namespace.FromContext(ctx) + headers, err := clone(resp.Headers) if err != nil { - return nil, err - } - - var respAuth *Auth - if resp.Auth != nil { - respAuth = &Auth{ - ClientToken: resp.Auth.ClientToken, - Accessor: resp.Auth.Accessor, - DisplayName: resp.Auth.DisplayName, - Policies: resp.Auth.Policies, - TokenPolicies: resp.Auth.TokenPolicies, - IdentityPolicies: resp.Auth.IdentityPolicies, - ExternalNamespacePolicies: resp.Auth.ExternalNamespacePolicies, - NoDefaultPolicy: resp.Auth.NoDefaultPolicy, - Metadata: resp.Auth.Metadata, - NumUses: resp.Auth.NumUses, - EntityID: resp.Auth.EntityID, - TokenType: resp.Auth.TokenType.String(), - TokenTTL: int64(resp.Auth.TTL.Seconds()), - } - if !resp.Auth.IssueTime.IsZero() { - respAuth.TokenIssueTime = resp.Auth.IssueTime.Format(time.RFC3339) - } + return nil, fmt.Errorf("unable to clone logical response: headers: %w", err) } - var respSecret *Secret + var secret *Secret if resp.Secret != nil { - respSecret = &Secret{ - LeaseID: resp.Secret.LeaseID, - } + secret = &Secret{LeaseID: resp.Secret.LeaseID} } - var respWrapInfo *ResponseWrapInfo + var wrapInfo *ResponseWrapInfo if resp.WrapInfo != nil { token := resp.WrapInfo.Token if jwtToken := parseVaultTokenFromJWT(token); jwtToken != nil { token = *jwtToken } - respWrapInfo = &ResponseWrapInfo{ - TTL: int(resp.WrapInfo.TTL / time.Second), + + ttl := int(resp.WrapInfo.TTL / time.Second) + wrapInfo = &ResponseWrapInfo{ + TTL: ttl, Token: token, Accessor: resp.WrapInfo.Accessor, CreationTime: resp.WrapInfo.CreationTime.UTC().Format(time.RFC3339Nano), @@ -467,184 +481,134 @@ func (f *entryFormatter) formatResponse(ctx context.Context, in *logical.LogInpu } } - respType := in.Type - if respType == "" { - respType = "response" - } - respEntry := &ResponseEntry{ - Type: respType, - Error: errString, - Forwarded: req.ForwardedFrom != "", - Auth: &Auth{ - ClientToken: auth.ClientToken, - Accessor: auth.Accessor, - DisplayName: auth.DisplayName, - Policies: auth.Policies, - TokenPolicies: auth.TokenPolicies, - IdentityPolicies: auth.IdentityPolicies, - ExternalNamespacePolicies: auth.ExternalNamespacePolicies, - NoDefaultPolicy: auth.NoDefaultPolicy, - Metadata: auth.Metadata, - RemainingUses: req.ClientTokenRemainingUses, - EntityID: auth.EntityID, - EntityCreated: auth.EntityCreated, - TokenType: auth.TokenType.String(), - TokenTTL: int64(auth.TTL.Seconds()), - }, - - Request: &Request{ - ID: req.ID, - ClientToken: req.ClientToken, - ClientTokenAccessor: req.ClientTokenAccessor, - ClientID: req.ClientID, - Operation: req.Operation, - MountPoint: req.MountPoint, - MountType: req.MountType, - MountAccessor: req.MountAccessor, - MountRunningVersion: req.MountRunningVersion(), - MountRunningSha256: req.MountRunningSha256(), - MountIsExternalPlugin: req.MountIsExternalPlugin(), - MountClass: req.MountClass(), - Namespace: &Namespace{ - ID: ns.ID, - Path: ns.Path, - }, - Path: req.Path, - Data: req.Data, - PolicyOverride: req.PolicyOverride, - RemoteAddr: getRemoteAddr(req), - RemotePort: getRemotePort(req), - ClientCertificateSerialNumber: getClientCertificateSerialNumber(connState), - ReplicationCluster: req.ReplicationCluster, - Headers: req.Headers, - }, + warnings, err := clone(resp.Warnings) + if err != nil { + return nil, fmt.Errorf("unable to clone logical response: warnings: %w", err) + } + + return &Response{ + Auth: auth, + Data: data, + Headers: headers, + MountAccessor: req.MountAccessor, + MountClass: req.MountClass(), + MountIsExternalPlugin: req.MountIsExternalPlugin(), + MountPoint: req.MountPoint, + MountRunningSha256: req.MountRunningSha256(), + MountRunningVersion: req.MountRunningVersion(), + MountType: req.MountType, + Redirect: resp.Redirect, + Secret: secret, + WrapInfo: wrapInfo, + Warnings: warnings, + }, nil +} - Response: &Response{ - MountPoint: req.MountPoint, - MountType: req.MountType, - MountAccessor: req.MountAccessor, - MountRunningVersion: req.MountRunningVersion(), - MountRunningSha256: req.MountRunningSha256(), - MountIsExternalPlugin: req.MountIsExternalPlugin(), - MountClass: req.MountClass(), - Auth: respAuth, - Secret: respSecret, - Data: respData, - Warnings: resp.Warnings, - Redirect: resp.Redirect, - WrapInfo: respWrapInfo, - Headers: resp.Headers, - }, - } +// createEntry takes the AuditEvent and builds an audit Entry. +// The Entry will be HMAC'd and elided where required. +func (f *entryFormatter) createEntry(ctx context.Context, a *AuditEvent) (*Entry, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: - if req.HTTPRequest != nil && req.HTTPRequest.RequestURI != req.Path { - respEntry.Request.RequestURI = req.HTTPRequest.RequestURI } - if auth.PolicyResults != nil { - respEntry.Auth.PolicyResults = &PolicyResults{ - Allowed: auth.PolicyResults.Allowed, - } + data := a.Data - for _, p := range auth.PolicyResults.GrantingPolicies { - respEntry.Auth.PolicyResults.GrantingPolicies = append(respEntry.Auth.PolicyResults.GrantingPolicies, PolicyInfo{ - Name: p.Name, - NamespaceId: p.NamespaceId, - NamespacePath: p.NamespacePath, - Type: p.Type, - }) - } + if data.Request == nil { + // Request should never be nil, even for a response. + return nil, fmt.Errorf("unable to parse request from '%s' audit event: request cannot be nil", a.Subtype) } - if !auth.IssueTime.IsZero() { - respEntry.Auth.TokenIssueTime = auth.IssueTime.Format(time.RFC3339) - } - if req.WrapInfo != nil { - respEntry.Request.WrapTTL = int(req.WrapInfo.TTL / time.Second) + ns, err := namespace.FromContext(ctx) + if err != nil { + return nil, fmt.Errorf("unable to retrieve namespace from context: %w", err) } - if !f.config.omitTime { - // Use the time provider to supply the time for this entry. - respEntry.Time = provider.formattedTime() + auth, err := newAuth(data.Auth, data.Request.ClientTokenRemainingUses) + if err != nil { + return nil, fmt.Errorf("cannot convert auth: %w", err) } - return respEntry, nil -} + req, err := newRequest(data.Request, ns) + if err != nil { + return nil, fmt.Errorf("cannot convert request: %w", err) + } -// getRemoteAddr safely gets the remote address avoiding a nil pointer -func getRemoteAddr(req *logical.Request) string { - if req != nil && req.Connection != nil { - return req.Connection.RemoteAddr + var resp *Response + if a.Subtype == ResponseType { + shouldElide := f.config.elideListResponses && req.Operation == logical.ListOperation + resp, err = newResponse(data.Response, data.Request, shouldElide) + if err != nil { + return nil, fmt.Errorf("cannot convert request: %w", err) + } } - return "" -} -// getRemotePort safely gets the remote port avoiding a nil pointer -func getRemotePort(req *logical.Request) int { - if req != nil && req.Connection != nil { - return req.Connection.RemotePort + var outerErr string + if data.OuterErr != nil { + outerErr = data.OuterErr.Error() } - return 0 -} -// getClientCertificateSerialNumber attempts the retrieve the serial number of -// the peer certificate from the specified tls.ConnectionState. -func getClientCertificateSerialNumber(connState *tls.ConnectionState) string { - if connState == nil || len(connState.VerifiedChains) == 0 || len(connState.VerifiedChains[0]) == 0 { - return "" + entryType := data.Type + if entryType == "" { + entryType = a.Subtype.String() } - return connState.VerifiedChains[0][0].SerialNumber.String() -} + entry := &Entry{ + Auth: auth, + Error: outerErr, + Forwarded: false, + ForwardedFrom: data.Request.ForwardedFrom, + Request: req, + Response: resp, + Type: entryType, + } -// parseVaultTokenFromJWT returns a string iff the token was a JWT, and we could -// extract the original token ID from inside -func parseVaultTokenFromJWT(token string) *string { - if strings.Count(token, ".") != 2 { - return nil + if !f.config.omitTime { + // Use the time provider to supply the time for this entry. + entry.Time = a.timeProvider().formattedTime() } - parsedJWT, err := jwt.ParseSigned(token) - if err != nil { - return nil + // If the request is present in the input data, apply header configuration + // regardless. We shouldn't be in a situation where the header formatter isn't + // present as it's required. + if entry.Request != nil { + // Ensure that any headers in the request, are formatted as required, and are + // only present if they have been configured to appear in the audit log. + // e.g. via: /sys/config/auditing/request-headers/:name + entry.Request.Headers, err = f.config.headerFormatter.ApplyConfig(ctx, entry.Request.Headers, f.salter) + if err != nil { + return nil, fmt.Errorf("unable to transform headers for auditing: %w", err) + } } - var claims jwt.Claims - if err = parsedJWT.UnsafeClaimsWithoutVerification(&claims); err != nil { - return nil + // If the request contains a Server-Side Consistency Token (SSCT), and we + // have an auth response, overwrite the existing client token with the SSCT, + // so that the SSCT appears in the audit log for this entry. + if data.Request != nil && data.Request.InboundSSCToken != "" && entry.Auth != nil { + entry.Auth.ClientToken = data.Request.InboundSSCToken } - return &claims.ID -} + // Hash the entry if we aren't expecting raw output. + if !f.config.raw { + // Requests and responses have auth and request. + err = hashAuth(ctx, f.salter, entry.Auth, f.config.hmacAccessor) + if err != nil { + return nil, err + } -// doElideListResponseData performs the actual elision of list operation response data, once surrounding code has -// determined it should apply to a particular request. The data map that is passed in must be a copy that is safe to -// modify in place, but need not be a full recursive deep copy, as only top-level keys are changed. -// -// See the documentation of the controlling option in formatterConfig for more information on the purpose. -func doElideListResponseData(data map[string]interface{}) { - for k, v := range data { - if k == "keys" { - if vSlice, ok := v.([]string); ok { - data[k] = len(vSlice) - } - } else if k == "key_info" { - if vMap, ok := v.(map[string]interface{}); ok { - data[k] = len(vMap) - } + err = hashRequest(ctx, f.salter, entry.Request, f.config.hmacAccessor, data.NonHMACReqDataKeys) + if err != nil { + return nil, err } - } -} -// newTemporaryEntryFormatter creates a cloned entryFormatter instance with a non-persistent Salter. -func newTemporaryEntryFormatter(n *entryFormatter) *entryFormatter { - return &entryFormatter{ - salter: &nonPersistentSalt{}, - config: n.config, + if a.Subtype == ResponseType { + if err = hashResponse(ctx, f.salter, entry.Response, f.config.hmacAccessor, data.NonHMACRespDataKeys); err != nil { + return nil, err + } + } } -} -// Salt returns a new salt with default configuration and no storage usage, and no error. -func (s *nonPersistentSalt) Salt(_ context.Context) (*salt.Salt, error) { - return salt.NewNonpersistentSalt(), nil + return entry, nil } diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 3fc088255274..56548435da3d 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/helper/testhelpers/corehelpers" "github.com/hashicorp/vault/internal/observability/event" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/helper/salt" @@ -258,42 +259,43 @@ func TestEntryFormatter_Process(t *testing.T) { }{ "json-request-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "cannot audit event (request) with no data: invalid internal parameter", + ExpectedErrorMessage: "cannot audit a 'request' event with no data: invalid internal parameter", Subtype: RequestType, RequiredFormat: JSONFormat, Data: nil, }, "json-response-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "cannot audit event (response) with no data: invalid internal parameter", + ExpectedErrorMessage: "cannot audit a 'response' event with no data: invalid internal parameter", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: nil, }, "json-request-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse request from audit event: request to request-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'request' audit event: request cannot be nil", Subtype: RequestType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Type: "magic"}, + RootNamespace: true, }, "json-response-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse response from audit event: request to response-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'response' audit event: request cannot be nil", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Type: "magic"}, }, "json-request-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse request from audit event: no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: RequestType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, }, "json-response-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse response from audit event: no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, @@ -314,42 +316,44 @@ func TestEntryFormatter_Process(t *testing.T) { }, "jsonx-request-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "cannot audit event (request) with no data: invalid internal parameter", + ExpectedErrorMessage: "cannot audit a 'request' event with no data: invalid internal parameter", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: nil, }, "jsonx-response-no-data": { IsErrorExpected: true, - ExpectedErrorMessage: "cannot audit event (response) with no data: invalid internal parameter", + ExpectedErrorMessage: "cannot audit a 'response' event with no data: invalid internal parameter", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: nil, }, "jsonx-request-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse request from audit event: request to request-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'request' audit event: request cannot be nil", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Type: "magic"}, + RootNamespace: true, }, "jsonx-response-basic-input": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse response from audit event: request to response-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'response' audit event: request cannot be nil", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Type: "magic"}, + RootNamespace: true, }, "jsonx-request-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse request from audit event: no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: RequestType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, }, "jsonx-response-basic-input-and-request-no-ns": { IsErrorExpected: true, - ExpectedErrorMessage: "unable to parse response from audit event: no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, @@ -382,7 +386,7 @@ func TestEntryFormatter_Process(t *testing.T) { cfg, err := newFormatterConfig(&testHeaderFormatter{}, map[string]string{"format": tc.RequiredFormat.String()}) require.NoError(t, err) - f, err := newEntryFormatter("juan", cfg, ss, hclog.NewNullLogger()) + f, err := newEntryFormatter("juan", cfg, ss, corehelpers.NewTestLogger(t)) require.NoError(t, err) require.NotNil(t, f) @@ -477,7 +481,7 @@ func BenchmarkAuditFileSink_Process(b *testing.B) { // TestEntryFormatter_FormatRequest exercises entryFormatter.formatRequest with // varying inputs. -func TestEntryFormatter_FormatRequest(t *testing.T) { +func TestEntryFormatter_Process_Request(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -490,17 +494,17 @@ func TestEntryFormatter_FormatRequest(t *testing.T) { "nil": { Input: nil, IsErrorExpected: true, - ExpectedErrorMessage: "request to request-audit a nil request", + ExpectedErrorMessage: "cannot audit a 'request' event with no data: invalid internal parameter", }, "basic-input": { Input: &logical.LogInput{}, IsErrorExpected: true, - ExpectedErrorMessage: "request to request-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'request' audit event: request cannot be nil", }, "input-and-request-no-ns": { Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, IsErrorExpected: true, - ExpectedErrorMessage: "no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", RootNamespace: false, }, "input-and-request-with-ns": { @@ -536,20 +540,41 @@ func TestEntryFormatter_FormatRequest(t *testing.T) { ctx = context.Background() } - entry, err := f.formatRequest(ctx, tc.Input, &testTimeProvider{}) + auditEvent, err := NewEvent(RequestType) + auditEvent.setTimeProvider(&testTimeProvider{}) + require.NoError(t, err) + auditEvent.Data = tc.Input + e := &eventlogger.Event{ + Type: event.AuditType.AsEventType(), + CreatedAt: time.Now(), + Formatted: make(map[string][]byte), + Payload: auditEvent, + } + + e2, err := f.Process(ctx, e) switch { case tc.IsErrorExpected: require.Error(t, err) require.EqualError(t, err, tc.ExpectedErrorMessage) - require.Nil(t, entry) + require.Nil(t, e2) case tc.ShouldOmitTime: require.NoError(t, err) - require.NotNil(t, entry) + require.NotNil(t, e2) + b, ok := e2.Format(JSONFormat.String()) + require.True(t, ok) + var entry *Entry + err = json.Unmarshal(b, &entry) + require.NoError(t, err) require.Zero(t, entry.Time) default: require.NoError(t, err) - require.NotNil(t, entry) + require.NotNil(t, e2) + b, ok := e2.Format(JSONFormat.String()) + require.True(t, ok) + var entry *Entry + err = json.Unmarshal(b, &entry) + require.NoError(t, err) require.NotZero(t, entry.Time) require.Equal(t, "2024-03-22T10:00:05.00000001Z", entry.Time) } @@ -572,17 +597,17 @@ func TestEntryFormatter_FormatResponse(t *testing.T) { "nil": { Input: nil, IsErrorExpected: true, - ExpectedErrorMessage: "request to response-audit a nil request", + ExpectedErrorMessage: "cannot audit a 'response' event with no data: invalid internal parameter", }, "basic-input": { Input: &logical.LogInput{}, IsErrorExpected: true, - ExpectedErrorMessage: "request to response-audit a nil request", + ExpectedErrorMessage: "unable to parse request from 'response' audit event: request cannot be nil", }, "input-and-request-no-ns": { Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, IsErrorExpected: true, - ExpectedErrorMessage: "no namespace", + ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", RootNamespace: false, }, "input-and-request-with-ns": { @@ -619,20 +644,42 @@ func TestEntryFormatter_FormatResponse(t *testing.T) { ctx = context.Background() } - entry, err := f.formatResponse(ctx, tc.Input, &testTimeProvider{}) + auditEvent, err := NewEvent(ResponseType) + auditEvent.setTimeProvider(&testTimeProvider{}) + require.NoError(t, err) + auditEvent.Data = tc.Input + + e := &eventlogger.Event{ + Type: event.AuditType.AsEventType(), + CreatedAt: time.Now(), + Formatted: make(map[string][]byte), + Payload: auditEvent, + } + + e2, err := f.Process(ctx, e) switch { case tc.IsErrorExpected: require.Error(t, err) require.EqualError(t, err, tc.ExpectedErrorMessage) - require.Nil(t, entry) + require.Nil(t, e2) case tc.ShouldOmitTime: require.NoError(t, err) - require.NotNil(t, entry) + require.NotNil(t, e2) + b, ok := e2.Format(JSONFormat.String()) + require.True(t, ok) + var entry *Entry + err = json.Unmarshal(b, &entry) + require.NoError(t, err) require.Zero(t, entry.Time) default: require.NoError(t, err) - require.NotNil(t, entry) + require.NotNil(t, e2) + b, ok := e2.Format(JSONFormat.String()) + require.True(t, ok) + var entry *Entry + err = json.Unmarshal(b, &entry) + require.NoError(t, err) require.NotZero(t, entry.Time) require.Equal(t, "2024-03-22T10:00:05.00000001Z", entry.Time) } @@ -760,14 +807,14 @@ func TestEntryFormatter_Process_JSON(t *testing.T) { t.Fatalf("no prefix: %s \n log: %s\nprefix: %s", name, expectedResultStr, tc.Prefix) } - expectedJSON := new(RequestEntry) + expectedJSON := new(Entry) if err := jsonutil.DecodeJSON([]byte(expectedResultStr), &expectedJSON); err != nil { t.Fatalf("bad json: %s", err) } expectedJSON.Request.Namespace = &Namespace{ID: "root"} - actualJSON := new(RequestEntry) + actualJSON := new(Entry) if err := jsonutil.DecodeJSON(jsonBytes[len(tc.Prefix):], &actualJSON); err != nil { t.Fatalf("bad json: %s", err) } @@ -1006,7 +1053,7 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { var formatter *entryFormatter var err error - format := func(t *testing.T, config formatterConfig, operation logical.Operation, inputData map[string]any) *ResponseEntry { + format := func(t *testing.T, config formatterConfig, operation logical.Operation, inputData map[string]any) *Entry { formatter, err = newEntryFormatter("juan", config, ss, hclog.NewNullLogger()) require.NoError(t, err) require.NotNil(t, formatter) @@ -1016,10 +1063,15 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { Response: &logical.Response{Data: inputData}, } - resp, err := formatter.formatResponse(ctx, in, &testTimeProvider{}) + auditEvent, err := NewEvent(ResponseType) + require.NoError(t, err) + auditEvent.Data = in + + entry, err := formatter.createEntry(ctx, auditEvent) require.NoError(t, err) + require.NotNil(t, entry) - return resp + return entry } t.Run("Default case", func(t *testing.T) { @@ -1040,7 +1092,8 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { require.NoError(t, err) tc := oneInterestingTestCase entry := format(t, config, logical.ReadOperation, tc.inputData) - assert.Equal(t, formatter.hashExpectedValueForComparison(tc.inputData), entry.Response.Data) + hashedExpected := formatter.hashExpectedValueForComparison(tc.inputData) + assert.Equal(t, hashedExpected, entry.Response.Data) }) t.Run("When elideListResponses is false, eliding does not happen", func(t *testing.T) { @@ -1113,17 +1166,6 @@ func TestEntryFormatter_Process_NoMutation(t *testing.T) { // Ensure the pointers are different. require.NotEqual(t, e2, e) - - // Do the same for the audit event in the payload. - a, ok := e.Payload.(*AuditEvent) - require.True(t, ok) - require.NotNil(t, a) - - a2, ok := e2.Payload.(*AuditEvent) - require.True(t, ok) - require.NotNil(t, a2) - - require.NotEqual(t, a2, a) } // TestEntryFormatter_Process_Panic tries to send data into the entryFormatter diff --git a/audit/event.go b/audit/event.go index f3d0d3cbcd0e..fcf9fd055b68 100644 --- a/audit/event.go +++ b/audit/event.go @@ -37,6 +37,23 @@ type AuditEvent struct { Subtype subtype `json:"subtype"` // the subtype of the audit event. Timestamp time.Time `json:"timestamp"` Data *logical.LogInput `json:"data"` + prov timeProvider +} + +// setTimeProvider can be used to set a specific time provider which is used when +// creating an Entry. +// NOTE: This is primarily used for testing to supply a known time value. +func (a *AuditEvent) setTimeProvider(t timeProvider) { + a.prov = t +} + +// timeProvider returns a configured time provider, or the default if not set. +func (a *AuditEvent) timeProvider() timeProvider { + if a.prov == nil { + return a + } + + return a.prov } // format defines types of format audit events support. diff --git a/audit/hashstructure.go b/audit/hashstructure.go index 6a1190801026..7f0dae641eb0 100644 --- a/audit/hashstructure.go +++ b/audit/hashstructure.go @@ -11,7 +11,6 @@ import ( "time" "github.com/hashicorp/go-secure-stdlib/strutil" - "github.com/hashicorp/vault/sdk/helper/wrapping" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/reflectwalk" ) @@ -29,7 +28,7 @@ func hashString(ctx context.Context, salter Salter, data string) (string, error) // hashAuth uses the Salter to hash the supplied Auth (modifying it). // hmacAccessor is used to indicate whether the accessor should also be HMAC'd // when present. -func hashAuth(ctx context.Context, salter Salter, auth *logical.Auth, hmacAccessor bool) error { +func hashAuth(ctx context.Context, salter Salter, auth *Auth, hmacAccessor bool) error { if auth == nil { return nil } @@ -56,7 +55,9 @@ func hashAuth(ctx context.Context, salter Salter, auth *logical.Auth, hmacAccess // prevents those specific keys from HMAC'd. // hmacAccessor is used to indicate whether some accessors should also be HMAC'd // when present. -func hashRequest(ctx context.Context, salter Salter, req *logical.Request, hmacAccessor bool, nonHMACDataKeys []string) error { +// nonHMACDataKeys is used when hashing any 'Data' field within the Request which +// prevents those specific keys from HMAC'd. +func hashRequest(ctx context.Context, salter Salter, req *Request, hmacAccessor bool, nonHMACDataKeys []string) error { if req == nil { return nil } @@ -68,11 +69,6 @@ func hashRequest(ctx context.Context, salter Salter, req *logical.Request, hmacA fn := salt.GetIdentifiedHMAC - err = hashAuth(ctx, salter, req.Auth, hmacAccessor) - if err != nil { - return err - } - if req.ClientToken != "" { req.ClientToken = fn(req.ClientToken) } @@ -107,14 +103,12 @@ func hashMap(hashFunc hashCallback, data map[string]interface{}, nonHMACDataKeys } // hashResponse uses the Salter to hash the supplied Response (modifying it). -// nonHMACDataKeys is used when hashing any 'Data' field within the Request which -// prevents those specific keys from HMAC'd. // hmacAccessor is used to indicate whether some accessors should also be HMAC'd // when present. -// elideListResponseData indicates whether any 'keys' or 'key_info' data present in -// the Response should be elided (when the request was a LIST operation). +// nonHMACDataKeys is used when hashing any 'Data' field within the Response which +// prevents those specific keys from HMAC'd. // See: /vault/docs/audit#eliding-list-response-bodies -func hashResponse(ctx context.Context, salter Salter, resp *logical.Response, hmacAccessor bool, nonHMACDataKeys []string, elideListResponseData bool) error { +func hashResponse(ctx context.Context, salter Salter, resp *Response, hmacAccessor bool, nonHMACDataKeys []string) error { if resp == nil { return nil } @@ -126,23 +120,11 @@ func hashResponse(ctx context.Context, salter Salter, resp *logical.Response, hm fn := salt.GetIdentifiedHMAC - err = hashAuth(ctx, salter, resp.Auth, hmacAccessor) - if err != nil { - return err - } - if resp.Data != nil { if b, ok := resp.Data[logical.HTTPRawBody].([]byte); ok { resp.Data[logical.HTTPRawBody] = string(b) } - // Processing list response data elision takes place at this point in the code for performance reasons: - // - take advantage of the deep copy of resp.Data that was going to be done anyway for hashing - // - but elide data before potentially spending time hashing it - if elideListResponseData { - doElideListResponseData(resp.Data) - } - err = hashMap(fn, resp.Data, nonHMACDataKeys) if err != nil { return err @@ -160,12 +142,10 @@ func hashResponse(ctx context.Context, salter Salter, resp *logical.Response, hm return nil } -// hashWrapInfo returns a hashed copy of the ResponseWrapInfo input. - // hashWrapInfo uses the supplied hashing function to hash ResponseWrapInfo (modifying it). // hmacAccessor is used to indicate whether some accessors should also be HMAC'd // when present. -func hashWrapInfo(hashFunc hashCallback, wrapInfo *wrapping.ResponseWrapInfo, hmacAccessor bool) error { +func hashWrapInfo(hashFunc hashCallback, wrapInfo *ResponseWrapInfo, hmacAccessor bool) error { if wrapInfo == nil { return nil } diff --git a/audit/hashstructure_test.go b/audit/hashstructure_test.go index 6c70f82ebcdd..653b31d80fff 100644 --- a/audit/hashstructure_test.go +++ b/audit/hashstructure_test.go @@ -7,12 +7,11 @@ import ( "context" "crypto/sha256" "encoding/json" - "fmt" "reflect" "testing" "time" - "github.com/go-test/deep" + "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/helper/certutil" "github.com/hashicorp/vault/sdk/helper/salt" "github.com/hashicorp/vault/sdk/helper/wrapping" @@ -135,34 +134,42 @@ func TestHashString(t *testing.T) { func TestHashAuth(t *testing.T) { cases := map[string]struct { Input *logical.Auth - Output *logical.Auth + Output *Auth HMACAccessor bool }{ "no-accessor-hmac": { &logical.Auth{ ClientToken: "foo", Accessor: "very-accessible", + LeaseOptions: logical.LeaseOptions{ + TTL: 1 * time.Hour, + }, + TokenType: logical.TokenTypeService, }, - &logical.Auth{ - ClientToken: "hmac-sha256:08ba357e274f528065766c770a639abf6809b39ccfd37c2a3157c7f51954da0a", - Accessor: "very-accessible", + &Auth{ + ClientToken: "hmac-sha256:08ba357e274f528065766c770a639abf6809b39ccfd37c2a3157c7f51954da0a", + Accessor: "very-accessible", + TokenTTL: 3600, + TokenType: "service", + RemainingUses: 5, }, false, }, "accessor-hmac": { &logical.Auth{ - LeaseOptions: logical.LeaseOptions{ - TTL: 1 * time.Hour, - }, Accessor: "very-accessible", ClientToken: "foo", - }, - &logical.Auth{ LeaseOptions: logical.LeaseOptions{ TTL: 1 * time.Hour, }, - Accessor: "hmac-sha256:5d6d7c8da5b699ace193ea453bbf77082a8aaca42a474436509487d646a7c0af", - ClientToken: "hmac-sha256:08ba357e274f528065766c770a639abf6809b39ccfd37c2a3157c7f51954da0a", + TokenType: logical.TokenTypeBatch, + }, + &Auth{ + ClientToken: "hmac-sha256:08ba357e274f528065766c770a639abf6809b39ccfd37c2a3157c7f51954da0a", + Accessor: "hmac-sha256:5d6d7c8da5b699ace193ea453bbf77082a8aaca42a474436509487d646a7c0af", + TokenTTL: 3600, + TokenType: "batch", + RemainingUses: 5, }, true, }, @@ -176,9 +183,11 @@ func TestHashAuth(t *testing.T) { require.NoError(t, err) salter := &TestSalter{} for _, tc := range cases { - err := hashAuth(context.Background(), salter, tc.Input, tc.HMACAccessor) + auditAuth, err := newAuth(tc.Input, 5) + require.NoError(t, err) + err = hashAuth(context.Background(), salter, auditAuth, tc.HMACAccessor) require.NoError(t, err) - require.Equal(t, tc.Output, tc.Input) + require.Equal(t, tc.Output, auditAuth) } } @@ -196,7 +205,7 @@ var _ logical.OptMarshaler = &testOptMarshaler{} func TestHashRequest(t *testing.T) { cases := []struct { Input *logical.Request - Output *logical.Request + Output *Request NonHMACDataKeys []string HMACAccessor bool }{ @@ -209,13 +218,17 @@ func TestHashRequest(t *testing.T) { "om": &testOptMarshaler{S: "bar", I: 1}, }, }, - &logical.Request{ + &Request{ Data: map[string]interface{}{ "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", "baz": "foobar", "private_key_type": "hmac-sha256:995230dca56fffd310ff591aa404aab52b2abb41703c787cfa829eceb4595bf1", "om": json.RawMessage(`{"S":"hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317","I":1}`), }, + Namespace: &Namespace{ + ID: namespace.RootNamespace.ID, + Path: namespace.RootNamespace.Path, + }, }, []string{"baz"}, false, @@ -230,62 +243,61 @@ func TestHashRequest(t *testing.T) { require.NoError(t, err) salter := &TestSalter{} for _, tc := range cases { - input := fmt.Sprintf("%#v", tc.Input) - err := hashRequest(context.Background(), salter, tc.Input, tc.HMACAccessor, tc.NonHMACDataKeys) - if err != nil { - t.Fatalf("err: %s\n\n%s", err, input) - } - if diff := deep.Equal(tc.Input, tc.Output); len(diff) > 0 { - t.Fatalf("bad:\nInput:\n%s\nDiff:\n%#v", input, diff) - } + auditReq, err := newRequest(tc.Input, namespace.RootNamespace) + require.NoError(t, err) + err = hashRequest(context.Background(), salter, auditReq, tc.HMACAccessor, tc.NonHMACDataKeys) + require.NoError(t, err) + require.Equal(t, tc.Output, auditReq) } } func TestHashResponse(t *testing.T) { now := time.Now() - cases := []struct { - Input *logical.Response - Output *logical.Response - NonHMACDataKeys []string - HMACAccessor bool - }{ - { - &logical.Response{ - Data: map[string]interface{}{ - "foo": "bar", - "baz": "foobar", - // Responses can contain time values, so test that with - // a known fixed value. - "bar": now, - "om": &testOptMarshaler{S: "bar", I: 1}, - }, - WrapInfo: &wrapping.ResponseWrapInfo{ - TTL: 60, - Token: "bar", - Accessor: "flimflam", - CreationTime: now, - WrappedAccessor: "bar", - }, - }, - &logical.Response{ - Data: map[string]interface{}{ - "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", - "baz": "foobar", - "bar": now.Format(time.RFC3339Nano), - "om": json.RawMessage(`{"S":"hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317","I":1}`), - }, - WrapInfo: &wrapping.ResponseWrapInfo{ - TTL: 60, - Token: "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", - Accessor: "hmac-sha256:7c9c6fe666d0af73b3ebcfbfabe6885015558213208e6635ba104047b22f6390", - CreationTime: now, - WrappedAccessor: "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", - }, - }, - []string{"baz"}, - true, + resp := &logical.Response{ + Data: map[string]interface{}{ + "foo": "bar", + "baz": "foobar", + // Responses can contain time values, so test that with a known fixed value. + "bar": now, + "om": &testOptMarshaler{S: "bar", I: 1}, + }, + WrapInfo: &wrapping.ResponseWrapInfo{ + TTL: 1 * time.Minute, + Token: "bar", + Accessor: "flimflam", + CreationTime: now, + WrappedAccessor: "bar", + }, + } + + req := &logical.Request{MountPoint: "/foo/bar"} + req.SetMountClass("kv") + req.SetMountIsExternalPlugin(true) + req.SetMountRunningVersion("123") + req.SetMountRunningSha256("256-256!") + + nonHMACDataKeys := []string{"baz"} + + expected := &Response{ + Data: map[string]interface{}{ + "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", + "baz": "foobar", + "bar": now.Format(time.RFC3339Nano), + "om": json.RawMessage(`{"S":"hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317","I":1}`), + }, + WrapInfo: &ResponseWrapInfo{ + TTL: 60, + Token: "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", + Accessor: "hmac-sha256:7c9c6fe666d0af73b3ebcfbfabe6885015558213208e6635ba104047b22f6390", + CreationTime: now.UTC().Format(time.RFC3339Nano), + WrappedAccessor: "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", }, + MountClass: "kv", + MountIsExternalPlugin: true, + MountPoint: "/foo/bar", + MountRunningVersion: "123", + MountRunningSha256: "256-256!", } inmemStorage := &logical.InmemStorage{} @@ -295,14 +307,11 @@ func TestHashResponse(t *testing.T) { }) require.NoError(t, err) salter := &TestSalter{} - for _, tc := range cases { - input := fmt.Sprintf("%#v", tc.Input) - err := hashResponse(context.Background(), salter, tc.Input, tc.HMACAccessor, tc.NonHMACDataKeys, false) - if err != nil { - t.Fatalf("err: %s\n\n%s", err, input) - } - require.Equal(t, tc.Output, tc.Input) - } + auditResp, err := newResponse(resp, req, false) + require.NoError(t, err) + err = hashResponse(context.Background(), salter, auditResp, true, nonHMACDataKeys) + require.NoError(t, err) + require.Equal(t, expected, auditResp) } func TestHashWalker(t *testing.T) { diff --git a/audit/types.go b/audit/types.go index 9bebc57afa9f..657c23febdb1 100644 --- a/audit/types.go +++ b/audit/types.go @@ -7,31 +7,17 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -// NOTE: Any exported changes made to RequestEntry, ResponseEntry or the structs -// used to compose them, must be reflected in the public facing documentation. -// See: /vault/docs/audit (website/content/docs/audit/index.mdx), which at the time -// of writing contains JSON examples and JSON schemas intended for use in audit -// exclusion. - -// RequestEntry is the structure of a request audit log entry. -type RequestEntry struct { - Auth *Auth `json:"auth,omitempty"` - Error string `json:"error,omitempty"` - ForwardedFrom string `json:"forwarded_from,omitempty"` // Populated in Enterprise when a request is forwarded - Request *Request `json:"request,omitempty"` - Time string `json:"time,omitempty"` - Type string `json:"type,omitempty"` -} - -// ResponseEntry is the structure of a response audit log entry. -type ResponseEntry struct { - Auth *Auth `json:"auth,omitempty"` - Error string `json:"error,omitempty"` - Forwarded bool `json:"forwarded,omitempty"` - Request *Request `json:"request,omitempty"` - Response *Response `json:"response,omitempty"` - Time string `json:"time,omitempty"` - Type string `json:"type,omitempty"` +// Entry represents an audit entry. +// It could be an entry for a request or response. +type Entry struct { + Auth *Auth `json:"auth,omitempty"` + Error string `json:"error,omitempty"` + Forwarded bool `json:"forwarded,omitempty"` + ForwardedFrom string `json:"forwarded_from,omitempty"` // Populated in Enterprise when a request is forwarded + Request *Request `json:"request,omitempty"` + Response *Response `json:"response,omitempty"` + Time string `json:"time,omitempty"` + Type string `json:"type,omitempty"` } type Request struct { From 00fa8617c519174d22082239d7541604b0a92aeb Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 12:52:16 +0100 Subject: [PATCH 2/9] changelog --- changelog/27952.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/27952.txt diff --git a/changelog/27952.txt b/changelog/27952.txt new file mode 100644 index 000000000000..aa7d2ba84f9d --- /dev/null +++ b/changelog/27952.txt @@ -0,0 +1,3 @@ +```release-note:improvement +audit: Internal implementation changes to the audit subsystem which improve performance. +``` From 9e3ad62d1e872edb4e37c6404bd4db93d12345fd Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 13:54:13 +0100 Subject: [PATCH 3/9] Return errors when trying to convert nil logical types --- audit/entry_formatter.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 28400cf03a6a..93379b59f703 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -263,7 +263,7 @@ func clone[V any](s V) (V, error) { // This usually can be found in logical.Request.ClientTokenRemainingUses. func newAuth(auth *logical.Auth, tokenRemainingUses int) (*Auth, error) { if auth == nil { - return nil, nil + return nil, fmt.Errorf("auth cannot be nil") } extNSPolicies, err := clone(auth.ExternalNamespacePolicies) @@ -338,7 +338,7 @@ func newAuth(auth *logical.Auth, tokenRemainingUses int) (*Auth, error) { // aggregates them into an audit Request. func newRequest(req *logical.Request, ns *namespace.Namespace) (*Request, error) { if req == nil { - return nil, nil + return nil, fmt.Errorf("request cannot be nil") } remoteAddr := remoteAddr(req) @@ -399,12 +399,12 @@ func newRequest(req *logical.Request, ns *namespace.Namespace) (*Request, error) // isElisionRequired is used to indicate that response 'Data' should be elided. func newResponse(resp *logical.Response, req *logical.Request, isElisionRequired bool) (*Response, error) { if resp == nil { - return nil, nil + return nil, fmt.Errorf("response cannot be nil") } if req == nil { // Request should never be nil, even for a response. - return nil, fmt.Errorf("request is nil") + return nil, fmt.Errorf("request cannot be nil") } auth, err := newAuth(resp.Auth, req.ClientTokenRemainingUses) From 371e5e0c98a1feb50f84c68e087433616ff813e9 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 14:05:36 +0100 Subject: [PATCH 4/9] Fix typo and tests --- audit/entry_formatter.go | 2 +- audit/entry_formatter_test.go | 80 +++++++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 93379b59f703..663f532e9611 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -541,7 +541,7 @@ func (f *entryFormatter) createEntry(ctx context.Context, a *AuditEvent) (*Entry shouldElide := f.config.elideListResponses && req.Operation == logical.ListOperation resp, err = newResponse(data.Response, data.Request, shouldElide) if err != nil { - return nil, fmt.Errorf("cannot convert request: %w", err) + return nil, fmt.Errorf("cannot convert response: %w", err) } } diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 56548435da3d..36c895d4337d 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -298,21 +298,33 @@ func TestEntryFormatter_Process(t *testing.T) { ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: ResponseType, RequiredFormat: JSONFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, }, "json-request-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: RequestType, RequiredFormat: JSONFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, - RootNamespace: true, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, + RootNamespace: true, }, "json-response-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: ResponseType, RequiredFormat: JSONFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, - RootNamespace: true, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{ + Auth: &logical.Auth{}, + }, + }, + RootNamespace: true, }, "jsonx-request-no-data": { IsErrorExpected: true, @@ -362,15 +374,65 @@ func TestEntryFormatter_Process(t *testing.T) { IsErrorExpected: false, Subtype: RequestType, RequiredFormat: JSONxFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, - RootNamespace: true, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, + RootNamespace: true, }, "jsonx-response-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: ResponseType, RequiredFormat: JSONxFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, - RootNamespace: true, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{ + Auth: &logical.Auth{}, + }, + }, + RootNamespace: true, + }, + "no-auth": { + IsErrorExpected: true, + ExpectedErrorMessage: "cannot convert auth: auth cannot be nil", + Subtype: RequestType, + RequiredFormat: JSONxFormat, + Data: &logical.LogInput{ + Request: &logical.Request{ID: "123"}, + }, + RootNamespace: true, + }, + "no-response-auth": { + IsErrorExpected: true, + ExpectedErrorMessage: "cannot convert auth: auth cannot be nil", + Subtype: ResponseType, + RequiredFormat: JSONxFormat, + Data: &logical.LogInput{ + Request: &logical.Request{ID: "123"}, + }, + RootNamespace: true, + }, + "no-request": { + IsErrorExpected: true, + ExpectedErrorMessage: "unable to parse request from 'response' audit event: request cannot be nil", + Subtype: ResponseType, + RequiredFormat: JSONxFormat, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + }, + RootNamespace: true, + }, + "no-response": { + IsErrorExpected: true, + ExpectedErrorMessage: "cannot convert response: response cannot be nil", + Subtype: ResponseType, + RequiredFormat: JSONxFormat, + Data: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, + RootNamespace: true, }, } From e02a55cdaac6c66115379a95524aefe77e2acf3b Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 14:28:45 +0100 Subject: [PATCH 5/9] Update tests and names --- audit/entry_formatter_test.go | 54 +++++++++++++++++++++++++---------- audit/hashstructure_test.go | 4 +++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 36c895d4337d..895a1014c684 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -245,7 +245,7 @@ func TestEntryFormatter_Type(t *testing.T) { } // TestEntryFormatter_Process attempts to run the Process method to convert the -// logical.LogInput within an audit event to JSON and JSONx (RequestEntry or ResponseEntry). +// logical.LogInput within an audit event to JSON and JSONx (Entry), func TestEntryFormatter_Process(t *testing.T) { t.Parallel() @@ -541,8 +541,8 @@ func BenchmarkAuditFileSink_Process(b *testing.B) { }) } -// TestEntryFormatter_FormatRequest exercises entryFormatter.formatRequest with -// varying inputs. +// TestEntryFormatter_Process_Request exercises entryFormatter process an event +// with varying inputs. func TestEntryFormatter_Process_Request(t *testing.T) { t.Parallel() @@ -570,12 +570,18 @@ func TestEntryFormatter_Process_Request(t *testing.T) { RootNamespace: false, }, "input-and-request-with-ns": { - Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + Input: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, IsErrorExpected: false, RootNamespace: true, }, "omit-time": { - Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + Input: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + }, ShouldOmitTime: true, RootNamespace: true, }, @@ -644,9 +650,9 @@ func TestEntryFormatter_Process_Request(t *testing.T) { } } -// TestEntryFormatter_FormatResponse exercises entryFormatter.formatResponse with -// varying inputs. -func TestEntryFormatter_FormatResponse(t *testing.T) { +// TestEntryFormatter_Process_ResponseType exercises entryFormatter +// with varying inputs also checking if the time can be omitted. +func TestEntryFormatter_Process_ResponseType(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -673,12 +679,24 @@ func TestEntryFormatter_FormatResponse(t *testing.T) { RootNamespace: false, }, "input-and-request-with-ns": { - Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + Input: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{ + Auth: &logical.Auth{}, + }, + }, IsErrorExpected: false, RootNamespace: true, }, "omit-time": { - Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + Input: &logical.LogInput{ + Auth: &logical.Auth{}, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{ + Auth: &logical.Auth{}, + }, + }, ShouldOmitTime: true, IsErrorExpected: false, RootNamespace: true, @@ -1042,9 +1060,9 @@ func TestEntryFormatter_Process_JSONx(t *testing.T) { } } -// TestEntryFormatter_FormatResponse_ElideListResponses ensures that we correctly -// elide data in responses to LIST operations. -func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { +// TestEntryFormatter_ElideListResponses ensures that we correctly elide data in +// responses to LIST operations. +func TestEntryFormatter_ElideListResponses(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -1121,8 +1139,12 @@ func TestEntryFormatter_FormatResponse_ElideListResponses(t *testing.T) { require.NotNil(t, formatter) in := &logical.LogInput{ - Request: &logical.Request{Operation: operation}, - Response: &logical.Response{Data: inputData}, + Auth: &logical.Auth{}, + Request: &logical.Request{Operation: operation}, + Response: &logical.Response{ + Auth: &logical.Auth{}, + Data: inputData, + }, } auditEvent, err := NewEvent(ResponseType) @@ -1274,6 +1296,7 @@ func TestEntryFormatter_Process_Panic(t *testing.T) { Data: map[string]interface{}{}, }, Response: &logical.Response{ + Auth: &logical.Auth{}, Data: map[string]any{ "token_bound_cidrs": []*sockaddr.SockAddrMarshaler{ {SockAddr: badAddr}, @@ -1379,6 +1402,7 @@ func fakeEvent(tb testing.TB, subtype subtype, input *logical.LogInput) *eventlo require.Equal(tb, date, auditEvent.Timestamp) auditEvent.Data = input + auditEvent.setTimeProvider(&testTimeProvider{}) e := &eventlogger.Event{ Type: eventlogger.EventType(event.AuditType), diff --git a/audit/hashstructure_test.go b/audit/hashstructure_test.go index 653b31d80fff..90c14ba267ed 100644 --- a/audit/hashstructure_test.go +++ b/audit/hashstructure_test.go @@ -255,6 +255,7 @@ func TestHashResponse(t *testing.T) { now := time.Now() resp := &logical.Response{ + Auth: &logical.Auth{}, Data: map[string]interface{}{ "foo": "bar", "baz": "foobar", @@ -280,6 +281,9 @@ func TestHashResponse(t *testing.T) { nonHMACDataKeys := []string{"baz"} expected := &Response{ + Auth: &Auth{ + TokenType: logical.TokenTypeDefault.String(), + }, Data: map[string]interface{}{ "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", "baz": "foobar", From 6140458032f610c91ae81d8c1b0f3bd8f5b34bc1 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 14:50:25 +0100 Subject: [PATCH 6/9] nil auth is allowed --- audit/entry_formatter.go | 4 +++- audit/entry_formatter_test.go | 20 -------------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 663f532e9611..3c0480608e50 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -261,9 +261,11 @@ func clone[V any](s V) (V, error) { // an audit Auth. // tokenRemainingUses should be the client token remaining uses to include in auth. // This usually can be found in logical.Request.ClientTokenRemainingUses. +// NOTE: supplying a nil value for auth will result in a nil return value and error. +// The caller should check the return value before attempting to use it. func newAuth(auth *logical.Auth, tokenRemainingUses int) (*Auth, error) { if auth == nil { - return nil, fmt.Errorf("auth cannot be nil") + return nil, nil } extNSPolicies, err := clone(auth.ExternalNamespacePolicies) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 895a1014c684..2bc0d69b251b 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -393,26 +393,6 @@ func TestEntryFormatter_Process(t *testing.T) { }, RootNamespace: true, }, - "no-auth": { - IsErrorExpected: true, - ExpectedErrorMessage: "cannot convert auth: auth cannot be nil", - Subtype: RequestType, - RequiredFormat: JSONxFormat, - Data: &logical.LogInput{ - Request: &logical.Request{ID: "123"}, - }, - RootNamespace: true, - }, - "no-response-auth": { - IsErrorExpected: true, - ExpectedErrorMessage: "cannot convert auth: auth cannot be nil", - Subtype: ResponseType, - RequiredFormat: JSONxFormat, - Data: &logical.LogInput{ - Request: &logical.Request{ID: "123"}, - }, - RootNamespace: true, - }, "no-request": { IsErrorExpected: true, ExpectedErrorMessage: "unable to parse request from 'response' audit event: request cannot be nil", From 81508cdc0550a59affe01987e412c783b9f3ed94 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 14:54:55 +0100 Subject: [PATCH 7/9] Remove auth from tests as not required --- audit/entry_formatter_test.go | 66 +++++++++-------------------------- 1 file changed, 17 insertions(+), 49 deletions(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 2bc0d69b251b..6d0082dcf51b 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -298,27 +298,20 @@ func TestEntryFormatter_Process(t *testing.T) { ExpectedErrorMessage: "unable to retrieve namespace from context: no namespace", Subtype: ResponseType, RequiredFormat: JSONFormat, - Data: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, + Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, }, "json-request-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: RequestType, RequiredFormat: JSONFormat, - Data: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, - RootNamespace: true, + Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + RootNamespace: true, }, "json-response-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{ - Auth: &logical.Auth{}, Request: &logical.Request{ID: "123"}, Response: &logical.Response{ Auth: &logical.Auth{}, @@ -374,22 +367,16 @@ func TestEntryFormatter_Process(t *testing.T) { IsErrorExpected: false, Subtype: RequestType, RequiredFormat: JSONxFormat, - Data: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, - RootNamespace: true, + Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + RootNamespace: true, }, "jsonx-response-basic-input-and-request-with-ns": { IsErrorExpected: false, Subtype: ResponseType, RequiredFormat: JSONxFormat, Data: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - Response: &logical.Response{ - Auth: &logical.Auth{}, - }, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{}, }, RootNamespace: true, }, @@ -408,11 +395,8 @@ func TestEntryFormatter_Process(t *testing.T) { ExpectedErrorMessage: "cannot convert response: response cannot be nil", Subtype: ResponseType, RequiredFormat: JSONxFormat, - Data: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, - RootNamespace: true, + Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, + RootNamespace: true, }, } @@ -550,18 +534,12 @@ func TestEntryFormatter_Process_Request(t *testing.T) { RootNamespace: false, }, "input-and-request-with-ns": { - Input: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, + Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, IsErrorExpected: false, RootNamespace: true, }, "omit-time": { - Input: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - }, + Input: &logical.LogInput{Request: &logical.Request{ID: "123"}}, ShouldOmitTime: true, RootNamespace: true, }, @@ -660,22 +638,16 @@ func TestEntryFormatter_Process_ResponseType(t *testing.T) { }, "input-and-request-with-ns": { Input: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - Response: &logical.Response{ - Auth: &logical.Auth{}, - }, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{}, }, IsErrorExpected: false, RootNamespace: true, }, "omit-time": { Input: &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{ID: "123"}, - Response: &logical.Response{ - Auth: &logical.Auth{}, - }, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{}, }, ShouldOmitTime: true, IsErrorExpected: false, @@ -1119,12 +1091,8 @@ func TestEntryFormatter_ElideListResponses(t *testing.T) { require.NotNil(t, formatter) in := &logical.LogInput{ - Auth: &logical.Auth{}, - Request: &logical.Request{Operation: operation}, - Response: &logical.Response{ - Auth: &logical.Auth{}, - Data: inputData, - }, + Request: &logical.Request{Operation: operation}, + Response: &logical.Response{Data: inputData}, } auditEvent, err := NewEvent(ResponseType) From 8d62b13bffdfa4a30330049bce76966ea64d19b9 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 15:01:17 +0100 Subject: [PATCH 8/9] Remove more unused auths --- audit/entry_formatter_test.go | 6 ++---- audit/hashstructure_test.go | 4 ---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 6d0082dcf51b..136260f15d3d 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -312,10 +312,8 @@ func TestEntryFormatter_Process(t *testing.T) { Subtype: ResponseType, RequiredFormat: JSONFormat, Data: &logical.LogInput{ - Request: &logical.Request{ID: "123"}, - Response: &logical.Response{ - Auth: &logical.Auth{}, - }, + Request: &logical.Request{ID: "123"}, + Response: &logical.Response{}, }, RootNamespace: true, }, diff --git a/audit/hashstructure_test.go b/audit/hashstructure_test.go index 90c14ba267ed..653b31d80fff 100644 --- a/audit/hashstructure_test.go +++ b/audit/hashstructure_test.go @@ -255,7 +255,6 @@ func TestHashResponse(t *testing.T) { now := time.Now() resp := &logical.Response{ - Auth: &logical.Auth{}, Data: map[string]interface{}{ "foo": "bar", "baz": "foobar", @@ -281,9 +280,6 @@ func TestHashResponse(t *testing.T) { nonHMACDataKeys := []string{"baz"} expected := &Response{ - Auth: &Auth{ - TokenType: logical.TokenTypeDefault.String(), - }, Data: map[string]interface{}{ "foo": "hmac-sha256:f9320baf0249169e73850cd6156ded0106e2bb6ad8cab01b7bbbebe6d1065317", "baz": "foobar", From 85b7e6a9c7de74ffd87cda5e3d6953c79740c0ad Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 2 Aug 2024 16:24:38 +0100 Subject: [PATCH 9/9] It seems logging a response with no response is fine. :/ --- audit/entry_formatter.go | 2 +- audit/entry_formatter_test.go | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/audit/entry_formatter.go b/audit/entry_formatter.go index 3c0480608e50..927058337fa3 100644 --- a/audit/entry_formatter.go +++ b/audit/entry_formatter.go @@ -401,7 +401,7 @@ func newRequest(req *logical.Request, ns *namespace.Namespace) (*Request, error) // isElisionRequired is used to indicate that response 'Data' should be elided. func newResponse(resp *logical.Response, req *logical.Request, isElisionRequired bool) (*Response, error) { if resp == nil { - return nil, fmt.Errorf("response cannot be nil") + return nil, nil } if req == nil { diff --git a/audit/entry_formatter_test.go b/audit/entry_formatter_test.go index 136260f15d3d..8060636d846b 100644 --- a/audit/entry_formatter_test.go +++ b/audit/entry_formatter_test.go @@ -388,14 +388,6 @@ func TestEntryFormatter_Process(t *testing.T) { }, RootNamespace: true, }, - "no-response": { - IsErrorExpected: true, - ExpectedErrorMessage: "cannot convert response: response cannot be nil", - Subtype: ResponseType, - RequiredFormat: JSONxFormat, - Data: &logical.LogInput{Request: &logical.Request{ID: "123"}}, - RootNamespace: true, - }, } for name, tc := range tests {