@mattcaswell Updated everything as per the discussion above (and included now some CI using the OpenSSL test framework: Is that OK or would you think we should have tests separate from the OpenSSL test framework?).
Changed the PR to draft as I'm not happy with the code here (which was necessary to remove the "internal" includes):

and included now some CI using the OpenSSL test framework: Is that OK or would you think we should have tests separate from the OpenSSL test framework?)

I think the main objective should be to remove any internal dependencies on internal OpenSSL APIs from the provider itself, so that your provider can be entirely standalone and rely on having stability in the APIs that it uses. Internal APIs are subject to change at any time (including in a patch release) - so you could suddenly find everything breaks (potentially even in deployed instances) if OpenSSL gets upgraded in a new patch release if you have internal dependencies.

Tests are slightly different, since they don't end up in the actual provider code - so if you want to reuse the OpenSSL test framework then fair enough. That said though, I would think that you would ultimately want to build your provider and tests in its own build tree and not be part of the OpenSSL one. If you get to the point where you're not modifying any OpenSSL internals any more then you no longer have a need to have a fork of OpenSSL, and really you only want the code in your repo that is directly relevant to your provider. There's no reason why you couldn't lift all the bits out of OpenSSL that you need for that (including the underlying build and test frameworks).

If you get to the point where you're not modifying any OpenSSL internals any more then you no longer have a need to have a fork of OpenSSL, and really you only want the code in your repo that is directly relevant to your provider.

Fully agree. For this clean separation to happen we must get to the point that both the OQS provider doesn't (need to) rely on OpenSSL internals (Thanks again for your help on this) and OpenSSL in turn must fully support externally provided sigs so we don't need the OQS-OpenSSL fork in the future. So let me really try to get down to the details of that. For now, I'd leave things as-is after implementing the OQS-sig provider to have something to run that is not OpenSSL-internal.

Guidance welcome as to how to combine the current OQS license and correctly reference the OpenSSL License

Licensing is a bit of a minefield. Of course I have to caveat anything I say on this before I start with "I am not a lawyer". Therefore anything I say might be wrong and should be treated with caution. Therefore (caveat 2) "you should seek professional advice on this".

That said here are some thoughts. I guess there are a few things you need to consider:

1. Licensing of pure OQS code that is entirely standalone
2. Licensing of code that you have "inherited" from OpenSSL and want to incorporate into OQS
3. Licensing of OQS code that you might one day want to have merged upstream into OpenSSL

(1) is of course entirely you're own affair and for you to decide.

For (2), if its been inherited from the OpenSSL master branch then Apache 2 applies (previous versions are using different licences). You can apply your own licence to changes that you make to this code but in that case both licences would have to apply (the Apache licence for the old code, your licence for the new code). That is usually considered undesirable, but is perfectly possible and is in fact what OpenSSL itself did. OpenSSL inherited its original code from the SSLeay project, and then applied its own licence on top. For all versions of OpenSSL up until (and including) OpenSSL 1.1.1, both the OpenSSL and the SSLeay licences applied. We went through a very major effort (that took several years) to contact all previous contributors in order to fix that problem so that we were eventually able to change the licence. The other alternative you have is to just use the the Apache licence for the code that you have inherited.

For (3) the issue here isn't so much about licensing but about CLAs. You can have whatever licence you see fit for this code but OpenSSL will relicense it to Apache 2 when it accepts the code. In order for that to work OpenSSL requires all contributors to have signed an OpenSSL CLA. When I say all, I mean everyone who has ever contributed to the code (even if that contribution is small). This can actually be a big problem, and has been a showstopper in the past for some highly desirable contributions that we would really have liked to incorporate, but were unable to because of CLA problems.

With regards to "correctly reference the OpenSSL Licence", I think this text applies from the Apache licence:

      (a) You must give any other recipients of the Work or
          Derivative Works a copy of this License; and

      (b) You must cause any modified files to carry prominent notices
          stating that You changed the files; and

      (c) You must retain, in the Source form of any Derivative Works
          that You distribute, all copyright, patent, trademark, and
          attribution notices from the Source form of the Work,
          excluding those notices that do not pertain to any part of
          the Derivative Works; and

So, basically, you should retain any existing licence/copyright banners. But you can add your own additional ones (e.g. citing your own licence file).

So, basically, you should retain any existing licence/copyright banners.

That is perfect advice: Thanks! I'll do that and we'll also discuss within the OQS team which license text to adopt for an OQS-OpenSSL provider given the new license conditions of OpenSSL you explained above. @dstebila : Please see the above for consideration.

When now activating testing for all KEMs using the OpenSSL test environment, I ran (again, as @christianpaquin did with OpenSSL1.1.1) into a problem with the OpenSSL constant SERVER_HELLO_MAX_LENGTH: While it's really cool we can run OQS KEMs via providers in an otherwise unmodified OpenSSL 3.0, this constant forces me to disable two algorithms to pass all tests (frodo1344aes and frodo1344shake) to retain this goal of "arms-length" integration. Any ideas how to resolve this would be very welcome.

When now activating testing for all KEMs using the OpenSSL test environment, I ran (again, as @christianpaquin did with OpenSSL1.1.1) into a problem with the OpenSSL constant SERVER_HELLO_MAX_LENGTH: While it's really cool we can run OQS KEMs via providers in an otherwise unmodified OpenSSL 3.0, this constant forces me to disable two algorithms to pass all tests (frodo1344aes and frodo1344shake) to retain this goal of "arms-length" integration. Any ideas how to resolve this would be very welcome.

I think an upstream fix for that constant would be appropriate for 3.0. The situation is different compared to 1.1.1. In, 1.1.1 there was no possibility of plugging in new groups etc from external sources. Therefore the only way you could go over the SERVER_HELLO_MAX_LENGTH limit was if you had already modified the OpenSSL source. This is no longer the case with 3.0, so it seems reasonable to change it. Want to propose something?

Want to propose something?

Gladly done: openssl#13868

@mattcaswell Quick question: Is there any way to test (new) signature providers -- short of adding all "plumbing" like asn1_meth, pkey_meth (i.e., effectively implementing openssl#10512) wired to call into a (generic) signature provider? I wanted to put "real code" into the OQS sig provider to finally complete this PR but wonder how to trigger it... If it's not simple, please just say No; I'll then get going on 10512 for real.

Adding a new signature provider and testing it should be straight forward - although a little laborious. You shouldn't have to touch the "meths" at all.

To implement the signature provider you need to:

1. Implement a provider side key manager. As an example you might look at providers/implementations/keymgmt/dsa_kmgmt.c

2. Write the signature implementation itself. For example see providers/implementations/signature/dsa.c

3. Reference the key manager and signature implementations from your main provider query function

An optional extra is to implement encoders/decoders for your algorithm so that you can actually serialise/deserialise your keys to/from files. But you don't need to do that to do some simple tests.

Once created, and with your provider loaded into your libctx it should just "work" with EVP signature code. First you'll have to generate a key (error handling omitted for brevity):

EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_from_name(NULL, "MY-SIGNATURE-ALG", NULL);
EVP_PKEY *key = NULL;

/* Depending on how your alg works you might need to generate a params EVP_PKEY first.
 * Otherwise go straight to generating the key */
EVP_PKEY_keygen_init(ctx);
/* Call EVP_PKEY_CTX_set_params() here if necessary for your alg */
EVP_PKEY_gen(ctx, &key);

Then to actually test the signature itself:

EVP_MD *md = EVP_MD_fetch(NULL, "SHA256", NULL);
EVP_MD_CTX *mdctx = EVP_MD_CTX_new();
unsigned char *sig;

EVP_DigestSignInit(mdctx, NULL, md, NULL, pkey);
EVP_DigestSignUpdate(mdctx, msg, sizeof(msg));

/* Determine the size of the signature. */
EVP_DigestSignFinal(mdctx, NULL, &siglen))

sig = OPENSSL_malloc(siglen);
EVP_DigestSignFinal(mdctx, sig, &siglen);

There are some docs on some of this, e.g. see:

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

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

Thanks for the sample code and pointers! Now all of that is working nicely (sign/verify for OQS algs via EVP). But this still didn't trigger the sig_get_ctx_params functions (that necessitated the NID/AID logic we'd like to "detach" from OpenSSL internals)... I probably could explicitly call EVP_PKEY_CTX_get_signature_md along the same lines as your sample code above, but then... how do I know we deliver the right values back? Which OpenSSL functions would need this (and could tell me whether our return values are OK)?

@mattcaswell Thanks again for your pointers and sample above! It really helped getting me going with the latest push; so let me change the question above to: Did I understand the operations of the (not exceptionally deeply documented) function a2d_ASN1_OBJECT correctly? It allowed me to eliminate the final logic I stole from the upstream code. Thus, I'm now re-requesting a review as CI passes also on signatures.
On a related note, was I right replacing EVP_DigestSignInit with EVP_DigestSignInit_ex as only the latter allowed me to utilize a libctx? In general, I think the documentation of those two functions (relative to their difference in the presence of libctx) could be improved in the corresponding .pod file: EVP_DigestSignInit really only performs nicely if used in conjunction with default provider and default libctx (NULL). If not, it quickly returns 0 if the default provider is not in the default libctx (as in the OQS test case). If you'd agree, let me know if I should try to do issue/PR to add some text -- although I'm pretty sure you would be quicker.

Will try and take a look at this later today and respond to your questions, once I've caught up on some other stuff.

Just to respond to this one question though:

On a related note, was I right replacing EVP_DigestSignInit with EVP_DigestSignInit_ex as only the latter allowed me to utilize a libctx? In general, I think the documentation of those two functions (relative to their difference in the presence of libctx) could be improved in the corresponding .pod file: EVP_DigestSignInit really only performs nicely if used in conjunction with default provider and default libctx (NULL). If not, it quickly returns 0 if the default provider is not in the default libctx (as in the OQS test case). If you'd agree, let me know if I should try to do issue/PR to add some text -- although I'm pretty sure you would be quicker.

Of course the motivation for introducing EVP_DigestSignInit_ex was precisely that it allows you to specify the libctx, whilst EVP_DigestSignInit will only work with the default libctx. However it should not be true that EVP_DigestSignInit only works with the default provider. You should be able to load any provider into the default libctx and it should work just fine. Is that not your experience? If so I'd like to understand what you were expecting to happen, and what actually happened.

You should be able to load any provider into the default libctx and it should work just fine.

Yes - that works. My mistake was to first use my own libctx in combination with EVP_DigestSignInit and not trying to load the oqsprovider into NULL (default libctx). So possibly only "user error" not warranting further doc update.

Will try and take a look at this later today

Thanks in advance!

But this still didn't trigger the sig_get_ctx_params functions (that necessitated the NID/AID logic we'd like to "detach" from OpenSSL internals)

The AID logic is triggered via functions such as: ASN1_item_sign_ctx, ASN1_item_sign_ex, ASN1_item_sign, X509_sign, X509_sign_ctx, etc

Did I understand the operations of the (not exceptionally deeply documented) function a2d_ASN1_OBJECT correctly?

a2d_ASN1_OBJECT is a really low level function, and not normally one used by end applications. Probably it should not be exposed in the OpenSSL API, but....history. If I understand the logic correctly it creates the content bytes of the DER encoding of the specified OID, but it does not include the tag/length bytes - so what you get as a result is not valid DER by itself.

I think probably what you need to do is call OBJ_txt2obj to create an ASN1_OBJECT from the numerical OID. From there you can call i2d_ASN1_OBJECT to create valid DER for the OID.

The AID logic is triggered via functions such as: ASN1_item_sign_ctx, ASN1_item_sign_ex, ASN1_item_sign, X509_sign, X509_sign_ctx, etc

OK -- when looking at how to manually trigger ASN1_item_sign_ex for testing I'm trying to understand its code; now, when looking at that is it possible that libctx is not properly passed through? There is again EVP_DigestSignInit (without specifying libctx):

https://github.com/openssl/openssl/blob/3aff5b4bac7186fda9208a76127eff040cafae13/crypto/asn1/a_sign.c#L126-L140

Wouldn't that trip me up when not using/configuring the NULL libctx? Also see issue openssl#13732

Could it be as simple as replacing https://github.com/openssl/openssl/blob/3aff5b4bac7186fda9208a76127eff040cafae13/crypto/asn1/a_sign.c#L139 with

if (!EVP_DigestSignInit_ex(ctx, NULL, EVP_MD_name(md), libctx, NULL, pkey))
        goto err;

?

I'm trying to understand its code; now, when looking at that is it possible that libctx is not properly passed through? There is again EVP_DigestSignInit (without specifying libctx):

That code does look a little suspect, but I think it ends up being ok because the evp_md_ctx_new_ex call creates an EVP_PKEY_CTX using the libctx, and associates that with the EVP_MD_CTX. It is the libctx associated with the EVP_PKEY_CTX that is actually used in the digest sign operation.

As I face the option of creating low-level (ASN.1) tests now using rather undocumented internal functions I think I'd prefer to call it a day and merge this PR now: The code already massively improved due to your input, @mattcaswell, so many thanks for that.
My preference would be to focus on sifting through OpenSSL at least trying to make sigs fully provider-enabled, even though it really looks daunting. Bugs remaining in the oqs(sig) provider will surely surface that way :-) Objections to this plan?
Or would it be more sensible to create (ASN.1) tests for the OQSprovider -- this way "working my way up" the OpenSSL APIs? Might teach me more, but probably be slower.

My preference would be to focus on sifting through OpenSSL at least trying to make sigs fully provider-enabled, even though it really looks daunting. Bugs remaining in the oqs(sig) provider will surely surface that way :-) Objections to this plan?

To the extent that my opinion matters on what gets merged in this repo - fine by me :-)

Unfortunately I think this bug is going to be a bit of a killer for you:
openssl#13893

Until that bug is fixed you're not going to be able to use X509 certs that contain externally provided algorithm public keys.

Once we can do X509 with external algs I'm thinking that the next problem will be the fixed table of cert types that libssl holds:

https://github.com/openssl/openssl/blob/47b784a41b729d5df9ad47c99355db2f2026a709/ssl/ssl_local.h#L2018

We currently store in that structure the certificate/chain and associated key for each algorithm type that we support. And the algorithms types are a fixed list. I think this should be where you start. We need that to be able to cope with algorithm types that we don't know up front. It's unclear how much you can do on this without the X509 bug being fixed - but perhaps you can start.

Once that works the next problem will be to introduce pluggable sigalgs in the same manner that we have pluggable groups.

Independently of the above I think you are going to need to write encoders/decoders in your provider for the various algs. At least for DER anyway because you need to encode and decode public keys for libssl.

To the extent that my opinion matters on what gets merged in this repo - fine by me :-)

It absolutely does. Thanks.

I think this should be where you start.

Yup - rings a bell. Current change in our 1.1.1 fork:

Given there are lots of "explicit" index-references to the pkeys array, would you agree it would be best to retain it but allocate it dynamically after loading the providers (and querying their algs)? Most notably, I do not see GOST in the providers but in the SSL core logic, so we probably can't easily remove the "current" (or classic) pkeys slots and make it completely provider-driven. Or should that be our goal: Making OpenSSL run possibly even without RSA and GOST? Reminds me of the default groups list discussion (optionally retaining the classic ones at the top of the list). SSL_PKEY_NUM of course would become a variable. So overall I'd say this area can be dealt with.

What currently makes me wonder more is how to deal with the function lists (pmeth and ameth): I see that the functions have their equivalents in the providers (hence also the need to implement encode/decode) but I don't see already how to automate this for any provider (shall we allocate the data structures EVP_PKEY_ASN1_METHOD and EVP_PKEY_METHOD also dynamically, populating them with provider calls?). In our 1.1.1 fork we completely "mirrored" those functions for all OQS key types: see

Most notably, I do not see GOST in the providers but in the SSL core logic, so we probably can't easily remove the "current" (or classic) pkeys slots and make it completely provider-driven.

GOST is only available via ENGINE at the moment, and we still need to be able to support that. So we will need to support "legacy" pkeys.

Given there are lots of "explicit" index-references to the pkeys array, would you agree it would be best to retain it but allocate it dynamically after loading the providers (and querying their algs)

This is possible. I would envisage using "capabilities" to enumerate the sig-algs that a provider supports, and then having some kind of dynamic storage to store the certs/keys etc. Possibly we need to add to the sig-algs that we have enumerated from providers the "well-known" classic algorithms that might be filled in from some other source. I'm less sure if the dynamic storage needs to necessarily be an array the size of all algorithms. It might be an array that is sufficiently big only to store the certs/keys that have been provided by the application. If we did that we would have to refactor everywhere that uses the array to call some function to "look up" the cert/key that we are interested in.

What currently makes me wonder more is how to deal with the function lists (pmeth and ameth): I see that the functions have their equivalents in the providers (hence also the need to implement encode/decode) but I don't see already how to automate this for any provider (shall we allocate the data structures EVP_PKEY_ASN1_METHOD and EVP_PKEY_METHOD also dynamically, populating them with provider calls?).

I'm not sure I understand your concern here. We shouldn't need to deal with the "METHOD"s at all? That should all be "under the covers" of the EVP interface.

