Repository navigation
Fix BUFF weak-key verify break and encode-overflow signing bug (0.1.1) - #1
Merged
Merged
Conversation
The verifier only rejected negative q00[0], never a small positive one, so a malformed public key with tiny q00[0] (e.g. constant q00=1 weak keys with preimage (1,0,a,1)) was accepted and broke HAWK's BUFF properties: one zero-signature verified under a constant weak key for essentially ALL messages (message-bound-signature / M-S-UEO break). Confirmed against THIS crate before fixing: the constant weak key + the trivial (salt=0, s1=0) witness verified for 256/256 distinct messages. After the fix: 0/256. Fix adds a KeyNormCheck floor in verify_inner: reject q00[0] < 2080. For an auto-adjoint key q00[0] = ||(f,g)||^2, and honest keygen resamples until ||(f,g)||^2 >= 2080 (HAWK_512_L2LOW in keygen/hawk_keygen.rs, from reference ng_hawk.c:462), so this is exact for HAWK-512 and a no-op for honest keys. 2080 is the HAWK-512 value, NOT the HAWK-256 value 556, which would leave a [556, 2080) gap that no honest key occupies. Measured over 48 honest keys, q00[0] ranged [2080, 2290] -- the floor is exactly tight and costs no margin. verify_inner is the single chokepoint every verification path routes through, so this one check covers HawkPublicKey::verify and all callers. Regression test tests/buff_poc.rs asserts the closed state for every message and every a in Dao's witness table, and pins the boundary (2080 accepts, 2079 rejects, 556 must REJECT so nobody reinstates the HAWK-256 value). Full suite green (225 tests, incl. NIST KATs); honest sign/verify unaffected.
sign_512 returned the signature un-encoded, on the false assumption that an in-bounds s1 always fits the fixed 555-byte wire buffer. The comment said so outright: "Encoding always succeeds when in_bounds (the GR encoder may still overflow in theory, but for HAWK-512 with LIM=512 it fits)." That is wrong. The Golomb-Rice variable section grows with sum(|s1[i]| >> 5), so a signature can be in-bounds per-coefficient (|s1[i]| < LIM = 512) yet overflow in aggregate. encode_gr then returns Err, surfacing only later when a caller serializes the signature. This is not theoretical: a single unlucky signature wedged a live single-validator testnet, failing the block commit with "encode sig: malformed signature: encode_gr s1 overflow". Move the encode inside the retry loop: on overflow, resign with a fresh salt instead of returning an unserializable signature. This restores the invariant that every HawkSignature returned by signing serializes to 555 bytes, and matches the reference C, which restarts on encode_sig failure (hawk_sign.c:1142-1158). No public API change; reuses the existing HAWK_SAMPLER_RETRY_BUDGET. Overflow now also feeds the shared retry budget whose exhaustion raises SamplingFailure. Measured upstream: 0 encode-overflow resigns over 20,000 signings, worst case 1 encode attempt per signature -- far from the 1024 ceiling, so sharing the budget is safe by a wide margin. Tests: - try_encode_attempt unit-tested both branches with a crafted in-bounds-but-overflowing s1. - sign_recovers_from_forced_encode_overflow drives the REAL retry loop through overflow -> resign -> valid+verifying signature via a no-op production / test-only s1_hook seam. Proven discriminating by sabotage: with the fix reverted it fails with "expected a resign (>=2 attempts), got 1". - hooked_signing_with_noop_equals_production pins the seam as byte-for-byte transparent in production. - signed_signatures_always_serialize guards the violated invariant directly. - encode_overflow_boundary pins the unary-sum threshold (42 fits, 45 overflows). Full suite green (231 tests, incl. NIST KAT byte-parity, so the seam is transparent); fmt + clippy clean.
Bump to 0.1.1 for the two fixes in this branch: the BUFF weak-key
KeyNormCheck at verify, and resign-on-Golomb-Rice-encode-overflow at sign.
Both are drop-in with no API change, so a patch bump is the right level.
- Cargo.toml + Cargo.lock: 0.1.0 -> 0.1.1.
- CHANGELOG.md: new, documenting both fixes with the measured evidence
(256/256 -> 0/256 forged verifications; the observed testnet commit stall)
and the scope limit that honest-key EUF/SUF-CMA was never affected.
- SECURITY.md: add a "Fixed in 0.1.1" section, and replace the dead reporting
pointer ("the address in Cargo.toml" -- authors is "PQE Chain Contributors",
no email) with private GitHub Security Advisories plus an upstream pointer
for issues in HAWK itself rather than this port.
- README.md: upgrade note for 0.1.0 users.
Verified: cargo fmt, clippy --all-targets -D warnings, 229 tests
(incl. NIST KATs), doc tests, rustdoc, and the slow-tests feature all clean;
cargo package builds the 0.1.1 tarball standalone. Licenses ship in the
package (LICENSE-MIT, LICENSE-APACHE, and upstream c-reference/LICENSE with
PROVENANCE.md).
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
1 participant
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.
Two correctness fixes, both reproduced against this crate before being fixed, and each covered by a regression test proven to fail without the fix. No API change, so 0.1.1 is a drop-in upgrade from 0.1.0.
1. Weak public keys broke the BUFF properties at verify
Reported by Quang Dao, eprint 2026/1298.
verify_innerrejected only a negativeq00[0], never a small positive one. So a maliciously formed public key with a tinyq00[0](the constantq00 = 1weak-key class, preimage(1,0,a,1)) was accepted, and a single trivial signature (salt = 0,s1 = 0) then verified under it for essentially every message.Measured on this crate before the fix: the witness verified for 256 of 256 distinct messages. After: 0 of 256.
The fix adds a
KeyNormCheckfloor rejectingq00[0] < 2080. For an auto-adjoint keyq00[0] = ||(f,g)||^2, and keygen resamples until that reachesHAWK_512_L2LOW = 2080, so the check is exact for HAWK-512 and a no-op for honest keys. Measured over 48 honest keys,q00[0]spans[2080, 2290]— the floor is exactly tight and costs no honest-key margin.This is the HAWK-512 floor, not the HAWK-256 value 556, which would leave a
[556, 2080)band no honest key occupies. The regression test pins that boundary so nobody reinstates 556.Scope: this does not affect EUF/SUF-CMA security under honestly generated keys. Only the BUFF add-on properties (message-bound signatures, M-S-UEO) over verifier-accepted-but-malformed keys were affected.
2. Signing could return a signature that fails to serialize
sign_512encoded outside its rejection loop, on the assumption that an in-boundss1always fits the fixed 555-byte wire buffer. The old comment stated it outright:That is wrong. The Golomb-Rice variable section grows with
sum(|s1[i]| >> 5), so a signature can be in bounds per coefficient (|s1[i]| < 512) yet overflow in aggregate.encode_grthen returned an error only later, when a caller serialized. Not theoretical: this stalled a live single-validator testnet on block commit withencode sig: malformed signature: encode_gr s1 overflow.Encoding now happens inside the retry loop, and an overflow resigns with a fresh salt. This restores the invariant that every returned
HawkSignatureencodes to 555 bytes, and matches the reference C, which restarts onencode_sigfailure (hawk_sign.c:1142-1158). Overflow shares the existingHAWK_SAMPLER_RETRY_BUDGET; measured upstream at 0 overflow-driven resigns over 20,000 signings, so headroom is ample.Both fixes proven load-bearing
Each was verified by sabotage rather than by the tests merely passing:
q00[0] < 0→ the BUFF test fails at 256/256 accepted.return Ok(sig)→sign_recovers_from_forced_encode_overflowfails withexpected a resign (>=2 attempts), got 1.The resign path is exercised through a test-only
s1_hookseam that forces the overflow deterministically (it is far too rare to hit by chance).hooked_signing_with_noop_equals_productionpins the seam as byte-for-byte transparent in production, and the NIST KATs passing independently confirms it.Release
Bumped to 0.1.1, with a new
CHANGELOG.md.SECURITY.mdgains a "Fixed in 0.1.1" section, and its dead reporting pointer (it referenced an address inCargo.toml, butauthorscarries no email) is replaced with private GitHub Security Advisories.Verification
All CI gates run locally:
cargo fmt,clippy --all-targets -D warnings, 229 tests (234 withslow-tests) including NIST KAT byte-parity, doc tests, rustdoc, andcargo packagebuilding the 0.1.1 tarball standalone.