Reduced combinations

[feventura]

This PR was created to facilitate the identification of the changes made to implement composite signatures and certificates.

This branch only contains:

• MLDSA65-RSA3072-PKCS15-SHA512
• MLDSA65-ECDSA-P256-SHA512

[ounsworth]

That is actually "pkixAlgs", not "sigAlgs". For example, key exchanges and KEMs are also in the same arc.

https://www.iana.org/assignments/smi-numbers#smi-numbers-1.3.6.1.5.5.7.6

I suggest renaming this to "pkixAlgs". Also, if you're trying to be really efficient this arc is probably already defined somewhere in the openssl includes (but maybe it's just easier to define it fresh here).

[ounsworth]

This function seems unintuitive to me and is probably worth a header comment.

(Part of my problem is that I don't know what openssl ossl_ml_dsa_key_has is, but a quick Google AI search tells me that this is checking for the ML-DSA private key in Full, Seed, or Both mode).

Will a composite key ever have the ML-DSA component in Full or Both mode? Or only ever Seed?

[ounsworth]

I know from the reference implementation that the RSA and EC public keys can change their length by one or two bytes (I think, depending on leading zeros, which get truncated in the DER encoding). You also need to account for the DER length tags in the classic component.

Is this code actually counting the encoded length, or is it assuming a maximum length? It looks like EVP_PKEY_get_bits is the maximum size that you could ever need for a buffer for this.

If that's the case, that the size_t returned from ossl_composite_key_get_pub_len might actually be longer than the key it contains, then you should say that in the header comment.

[ounsworth]

That's a bit of a weird way to measure the security strength of a composite. In the draft, we tried very hard to not do this because it's rather difficult to compare the security of PQ and classical algs.

Is this as function that you're required to implement in order for the EVP APIs to work correctly? If you deleted this function, does anything break?

[ounsworth]

This might be my unfamiliarity with openssl, but what is this validating? That the public keys and private keys match? It's not validating a signature.
It's probably worth adding a header comment.
(also, maybe there is a header file somewhere that declares these functions, and there's a comment on that?)

[ounsworth]

This is excellent and shows that you're ready for the RSA-PSS stuff, even though it's not in this PR.

[ounsworth]

I'm not super convinced that a single-line helper function is all that helpful, but that's really a nit-pick.

[ounsworth]

I think this needs to be a bit careful here because, as I understand this, this will return a max length, not necessarily the exact length.
That should be noted in the comment here for future maintainers, and check that this is only used for sizing buffers, which are ok to be oversized and contain some trailing un-used bytes, and nothing is expecting this to be an exact length.

[ounsworth]

Would it be better to pre-compute the DER strings and hard-code them as hex, rather than having this code?

"Better" in the sense that it's easier to code review?

[ounsworth]

These could be implemented, right?
If I am guessing correctly at what these are supposed to do, they could be supported.
_init() would need to initialize the composite PH, each chunk of message handed into _update() would be fed into that hash, and _final() would finish the PH, construct the M' and finish the operation.

You might find it easier to refactor so that you have a composite_sign_ph() and composite_verify_ph() as a shared building block between the streaming and one-shot functions.

Moreover, it would be worth exposing the composite_sign_ph() externally to openssl because calling applications may want to pre-compute the PH themselves (for example on another machine so that you only need to send the hash value, and not the whole message, over the network).

I think these are definitely worth implementing.