fix(kv1): use random G1 element for hzero public parameter#46
Conversation
The KV1 KEM `setup` initialised the public parameter `hzero` to the identity point via `G1Affine::default()`. The scheme specification requires `hzero` to be a uniformly random G1 element; using the identity point breaks the intended security assumptions. Generate `hzero` with `rand_g1(rng)` like the other public parameters. Refs #42 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules Dobby 2 — rule-compliance gate (cycle 1/3)
The code change itself is correct and verified: making hzero a uniformly random G1 element matches the KV1 spec, and Review Dobby 2 confirmed no correctness or serialization regression (all 21 tests pass, incl. 4 KV1 tests). One rule-compliance gap remains:
- Missing regression test (
tests-required-on-fixes): this is a security fix but adds no test. The standing expectation (Ruben, cryptify#99) is that every fix ships with a test — and for a security fix, a regression test that would have failed against the pre-fix code. That's cheap here: afterKV1::setup, assert the generated public key'shzerois not the identity point (G1Affine::default()), which the old code always produced.
Verdict is request-changes — posted as
COMMENTonly because GitHub blocks request-changes on Dobby's own PR. Treat this as blocking.
Regression test for the hzero public parameter fix: setup must sample hzero as a random G1 element rather than G1Affine::default(). Fails on the old identity-point code, passes now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules Dobby 2 — rule-compliance gate (cycle 2/3): SIGN-OFF
Verdict is approve — posted as
COMMENTonly because GitHub blocks self-approval on Dobby's own PR. Treat this as a clean sign-off, not a withheld approval.
The cycle-1 finding (tests-required-on-fixes — missing regression test) is resolved: this revision adds setup_hzero_is_not_the_identity_point, which fails against the old identity-point code and passes with the fix. Verified locally on the PR head:
cargo fmt --check— cleancargo test --all-features— 22 passed (incl. the new regression test, KV1 encaps/decaps, and serialize round-trip)cargo test --features kv1— 6 passed
Review Dobby 2 reported 0 correctness/serialization findings. One trailing rule gap (pr-close-issue-keywords — body used Refs #42, which does not auto-close the issue on merge) was fixed inline: the body now reads Closes #42, and the stale test counts (5→6, 21→22) were corrected.
No blocking issues remain. Flipping the PR to ready-for-review.
Summary
Fixes a cryptographic correctness issue in the KV1 KEM scheme's key
setupfunction.The public parameter
hzerowas initialised to the identity point viaG1Affine::default(). The KV1 scheme specification requireshzeroto be a uniformly random G1 element; using the identity point breaks the intended security assumptions.Change
In
src/kem/kiltz_vahlis_one.rs,setup:This matches how the other public parameters (
u,alpha, and thehhash parameters) are already generated. A regression test (setup_hzero_is_not_the_identity_point) pins the new behaviour; it fails against the old identity-point code.Testing
cargo test --features kv1— 6 passedcargo test --all-features— 22 passedcargo build --features kv1— cleanNotes
This changes the distribution of generated KV1 public keys, but the public API, serialization format, and byte sizes are unchanged. Existing serialized keys remain deserializable; only freshly generated keys differ (as intended).
Closes #42
Security advisory: GHSA-wvwc-w2wm-hx95