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

-output-policy CLI flag is broken in multiple ways #18559

Open
maxb opened this issue Dec 27, 2022 · 1 comment
Open

-output-policy CLI flag is broken in multiple ways #18559

maxb opened this issue Dec 27, 2022 · 1 comment
Labels
bug Used to indicate a potential bug core/cli devex Developer Experience

Comments

@maxb
Copy link
Contributor

maxb commented Dec 27, 2022

I've noticed the recently added -output-policy CLI flag. Unfortunately, the implementation suffers from multiple problems causing it to produce incorrect answers.

Does not work for vault kv ... commands

$ vault kv get -output-policy kv/foobar
This output flag requires the success of a preflight request 
to determine the version of a KV secrets engine. Please 
re-run this command with a token with read access to kv/foobar
Unable to generate policy from command

(Contrary to the message, a token was supplied to this operation.)

Incorrect answers for vault list commands

$ vault list -output-policy something
path "something" {
  capabilities = ["read"]
}

It should be recommending list, not read.

Failure to recommend sudo for auth/token/accessors/

$ vault list -output-policy auth/token/accessors/
path "auth/token/accessors" {
  capabilities = ["read"]
}

Because the operation is a ListOperation and therefore defined with a trailing slash, but the slash has been stripped.

Hardcoded assumptions about PKI secrets engines

It suggests sudo for paths in the PKI secrets engine that need it, but only if the secrets engine is mounted at pki/ exactly - if mounted at any other path, it does not.

Failure to recommend sudo for paths in namespaces

$ vault read -output-policy ns1/sys/auth
path "ns1/sys/auth" {
  capabilities = ["read"]
}

In summary

  • The current approach of using adhoc regexps hardcoded into the api package is too fragile - a successful implementation of -output-policy needs to be able to query the server to find out if a path is sudo-protected - probably via a new sys/ API. This could also enable it to give a clear indication of which endpoints actually support the create capability.

  • The code needs to understand the difference between a ReadOperation and a ListOperation.

  • Special handling around KV preflight requests is needed.

@lursu
Copy link
Collaborator

lursu commented Feb 21, 2023

I have merged a pr with some mitigations to the first three issues that you called out. However for the later two, after some digging, I agree with your assessment that the policy needs to be derived in the long term from the server side. This is a bit of a larger lift than the time we have for now. So I have created a backlog item for us to convert this flags processing to a server side call this is realistically the only way we could properly handle the more dynamic and context dependent use cases as PKI, namespaces, and mount paths more generally.

@lursu lursu closed this as completed Feb 21, 2023
@lursu lursu reopened this Feb 21, 2023
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/cli devex Developer Experience
Projects
None yet
Development

No branches or pull requests

3 participants