Make public_key.pkcs7.pem a RSA public key instead of a X509 certificate #367
Conversation
Every combination including Ruby 2.6 was excluded already.
In PKCS7 RFC, the recipient certificate is not mandatory when decrypting. This is also how it is implemented in OpenSSL PKCS7_decrypt(). However, it is only since version 2.2.0 of ruby-openssl that it is possible to call OpenSSL::PKCS7#decrypt with only the private key. Ref: ruby/openssl#183 The issue of hiera-eyaml requiring the public key when decrypting has been brought before in voxpupuli#137, but ruby-openssl was yet patched.
OpenSSL PKCS7 encrypt function needs a list of X509 certs to encrypt and sign data. However, in the context of hiera-eyaml, the certificate serves no purpose. For the user, it is easier to understand that eyaml deals with a keypair. Internally, we wrap the public key in a X509 dummy certificate to call PKCS7 encrypt but the user does not have to be exposed to it.
|
I'm 👎 on breaking changes unless they deliver an exceptionally strong benefit or prevent a significant harm. After looking through this patch, it appears that So, I'm with Linus on this one: "don't break userland" |
|
Hi!
@cmd-ntrf which documentation do you have in mind?
Yes. But I think that's the reason why you can generate everything via I don't mind the additional support for plain private + public key, if the diff is easy to review and doesn't break existing setups. |
|
A tool like cfssl can also be used to generate self-signed certificates with much less hassle than |
|
First, thank you for taking the time to review this properly. I appreciate it. I understand it is a breaking change. Let me try to answer your interrogations the best I can and I am open to discussion on how we could me make it break less things. My work on this started when asking myself why does hiera-eyaml requires the "public key" to decrypt. I found a partial solution to this and made PR #364. Then, I kept digging and realized the public key was not only a public key but a certificate. I also found issue #137 where @gtmtech said this about public key being required:
So 9 years later, I decided to investigate and give a shot at a remedy. @bastelfreak : > which documentation do you have in mind? @Sharpie: > it appears that hiera-eyaml will no longer be able to load existing .pem files that are in wide use today in production systems Correct me I understand this improperly, but if we were to introduce backward compatibility with previous the previous public key format, it would make this PR acceptable? If so, I am confident that can be arranged. |
|
Quick update: I think it is possible to entirely remove that serial number constant by retrieving the certificate serial number value from the PKCS7 data object. This means decrypt would no longer require the public key, and therefore its format would no longer matter. There is a breaking in the test for Ruby 2.7, but I am confident I can fix it. As for the encrypt function, it would be possible to support both the X509 certificate and the RSA public key format. This would no longer make this a breaking change PR, but a feature PR to support RSA public key for encrypting. |
|
Preserving backwards compatibility would be a requirement. However, I'm generally not a fan of complicating cryptographic workflows with branching logic --- especially when there are ways to generate the required input. |
|
The PR is now backward compatible.
|
|
It looks like 2 years ago I also got confused by the behavior of this backend, but I finally just did a copy of the code and adjusted it to use bare RSA keys: https://github.com/smortex/hiera-eyaml-rsa At that time I was thinking that this module would make a lot of sense if it was able to handle a bunch of public keys (e.g. 1 for the puppet sever and 1 for each sysadmin) and when encrypting a secret allow to choose who will be able to decrypt it (e.g. the puppet server, and a selected group of admins). In a bolt context where multiple small teams might need to have access to some secrets that should not be available to other teams, this could be handy. |
|
@smortex : I started going down a similar route before stopping when I realized that there is a limit to the number of bytes that can be encrypted with an RSA key. It depends on the keysize, but for the default 2048 bits, a RSA only backend could only encrypt 214 bytes data. While the PKCS7 backend does support encrypting files of arbitrary size. |
|
To help with review and avoid confusion, I am splitting this PR in three independent components and separate PR:
|
OpenSSL PKCS7 encrypt function needs a list of X509 certs to encrypt and sign data.
In hiera-eyaml, the certificate is only a vehicle to wrap the public key in order to use PKCS7 encrypt/decrypt functions. It has no signature checking function since the certificate serial number is harcoded to 1. All other attributes configured for the certificate are not involved in the decryption i.e: subject, start date, end date, digest, extensions.
In this PR, we propose to replace the format of the public key file from an X509 certificate with serial number 1, to an RSA public key file. Internally, we wrap the public key in a dummy certificate with an hardcoded serial number.
For the hiera-eyaml user, it is easier to understand that eyaml deals with a keypair. This is also what is documented. It is also make it easier for users to generate a keypair with openssl CLI.
We moved from serial number 1 to serial number 0 to make all tests pass. The test encrypted data were originally signed with a certificate serial number 0. The harcoded serial number was changed in commit f9fde79, but the encrypted data and the key files were not regenerated. Since it's been more than 10 years where the serial number has been
1, if this PR is accepted, we would updated the test data to make it sign with serial number 1 instead.To upgrade, If the data has been encrypted with certificate serial number 1 (in the last 10 years), a user would only have to only have to extract the extracting the public key from the private key file: