Implement encoder/decoder

[baentsch]

As per https://www.openssl.org/docs/manmaster/man7/provider-encoder.html

[baentsch]

The more I look into this, the more I'm confused. Sample questions (I guess to @levitte @mattcaswell @romen if you'd have time):

1. How to best test new code? Testing a new encoder against a new decoder looks a bit "incestuous". Or would a simple "roundtrip" like this already be reasonable: OSSL_ENCODER_fetch -> OSSL_ENCODER_to_bio (s_membuf) + OSSL_DECODER_fetch/OSSL_DECODER_from_bio?
2. Which output types have/need to be supported to use this beyond "internal testing"? Only "DER"? Or also "PEM"? "text"? Others?
3. Which way to exercise this encoder/decoder "for real"? What I'd like to do is create OQS keys via openssl commands and then store/retrieve them into/from PEM files (and finally use them, e.g., for CMS-style signing). First issue: How to facilitate that? openssl genpkey for example doesn't seem to work:

> LD_LIBRARY_PATH=.local/lib .local/bin/openssl list -signature-algorithms -provider-path _build/oqsprov  -provider oqsprovider
  oqs_sig_default @ oqsprovider
  dilithium2 @ oqsprovider
  dilithium3 @ oqsprovider
  dilithium4 @ oqsprovider
  falcon512 @ oqsprovider
  falcon1024 @ oqsprovider
  picnicl1full @ oqsprovider
  picnic3l1 @ oqsprovider
  rainbowIclassic @ oqsprovider
  rainbowVclassic @ oqsprovider
  sphincsharaka128frobust @ oqsprovider
  sphincssha256128frobust @ oqsprovider
  sphincsshake256128frobust @ oqsprovider
> LD_LIBRARY_PATH=.local/lib .local/bin/openssl genpkey -algorithm dilithium2 -provider-path _build/oqsprov  -provider oqsprovider
Error initializing dilithium2 context
C0B14AE0007F0000:error:0308010C:digital envelope routines:inner_evp_generic_fetch:unsupported:crypto/evp/evp_fetch.c:331:Global default library context, Algorithm (dilithium2 : 0), Properties (<null>)

-> How and where does the "global default lib context" as per the error message come into play again? I'd have thought my explicit command line has switched it off (?)

Or did I already at first try run into a corner as per the below?

Keys have support, at least in principle. There are corners where we haven't quite fulfilled that yet, but provider backed are meant to be possible to use (almost) everywhere, and transparently.

(from open-quantum-safe/openssl#243 (comment))

-> Should I rather open issues/questions for this in the main OpenSSL issues tracking system?

Thanks in advance for any feedback/input as per the questions above!

[romen]

Quickly commenting on the third issue: does it go away if the provider argument comes before the algorithm selection (or if the provider is loaded via conf file)?

[mattcaswell]

How to best test new code? Testing a new encoder against a new decoder looks a bit "incestuous". Or would a simple "roundtrip" like this already be reasonable: OSSL_ENCODER_fetch -> OSSL_ENCODER_to_bio (s_membuf) + OSSL_DECODER_fetch/OSSL_DECODER_from_bio?

This seems like a reasonable approach - with the addition that you should ideally check that the thing you have decoded has the same values in it as the thing you encoded. You might take some inspiration from test/endecode_test.c.

Obviously this will only test that you can serialize a key to a file and deserialize it again. What it won't do is test that you got the encoding correct according to standards. The only real way to do that is to do some kind of interoperability testing with other implementations (if there are any).

Which output types have/need to be supported to use this beyond "internal testing"? Only "DER"? Or also "PEM"? "text"? Others?

For decoding there is a generic pem-to-der decoder. So you only need to implement the der-to-key decoder. The implementation should be able to chain the two together and give you pem-to-key "for free".

I believe the current encoder implementation doesn't have this same flexibility and so you need to support both pem and der. I think this is probably one of those "rough" corners and I think @levitte wanted to harmonise the way encoders work with the way decoders work a bit more. Perhaps @levitte can comment here?

Wrt "text", that's only relevant for encoding. The command line apps use the text encoders to be able to display details about keys to the user with the various "-text" command line options (e.g. "openssl pkey -text"). So if you want end users to be able to examine the details of keys you'll need a text encoder.

LD_LIBRARY_PATH=.local/lib .local/bin/openssl genpkey -algorithm dilithium2 -provider-path _build/oqsprov -provider oqsprovider
Error initializing dilithium2 context

Hmmm. This looks like it should work. Probably raise this as an issue in our tracker so someone takes a look at it.

How and where does the "global default lib context" as per the error message come into play again? I'd have thought my explicit command line has switched it off (?)

No - your command line just loads providers. It doesn't say anything about which lib ctx is used. Its perfectly possible (and normal) to load providers into the global default lib ctx, and this is what the command line apps generally do - so nothing unusual there.

[baentsch]

Quickly commenting on the third issue: does it go away if the provider argument comes before the algorithm selection (or if the provider is loaded via conf file)?

Excellent question. The answer points to a real bug in the code:

LD_LIBRARY_PATH=.local/lib .local/bin/openssl genpkey -provider-path _build/oqsprov -provider oqsprovider -algorithm dilithium4

never returns but sends my CPU load to 100%..... Will open an issue: openssl/openssl#14069

[levitte]

For testing, I would actually check the result against files that you have generated with your 1.1.1 port of OQS. Basically, build up a set of test vectors and use that as a reference for "getting it right".

[levitte]

I believe the current encoder implementation doesn't have this same flexibility and so you need to support both pem and der. I think this is probably one of those "rough" corners and I think @levitte wanted to harmonise the way encoders work with the way decoders work a bit more. Perhaps @levitte can comment here?

Yes, that's more or less it.

[anhu]

@baentsch : quick tip:

have a look at providers/implementations/encode_decode/encode_key2any.c . Run it through the C compiler's pre-processor and look at the the output. It makes it much clearer whats going on with some of the implementation. If you can follow what is going on with X25519 in there then you're on a good start. Your implementation of an encoder will probably be similar to that, except much more simplified.

[baentsch]

Implementing encoder functionality means the OQS-provider needs to deliver ASN.1 objects for all OQS algorithms. To avoid writing our own ASN.1 generator, we use in the OQS-OpenSSL 1.1.1 fork the Makefile target generate_crypto_objects (after suitably adding OQS-OIDs to crypto/objects/objects.txt) and later on tying into the OBJ_nid2obj API.

For the standalone provider, it'd also be straightforward to (re)use the OpenSSL object generator perl scripts, but there's something making me uneasy about this: a) re-using "someone else's code" -- albeit that should be OK as we'd be calling into source code complete with OpenSSL Copyright notices. More troubling in my eyes is that b) this generates header files with OpenSSL Copyright notices -- and we'd need to check them in: That arguably is not OK, right? Specifically tagging @levitte and @mattcaswell for guidance on this.

