[VAULT-22481] Add audit filtering feature #24558
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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]>
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
|
Build Results: |
miagilepner
reviewed
Dec 18, 2023
| // 1. formatter (temporary) | ||
| // 2. sink | ||
| // 1. filter (optional if configured) | ||
| // 2. formatter (temporary) |
There was a problem hiding this comment.
I'm a bit confused by this comment versus the comment on the function. What does it look like if the last node is a filter node? Is the formatter then skipped?
There was a problem hiding this comment.
We've got a question about whether or not we should allow test messages (which is what would cause ProcessManual to be requested if a filter node is part of the pipeline). So this code may change in a future PR based on the outcomes of that.
To answer the question though 😄 if the filter node filters the message that ends the pipeline, so the formatter and sink nodes never get called.
added 7 commits
December 18, 2023 16:53
…red device sets sink threshold
marcboudreau
suggested changes
Dec 18, 2023
| @@ -29,9 +30,19 @@ type SocketSink struct { | |||
|
|
|||
| // NewSocketSink should be used to create a new SocketSink. | |||
| // Accepted options: WithMaxDuration and WithSocketType. | |||
| func NewSocketSink(format string, address string, opt ...Option) (*SocketSink, error) { | |||
| func NewSocketSink(address string, format string, opt ...Option) (*SocketSink, error) { | |||
There was a problem hiding this comment.
Since both address and format are of type string, you can do address, format string But not important to get this merged.
marcboudreau
approved these changes
Dec 18, 2023
Monkeychip
pushed a commit
that referenced
this pull request
Jan 7, 2024
* VAULT-22481: Audit filter node (#24465) * 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 * Add audit filtering feature (#24554) * 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]> * Add changelog file * update bexpr datum to use a strong type * go docs updates * test path * PR review comments * handle scenarios/outcomes from broker.send * don't need to re-check the complete sinks * add extra check to deregister to ensure that re-registering non-filtered device sets sink threshold * Ensure that the multierror is appended before attempting to return it --------- Co-authored-by: Peter Wilson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a filtering feature to Vault's audit, which includes:
filteroption in the API and CLI calls to enable an audit device, which accepts go-bexpr expressions.For more details see RFC number VLT-291.