Split memclear into two versions

secp256k1_memclear has the side effect of undefining bytes for
valgrind checks. In some cases, we may want to zero bytes
but allow subsequent reads. So we split memclear into
memclear_explicit, which makes no guarantees about the content
of the buffer on return, and memzero_explicit, which guarantees
zero value on return.

Change the memset in partial_sign to use memzero_explicit.
This commit is contained in:
John Moffett
2025-09-08 12:21:48 -04:00
parent 4985ac0f89
commit 399b582a5f
11 changed files with 41 additions and 28 deletions

View File

@@ -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);

View File

@@ -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) {

View File

@@ -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) {

View File

@@ -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

View File

@@ -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);

View File

@@ -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);

View File

@@ -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;

View File

@@ -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;

View File

@@ -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) {

View File

@@ -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);

View File

@@ -219,8 +219,8 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
}
}
/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */
static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) {
/* Zeroes memory to prevent leaking sensitive info. Won't be optimized out. */
static SECP256K1_INLINE void secp256k1_memzero_explicit(void *ptr, size_t len) {
#if defined(_MSC_VER)
/* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */
SecureZeroMemory(ptr, len);
@@ -242,6 +242,19 @@ static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) {
void *(*volatile const volatile_memset)(void *, int, size_t) = memset;
volatile_memset(ptr, 0, len);
#endif
}
/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out.
* The state of the memory after this call is unspecified so callers must not
* make any assumptions about its contents.
*
* In VERIFY builds, it has the side effect of marking the memory as undefined.
* This helps to detect use-after-clear bugs where code incorrectly reads from
* cleansed memory during testing.
*/
static SECP256K1_INLINE void secp256k1_memclear_explicit(void *ptr, size_t len) {
/* The current implementation zeroes, but callers must not rely on this */
secp256k1_memzero_explicit(ptr, len);
#ifdef VERIFY
SECP256K1_CHECKMEM_UNDEFINE(ptr, len);
#endif
@@ -277,7 +290,7 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size
}
ret = (acc == 0);
/* acc may contain secret values. Try to explicitly clear it. */
secp256k1_memclear(&acc, sizeof(acc));
secp256k1_memclear_explicit(&acc, sizeof(acc));
return ret;
}