Files
secp256k1-zkp/tools
Kgothatso Ngako 7e73badcbd frost: fix constant-time violations and C90 conformance
The module's BIP 445 logic itself is unchanged and was independently
validated against the pinned spec commit bb5396f (BIP v0.10.0), both via
the checked-in test vectors and via differential testing against the
Python reference over 360 randomised configurations (n up to 128,
shuffled non-contiguous signer ids, mixed xonly/plain tweak chains,
variable-length messages, pubshares present and absent). Every change
below is structural: the differential harness produces byte-identical
pubnonces, aggnonces, partial signatures and final signatures before and
after.

Two classes of problem prevented the module from passing CI.

1. Constant-time violations (ctime_tests)
-----------------------------------------

The CI matrix enables FROST in rows that also run
"valgrind --error-exitcode=42 ./ctime_tests" -- WITH_VALGRIND and
CTIMETESTS both default to 'yes'. With the module enabled that job
reported 639 "conditional jump depends on uninitialised value" errors,
all originating from two sites:

  - secp256k1_frost_derive_coefficient returned

        !overflow && !secp256k1_scalar_is_zero(out)

    where the short-circuiting && branches on `overflow`, which is
    derived from the threshold secret key. The caller declassifies the
    return value, but the branch has already happened inside the callee.
    Replaced with a bitwise &, matching the existing idiom in
    secp256k1_scalar_set_b32_seckey (src/scalar_impl.h).

  - secp256k1_frost_sign_internal performs the self-verification
    recommended by BIP 445, which runs the *variable-time*
    secp256k1_ecmult over the partial signature s. nonce_pts and pk were
    already declassified ahead of that call; s was not. Since s is the
    public output of the function, declassifying it before the
    self-verification is both correct and sufficient.

secp256k1_frost_deterministic_sign carried three more instances of the
same class, invisible until now because ctime_tests did not exercise
that path at all:

  - the `if (!valid)` check on secp256k1_scalar_set_b32_seckey lacked the
    declassify that the identical checks in secp256k1_frost_nonce_gen and
    secp256k1_frost_sign_internal already have;
  - secp256k1_frost_det_nonce_function used the same short-circuiting &&,
    here over the secret nonces;
  - the branch on that function's result was not declassified.

The && in det_nonce_function is rewritten via two int locals rather than
a bare bitwise &: clang's -Wbitwise-instead-of-logical fires when both
operands are `!f(...)` expressions, which would break the -Werror clang
builds.

ctime_tests now also covers secp256k1_frost_deterministic_sign, so that
path stays checked from here on.

None of these leak anything of value in practice -- they reveal only
negligible-probability events (a hash overflowing the curve order, a zero
nonce) or whether a secret share is a valid secret key -- but they
violate the project's declassification discipline and fail the ctime
test.

2. C90 conformance (-Werror -pedantic-errors)
---------------------------------------------

The project targets C90 (CMAKE_C_STANDARD 90, -std=c89 -pedantic) and CI
passes WERROR_CFLAGS='-Werror -pedantic-errors'. Compiling src/tests.c
with those flags produced 62 errors in three groups:

  - 40x "ISO C forbids empty initializer braces before C2X" in the
    generated vectors.h; empty {} initializers are C23-only. Fixed in
    tools/test_vectors_frost_generate.py so it survives regeneration:
    hexstr_to_intarray now emits "0" for an empty byte string (all six
    of its call sites wrap the result in braces), and init_group's
    `counted` helper emits "{ 0 }" for an empty group. In every affected
    slot the paired count/length field is 0, so the padding element is
    never read.

  - 1x "comma at end of enumerator list" (C99+), also in the generator.

  - 21x "initializer element is not computable at load time" across 11
    lines of tests_impl.h. C90 requires constant expressions in
    initializers for automatic aggregates, so

        const secp256k1_frost_pubnonce *ptrs[2] = { &a, &b };

    is invalid. Rewritten as a declaration plus assignments, the style
    the musig tests already use, which is why the pre-existing tree was
    green.

vectors.h is regenerated from the spec's JSON vectors. Its hex payload is
byte-identical (verified by hashing every 0xNN token) and the file still
reproduces exactly from tools/test_vectors_frost_generate.py.

Verification
------------

  - gcc and clang, -std=c89 -pedantic-errors -Werror, with and without
    -DVERIFY: clean (was 62 errors)
  - ctime_tests under MemorySanitizer: 0 reports (was 639); exits 0 with
    halt_on_error=1
  - tests, noverify_tests and frost_example: pass
  - vectors.h regenerates identically from the pinned spec vectors
  - 240 signing + 120 deterministic-signing differential cases against
    the BIP 445 Python reference: byte-identical to the pre-fix build

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-31 00:57:43 +02:00
..