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-19863: Per-listener redaction settings #23534

Merged
merged 16 commits into from
Oct 6, 2023

Conversation

peteski22
Copy link

@peteski22 peteski22 commented Oct 5, 2023

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, for cluster_name in seal-status and health., 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 (redacts version)
  • redact_cluster_name (redacts cluster_name)

/sys/seal-status:

  • redact_version (redacts version and build_date)
  • redact_cluster_name (redacts cluster_name)

/sys/leader:

  • redact_addresses parameter (redacts leader_address and cluster_leader_address)

NOTE: Docs PR #23568


Manual Testing:

Vault Config

(Tested using VCM)

Before:

ui            = false
cluster_addr  = "http://127.0.0.1:8201"
api_addr      = "http://127.0.0.1:8200"
disable_mlock = true

storage "file" {
  path = "/tmp/vault/data"
}

listener "tcp" {
  address       = "127.0.0.1:8200"
  tls_disable   = "true"
}

After:

ui            = false
cluster_addr  = "http://127.0.0.1:8201"
api_addr      = "http://127.0.0.1:8200"
disable_mlock = true

storage "file" {
  path = "/tmp/vault/data"
}

listener "tcp" {
  address             = "127.0.0.1:8200"
  tls_disable         = "true"
  redact_addresses    = "true"
  redact_cluster_name = "true"
  redact_version      = "true"
}

/sys/health

curl -s http://127.0.0.1:8200/v1/sys/health | jq

Before:

{
  "initialized": true,
  "sealed": false,
  "standby": true,
  "performance_standby": false,
  "replication_performance_mode": "unknown",
  "replication_dr_mode": "unknown",
  "server_time_utc": 1696598893,
  "version": "1.16.0-beta1",
  "cluster_name": "vault-cluster-369723ec",
  "cluster_id": "a1a7a078-0ae1-7fb9-41ec-2f4f583c773e"
}

After:

{
  "initialized": true,
  "sealed": false,
  "standby": true,
  "performance_standby": false,
  "replication_performance_mode": "disabled",
  "replication_dr_mode": "disabled",
  "server_time_utc": 1696598650,
  "version": "",
  "cluster_id": "a1a7a078-0ae1-7fb9-41ec-2f4f583c773e"
}

/sys/seal-status

curl -s http://127.0.0.1:8200/v1/sys/seal-status | jq

Before:

{
  "type": "shamir",
  "initialized": true,
  "sealed": false,
  "t": 1,
  "n": 1,
  "progress": 0,
  "nonce": "",
  "version": "1.16.0-beta1",
  "build_date": "2023-10-06T11:48:28Z",
  "migration": false,
  "cluster_name": "vault-cluster-369723ec",
  "cluster_id": "a1a7a078-0ae1-7fb9-41ec-2f4f583c773e",
  "recovery_seal": false,
  "storage_type": "raft"
}

After:

{
  "type": "shamir",
  "initialized": true,
  "sealed": false,
  "t": 1,
  "n": 1,
  "progress": 0,
  "nonce": "",
  "version": "",
  "build_date": "",
  "migration": false,
  "cluster_id": "a1a7a078-0ae1-7fb9-41ec-2f4f583c773e",
  "recovery_seal": false,
  "storage_type": "raft"
}

/sys/leader

curl -s http://127.0.0.1:8200/v1/sys/leader | jq

Before:

{
  "ha_enabled": true,
  "is_self": false,
  "active_time": "0001-01-01T00:00:00Z",
  "leader_address": "http://127.0.0.1:8204",
  "leader_cluster_address": "https://127.0.0.1:8205",
  "performance_standby": false,
  "performance_standby_last_remote_wal": 0,
  "raft_committed_index": 193,
  "raft_applied_index": 193
}

After:

{
  "ha_enabled": true,
  "is_self": false,
  "active_time": "0001-01-01T00:00:00Z",
  "leader_address": "",
  "leader_cluster_address": "",
  "performance_standby": false,
  "performance_standby_last_remote_wal": 0,
  "raft_committed_index": 164,
  "raft_applied_index": 164
}

CLI

vault status

Before:

Key                     Value
---                     -----
Seal Type               shamir
Initialized             true
Sealed                  false
Total Shares            1
Threshold               1
Version                 1.16.0-beta1
Build Date              2023-10-06T11:48:28Z
Storage Type            raft
Cluster Name            vault-cluster-369723ec
Cluster ID              a1a7a078-0ae1-7fb9-41ec-2f4f583c773e
HA Enabled              true
HA Cluster              https://127.0.0.1:8205
HA Mode                 standby
Active Node Address     http://127.0.0.1:8204
Raft Committed Index    196
Raft Applied Index      196

After:

Key                     Value
---                     -----
Seal Type               shamir
Initialized             true
Sealed                  false
Total Shares            1
Threshold               1
Version                 n/a
Build Date              n/a
Storage Type            raft
HA Enabled              true
HA Cluster              n/a
HA Mode                 standby
Active Node Address     <none>
Raft Committed Index    219
Raft Applied Index      219

@peteski22 peteski22 added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Oct 5, 2023
@peteski22 peteski22 added this to the 1.16.0-rc1 milestone Oct 5, 2023
Copy link
Contributor

@marcboudreau marcboudreau left a 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.

changelog/23534.txt Outdated Show resolved Hide resolved
@hashicorp hashicorp deleted a comment from github-actions bot Oct 6, 2023
@ccapurso
Copy link
Contributor

ccapurso commented Oct 6, 2023

@peteski22, love the inclusion of all the manual testing examples, thanks!

Copy link
Contributor

@VioletHynes VioletHynes left a 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!

changelog/23534.txt Outdated Show resolved Hide resolved
http/options.go Outdated Show resolved Hide resolved
internalshared/configutil/listener.go Show resolved Hide resolved
@@ -174,14 +174,25 @@ func handleSysSealBackendStatus(core *vault.Core) http.Handler {
})
}

func handleSysSealStatusRaw(core *vault.Core, w http.ResponseWriter, r *http.Request) {
Copy link
Contributor

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!

@VioletHynes
Copy link
Contributor

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)

Peter Wilson added 2 commits October 6, 2023 15:44
… github.com:hashicorp/vault into peteski22/VAULT-19863/listener-response-redaction-2
Copy link
Contributor

@ccapurso ccapurso left a 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!

http/options_test.go Outdated Show resolved Hide resolved
@peteski22
Copy link
Author

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)

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. 😄

@hashicorp hashicorp deleted a comment from github-actions bot Oct 6, 2023
@hashicorp hashicorp deleted a comment from github-actions bot Oct 6, 2023
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Build Results:
All builds succeeded! ✅

@peteski22 peteski22 merged commit e5432b0 into main Oct 6, 2023
102 checks passed
@peteski22 peteski22 deleted the peteski22/VAULT-19863/listener-response-redaction-2 branch October 6, 2023 16:39
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

CI Results:
All Go tests succeeded! ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants