PQC: ML-DSA

[FAlbertDev]

Adds an implementation of the final standard of FIPS 204 (ML-DSA) to the existing Dilithium code. Since it is based on the refactored version of Kyber/Dilithium, it replaces #4062. (I did not base the changes on the old PR since the new dilithium code base is too different)

@atreiber94 I added you as a co-author since I copied half of your code from #4062.

Things to do

Effective: 17 September

Here's what is left to do:

• Remove "_ipd" suffix from all the things
  Suggestion: rename the internal ml_dsa_ipd.h to ml_dsa_impl.h in order to leave room for a future public ml_dsa.h
  • Headers
  • Include guards
  • enum definitions
  • method names (eg. is_ml_dsa_ipd())
  • tests
  • algorithm specifiers and stringified names
  • file names of test vectors
• Let ML-DSA store the seed of the private key instead of its expanded serialization (private key in seed format for ML-KEM, ML-DSA, and SLH-DSA ? #4327)
• X.509
  • Register the official OIDs
  • Read through the IETF draft and validate that our X.509 implementation handles ML-DSA keys correctly
    (18.09. RM: looks good to me. Except the private key encoding as a seed (considering the draft linked above from July '24. But that's supposed to change).
• Remove the TODOs and enable the adaptions for the final ML-DSA implementation in ml_dsa_ipd.h
• Integrate public test vectors
  • Wycheproof draft tests? (will come later)
  • ACVP tests (#1, #2, #3)
• Go through FIPS 204 again and see if all validations are implemented (add tests)
• Clean the Commit history
• Implement PrehashML-DSA
  depends on Add PK_Signature_Options #4318 (planned for 3.7.0)
• Implement the new "context" parameter in signing/verifying
  depends on Add PK_Signature_Options #4318 (planned for 3.7.0)

• Add dedicated raw key loading functions to FFI/Python (see also Generic de-/encoding of raw public keys via FFI (in particular for KEMs)? #4366)

[coveralls]

coverage: 91.146% (+0.02%) from 91.131%
when pulling 041fb0a on Rohde-Schwarz:feature/ml-dsa-ipd-after-refactoring
into 7dd5905 on randombit:master.

[randombit]

https://nvlpubs.nist.gov/nistpubs/fips/nist.fips.204.pdf

[reneme]

Appendix D.3 lists a few minor changes in the truncation of certain hash values. Those should be simple changes. Also, there's now a domain separation in the hashing of the random seed in the key generation, similar to ML-DSA.

Also there's now a "pre-hash" version of ML-DSA, where the user provides a hash value instead of the entire "data to sign". Key generation for HashML-DSA is equivalent, but signing and verification is not.

I feel that supporting HashML-DSA should be fairly simple to do, but I would suggest to not do that in this particular PR. Perhaps, we even want to take a step back and make this a first-class API extension. This may (or may not) align quite nicely with other use cases where a user might want to handle the data hashing independently from the actual signing (eg. for TPM or #4269 (comment)).

[randombit]

I feel that supporting HashML-DSA should be fairly simple to do, but I would suggest to not do that in this particular PR.

TBH my inclination would be to just wait and see if anyone even adopts this. For comparison the Ed25519 RFC introduced the Ed25519ph variant, and I don't know that I've ever seen a system where it was actually used.

I looked at RFC 8032 again, and - regarding the prehashed variant that the RFC itself introduced, which did not exist prior - it says

These variants SHOULD NOT be used.

Thanks I guess. 😂

[falko-strenzke]

I looked at RFC 8032 again, and - regarding the prehashed variant that the RFC itself introduced, which did not exist prior - it says

These variants SHOULD NOT be used.

Thanks I guess. 😂

Well, I think the problem is a somewhat more subtle. In the RFC, the statement "SHOULD NOT be used" is preceded by the text

This is mainly useful for interoperation with legacy APIs [...] The prehashing also makes the functions greatly more vulnerable to weaknesses in hash functions used.

So I think the "SHOULD NOT" needs to interpreted as: "The pure variant is better. Use the pre-hash variant only if you cannot use the pure variant".

For instance for the integration of PQC into OpenPGP, we need pre-hashing, as changing that would require rewriting substantial parts of the protocol specification. And I think the fact that previously the prehash-variants of RFC 8032 have not been widely used is often by mistake. By that I mean that in certain cases where the pre-hash variant had to be used, the pure variant was used. One such case is the OpenPGP specification given by RFC 9580 itself. It simply specifies hash-and-sign with the pure variant of Ed25519. If digging enough, one probably finds other examples. Uses the pure variant for hash-and-sign, i.e. simply setting the hash as the message, works fine unless the "correct" use of the pure variant is also added (i.e., actually setting the message as the message). When that happens, signature forgeries are introduced into the protocol.

So I would ask to add the pre-hash variant, as that is what we need for using it in Thunderbird (via RNP, which uses Botan) according to what will be specified in https://datatracker.ietf.org/doc/draft-ietf-openpgp-pqc/ for the adaption to the final NIST standards of the two signature schemes. Otherwise we have to resort to the same formally invalid construction by using the "pure" variant for hash-and-sign.

[reneme]

So I would ask to add the pre-hash variant, as that is what we need for using it in Thunderbird (via RNP, which uses Botan)

With the new public API being discussed in #4318 we'll remove the biggest obstacle to the implementation of Prehash-Mode; namely that it isn't straightforward to integrate in the existing PK_Signer API. With that out of the way, there's no reason not to add support for prehash mode.

[twiss]

So I would ask to add the pre-hash variant, as that is what we need for using it in Thunderbird (via RNP, which uses Botan) according to what will be specified in https://datatracker.ietf.org/doc/draft-ietf-openpgp-pqc/ for the adaption to the final NIST standards of the two signature schemes.

At the risk of derailing this discussion, I'd like to add that this part is still being discussed on the OpenPGP mailing list, and I personally disagree with the proposal to use the pre-hash variant, because FIPS 204 also says (similarly to RFC 8032):

5.4 Pre-Hash ML-DSA
(...) This version of ML-DSA is known as “pre-hash” ML-DSA or HashML-DSA . In general, the “pure”
ML-DSA version is preferred.
(...)
If the content to be signed is large, hashing of the content is often performed at the application level.
For example, in the Cryptographic Message Syntax [29], a digest of the content may be computed, and
that digest is signed along with other attributes. If the content is not hashed at the application level, the
pre-hash version of ML-DSA signing may be used.

So, basically, if the application can prehash the data itself and ensure proper domain separation, there's no need for HashML-DSA, and the use of pure ML-DSA seems to be recommended instead.

[falko-strenzke]

So I would ask to add the pre-hash variant, as that is what we need for using it in Thunderbird (via RNP, which uses Botan) according to what will be specified in https://datatracker.ietf.org/doc/draft-ietf-openpgp-pqc/ for the adaption to the final NIST standards of the two signature schemes. Otherwise we have to resort to the same formally invalid construction by using the "pure" variant for hash-and-sign.

Yes, correct, I needed to say "might be specified in ...".

[reneme]

Just like ML-KEM, ML-DSA now exclusively uses the private seeds (xi - 32 bytes) as the private key's storage format. The expanded key format is not supported for ML-DSA. This does not affect the Dilithium-R3 implementations, which continue to use the expanded private key.

[reneme]

@randombit This is now ready for final review. It would be amazing to have this merged before the 3.6.0 release. Please let us know if we can help in some way.

Left for future work:

• API Future-Proofing, currently the public API is mostly Dilithium_*** and we should probably provide ML_DSA_*** aliases

Note:

The commits that introduce Bounded_XOF and that alter the PK_Keygen_Test class are repeated in the ML-KEM PR. We'll rebase and resolve conflicts once one of them got merged.

[reneme]

Rebased after #4367 caused a conflict.