Vault: SHA1 Padding on KeyType_RSA (Transit Key)

[higonetr]

Hi!

We import a key that we alredy have in production to the vault. Tweaking a little the backup file from a key, We succeded import our key.

But that key is being used for some payments devices that encrypt the data with a SHA1 padding. So we can't not be able to use decrypt function from the vault in our application.
So we need to change the source code from the vault, in \sdk\helper\keysutil\policy.go File on lines 904 and 1038. Because SHA256 was hardcode in one of the parameters on the function. So we just replace that with SHA1 and everything works.

But making optional the type of padding in transit keys would be a great feature.

Thanks!

[catsby]

Hello - could you provide a bit more detail on how you're importing your key, and the workflow you're using to encrypt and decrypt things? An example of the steps to reproduce this would be helpful, especially if you can share parts of the error messages if you are getting any.

Thanks!

[catsby]

Just to be clear, you're requesting we use sha1.New() in place of sha256.New() as the first parameter of the function call on this line:

https://github.com/hashicorp/vault/blob/c5ae51e59762102d7a07701d785d3f84475855db/sdk/helper/keysutil/policy.go#L904

If I understand correctly then it's very unlikely that change would be a good idea, as crypto/sha1 is documented to be "cryptographically broken and should not be used for secure applications."

[higonetr]

We are not getting any error when we import the key. We did it correctly.

We've some devices that not supported another RSA-OAEP padding than SHA1. So for be enable to use all functionality of Transit Vault Keys we tuned the source code.
But how the SHA padding is the first parameter of EncryptOAEP DecryptOAEP functions, it's not dificult to add that option to transit key settings.
It's easier change that functions than to change all devices from production.

Thanks for response!

[catsby]

Hello - adding an option to choose the hash function given is likely easy enough, sure, however I'm not convinced enabling a "cryptographically broken" configuration setting for users is a good idea.

[catsby]

I'm going to close this issue for now, as I still don't believe it's something we should do. If anyone has more supporting information or reasoning for doing so please follow-up here. Thanks!

[DorianGreen]

SHA1 for OAEP padding has no real disadvantage compared to SHA2.

Granted, SHA2 is more secure, but that is arguably a non issue when padding RSA encrypted data.

If the key is to be compatible with Microsoft key storages like CAPI or even Azure Key Vault, then an option to set the padding hash algorithm to SHA1 is required.