-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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-19863: Per-listener redaction settings #23534
VAULT-19863: Per-listener redaction settings #23534
Conversation
…/listener-response-redaction-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment for now.
@peteski22, love the inclusion of all the manual testing examples, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for all the tests, and the solution is very elegant!
@@ -174,14 +174,25 @@ func handleSysSealBackendStatus(core *vault.Core) http.Handler { | |||
}) | |||
} | |||
|
|||
func handleSysSealStatusRaw(core *vault.Core, w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for cleaning up this unused parameter!
I wasn't sure where to put this -- but is it worth adding an automated test for the CLI output? I'm not sure on the level of effort to gain, but it's something I noticed being present in your manual tests but not in the go tests (that I could see) |
… github.com:hashicorp/vault into peteski22/VAULT-19863/listener-response-redaction-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had one very minor question about tests but this looks really great!
After chatting with Violet I'm going to try and get this PR merged without this test and come back shortly and see how much effort it is to add after the fact. It would be good to have a way to assert we get the right output in both redacted/non-redacted cases though. 😄 |
Build Results: |
CI Results: |
Summary
This PR is part of the work on reducing disclosure from unauthenticated endpoints in Vault (VAULT-7999).
It allows users to specify per-listener configuration which will redact information from the response of certain endpoints.
The redaction value (as per the RFC) is
""
but this can be configured at a later date in code (not by customers) if required. Some response types will omit a field if the value is empty, forcluster_name
inseal-status
andhealth
., furthermore the CLI may format the empty results in different ways depending on the field e.g.n/a
or<none>
./sys/health
:redact_version
(redactsversion
)redact_cluster_name
(redactscluster_name
)/sys/seal-status
:redact_version
(redactsversion
andbuild_date
)redact_cluster_name
(redactscluster_name
)/sys/leader
:redact_addresses
parameter (redactsleader_address
andcluster_leader_address
)NOTE: Docs PR #23568
Manual Testing:
Vault Config
(Tested using
VCM
)Before:
After:
/sys/health
curl -s http://127.0.0.1:8200/v1/sys/health | jq
Before:
After:
/sys/seal-status
curl -s http://127.0.0.1:8200/v1/sys/seal-status | jq
Before:
After:
/sys/leader
curl -s http://127.0.0.1:8200/v1/sys/leader | jq
Before:
After:
CLI
vault status
Before:
After: