Implement asymmetric cipher support#10152
Implement asymmetric cipher support#10152mattcaswell wants to merge 16 commits intoopenssl:masterfrom
Conversation
|
I strongly support your intention because I need to do the same exercise with the GOST engine. BTW, are there any guidelines available? |
For this PR specifically? No, not yet. Documentation is not yet done (part of the reason this is WIP). If you're after more general provider documentation I suggest you start here: |
I love it when you feign innocence 😉 |
|
CMS seems to be the hardest battlefront, constantly. I've stopped counting the times I got stuck with an issue there... |
On the other hand it provides a good testing ground on what weird API calls can be expected from third party apps. |
I know, right? Infuriating as it can be, it has helped to cut down a number of corner cases! |
02d435e to
eb80aa3
Compare
|
Rebased so that it now merges cleanly with master. |
crypto/evp/pmeth_lib.c
Outdated
There was a problem hiding this comment.
At the very least, I would like to see aliases with rsa_ removed.
There was a problem hiding this comment.
These are legacy ctrls - so I'm not sure why we would add new aliases for them?
There was a problem hiding this comment.
Because -pkeyopt keys will eventually be directly used as OSSL_PARAM keys, and we might as well allow the legacy controls to be accessed with similar names (plus it's a pet peeve of mine... I have never understood why it couldn't simply be "mode", for example)
|
First quick look, looks good. I suspect that some amendment will be needed related to #9979 |
levitte
left a comment
There was a problem hiding this comment.
We're doing double work... I've RSA stuff in my key generation branch 😒
As soon as #10190 is done I'll rebase on top of that is remove any RSA export/import stuff from here. |
This does show that we might do well to split some of our PRs topic-wise... I often try to do that when I discover that something I'm working on contains more general components; I often end up making those into separate PRs, which also helps keep each PR in focus. |
592e875 to
8556358
Compare
|
Please rebase this |
f885681 to
fe21f57
Compare
|
This has now been rebased and all missing documentation has been added. I've taken this out of WIP. Please take a look! |
|
Fixup pushed to address travis failure. |
levitte
left a comment
There was a problem hiding this comment.
Also, didn't we discuss EVP_PKEY_CTX_get_rsa_oaep_md_name and EVP_PKEY_CTX_get_rsa_mgf1_md_name as alternatives that return strings instead of const EVP_MD *?
Oh, yes. So we did. I forgot. Will add. |
|
Fixup commits pushed addressing latest feedback. |
Is that intended to be an approval? @levitte can you reconfirm? I cant approve as Pauli already has - I approve in theory. |
|
Pushed. Thanks! |
Sometimes it is valid to send a NULL pointer in params. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
Sometimes it is useful to be able to pass NULL/zero length strings Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
The old value of 10 for OSSL_PARAM_BLD_MAX is insufficient for multi-prime
RSA. That code has this assert:
if (!ossl_assert(/* n, e */ 2 + /* d */ 1 + /* numprimes */ 1
+ numprimes + numexps + numcoeffs
<= OSSL_PARAM_BLD_MAX))
goto err;
So we increase OSSL_PARAM_BLD_MAX which would be enough for 7 primes
(more than you would ever reasonably want).
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #10152)
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
We have converted a number of macros to functions and made them work with providers. We've also added some *_ex() variants that needed documenting. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)
This PR implements asymmetric cipher support for providers, and moves that aspect of RSA into the default provider.
This is WIP because there are a number of outstanding issues: