hashicorp / vault

A tool for secrets management, encryption as a service, and privileged access management
https://www.vaultproject.io/
Other
31.17k stars 4.21k forks source link

Wrong policy priorization when deciding capabilities for a path #10096

Closed thomasd- closed 4 years ago

thomasd- commented 4 years ago

Describe the bug A token is associated with a policy that defines two paths : /secrets/ (list only) and /secrets/+/prefix- (write only). When trying to write a secret to /secrets/alice/prefix-test, we get a permission denied.

The capabilities retained for this is path is list only. The doc says that the most specific path should have priority when resolving capabilities, so the capabilities for /secrets/+/prefix- should be used, not /secrets/

To Reproduce Run the following script:

#!/bin/bash

vault server -dev > vault.log 2>&1 &
VAULT_PID=$!
sleep 2
export VAULT_ADDR=http://127.0.0.1:8200
vault secrets enable -version=1 -path=secrets kv

vault policy write prefix-writer <(echo '
path "secrets/*" {
     capabilities = [ "list" ]
}
path "secrets/+/prefix-*" {
     capabilities = [ "create", "update", "delete" ]
}
')

token=$(vault token create -policy=prefix-writer -format=json | jq -r '.auth.client_token')
vault login -no-print "$token"
vault write secrets/alice/prefix-test value="Hello world!"
vault write sys/capabilities-self paths="secrets/alice/prefix-test"
vault list secrets/alice

kill "$VAULT_PID"

You get the following output :

Success! Enabled the kv secrets engine at: secrets/
Success! Uploaded policy: prefix-writer
Error writing data to secrets/alice/prefix-test: Error making API request.

URL: PUT http://127.0.0.1:8200/v1/secrets/alice/prefix-test
Code: 403. Errors:

* 1 error occurred:
    * permission denied

Key                          Value
---                          -----
capabilities                 [list]
secrets/alice/prefix-test    [list]
No value found at secrets/alice/

Expected behavior I expeted the vault write command to succeed and the sys/capabilitites-self to return permissions [write,update,delete] for secrets/alice/prefix-test. Then the vault list command should show the prefix-test key in secrets/alice

Environment:

Vault server configuration file(s): see self-contained test script

Additional context Add any other context about the problem here.

raskchanky commented 4 years ago

I think the problem here is the trailing glob on line 1 of your policy is confusing the ACL system. If you change your policy to:

path "secrets/" {
     capabilities = [ "list" ]
}
path "secrets/+/prefix-*" {
     capabilities = [ "create", "update", "delete" ]
}

then everything works, and your script provides the output that you expected to see.

At the end of the docs on policy syntax, it mentions:

When providing list capability, it is important to note that since listing always operates on a prefix, policies must operate on a prefix because Vault will sanitize request paths to be prefixes. In other words, policy paths targeting list capability should end with a trailing slash.

I'm guessing that's what caused this problem. I'm going to close this issue for now, since I don't think there's much for us to do here, but feel free to reopen it if necessary with additional information.

Thanks!

thomasd- commented 4 years ago

Thanks for your quick response, but I'm afraid this is not enough.

The list policy is needed, I need to be able to list any folders and sub-folders under /secrets, your solution does block this.

The same documentation page you linked states :

Policy paths are matched using the most specific path match. This may be an exact match or the longest-prefix match of a glob.

It is clear for me that secrets/+/prefix-* is more specific than secrets/* and that you have a bug in your prefix resolution.

Please re-open this bug and investigate.

thomasd- commented 4 years ago

Note : I have updated the test script to add a vault list command.

thomasd- commented 4 years ago

And here is an aternative test script that does not use the list capability

#!/bin/bash

vault server -dev > vault.log 2>&1 &
VAULT_PID=$!
sleep 2
export VAULT_ADDR=http://127.0.0.1:8200
vault secrets enable -version=1 -path=secrets kv

vault policy write prefix-writer <(echo '
path "secrets/*" {
     capabilities = [ "deny" ]
}
path "secrets/+/prefix-*" {
     capabilities = [ "create", "update", "delete" ]
}
')

token=$(vault token create -policy=prefix-writer -format=json | jq -r '.auth.client_token')
vault login -no-print "$token"
vault write secrets/alice/prefix-test value="Hello world!"
vault write sys/capabilities-self paths="secrets/alice/prefix-test"

kill "$VAULT_PID"

And the result :

Success! Enabled the kv secrets engine at: secrets/
Success! Uploaded policy: prefix-writer
Error writing data to secrets/alice/prefix-test: Error making API request.

URL: PUT http://127.0.0.1:8200/v1/secrets/alice/prefix-test
Code: 403. Errors:

* 1 error occurred:
    * permission denied

Key                          Value
---                          -----
capabilities                 [deny]
secrets/alice/prefix-test    [deny]
raskchanky commented 4 years ago

@thomasd- Thank you for the additional details. In digging into this further, I've found a few things:

  1. I don't think our documentation does an adequate job of explaining exactly what's happening under the hood. In looking at the code and examining your specific case, I think secrets/+/prefix-* is getting marked as lower priority than secrets/* because of the +. Generally speaking, if one path has more + than another path, the one with more + is lower priority.
  2. This behavior has been reported by others and we have work scheduled for the very near future to improve the documentation around this, as well as change the behavior to be more consistent with what users expect.

I'm going to leave this issue closed because we're tracking this work internally already, but I really appreciate you taking the time to write up your findings. The bash scripts in particular made reproduction of the problem a breeze. Thanks again!

con-f-use commented 3 months ago

Edit: Nevermind, got it wrong the first time, here's the correct rule application:

According to the documentation on priority matching for the two paths:

we have:


However, I have observed that something with priority matching is still fishy. I have the two path with Vault v1.15.4:

where according to docs:

yet it seems that P1 is applied in my case. I cannot update ssh/roles/qa.

heatherezell commented 3 months ago

Edit: Nevermind, got it wrong the first time, here's the correct rule application:

According to the documentation on priority matching for the two paths:

  • P1: secrets/*
  • P2: secrets/+/prefix-*

we have:

  • Rule one: P1 and P2 have their earliest wildcard (+ or *) at the same place, so this rule does not apply --> go to next rule
  • Rule two: both paths end in *, rule does not apply --> go to next rule
  • Rule three: P2 has more + wildcard segments --> P2 is of lower priority, P1 is applied

However, I have observed that something with priority matching is still fishy. I have the two path with Vault v1.15.4:

  • P1: ssh/* --> list, read
  • P2: ssh/*/qa --> list, read, update

where according to docs:

  • Rule one: P1 and P2 have their earliest wildcard (+ or *) at the same place, so this rule does not apply --> go to next rule
  • Rule two: P1 ends in *, so it should have lower priority, P2 should get applied

yet it seems that P1 is applied in my case. I cannot update ssh/roles/qa.

Would you be willing to open a new issue for the documentation? I hesitate to resurface this one because the context is so old. Thanks!