Update Dilithium from crystals upstream #1894
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1894 +/- ##
==========================================
- Coverage 78.47% 78.46% -0.02%
==========================================
Files 585 585
Lines 99518 99517 -1
Branches 14242 14242
==========================================
- Hits 78099 78086 -13
- Misses 20783 20794 +11
- Partials 636 637 +1 ☔ View full report in Codecov by Sentry. |
|
|
||
| **Source code origin and modifications.** The source code was imported from a branch of the official repository of the Crystals-Dilithium team: https://github.com/pq-crystals/dilithium. The code was taken at [commit](https://github.com/pq-crystals/dilithium/commit/cbcd8753a43402885c90343cd6335fb54712cda1) as of 10/01/2024. At the moment, only the reference C implementation is imported. | ||
|
|
||
| The `api.h`, `fips202.h` and `params.h` header files were modified to support our [prefixed symbols build](https://github.com/awslabs/aws-lc/blob/main/BUILDING.md#building-with-prefixed-symbols). |
There was a problem hiding this comment.
It may be helpful to describe how they were modified in order to help future maintainers that have to deal with conflicts.
There was a problem hiding this comment.
I completely agree. There are going to be further changes to the Dilithium code, then I will update this readme after to reflect all changes. I've left the note about the prefix build stuff in here as-is for now, as in a following PR I will be removing all the prefix build stuff for Dilithium. Essentially, the same changes made to ML-KEM (7b5ba89) will be made for ML-DSA. Once those changes are in, I will update this text with similar text from the ML-KEM readme.
There was a problem hiding this comment.
Yeah this would be helpful so I can verify the code was copied over without unintended modifications.
There was a problem hiding this comment.
Addressed in 72d7271. Will continue to update this readme with additional changes.
There was a problem hiding this comment.
Added more detail in 7f31525.
|
|
||
| **Source code origin and modifications.** The source code was imported from a branch of the official repository of the Crystals-Dilithium team: https://github.com/pq-crystals/dilithium. The code was taken at [commit](https://github.com/pq-crystals/dilithium/commit/cbcd8753a43402885c90343cd6335fb54712cda1) as of 10/01/2024. At the moment, only the reference C implementation is imported. | ||
|
|
||
| The `api.h`, `fips202.h` and `params.h` header files were modified to support our [prefixed symbols build](https://github.com/awslabs/aws-lc/blob/main/BUILDING.md#building-with-prefixed-symbols). |
There was a problem hiding this comment.
Yeah this would be helpful so I can verify the code was copied over without unintended modifications.
Issues:
Resolves #CryptoAlg-2722
Description of changes:
This PR updates the implementation of Dilithium (v3.1) with subsequent changes made in the official upstream repository (https://github.com/pq-crystals/dilithium).
As there are significant algorithmic changes between pre-FIPS 204 Dilithium (v3.1) and FIPS 204 IPD/Final we note that the integration of this code will cause Dilithium v3.1 signatures to fail. Consumers of AWS-LC were made aware of the unstable nature of the Dilithium implementation and API during the integration into the library (see https://github.com/aws/aws-lc/blob/8a1ee93969d8df64c4c51b2d6ddffb26a54adea9/crypto/dilithium/README.md).
As part of our due diligence we have verified that there are no existing deployments contingent on the stability of Dilithium. As such, we will continue to support the most up to date version of the algorithm from the authors upstream.
Call-outs:
Among the changes are modifications made per the FIPS 204 ML-DSA standard:
fqmulrefactor added in code refactor to add fqmul #1748Changes from upstream source:
Testing:
As the signature size and private key size of ML-DSA has changed since v3.1, so too must the tests
EVPExtraTest.d2i_PrivateKeyfor testing the parsing of ML-DSA private keys, as well as the certificates used forX509Test.TestDilithium3.I have updated the private key
kExampleDilithium3KeyDERand test certificatekDilithium3Certto reflect changes to the signature/key lengths, but will need to manually reproduce test-case certificateskDilithium3CertNull,kDilithium3CertParam.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.