Add ML-DSA-44 and ML-DSA-87 to PQDSA API #2009
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2009 +/- ##
==========================================
- Coverage 78.90% 78.69% -0.22%
==========================================
Files 594 598 +4
Lines 102415 103323 +908
Branches 14517 14686 +169
==========================================
+ Hits 80812 81308 +496
- Misses 20953 21364 +411
- Partials 650 651 +1 ☔ View full report in Codecov by Sentry. |
crypto/dilithium/p_pqdsa_test.cc
Outdated
| @@ -403,8 +1023,14 @@ TEST_P(PQDSAParameterTest, KAT) { | |||
|
|
|||
| // Generate key pair from seed xi and assert that public and private keys | |||
| // are equal to expected values from KAT | |||
| if (name == "MLDSA65") { | |||
| ASSERT_TRUE(ml_dsa_65_keypair_internal(pub.data(), priv.data(), xi.data())); | |||
| if (name == "MLDSA44") { | |||
There was a problem hiding this comment.
Can the KAT Test use the EVP API like the following tests, or it's intended to use the internal APIs?
There was a problem hiding this comment.
If we would like the KATs to use the EVP APIs, we would need to add new EVP methods for sign_internal and verify_internal. Those new methods would allow the randomness to be supplied as an additional argument, and call the internal functions rather than the external. Given that these internal methods are for testing/validation purposes only, there is no use for them within the EVP APIs.
These internal functions were not designed for any other use-cases than testing, see FIPS 204 page 22 section 6.
Other than for testing purposes, the interfaces for key generation and signature generation specified
in this section should not be made available to applications, as any random values required for key
generation and signature generation shall be generated by the cryptographic module.
crypto/dilithium/pqdsa.c
Outdated
| out->nid = NID_MLDSA44; | ||
| out->oid = kOIDMLDSA44; | ||
| out->oid_len = sizeof(kOIDMLDSA44); | ||
| out->comment = "MLDSA44 "; |
There was a problem hiding this comment.
I see that there's a trailing space here and that it's consistent with ML-DSA-65. Can you confirm that this is intentional?
There was a problem hiding this comment.
It was intentional, as the ML-KEM comments all have a space, I assumed it was needed (https://github.com/jakemas/aws-lc/blob/e4092fb224a2336b8e0d908f311924794d739042/crypto/fipsmodule/kem/kem.c#L137)
However, looking at the EC curves it appears it is not:
https://github.com/jakemas/aws-lc/blob/97a6706becf18605c83f526d4d08e90b74e7f3c6/crypto/fipsmodule/ec/ec.c#L118
crypto/dilithium/pqdsa.c
Outdated
| case NID_MLDSA44: | ||
| return &pqdsa_asn1_meth; | ||
| case NID_MLDSA65: | ||
| return &pqdsa_asn1_meth; | ||
| case NID_MLDSA87: | ||
| return &pqdsa_asn1_meth; |
There was a problem hiding this comment.
| case NID_MLDSA44: | |
| return &pqdsa_asn1_meth; | |
| case NID_MLDSA65: | |
| return &pqdsa_asn1_meth; | |
| case NID_MLDSA87: | |
| return &pqdsa_asn1_meth; | |
| case NID_MLDSA44: | |
| case NID_MLDSA65: | |
| case NID_MLDSA87: | |
| return &pqdsa_asn1_meth; |
Issues:
Resolves #CryptoAlg-2725
Description of changes:
This PR adds ML-DSA-44 and ML-DSA-87 to AWS-LC. As we already have support for ML-DSA-65 through the PQDSA signature API (see #1963) and already support internal functions and KATs that use these internal APIs (see #1999), this PR consists of:
pqdsaAPIs for ML-DSA-44 and ML-DSA-87:ml_dsa_{44/87}_keypair,ml_dsa_{44/87}_keypair_internal,ml_dsa_{44/87}_sign,ml_dsa_{44/87}_sign_internal,ml_dsa_{44/87}_verify, andml_dsa_{44/87}_verify_internalsig_ml_dsa_44_methodandsig_ml_dsa_87_methodsig_ml_dsa_44andsig_ml_dsa_87Call-outs:
I haven't hooked up ML-DSA-44/87 to X.509 in this PR, to keep the PR focused to a single feature addition.
I have hooked up ML-DSA-44/87 to the speed tool; see example output:
Testing:
#1999 provided a test frame work for all
pqdsasignature types, as such,ML-DSA-44/87are added to this test harness by:This requires the inclusion of test harness raw public keys
mldsa{44/87}kPublicKeyand encoded public keysmldsa{44/87}kPublicKeySPKI.The lengths of the encodings are well defined by https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.