Improve SHA3 test by mdcornu · Pull Request #1 · mdcornu/liboqs

mdcornu · 2026-05-27T15:58:57Z

Add extra test vector to SHA3 test and remove duplicated per test message vector definitions.

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.)
Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged. Also, make sure to update the list of algorithms in the continuous benchmarking files: .github/workflows/kem-bench.yml and sig-bench.yml)

Copilot

Pull request overview

This PR improves SHA3/SHAKE test coverage by deduplicating shared KAT input messages and adding an additional SHA3 test vector (896-bit / 112-byte message). It also fixes a correctness/safety bug in the AVX512VL SHA3 incremental absorb path when completing a partial block and permuting.

Changes:

Refactor tests/test_sha3.c to share common KAT input messages across tests and add SHA3-256/384/512 KAT coverage for a 112-byte (896-bit) message, including additional incremental “chunked absorb” checks.
Fix AVX512VL incremental absorb logic in SHA3-AVX512VL.S to correctly decrement the remaining message length before the partial-fill-and-permute step (preventing double-absorb and potential out-of-bounds reads).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/test_sha3.c	Centralizes shared SHA3/SHAKE test messages and adds new msg896 KATs plus chunked incremental-absorb test cases.
src/common/sha3/avx512vl_low/SHA3-AVX512VL.S	Corrects the partial-fill-and-permute absorb path to maintain the correct remaining-length accounting (avoids double-absorb / OOB).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Move msg0, msg24, msg448, msg896, and msg1600 from local declarations repeated in each of the seven test functions into static const globals. This removes 140 lines of duplicated initialisers and makes it easier to extend the test suite without repeating the same byte arrays across sha3_256, sha3_384, sha3_512, shake_128, shake_256, shake_128_x4 and shake_256_x4. Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

mdcornu · 2026-05-28T16:49:54Z

@tkanteck Can you have a scan of this? For some reason I can't add you as a reviewer.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

coveralls · 2026-05-28T19:25:14Z

Coverage Report for CI Build 26588835144

Warning

No base build found for commit f986aea on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 82.181%

Details

Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	252643
Covered Lines:	207625
Line Coverage:	82.18%
Coverage Strength:	8216708.87 hits per line

💛 - Coveralls

…idation Add 896-bit NIST vector and cross-validate inc API against one-shot API. Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

Cross-validate SHAKE inc API against one-shot using absorb sweep (1..168) and squeeze sweep (1..512), covering all code paths and rate-boundary transitions. Add distinct x4 lane inputs to the four-way tests. Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

tkanteck

cool - just thinking it could potentially be a bit improved

tkanteck · 2026-05-29T12:44:51Z

+
+	/* Verify ctx_reset: absorb msg24, finalize, then reset and re-absorb msg24;
+	   result must equal a fresh inc_init hash of msg24. */
+	uint8_t hash_fresh[48];


same as above, there is potentially to make the code more compact and test bit more

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

mdcornu force-pushed the tests/improve_sha3_test branch from 86f381c to 51c0822 Compare May 27, 2026 16:00

mdcornu requested a review from Copilot May 27, 2026 16:00

Copilot started reviewing on behalf of mdcornu May 27, 2026 16:01 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

Comment thread tests/test_sha3.c Outdated

mdcornu force-pushed the tests/improve_sha3_test branch from 51c0822 to f796c28 Compare May 28, 2026 13:54

mdcornu requested a review from Copilot May 28, 2026 13:54

Copilot started reviewing on behalf of mdcornu May 28, 2026 13:55 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread tests/test_sha3.c Outdated

Comment thread tests/test_sha3.c Outdated

Comment thread tests/test_sha3.c Outdated

Comment thread tests/test_sha3.c Outdated

mdcornu force-pushed the tests/improve_sha3_test branch 2 times, most recently from 34038c8 to 96ecb66 Compare May 28, 2026 15:43

mdcornu requested a review from Copilot May 28, 2026 16:07

Copilot started reviewing on behalf of mdcornu May 28, 2026 16:07 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread tests/test_sha3.c Outdated

Comment thread tests/test_sha3.c Outdated

mdcornu force-pushed the tests/improve_sha3_test branch from c7995ad to 2ffb62f Compare May 28, 2026 16:47

mdcornu requested a review from Copilot May 28, 2026 16:48

Copilot started reviewing on behalf of mdcornu May 28, 2026 16:48 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

mdcornu added 3 commits May 29, 2026 02:18

tests: add SHA3-256/384/512 KAT vectors and incremental-API cross-val…

1a25053

…idation Add 896-bit NIST vector and cross-validate inc API against one-shot API. Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

tests: remove hardcoded values where possible in SHA3 test module

e59598d

Signed-off-by: Cornu, Marcel D <marcel.d.cornu@intel.com>

tkanteck reviewed May 29, 2026

View reviewed changes

mdcornu force-pushed the tests/improve_sha3_test branch from 2ffb62f to bb3b9aa Compare May 29, 2026 14:58

mdcornu requested a review from Copilot May 29, 2026 15:41

Copilot AI reviewed May 29, 2026

View reviewed changes

mdcornu force-pushed the tests/improve_sha3_test branch from bb3b9aa to a19c60e Compare May 29, 2026 16:10

mdcornu requested a review from Copilot May 29, 2026 16:10

Copilot AI reviewed May 29, 2026

View reviewed changes

mdcornu force-pushed the tests/improve_sha3_test branch from a19c60e to e59598d Compare May 29, 2026 16:20

pablodelara requested a review from Copilot May 29, 2026 16:24

Copilot started reviewing on behalf of pablodelara May 29, 2026 16:25 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Conversation

mdcornu commented May 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mdcornu commented May 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

coveralls commented May 28, 2026

Coverage Report for CI Build 26588835144

Coverage: 82.181%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

tkanteck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tkanteck May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants