refactor: introduce _ecmult_gen_ge helper (preventing accidental gej leaks)

Scalar multiplication with the generator point frequently involves a
conversion to affine coordinates and clearing out the temporary Jacobian
group element object after to avoid leaking secret key material, i.e.
executing the following three steps:
    - secp256k1_ecmult_gen(ctx, &rj, ...)
    - secp256k1_ge_set_gej(&r, &rj)
    - secp256k1_gej_clear(&rj)

This commit introduces a corresponding helper to deduplicate code
and mitigate the risk that last step is forgotten (which can easily
happen and is not detected by tests).

The idea came up during a conversation with furszy, see
https://github.com/bitcoin-core/secp256k1/pull/1765#issuecomment-4482838033
This commit is contained in:
Sebastian Falbesoner
2026-05-19 16:29:05 +02:00
parent c63062380f
commit a3296d5e23
5 changed files with 14 additions and 16 deletions

View File

@@ -273,14 +273,12 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_scalar *sigr, const secp25
static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, secp256k1_scalar *sigr, secp256k1_scalar *sigs, const secp256k1_scalar *seckey, const secp256k1_scalar *message, const secp256k1_scalar *nonce, int *recid) {
unsigned char b[32];
secp256k1_gej rp;
secp256k1_ge r;
secp256k1_scalar n;
int overflow = 0;
int high;
secp256k1_ecmult_gen(ctx, &rp, nonce);
secp256k1_ge_set_gej(&r, &rp);
secp256k1_ecmult_gen_ge(ctx, &r, nonce);
secp256k1_fe_normalize(&r.x);
secp256k1_fe_normalize(&r.y);
secp256k1_fe_get_b32(b, &r.x);
@@ -296,7 +294,6 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
secp256k1_scalar_inverse(sigs, nonce);
secp256k1_scalar_mul(sigs, sigs, &n);
secp256k1_scalar_clear(&n);
secp256k1_gej_clear(&rp);
secp256k1_ge_clear(&r);
high = secp256k1_scalar_is_high(sigs);
secp256k1_scalar_cond_negate(sigs, high);

View File

@@ -138,6 +138,7 @@ static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx
/** Multiply with the generator: R = a*G */
static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context* ctx, secp256k1_gej *r, const secp256k1_scalar *a);
static void secp256k1_ecmult_gen_ge(const secp256k1_ecmult_gen_context* ctx, secp256k1_ge *r, const secp256k1_scalar *a);
static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_hash_ctx *hash_ctx, const unsigned char *seed32);

View File

@@ -281,11 +281,19 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
secp256k1_memclear_explicit(&recoded, sizeof(recoded));
}
SECP256K1_INLINE static void secp256k1_ecmult_gen_ge(const secp256k1_ecmult_gen_context *ctx, secp256k1_ge *r, const secp256k1_scalar *a) {
secp256k1_gej rj;
secp256k1_ecmult_gen(ctx, &rj, a);
secp256k1_ge_set_gej(r, &rj);
/* Jacobian coordinates resulting from our multiplication algorithm could potentially leak
* information about the secret input scalar, so clear the memory out to be on the safe side. */
secp256k1_gej_clear(&rj);
}
/* Setup blinding values for secp256k1_ecmult_gen. */
static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_hash_ctx *hash_ctx, const unsigned char *seed32) {
secp256k1_scalar b;
secp256k1_scalar diff;
secp256k1_gej gb;
secp256k1_fe f;
unsigned char nonce32[32];
secp256k1_rfc6979_hmac_sha256 rng;
@@ -325,15 +333,13 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
* which secp256k1_gej_add_ge cannot handle. */
secp256k1_scalar_cmov(&b, &secp256k1_scalar_one, secp256k1_scalar_is_zero(&b));
secp256k1_rfc6979_hmac_sha256_finalize(&rng);
secp256k1_ecmult_gen(ctx, &gb, &b);
secp256k1_ecmult_gen_ge(ctx, &ctx->ge_offset, &b);
secp256k1_scalar_negate(&b, &b);
secp256k1_scalar_add(&ctx->scalar_offset, &b, &diff);
secp256k1_ge_set_gej(&ctx->ge_offset, &gb);
/* Clean up. */
secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
secp256k1_scalar_clear(&b);
secp256k1_gej_clear(&gb);
secp256k1_fe_clear(&f);
secp256k1_rfc6979_hmac_sha256_clear(&rng);
}

View File

@@ -123,7 +123,6 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
secp256k1_scalar sk;
secp256k1_scalar e;
secp256k1_scalar k;
secp256k1_gej rj;
secp256k1_ge pk;
secp256k1_ge r;
unsigned char nonce32[32] = { 0 };
@@ -160,8 +159,7 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
ret &= !secp256k1_scalar_is_zero(&k);
secp256k1_scalar_cmov(&k, &secp256k1_scalar_one, !ret);
secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &rj, &k);
secp256k1_ge_set_gej(&r, &rj);
secp256k1_ecmult_gen_ge(&ctx->ecmult_gen_ctx, &r, &k);
/* We declassify r to allow using it as a branch point. This is fine
* because r is not a secret. */
@@ -183,7 +181,6 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
secp256k1_scalar_clear(&sk);
secp256k1_memclear_explicit(seckey, sizeof(seckey));
secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
secp256k1_gej_clear(&rj);
return ret;
}

View File

@@ -624,15 +624,12 @@ int secp256k1_ec_seckey_verify(const secp256k1_context* ctx, const unsigned char
}
static int secp256k1_ec_pubkey_create_helper(const secp256k1_ecmult_gen_context *ecmult_gen_ctx, secp256k1_scalar *seckey_scalar, secp256k1_ge *p, const unsigned char *seckey) {
secp256k1_gej pj;
int ret;
ret = secp256k1_scalar_set_b32_seckey(seckey_scalar, seckey);
secp256k1_scalar_cmov(seckey_scalar, &secp256k1_scalar_one, !ret);
secp256k1_ecmult_gen(ecmult_gen_ctx, &pj, seckey_scalar);
secp256k1_ge_set_gej(p, &pj);
secp256k1_gej_clear(&pj);
secp256k1_ecmult_gen_ge(ecmult_gen_ctx, p, seckey_scalar);
return ret;
}