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

READ capability on uppermost level is needed to be able to delete a secret anywhere in the tree #8456

Closed
ofayans opened this issue Mar 3, 2020 · 8 comments
Labels
bug Used to indicate a potential bug ui
Milestone

Comments

@ofayans
Copy link

ofayans commented Mar 3, 2020

Describe the bug
Given a team member has a full access to one of the secret subfolders in vault, for example, secret/nemo/dev-master/
The corresponding part of the policy looks like this:

path "secret/data/nemo/dev-master/*"
{
  capabilities = ["read", "list", "create", "update", "delete"]
}

The problem is that this team member is able neither to delete the secret, residing under this path, nor see the secret version history (there are simply no corresponding buttons in the UI)
However, if I add this team member the capability to read on the uppermost level, then suddenly this functionality becomes available to him:

path "secret/*"
{
  capabilities = ["list", "read"]
}

But the thing is, we don't want to give everybody READ access to the whole secret tree, it makes no sense.

To Reproduce
See description

Expected behavior
Defining full acces to to a secret subfolder enables full access!

Environment:

  • Vault version: 1.3.2
  • Vault CLI Version irrelevant
  • Server Operating System/Architecture: vault running in official Hashicorp's docker image

Vault server configuration file(s):

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

listener "tcp" {
  address = "0.0.0.0:8200"
  tls_disable = 1
  x_forwarded_for_authorized_addrs = [
    "10.0.0.0/24"
  ]
  x_forwarded_for_hop_skips = "1"
  x_forwarded_for_reject_not_authorized = "false"
  x_forwarded_for_reject_not_present = "false"
}

ui = true
disable_mlock = true
@catsby catsby added bug Used to indicate a potential bug ui labels Mar 4, 2020
@chrishoffman chrishoffman added this to the 1.5 milestone Mar 10, 2020
@Monkeychip
Copy link
Contributor

@ofayans I think the issue has to do with having "data" in the kv path. The ACL policy should have "data" in the path, but not in the kv path. See more information in the kv acl rules doc here.

Let us know if this resolves your issue.

Thank you!

@ofayans
Copy link
Author

ofayans commented Apr 1, 2020

@Monkeychip I'd like a more detailed explanation as to why this bug was closed. The described behavior is obviously a bug. Why must I have read permission for the whole "secret" folder to be able to delete individual secrets?

@ofayans
Copy link
Author

ofayans commented Apr 1, 2020

@Monkeychip, or at least you guys should update official policies documentation, because it simply contradicts with the document you are referring to.

@Monkeychip
Copy link
Contributor

@ofayans. The ticket was closed because the behavior is expected. It behaves the same way in the UI as it does in the CLI. One point I should have made in my first comment was that you don't need read permission to the whole secrets folder. Instead, use the metadata path, mentioned here in the docs.

Replace:

path "secret/*"
{
  capabilities = ["list", "read"]
}

With:

path "secret/metadata/nemo/dev-master/*" {
  capabilities = ["list"]
}

Additionally, in the UI, if you use the above "secret/metadata/nemo/dev-master/*" policy path, you will only be able to navigate to the direct URL (e.g. ui/vault/secrets/secret/list/nemo/dev-master/). If you'd like to navigate to the correct path via the UI use, "secret/metadata/*".

@ofayans
Copy link
Author

ofayans commented Apr 29, 2020

@Monkeychip
Okay, I probably did not put my question clear enough. The question is:
In the following two examples why does the first "delete" work and the second one does NOT:

path "secret/*"
{
  capabilities = ["list", "read"]
}
path "secret/blahblah/moreblahblah"
{
  capabilities = ["read", "list", "update", "delete"]
}
path "secret/*"
{
  capabilities = ["list"]
}
path "secret/blahblah/moreblahblah"
{
  capabilities = ["read", "list", "update", "delete"]
}

I mean, they either should not work both, or should work both, but not only one of them

@Monkeychip
Copy link
Contributor

Monkeychip commented May 8, 2020

@ofayans I may be misunderstanding your question, but when I have the following policy I am able to delete a secret.

# Policy 
path "secret/*" {
  capabilities = ["list"]
}

path "secret/data/nemo/dev-master/*"
{
  capabilities = ["read", "list", "create", "update", "delete"]
}

Here is an image of my policy next to my terminal output.
image

@ofayans
Copy link
Author

ofayans commented Jun 4, 2020

@Monkeychip through CLI - yes, but not through UI. With this policy you would be able to neither delete the secret nor manage it's versions in vault UI. It's simply inconsistent with the CLI capabilities, that's all this bug is about.

@Monkeychip
Copy link
Contributor

Monkeychip commented Jun 9, 2020

@ofayans OK. I figured out why you can't delete in this specific case from the UI. When you have your policy, you are only able to delete a secret version via the following CLI and respective CURL command:

# CLI command
vault kv delete secret/nemo/dev-master/test

# CURL command
curl \
	--header "X-Vault-Token: ..." \
	--request DELETE \
        https://127.0.0.1:8200/v1/secret/data/nemo/dev-master/test

However, the two commands the UI uses for deleting a secret are as follows:

# CLI command to delete a specific secret version
vault kv delete -versions="2" secret/nemo/dev-master/test

#CURL command to delete a specific secret version
curl \
    --header "X-Vault-Token: ..." \
    --request POST \
    --data '{ "versions": [2] }' \
    https://127.0.0.1:8200/v1/secret/delete/nemo/dev-master/test

Here is gif of deleting a specific version via the UI. This is viewing with a root policy.

The other kind of delete you can do in the UI is to delete the whole version history. Again here are the respective CLI, CURL, and UI of this action.

# CLI command to delete all versions and metadata
vault kv metadata delete secret/test

#CURL command to delete a specific secret version. Notice the "metadata" inside the path, which is different than the "data" in example 1 
curl \
    --header "X-Vault-Token: ..." \
    --request DELETE \
        https://127.0.0.1:8200/v1/secret/metadata/nemo/dev-master/test

We have two places in the UI you can execute this command.

To summarize: We have 3 delete actions in the API (e.g. soft delete, version specific delete, and metadata delete). However, in the UI we only show #2 and #3, we are missing the soft delete option, which given your policy is the only delete option you have available to you. This is why you can't delete via the UI. I am communicating with design, and we are discussing how we want to approach this. Thank you for your continued communication on this. I'll message back when we figure out next steps and if we do proceed with changes, in what version those will be released.

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 ui
Projects
None yet
Development

No branches or pull requests

4 participants