Unfortunately, I have to be the bearer of bad news. The OTC discussed the possible addition of pluggable signatures in the 3.0 timeframe. The conclusion is that this is unlikely to be accepted on the basis that we have so much other stuff we need to do by then then adding this significant feature in the mix as well (even if its just review time etc) is likely to cause us to miss deadlines... :-(

The conclusion is that this is unlikely to be accepted

Not entirely unexpected, so no problem. We have a fully working OQS KEM provider for OSSL3 and a fork for 1.1.1 doing certs and pretty much everything else, so I guess we're fine for now.

I'd be glad to reciprocate your support so far (and maybe in the future) by trying to resolve existing issues you have -- in the process picking up knowledge to make progress on pluggable signatures eventually. So if there's a list of items to resolve before v3beta1, please let me know, maybe pointing to ones most suitable for someone not entirely certain about all nuts and bolts in OpenSSL (if such issues were around).

I'd be glad to reciprocate your support so far (and maybe in the future) by trying to resolve existing issues you have

Any help you can offer would be gladly accepted. The list of issues that we are most concerned with is here:
https://github.com/openssl/openssl/issues?q=is%3Aopen+is%3Aissue+milestone%3A%223.0.0+beta1%22

However, I might suggest openssl#13893 would be a good one to tackle. It's a little tricky, but actually is helpful to OQS even without pluggable signatures. Wouldn't it be cool if you could load X509 certs using standard OpenSSL 3.0 using OQS algs (even if you can't actually use those certs in libssl)? If you also had encoders/decoders implemented in your provider then (in theory) you should be able to use the standard openssl "x509" app to create and examine such certs.

However, I might suggest openssl#13893 would be a good one to tackle. It's a little tricky, but actually is helpful to OQS even without pluggable signatures. Wouldn't it be cool if you could load X509 certs using standard OpenSSL 3.0 using OQS algs

Looking into it -- particularly interesting as we can already generate OQS X509 certs (using the 1.1.1 fork), so have something to test against.

So if I understand the goal right, one would need to create an EVP_PKEY given the appID, iff a matching provider decoder can parse the X509 key data (to keep the API of x509_pubkey_decode stable)? At first glance this indeed doesn't look quite straightforward, but let me dig and understand more how all those pieces fit together....

I think it may not be that difficult with a decoder. The "x509_pubkey_decode" function mentioned in the issue above would have to have some code, something like the following for the OID "1.2.3.4":

EVP_PKEY *pkey = EVP_PKEY_new();
OSSL_DECODER_CTX *dctx = OSSL_DECODER_CTX_new_by_EVP_PKEY(&pkey,
                                                          "DER", "SubjectPublicKeyInfo",
                                                          "1.2.3.4", EVP_PKEY_PUBLIC_KEY,
                                                          libctx, propq);
OSSL_DECODER_from_bio(dctx, membio);

This requires the decoder algorithms to have "1.2.3.4" as an alias to their normal name in the provider so that we can find them. This also requires all algorithms to have a decoder - which you don't yet in your OQS implementation.

OSSL_DECODER_CTX_new_by_EVP_PKEY

Thanks for this pointer: Going through this code I now better understand how this decoder logic works (basically iterating through all decoders), hence understanding this statement better:

This also requires all algorithms to have a decoder - which you don't yet in your OQS implementation.

I'll work on that then, too. Original plan was to understand and enable this for traditional crypto first, but after seeing the decoder iterator (and finally reading provider-encoder) things become clearer.

Next then is to see how to add OID aliases in all provider decoders -- or is it as simple as just adding (all proper) ":" to the DECODER_w_structure("RSA... "registrars" in providers/decoders.inc ?

This also requires all algorithms to have a decoder - which you don't yet in your OQS implementation.

Let me clarify that statement a bit. It requires all algorithms that you want to decode through this route to have decoders. So you could work on the traditional crypto first and get that working and the then add the decoders for OQS later.

Next then is to see how to add OID aliases in all provider decoders -- or is it as simple as just adding (all proper) ":" to the DECODER_w_structure("RSA... "registrars" in providers/decoders.inc ?

In theory...yes.

So you could work on the traditional crypto first

Probably a better approach, indeed.

In theory...yes.

It does even in practice (after adding "1.2.840.113549.1.1.1" to the "RSA" decoder registrar), OBJ_obj2txt(algname, 100, key->algor->algorithm, 1); OSSL_DECODER_CTX_new_by_EVP_PKEY(...algname...) then delivers several decoders... However, to follow this

I think it may not be that difficult with a decoder. The "x509_pubkey_decode" function mentioned in the issue above would have to have some code, something like the following for the OID "1.2.3.4":

EVP_PKEY *pkey = EVP_PKEY_new();
OSSL_DECODER_CTX *dctx = OSSL_DECODER_CTX_new_by_EVP_PKEY(&pkey,
                                                          "DER", "SubjectPublicKeyInfo",
                                                          "1.2.3.4", EVP_PKEY_PUBLIC_KEY,
                                                          libctx, propq);
OSSL_DECODER_from_bio(dctx, membio);

I feel I'm missing way too much insight on how the X509 decoding logic works to follow through: For example, when running this, x509_pubkey_decode is called 15 levels deep in macros like asn1_item_embed_d2i and I'm at a loss as to understanding what's (supposed to be) going on :-( Indeed, I've gained the impression the proposal above would introduce a recursion (as x509_pubkey_decode appears to be called by OSSL_DECODER_from_bio and thus probably shouldn't call it again (unless, maybe it could work on an unprocessed subsection of the X509, hence the question: Is there a way to get at the currently operated-on BIO (state) to ensure progress?)

Which documentation pertaining to all this would you recommend I should read to better get a grasp on this? Just reading the code seems suboptimal.

For example, when running this, x509_pubkey_decode is called 15 levels deep in macros like asn1_item_embed_d2i and I'm at a loss as to understanding what's (supposed to be) going on :-(

This is the ASN.1 DER decoding logic. Really you don't need to understand this to tackle this particular problem. We have an X509 structure which contains many fields and nested sequences that aren't particularly interesting for this problem. Eventually, within the bigger X509 structures, we hit a sub-structure, i.e. the "SubjectPublicKeyInfo" ASN.1 structure. This is represented internally as an X509_PUBKEY:

https://github.com/openssl/openssl/blob/8a9394c1eddbac210d4a2dceab521efa7518fa1f/crypto/x509/x_pubkey.c#L89-L92

You will note that the definition of that structure has a callback associated with it called pubkey_cb. That's defined here:

https://github.com/openssl/openssl/blob/8a9394c1eddbac210d4a2dceab521efa7518fa1f/crypto/x509/x_pubkey.c#L57-L87

By this stage in the ASN.1 DER decoding logic we have successfully decoded an X509_PUBKEY structure and the callback gets called to do whatever post-processing we care to do on it. pubkey_cb subsequently calls x509_pubkey_decode - and its the job of that function to get hold of an EVP_PKEY and cache it away. Don't worry about the 15 level deep macros....magic has happened and we now have to convert an X509_PUBKEY into an EVP_PKEY.

I've gained the impression the proposal above would introduce a recursion (as x509_pubkey_decode appears to be called by OSSL_DECODER_from_bio and thus probably shouldn't call it again

I've not investigated this, but I guess the upper layer decoder is to decode the higher level X509 cert structure. There shouldn't be a problem with re-entering the decoder code because we are now decoding a different sub-structure.

Is there a way to get at the currently operated-on BIO (state) to ensure progress?)

We can't get hold of the BIO at this point - what we have to work with is the X509_PUBKEY structure. The simplest thing to do is probably counter-intuitive and a bit nasty....call i2d_X509_PUBKEY to convert it back into a string of bytes - and then wrap that up in a mem BIO.

Which documentation pertaining to all this would you recommend I should read to better get a grasp on this? Just reading the code seems suboptimal.

There are a number of pages on the OSSL_DECODER API, e.g. start with:

https://www.openssl.org/docs/manmaster/man3/OSSL_DECODER_CTX_new_by_EVP_PKEY.html
https://www.openssl.org/docs/manmaster/man3/OSSL_DECODER_CTX.html
https://www.openssl.org/docs/manmaster/man3/OSSL_DECODER.html

There doesn't seem to be an "overview" (man7) page for this, which seems like a bit of an omission.

The simplest thing to do is probably counter-intuitive and a bit nasty....call i2d_X509_PUBKEY to convert it back into a string of bytes - and then wrap that up in a mem BIO.

Hmmm... If we re-encode the decoded DER structure (SubjectPublicKeyInfo) to then apply a (arguably the same) DER decoder (again) wouldn't it be logical that we recurse (into the same "done decoding this DER component" code)? To show the point, here's the code executed within x509_pubkey_decode enhanced with some printf's to make the point:

    printf("  ENTRY pub decode\n");
    if (OBJ_obj2txt(algname, 100, key->algor->algorithm, 1) > 0)
        printf("X509 OID: %s\n", algname);
    dec_ctx = OSSL_DECODER_CTX_new_by_EVP_PKEY(&pkey, "DER", NULL, algname, 0, NULL, NULL);
    if (!dec_ctx) {
        printf("No Decoder CTX retrieved\n");
    }
    else {
        printf("%d decoders found\n", OSSL_DECODER_CTX_get_num_decoders(dec_ctx));
        printf("Putting into membio: %d\n", i2d_X509_PUBKEY_bio(membio, key));
        printf("Stored %ld bytes\n", BIO_get_mem_data(membio, &data));
        printf("Decode result: %d\n", OSSL_DECODER_from_bio(dec_ctx, membio));
    }

and its result:

  ENTRY pub decode
X509 OID: 1.2.840.113549.1.1.1
3 decoders found
Putting into membio: 1
Stored 294 bytes
  ENTRY pub decode
X509 OID: 1.2.840.113549.1.1.1
....

Indeed, it blows the stack apart -- "Decode result" never appears.

Or did I overlook a mechanism to just call the EVP_PKEY constructor (which is apparently properly set after calling OSSL_DECODER_CTX_new_by_EVP_PKEY to https://github.com/openssl/openssl/blob/c9603dfa42d0643a6c8cac3e14364d9fd63303c4/crypto/encode_decode/decoder_pkey.c#L65 without doing all the rest (incl. calling into x509_pubkey_decode) again? Also/alternatively, is there a way to "manually" walk and execute single DECODERs in the DECODER_CTX structure? It really seems OSSL_DECODER_from_bio is the only way to execute all decoders --- which then leads to the recursion.

There doesn't seem to be an "overview" (man7) page for this, which seems like a bit of an omission.

Yup. Same with provider doc: Only provider-encoder, nothing on decoders.

without doing all the rest (incl. calling into x509_pubkey_decode) again?

Hmmm....that's an unexpected problem....somehow we need to act differently when called libcrypto side, to how we act provider side....because it seems the implementation of the provider side decoder code is itself calling d2i_X509_PUBKEY and hence we get the recursion, i.e. if an application calls d2i_X509_PUBKEY then we need need x509_pubkey_decode to call the OSSL_DECODER_CTX code - but if a provider calls d2i_X509_PUBKEY then we need it to go the old route.

but if a provider calls d2i_X509_PUBKEY then we need it to go the old route

Arguably not a(ny) (decoder) provider but only the built-in provider(s), right? Or is the assumption that all providers (incl. oqsprovider) could utilize d2i... (and i2d...) code? Just curious considering doing this for the oqsprovider: Actually, the more I'm reading the documentation, the more questions I have (also see openssl#13949): provider-encoder doesn't read as smoothly as the other provider-docs (keymgmt, signature, etc) and could do with some more guidance for provider developers (e.g., what not to use).

Now, when checking who's calling d2i_X509_PUBKEY only three locations turned up:

• d2i_X509_PUBKEY_fp and ..._bio, both of which are completely non-utilized (and un-tested) functions (strange in itself: I'd have assumed all public functions have at least 1 test: Isn't that a reasonable (test coverage) goal? Or am I overlooking something? If not, worthwhile opening an issue on this, too?).
• X509_PUBKEY_set -- which arguably is an encoder-side function, so not relevant in this context
• d2i_PUBKEY_ex which in turn means d2i_PUBKEY which is used pretty often, incl. code paths I wouldn't be able to label "provider-side only" or "crypto-side only" -- plus some clear provider-side calls. Anyway, I'm at a loss finding a clear-cut code location where to distinguish whether either function x509_pubkey_decode or d2i_X509_PUBKEY is called from a provider or not.

The only idea reading the code gave me is to check whether or not libctx within the X509_pubkey_st structure is set: I got the impression it is set when a provider has been involved and NULL if that's not the case: Is that correct? A quick test adding if (key->libctx) to x509_pubkey_decode executing the old code in that case and my new code switching on OID above in the else clause did break the recursion and decoded the RSA cert. I'm pretty unsure whether this is a valid solution now, though, given it's relying on an assumption.

Edit: Here's the diff at a glance: https://github.com/openssl/openssl/compare/master...baentsch:mb-providerdecoderselect?expand=1

Arguably not a(ny) (decoder) provider but only the built-in provider(s), right?

Right.

I suspect the provider side handling of this is not quite right. Maybe, it should not call d2i_PUBKEY at all - but possibly an alternative internal only version that uses a different callback...i.e. not x509_pubkey_decode. I had an exchange with @levitte about this issue this morning - so possibly he might chip in.

Uhmmmm, I don't see why calling d2i_PUBKEY_ex() from the der2key decoder would make much harm. Do note that after calling d2i_X509_PUBKEY() (which is the generated ASN1 function to decode DER into a X509_PUBKEY), d2i_PUBKEY() calls X509_PUBKEY_get(), which calls x509_pubkey_decode(). So in essence, calling d2i_PUBKEY_ex() does the same thing as when ASN1_item_d2i() comes to the point of decoding the SubjectPublicKeyInfo part of an X.509 certificate.

So yeah, for a conversion of a SubjectPublicKeyInfo to a X509_PUBKEY with a provider side EVP_PKEY, the function to rewrite is d2i_X509_PUBKEY()... and yes, I do hear the problem with knowing if it gets called down the line from a decoder or not and the recursion problem there. We have a similar problem with d2i_PrivateKey(), which has a proposed workaround that's still in an OpenSSL PR: openssl#13591. I haven't given that workaround enough thought yet to comment on it, but it might be an inspiration.

We have avoided, as much as possible, to make too much changes for provider stuff in the ASN.1 code... some of it is definitely not made for that.
You mentioned ASN1_item_sign_ex()... The ASN1_item_sign and ASN1_item_verify set of functions would really need to be rewritten from zero, and carry more signs of hacks than well thought out functionality, in my not very humble opinion. I was able to butcher ASN1_item_verify_ctx() to work with provider side keys, but oh boy how I dislike the result...

We have a similar problem with d2i_PrivateKey(), which has a proposed workaround that's still in an OpenSSL PR: openssl#13591. I haven't given that workaround enough thought yet to comment on it, but it might be an inspiration.

When looking at it, isn't this PR rather "perpetuating" the concept of NIDs/legacy keys:
EVP_PKEY *evp_privatekey_from_binary(int keytype,... with keytype being used in EVP_PKEY_set_type(ret, keytype) ?

As such it's not much of an inspiration to resolve openssl#13893 (where exactly the EVP_PKEY_set_type(...OBJ_obj2nid) pattern is considered the problem).

The ASN1_item_sign and ASN1_item_verify set of functions would really need to be rewritten from zero

I tend to agree: This https://github.com/openssl/openssl/blob/b897b353dff8138aa838bae9766ecd3de8c03280/crypto/asn1/a_verify.c#L133-L137 also looks like it wouldn't possibly be able to work with provider( key)s...

Maybe time to re-word openssl#13893 as "add provider key support everywhere" (or "eliminate use of OBJ_obj2nid" or --given the latter is unlikely considering the 196 places where it's used in OpenSSL-- turn things upside down and "assign provider algorithms their own NIDs")?

Something to remember is that OpenSSL is currently in a state of transition. For backward compatibility reasons, we need to consider the possibility that there are legacy implementations in the wild, and that involves both NIDs and structures like EVP_PKEY_METHOD. We may get them because of engines, or because the calling application has its own built-in implementations!

NIDs as we know them aren't really that good to keep around, because of their "constant" nature. I have it on good authority that the constants were never meant to become public, but they became so, and we got the role of OID/NID registry. We don't want to continue with that role, so we're avoiding the cementing of NIDs in the libcrypto<->provider interface, which is the boundary where we care very much what's assumed and what's being passed back and forth.
We do have a numeric identifier for the diverse provider algorithms in libcrypto, but that's a separate set of numbers, and completely dynamic. You can find out the numeric identifier for an algorithm with functions like EVP_SIGNATURE_number(), but there's no point attributing any more importance than a temporary number in that process... the same algorithm may have a different number in the next run.
Since we've chosen a name based approach for the new public EVP functionality and for key types, my expectation is that when all the legacy stuff is gone, we won't really need NIDs any more.

Another thing we decided is that we don't want to see any ASN1 code inside the FIPS module.

Apart from that, what happens within the confines of a provider is a different matter, and as you can see, we play pretty hard and fast in places, and that's ok, as long as we're careful what passes over the libcrypto<->provider interface.

The problem isn't so much about OBJ_obj2nid, all we'd need is to switch to OBJ_obj2txt and pass the result to something like EVP_PKEY_CTX_new_from_name()... something like that.

I believe I found the correct solution for openssl#13893, BTW. Thank you for the discussion so far, @baentsch, that was valuable input for finding my direction in this.

Thanks in turn for your insight and feedback, @levitte and @mattcaswell : It helped me understand better where things stand and how to better contribute. Please let me know when you're doing the PR addressing openssl#13893, so I understand where this is heading. Again, please let me know if you think there'd be issues for me to look into (short of re-writing ASN.1 for good :-) Until then, I'll try creating an oqsprovider encoder/decoder. Let's see how that pans out...

Until then, I'll try creating an oqsprovider encoder/decoder. Let's see how that pans out...

That will be a really useful exercise to make sure we got the API right.

Please let me know when you're doing the PR addressing openssl#13893

I've started on that. It will be a hack, but hopefully not too horrible...

See openssl#13994.

With that, it should be fairly easy to write your own decoders, with just one caveat: there's now a d2i_PUBKEY_legacy() for generic decoding that avoids the recursion problem, but it's currently internal. Unfortunately, I have a very hard time seeing a way around the recursion problem as long as there's this automatic EVP_PKEY decoding happening in d2i_X509_PUBKEY(). The decoder functionality is really designed around the providers handle all decoding themselves, including of common structures.

We're able to cheat quite a lot in OpenSSL's providers, mainly because we know that our decoders are built in and has access to all of libcrypto's internals.

The ideal thing would be, of course, if OpenSSL would offer unencumbered structure decoders (for example a variant of d2i_X509_PUBKEY() that didn't try to automatically decode its contents further, and something similar for PKCS#8)... it should be possible to do, and what to name those is the question.

Anyway, the best I can recommend is, unfortunately, that you create your own SubjectPublicKeyInfo structure for provider usage, using our crypto/x509/x_pubkey.h as a code template... at least for now.
(actually, just writing this has my mind going on how we can help support this sort of thing)