OSSL_DESERIALIZER: New API for provider based deserializers#12410
OSSL_DESERIALIZER: New API for provider based deserializers#12410levitte wants to merge 34 commits intoopenssl:masterfrom
Conversation
|
This becomes a fundament for the OSSL_STORE work, which has stalled gravely in #9389 |
|
Will review on Monday.. |
Cool, that'll give me time to work out the kinks I missed |
|
I've had some thoughts on the public API, which is currently a bit too incomplete, so this is temporarly WIP. Comments are still welcome |
ccfb7e6 to
4048c4d
Compare
|
Ok, now I got something that I'm satisfied with, and that makes this fairly easy to extend. Unfortunately, it turns out that the SYNOPSIS parser in |
@levitte - maybe you could delegate that to someone- so that you can do some more important tasks? |
crypto/property/property_parse.c
Outdated
There was a problem hiding this comment.
@paulidale Is there a place where these should be documented?
crypto/serializer/serializer_local.h
Outdated
There was a problem hiding this comment.
Well these are a mouthful - wouldnt it be better to keep it consistent with other methods?
i.e.
OSSL_DESERIALIZER_FINAL *final;
OSSL_DESERIALIZER_CLEANUP *cleanup;
There was a problem hiding this comment.
It's not related the way you seem to assume. But sure.
There was a problem hiding this comment.
Actually, I'm backing off from that change for now. If you read the manual on the subject, the text is quite fluent when written in terms of a finalizer... I'm not quite sure how to rewrite that using final with the same kind of flow.
Perhaps we need another term entirely instead of finalize? Like I said, this is nowhere near the meaning of "final" as seen in EVP.
There was a problem hiding this comment.
Constructor? As in object construction.
There was a problem hiding this comment.
... "constructor" is less a mouthful than "finalizer"??? 😉
But, other than that, it is a viable option. But boy, those function names are getting long...
There was a problem hiding this comment.
I meant it as being more descriptive not less mouthful.
crypto/serializer/deserializer_lib.c
Outdated
There was a problem hiding this comment.
If the up_ref call failed.. Should the err: path be doing OSSL_DESERIALIZER_free ??
There was a problem hiding this comment.
Do note that the err path does that on deser_inst->deser, which is assigned further down
crypto/serializer/deserializer_lib.c
Outdated
There was a problem hiding this comment.
You could just use deser instead of deser_inst->deser in quite a few places.
crypto/serializer/deserializer_lib.c
Outdated
There was a problem hiding this comment.
This code could be a good place to start adding error stacks to :)
crypto/serializer/deserializer_lib.c
Outdated
There was a problem hiding this comment.
this looks like it should be
if (deser_inst->deserctx != NULL)
deser->freectx(deser_inst->deserctx);
There was a problem hiding this comment.
Not sure I understand...
There was a problem hiding this comment.
You have already asserted that deser is not null.
The value of deser_inst->deser is irrelevant to the freectx.
The value is newed via
deser_inst->deserctx = deser_inst->deser->newctx(provctx)
So that is what should determine if the freectx is called.
crypto/serializer/deserializer_lib.c
Outdated
There was a problem hiding this comment.
I would just like to say that this function is not easy to understand. I will come back to it when I have browsed further, and have figured out where it is used.
There was a problem hiding this comment.
Yeah... although, those names are already a mouthful, I tried to keep it short. Ideas are certainly welcome.
There was a problem hiding this comment.
Perhaps use "DESER" like you do for the variables? (And "_instance" as a prefix can probably just be removed.)
There was a problem hiding this comment.
So prefix everything with OSSL_DESER instead of OSSL_DESERIALIZE... for consistency, we should probably change OSSL_SERIALIZE to OSSL_SER, then... it doesn't strike me as the most intuitive prefixes, but it is a viable option if we can all agree that the less readable trumps the mouthful.
There was a problem hiding this comment.
(I have some refactoring thoughts for the serializer API as well, so a name change would simply become part of that)
There was a problem hiding this comment.
Frankly, I cannot pronounce serializer deserializer without breaking my tongue 😄
Have you considered exporter/importer?
There was a problem hiding this comment.
@richsalz, I didn't mean to disqualify your suggestion. I find the function names I came up with excruciatingly long to type, but the naming was stuck in my mind, so I do appreciate suggestions to get unstuck.
Serialize / parse sounds like an odd pair, though. But, makes me think of pack / unpack as an option (although I would probably confuse the two, so maybe not the best choice).
@t8m, export / import could actually be viable! OSSL_EXPORT and OSSL_IMPORT, I kinda like the ring of that. However, it might be confusing when we thing of the export / import functionality in KEYMGMT... Still, that might be a possibility.
(also, since the KEYMGMT export / import functionality convert internal data to / from OSSL_PARAM, it is essentially a serialization / deserialization... That might help)
There was a problem hiding this comment.
No need to apologize, glad to help unstick. Just make sure it also didn't dislodge any perl knowledge :)
There was a problem hiding this comment.
I'd prefer the shorter name too. I don't think most users would have a problem with OSSL_IMPORT/EXPORT. The KEYMGMT import/export is mostly hidden in the provider boundary.
Kind of similar to the user facing EVP_*_get_params calls versus the get_params calls providers use.
This makes the following functions available for libcrypto code: evp_keymgmt_util_try_import() - callback function evp_keymgmt_util_assign_pkey() - assigns keymgmt and keydata to an EVP_PKEY evp_keymgmt_util_make_pkey() - creates an EVP_PKEY from keymgmt and keydata Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
This function is used to create a keydata for a key that libcrypto only has a reference to. This introduces provider references, the contents which only the provider know how to interpret. Outside of the provider, this is just an array of bytes. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
This adds a method OSSL_DESERIALIZER, a deserializer context and basic support to use a set of serializers to get a desired type of data, as well as deserializer chains. The idea is that the caller can call OSSL_DESERIALIZER_CTX_add_serializer() to set up the set of desired results, and to add possible chains, call OSSL_DESERIALIZER_CTX_add_extra(). All these deserializers are pushed on an internal stack. The actual deserialization is then performed using functions like OSSL_DESERIALIZER_from_bio(). When performing deserialization, the inernal stack is walked backwards, keeping track of the deserialized data and its type along the way, until the data kan be processed into the desired type of data. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
EVP_PKEY is the fundamental type for provider side code, so we implement specific support for it, in form of a special context constructor. This constructor looks up and collects all available KEYMGMT implementations, and then uses those names to collect deserializer implementations, as described in the previous commit. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
This test revolves around a central function that will first serialize
an EVP_PKEY, then deserialize the result into a new EVP_PKEY and
compare the two.
The following tests are currently implemented:
1. EVP_PKEY (RSA) -> DER, then DER -> EVP_PKEY (RSA).
2. EVP_PKEY (RSA) -> PEM, then PEM -> EVP_PKEY (RSA).
This one exercises deserializer chains, as we know that there is a
PEM -> DER and a DER -> EVP_PKEY (RSA) deserializer, but no direct
PEM -> EVP_PKEY (RSA) deserializer.
Additionally, a small fix in test_fail_string_common(), as strcmp()
could run past a buffer if one of the strings isn't terminated with
a null byte within the given length.
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
(Merged from openssl#12410)
Serialization will only encrypt, so there's no point telling OSSL_SERIALIZER_CTX_set_passphrase_cb() that's going to happen. We fix the declaration of OSSL_DESERIALIZER_CTX_set_passphrase_cb() the same way. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
This implements these functions: OSSL_DESERIALIZER_CTX_set_cipher() OSSL_DESERIALIZER_CTX_set_passphrase() OSSL_DESERIALIZER_CTX_set_passphrase_ui() OSSL_DESERIALIZER_CTX_set_passphrase_cb() To be able to deal with multiple deserializers trying to work on the same byte array and wanting to decrypt it while doing so, the deserializer caches the passphrase. This cache is cleared at the end of OSSL_DESERIALIZER_from_bio(). Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
This adds variants of already existing tests, but where the object is encrypted / decrypted along the way as well. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
…ncryption This adds legacy PEM variants of already existing tests. Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Shane Lontis <[email protected]> (Merged from openssl#12410)
Deserializing may be a complex thing. Input needs to be recognised and then dealt with, possibly in multiple steps (PEM -> DER, DER -> RSA).
The deserializers should be possible to pick very precisely, and should also be searchable and combinable in chains.
To allow precise picking, we base the "algorithm name" on the product that is desired ("RSA" for an RSA key), and a gettable algorithm parameter "input" specifying the expected input by name (for example, "DER" if DER is expected). This input name can then be used to find deserializers that produces the desired input (in this example, serializers named "DER"). That's how they get chained together.
On the provider side, the deserializing function feeds back the deserialized data to the caller through an OSSL_CALLBACK function. The possible parameters are:
In case that's needed, the deserializer also has an export function, which takes an object reference, and exports that object's parameters via a callback, exactly like the KEYMGMT export function. This can be used to transfer an object to a different provider, should the need arise.
What's that object reference?
An object reference is a reference to the actual desired object. The exact contents of the reference itself depends entirely on the provider. It could be a UUID for all we care...
With this, the KEYMGMT set of functions has gained a new one, OSSL_FUNC_keymgmt_load(), which takes a references and returns the corresponding keydata. The combined EVP_KEYMGMT and keydata can then be wrapped in an EVP_PKEY and used.
In OpenSSL providers, that's actually simply an indirect pointer to the object, and allows OSSL_FUNC_keymgmt_load() to be called from inside a deserializer callback call, and to "take over" the object held by the deserializer. The result is that we avoid having to do some tricky memory management to align DESERIALIZER code and KEYMGMT code, all we need is [ahem] an easy pointer trick.