Redesign the KEYMGMT libcrypto <-> provider interface#11006
Redesign the KEYMGMT libcrypto <-> provider interface#11006levitte wants to merge 30 commits intoopenssl:masterfrom
Conversation
levitte
added
branch: master
|
I'm a bit amazed that "domainparams" didn't catch fire earlier, but hey, it's never too late... So, I kinda like "properties" or "attributes"... PKCS#8 uses the latter, so that could be quite fitting. Anyone else with better ideas, or just helping me choose? |
Dont care too much , we can change that name if needed. I still want domainparams to exist. And flags would also be better just in case this expands to do other things in the future. |
If we end up having esssentially the same thing, but call it "domainparams" in some cases and "attributes" in another, we haven't done things right. I want one common name for these things, and then leave it to each implementation to treat that as they wish. It's still perfectly acceptable to document that in the case of DH and DSA, "whatever-name-we-choose" holds the domain parameters. |
|
I think I'll let PKCS#8 lead the way, and rename DOMPARAMS to ATTRIBUTES |
|
From a conceptual point of view.. There are These 'other' parameters sound like a hack. So I guess you are trying to lump domain parameters into the key_properties? |
|
If RSA returns true to hasdomparameters then I will not be pleased. |
|
We could also end up with a key that 'can' have domain parameters, that returns true because it has some other key properties but no domain parameters. |
|
Let me backtrack a little. My talk of PKCS#8 was la-la-land-talk. What I was actually leaning on was this: Now, looking up the latest PKCS#1 RFC:
Looking at RFC 3279 - Algorithms and Identifiers for the Internet X.509 Public Key Infrastructure Certificate and Certificate Revocation List (CRL) Profile, I find:
See what I'm getting at? I believe the "parameters" that are mentioned all over the place comes directly from the So I guess this ends up being about removing "domain" everywhere I've mistakenly written "domain parameters". Fine, no problem at all, and that approaches already existing code anyway, so... |
|
Those two groups of things are completely different concepts. |
|
fair enough - but you cant get rid of it 'everywhere'.. |
If you had them as flags then it shouldnt be a big issue to do both at once without having to collapse them into one... i.e you could request _ATTRIBUTES | _DOMPARAMS |
|
Having spoken in a vid call and thought about it a little bit more and sought some last minute feedback from @mattcaswell, I figured that the selector bit approach does make sense after all. It would allow us to codify the needs of the EVP layer in an extensible manner, and the broader view is simply a combination of bits. This could also allow us to reduce the set of A hard decision has to be made whether a combination of selector bits should mean that all of those subsets of keydata is considered, or just any (i.e. do we look on them with AND in mind or with OR in mind). Intuitively, it seems that all inclusive approach makes better sense. If everyone agrees with that approach (I already know that @mattcaswell does), we're good. |
iamamoose
added
the
approval: ready to merge
Some of the evp_keymgmt_ functions are just wrappers around the EVP_KEYMGMT function pointers. We move those from keymgmt_lib.c to keymgmt_meth.c. Other evp_keymgmt_ functions are utility functions to help the rest of the EVP functions. Since their names are easily confused with the functions that were moved to keymgmt_meth.c, we rename them so they all start with evp_keymgmt_util_. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #11006)
The KEYMGMT libcrypto <-> provider interface currently makes a few
assumptions:
1. provider side domain parameters and key data isn't mutable. In
other words, as soon as a key has been created in any (loaded,
imported data, ...), it's set in stone.
2. provider side domain parameters can be strictly separated from the
key data.
This does work for the most part, but there are places where that's a
bit too rigid for the functionality that the EVP_PKEY API delivers.
Key data needs to be mutable to allow the flexibility that functions
like EVP_PKEY_copy_parameters promise, as well as to provide the
combinations of data that an EVP_PKEY is generally assumed to be able
to hold:
- domain parameters only
- public key only
- public key + private key
- domain parameters + public key
- domain parameters + public key + private key
To remedy all this, we:
1. let go of the distinction between domain parameters and key
material proper in the libcrypto <-> provider interface.
As a consequence, functions that still need it gain a selection
argument, which is a set of bits that indicate what parts of the
key object are to be considered in a specific call. This allows
a reduction of very similar functions into one.
2. Rework the libcrypto <-> provider interface so provider side key
objects are created and destructed with a separate function, and
get their data filled and extracted in through import and export.
(future work will see other key object constructors and other
functions to fill them with data)
Fixes #10979
squash! Redesign the KEYMGMT libcrypto <-> provider interface - the basics
Remedy 1 needs a rewrite:
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from #11006)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #11006)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #11006)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #11006)
The same go for the pairs import + import_types and export + export_types. This required some additional changes in our KEYMGMT implementations. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #11006)
|
Merged!!!!! 68552cd Reorganize the internal evp_keymgmt functions |
|
I'm very happy this went so well, this was a big stone lifting for me. Thank you all for the feedback! |
|
good work richard.. |
|
Agreed. This is a significant clean up. |
Thanks, but I actually view it as good work on all of us. |
|
If I believe that in general inside the provider implementation the use of the aggregated selectors should be close to zero (the only one that seems useful at the moment is the KEYPAIR selector for |
|
Currently the private key implies the public key must also exist currently. I dont think this is right either, but there are other places that are assuming this also at the moment. This may bite us in the future (say with p11) and then be fairly hard to fix properly. |
|
Then again, can we import a private key without importing the public key AND the domain parameters? I feel that for import/export the selectors are creating more confusion than they should, and the current documentation does not clarify enough what the provider implementer should do about the selection bits for import and export. |
|
If there are bits that seem vague, it can certainly be changed for better clarity. Generally speaking, I agree that there should be precision. PR welcome (if I don't get to it first)! That being said, the interpretation of selection is very much up to the provider. The EVP code can only express a desire, then it's up to the provider code to respond to that desire in what way it deems appropriate. Basically, the EVP code has no control of what stuff the provider side key actually contains. However, a provider must also be smart, in that it needs to ensure that the provider side keys are actually usable for operations it's supposed to be usable with (signature, key exchange, whatever...) |
| =head2 Key Object Import and Export Functions | ||
|
|
||
| OP_keymgmt_import() should import data indicated by I<selection> into | ||
| I<keydata> with values taken from the B<OSSL_PARAM> array I<params>. | ||
|
|
||
| OP_keymgmt_export() should extract values indicated by I<selection> | ||
| from I<keydata>, create an B<OSSL_PARAM> array with them and call | ||
| I<param_cb> with that array as well as the given I<cbarg>. | ||
|
|
||
| OP_keymgmt_import_types() should return a constant array of descriptor | ||
| B<OSSL_PARAM> for data indicated by I<selection>, for parameters that | ||
| OP_keymgmt_import() can handle. | ||
|
|
||
| OP_keymgmt_export_types() should return a constant array of descriptor | ||
| B<OSSL_PARAM> for data indicated by I<selection>, that the | ||
| OP_keymgmt_export() callback can expect to receive. | ||
|
|
There was a problem hiding this comment.
So looking at current dh_kmgmt.c, the implementation does not match the documentation.
Theoretically libcrypto can ask the provider to import only the private key in keydata, but that is not possible and no check is done to prevent it.
If the rule of thumb is that the OSSL_PARAM array resulting from OP_keymgmt_export() with selection = FOO must be at least capable of importing a valid keydata object when the same OSSL_PARAM array is passed to OP_keymgmt_import() with the same value for selection, this should be stated clearly.
We should also state what should the provider do if libcrypto sets selection to some combination that makes it impossible to do what stated above.
- should
OP_keymgmt_export()ignore the restrictions set inselectionand export whatever it wants? - should
OP_keymgmt_export()alternatively return an error as soon as an unsupported selection is requested? - should
OP_keymgmt_import()return an error as soon as an unsupported selection is requested?
The previous separation between OP_keymgmt_importkey()+OP_keymgmt_exportkey() and OP_keymgmt_importdomparams()+OP_keymgmt_exportdomparams() was IMHO less ambiguous and easier for provider implementers.
At this point I am not sure anymore when import and export can be triggered, but I see the potential risk for a key being generated in the provider, exported to libcrypto only partially because of selection, some dirty_cnt change is triggered in libcrypto which then triggers an import in the provider again resulting in loss of information or error.
There was a problem hiding this comment.
Uhmmm... have you read what's written in provider-keymgmt(7) / Key Object Import and Export Functions. Doesn't "indicated by selection" tell you that selection is not supposed to be ignored?
Of course, the provider code might ignore selection, but that seems rather dumb, doesn't it? But most of all, how are we going to stop that?
|
Also, in contrast with DH and DSA, for EC we have separate objects to represent a domain parameters object ( The previous separation between importkey and importdomparams (and the equivalent exports) supported cleanly this model, but unifying them makes things quite ugly on the provider side!
|
|
So the weird part is that, for EC keys, the assumptions done at the EVP_PKEY level were bad as well? After all, exactly the same thing applies there, that the underlying backend key that an EVP_PKEY points at may contain all sorts of data combinations. These are the combinations I have detected so far:
So yeah, at the EVP level, *we already assume that the underlying key data is in all sorts of shapes.
|
(DH, DSA, EC support this, but in EC parameters only is represented as an
(only RSA, X/Ed25519 and X/Ed448 can have public key only, as there are no "domain parameters" or the "domain parameters" are embedded in the key type; but technically for the very same reason, public key only and public key + domain parameters for these key types is the same thing, so I don't really think public key only really exists)
(as above, arguably public key does not exist without domain parameters, although domain parameters could be an empty set, so these 2 items are conceptually one and the same) I don't know how to factor the "other parameters" in this discussion, as they are still a bit alien to me.
|
I think you should study this code: Lines 1922 to 1925 in 87d3bb8 |
Please don't ask me to explain the difference between domain parameters and "other" parameters, I find this part utterly confusing, and considering what has been said in diverse discussion, no one agrees on an absolutely clear definition. All I know is that at least one person tells me that the FFC parameters associated with DH and DSA keys are domain parameters, and that the "fancy" RSA parameters - PSS and OAEP - are not domain parameters. |
|
As for EC specific stuff, maybe we should discuss that where it belongs, i.e. in #10631. I'll get there, just not now. |
10 participants
The KEYMGMT libcrypto <-> provider interface currently makes a few
assumptions:
other words, as soon as a key has been created in any (loaded,
imported data, ...), it's set in stone.
key data.
This does work for the most part, but there are places where that's a
bit too rigid for the functionality that the EVP_PKEY API delivers.
Key data needs to be mutable to allow the flexibility that functions
like EVP_PKEY_copy_parameters promise, as well as to provide the
combinations of data that an EVP_PKEY is generally assumed to be able
to hold:
To remedy all this, we:
material proper in the libcrypto <-> provider interface (but only
to a fashion, we still leave the possibility to choose a selection
when importing and exporting).
objects are created and destructed with a separate function, and
get their data filled and extracted in through import and export.
(future work will see other key object constructors and other
functions to fill them with data)
Fixes #10979