Ping

Sidenote:

maybe this is intentional, but looking at the debugger after implementing

    { OSSL_FUNC_KEYEXCH_GETTABLE_PARAMS, (void (*)(void))ecdh_gettable_params },
    { OSSL_FUNC_KEYEXCH_GET_PARAMS, (void (*)(void))ecdh_get_params },

I notice that ecdh_get_params is called directly (correctly asking about OSSL_ALG_PARAM_KEYTYPE) without ever calling ecdh_gettable_params to check what params are gettable.

Is this behaviour by design or are we missing a check?

Is this behaviour by design or are we missing a check?

By design. If there is no name, the variable in libcrypto will remain NULL.

This lets EVP know that providers ProvA and ProvB can handle such objects (and in the future also generate such objects), but still somehow EVP needs to know what kind of operations each provider allows on such key objects (or if it cannot enumerate all the supported operations, because maybe it's superfluous, it needs to know what to do if the user asks to generate a signature or a keyexchange using such a key object).

Oooohhhhh... hmm, yeah I see what you mean. Dang, I actually think you approved this too early.

You can decide to put it back to WIP if you prefer and we will resume the reviews once it satisfies you again!

Some form of test might be nice... I guess romen will test it for you :)

I'm totally trusting that this will get exercised by new code, yeah.

So, the grander issue here, and this is what @romen identified, is that the "algorithm name" for keymgmt has served two purposes:

1. Identifying the name of the algorithm that can make use of a particular key.
2. Identifying the type of the key.

Operations, such as signature, key exchange, need the first identification, to make sure they have the right key to do their job. This becomes crucial with EVP_PKEY_CTX based functions, since they often (not always, but often) work from a given key to find the appropriate algorithm to do signatures, key exchange, whatnot.

Key exporters (the export_to functions that we add to the diverse _ameth.c files), serializers (and upcoming, deserializers), and other things I cannot remember for the moment, are much more interested in the second identification, i.e. the key type.

We sniffed at this problem when discussing RSA vs RSA-PSS, but came out lucky, because RSA keys with the associated PSS parameters attached are identified with a separate OID at a PKCS#8 level, so we're getting away with it because the algorithm and key type happen to match, so we didn't need to bother, and as @mattcaswell put it, the "algorithm name" for keymgmt expressed what a specific key could be used for rather than what it is. So far so good.

Enter EC... and suddenly we have a distinguishable key type that doesn't have the same name as the algorithms that can use it. In other words, the same problem re-surfaces again, but much more tangible and without the possibility to "get away with it" (**).

In summary, it looks like we need to separate what a key is, and what it's used for, while being able to tell how the two sides of that coin are related.

(*) This is a hard problem which was partly resolved by plain C types, and partly by making certain assumptions. Essentially, the key type was always a C type, and since the support of all operations was in one and the same method structure, we could quite liberally use whatever underlying key type we wanted without batting an eye (not entirely... fitting the EC stuff into it was not an easy task and was paved with some pretty heavy refactoring at one point).

(**) This is not quite true, but the solution is full of assumptions. In the use case of EC, for example, the assumption in the legacy EVP_PKEY_METHOD for EVP_PKEY_EC (which is the NID for "id-ecPublicKey") is that the signing function would do ECDSA. This is quite visible when you have a look at crypto/ec/ecdsa_sign.c. This will fall apart as soon as another generic signature algorithm that uses EC keys comes along.

A problem is to figure out a way that doesn't put too much burden on the provider author.

The EVP_PKEY_CTX is created from a key, or in some cases, from a key type (EVP_PKEY_CTX_new_id is expressly said to be useful for domain parameter and key generation, which suggests that the NID used is meant to identity the key type rather than what it can be used for). With that in mind, it seems reasonable to think that what we really need to do is the inverse of what this PR does, i.e. we need to find an appropriate operation algorithm for the key type associated with the EVP_PKEY_CTX rather than finding the appropriate key type, coming from operation that was derived... from a key type.

I can see two ways of doing this:

1. An exhaustive search for algorithms that respond with the correct name when asked with OSSL_ALG_PARAM_KEYTYPE... but only as a fallback in case there is no algorithm with the same name as the key type.
2. Add a query function to keymgmt, just like the provider-wide operation query, but specific to keys. Again, only needed in case the key type name doesn't match the operation algorithm.

The first alternative is fairly easy to do, but has a computational impact that I'm not sure I want to deal with. The second alternative would be easy enough as long as there's only one algorithm for each type of operation.

Unfortunately, time is running out on me today, so I won't get to this before possibly tomorrow.

Ideas and thoughts welcome.

This will fall apart as soon as another generic signature algorithm that uses EC keys comes along.

You mean like SM2!

I am thinking about the second option: if I am understanding correctly it would mean that the keymgmt struct will have to signal by answering to a GET_KEYEXCHANGE, GET_SIGNATURE, GET_ASYM_ENC if it supports any of the above operations and what is the name of the associated algorithm (or maybe directly the pointer to the functions structure, as that is internal to the provider anyway and maybe we can save the second lookup).

This would limit to one keyexchange/signature/asym_enc for key type for provider.

I think this might work, as long as we manage to support that we will have loaded at the same time more than one providers that can deserialize a given key.

Example: SM2 and EC

The keys are serialized using the same global OID "id-ecPublicKey", and we can distinguish them only when parsing inside the ASN1 structure, as luckily the default curve for SM2 has its own unique OID (even if technically if I remember correctly is exactly the same curve as NIST P-256).

So right now, the EC deserializer flags the key as an SM2 key if it detects that it is serialized as a named elliptic curve and curve matches the default SM2 curve.

First problem here is that in theory one could run SM2 as a signature scheme on any other curve, but we can hope that the Chinese standardization body will keep the trend of publishing a dedicated OID for aliased curves, so we can pretend this is not a problem

The real problem is that if we have the default provider supporting deserializing EC keys and a separate Chinese (or other national) crypto provider also capable of deserializing EC keys we shouldn't pick only one but have both try and hopefully only one will be successful (e.g. the default provider could not recognize the SM2 default curve OID while parsing and fail, while the Chinese crypto provider should fail if it does not find the OID for the default SM2 curve or future SM2 specific curve aliases).

If more than one providers manage to deserialize the key, then the conflict could be solved by establishing priorities in the conf file maybe?

That would also allow to avoid running always all the potential deserializers (e.g. we try the Chinese provider only if default failed or viceversa).

If we manage to have this flexibility then I don't see the limit of only one operation per operation type per key type per provider as a limit for provider authors, as basically the problem can then be solved by instantiating 2 different providers rather than a monolithic one.

Other example meaningful for external provider authors.

I could be writing a provider that supports an hardware accelerator that only supports NIST P-256 keys and only ECDH/ECDSA on it.

The provider should be able to deserialize EC keys (hopefully core can provide help here to avoid duplicating all these EC ASN1 structure parsers!) and retrieve what is the OID of the named curve.
It should return a valid key object only for P-256 keys, and fail otherwise.

This failure shouldn't prevent applications from using e.g. P-521 (or any other curve, or SM2) using the software implementation from the default provider or another provider.

Right now in 1.1.1 this is already quite painful for ENGINEs that need to become providers of the whole EC_KEY_method, save a reference to the default method before registering, intercept any EC operation and fallback on the previous default method in case they don't recognize a supported curve NID.
I really hope we can do better with providers!

A thing to remember here is that keymgmt and whatever operation implementation that's being used must be able to cooperate internally. Libcrypto must ensure this by making sure they both come from the same provider. So you're not escaping the monolithic nature...

A thing to remember here is that keymgmt and whatever operation implementation that's being used must be able to cooperate internally. Libcrypto must ensure this by making sure they both come from the same provider. So you're not escaping the monolithic nature...

Well, authors could use #ifdefs to avoid duplicating source code while still exploiting their build system to build 2 different provider binaries, one that does ECDSA over supported EC keys and the other doing e.g. SM2 over supported EC keys.

Keymgmt and signature functions structures will be self contained in each provider to ensure consistency, you will have 2 separate providers, but the code will still be unified.

Not at all dissimilar from what we do with the default and the FIPS providers.

Edit: I want to avoid saying EdDSA as that is associated traditionally with curve25519 or curve448 which avoid all this mess as they are not serialized as EC curves but they have their own top level OID.

Yup, that works

You mean like SM2!

I was thinking í future proofing terms, but yeah, that's a fitting example

I am thinking about the second option: if I am understanding correctly it would mean that the keymgmt struct will have to signal by answering to a GET_KEYEXCHANGE, GET_SIGNATURE, GET_ASYM_ENC if it supports any of the above operations and what is the name of the associated algorithm (or maybe directly the pointer to the functions structure, as that is internal to the provider anyway and maybe we can save the second lookup).

This would limit to one keyexchange/signature/asym_enc for key type for provider.

I think this might work, as long as we manage to support that we will have loaded at the same time more than one providers that can deserialize a given key.

The more I think about it, the more I get to the same conclusion, that we simply cannot support more than one signature scheme, key exchange scheme, yada yada yada, per key type... at least on a libcrypto<->provider interface level (nothing stops a provider from having variants of those schemes based on parameters that come along with the keys, if there are such variations (RSA-OAEP comes to mind)).

For simplicity, I think we need to deal with operation algorithm names and let libcrypto fetch them as it sees fit. Anything else will add complexity to the fetching mechanism, and I think is complex enough as it is. So, a keymgmt query function, much like the provider-wide one, that takes an operation identity, but simply returns a constant string?

We could have it in the back of our minds to allow for multiple schemes of each operation type. I think the answer to that stares us in the face, as we have already solved that problem for algorithm names in OSSL_ALGORITHM: multiple names separate by colons. We don't currently have any support for this in libcrypto as far as I know, so this would be a future extension, not something we should rush into now.

See #10647 for an implementation of the other way around. I believe that's the way to do it and still keep our sanity.

(if accepted, I will close this PR in favor of that one)

Closing thi in favor of #10647