main
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kgothatso Ngako
|
3c962b00cc |
frost_enrollment: fix the example's stale n and failure-path hygiene
The last of the review findings, plus the comment and structure fixes it
listed.
The example's repair run used the pre-enrollment participant count.
Two blocks earlier the example teaches that every participant must
update its record of n from 3 to 4 after an enrollment, and the
signing session duly uses N_PARTICIPANTS_AFTER. Then enroll() -- which
hard-coded N_PARTICIPANTS -- ran the repair at n = 3. It worked only
because the Lagrange math never involves n and every party in this
single-process demo passed the same stale value.
In a real post-enrollment repair it would not. n is bound into the
parameters hash, so helpers feeding their updated n = 4 into
shares_gen while the requester feeds n = 3 abort round 1.2 with no
visible cause. enroll() now takes n_participants as a parameter, the
repair passes N_PARTICIPANTS_AFTER, and both the function's contract
comment and the repair call site say why. The repaired share is still
byte-identical to the original, which is the point: n changes the
hash, not the arithmetic.
The example leaked secrets on its failure paths.
enroll() erased the delta and sigma buffers only on success; four
early returns left them live. sign_and_verify() returned from three
places without erasing already-generated secnonces. Both now route
every exit through a cleanup block. This example is otherwise more
careful about erasure than its siblings, so the asymmetry was exactly
what a reader copying it would carry into production -- on the fault
paths where hygiene matters most.
The double-wipe of session_secrand is gone with it: shares_gen and
nonce_gen both wipe the seed on every path, and doing it again read
as uncertainty about the contract. The comment now states the
contract instead. The fill_random failure path does erase, since
nothing else has touched the buffer there.
The example's mismatch message asserted a cause it cannot know.
It printed "Helper %u disagrees about the enrollment parameters" for
what may equally be a corrupted share, per the previous commit's
finding. It now says the helper "contributed a share this helper
cannot use", with a comment noting that share_agg does not
distinguish the two causes so neither can the message.
Comment and structure fixes, all noted in the review:
- The vector generator claimed case 4 was "the only case whose DERIVED
public share has odd Y". It is not -- cases 1, 2 and 4 are odd and
case 3 is even. The comment existed to justify a coverage choice and
misinformed; both parity comments now describe the set accurately and
say they document it rather than constrain it. Regenerating vectors.h
still reproduces it byte for byte.
- The secp256k1_frost_sort_ids declaration in frost/session.h no longer
duplicates the definition's doc comment, which was two copies to keep
in sync. It says what the function is for and points at the
definition for the contract.
- The t >= 2 rationale was stated in full in three places. The impl now
states the conclusion and names frost_enrollment.md as the single
place to edit if the policy moves.
- The ctime_tests comments read ambiguously ("the parameters hash is
public, the delta values are not" against a header calling deltas
secret), and computing direct_hash without asserting anything invited
a "forgotten assertion" reading. Both are now explicit.
- The example moves next to frost_example in Makefile.am rather than
after iceberg, matching the FROST-stack grouping used in
configure.ac, ci.sh, ci.yml and README.
- frost_enrollment.md now distinguishes what is unstable (the C API)
from what is frozen (the wire-visible encodings), which the two
statements together previously left easy to conflate.
Not fixed, deliberately, and now recorded where the tree can see it: the
plan called for a CHANGELOG.md entry. That file states in its first two
lines that it is upstream libsecp256k1's changelog and not this fork's,
and none of frost, chilldkg, iceberg or prefractal has an entry. Adding
the first one is a decision about all five modules, not this one. The
README link is the fork's actual convention for announcing a module and
is in place.
Verification: autotools builds warning-free and `make check` is 12/12
including the example; ctime_tests is clean under valgrind; `make dist`
carries all nine frost_enrollment files; CMake with examples builds
warning-free and ctest is 542/542; the example source is clean under
gcc -std=c89 -pedantic -Wall -Wextra; regenerating vectors.h reproduces
it byte for byte.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
5155ee03f7 |
frost_enrollment: add the example program
Fifth of six commits. examples/frost_enrollment.c runs a 2-of-3 group
through an enrollment to 2-of-4, signs with the new participant, and then
repairs a lost share -- all roles in one process, following
examples/frost.c's structure.
The example exists mostly to demonstrate two things the API cannot
enforce and that a reader would otherwise have to reconstruct from the
documentation.
First, the verification flow, in the order that makes it non-circular:
1. obtain thresh_pk from a source authenticated INDEPENDENTLY of the
helpers (here, the dealer step, commented as the stand-in);
2. validate the helpers' public shares against it with
secp256k1_frost_threshold_info_validate;
3. derive the expected public share from those validated shares;
4. only then run round 2, passing the same authenticated thresh_pk.
Skip step 1 or 2 and every check in round 2 still passes -- on a share
from whatever polynomial t colluding helpers chose to present. The
example says so at the point where it would be tempting to skip them.
Second, the authorization gap. There is no authorization step in the
protocol: anyone who convinces t helpers to run it receives a valid
share, and in repair mode that is an existing participant's actual
share. The precondition sits in the comment on enroll(), where a reader
copying the function will see it, and again at the repair call site,
which is where it bites hardest.
Beyond that the example is a working reference for the parts that are
fiddly to get right from the header alone: the transposition between
round 1.1's output buffers and round 1.2's input buffer (helper j
collects entry j of every helper's buffer), the opposite own-slot
conventions of the two round-1.2 buffers, the n -> n+1 bookkeeping with
threshold_info_validate over the extended table, and the fact that the
resulting signature verifies against the group's ORIGINAL threshold
public key, since enrollment changes neither the polynomial nor any
existing share.
The repair half asserts byte equality with the original secret share and
the original public share, so a regression there fails the example rather
than passing quietly.
Wired into both build systems next to the iceberg example: Makefile.am
(noinst_PROGRAMS and TESTS under ENABLE_MODULE_FROST_ENROLLMENT) and
examples/CMakeLists.txt. The .gitignore entry landed with the Phase 1
scaffolding.
Verification. Autotools: builds warning-free and `make check` reports
12/12 PASS including frost_enrollment_example; five consecutive runs exit
0 (the key material is freshly random each time, so this exercises both
threshold-key parities in practice). CMake: with
-DSECP256K1_BUILD_EXAMPLES=ON, ctest runs all nine examples and
secp256k1.example.frost_enrollment passes; the full ctest suite is
523/523 with frost, chilldkg, iceberg, prefractal and frost-enrollment
all enabled. The source is clean under gcc -std=c89 -pedantic -Wall
-Wextra.
Note for anyone reproducing this: examples are OFF by default in both
build systems (--enable-examples for autotools,
-DSECP256K1_BUILD_EXAMPLES=ON for CMake), so a plain build will not
compile this file at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|