Make ML-DSA SHAKE finalization idempotent#1243
Open
tob-joe wants to merge 2 commits into
Open
Conversation
Contributor
CBMC Results (ML-DSA-65, REDUCE-RAM)Full Results (205 proofs)
|
Contributor
CBMC Results (ML-DSA-65)Full Results (205 proofs)
|
Contributor
CBMC Results (ML-DSA-87)Full Results (205 proofs)
|
Contributor
CBMC Results (ML-DSA-87, REDUCE-RAM)Full Results (205 proofs)
|
Contributor
CBMC Results (ML-DSA-44)Full Results (205 proofs)
|
Contributor
CBMC Results (ML-DSA-44, REDUCE-RAM)Full Results (205 proofs)
|
Contributor
There was a problem hiding this comment.
Thanks @tob-joe. Could you elaborate what is the motivation for this change?
This is an internal function that we are not exposing to consumers.
I'm not convinced that we need to make internal functions misuse resistant.
4a93f59 to
157f66e
Compare
Skip Keccak finalization when SHAKE128 or SHAKE256 state is already finalized, tighten the internal finalize precondition, and replace arbitrary XOF proof-contract bounds with MLD_MAX_BUFFER_SIZE. Add regression test coverage comparing one-finalize and two-finalize SHAKE128/SHAKE256 output across boundary absorb lengths. Verification: - pre-fix targeted test failed with ERROR (test/src/test_unit.c,113) - make -j1 run_unit_44 - scripts/autogen Co-authored-by: Codex <[email protected]> Signed-off-by: Joe Doyle <[email protected]>
Keep the idempotent SHAKE finalization change, but restore the prior small CBMC contract bound on the four-way squeeze helpers. The broader symbolic nblocks bound is not needed for this PR and causes the keccak_squeezeblocks_x4 proof to time out across the CBMC matrix. Verification: - ./scripts/tests cbmc -kl 44 -p keccak_squeezeblocks_x4 --per-proof-timeout 600 -f -v - ./scripts/tests cbmc -kl 44 -p keccak_squeezeblocks_x4 --per-proof-timeout 600 -f -v --reduce-ram - lint Co-authored-by: Codex <[email protected]> Signed-off-by: Joe Doyle <[email protected]>
157f66e to
1913190
Compare
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
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.
Summary
Rationale
This is narrow misuse resistance for the built-in incremental FIPS202 implementation. The SHAKE helpers are internal, but they have external linkage by default and sit behind helper boundaries where names such as
*_absorb_oncealready include finalization before later squeezing. A maintainer adding an explicit finalize at such a boundary should not silently corrupt the state by applying the Keccak delimiter twice.This does not change the intended absorb/finalize/squeeze lifecycle, does not make absorb-after-finalize valid, and does not claim to cover finalizing again after squeezing has begun.
Testing
make clean run_unitnix develop --command python3 scripts/autogen --dry-runCo-authored-by: Codex [email protected]
Signed-off-by: Joe Doyle [email protected]
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.