Merge #1735: musig: Invalidate secnonce in secp256k1_musig_partial_sign

399b582 Split memclear into two versions (John Moffett)

Pull request description:

  Replace `memset` which can still be optimized out as `secnonce` isn't read later in this function. The API makes it clear the callee is responsible for it, so we need to assure it's cleared properly.

ACKs for top commit:
  real-or-random:
    utACK 399b582 thanks!
  jonasnick:
    ACK 399b582

Tree-SHA512: 98cd6c44a45218ad66ca99ceacb9bbccee7ff105957363bcde9c3e17d55ac4a81afc881af8adfce7ff3d6ef5d75f3b111d7220256914ca0bedeb43769b38dd20

Author: real-or-random

src/ecmult_gen_impl.h

@@ -277,8 +277,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
     /* Cleanup. */
     secp256k1_fe_clear(&neg);
     secp256k1_ge_clear(&add);
-    secp256k1_memclear(&adds, sizeof(adds));
-    secp256k1_memclear(&recoded, sizeof(recoded));
+    secp256k1_memclear_explicit(&adds, sizeof(adds));
+    secp256k1_memclear_explicit(&recoded, sizeof(recoded));
 }
 
 /* Setup blinding values for secp256k1_ecmult_gen. */
@@ -310,7 +310,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
     VERIFY_CHECK(seed32 != NULL);
     memcpy(keydata + 32, seed32, 32);
     secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64);
-    secp256k1_memclear(keydata, sizeof(keydata));
+    secp256k1_memclear_explicit(keydata, sizeof(keydata));
 
     /* Compute projective blinding factor (cannot be 0). */
     secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
@@ -331,7 +331,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
     secp256k1_ge_set_gej(&ctx->ge_offset, &gb);
 
     /* Clean up. */
-    secp256k1_memclear(nonce32, sizeof(nonce32));
+    secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
     secp256k1_scalar_clear(&b);
     secp256k1_gej_clear(&gb);
     secp256k1_fe_clear(&f);

src/field_impl.h

@@ -19,7 +19,7 @@
 #endif
 
 SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) {
-    secp256k1_memclear(a, sizeof(secp256k1_fe));
+    secp256k1_memclear_explicit(a, sizeof(secp256k1_fe));
 }
 
 SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) {

src/group_impl.h

@@ -337,11 +337,11 @@ static void secp256k1_ge_set_infinity(secp256k1_ge *r) {
 }
 
 static void secp256k1_gej_clear(secp256k1_gej *r) {
-    secp256k1_memclear(r, sizeof(secp256k1_gej));
+    secp256k1_memclear_explicit(r, sizeof(secp256k1_gej));
 }
 
 static void secp256k1_ge_clear(secp256k1_ge *r) {
-    secp256k1_memclear(r, sizeof(secp256k1_ge));
+    secp256k1_memclear_explicit(r, sizeof(secp256k1_ge));
 }
 
 static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) {

src/hash_impl.h

@@ -172,7 +172,7 @@ static void secp256k1_sha256_initialize_tagged(secp256k1_sha256 *hash, const uns
 }
 
 static void secp256k1_sha256_clear(secp256k1_sha256 *hash) {
-    secp256k1_memclear(hash, sizeof(*hash));
+    secp256k1_memclear_explicit(hash, sizeof(*hash));
 }
 
 static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t keylen) {
@@ -200,7 +200,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const
         rkey[n] ^= 0x5c ^ 0x36;
     }
     secp256k1_sha256_write(&hash->inner, rkey, sizeof(rkey));
-    secp256k1_memclear(rkey, sizeof(rkey));
+    secp256k1_memclear_explicit(rkey, sizeof(rkey));
 }
 
 static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size) {
@@ -211,12 +211,12 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned
     unsigned char temp[32];
     secp256k1_sha256_finalize(&hash->inner, temp);
     secp256k1_sha256_write(&hash->outer, temp, 32);
-    secp256k1_memclear(temp, sizeof(temp));
+    secp256k1_memclear_explicit(temp, sizeof(temp));
     secp256k1_sha256_finalize(&hash->outer, out32);
 }
 
 static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash) {
-    secp256k1_memclear(hash, sizeof(*hash));
+    secp256k1_memclear_explicit(hash, sizeof(*hash));
 }
 
 static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen) {
@@ -285,7 +285,7 @@ static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256
 }
 
 static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng) {
-    secp256k1_memclear(rng, sizeof(*rng));
+    secp256k1_memclear_explicit(rng, sizeof(*rng));
 }
 
 #undef Round

src/modules/ecdh/main_impl.h

@@ -62,8 +62,8 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se
 
     ret = hashfp(output, x, y, data);
 
-    secp256k1_memclear(x, sizeof(x));
-    secp256k1_memclear(y, sizeof(y));
+    secp256k1_memclear_explicit(x, sizeof(x));
+    secp256k1_memclear_explicit(y, sizeof(y));
     secp256k1_scalar_clear(&s);
     secp256k1_ge_clear(&pt);
     secp256k1_gej_clear(&res);

src/modules/ellswift/main_impl.h

@@ -582,7 +582,7 @@ int secp256k1_ellswift_xdh(const secp256k1_context *ctx, unsigned char *output,
     /* Invoke hasher */
     ret = hashfp(output, sx, ell_a64, ell_b64, data);
 
-    secp256k1_memclear(sx, sizeof(sx));
+    secp256k1_memclear_explicit(sx, sizeof(sx));
     secp256k1_fe_clear(&px);
     secp256k1_scalar_clear(&s);
 

src/modules/musig/session_impl.h

@@ -385,10 +385,10 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
         secp256k1_scalar_set_b32(&k[i], buf, NULL);
 
         /* Attempt to erase secret data */
-        secp256k1_memclear(buf, sizeof(buf));
+        secp256k1_memclear_explicit(buf, sizeof(buf));
         secp256k1_sha256_clear(&sha_tmp);
     }
-    secp256k1_memclear(rand, sizeof(rand));
+    secp256k1_memclear_explicit(rand, sizeof(rand));
     secp256k1_sha256_clear(&sha);
 }
 
@@ -518,7 +518,7 @@ int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_mu
     if (!secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32)) {
         return 0;
     }
-    secp256k1_memclear(seckey, sizeof(seckey));
+    secp256k1_memclear_explicit(seckey, sizeof(seckey));
     return 1;
 }
 
@@ -679,7 +679,7 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, secp256k1_musig_p
     ret = secp256k1_musig_secnonce_load(ctx, k, &pk, secnonce);
     /* Set nonce to zero to avoid nonce reuse. This will cause subsequent calls
      * of this function to fail */
-    memset(secnonce, 0, sizeof(*secnonce));
+    secp256k1_memzero_explicit(secnonce, sizeof(*secnonce));
     if (!ret) {
         secp256k1_musig_partial_sign_clear(&sk, k);
         return 0;

src/modules/schnorrsig/main_impl.h

@@ -94,7 +94,7 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms
     secp256k1_sha256_write(&sha, msg, msglen);
     secp256k1_sha256_finalize(&sha, nonce32);
     secp256k1_sha256_clear(&sha);
-    secp256k1_memclear(masked_key, sizeof(masked_key));
+    secp256k1_memclear_explicit(masked_key, sizeof(masked_key));
 
     return 1;
 }
@@ -190,8 +190,8 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
     secp256k1_memczero(sig64, 64, !ret);
     secp256k1_scalar_clear(&k);
     secp256k1_scalar_clear(&sk);
-    secp256k1_memclear(seckey, sizeof(seckey));
-    secp256k1_memclear(nonce32, sizeof(nonce32));
+    secp256k1_memclear_explicit(seckey, sizeof(seckey));
+    secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
     secp256k1_gej_clear(&rj);
 
     return ret;

src/scalar_impl.h

@@ -28,7 +28,7 @@ static const secp256k1_scalar secp256k1_scalar_one = SECP256K1_SCALAR_CONST(0, 0
 static const secp256k1_scalar secp256k1_scalar_zero = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 0);
 
 SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) {
-    secp256k1_memclear(r, sizeof(secp256k1_scalar));
+    secp256k1_memclear_explicit(r, sizeof(secp256k1_scalar));
 }
 
 static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin) {

src/secp256k1.c

@@ -499,7 +499,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m
    }
    secp256k1_rfc6979_hmac_sha256_finalize(&rng);
 
-   secp256k1_memclear(keydata, sizeof(keydata));
+   secp256k1_memclear_explicit(keydata, sizeof(keydata));
    secp256k1_rfc6979_hmac_sha256_clear(&rng);
    return 1;
 }
@@ -550,7 +550,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc
      * seckey. As a result is_sec_valid is included in ret only after ret was
      * used as a branching variable. */
     ret &= is_sec_valid;
-    secp256k1_memclear(nonce32, sizeof(nonce32));
+    secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
     secp256k1_scalar_clear(&msg);
     secp256k1_scalar_clear(&non);
     secp256k1_scalar_clear(&sec);