diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0fa84ce0..c88480ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -854,3 +854,13 @@ jobs: run: | cmake -B ${{ env.CI_BUILD }} -DCMAKE_INSTALL_PREFIX=${{ env.CI_INSTALL }} -DSECP256K1_EXPERIMENTAL=ON -DSECP256K1_ENABLE_MODULE_FROST=ON -DSECP256K1_ENABLE_MODULE_CHILLDKG=ON -DSECP256K1_ENABLE_MODULE_ICEBERG=ON -DSECP256K1_ENABLE_MODULE_PREFRACTAL=ON -DSECP256K1_ENABLE_MODULE_FROST_ENROLLMENT=ON && cmake --build ${{ env.CI_BUILD }} && cmake --install ${{ env.CI_BUILD }} && ls -RlAh ${{ env.CI_INSTALL }} gcc -o ecdsa examples/ecdsa.c -I ${{ env.CI_INSTALL }}/include -L ${{ env.CI_INSTALL }}/lib*/ -l secp256k1 -Wl,-rpath,"${{ env.CI_INSTALL }}/lib",-rpath,"${{ env.CI_INSTALL }}/lib64" && ./ecdsa + # Assert the modules actually landed. `ls -RlAh` above only prints, + # so without these a regression that silently drops a module from + # the CMake build -- a deleted add_compile_definitions or + # PUBLIC_HEADER line during an upstream sync -- would keep this step + # green: no module code means no undefined references, and the ecdsa + # link exercises core symbols only. + ls ${{ env.CI_INSTALL }}/include/secp256k1_frost.h ${{ env.CI_INSTALL }}/include/secp256k1_chilldkg.h ${{ env.CI_INSTALL }}/include/secp256k1_iceberg.h ${{ env.CI_INSTALL }}/include/secp256k1_prefractal.h ${{ env.CI_INSTALL }}/include/secp256k1_frost_enrollment.h + # And that the symbols are really in the library, not just the + # headers on disk. + gcc -o frost_enrollment examples/frost_enrollment.c -I ${{ env.CI_INSTALL }}/include -L ${{ env.CI_INSTALL }}/lib*/ -l secp256k1 -Wl,-rpath,"${{ env.CI_INSTALL }}/lib",-rpath,"${{ env.CI_INSTALL }}/lib64" && ./frost_enrollment diff --git a/include/secp256k1_frost_enrollment.h b/include/secp256k1_frost_enrollment.h index 8cee13d9..91344776 100644 --- a/include/secp256k1_frost_enrollment.h +++ b/include/secp256k1_frost_enrollment.h @@ -172,10 +172,11 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_frost_enrollment_params * Returns: 0 if the arguments are invalid, 1 otherwise * Args: ctx: pointer to a context object * Out: shares32_out: pointer to an array of u*32 bytes for the enrollment - * shares, aligned with `ids`. Zeroed if this function - * returns 0 -- except when n_ids itself is out of - * range, in which case the buffer is not written at - * all, since its size is not known to be u*32. + * shares, aligned with `ids`. Not written at all when + * the parameter tuple is rejected, since its extent is + * only known to be u*32 once n_ids has been validated + * against the rest of the tuple; zeroed on every + * failure detected after that point. * params_hash32_out: pointer to a 32-byte array for the parameters hash, * identical to what * `secp256k1_frost_enrollment_params_hash` returns for diff --git a/src/modules/frost_enrollment/frost_enrollment.md b/src/modules/frost_enrollment/frost_enrollment.md index 6e2f2568..b02f0210 100644 --- a/src/modules/frost_enrollment/frost_enrollment.md +++ b/src/modules/frost_enrollment/frost_enrollment.md @@ -324,3 +324,14 @@ operations. Every branch driven by identifiers, counts or hash comparisons operates on public values. Secret intermediates are cleansed with `secp256k1_memclear_explicit` before the functions return, and outputs are zeroed on every failure path. + +The one exception is `shares32_out` in +`secp256k1_frost_enrollment_shares_gen`, the only output in this module whose +size is caller-supplied. When the parameter tuple is rejected, that buffer is +left untouched rather than zeroed: its extent is only known to be `u*32` once +`n_ids` has been validated against the rest of the tuple, so zeroing first +would write past a buffer whose length the caller got wrong. Failures +detected after validation — an unusable `thresh_pk`, `my_id` not among the +helpers, an invalid secret share — do zero it. Callers that inspect an output +buffer after a failed call must therefore treat a rejected tuple as leaving it +unmodified, not as leaving it zeroed. diff --git a/src/modules/frost_enrollment/tests_impl.h b/src/modules/frost_enrollment/tests_impl.h index 56183a39..dd6485ca 100644 --- a/src/modules/frost_enrollment/tests_impl.h +++ b/src/modules/frost_enrollment/tests_impl.h @@ -911,8 +911,10 @@ static void run_frost_enrollment_contract_test(void) { /* share_agg reports a share that is not a valid scalar the same way it * reports a parameters disagreement: by naming the responsible helper. - * The helper set here is {0, 2}, so an implementation returning the array - * index rather than the identifier would be caught. */ + * The helper set is {0, 2}; note that it is the SECOND case below, which + * corrupts slot 1 (identifier 2), that discriminates -- slot 0's + * identifier is also 0, so on its own it would pass for an + * implementation returning array indices. */ frost_enrollment_test_deal(&r, 3, 2, 2, 1); CHECK(r.ids[0] == 0 && r.ids[1] == 2); frost_enrollment_test_round1_gen(&r); @@ -935,6 +937,33 @@ static void run_frost_enrollment_contract_test(void) { CHECK(mismatch_id == r.ids[1]); CHECK(mismatch_id == 2); + /* The other attribution path -- a disagreeing parameters hash -- also has + * to name the identifier. Its own tests use the helper set {0, 1}, where + * every identifier equals its index, so it is pinned here instead on + * {1, 3}: the corrupted slot is index 0 and must be reported as 1. */ + frost_enrollment_test_deal(&r, 5, 2, 2, 4); + { + static const uint32_t skewed[2] = { 1, 3 }; + frost_enrollment_test_use_helpers(&r, skewed, 2); + frost_enrollment_test_round1_gen(&r); + frost_enrollment_test_collect(&r, 1, all_shares, received); + memset(&received[0], 0xa5, 32); + mismatch_id = 0; + memset(sigma, 0xff, sizeof(sigma)); + CHECK(secp256k1_frost_enrollment_share_agg(CTX, sigma, &mismatch_id, all_shares, received, &r.thresh_pk, r.ids, r.u, r.ids[1], r.new_id, r.n, (uint32_t)r.t) == 0); + CHECK(mismatch_id == r.ids[0]); + CHECK(mismatch_id == 1); + CHECK(secp256k1_is_zero_array(sigma, sizeof(sigma))); + /* And the same corruption one slot along, to fix the mapping at both + * ends: index 1, identifier 3. */ + frost_enrollment_test_collect(&r, 0, all_shares, received); + memset(&received[32], 0xa5, 32); + mismatch_id = 0; + CHECK(secp256k1_frost_enrollment_share_agg(CTX, sigma, &mismatch_id, all_shares, received, &r.thresh_pk, r.ids, r.u, r.ids[0], r.new_id, r.n, (uint32_t)r.t) == 0); + CHECK(mismatch_id == r.ids[1]); + CHECK(mismatch_id == 3); + } + /* An out-of-range sigma is rejected by secshare_gen too, which has no * attribution to offer. */ frost_enrollment_test_full_run(&r, 3, 2, 2, 3);