Add EVP_PKEY_get_params support for accessing key fields by slontis · Pull Request #11365 · openssl/openssl

slontis · 2020-03-19T10:57:26Z

Currently only RSA, EC and ECX are supported (DH and DSA need to be added to the keygen
PR's seperately because the fields supported have changed significantly).

Also added EVP_PKEY_gettable_params. Neither of these functions are dependant on any
particular operation - but they do require the keys to be provider based.

Made the export and get_params functions share the same code by supplying
support functions that work for both a OSSL_PARAM_BLD as well as a OSSL_PARAM[].
This approach means that complex code is not required to build an
empty OSSL_PARAM[] with the correct sized fields before then doing a second
pass to populate the array.

Checklist

documentation is added or updated
tests are added or updated

slontis · 2020-03-19T10:59:34Z

This is a replacement of #11352. It started out as just adding EC support, but quite a few changes also applied to the RSA code so it is easier just to merge them into a single cleaned up PR.

slontis · 2020-03-19T20:26:56Z

@levitte I have reverted the multiprime changes in this PR.

doc/man3/EVP_PKEY_gettable_params.pod

levitte · 2020-03-23T05:48:02Z

If you're introducing numbered OSSL_PARAM key names, then you should probably revert 45211c5
(MPRSA keys were uniquely the driving force)

slontis · 2020-03-23T06:48:12Z

If you're introducing numbered OSSL_PARAM key names, then you should probably revert 45211c5
(MPRSA keys were uniquely the driving force)

Although this doesnt actually exclude it from being used that way.
I think the KDF's still do this also?

For the RSA case it didnt sit well to do it as an unordered array - considering they are kind of ordered (since there is a triplet mapping going on for each index).

levitte · 2020-03-23T07:48:03Z

For the RSA case it didnt sit well to do it as an unordered array

I'm lost, what was the unordered part?

slontis · 2020-03-23T08:14:58Z

For the RSA case it didnt sit well to do it as an unordered array

I'm lost, what was the unordered part?

You are correct - There was no unordered part for them :)
Giving them unique names makes it a very simple interface for the end user (i.e we dont need a stack or index reference) as well as not requiring extra args being passed down to the keymanagement get_params method() (OR yet another method to do this). The trade off being that a table of const names is required (since the names are just pointed to during param building).

slontis · 2020-03-23T21:52:07Z

ping

paulidale · 2020-03-24T03:53:47Z

I think the KDF's still do this also?

Several of the PRFs replicate the same named item.

providers/implementations/keymgmt/kmgmt_util.c

crypto/evp/p_lib.c

crypto/rsa/rsa_ameth.c

doc/man3/EVP_PKEY_gettable_params.pod

include/internal/kmgmt_util.h

providers/implementations/keymgmt/ec_kmgmt.c

crypto/evp/p_lib.c

openssl-machine · 2020-03-27T22:00:19Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

slontis · 2020-03-27T22:09:07Z

Rebased.
Why is travis not showing up here???

slontis · 2020-03-27T22:10:03Z

@levitte - has to rebase (also addressed your nits). Does your approval still hold?

levitte · 2020-03-28T00:09:05Z

Did you need to adjust more than util/libcrypto.num? If not, it still stands

slontis · 2020-03-28T04:31:45Z

Sigh - Got a merge error in ec_kmgmt.c - so it does need another review now

slontis · 2020-03-29T23:27:27Z

Rebased yet again.
Modified EVP_PKEY_get_bn_param so that it optionally used an allocated buffer and added a test for this case.

slontis · 2020-03-29T23:48:59Z

collapsed the commits so that if someone causes me yet another merge it wont be as bad.

Changed EVP_PKEY_get_bn_param() so it cleanses the buffers it uses (Since it could be retrieving private data).

…fields Currently only RSA, EC and ECX are supported (DH and DSA need to be added to the keygen PR's seperately because the fields supported have changed significantly). The API's require the keys to be provider based. Made the keymanagement export and get_params functions share the same code by supplying support functions that work for both a OSSL_PARAM_BLD as well as a OSSL_PARAM[]. This approach means that complex code is not required to build an empty OSSL_PARAM[] with the correct sized fields before then doing a second pass to populate the array. The RSA factor arrays have been changed to use unique key names to simplify the interface needed by the user.

…y data fields

slontis · 2020-03-31T01:23:02Z

rebased to fix libcrypto.num - no reapproval is required.

openssl-machine · 2020-03-31T02:00:16Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

…fields Currently only RSA, EC and ECX are supported (DH and DSA need to be added to the keygen PR's seperately because the fields supported have changed significantly). The API's require the keys to be provider based. Made the keymanagement export and get_params functions share the same code by supplying support functions that work for both a OSSL_PARAM_BLD as well as a OSSL_PARAM[]. This approach means that complex code is not required to build an empty OSSL_PARAM[] with the correct sized fields before then doing a second pass to populate the array. The RSA factor arrays have been changed to use unique key names to simplify the interface needed by the user. Reviewed-by: Richard Levitte <[email protected]> (Merged from #11365)

slontis · 2020-04-01T05:58:48Z

Thanks.. Merged.

slontis added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Mar 19, 2020

slontis mentioned this pull request Mar 19, 2020

Add PKEY_get_params support for accessing RSA key fields #11352

Closed

2 tasks

slontis force-pushed the pkey_get_params_ec branch 4 times, most recently from 7e788e9 to 295e398 Compare March 19, 2020 20:24

slontis force-pushed the pkey_get_params_ec branch 2 times, most recently from 4096508 to 1e96783 Compare March 19, 2020 23:50

slontis mentioned this pull request Mar 19, 2020

Add EVP_PKEY API to retrieve raw OSSL_PARAM values from a provided key #11340

Closed

2 tasks

slontis force-pushed the pkey_get_params_ec branch from 1e96783 to a996d8e Compare March 23, 2020 04:58

slontis requested a review from levitte March 23, 2020 05:15

levitte reviewed Mar 23, 2020

View reviewed changes

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

mattcaswell requested changes Mar 24, 2020

View reviewed changes

providers/implementations/keymgmt/kmgmt_util.c Outdated Show resolved Hide resolved