Reject MLD_PREHASH_NONE in prehash APIs#1228
Conversation
Signed-off-by: Fredrik Dahlgren <fredrik.dahlgren@trailofbits.com>
Signed-off-by: Fredrik Dahlgren <fredrik.dahlgren@trailofbits.com>
CBMC Results (ML-DSA-65, REDUCE-RAM)Full Results (203 proofs)
|
CBMC Results (ML-DSA-44, REDUCE-RAM)Full Results (203 proofs)
|
CBMC Results (ML-DSA-87, REDUCE-RAM)Full Results (203 proofs)
|
CBMC Results (ML-DSA-65)Full Results (203 proofs)
|
CBMC Results (ML-DSA-44)Full Results (203 proofs)
|
CBMC Results (ML-DSA-87)Full Results (203 proofs)
|
| MLD_CT_TESTING_DECLASSIFY(&rc, sizeof(rc)); | ||
| MLD_CT_TESTING_DECLASSIFY(&siglen, sizeof(siglen)); |
There was a problem hiding this comment.
I don't those declassifications are necessary? Those values should be public already, I believe.
There was a problem hiding this comment.
No, I missed that during my review. I'll go ahead and remove them.
| CHECK(randombytes(ph1, sizeof(ph1)) == 0); | ||
| MLD_CT_TESTING_SECRET(ph1, sizeof(ph1)); | ||
| CHECK(randombytes(ph2, sizeof(ph2)) == 0); | ||
| MLD_CT_TESTING_SECRET(ph2, sizeof(ph2)); |
There was a problem hiding this comment.
What's the difference between the tests exercising ph1 vs. ph2?
There was a problem hiding this comment.
No difference. I'll go ahead and make this one test for signing and one for verification.
| * MLD_PREHASH_SHA2_512, MLD_PREHASH_SHA2_512_224, MLD_PREHASH_SHA2_512_256, | ||
| * MLD_PREHASH_SHA3_224, MLD_PREHASH_SHA3_256, MLD_PREHASH_SHA3_384, | ||
| * MLD_PREHASH_SHA3_512, MLD_PREHASH_SHAKE_128, MLD_PREHASH_SHAKE_256. | ||
| * MLD_PREHASH_NONE is rejected by this HashML-DSA API. |
There was a problem hiding this comment.
Nit: This is still indented under the Supported hash algorithm constants: block.
Suggestion:
| * MLD_PREHASH_NONE is rejected by this HashML-DSA API. | |
| * Supported hash algorithm constants: | |
| * MLD_PREHASH_SHA2_224, MLD_PREHASH_SHA2_256, MLD_PREHASH_SHA2_384, | |
| * MLD_PREHASH_SHA2_512, MLD_PREHASH_SHA2_512_224, MLD_PREHASH_SHA2_512_256, | |
| * MLD_PREHASH_SHA3_224, MLD_PREHASH_SHA3_256, MLD_PREHASH_SHA3_384, | |
| * MLD_PREHASH_SHA3_512, MLD_PREHASH_SHAKE_128, MLD_PREHASH_SHAKE_256. | |
| * | |
| * MLD_PREHASH_NONE is rejected by this API. |
hanno-becker
left a comment
There was a problem hiding this comment.
Thank you @fegge, I support this change.
Minor nit regarding the documentation, and I believe the test can be simplified -- see comments.
Signed-off-by: Fredrik Dahlgren <fredrik.dahlgren@trailofbits.com>
Summary
MLD_PREHASH_NONEin the unstable pre-hash signing and verification APIs.MLD_PREHASH_NONEis not supported by these HashML-DSA entry points.Validation
clang-format -i mldsa/src/sign.c mldsa/src/sign.h mldsa/mldsa_native.h test/src/test_mldsa.c: passedgit diff --check -- mldsa/src/sign.c mldsa/src/sign.h mldsa/mldsa_native.h test/src/test_mldsa.c: passedmake run_func -j4: passed./scripts/formatbecausenixpkgs-fmtis not installed in this local environment./scripts/lintbecauseshfmtis not installed in this local environmentFixes #1215
This work was completed by Trail of Bits as part of the Patch The Planet project in collaboration with OpenAI. The issue was identified primarily by the Codex coding agent, and manually reviewed before submission.