vault
vault copied to clipboard
Provide public key encryption via transit engine
Issue: https://github.com/hashicorp/vault/issues/7256
Description:
Added support to import public keys (RSA, ECDSA) in transit engine.
For that, an additional field public_key
was added to the "Import Key" endpoint and if filled, will import a public key. If ciphertext
is also filled, public_key
is ignored and only the private key is imported.
On the "Import Key Version" endpoint, three new fields were added, public_key
, public_key
and bump_version
. version
has the same behavior as in "Import Key" endpoint. bump_version
defines if a new key version is going to be imported or an existing version updated. Is set to "true" by default to stay aligned with the current behaviour so each operation creates a new key version instead of trying to update an existing version. If set to "false", the value of "version" is taken into account and is the key version to be updated.
"Encrypt" endpoint was also adapted to make use of the public key if the private key isn't in the KeyEntry.
I am not sure if this PR is finished as there might still be some transit endpoints that need to be slightly changed given the fact that can exist keys without Key
or RSAKey
filled which can lead to unexpected errors (I think). Some documentation might need to be updated and test cases that I haven't considered.
Hi @Gabrielopesantos, ~~I will be reviewing your PR~~, thank you for submitting it.
In the meanwhile, can I suggest that you please update the relevant documentation? You can find the API documentation under website/content/api-docs/secret/transit.mdx. File website/README.md has useful information about writing documentation, but it should be a straightforward process to perform the updates.
Update: I have unassigned myself from the PS since others are already reviewing.
@Gabrielopesantos is attempting to deploy a commit to the HashiCorp Team on Vercel.
A member of the Team first needs to authorize it.
Hello @Gabrielopesantos, truly sorry about the delay getting back to this. I'm starting to re-review your changes now, would you mind updating this PR and resolving the conflicts that have occurred due to my delay in getting back to you?
Hey Steve,
No worries, thank you for the feedback. I will update the PR and merge the conflicts as soon as I can. And, as you also left already left some comments, I will also address them.
EDIT: I believe all PR conflicts have been addressed. UPDATE: All comments have also been addressed.
@Gabrielopesantos Looks great, I'd say this is in a good spot now code-wise, perhaps just a test or two around the export would be nice to have.
The only remaining thing I'd say would be to clean up the note thingy I posted about above, and resolve the TODO's in the docs
About merging the PR in its current state, there are still some things missing and others that could be improved but I am ok with that if you guys think it's best. The PR is also a bigger than what is usually expected, at least in my opinion.
I would gladly work on what's missing once you decide what exactly you expect.
(Forgot the send this message the other day)
\o hey @Gabrielopesantos! Congrats on the merge and sorry about the delay :-) Wasn't sure if we'd get the time to get this in this next release or not.
Hi,
I keep getting error when trying to impor this public key. Please help.
{ "type": "rsa-2048", "public_key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAt1jJLdS4xnClx0Cy/JyaduQE6vGhy8cHz/t3DcZyqFSPG3BfSUAytSWJWj/XS0SdIIq1D/peO7FQnSF4oiARhmO0U9QXE79WKXzZEXaTNMPr7bVDtv7CpfMv8yjVgiVhc2Riv2/+zTtjmp05Yl0XAZT57O9Sbvqnc1UlHD/kwfTKEV1TAbdy81sfYf5w1SKUUQDhoTNmpTm/GdhqFczuoOBDKjjlLV/1G3niuAPlC06wAYaHKNpn2SDa7P0XNQgYTA6CuIFqH/gMfai0CVsTbUzNIKCB/p9/n+5G1vszV+hufasx3xfns7Mb3SEshLLsczt2IhefU8txUwW4Bhrf8QIDAQAB" }