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

sys: don't set X-Vault-Namespace header for root-only paths #14934

Closed
aead opened this issue Apr 6, 2022 · 4 comments · Fixed by #14963
Closed

sys: don't set X-Vault-Namespace header for root-only paths #14934

aead opened this issue Apr 6, 2022 · 4 comments · Fixed by #14963
Labels
bug Used to indicate a potential bug core/api devex Developer Experience

Comments

@aead
Copy link

aead commented Apr 6, 2022

Describe the bug
Vault exposes root-only API paths. For example: `sys/health.
Ref: https://www.vaultproject.io/docs/enterprise/namespaces#root-only-api-paths

However, a vault.Client with a namespace set via SetNamespace sends the X-Vault-Namespace header to those root-only API paths.
For example:

User-Agent: [Go-http-client/1.1]
X-Vault-Namespace: [test]
X-Vault-Request: [true]
X-Vault-Token: [abc]
Accept-Encoding: [gzip]

This causes Vault 1.9.1 to respond with:

Code: 404. Errors:

    unsupported path

To Reproduce

As a simple reproducer - the 1st curl request succeeds while the 2nd fails:

curl -L -X GET "https://localhost:8200/v1/sys/health?drsecondarycode=299&performancestandbycode=299&sealedcode=299&standbycode=299&uninitcode=299"

curl -L -X GET -H 'X-Vault-Namespace: test' "https://localhost:8200/v1/sys/health?drsecondarycode=299&performancestandbycode=299&sealedcode=299&standbycode=299&uninitcode=299"

Expected behavior
Either thevault.Client should not send the X-Vault-Namespace header for root-only API paths or Vault itself should ignore this header in such a case.

Environment:

  • Vault Server Version (retrieve with vault status): 1.9.1
  • Vault SDK Version (retrieve with vault version): 1.5.0
  • Server Operating System/Architecture: linux/amd64
@heatherezell heatherezell added core/api bug Used to indicate a potential bug devex Developer Experience labels Apr 6, 2022
@VinnyHC
Copy link
Contributor

VinnyHC commented Apr 6, 2022

Hello and thanks for opening this issue; I've confirmed the behavior you've described. I'll work this through our processes and get a solution together. Feel free to follow this issue for updates!

@ncabatoff
Copy link
Collaborator

I'm not sure the expected behaviour is a good idea. We may change some root-only paths to be available in namespaces in the future. And if you're specifying X-Vault-Namespace, you probably authenticated into that namespace, and won't be likely to have access to the root-only paths anyway unless they're unauthenticated.

If the client doesn't provide the namespace header for root-only API paths, IMO that's putting too much knowledge of possibly version-specific Vault behaviour into the client. If Vault ignores the specific namespace because it knows that the request doesn't make sense in a namespace but would make sense in the root namespace, that feels like surprising magical behaviour that could come back to bite us in the future, that would be backwards incompatible, and that doesn't seem justified relative to the minor pain associated with having to set the namespace header appropriately for these root-level requests.

Also note that on HCP Vault there is no access to the root namespace, so this behaviour wouldn't make sense at all there.

My suggestion is that you either use two clients for the two different namespaces, or that we find some way to make it easier to specify a namespace on a per-request basis, but still require that it be an explicit choice in the client code that the code author must be aware of.

@aead
Copy link
Author

aead commented Apr 7, 2022

Just to elaborate on a possible use case:
A client is using a secret engine (K/V, transient, ...) and at the same time checks Vault's health status periodically.
Since, sys/health is currently a root-only API path you have to use two clients to achieve this.
Using the same client results in the pretty confusing error 404 unsupported path.

From the client perspective this is a total surprise.
If the solution should be:

My suggestion is that you either use two clients for the two different namespaces

Then this behavior should be documented more explicitly IMO. In particular since certain API paths are currently tied to one namespace.

@VinnyHC
Copy link
Contributor

VinnyHC commented Apr 14, 2022

Hello @aead I just wanted to loop back and update you; after some internal discussion, we decided a good approach was to introduce a new method (*Client).WithNamespace() that hopefully improves your experience. PR #14963 has the update if you'd like to investigate further. Thanks again for raising this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/api devex Developer Experience
Projects
None yet
4 participants