Implement asymmetric cipher support #10152

mattcaswell · 2019-10-11T16:54:54Z

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:

Test failure in CMS for unknown reasons
There's a weird memory issue I need to investigate
It currently doesn't merge with master because of @levitte moving everything around under my feet :-)
Needs some documentation updates

beldmit · 2019-10-11T17:07:05Z

I strongly support your intention because I need to do the same exercise with the GOST engine. BTW, are there any guidelines available?

mattcaswell · 2019-10-11T17:16:50Z

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:

https://www.openssl.org/docs/manmaster/man7/provider.html

levitte · 2019-10-11T20:50:02Z

It currently doesn't merge with master because of @levitte moving everything around under my feet :-)

I love it when you feign innocence 😉

levitte · 2019-10-11T20:57:41Z

CMS seems to be the hardest battlefront, constantly. I've stopped counting the times I got stuck with an issue there...

t8m · 2019-10-14T14:42:39Z

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.

levitte · 2019-10-14T15:05:05Z

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!

mattcaswell · 2019-10-14T16:28:36Z

Rebased so that it now merges cleanly with master.

levitte · 2019-10-14T19:37:39Z

crypto/evp/pmeth_lib.c

@@ -686,6 +760,52 @@ static int legacy_ctrl_str_to_param(EVP_PKEY_CTX *ctx, const char *name,
        return ret;
    }

+    if (strcmp(name, "rsa_padding_mode") == 0) {


At the very least, I would like to see aliases with rsa_ removed.

These are legacy ctrls - so I'm not sure why we would add new aliases for them?

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)

Any more thoughts on this?

providers/defltprov.c

levitte · 2019-10-14T19:42:29Z

First quick look, looks good. I suspect that some amendment will be needed related to #9979

levitte

We're doing double work... I've RSA stuff in my key generation branch 😒

include/openssl/core_names.h

mattcaswell · 2019-10-16T16:38:55Z

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.

levitte · 2019-10-16T18:04:24Z

We're doing double work... I've RSA stuff in my key generation branch

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.

mattcaswell · 2019-10-28T17:07:39Z

I've rebased this on top of latest master to pick up @levitte's work from #10190. I've also fixed various bugs, and I'm now getting a clean test run (at least locally).

The main outstanding thing is documentation now.

crypto/evp/c_alld.c

crypto/evp/pmeth_fn.c

providers/implementations/asymciphers/rsa.c

crypto/params.c

levitte · 2019-11-04T06:10:18Z

Please rebase this

mattcaswell · 2019-11-06T13:58:10Z

This has now been rebased and all missing documentation has been added. I've taken this out of WIP. Please take a look!

mattcaswell · 2019-11-06T15:50:12Z

Fixup pushed to address travis failure.

crypto/rsa/rsa_lib.c

doc/man3/EVP_ASYM_CIPHER_free.pod

doc/man7/provider-asymcipher.pod

providers/implementations/asymciphers/rsa_enc.c

mattcaswell · 2019-11-12T11:27:02Z

Fixup commits pushed addressing latest feedback.

slontis

LGTM

mattcaswell · 2019-11-12T22:32:32Z

LGTM

Is that intended to be an approval?

@levitte can you reconfirm?

I cant approve as Pauli already has - I approve in theory.

mattcaswell · 2019-11-14T09:35:48Z

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)

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)

mattcaswell added this to In progress in 3.0 New Core + FIPS via automation Oct 11, 2019

mattcaswell force-pushed the asymcipher branch from 02d435e to eb80aa3 Compare October 14, 2019 16:28

levitte reviewed Oct 14, 2019

View reviewed changes

providers/defltprov.c Outdated Show resolved Hide resolved

levitte reviewed Oct 14, 2019

View reviewed changes

providers/defltprov.c Outdated Show resolved Hide resolved

3.0 New Core + FIPS automation moved this from In progress to Needs review Oct 15, 2019

levitte requested changes Oct 15, 2019

View reviewed changes

include/openssl/core_names.h Outdated Show resolved Hide resolved

levitte mentioned this pull request Oct 15, 2019

Export / import RSA to / from provider #10190

Closed

2 tasks

mattcaswell force-pushed the asymcipher branch from 592e875 to 8556358 Compare October 28, 2019 17:06

levitte requested changes Oct 28, 2019

View reviewed changes

levitte mentioned this pull request Oct 30, 2019

EVP: Make the SIGNATURE implementation leaner #10303

Closed

levitte reviewed Oct 31, 2019

View reviewed changes

crypto/params.c Outdated Show resolved Hide resolved

paulidale mentioned this pull request Nov 4, 2019

Move SM2 asymmetric cipher implementation to provider. #10358

Closed

