PATCH API needed for updating multiple secrets stored at a single URI path

[jaxley]

Currently, Vault does not have any PATCH API for updating single fields when storing multiple fields at a given URI path. A common use case is to just update a single value. To do this, it means that every consumer of the API for updating values has to do one of several possible things that are each problematic in their own right to avoid corrupting pre-existing values:

• correctly implement logic to fetch all existing fields at the URI path; merge locally; submit back (requires 2 API calls for every update)
  • Potentially complex logic that would have to be replicated in all clients across multiple platforms.
  • ideal to not have to do a network read for every update.
• Require the end-user to re-store all stored secrets at a URI path even when only needing to update a single value
  • may encourage users to store secrets outside of vault just so updates can be done. Would also encourage/require granting the "read" ACL to users when write-only would be more secure.
  • also has audit implications. Ideal to have fine-grained audit for which secret(s) users are accessing but if all have to be retrieved, then all would be potentially exposed instead of a single value needed.
• Expose all existing stored secrets to the user and have them manually change one value and resubmit all back.
  • Undesirable as it requires exposing all secrets to update just one. Also puts the burden on avoiding corruption on the user, likely leading to more human error.

Having Vault provide secure, reliable storage and ideally being the only place where these sensitive secrets reside means that any chance of corrupting the data which could be difficult or impossible to recover from without recreating all secrets and re-entering them.

It would be best if Vault provided a PATCH API that could be used to ensure that updates to multiple values at a path are done properly and with minimal chance of data corruption.

[jefferai]

Hi @jaxley,

This is something that is on our radar already!

[jaxley]

Didn't see anything in the issues, is there a reference?

This is related to #272

[jefferai]

No, it's on our internal roadmap already.

[jefferai]

@jaxley As your request is entirely separate from the original request on #272 I'm going to delete the comment there to not muddy the waters.

[jaxley]

But they are not separate though - if you use a model of only a single secret per URI path to avoid the data corruption/exposure issues caused by the lack of PATCH API support, then you suffer from performance issues of having to request secrets one-at-a-time. Therefore, the batching would alleviate this read performance issue.

[jefferai]

@jaxley Batching introduces its own potential performance issues, which is why it hasn't been written. PATCH would help in some use cases, but it's really a separate concern -- you could have PATCH support but still want to read 100 entries at once. That's why PATCH is on our roadmap but multi-get currently isn't.

[vishesh92]

+1
Any idea by when will it be available?

[chen01]

+1 This will be extremely helpful, any news on when it's going to be ready?

[jefferai]

There are currently no plans for this feature.

[duijf]

Chiming in with another datapoint that this would be a really important ergonomic improvement. At work we have been bitten by the lack of a PATCH API two times now (we now have our own wrapper script that has patch semantics).

[ixe013]

Hey @duijf, how do you manage write-after-write race conditions?

1. A read_key
2. B read_key
3. B partial_write_key
4. A partial_write_key

B partial write will be lost...

[Nowaker]

Any news on this?

[danlsgiga]

Wondering how is this coming along. I could use key/value pairs in vault but when using templates in nomad to grab multiple values, it becomes a lot of repetitive code.

For example, this is way clear...

{{ with secret "secret/php"}}
              $dbhost = '{{ .Data.dbhost }}';
              $dbuser = '{{ .Data.dbuser }}';
              $dbpass = '{{ .Data.dbpass }}';
              $dbname = '{{ .Data.dbname }}';
{{ end }}

than this...

{{ with secret "secret/php/dbhost"}}
              $dbhost = '{{ .Data.value }}';{{ end }}
{{ with secret "secret/php/dbuser"}}
              $dbuser = '{{ .Data.value }}';{{ end }}
{{ with secret "secret/php/dbpass"}}
              $dbpass = '{{ .Data.value }}';{{ end }}
{{ with secret "secret/php/dbname"}}
              $dbname = '{{ .Data.value }}';{{ end }}

If there's a way of doing an "UPSERT" in the key it would be awesome!

Just my .2 cents.

[jefferai]

@danlsgiga That doesn't seem related to a PATCH API, merely the templating engine being used.

[danlsgiga]

@jefferai Not exactly.

What I'm looking for is to have a way of updating k/v in vault without the need to worry about previous k/v stored on a specific URI path. That will allow me to have confidence on using something like:

# vault write /secret/php dbhost=xxx dbpass=yyy dbuser=www dbname=zzz

instead of

# vault write /secret/php/dbhost value=xxx 
# vault write /secret/php/dbpass value=yyy 
# vault write /secret/php/dbuser value=www
# vault write /secret/php/dbname value=zzz

without losing data.

This is very importat specifically in a company where developers are still getting used to vault and are very error prone on doing vault write secret/php dbpass=xxx and losing all the other k/vs in that same path, like I did! =)

Anyhow, after some research I was able to do what I wanted without repetition and using the Hashicorp best practices the way it is now.

{{ range secrets "secret/php/" }}
${{ . }} = {{ with secret (printf "secret/php/%s" .) }}"{{ .Data.value }}";
{{ end }}{{ end }}

[jefferai]

Ah, I see, I thought in your template in the previous comment you were reading, not writing. But looks like that range statement should be great!

[danlsgiga]

It is! Works beautifully! ;)

[james-lawrence]

@ixe013 tbh a partial write being lost is better than losing all the values. so they probably don't need solve that issue.

imo as it is now the multiple fields per key isn't useful unless patch is implemented because of how error prone it is.

for the record here is how we are working around the issue:
vault read -field=environment secret/dev/example | vipe | vault write secret/dev/example environment=-

[james-lawrence]

random side note: this could be implemented by storing an incrementing version value that gets checked on the update call combined with merge semantics of the top level keys.

[hamstah]

Hey guys

We just wrote a small tool to help work around this issue until this is resolved
https://github.com/paywithcurl/vault-update

Obviously, the issue of race conditions mentioned above still apply, but should still be useful for most people who don't have secrets changing all the time.

Thanks

[Nowaker]

Thanks Nicolas! Great job.

[dragon788]

This is definitely annoying, luckily I was adding an application secret that didn't yet get accessed by anything when I clobbered the existing properties by doing a write to the same overall path with a unique property name.

I was rather confused until I searched and ran across this issue and #182 tracking it, as typically individual attribute/property updates don't destroy data in most programming models.

[jefferai]

as typically individual attribute/property updates don't destroy data in most programming models.

Vault takes in the entire blob, not individual attributes. It's not destroying data, it's replacing it.

[james-lawrence]

@jefferai everyone understands if they look at either the code or find this issue what is actually happening, no one is disputing that.

The problem is the behaviour is very surprising based on the CLI, and reasonable expectations. Lets stop discussing the state of the world at the moment and move onto how we can fix this irritating behaviour for a common use case.

easiest fix would be to add a vault update secret/foo k1=v1 k2=v2 that does a read, merge, write. then we document the current limitations to the command given the current architecture (i.e. race conditions).

vault is general has a general problem with a pretty horrible user interface, and the common response has been relative indifference to these class of issues.

[dragon788]

@jefferai To be fair I haven't read the code, but in reading and extrapolating from the documentation when it states I can pass in separate json files for the various keys I am storing under a path, it doesn't scream "this is getting crammed into a single blob", it really makes it seem like you could do individual bits. I get what you are saying about the blob update mechanism, and it makes sense to avoid a race condition of multiple things trying to write updates to a path for different k/v pairs, but a vault native patch wrapper as proposed would be a fantastic addition.

Something like this in the documentation was where I got confused.

vault write secret/application/jdbc [email protected] [email protected] [email protected]

Potential use case:
If I stored these json files in say an encrypted S3 bucket and wanted to use the S3 "file changed" trigger to trigger a runbook to update vault without allowing users to directly interact, I basically have to write a special handler to sync and pass ALL the files related to this jdbc entry in again, even though only one portion may have changed instead of simply sending "file password.json updated" which would trigger a vault write secret/application jdbc $(basename $somepropvar .json)=@$someprodvar where I could reuse the code to update the corresponding k/v by using a simple naming convention.

[jefferai]

but in reading and extrapolating from the documentation when it states I can pass in separate json files for the various keys I am storing under a path, it doesn't scream "this is getting crammed into a single blob"

I think you are confusing "key" as in the path in the K/V store with keys of the map stored at that location. You can pass in separate JSON files for various path keys.

[Nowaker]

@jefferai I think y'all (Vault developers) are missing the point... Everyone knows what the key is and what the value is. The community wants a usability improvement. Being able to work on secrets like requested in this ticket would be a tremendous improvement for the better for every user of Vault. In fact it's one of most upvoted issues (5th). Thank you in advance for considering this!

[jefferai]

@Nowaker I don't believe the previous poster did in fact understand the difference between keys and values. Helping out a confused user does not mean that I am missing the point of the ticket.

The point is well understood.

[jefferai]

We've been deliberating this for a long time internally, and honestly, it's not going to happen.

There are too many edge conditions where it's hard to know the right thing to do in each situation (for instance: when PATCHing, does the absence of a value mean you want to clear it or that you aren't changing it? If you set the value to empty, are you trying to clear it or actually set it to empty but keep the key? If using null, what about with a bool if you want to clear the fact that it's set but make it clear that it actually hasn't been set?)

In addition with the upcoming versioned K/V functionality we will support check-and-set (which will take care of some of the asks in this thread) but also make PATCHing even more complicated since the CAS functionality operates at the key level.

Sorry...closing this.

[james-lawrence]

(for instance: when PATCHing, does the absence of a value mean you want to clear it or that you aren't changing it? If you set the value to empty, are you trying to clear it or actually set it to empty but keep the key? If using null, what about with a bool if you want to clear the fact that it's set but make it clear that it actually hasn't been set?)

you make it sound like these are unsolvable issues, they arent they just require a design decision.

when PATCHing, does the absence of a value mean you want to clear it or that you aren't changing it?

that you are not changing it, if we want to clear it we have the current mechanism of clobbering the entire key.

If using null, what about with a bool if you want to clear the fact that it's set but make it clear that it actually hasn't been set?)

this is no different then the above. again mountains of mole hills.

[jefferai]

#4432

[james-lawrence]

BTW thanks for implementing this, its been a huge improvement.

[pubudusj]

@jefferai Thanks for this feature, really useful.
Is this same available for kv v2 API?

[jefferai]

No...it's just some syntactic sugar that uses the API's features.

[sagitsofan]

So how can i update a specific key using the API ?

[darmach]

Is PATCH #4432 available in http API?