KV2 engine and restrictive policy leads to strange behaviour

[johnnybubonic]

I have the following policy:

"paths": {
(...)
        "secret/data/foo/bar/*": {
            "allowed_parameters": {
                "x": [],
                "y": [],
                "z": []
            },
            "capabilities": [
                "create",
                "read",
                "update",
                "delete",
                "list"
            ],
(...)
}

The following works fine: vault write secret/data/foo/bar/baz x=1 y=2 z=3.

However, using the same exact token in hvac:

>>> client.secrets.kv.v2.create_or_update_secret('foo/bar/baz', secret = {'x': '1', 'y': '2', 'z': '3'})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/[REDACTED]/.local/lib/python3.7/site-packages/hvac/api/secrets_engines/kv_v2.py", line 122, in create_or_update_secret
    json=params,
  File "/home/[REDACTED]/.local/lib/python3.7/site-packages/hvac/adapters.py", line 106, in post
    return self.request('post', url, **kwargs)
  File "/home/[REDACTED]/.local/lib/python3.7/site-packages/hvac/adapters.py", line 276, in request
    utils.raise_for_error(response.status_code, text, errors=errors)
  File "/home/[REDACTED]/.local/lib/python3.7/site-packages/hvac/utils.py", line 33, in raise_for_error
    raise exceptions.Forbidden(message, errors=errors)
hvac.exceptions.Forbidden: 1 error occurred:
	* permission denied

I did a tcpdump (since it was to an HTTP Vault server on localhost), and found that the Vault CLI client does e.g. (real values have been replaced):

PUT /v1/secret/data/foo/bar/baz HTTP/1.1
Host: 127.0.0.1:8200
User-Agent: Go-http-client/1.1
Content-Length: [REDACTED FOR DEMO]
X-Vault-Token: [REDACTED FOR DEMO]
Accept-Encoding: gzip

{"x":"1","y":"2","z":"3"}

BUT the client.secrets.kv.v2.create_or_update_secret call does e.g. this (actual values replaced):

POST /v1/secret/data/foo/bar/baz HTTP/1.1
Host: localhost:8200
User-Agent: python-requests/2.21.0
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
X-Vault-Token: [REDACTED FOR DEMO]
Content-Length: [REDACTED FOR DEMO]
Content-Type: application/json

{"options": {}, "data": {"x": "1", "y": "2", "z": "3"}}

Notable differences are a POST vs. PUT, and the structure of the payload.

For whatever reason, this causes a permission denied error (403) for hvac, but the vault client succeeds.

At a guess, maybe the docs are wrong? As the Vault client does not seem to conform to the docs (but hvac does).

Are you able to replicate?

[johnnybubonic]

hrm... weirdly, vault kv put ... also gives a permission error. what the hell. i was a dumb and was using foo/bar instead of secret/data/foo/bar hah whoops. kv put works.

[johnnybubonic]

at first i thought this behaviour might be explained if the secret engine was actually KV1 instead of KV2, but that doesn't seem to be the case. vault secrets list -detailed confirms as:

Path                Plugin       Accessor              Default TTL    Max TTL       Force No Cache    Replication    Seal Wrap    Options           Description
----                ------       --------              -----------    -------       --------------    -----------    ---------    -------           -----------
(...)
foo/bar/          kv           kv_[REDACTED]           system         system        false             replicated     false        map[version:2]    Test KV2 store.
(...)

So I have no idea what's going on here.

[jeffwecan]

Looks like I was able to replicate things locally with a test case along these lines: https://github.com/hvac/hvac/compare/issue_389?expand=1

Running it through our travis-ci build process now (which should help clarify if the behavior is different in different releases of Vault or not...): https://travis-ci.org/hvac/hvac/builds/489664686

[johnnybubonic]

Thanks, @jeffwecan. I was worried I was doing another dumb here; I'm still pretty new to Vault. If it's documented wrong in the API docs, I can file the upstream (but obviously the kv.v2 function would still need to be updated as well).

Worth noting that the vault client also makes a call to the UI path on a vault kv get secret/data/foo/bar/baz, for... reasons unknown?

GET /v1/sys/internal/ui/mounts/foo/bar/baz HTTP/1.1
Host: 127.0.0.1:8200
User-Agent: Go-http-client/1.1
X-Vault-Token: [REDACTED]
Accept-Encoding: gzip

Which also isn't documented. It seems the vault client and HTTP API (the docs, at the least) diverge a fair bit.

[jeffwecan]

On that second point, I imagine you're seeing this bit of logic https://github.com/hashicorp/vault/blob/master/command/kv_helpers.go#L43 which we've discussed to some extent within this project under: #383 (comment)

[johnnybubonic]

AH! Indeed, that seems to be it. Thanks!

[johnnybubonic]

following up, forgive me if i'm incorrect in this but based on the travis output, it looks like the failures start with 0.10.4. is this correct? shall i file upstream?

[jeffwecan]

As my apologies, I should have clarified what I expected the build to indicate. As KV v2 was added in Vault v0.10.0, we have the kv_v2 test class set to be skipped when running integration tests against earlier versions of Vault. I.e., I would expect that we'd see the same issue with Vault v0.10.0 and presumably its always been so.

Having said that, I would still agree that an upstream bug report is the reasonable next step to take here. As you alluded to earlier, I imagine it comes down to one of the following two options:

• the upstream hvac documentation is incorrect and needs to be updated
• the documentation correctly captures the intended design and the implementation needs to be updated to match.

In the first case, we would naturally also want to update hvac's implementation to match. 👍

[johnnybubonic]

^ Filed upstream.

[johnnybubonic]

Aaaand upstream closed. They're claiming it's a body formatting issue, even though hvac conforms to KV2 docs.

[jeffwecan]

Hah okay 👌. I'll take a look and aim to get some sort of fix implemented in the next hvac release.

[johnnybubonic]

Thanks!

[jefferai]

@jeffwecan I'm not sure what it means for HVAC and any future work here but it's now very clear that the reporter is writing to a v1 mount, not a v2 mount.

[johnnybubonic]

confirmed, closing. apologies, all.