Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[VAULT-22480] Add audit fallback device #24583

Merged
merged 44 commits into from
Jan 8, 2024
Merged

Conversation

kubawi
Copy link
Contributor

@kubawi kubawi commented Dec 19, 2023

This PR adds a fallback device feature to Vault's audit, which includes:

  • A new fallback option in the API and CLI calls to enable an audit device, which accepts a boolean value and is mutually exclusive with the filter option added in [VAULT-22481] Add audit filtering feature #24558.
  • Changes to AuditBroker adding a separate eventlogger.Broker for a fallback device.
  • A bunch of new tests.

The fallback device is designed to be used in cases when the Vault operator has configured only filtered audit device(s) and wants to have an additional device that will "catch" all of the entries that were filtered out of the other devices. This can be thought of as being similar to a default case in a Go switch statement.

The fallback device is a singleton, in that you can only have one enabled at a time.

For more details see RFC number VLT-291.

No changelog entry as this is a new feature with a changelog added in PR #24558.

Peter Wilson and others added 18 commits December 11, 2023 13:47
* Initial commit on adding filter nodes for audit

* tests for audit filter

* test: longer filter - more conditions

* copywrite headers

* Check interface for the right type
* Support filter nodes in backend factories and add some tests

* More tests and cleanup

* Attempt to move control of registration for nodes and pipelines to the audit broker (#24505)

* invert control of the pipelines/nodes to the audit broker vs. within each backend

* update noop audit test code to implement the pipeliner interface

* noop mount path has trailing slash

* attempting to make NoopAudit more friendly

* NoopAudit uses known salt

* Refactor audit.ProcessManual to support filter nodes

* HasFiltering

* rename the pipeliner

* use exported AuditEvent in Filter

* Add tests for registering and deregistering backends on the audit broker

* Add missing licence header to one file, fix a typo in two tests

---------

Co-authored-by: Peter Wilson <[email protected]>
@kubawi kubawi added this to the 1.16.0-rc1 milestone Dec 19, 2023
@kubawi kubawi self-assigned this Dec 19, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 19, 2023
Copy link

github-actions bot commented Dec 19, 2023

CI Results:
All Go tests succeeded! ✅

@peteski22 peteski22 requested a review from a team January 5, 2024 16:04
@kubawi kubawi marked this pull request as ready for review January 5, 2024 16:32
Copy link

github-actions bot commented Jan 5, 2024

Build Results:
All builds succeeded! ✅

cfg, err := formatterConfig(conf.Config)
if err != nil {
return nil, fmt.Errorf("%s: failed to create formatter config: %w", op, err)
}

cfg, err = formatterConfig(conf.Config)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this repeated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot! Checked the history and that's a leftover from updating the branch that I missed (git duplicated the code block above 🤨).

vault/audit_broker.go Show resolved Hide resolved
return nil
}

name = strings.TrimSpace(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

This TrimSpace mutation was already done above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@@ -60,6 +62,21 @@ func Factory(_ context.Context, conf *audit.BackendConfig, useEventLogger bool,
return nil, fmt.Errorf("%s: nil salt view", op)
}

// The config options 'fallback' and 'filter' are mutually exclusive, a fallback
Copy link
Contributor

Choose a reason for hiding this comment

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

This same chunk of code and its tests are repeated for each of the audit backends. Can this instead be moved to a shared function?

@kubawi kubawi merged commit 2047ce7 into main Jan 8, 2024
109 checks passed
@kubawi kubawi deleted the VAULT-22480/audit-fallback-device branch January 8, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/audit hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants