cubbyhole entries not getting removed on token expiration

[ric03uec]

Describe the bug
When using postgresql backend with cubbyhole, the entries in postgresql are not getting removed once the token expires, causing db size to increase with each cubbyhole operation. vault cli reports incorrectly that the entry no longer exists.

To Reproduce
Steps to reproduce the behavior:

1. Install vault using postgresql backend
2. Check the size of vault table in postgres after initializing and unsealing vault

psql> select count(*) from vault_kv_store ;
 count 
-------
    17
(1 row)

2. create a short-lived token using following commands

$ cat tokenData.json
{
  "ttl": "2m",
  "renewable": false,
  "policies": [
    "default"
  ]
}
$ curl -XPOST http://127.0.0.1:8200/v1/auth/token/create -H 'X-Vault-Token: <vault_root_token_here>' -H 'content-type:application/json' -d @tokenData.json
{
  "renewable": false,
  "lease_id": "",
  "lease_duration": 0,
  "data": null,
  "wrap_info": null,
  "warnings": null,
  "auth": {
    "client_token": "<short_lived_client_token>",
    "accessor": "<some_accessor_value_here>",
    "policies": [
      "default"
    ],
    "metadata": null,
    "lease_duration": 120,
    "renewable": false
  }
}
# check size of table
psql> select count(*) from vault_kv_store ;
 count 
-------
    21
(1 row)

3. Save some data into cubbyhole mount using the short lived token

$ cat secretData.json
{
  "user_name": {
    "sshPrivateKey": "ooh_look_a_private_key_is_here"
  }
}
$ curl -XPOST http://127.0.0.1:8200/v1/cubbyhole/path/to/short/lived/bucket -H 'X-Vault-Token: <short_lived_client_token>' -H 'content-type:application/json' -d @secretData.json

# check size of table
psql > select count(*) from vault_kv_store ;
 count 
-------
    22
(1 row)

4. Wait for 2 minutes for token to expire
5. Check cubbbyhole

$ vault login <short_lived_token>
Error authenticating: error looking up token: Error making API request.

URL: GET http://127.0.0.1:8200/v1/auth/token/lookup-self
Code: 403. Errors:

* permission denied

# check size of table
psql > select count(*) from vault_kv_store ;
 count 
-------
    18 <--- Should ideally go back to 17
(1 row)

# check table entries
psql> select * from vault_kv_store  where parent_path like '%short%';
-[ RECORD 1 ]-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
parent_path | /logical/<some_string>/<another_string>/path/to/short/ <--- should this even exist ?
path        | /logical/<some_string>/<another_string>/path/to/short/lived/
key         | bucket
value       | \x<very_long_string_of_data>

Expected behavior

1. Token expiration for cubbyhole should ideally remove all the entries from database associated with that cubbyhole. In the example, the final count of db entries should be 17, not 18.

Environment:

• Vault Server Version (retrieve with vault status): 1.0.0
• Vault CLI Version (retrieve with vault version): Vault v1.0.0 ('c19cef14891751a23eaa9b41fd456d1f99e7e856')
• Server Operating System/Architecture: Ubuntu 16.04.2 LTS (xenial) 4.4.0-1022-aws x86_64 x86_64 x86_64 GNU/Linux

Vault server configuration file(s):

backend "postgresql" {
  connection_url = "postgres://db_user:db_password@db_ip:db_port/db_name?sslmode=disable"
}

listener "tcp" {
  address = "0.0.0.0:8200"
  tls_disable = 1
}

Additional context

• If only the token is created (without any cubbyhole associated with it), all associated entries in the database are getting cleaned up when the token expires.

[tyrannosaurus-becks]

I poked at this and found that this line is not resulting in the same cubbyholeID as was used to save the secret. The cubbyholeID is generated randomly so will have to come from storage.

When I simply update the token store to always revoke the cubbyhole secret using the te.CubbyholeID, it causes the deletion to be successful and solves the problem in this issue reproduction. However, that approach to fixing it causes the following tests to fail:

TestTokenStore_HandleRequest_RevokeOrphan
TestTokenStore_HandleRequest_Revoke
TestTokenStore_HandleRequest_RevokeAccessor

It's because none of the tokens used for revoking in those cases have a CubbyholeID coming out of storage with them.

Should we update the code to add a cubbyhole ID to all the token entries in storage?

[jefferai]

We should use the cubbyhole id if it exists and fall back if it does not, akin to the other places we have to deal with the ID