Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add checksum KAT to Github Workflow #29

Merged

Conversation

cothan
Copy link
Contributor

@cothan cothan commented Mar 29, 2024

This PR should be merged after #24 .

This PR add:

  • gen_KAT.c: A way to deterministic generate KAT file.
  • gen_NISTKAT.c: Generate NISTKAT file to compare with official/upstream ML-KEM KATs.
  • checksum.sh: A bash script run executable and compare output of executable with an hash.
  • Github workflow to test checksum of MLKEM512, 768, 1024 from 10,000 iterations.
  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)

@cothan cothan requested a review from a team March 29, 2024 04:17
@cothan cothan marked this pull request as ready for review March 29, 2024 04:17
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase on top of main and resolve any conflicts.

Do you think it's possible to switch to actual NISTKATs?
Unfortunately, the hashes in the kyber repo are not up to date, but you can generate them yourself from the code in https://github.com/pq-crystals/kyber/tree/standard.
You would have to re-implement it along the lines of https://github.com/PQClean/PQClean/blob/master/test/crypto_sign/nistkat.c - which should not be too hard. This will be much easier to match against the official testvectors that will be published by NIST togethr

MLKEM512:
4b88ac7643ff60209af1175e025f354272e88df827a0ce1c056e403629b88e04
MLKEM768:
21b4a1e1ea34a13c26a9da5eeb9325afb5ca11596ca6f3704c3f2637e3ea7524
MLKEM1024:
6471398b0a728ee1ef39e93bb89b526fbf59587a3662edadbcfc6c88a512cd71

.github/actions/setup-nix~ef376a0 (refactor build.yml) Outdated Show resolved Hide resolved
@cothan cothan force-pushed the Add-checksum-KAT-to-Github-Action branch 2 times, most recently from b56bf74 to ec3d9cd Compare April 2, 2024 12:58
@cothan
Copy link
Contributor Author

cothan commented Apr 2, 2024

Good idea. I will add NISTKAT test. I see it's no harm to add independent test as long as our checksum match NISTKAT.
The reason I didn't add it as first place was the AES part to generate random data, which can be done with FIPS202 (already included in XOF of MLKEM).
I want to separate the AES part out of the code, perhaps out of the repo so the actual code does not have any dependency with AES, do you have any idea?

We can always fall back to add a common/nistrng.c and a common/README.md say that nistrng.c is only used for NIST KAT testing.

test/gen_KAT.c Show resolved Hide resolved
checksum.sh Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@cothan cothan force-pushed the Add-checksum-KAT-to-Github-Action branch from 4a3b2c3 to 2feba80 Compare April 14, 2024 19:10
cothan added 9 commits April 14, 2024 15:16
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
@cothan cothan force-pushed the Add-checksum-KAT-to-Github-Action branch from b76b05a to 6ba15c6 Compare April 14, 2024 19:16
cothan added 2 commits April 14, 2024 15:40
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
@cothan cothan requested a review from mkannwischer April 14, 2024 19:45
cothan added 3 commits April 14, 2024 18:54
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
test/nistrng/aes.c Outdated Show resolved Hide resolved
test/nistrng/rng.c Outdated Show resolved Hide resolved
cothan and others added 2 commits May 3, 2024 15:51
Co-authored-by: Matthias J. Kannwischer <[email protected]>
Signed-off-by: cothan <[email protected]>
Signed-off-by: Duc Tri Nguyen <[email protected]>
@cothan cothan requested a review from mkannwischer May 3, 2024 23:03
Signed-off-by: Duc Tri Nguyen <[email protected]>
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes. This looks good to me now.

@mkannwischer mkannwischer merged commit f31d9cb into pq-code-package:main May 7, 2024
2 checks passed
mkannwischer added a commit to mkannwischer/mlkem-c-aarch64 that referenced this pull request May 9, 2024
* Add checksum KAT to Github Action

Signed-off-by: Duc Tri Nguyen <[email protected]>

* add make mlkem and make clean to build

Signed-off-by: Duc Tri Nguyen <[email protected]>

* add checksum for test_kyber*

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Add NISTKAT

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Add hashsum for NISTKAT

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Add SPDX header

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Add NISTKAT to Makefile

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Update gitignore

Signed-off-by: Duc Tri Nguyen <[email protected]>

* fix format

Signed-off-by: Duc Tri Nguyen <[email protected]>

* forward output to pipe directly

Signed-off-by: Duc Tri Nguyen <[email protected]>

* extract 1st column

Signed-off-by: Duc Tri Nguyen <[email protected]>

* remove for loop

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Simplify Makefile

Signed-off-by: Duc Tri Nguyen <[email protected]>

* check return code only

Signed-off-by: Duc Tri Nguyen <[email protected]>

* Update .github/workflows/build.yml

Co-authored-by: Matthias J. Kannwischer <[email protected]>
Signed-off-by: cothan <[email protected]>

* replace nistkat

Signed-off-by: Duc Tri Nguyen <[email protected]>

* fix incorrect space

Signed-off-by: Duc Tri Nguyen <[email protected]>

---------

Signed-off-by: Duc Tri Nguyen <[email protected]>
Signed-off-by: cothan <[email protected]>
Co-authored-by: Matthias J. Kannwischer <[email protected]>
@cothan cothan deleted the Add-checksum-KAT-to-Github-Action branch June 4, 2024 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants