From 4cc0b1b669392d38770f74cb3fb5c801c82f67a0 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 5 Jul 2022 18:43:23 +0200 Subject: [PATCH 1/3] ecmult_gen: Skip RNG when creating blinding if no seed is available Running the RNG is pointless if no seed is available because the key will be fixed. The computation just wastes time. Previously, users could avoid this computation at least by asking for a context without signing capabilities. But since 3b0c218 we always build an ecmult_gen context, ignoring the context flags. Moreover, users could never avoid this pointless computation when asking for the creation of a signing context. --- src/ecmult_gen_impl.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 2c8a503acc483..695769c993183 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -88,12 +88,13 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const unsigned char nonce32[32]; secp256k1_rfc6979_hmac_sha256 rng; int overflow; - unsigned char keydata[64] = {0}; + unsigned char keydata[64]; if (seed32 == NULL) { /* When seed is NULL, reset the initial point and blinding value. */ secp256k1_gej_set_ge(&ctx->initial, &secp256k1_ge_const_g); secp256k1_gej_neg(&ctx->initial, &ctx->initial); secp256k1_scalar_set_int(&ctx->blind, 1); + return; } /* The prior blinding value (if not reset) is chained forward by including it in the hash. */ secp256k1_scalar_get_b32(nonce32, &ctx->blind); @@ -102,10 +103,9 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const * asking the caller for blinding values directly and expecting them to retry on failure. */ memcpy(keydata, nonce32, 32); - if (seed32 != NULL) { - memcpy(keydata + 32, seed32, 32); - } - secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, seed32 ? 64 : 32); + VERIFY_CHECK(seed32 != NULL); + memcpy(keydata + 32, seed32, 32); + secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64); memset(keydata, 0, sizeof(keydata)); /* Accept unobservably small non-uniformity. */ secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); From 7a869558004b70803717d8169dd8b090e04df4af Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 5 Jul 2022 18:50:05 +0200 Subject: [PATCH 2/3] ecmult_gen: Simplify code (no observable change) --- src/ecmult_gen_impl.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 695769c993183..b3e080a58b037 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -97,12 +97,11 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const return; } /* The prior blinding value (if not reset) is chained forward by including it in the hash. */ - secp256k1_scalar_get_b32(nonce32, &ctx->blind); + secp256k1_scalar_get_b32(keydata, &ctx->blind); /** Using a CSPRNG allows a failure free interface, avoids needing large amounts of random data, * and guards against weak or adversarial seeds. This is a simpler and safer interface than * asking the caller for blinding values directly and expecting them to retry on failure. */ - memcpy(keydata, nonce32, 32); VERIFY_CHECK(seed32 != NULL); memcpy(keydata + 32, seed32, 32); secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64); From 55f8bc99dce8846e0da99b92e52353c8cf893287 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 5 Jul 2022 19:12:01 +0200 Subject: [PATCH 3/3] ecmult_gen: Improve comments about projective blinding Whenever I read this code, I first think that rescaling ctx->initial is a dead store because we overwrite it later with gb. But that's wrong. The rescaling blinds the computation of gb and affects its result. --- src/ecmult_gen_impl.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index b3e080a58b037..4f5ea9f3c0858 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -111,7 +111,8 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const overflow = !secp256k1_fe_set_b32(&s, nonce32); overflow |= secp256k1_fe_is_zero(&s); secp256k1_fe_cmov(&s, &secp256k1_fe_one, overflow); - /* Randomize the projection to defend against multiplier sidechannels. */ + /* Randomize the projection to defend against multiplier sidechannels. + Do this before our own call to secp256k1_ecmult_gen below. */ secp256k1_gej_rescale(&ctx->initial, &s); secp256k1_fe_clear(&s); secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); @@ -120,6 +121,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_scalar_cmov(&b, &secp256k1_scalar_one, secp256k1_scalar_is_zero(&b)); secp256k1_rfc6979_hmac_sha256_finalize(&rng); memset(nonce32, 0, 32); + /* The random projection in ctx->initial ensures that gb will have a random projection. */ secp256k1_ecmult_gen(ctx, &gb, &b); secp256k1_scalar_negate(&b, &b); ctx->blind = b;