diff --git a/Makefile.am b/Makefile.am index 4a0c4f18..17834053 100644 --- a/Makefile.am +++ b/Makefile.am @@ -229,6 +229,17 @@ frost_example_LDFLAGS += -lbcrypt endif TESTS += frost_example endif +if ENABLE_MODULE_FROST_ENROLLMENT +noinst_PROGRAMS += frost_enrollment_example +frost_enrollment_example_SOURCES = examples/frost_enrollment.c +frost_enrollment_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC +frost_enrollment_example_LDADD = libsecp256k1.la +frost_enrollment_example_LDFLAGS = -static +if BUILD_WINDOWS +frost_enrollment_example_LDFLAGS += -lbcrypt +endif +TESTS += frost_enrollment_example +endif if ENABLE_MODULE_CHILLDKG noinst_PROGRAMS += chilldkg_example chilldkg_example_SOURCES = examples/chilldkg.c @@ -251,17 +262,6 @@ iceberg_example_LDFLAGS += -lbcrypt endif TESTS += iceberg_example endif -if ENABLE_MODULE_FROST_ENROLLMENT -noinst_PROGRAMS += frost_enrollment_example -frost_enrollment_example_SOURCES = examples/frost_enrollment.c -frost_enrollment_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC -frost_enrollment_example_LDADD = libsecp256k1.la -frost_enrollment_example_LDFLAGS = -static -if BUILD_WINDOWS -frost_enrollment_example_LDFLAGS += -lbcrypt -endif -TESTS += frost_enrollment_example -endif endif ### Precomputed tables diff --git a/examples/frost_enrollment.c b/examples/frost_enrollment.c index 7edac885..fdf5d73f 100644 --- a/examples/frost_enrollment.c +++ b/examples/frost_enrollment.c @@ -98,16 +98,28 @@ static int trusted_dealer_keygen(const secp256k1_context* ctx, unsigned char *th * secret share is written to `new_secshare` and its public counterpart to * `new_pubshare`. * + * `n_participants` must be the group size all parties CURRENTLY agree on: it + * is bound into the parameters hash, so a helper using a stale n and one + * using the updated n abort round 1.2 against each other. That is why this is + * a parameter and not the N_PARTICIPANTS constant -- the repair below runs + * after the enrollment, when the group has already grown to + * N_PARTICIPANTS_AFTER. + * * PRECONDITION THE LIBRARY CANNOT ENFORCE: the helpers must already have * agreed, out of band, that this party is entitled to a share at `new_id`. * The protocol has no authorization step of its own: anyone who convinces t * helpers to run it walks away with a valid share, and in repair mode that is * an existing participant's actual share. Authenticated channels establish who * is speaking, not that the group approved the request. */ -static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, secp256k1_pubkey *new_pubshare, struct helper *helpers, const uint32_t *ids, const secp256k1_pubkey *helper_pubshares, const secp256k1_pubkey *thresh_pk, uint32_t new_id) { +static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, secp256k1_pubkey *new_pubshare, struct helper *helpers, const uint32_t *ids, const secp256k1_pubkey *helper_pubshares, const secp256k1_pubkey *thresh_pk, uint32_t new_id, size_t n_participants) { unsigned char sigmas[N_HELPERS * 32]; + unsigned char all_shares[N_HELPERS * 32]; unsigned char enrollee_params_hash[32]; int i, j; + int ret = 0; + + memset(sigmas, 0, sizeof(sigmas)); + memset(all_shares, 0, sizeof(all_shares)); /* --- Round 1.1 ------------------------------------------------------ * Every helper splits its Lagrange-weighted share into one additive share @@ -115,16 +127,19 @@ static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, sec for (i = 0; i < N_HELPERS; i++) { unsigned char session_secrand[32]; - /* Fresh randomness for every run. Reusing it across runs leaks share - * information. shares_gen wipes it before returning. */ + /* Fresh randomness for every run: reusing it across runs leaks share + * information. No secure_erase is needed here -- shares_gen wipes the + * seed itself, on success and on failure alike, so that a failed call + * cannot be retried on the same randomness. */ if (!fill_random(session_secrand, sizeof(session_secrand))) { printf("Failed to generate randomness\n"); - return 0; + secure_erase(session_secrand, sizeof(session_secrand)); + goto cleanup; } - if (!secp256k1_frost_enrollment_shares_gen(ctx, helpers[i].shares, helpers[i].params_hash, session_secrand, helpers[i].secshare, thresh_pk, ids, N_HELPERS, helpers[i].id, new_id, N_PARTICIPANTS, THRESHOLD)) { - return 0; + if (!secp256k1_frost_enrollment_shares_gen(ctx, helpers[i].shares, helpers[i].params_hash, session_secrand, helpers[i].secshare, thresh_pk, ids, N_HELPERS, helpers[i].id, new_id, n_participants, THRESHOLD)) { + /* shares_gen wipes the seed on every path, including this one. */ + goto cleanup; } - secure_erase(session_secrand, sizeof(session_secrand)); } /* --- Round 1.2 ------------------------------------------------------ @@ -132,7 +147,6 @@ static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, sec * confidential and authenticated channel, together with that helper's * parameters hash. It recomputes the hash itself and compares. */ for (j = 0; j < N_HELPERS; j++) { - unsigned char all_shares[N_HELPERS * 32]; unsigned char received_hashes[N_HELPERS * 32]; uint32_t mismatch_id; @@ -149,13 +163,15 @@ static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, sec memcpy(&received_hashes[32 * i], helpers[i].params_hash, 32); } } - if (!secp256k1_frost_enrollment_share_agg(ctx, helpers[j].sigma, &mismatch_id, all_shares, received_hashes, thresh_pk, ids, N_HELPERS, helpers[j].id, new_id, N_PARTICIPANTS, THRESHOLD)) { + if (!secp256k1_frost_enrollment_share_agg(ctx, helpers[j].sigma, &mismatch_id, all_shares, received_hashes, thresh_pk, ids, N_HELPERS, helpers[j].id, new_id, n_participants, THRESHOLD)) { if (mismatch_id != UINT32_MAX) { - printf("\nHelper %u disagrees about the enrollment parameters\n", mismatch_id); + /* Either that helper ran round 1.1 on different parameters, + * or its share did not survive transit. share_agg does not + * distinguish the two, so neither can this message. */ + printf("\nHelper %u contributed a share this helper cannot use\n", mismatch_id); } - return 0; + goto cleanup; } - secure_erase(all_shares, sizeof(all_shares)); } /* --- Round 2 -------------------------------------------------------- @@ -173,18 +189,25 @@ static int enroll(const secp256k1_context* ctx, unsigned char *new_secshare, sec } memcpy(enrollee_params_hash, helpers[0].params_hash, 32); - if (!secp256k1_frost_enrollment_pubshare_derive(ctx, new_pubshare, helper_pubshares, ids, N_HELPERS, new_id, N_PARTICIPANTS, THRESHOLD)) { - return 0; + if (!secp256k1_frost_enrollment_pubshare_derive(ctx, new_pubshare, helper_pubshares, ids, N_HELPERS, new_id, n_participants, THRESHOLD)) { + goto cleanup; } - if (!secp256k1_frost_enrollment_secshare_gen(ctx, new_secshare, sigmas, thresh_pk, ids, N_HELPERS, new_id, N_PARTICIPANTS, THRESHOLD, enrollee_params_hash, new_pubshare)) { - return 0; + if (!secp256k1_frost_enrollment_secshare_gen(ctx, new_secshare, sigmas, thresh_pk, ids, N_HELPERS, new_id, n_participants, THRESHOLD, enrollee_params_hash, new_pubshare)) { + goto cleanup; } + ret = 1; + +cleanup: + /* Every exit runs this, not just the successful one. The delta and sigma + * values are additive shares of real secret shares, and the paths where + * hygiene matters most are exactly the ones a protocol fault takes. */ secure_erase(sigmas, sizeof(sigmas)); + secure_erase(all_shares, sizeof(all_shares)); for (i = 0; i < N_HELPERS; i++) { secure_erase(helpers[i].shares, sizeof(helpers[i].shares)); secure_erase(helpers[i].sigma, sizeof(helpers[i].sigma)); } - return 1; + return ret; } /* Produce a BIP340 signature with the given signer set and verify it against @@ -206,34 +229,39 @@ static int sign_and_verify(const secp256k1_context* ctx, const uint32_t *ids, un int i; int ret = 0; + /* Zeroed up front so the cleanup below can erase them unconditionally, + * whichever failure path got there. */ + memset(secnonces, 0, sizeof(secnonces)); + if (!secp256k1_frost_tweak_cache_init(ctx, &cache, thresh_pk)) { - return 0; + goto cleanup; } /* No tweaks are applied here, so the "tweaked" key is the threshold public * key itself in its x-only encoding. */ if (!secp256k1_frost_tweaked_pubkey_get(ctx, &tweaked_pk, &cache) || !secp256k1_xonly_pubkey_serialize(ctx, tweaked_pk32, &tweaked_pk)) { - return 0; + goto cleanup; } for (i = 0; i < N_SIGNERS; i++) { unsigned char session_secrand[32]; if (!fill_random(session_secrand, sizeof(session_secrand))) { printf("Failed to generate randomness\n"); - return 0; + secure_erase(session_secrand, sizeof(session_secrand)); + goto cleanup; } + /* nonce_gen wipes the seed itself, on every path. */ if (!secp256k1_frost_nonce_gen(ctx, &secnonces[i], &pubnonces[i], session_secrand, secshares[i], &pubshares[i], tweaked_pk32, msg, msglen, NULL, 0)) { - return 0; + goto cleanup; } - secure_erase(session_secrand, sizeof(session_secrand)); pubnonce_ptrs[i] = &pubnonces[i]; partial_sig_ptrs[i] = &partial_sigs[i]; } if (!secp256k1_frost_nonce_agg(ctx, &aggnonce, NULL, pubnonce_ptrs, N_SIGNERS)) { - return 0; + goto cleanup; } if (!secp256k1_frost_session_init(ctx, &session, &aggnonce, ids, pubshares, N_SIGNERS, n_participants, THRESHOLD, &cache, msg, msglen)) { - return 0; + goto cleanup; } for (i = 0; i < N_SIGNERS; i++) { if (!secp256k1_frost_sign(ctx, &partial_sigs[i], &secnonces[i], secshares[i], &session, ids, pubshares, N_SIGNERS, ids[i])) { @@ -249,6 +277,8 @@ static int sign_and_verify(const secp256k1_context* ctx, const uint32_t *ids, un ret = secp256k1_schnorrsig_verify(ctx, sig, msg, msglen, &tweaked_pk); cleanup: + /* frost_sign wipes a secnonce it consumed, but a failure before or during + * the signing loop can leave others live. */ for (i = 0; i < N_SIGNERS; i++) { secure_erase(&secnonces[i], sizeof(secnonces[i])); } @@ -333,7 +363,7 @@ int main(void) { } printf("Enrolling participant %d with %d helpers...", NEW_ID, N_HELPERS); fflush(stdout); - if (!enroll(ctx, new_secshare, &new_pubshare, helpers, helper_ids, helper_pubshares, &thresh_pk, NEW_ID)) { + if (!enroll(ctx, new_secshare, &new_pubshare, helpers, helper_ids, helper_pubshares, &thresh_pk, NEW_ID, N_PARTICIPANTS)) { printf("FAILED\n"); return EXIT_FAILURE; } @@ -376,10 +406,16 @@ int main(void) { /* Repair: participant 1 lost its share. The same three rounds at * new_id = 1 reproduce it exactly -- the share is a fixed value, f(x_1), - * not a fresh random one. Note that n does NOT change here, and that this - * is the mode where the missing authorization step bites hardest: whoever - * convinces the helpers to run it receives participant 1's actual share. - */ + * not a fresh random one. + * + * Note that n is N_PARTICIPANTS_AFTER here, not N_PARTICIPANTS: the group + * grew above, and n is bound into the parameters hash, so a party still + * using the pre-enrollment value would abort round 1.2 against the + * others. Repair itself does not change n. + * + * This is also the mode where the missing authorization step bites + * hardest: whoever convinces the helpers to run it receives participant + * 1's actual share. */ for (i = 0; i < N_HELPERS; i++) { repair_helpers[i].id = repair_ids[i]; memcpy(repair_helpers[i].secshare, &secshares[32 * repair_ids[i]], 32); @@ -387,7 +423,7 @@ int main(void) { } printf("Repairing participant 1's lost share..."); fflush(stdout); - if (!enroll(ctx, repaired_secshare, &repaired_pubshare, repair_helpers, repair_ids, repair_pubshares, &thresh_pk, 1)) { + if (!enroll(ctx, repaired_secshare, &repaired_pubshare, repair_helpers, repair_ids, repair_pubshares, &thresh_pk, 1, N_PARTICIPANTS_AFTER)) { printf("FAILED\n"); return EXIT_FAILURE; } diff --git a/src/ctime_tests.c b/src/ctime_tests.c index 30bb2e01..bb312464 100644 --- a/src/ctime_tests.c +++ b/src/ctime_tests.c @@ -735,12 +735,15 @@ static void run_tests(secp256k1_context *ctx, unsigned char *key) { SECP256K1_CHECKMEM_DEFINE(pubshares, sizeof(pubshares)); /* The parameters hash and the public share at the target identifier - * are functions of public data alone. */ + * are functions of public data alone, so these calls take no secret + * input. Their outputs are not compared against anything here: this + * is a constant-time harness, and correctness is tests_impl.h's job. */ CHECK(secp256k1_frost_enrollment_params_hash(ctx, direct_hash, &thresh_pk, fe_ids, 2, 3, 3, 2) == 1); CHECK(secp256k1_frost_enrollment_pubshare_derive(ctx, &new_pubshare, pubshares, fe_ids, 2, 3, 3, 2) == 1); /* Round 1.1. The seed and the secret share are secret; the parameters - * hash is public, the delta values are not. */ + * hash is public and is marked so, while the delta values stay + * secret and are carried into round 1.2 undefined. */ for (i = 0; i < 2; i++) { SECP256K1_CHECKMEM_UNDEFINE(session_secrand[i], 32); SECP256K1_CHECKMEM_UNDEFINE(&secshares[32 * i], 32); diff --git a/src/modules/frost/session.h b/src/modules/frost/session.h index 7c560a3c..606c62d4 100644 --- a/src/modules/frost/session.h +++ b/src/modules/frost/session.h @@ -31,11 +31,10 @@ typedef struct { int g_times_gacc_parity; } secp256k1_frost_session_internal; -/* Sorts n_ids identifiers from ids into out (which must have room for - * n_ids entries and must not alias ids) in ascending order. Used to - * canonicalize a signer set before hashing it, so that parties holding the - * same set in different orders agree on the digest. Requires n_ids <= - * SECP256K1_FROST_MAX_PARTICIPANTS. */ +/* Canonicalizes a signer set by sorting it, so that parties holding the same + * set in different orders agree on any digest taken over it. Declared here so + * that other modules can reach it through an interface rather than through + * translation-unit ordering; see session_impl.h for the contract. */ static void secp256k1_frost_sort_ids(uint32_t *out, const uint32_t *ids, size_t n_ids); /* Saves the two secret scalars k[0], k[1] into a secnonce. */ diff --git a/src/modules/frost_enrollment/enrollment_impl.h b/src/modules/frost_enrollment/enrollment_impl.h index 55168229..6339726a 100644 --- a/src/modules/frost_enrollment/enrollment_impl.h +++ b/src/modules/frost_enrollment/enrollment_impl.h @@ -46,11 +46,10 @@ static void secp256k1_frost_enrollment_sha256_tagged_share_split(const secp256k1 * allowed. * * threshold >= 2 is a deliberate divergence from the frost module, which - * accepts threshold >= 1. Because this API permits any threshold <= n_ids, - * t = 1 would permit u = 1, and at u = 1 the additive split of shares_gen - * degenerates to a single share -- the lone helper would send the unsplit - * v_1, which at t = 1 is the whole group secret. Requiring t >= 2 forces - * u >= 2, which is what makes the split non-degenerate. + * accepts threshold >= 1: it is what forces u >= 2 and so keeps the additive + * split in shares_gen non-degenerate. The argument is given in full in + * frost_enrollment.md, "Threshold must be at least 2", which is the single + * place to edit if the policy ever moves. * * Operates on public data only. Returns 1 if the parameters are valid, 0 * otherwise. */ diff --git a/src/modules/frost_enrollment/frost_enrollment.md b/src/modules/frost_enrollment/frost_enrollment.md index 20896122..6e2f2568 100644 --- a/src/modules/frost_enrollment/frost_enrollment.md +++ b/src/modules/frost_enrollment/frost_enrollment.md @@ -19,6 +19,14 @@ example can be found in `examples/frost_enrollment.c`. **This module is experimental.** Do not use it in production. The API should not be considered stable. +"Not stable" and "frozen" apply to different things, and the distinction +matters if you are writing an interoperating implementation. The C API — the +function signatures, the argument order, the buffer conventions — may change. +The wire-visible encodings — the two tagged hashes below, the parameters hash +serialization and the share-splitting derivation — are frozen by the +regression vectors, so changing one is a deliberate, vector-breaking change +rather than a silent one. + The protocol ------------ diff --git a/tools/test_vectors_frost_enrollment_generate.py b/tools/test_vectors_frost_enrollment_generate.py index bf8adbe0..80c2fe2e 100755 --- a/tools/test_vectors_frost_enrollment_generate.py +++ b/tools/test_vectors_frost_enrollment_generate.py @@ -247,10 +247,14 @@ def emit_case(c): CASES = [ # A 2-of-3 group enrolling a fourth participant with the minimum helper # set. The base case, and the one the module documentation walks through. - # This key has EVEN Y; the three below have odd Y. Nothing in enrollment - # depends on the parity of the threshold key -- unlike frost signing, it - # never takes an x-only view of it -- so this is coverage rather than a - # distinction the code makes. + # This key and the next have EVEN Y, the last two odd Y; among the derived + # public shares, case 3 is the even one and the rest are odd. Nothing in + # enrollment depends on either parity -- unlike frost signing, it never + # takes an x-only view of a key -- so this is coverage rather than a + # distinction the code makes. run_case() in this file prints nothing about + # parity; the values above were read off the generated vectors, so treat + # this comment as documentation of the current set rather than a + # constraint on it. dict( thresh_sk=0x0202020202020202020202020202020202020202020202020202020202020202, n_participants=3, @@ -282,8 +286,8 @@ CASES = [ new_id=2, seeds=[bytes([0x30 + i] * 32) for i in range(3)], ), - # A larger enrollment, 4-of-6 to 4-of-7, and the only case whose DERIVED - # public share has odd Y. + # A larger enrollment, 4-of-6 to 4-of-7, at the largest threshold and + # helper count in this set. dict( thresh_sk=0x1122334455667788990011223344556677889900112233445566778899001122, n_participants=6,