Merge bitcoin-core/secp256k1#1579: Clear sensitive memory without getting optimized out (revival of #636)

765ef53335 Clear _gej instances after point multiplication to avoid potential leaks (Sebastian Falbesoner)
349e6ab916 Introduce separate _clear functions for hash module (Tim Ruffing)
99cc9fd6d0 Don't rely on memset to set signed integers to 0 (Tim Ruffing)
97c57f42ba Implement various _clear() functions with secp256k1_memclear() (Tim Ruffing)
9bb368d146 Use secp256k1_memclear() to clear stack memory instead of memset() (Tim Ruffing)
e3497bbf00 Separate between clearing memory and setting to zero in tests (Tim Ruffing)
d79a6ccd43 Separate secp256k1_fe_set_int( . , 0 ) from secp256k1_fe_clear() (Tim Ruffing)
1c08126222 Add secp256k1_memclear() for clearing secret data (Tim Ruffing)
e7d384488e Don't clear secrets in pippenger implementation (Tim Ruffing)

Pull request description:

  This PR picks up #636 (which in turn picked up #448, so this is take number three) and is essentially a rebase on master.

  Some changes to the original PR:
  * the clearing function now has the `secp256k1_` prefix again, since the related helper `_memczero` got it as well (see PR #835 / commit e89278f211)
  * the original commit b17a7df8145a6a86d49c354c7e7b59a432ea5346 ("Make _set_fe_int( . , 0 ) set magnitude to 0") is not needed anymore, since it was already applied in PR #943 (commit d49011f54c)
  * clearing of stack memory with `secp256k1_memclear` is now also done on modules that have been newly introduced since then, i.e. schnorr and ellswift (of course, there is still no guarantee that all places where clearing is necessary are covered)

  So far I haven't looked at any disassembly and possible performance implications yet (there were some concerns expressed in https://github.com/bitcoin-core/secp256k1/pull/636#issuecomment-620118629), happy to go deeper there if this gets Concept ACKed.

  The proposed method of using a memory barrier to prevent optimizating away the memset is still used in BoringSSL (where it was originally picked up from) and in the Linux Kernel, see e.g. 5af122c3df/crypto/mem.c (L335) and d456068672/include/linux/string.h (L348) / d456068672/include/linux/compiler.h (L102)

  Fixes #185.

ACKs for top commit:
  sipa:
    reACK 765ef53335
  real-or-random:
    ACK 765ef53335

Tree-SHA512: 5a034d5ad14178c06928022459f3d4f0877d06f576b24ab07b86b3608b0b3e9273217b8309a1db606f024f3032731f13013114b1e0828964b578814d1efb2959

src/modules/ecdh/main_impl.h

@@ -19,6 +19,7 @@ static int ecdh_hash_function_sha256(unsigned char *output, const unsigned char
     secp256k1_sha256_write(&sha, &version, 1);
     secp256k1_sha256_write(&sha, x32, 32);
     secp256k1_sha256_finalize(&sha, output);
+    secp256k1_sha256_clear(&sha);
 
     return 1;
 }
@@ -61,9 +62,11 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se
 
     ret = hashfp(output, x, y, data);
 
-    memset(x, 0, 32);
-    memset(y, 0, 32);
+    secp256k1_memclear(x, sizeof(x));
+    secp256k1_memclear(y, sizeof(y));
     secp256k1_scalar_clear(&s);
+    secp256k1_ge_clear(&pt);
+    secp256k1_gej_clear(&res);
 
     return !!ret & !overflow;
 }

src/modules/ellswift/main_impl.h

@@ -510,6 +510,7 @@ static int ellswift_xdh_hash_function_prefix(unsigned char *output, const unsign
     secp256k1_sha256_write(&sha, ell_b64, 64);
     secp256k1_sha256_write(&sha, x32, 32);
     secp256k1_sha256_finalize(&sha, output);
+    secp256k1_sha256_clear(&sha);
 
     return 1;
 }
@@ -539,6 +540,7 @@ static int ellswift_xdh_hash_function_bip324(unsigned char* output, const unsign
     secp256k1_sha256_write(&sha, ell_b64, 64);
     secp256k1_sha256_write(&sha, x32, 32);
     secp256k1_sha256_finalize(&sha, output);
+    secp256k1_sha256_clear(&sha);
 
     return 1;
 }
@@ -580,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);
 
-    memset(sx, 0, 32);
+    secp256k1_memclear(sx, sizeof(sx));
     secp256k1_fe_clear(&px);
     secp256k1_scalar_clear(&s);
 

src/modules/musig/session_impl.h

@@ -385,11 +385,11 @@ 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 */
-        memset(buf, 0, sizeof(buf));
-        memset(&sha_tmp, 0, sizeof(sha_tmp));
+        secp256k1_memclear(buf, sizeof(buf));
+        secp256k1_sha256_clear(&sha_tmp);
     }
-    memset(rand, 0, sizeof(rand));
-    memset(&sha, 0, sizeof(sha));
+    secp256k1_memclear(rand, sizeof(rand));
+    secp256k1_sha256_clear(&sha);
 }
 
 int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
@@ -450,6 +450,7 @@ int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_m
         secp256k1_ge_set_gej(&nonce_pts[i], &nonce_ptj);
         secp256k1_declassify(ctx, &nonce_pts[i], sizeof(nonce_pts[i]));
         secp256k1_scalar_clear(&k[i]);
+        secp256k1_gej_clear(&nonce_ptj);
     }
     /* None of the nonce_pts will be infinity because k != 0 with overwhelming
      * probability */
@@ -509,7 +510,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;
     }
-    memset(seckey, 0, sizeof(seckey));
+    secp256k1_memclear(seckey, sizeof(seckey));
     return 1;
 }
 

src/modules/schnorrsig/main_impl.h

@@ -93,6 +93,7 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms
     secp256k1_sha256_write(&sha, xonly_pk32, 32);
     secp256k1_sha256_write(&sha, msg, msglen);
     secp256k1_sha256_finalize(&sha, nonce32);
+    secp256k1_sha256_clear(&sha);
     return 1;
 }
 
@@ -187,7 +188,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);
-    memset(seckey, 0, sizeof(seckey));
+    secp256k1_memclear(seckey, sizeof(seckey));
+    secp256k1_gej_clear(&rj);
 
     return ret;
 }