WIP code to see what I mean available here with resultant headers e.g. here.

Comments and alternative suggestions (pointer to other OID->ASN-object generator code if available?) very welcome.

[mattcaswell]

a) re-using "someone else's code" -- albeit that should be OK as we'd be calling into source code complete with OpenSSL Copyright notices.

I don't see a problem here. You aren't distributing OpenSSL itself. You just expect it to be present in order to form part of your toolchain, e.g. just like gcc, or "make", or whatever other tools you use.

b) this generates header files with OpenSSL Copyright notices -- and we'd need to check them in: That arguably is not OK, right?

Caveating my answer with "I am not a lawyer" so I can only offer "opinion", but... its difficult to see how we (OpenSSL) could claim copyright on those generated sources any more than "gcc" could claim copyright on the object files it generates from your ".c" files. The "data" is just your data from the objects.txt input file that you wrote in a reformatted form. And the "structural" bits seem trivial.

The objects.pl script is available under the Apache license and therefore you are allowed to amend it. So one "hack" you could do to "fix" this, is for your scripts to create a temporary copy of objects.pl and then automatically hack it to amend the generated copyright message to one of your choosing. Create the header files using the hacked objects.pl, and then delete it.

Or alternatively, if you're willing to have the Apache 2 licensed code in your repo, just take a copy of that one script. Modify the copyright message it generates and commit the script, and use your copy of the script henceforth. We do something similar with some third party perl stuff that we use here:

https://github.com/openssl/openssl/blob/master/external/perl/Downloaded.txt

[baentsch]

Thanks for the many suggestions, @mattcaswell ! I think I'll go for

Create the header files using the hacked objects.pl, and then delete it.

[baentsch]

Now "found" an alternative way to the above: Using the core/provider callbacks to register new algorithms' OIDs (OSSL_FUNC_core_obj_create) when initializing the provider seems to be the "intended" way of doing this, right?

However, one issue has me wondering: In order for the X509(pubkey) logic to work correctly ("find" the new/provider signature algorithms), it seems to be required to also call into OSSL_FUNC_core_obj_add_sigid: Is that intentional? Probably, as without doing this, the new signature algorithms are not found by the X509 logic -- courtesy the code in OBJ_find_sigid_algs if I understand things right (?)

If so, this is conceptually great, if there were a message digest that one could meaningfully register when calling core_obj_add_sigid -- unfortunately, for QSC there is none, as those algorithms don't need digesting (i.e., work on arbitrary length data when signing).

So my question is this: Would it make sense (to change its implementation) for OBJ_find_sigid_algs to also return sig algs that have not been registered via core_obj_add_sigid (but only via core_obj_create) -- and consequently don't "force" application of a message digest algorithm before entering the provider's "sign" method? Or is the logic of requiring digesting so deeply embedded in the OpenSSL (X509) logic that our provider better provide its own digest implementation (probable name "DO_NOTHING" :) to register with core_obj_add_sigid?

As always thanks in advance for suggestions/corrections.

[baentsch]

@levitte Suggestion sought for getting at passphrase, e.g., here. Thoughts on the issue above also very welcome.

[levitte]

@levitte Suggestion sought for getting at passphrase, e.g., here. Thoughts on the issue above also very welcome.

Well, my first question is, why do you want to implement EncryptedPrivateKeyInfo -> PrivateKeyInfo decoding? There is already a decoder that does this explicitly in OpenSSL.

However, there's really nothing magic about our internal pw functions, they should be easy to replicate... something like this would do:

struct key2any_ctx_st {
    PROV_OQS_CTX *provctx;

    /* Set to 0 if parameters should not be saved (dsa only) */
    int save_parameters;

    /* Set to 1 if intending to encrypt/decrypt, otherwise 0 */
    int cipher_intent;

    EVP_CIPHER *cipher;

    OSSL_PASSPHRASE_CALLBACK *pwcb;
    void *pwcbarg;
};

// A passphrase wrapper
int get_passphrase(char *pass, size_t pass_size, size_t *pass_len, struct key2any_ctx_st *ctx)
{
    return ctx->pwcb(pass, pass_size, pass_len, NULL, ctx->pwcbarg);
}

Then use this wrapper instead of ossl_pw_get_passphrase(), and replace the call of ossl_pw_set_ossl_passphrase_cb() with this:

    ctx->pwcb = pwcb;
    ctx->pwcbarg = pwcbarg;

There's really not much more to it.

The reason that OpenSSL has the set of ossl_pw function is primarily because we have a total of three methods to get a passphrase: pem_password_cb, UI methods, and now OSSL_PASSPHRASE_CALLBACK. The ossl_pw functionality is an attempt to gateway between all those methods through a common interface.

[levitte]

Also, the get_passphrase wrapper can be modified to simply call whatever passphrase reader you want to use. There's nothing obligating you to call the passphrase callback (ultimately passed from the application). If you can open a terminal internally, then you can read it from there as well.

[baentsch]

@levitte Thanks very much for the explanation. I failed to grasp how to use the callback but now got it. Is there documentation for this? Would it make sense to point to that in closing openssl/openssl#16746 for the benefit of others? What I found in documentation about the callback arguments doesn't seem to be totally in line with the above.

[baentsch]

Asking for advice from @mattcaswell @levitte (rephrasing/shortening the above]:

Is it intentional/required for a provider to call OSSL_FUNC_core_obj_add_sigid when registering a signature algorithm that does not need a digest (QSC algs can sign arbitrary length data)?

When not doing this, OBJ_find_sigid_algs does not find a new signature algorithm "only" registered via OSSL_FUNC_core_obj_create and the X509(pubkey) logic consequently fails.

So my question is this: Would it make sense (to change its implementation) for OBJ_find_sigid_algs to also return sig algs that have not been registered via core_obj_add_sigid (but only via core_obj_create) -- and consequently don't "force" application of a message digest algorithm before entering the provider's "sign" method? Or is the logic of requiring digesting so deeply embedded in the OpenSSL (X509) logic that our provider better have its own ("do nothing") digest and register that via OSSL_FUNC_core_obj_add_sigid?

[anhu]

Hi @baentsch ,

Or is the logic of requiring digesting so deeply embedded in the OpenSSL (X509) logic...

I don't think the logic would be so deeply embedded as both ED25519 and ED448 can also sign arbitrary length data. If you investigate what they do, you can try to mimic the approach taken there.

[mattcaswell]

There is precedent in OpenSSL for signature algs without an associated digest. For example Ed25519 and Ed448 do not use one. See:

https://github.com/openssl/openssl/blob/64da15c40d15aac58e211fd25d00e9ae84d0379b/crypto/objects/obj_xref.h#L76-L77

IMO the correct thing to do would be to still call OSSL_FUNC_core_obj_add_sigid but for signid and pkey_id to be the same, and for dig_id to be NID_undef.

However, I note that we actually explicitly disallow this, even though we have built-in algorithms that do this:

https://github.com/openssl/openssl/blob/64da15c40d15aac58e211fd25d00e9ae84d0379b/crypto/objects/obj_xref.c#L144-L145

This seems like a bug to me.

[mattcaswell]

I raised this as a bug in OpenSSL:

openssl/openssl#16761

[anhu]

DOH! I stand corrected.

[baentsch]

This seems like a bug to me.

"Glad" to hear that: Already ran into that but assumed there was a good reason for that check. Thanks for creating OpenSSL#16761: If that were resolved (permitting sig ops without digest) the issue here would indeed be dealt with.