Add support for RSA-OAEP decryption #4
Conversation
Changes from mozilla-services#48 are included in this commit. Slightly adapted to fit with how we work with `crypto.Decrypter`.
Currently the test suite still goes back as far as Go 1.12. This failed the test.
| // echo -n "This is a test" > test.txt | ||
| // - Generate PKCS #7 enveloped data encrypted using AES using | ||
| // openssl smime -encrypt -in test.txt -aes256 -outform pem certificate.pem | ||
| var RSAPKCS1v15EncryptedTestFixture = ` |
There was a problem hiding this comment.
the embed package could help you out here.
There was a problem hiding this comment.
This repo is still tested against Go 1.12, because it's a fork. I've added a note instead to look into that when the version is upped to at least 1.16. We're getting closer to actually maintaining the fork for others to be used, so after that we can think about upgrading Go versions, I would say.
There was a problem hiding this comment.
You could put these in files under testdata and load them during TestMain or init in the test file.
There was a problem hiding this comment.
It's a good idea, but I propose to keep it like this for a bit, as it's the format used by the upstream and I took their tests. There's some more cases not currently covered by tests, so I want to revisit this soon, but do it with embed at once.
| @@ -317,7 +317,7 @@ func UnmarshalTestFixture(testPEMBlock string) TestFixture { | |||
| result.Input = derBlock.Bytes | |||
| case "CERTIFICATE": | |||
| result.Certificate, _ = x509.ParseCertificate(derBlock.Bytes) | |||
| case "PRIVATE KEY": | |||
| case "PRIVATE KEY", "RSA PRIVATE KEY": | |||
There was a problem hiding this comment.
The consensus is to use PRIVATE KEY for PKCS#8 and RSA PRIVATE KEY for PKCS#1.
There was a problem hiding this comment.
Adding "RSA PRIVATE KEY" was required for the test cases from the existing PRs that I ported over to here.
There was a problem hiding this comment.
Your change is ok, but if you get PEM with PRIVATE KEY is very likely that is encoded using PKCS#8
$ openssl genrsa -out rsa.pem
$ head -1 rsa.pem
-----BEGIN RSA PRIVATE KEY-----
$ openssl pkcs8 -topk8 -in rsa.pem -nocrypt | head -1
-----BEGIN PRIVATE KEY-----
There was a problem hiding this comment.
I'm ok leaving this for backward compatibility. I don't think we are using this path.
There was a problem hiding this comment.
It's also just in a _test.go file to extract that from the hardcoded PEMs. We could replace the tests if we want to.
Addressing most of the review comments.
I think keeping the core I've also thought about changing some details of this package, as there's a couple of things I don't really like, such as how encryption algorithms need to be set globally before encrypting. Those would break the current API, so it might be possible to put that in a safe wrapper around this lib and put that in |
Sure, we can keep the pkcs7, and if we add a wrapper into crypto we should include the PKCS#8 thing. |
Noted! |
Changes from mozilla-services#48 are included with this PR. Slightly adapted to fit with how we work with
crypto.Decrypter.Changes from mozilla-services#44 for decryption are included with this PR.