vault icon indicating copy to clipboard operation
vault copied to clipboard

Provide public key encryption via transit engine

Open Gabrielopesantos opened this issue 2 years ago • 1 comments

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.

Gabrielopesantos avatar Nov 14 '22 23:11 Gabrielopesantos

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.

victorr avatar Nov 28 '22 19:11 victorr

@Gabrielopesantos is attempting to deploy a commit to the HashiCorp Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Dec 06 '22 22:12 vercel[bot]

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?

stevendpclark avatar Mar 21 '23 18:03 stevendpclark

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 avatar Mar 22 '23 13:03 Gabrielopesantos

@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

stevendpclark avatar Mar 31 '23 14:03 stevendpclark

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)

Gabrielopesantos avatar Apr 03 '23 14:04 Gabrielopesantos

\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.

cipherboy avatar May 11 '23 12:05 cipherboy

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" }

haumanto avatar Sep 05 '23 03:09 haumanto