mattcaswell force-pushed the asymcipher branch from f885681 to fe21f57 Compare November 6, 2019 13:55

mattcaswell changed the title ~~WIP: Implement asymmetric cipher support~~ Implement asymmetric cipher support Nov 6, 2019

levitte reviewed Nov 6, 2019

View reviewed changes

crypto/rsa/rsa_lib.c Show resolved Hide resolved

slontis reviewed Nov 12, 2019

View reviewed changes

doc/man3/EVP_ASYM_CIPHER_free.pod Outdated Show resolved Hide resolved

slontis reviewed Nov 12, 2019

View reviewed changes

doc/man7/provider-asymcipher.pod Outdated Show resolved Hide resolved

slontis reviewed Nov 12, 2019

View reviewed changes

providers/implementations/asymciphers/rsa_enc.c Outdated Show resolved Hide resolved

slontis reviewed Nov 12, 2019

View reviewed changes

providers/implementations/asymciphers/rsa_enc.c Show resolved Hide resolved

mattcaswell added 3 commits November 12, 2019 11:19

fixup! Move RSA Asym cipher code to the default provider

1696403

fixup! Document the public EVP_ASYM_CIPHER releated functions

248e20b

fixup! Add documentation for the Asymmetric Cipher Operation

18bc776

levitte added approval: review pending This pull request needs review by a committer and removed approval: done This pull request has the required number of approvals labels Nov 12, 2019

slontis reviewed Nov 12, 2019

View reviewed changes

slontis requested a review from levitte November 13, 2019 03:37

levitte approved these changes Nov 13, 2019

View reviewed changes

levitte added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Nov 13, 2019

mattcaswell added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Nov 14, 2019

mattcaswell closed this Nov 14, 2019

3.0 New Core + FIPS automation moved this from Reviewer approved to Done Nov 14, 2019

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Allow setting a NULL pointer in a params structure

b3f3ba7

Sometimes it is valid to send a NULL pointer in params. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Allow strings in params to be of zero length

46ef075

Sometimes it is useful to be able to pass NULL/zero length strings Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Implement provider support for Asym Ciphers

2c938e2

Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Move RSA Asym cipher code to the default provider

89abd1b

Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Add documentation for the Asymmetric Cipher Operation

11d876d

Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019

Document the public EVP_ASYM_CIPHER releated functions

c68f505

Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #10152)

fekir mentioned this pull request Nov 23, 2019

Replace rsa macro functions with real functions #9655

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement asymmetric cipher support #10152

Implement asymmetric cipher support #10152

mattcaswell commented Oct 11, 2019

beldmit commented Oct 11, 2019

mattcaswell commented Oct 11, 2019

levitte commented Oct 11, 2019

levitte commented Oct 11, 2019

t8m commented Oct 14, 2019

levitte commented Oct 14, 2019

mattcaswell commented Oct 14, 2019

levitte Oct 14, 2019

mattcaswell Oct 28, 2019

levitte Oct 29, 2019

levitte Nov 6, 2019

levitte commented Oct 14, 2019

levitte left a comment

mattcaswell commented Oct 16, 2019

levitte commented Oct 16, 2019

mattcaswell commented Oct 28, 2019

levitte commented Nov 4, 2019

mattcaswell commented Nov 6, 2019

mattcaswell commented Nov 6, 2019

mattcaswell commented Nov 12, 2019

slontis left a comment

mattcaswell commented Nov 12, 2019 •

edited by slontis

mattcaswell commented Nov 14, 2019

Implement asymmetric cipher support #10152

Implement asymmetric cipher support #10152

Conversation

mattcaswell commented Oct 11, 2019

beldmit commented Oct 11, 2019

mattcaswell commented Oct 11, 2019

levitte commented Oct 11, 2019

levitte commented Oct 11, 2019

t8m commented Oct 14, 2019

levitte commented Oct 14, 2019

mattcaswell commented Oct 14, 2019

levitte Oct 14, 2019

Choose a reason for hiding this comment

mattcaswell Oct 28, 2019

Choose a reason for hiding this comment

levitte Oct 29, 2019

Choose a reason for hiding this comment

levitte Nov 6, 2019

Choose a reason for hiding this comment

levitte commented Oct 14, 2019

levitte left a comment

Choose a reason for hiding this comment

mattcaswell commented Oct 16, 2019

levitte commented Oct 16, 2019

mattcaswell commented Oct 28, 2019

levitte commented Nov 4, 2019

mattcaswell commented Nov 6, 2019

mattcaswell commented Nov 6, 2019

mattcaswell commented Nov 12, 2019

slontis left a comment

Choose a reason for hiding this comment

mattcaswell commented Nov 12, 2019 • edited by slontis

mattcaswell commented Nov 14, 2019

mattcaswell commented Nov 12, 2019 •

edited by slontis