Improve ML-DSA: Keygen, pure/prehash, NIST vectors, Wycheproof - #34
Open
PierreAndouche-Oppida wants to merge 2 commits into
Open
Improve ML-DSA: Keygen, pure/prehash, NIST vectors, Wycheproof#34PierreAndouche-Oppida wants to merge 2 commits into
PierreAndouche-Oppida wants to merge 2 commits into
Conversation
ML-DSA: - Extend ML-DSA to support pure/prehash variant, following SLH-DSA convention - Add keygen tests - Add deterministic signing tests - Replace the self-generated test vectors with official public ACVP FIPS 204 test vectors by NIST - As a result of the new vectors, add invalid tests for signature verification - Update the test ML-DSA harness correspondingly Choices: - The old test vectors still work, this can be tested by not passing optional args to load_vectors. Both source of vectors can coexist if the rd.add calls are adapted accordingly. However I went with unilateral use of the official vectors only in the code I'm presenting. Misc: - Specify example system dependencies for Debian in CONTRIBUTING - Fix some docstrings and messages (not all) - Fix sig being ctx in SignData debug info - Fix some silent invalid branches
Changes: - Adds the datamodel-code-generator dependency - Generates wycheproof schema parsers with datamodel-codegen, ignore them in linting - sign functions can now fail - Updated test harness for wycheproof: the ref does not take (most) length info, so auto-succeed the corresponding InvalidLength tests - Add compliance/resilience flag support for ML-DSA - Added input checking in the example harness where the ref is not wrong - Added input checking in the pytests where necessary for success with regard to ref/the use of the internal function Notes: - Tests are either valid or invalid. testvectors_v1 does not have any "acceptable" for ML-DSA. The tests have a bugType such as "LEGACY" or "WEAK_PARAMS". Support was not added. - The ref implementation really does fail the InvalidPrivateKey signing tests
Collaborator
|
Thanks for the PR, I'll review it this week. It seems to cover everything we need for ML-DSA. :) No worries about the documentation, I'll update it after merging whenever I have the chances of pushing the new API. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
More details of changes are in the commit messages.
Notably, the more official FIPS 204 and Wycheproof vectors bring in invalid tests for signature verification, whereas currently a signature verification implementation under test just needs to
return True;to succeed.Similarly, support for keygen testing allows to cover seed expansion code which is currently not covered.
The pure/prehash setup was done by copying SLH-DSA, where possible.
Documentation was not modified, in the event the chosen API needs changing.
make allfully passes, except for test_harness since the ref implementation fails 1 Wycheproof test.