From e7d384488e85650cd2a29493bc55fa5e68bf59c3 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 6 Jun 2019 16:51:33 +0200 Subject: [PATCH 01/21] Don't clear secrets in pippenger implementation This code is not supposed to handle secret data. --- src/ecmult_impl.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 07d8477e..8f584552 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -660,7 +660,6 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call struct secp256k1_pippenger_state *state_space; size_t idx = 0; size_t point_idx = 0; - int i, j; int bucket_window; secp256k1_gej_set_infinity(r); @@ -708,18 +707,6 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call } secp256k1_ecmult_pippenger_wnaf(buckets, bucket_window, state_space, r, scalars, points, idx); - - /* Clear data */ - for(i = 0; (size_t)i < idx; i++) { - secp256k1_scalar_clear(&scalars[i]); - state_space->ps[i].skew_na = 0; - for(j = 0; j < WNAF_SIZE(bucket_window+1); j++) { - state_space->wnaf_na[i * WNAF_SIZE(bucket_window+1) + j] = 0; - } - } - for(i = 0; i < 1< Date: Thu, 6 Jun 2019 21:22:28 +0200 Subject: [PATCH 02/21] Add secp256k1_memclear() for clearing secret data We rely on memset() and an __asm__ memory barrier where it's available or on SecureZeroMemory() on Windows. The fallback implementation uses a volatile function pointer to memset which the compiler is not clever enough to optimize. --- src/util.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/util.h b/src/util.h index 49af867a..c2add8ac 100644 --- a/src/util.h +++ b/src/util.h @@ -8,11 +8,17 @@ #define SECP256K1_UTIL_H #include "../include/secp256k1.h" +#include "checkmem.h" +#include #include #include #include #include +#if defined(_MSC_VER) +/* For SecureZeroMemory */ +#include +#endif #define STR_(x) #x #define STR(x) STR_(x) @@ -221,6 +227,34 @@ 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) { +#if defined(_MSC_VER) + /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ + SecureZeroMemory(ptr, len); +#elif defined(__GNUC__) + /* We use a memory barrier that scares the compiler away from optimizing out the memset. + * + * Quoting Adam Langley in commit ad1907fe73334d6c696c8539646c21b11178f20f + * in BoringSSL (ISC License): + * As best as we can tell, this is sufficient to break any optimisations that + * might try to eliminate "superfluous" memsets. + * This method is used in memzero_explicit() the Linux kernel, too. Its advantage is that it + * is pretty efficient, because the compiler can still implement the memset() efficently, + * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by + * Yang et al. (USENIX Security 2017) for more background. + */ + memset(ptr, 0, len); + __asm__ __volatile__("" : : "r"(ptr) : "memory"); +#else + void *(*volatile const volatile_memset)(void *, int, size_t) = memset; + volatile_memset(ptr, 0, len); +#endif +#ifdef VERIFY + SECP256K1_CHECKMEM_UNDEFINE(ptr, len); +#endif +} + /** Semantics like memcmp. Variable-time. * * We use this to avoid possible compiler bugs with memcmp, e.g. From d79a6ccd43ae1681280e5e27d9cd181285bf8540 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 12 Jun 2019 16:05:06 +0200 Subject: [PATCH 03/21] Separate secp256k1_fe_set_int( . , 0 ) from secp256k1_fe_clear() There are two uses of the secp256k1_fe_clear() function that are now separated into these two functions in order to reflect the intent: 1) initializing the memory prior to being used -> converted to fe_set_int( . , 0 ) 2) zeroing the memory after being used such that no sensitive data remains. -> remains as fe_clear() In the latter case, 'magnitude' and 'normalized' need to be overwritten when VERIFY is enabled. Co-Authored-By: isle2983 --- src/field.h | 6 +----- src/field_impl.h | 2 +- src/group_impl.h | 10 +++++----- src/testutil.h | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/field.h b/src/field.h index 8c65a3af..95990309 100644 --- a/src/field.h +++ b/src/field.h @@ -144,11 +144,7 @@ static int secp256k1_fe_normalizes_to_zero_var(const secp256k1_fe *r); */ static void secp256k1_fe_set_int(secp256k1_fe *r, int a); -/** Set a field element to 0. - * - * On input, a does not need to be initialized. - * On output, a represents 0, is normalized and has magnitude 0. - */ +/** Clear a field element to prevent leaking sensitive information. */ static void secp256k1_fe_clear(secp256k1_fe *a); /** Determine whether a represents field element 0. diff --git a/src/field_impl.h b/src/field_impl.h index 989e9cdb..8cf6f22c 100644 --- a/src/field_impl.h +++ b/src/field_impl.h @@ -235,7 +235,7 @@ SECP256K1_INLINE static void secp256k1_fe_add_int(secp256k1_fe *r, int a) { static void secp256k1_fe_impl_clear(secp256k1_fe *a); SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { a->magnitude = 0; - a->normalized = 1; + a->normalized = 0; secp256k1_fe_impl_clear(a); SECP256K1_FE_VERIFY(a); diff --git a/src/group_impl.h b/src/group_impl.h index 2e096f41..10aff433 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -283,17 +283,17 @@ static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const se static void secp256k1_gej_set_infinity(secp256k1_gej *r) { r->infinity = 1; - secp256k1_fe_clear(&r->x); - secp256k1_fe_clear(&r->y); - secp256k1_fe_clear(&r->z); + secp256k1_fe_set_int(&r->x, 0); + secp256k1_fe_set_int(&r->y, 0); + secp256k1_fe_set_int(&r->z, 0); SECP256K1_GEJ_VERIFY(r); } static void secp256k1_ge_set_infinity(secp256k1_ge *r) { r->infinity = 1; - secp256k1_fe_clear(&r->x); - secp256k1_fe_clear(&r->y); + secp256k1_fe_set_int(&r->x, 0); + secp256k1_fe_set_int(&r->y, 0); SECP256K1_GE_VERIFY(r); } diff --git a/src/testutil.h b/src/testutil.h index fc56854d..64b3bb41 100644 --- a/src/testutil.h +++ b/src/testutil.h @@ -34,7 +34,7 @@ static void testutil_random_fe_magnitude(secp256k1_fe *fe, int m) { if (n == 0) { return; } - secp256k1_fe_clear(&zero); + secp256k1_fe_set_int(&zero, 0); secp256k1_fe_negate(&zero, &zero, 0); secp256k1_fe_mul_int_unchecked(&zero, n - 1); secp256k1_fe_add(fe, &zero); From e3497bbf001f8e06dcc21d27fed173be77ef7136 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sat, 8 Jun 2019 11:56:15 +0200 Subject: [PATCH 04/21] Separate between clearing memory and setting to zero in tests Co-Authored-By: isle2983 Co-Authored-By: Pieter Wuille --- src/bench_ecmult.c | 2 +- src/tests.c | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/bench_ecmult.c b/src/bench_ecmult.c index 7dc52ad8..3974af75 100644 --- a/src/bench_ecmult.c +++ b/src/bench_ecmult.c @@ -71,7 +71,7 @@ static void bench_ecmult_teardown_helper(bench_data* data, size_t* seckey_offset secp256k1_scalar sum_scalars; secp256k1_gej_set_infinity(&sum_output); - secp256k1_scalar_clear(&sum_scalars); + secp256k1_scalar_set_int(&sum_scalars, 0); for (i = 0; i < iters; ++i) { secp256k1_gej_add_var(&sum_output, &sum_output, &data->output[i], NULL); if (scalar_gen_offset != NULL) { diff --git a/src/tests.c b/src/tests.c index dba4097b..270b1c83 100644 --- a/src/tests.c +++ b/src/tests.c @@ -3671,8 +3671,7 @@ static void test_ge(void) { secp256k1_fe zfi2, zfi3; secp256k1_gej_set_infinity(&gej[0]); - secp256k1_ge_clear(&ge[0]); - secp256k1_ge_set_gej_var(&ge[0], &gej[0]); + secp256k1_ge_set_infinity(&ge[0]); for (i = 0; i < runs; i++) { int j, k; secp256k1_ge g; @@ -4797,12 +4796,12 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi testutil_random_ge_test(&pt[ncount]); } - secp256k1_scalar_clear(&sc[0]); + secp256k1_scalar_set_int(&sc[0], 0); CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 20)); - secp256k1_scalar_clear(&sc[1]); - secp256k1_scalar_clear(&sc[2]); - secp256k1_scalar_clear(&sc[3]); - secp256k1_scalar_clear(&sc[4]); + secp256k1_scalar_set_int(&sc[1], 0); + secp256k1_scalar_set_int(&sc[2], 0); + secp256k1_scalar_set_int(&sc[3], 0); + secp256k1_scalar_set_int(&sc[4], 0); CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 6)); CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, 5)); CHECK(secp256k1_gej_is_infinity(&r)); From 9bb368d146683aa9b44645f1b75a0ed230df97e9 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sat, 8 Jun 2019 13:54:14 +0200 Subject: [PATCH 05/21] Use secp256k1_memclear() to clear stack memory instead of memset() All of the invocations of secp256k1_memclear() operate on stack memory and happen after the function is done with the memory object. This commit replaces existing memset() invocations and also adds secp256k1_memclear() to code locations where clearing was missing; there is no guarantee that this commit covers all code locations where clearing is necessary. Co-Authored-By: isle2983 --- src/ecmult_gen_impl.h | 8 ++++---- src/hash_impl.h | 11 ++++++----- src/modules/ecdh/main_impl.h | 5 +++-- src/modules/ellswift/main_impl.h | 2 +- src/modules/musig/session_impl.h | 10 +++++----- src/modules/schnorrsig/main_impl.h | 2 +- src/secp256k1.c | 4 ++-- src/util.h | 2 +- 8 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 2fbf623c..6903cd1b 100644 --- a/src/ecmult_gen_impl.h +++ b/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); - memset(&adds, 0, sizeof(adds)); - memset(&recoded, 0, sizeof(recoded)); + secp256k1_memclear(&adds, sizeof(adds)); + secp256k1_memclear(&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); - memset(keydata, 0, sizeof(keydata)); + secp256k1_memclear(keydata, sizeof(keydata)); /* Compute projective blinding factor (cannot be 0). */ secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); @@ -325,13 +325,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); - memset(nonce32, 0, 32); secp256k1_ecmult_gen(ctx, &gb, &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(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&b); secp256k1_gej_clear(&gb); secp256k1_fe_clear(&f); diff --git a/src/hash_impl.h b/src/hash_impl.h index 89f75ace..49393c67 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -156,6 +156,9 @@ static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out secp256k1_write_be32(&out32[4*i], hash->s[i]); hash->s[i] = 0; } + + secp256k1_memclear(sizedesc, sizeof(sizedesc)); + secp256k1_memclear(hash, sizeof(secp256k1_sha256)); } /* Initializes a sha256 struct and writes the 64 byte string @@ -196,7 +199,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const rkey[n] ^= 0x5c ^ 0x36; } secp256k1_sha256_write(&hash->inner, rkey, sizeof(rkey)); - memset(rkey, 0, sizeof(rkey)); + secp256k1_memclear(rkey, sizeof(rkey)); } static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size) { @@ -207,7 +210,7 @@ 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); - memset(temp, 0, 32); + secp256k1_memclear(temp, sizeof(temp)); secp256k1_sha256_finalize(&hash->outer, out32); } @@ -274,9 +277,7 @@ static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 } static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 *rng) { - memset(rng->k, 0, 32); - memset(rng->v, 0, 32); - rng->retry = 0; + secp256k1_memclear(rng, sizeof(secp256k1_rfc6979_hmac_sha256)); } #undef Round diff --git a/src/modules/ecdh/main_impl.h b/src/modules/ecdh/main_impl.h index 82b082a9..9286f70f 100644 --- a/src/modules/ecdh/main_impl.h +++ b/src/modules/ecdh/main_impl.h @@ -61,9 +61,10 @@ 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); return !!ret & !overflow; } diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h index b54ec08a..78e85004 100644 --- a/src/modules/ellswift/main_impl.h +++ b/src/modules/ellswift/main_impl.h @@ -580,7 +580,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); diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 2715b09d..e22d1a54 100644 --- a/src/modules/musig/session_impl.h +++ b/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_memclear(&sha_tmp, sizeof(sha_tmp)); } - memset(rand, 0, sizeof(rand)); - memset(&sha, 0, sizeof(sha)); + secp256k1_memclear(rand, sizeof(rand)); + secp256k1_memclear(&sha, sizeof(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) { @@ -509,7 +509,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; } diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 57f7eadd..783714fe 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -187,7 +187,7 @@ 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)); return ret; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 93b7b459..f4e83dc2 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -494,7 +494,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m buffer_append(keydata, &offset, algo16, 16); } secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, offset); - memset(keydata, 0, sizeof(keydata)); + secp256k1_memclear(keydata, sizeof(keydata)); for (i = 0; i <= counter; i++) { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); } @@ -548,7 +548,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; - memset(nonce32, 0, 32); + secp256k1_memclear(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); diff --git a/src/util.h b/src/util.h index c2add8ac..2605141d 100644 --- a/src/util.h +++ b/src/util.h @@ -285,7 +285,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. */ - acc = 0; + secp256k1_memclear(&acc, sizeof(acc)); return ret; } From 97c57f42ba8aca51479d23874b7608b9500a5bc9 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sat, 8 Jun 2019 11:29:49 +0200 Subject: [PATCH 06/21] Implement various _clear() functions with secp256k1_memclear() --- src/field.h | 1 - src/field_10x26_impl.h | 7 ------- src/field_5x52_impl.h | 7 ------- src/field_impl.h | 13 ++++--------- src/group_impl.h | 13 ++----------- src/scalar_4x64_impl.h | 7 ------- src/scalar_8x32_impl.h | 11 ----------- src/scalar_impl.h | 4 ++++ src/scalar_low_impl.h | 2 -- 9 files changed, 10 insertions(+), 55 deletions(-) diff --git a/src/field.h b/src/field.h index 95990309..1f6ba746 100644 --- a/src/field.h +++ b/src/field.h @@ -81,7 +81,6 @@ static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST( # define secp256k1_fe_normalizes_to_zero secp256k1_fe_impl_normalizes_to_zero # define secp256k1_fe_normalizes_to_zero_var secp256k1_fe_impl_normalizes_to_zero_var # define secp256k1_fe_set_int secp256k1_fe_impl_set_int -# define secp256k1_fe_clear secp256k1_fe_impl_clear # define secp256k1_fe_is_zero secp256k1_fe_impl_is_zero # define secp256k1_fe_is_odd secp256k1_fe_impl_is_odd # define secp256k1_fe_cmp_var secp256k1_fe_impl_cmp_var diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index 666068c7..ea14c273 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -270,13 +270,6 @@ SECP256K1_INLINE static int secp256k1_fe_impl_is_odd(const secp256k1_fe *a) { return a->n[0] & 1; } -SECP256K1_INLINE static void secp256k1_fe_impl_clear(secp256k1_fe *a) { - int i; - for (i=0; i<10; i++) { - a->n[i] = 0; - } -} - static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; for (i = 9; i >= 0; i--) { diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 76031f75..46dca6b9 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -212,13 +212,6 @@ SECP256K1_INLINE static int secp256k1_fe_impl_is_odd(const secp256k1_fe *a) { return a->n[0] & 1; } -SECP256K1_INLINE static void secp256k1_fe_impl_clear(secp256k1_fe *a) { - int i; - for (i=0; i<5; i++) { - a->n[i] = 0; - } -} - static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; for (i = 4; i >= 0; i--) { diff --git a/src/field_impl.h b/src/field_impl.h index 8cf6f22c..896507a3 100644 --- a/src/field_impl.h +++ b/src/field_impl.h @@ -18,6 +18,10 @@ #error "Please select wide multiplication implementation" #endif +SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { + secp256k1_memclear(a, sizeof(secp256k1_fe)); +} + SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { secp256k1_fe na; SECP256K1_FE_VERIFY(a); @@ -232,15 +236,6 @@ SECP256K1_INLINE static void secp256k1_fe_add_int(secp256k1_fe *r, int a) { SECP256K1_FE_VERIFY(r); } -static void secp256k1_fe_impl_clear(secp256k1_fe *a); -SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { - a->magnitude = 0; - a->normalized = 0; - secp256k1_fe_impl_clear(a); - - SECP256K1_FE_VERIFY(a); -} - static int secp256k1_fe_impl_is_zero(const secp256k1_fe *a); SECP256K1_INLINE static int secp256k1_fe_is_zero(const secp256k1_fe *a) { SECP256K1_FE_VERIFY(a); diff --git a/src/group_impl.h b/src/group_impl.h index 10aff433..c668fb24 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -299,20 +299,11 @@ static void secp256k1_ge_set_infinity(secp256k1_ge *r) { } static void secp256k1_gej_clear(secp256k1_gej *r) { - r->infinity = 0; - secp256k1_fe_clear(&r->x); - secp256k1_fe_clear(&r->y); - secp256k1_fe_clear(&r->z); - - SECP256K1_GEJ_VERIFY(r); + secp256k1_memclear(r, sizeof(secp256k1_gej)); } static void secp256k1_ge_clear(secp256k1_ge *r) { - r->infinity = 0; - secp256k1_fe_clear(&r->x); - secp256k1_fe_clear(&r->y); - - SECP256K1_GE_VERIFY(r); + secp256k1_memclear(r, sizeof(secp256k1_ge)); } static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) { diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 4aaec85b..807b9b70 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -29,13 +29,6 @@ #define SECP256K1_N_H_2 ((uint64_t)0xFFFFFFFFFFFFFFFFULL) #define SECP256K1_N_H_3 ((uint64_t)0x7FFFFFFFFFFFFFFFULL) -SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) { - r->d[0] = 0; - r->d[1] = 0; - r->d[2] = 0; - r->d[3] = 0; -} - SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsigned int v) { r->d[0] = v; r->d[1] = 0; diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h index c7d87b17..26104960 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -38,17 +38,6 @@ #define SECP256K1_N_H_6 ((uint32_t)0xFFFFFFFFUL) #define SECP256K1_N_H_7 ((uint32_t)0x7FFFFFFFUL) -SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) { - r->d[0] = 0; - r->d[1] = 0; - r->d[2] = 0; - r->d[3] = 0; - r->d[4] = 0; - r->d[5] = 0; - r->d[6] = 0; - r->d[7] = 0; -} - SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsigned int v) { r->d[0] = v; r->d[1] = 0; diff --git a/src/scalar_impl.h b/src/scalar_impl.h index 972d8041..dbb5b0a0 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -27,6 +27,10 @@ static const secp256k1_scalar secp256k1_scalar_one = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 1); 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)); +} + static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin) { int overflow; secp256k1_scalar_set_b32(r, bin, &overflow); diff --git a/src/scalar_low_impl.h b/src/scalar_low_impl.h index 45d2f3e4..84e1a380 100644 --- a/src/scalar_low_impl.h +++ b/src/scalar_low_impl.h @@ -19,8 +19,6 @@ SECP256K1_INLINE static int secp256k1_scalar_is_even(const secp256k1_scalar *a) return !(*a & 1); } -SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) { *r = 0; } - SECP256K1_INLINE static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsigned int v) { *r = v % EXHAUSTIVE_TEST_ORDER; From 99cc9fd6d01db1165d08b88d45c0de85a59b70cf Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sat, 8 Jun 2019 13:21:52 +0200 Subject: [PATCH 07/21] Don't rely on memset to set signed integers to 0 --- src/ecmult_impl.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 8f584552..0b53b3fc 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -171,7 +171,9 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a, VERIFY_CHECK(a != NULL); VERIFY_CHECK(2 <= w && w <= 31); - memset(wnaf, 0, len * sizeof(wnaf[0])); + for (bit = 0; bit < len; bit++) { + wnaf[bit] = 0; + } s = *a; if (secp256k1_scalar_get_bits_limb32(&s, 255, 1)) { @@ -179,6 +181,7 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a, sign = -1; } + bit = 0; while (bit < len) { int now; int word; From 349e6ab916b3dd106fea9b56dacec065df5fa457 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 27 Apr 2020 18:55:36 +0200 Subject: [PATCH 08/21] Introduce separate _clear functions for hash module This gives the caller more control about whether the state should be cleaned (= should be considered secret). Moreover, it gives the caller the possibility to clean a hash struct without finalizing it. --- src/ecmult_gen_impl.h | 1 + src/hash.h | 3 +++ src/hash_impl.h | 16 ++++++++++++---- src/modules/ecdh/main_impl.h | 1 + src/modules/ellswift/main_impl.h | 2 ++ src/modules/musig/session_impl.h | 4 ++-- src/modules/schnorrsig/main_impl.h | 1 + src/secp256k1.c | 5 ++++- 8 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 6903cd1b..070a1213 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -335,6 +335,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_scalar_clear(&b); secp256k1_gej_clear(&gb); secp256k1_fe_clear(&f); + secp256k1_rfc6979_hmac_sha256_clear(&rng); } #endif /* SECP256K1_ECMULT_GEN_IMPL_H */ diff --git a/src/hash.h b/src/hash.h index 4e0384cf..6d903ca7 100644 --- a/src/hash.h +++ b/src/hash.h @@ -19,6 +19,7 @@ typedef struct { static void secp256k1_sha256_initialize(secp256k1_sha256 *hash); static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *data, size_t size); static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out32); +static void secp256k1_sha256_clear(secp256k1_sha256 *hash); typedef struct { secp256k1_sha256 inner, outer; @@ -27,6 +28,7 @@ typedef struct { static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t size); static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size); static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned char *out32); +static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash); typedef struct { unsigned char v[32]; @@ -37,5 +39,6 @@ typedef struct { static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen); static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 *rng, unsigned char *out, size_t outlen); static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 *rng); +static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng); #endif /* SECP256K1_HASH_H */ diff --git a/src/hash_impl.h b/src/hash_impl.h index 49393c67..956e0ea4 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -156,9 +156,6 @@ static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out secp256k1_write_be32(&out32[4*i], hash->s[i]); hash->s[i] = 0; } - - secp256k1_memclear(sizedesc, sizeof(sizedesc)); - secp256k1_memclear(hash, sizeof(secp256k1_sha256)); } /* Initializes a sha256 struct and writes the 64 byte string @@ -174,6 +171,10 @@ static void secp256k1_sha256_initialize_tagged(secp256k1_sha256 *hash, const uns secp256k1_sha256_write(hash, buf, 32); } +static void secp256k1_sha256_clear(secp256k1_sha256 *hash) { + secp256k1_memclear(hash, sizeof(*hash)); +} + static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t keylen) { size_t n; unsigned char rkey[64]; @@ -214,6 +215,9 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned secp256k1_sha256_finalize(&hash->outer, out32); } +static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash) { + secp256k1_memclear(hash, sizeof(*hash)); +} static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen) { secp256k1_hmac_sha256 hmac; @@ -277,7 +281,11 @@ static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 } static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 *rng) { - secp256k1_memclear(rng, sizeof(secp256k1_rfc6979_hmac_sha256)); + (void) rng; +} + +static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng) { + secp256k1_memclear(rng, sizeof(*rng)); } #undef Round diff --git a/src/modules/ecdh/main_impl.h b/src/modules/ecdh/main_impl.h index 9286f70f..a3dc1833 100644 --- a/src/modules/ecdh/main_impl.h +++ b/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; } diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h index 78e85004..745a9691 100644 --- a/src/modules/ellswift/main_impl.h +++ b/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; } diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index e22d1a54..d646ec11 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -386,10 +386,10 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c /* Attempt to erase secret data */ secp256k1_memclear(buf, sizeof(buf)); - secp256k1_memclear(&sha_tmp, sizeof(sha_tmp)); + secp256k1_sha256_clear(&sha_tmp); } secp256k1_memclear(rand, sizeof(rand)); - secp256k1_memclear(&sha, sizeof(sha)); + 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) { diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 783714fe..261f4e4e 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/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; } diff --git a/src/secp256k1.c b/src/secp256k1.c index f4e83dc2..00c7285a 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -494,11 +494,13 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m buffer_append(keydata, &offset, algo16, 16); } secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, offset); - secp256k1_memclear(keydata, sizeof(keydata)); for (i = 0; i <= counter; i++) { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); } secp256k1_rfc6979_hmac_sha256_finalize(&rng); + + secp256k1_memclear(keydata, sizeof(keydata)); + secp256k1_rfc6979_hmac_sha256_clear(&rng); return 1; } @@ -799,6 +801,7 @@ int secp256k1_tagged_sha256(const secp256k1_context* ctx, unsigned char *hash32, secp256k1_sha256_initialize_tagged(&sha, tag, taglen); secp256k1_sha256_write(&sha, msg, msglen); secp256k1_sha256_finalize(&sha, hash32); + secp256k1_sha256_clear(&sha); return 1; } From 765ef53335a3e0fafdafe1e757f6fe0789f2797f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 11 Oct 2024 18:00:59 +0200 Subject: [PATCH 09/21] Clear _gej instances after point multiplication to avoid potential leaks Quoting sipa (see https://github.com/bitcoin-core/secp256k1/pull/1479#discussion_r1790079414): "When performing an EC multiplication A = aG for secret a, the resulting _affine_ coordinates of A are presumed to not leak information about a (ECDLP), but the same is not necessarily true for the Jacobian coordinates that come out of our multiplication algorithm." For the ECDH point multiplication result, the result in Jacobi coordinates should be cleared not only to avoid leaking the scalar, but even more so as it's a representation of the resulting shared secret. --- src/modules/ecdh/main_impl.h | 1 + src/modules/musig/session_impl.h | 1 + src/modules/schnorrsig/main_impl.h | 1 + src/secp256k1.c | 1 + 4 files changed, 4 insertions(+) diff --git a/src/modules/ecdh/main_impl.h b/src/modules/ecdh/main_impl.h index a3dc1833..842b5359 100644 --- a/src/modules/ecdh/main_impl.h +++ b/src/modules/ecdh/main_impl.h @@ -66,6 +66,7 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se secp256k1_memclear(y, sizeof(y)); secp256k1_scalar_clear(&s); secp256k1_ge_clear(&pt); + secp256k1_gej_clear(&res); return !!ret & !overflow; } diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index d646ec11..2733e47d 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -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)); secp256k1_scalar_clear(&k[i]); + secp256k1_gej_clear(&nonce_ptj); } /* None of the nonce_pts will be infinity because k != 0 with overwhelming * probability */ diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 261f4e4e..82bba2f5 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -189,6 +189,7 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi secp256k1_scalar_clear(&k); secp256k1_scalar_clear(&sk); secp256k1_memclear(seckey, sizeof(seckey)); + secp256k1_gej_clear(&rj); return ret; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 00c7285a..a248519d 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -597,6 +597,7 @@ static int secp256k1_ec_pubkey_create_helper(const secp256k1_ecmult_gen_context secp256k1_ecmult_gen(ecmult_gen_ctx, &pj, seckey_scalar); secp256k1_ge_set_gej(p, &pj); + secp256k1_gej_clear(&pj); return ret; } From 145868a84d28512fe0b3af83c1474b288261e720 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 1 Nov 2024 20:24:49 +0000 Subject: [PATCH 10/21] Do not export `secp256k1_musig_nonce_gen_internal` --- src/modules/musig/session_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 1251b378..dde38085 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -392,7 +392,7 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c 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) { +static 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) { secp256k1_scalar k[2]; secp256k1_ge nonce_pts[2]; int i; From a306bb7e903fef32bc7a5ea0f8144955168c7fca Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 4 Nov 2024 14:08:27 +0000 Subject: [PATCH 11/21] tools: fix check-abi.sh after cmake out locations were changed --- tools/check-abi.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tools/check-abi.sh b/tools/check-abi.sh index 94a98831..601a64ba 100755 --- a/tools/check-abi.sh +++ b/tools/check-abi.sh @@ -49,7 +49,14 @@ checkout_and_build() { -DSECP256K1_BUILD_CTIME_TESTS=OFF \ -DSECP256K1_BUILD_EXAMPLES=OFF cmake --build . -j "$(nproc)" - abi-dumper src/libsecp256k1.so -o ABI.dump -lver "$2" -public-headers ../include/ + # FIXME: Just set LIBPATH to lib/libsecp256k1.so once version 0.6.0 is + # released. + if [ -f "src/libsecp256k1.so" ]; then + LIBPATH="src/libsecp256k1.so" + else + LIBPATH="lib/libsecp256k1.so" + fi + abi-dumper $LIBPATH -o ABI.dump -lver "$2" -public-headers ../include/ cd "$_orig_dir" } From df2eceb2790fb63812955c1bd0ab9fe11c930584 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 4 Nov 2024 15:58:36 +0000 Subject: [PATCH 12/21] build: add ellswift.md and musig.md to release tarball --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index a4c8d33d..a95b4809 100644 --- a/Makefile.am +++ b/Makefile.am @@ -265,6 +265,7 @@ maintainer-clean-local: clean-testvectors ### Additional files to distribute EXTRA_DIST = autogen.sh CHANGELOG.md SECURITY.md EXTRA_DIST += doc/release-process.md doc/safegcd_implementation.md +EXTRA_DIST += doc/ellswift.md doc/musig.md EXTRA_DIST += examples/EXAMPLES_COPYING EXTRA_DIST += sage/gen_exhaustive_groups.sage EXTRA_DIST += sage/gen_split_lambda_constants.sage From 39d5dfd542a12bb1ad236c78e9fcc1f044998385 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 4 Nov 2024 13:45:03 +0000 Subject: [PATCH 13/21] release: prepare for 0.6.0 --- CHANGELOG.md | 16 +++++++++++++--- CMakeLists.txt | 8 ++++---- configure.ac | 12 ++++++------ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5026aab1..ee447c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,16 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.6.0] - 2024-11-04 #### Added - New module `musig` implements the MuSig2 multisignature scheme according to the [BIP 327 specification](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki). See: - Header file `include/secp256k1_musig.h` which defines the new API. - Document `doc/musig.md` for further notes on API usage. - Usage example `examples/musig.c`. + - New CMake variable `SECP256K1_APPEND_LDFLAGS` for appending linker flags to the build command. + +#### Changed + - API functions now use a significantly more robust method to clear secrets from the stack before returning. However, secret clearing remains a best-effort security measure and cannot guarantee complete removal. + - Any type `secp256k1_foo` can now be forward-declared using `typedef struct secp256k1_foo secp256k1_foo;` (or also `struct secp256k1_foo;` in C++). + - Organized CMake build artifacts into dedicated directories (`bin/` for executables, `lib/` for libraries) to improve build output structure and Windows shared library compatibility. #### Removed - - Removed the `secp256k1_scratch_space` struct and its associated functions `secp256k1_scratch_space_create` `secp256k1_scratch_space_destroy` because the scratch space was unused in the API. + - Removed the `secp256k1_scratch_space` struct and its associated functions `secp256k1_scratch_space_create` and `secp256k1_scratch_space_destroy` because the scratch space was unused in the API. + +#### ABI Compatibility +The symbols `secp256k1_scratch_space_create` and `secp256k1_scratch_space_destroy` were removed. +Otherwise, the library maintains backward compatibility with versions 0.3.x through 0.5.x. ## [0.5.1] - 2024-08-01 @@ -152,7 +162,7 @@ This version was in fact never released. The number was given by the build system since the introduction of autotools in Jan 2014 (ea0fe5a5bf0c04f9cc955b2966b614f5f378c6f6). Therefore, this version number does not uniquely identify a set of source files. -[unreleased]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.1...HEAD +[0.6.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.1...v0.6.0 [0.5.1]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.0...v0.5.1 [0.5.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.4.1...v0.5.0 [0.4.1]: https://github.com/bitcoin-core/secp256k1/compare/v0.4.0...v0.4.1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 84078101..041bfa3d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ project(libsecp256k1 # The package (a.k.a. release) version is based on semantic versioning 2.0.0 of # the API. All changes in experimental modules are treated as # backwards-compatible and therefore at most increase the minor version. - VERSION 0.5.2 + VERSION 0.6.0 DESCRIPTION "Optimized C library for ECDSA signatures and secret/public key operations on curve secp256k1." HOMEPAGE_URL "https://github.com/bitcoin-core/secp256k1" LANGUAGES C @@ -31,9 +31,9 @@ endif() # https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. -set(${PROJECT_NAME}_LIB_VERSION_CURRENT 4) -set(${PROJECT_NAME}_LIB_VERSION_REVISION 2) -set(${PROJECT_NAME}_LIB_VERSION_AGE 2) +set(${PROJECT_NAME}_LIB_VERSION_CURRENT 5) +set(${PROJECT_NAME}_LIB_VERSION_REVISION 0) +set(${PROJECT_NAME}_LIB_VERSION_AGE 0) #============================= # Language setup diff --git a/configure.ac b/configure.ac index dca1e662..f880a357 100644 --- a/configure.ac +++ b/configure.ac @@ -4,18 +4,18 @@ AC_PREREQ([2.60]) # the API. All changes in experimental modules are treated as # backwards-compatible and therefore at most increase the minor version. define(_PKG_VERSION_MAJOR, 0) -define(_PKG_VERSION_MINOR, 5) -define(_PKG_VERSION_PATCH, 2) -define(_PKG_VERSION_IS_RELEASE, false) +define(_PKG_VERSION_MINOR, 6) +define(_PKG_VERSION_PATCH, 0) +define(_PKG_VERSION_IS_RELEASE, true) # The library version is based on libtool versioning of the ABI. The set of # rules for updating the version can be found here: # https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. -define(_LIB_VERSION_CURRENT, 4) -define(_LIB_VERSION_REVISION, 2) -define(_LIB_VERSION_AGE, 2) +define(_LIB_VERSION_CURRENT, 5) +define(_LIB_VERSION_REVISION, 0) +define(_LIB_VERSION_AGE, 0) AC_INIT([libsecp256k1],m4_join([.], _PKG_VERSION_MAJOR, _PKG_VERSION_MINOR, _PKG_VERSION_PATCH)m4_if(_PKG_VERSION_IS_RELEASE, [true], [], [-dev]),[https://github.com/bitcoin-core/secp256k1/issues],[libsecp256k1],[https://github.com/bitcoin-core/secp256k1]) From c97059f5948a2d4070e0f19922e451079c749d78 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 4 Nov 2024 17:17:39 +0000 Subject: [PATCH 14/21] release cleanup: bump version after 0.6.0 --- CHANGELOG.md | 3 +++ CMakeLists.txt | 4 ++-- configure.ac | 6 +++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee447c0c..05222e5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + ## [0.6.0] - 2024-11-04 #### Added @@ -162,6 +164,7 @@ This version was in fact never released. The number was given by the build system since the introduction of autotools in Jan 2014 (ea0fe5a5bf0c04f9cc955b2966b614f5f378c6f6). Therefore, this version number does not uniquely identify a set of source files. +[unreleased]: https://github.com/bitcoin-core/secp256k1/compare/v0.6.0...HEAD [0.6.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.1...v0.6.0 [0.5.1]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.0...v0.5.1 [0.5.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.4.1...v0.5.0 diff --git a/CMakeLists.txt b/CMakeLists.txt index 041bfa3d..5d3c9061 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ project(libsecp256k1 # The package (a.k.a. release) version is based on semantic versioning 2.0.0 of # the API. All changes in experimental modules are treated as # backwards-compatible and therefore at most increase the minor version. - VERSION 0.6.0 + VERSION 0.6.1 DESCRIPTION "Optimized C library for ECDSA signatures and secret/public key operations on curve secp256k1." HOMEPAGE_URL "https://github.com/bitcoin-core/secp256k1" LANGUAGES C @@ -32,7 +32,7 @@ endif() # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. set(${PROJECT_NAME}_LIB_VERSION_CURRENT 5) -set(${PROJECT_NAME}_LIB_VERSION_REVISION 0) +set(${PROJECT_NAME}_LIB_VERSION_REVISION 1) set(${PROJECT_NAME}_LIB_VERSION_AGE 0) #============================= diff --git a/configure.ac b/configure.ac index f880a357..22a7ac0d 100644 --- a/configure.ac +++ b/configure.ac @@ -5,8 +5,8 @@ AC_PREREQ([2.60]) # backwards-compatible and therefore at most increase the minor version. define(_PKG_VERSION_MAJOR, 0) define(_PKG_VERSION_MINOR, 6) -define(_PKG_VERSION_PATCH, 0) -define(_PKG_VERSION_IS_RELEASE, true) +define(_PKG_VERSION_PATCH, 1) +define(_PKG_VERSION_IS_RELEASE, false) # The library version is based on libtool versioning of the ABI. The set of # rules for updating the version can be found here: @@ -14,7 +14,7 @@ define(_PKG_VERSION_IS_RELEASE, true) # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. define(_LIB_VERSION_CURRENT, 5) -define(_LIB_VERSION_REVISION, 0) +define(_LIB_VERSION_REVISION, 1) define(_LIB_VERSION_AGE, 0) AC_INIT([libsecp256k1],m4_join([.], _PKG_VERSION_MAJOR, _PKG_VERSION_MINOR, _PKG_VERSION_PATCH)m4_if(_PKG_VERSION_IS_RELEASE, [true], [], [-dev]),[https://github.com/bitcoin-core/secp256k1/issues],[libsecp256k1],[https://github.com/bitcoin-core/secp256k1]) From 39705450ebcfc7a2a8be540c868da9d2c44900cb Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Tue, 5 Nov 2024 16:44:29 +0100 Subject: [PATCH 15/21] Fix some misspellings Some files contained English misspellings or math issues (`lamba` instead of `lambda`). --- CMakeLists.txt | 2 +- configure.ac | 4 ++-- src/scalar_impl.h | 4 ++-- src/util.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d3c9061..aba6e512 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,7 +55,7 @@ option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL}) ## Modules # We declare all options before processing them, to make sure we can express -# dependendencies while processing. +# dependencies while processing. option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON) option(SECP256K1_ENABLE_MODULE_RECOVERY "Enable ECDSA pubkey recovery module." OFF) option(SECP256K1_ENABLE_MODULE_EXTRAKEYS "Enable extrakeys module." ON) diff --git a/configure.ac b/configure.ac index 22a7ac0d..c62a391d 100644 --- a/configure.ac +++ b/configure.ac @@ -254,8 +254,8 @@ fi print_msan_notice=no if test x"$enable_ctime_tests" = x"yes"; then SECP_MSAN_CHECK - # MSan on Clang >=16 reports unitialized memory in function parameters and return values, even if - # the uninitalized variable is never actually "used". This is called "eager" checking, and it's + # MSan on Clang >=16 reports uninitialized memory in function parameters and return values, even if + # the uninitialized variable is never actually "used". This is called "eager" checking, and it's # sounds like good idea for normal use of MSan. However, it yields many false positives in the # ctime_tests because many return values depend on secret (i.e., "uninitialized") values, and # we're only interested in detecting branches (which count as "uses") on secret data. diff --git a/src/scalar_impl.h b/src/scalar_impl.h index dbb5b0a0..0232a8c2 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -90,11 +90,11 @@ static void secp256k1_scalar_split_lambda_verify(const secp256k1_scalar *r1, con #endif /* - * Both lambda and beta are primitive cube roots of unity. That is lamba^3 == 1 mod n and + * Both lambda and beta are primitive cube roots of unity. That is lambda^3 == 1 mod n and * beta^3 == 1 mod p, where n is the curve order and p is the field order. * * Furthermore, because (X^3 - 1) = (X - 1)(X^2 + X + 1), the primitive cube roots of unity are - * roots of X^2 + X + 1. Therefore lambda^2 + lamba == -1 mod n and beta^2 + beta == -1 mod p. + * roots of X^2 + X + 1. Therefore lambda^2 + lambda == -1 mod n and beta^2 + beta == -1 mod p. * (The other primitive cube roots of unity are lambda^2 and beta^2 respectively.) * * Let l = -1/2 + i*sqrt(3)/2, the complex root of X^2 + X + 1. We can define a ring diff --git a/src/util.h b/src/util.h index 88a41494..5f29f407 100644 --- a/src/util.h +++ b/src/util.h @@ -232,7 +232,7 @@ static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) { * As best as we can tell, this is sufficient to break any optimisations that * might try to eliminate "superfluous" memsets. * This method is used in memzero_explicit() the Linux kernel, too. Its advantage is that it - * is pretty efficient, because the compiler can still implement the memset() efficently, + * is pretty efficient, because the compiler can still implement the memset() efficiently, * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by * Yang et al. (USENIX Security 2017) for more background. */ From 2ac9f558c43b5ef11f0d6e1088a8295a6d162659 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Tue, 26 Nov 2024 22:35:25 +0100 Subject: [PATCH 16/21] doc: Improve cmake instructions in README --- README.md | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 222e5fb7..6a311aa6 100644 --- a/README.md +++ b/README.md @@ -64,11 +64,11 @@ Implementation details Building with Autotools ----------------------- - $ ./autogen.sh - $ ./configure - $ make - $ make check # run the test suite - $ sudo make install # optional + $ ./autogen.sh # Generate a ./configure script + $ ./configure # Generate a build system + $ make # Run the actual build process + $ make check # Run the test suite + $ sudo make install # Install the library into the system (optional) To compile optional modules (such as Schnorr signatures), you need to run `./configure` with additional flags (such as `--enable-module-schnorrsig`). Run `./configure --help` to see the full list of available flags. @@ -79,24 +79,23 @@ To maintain a pristine source tree, CMake encourages to perform an out-of-source ### Building on POSIX systems - $ mkdir build && cd build - $ cmake .. - $ cmake --build . - $ ctest # run the test suite - $ sudo cmake --install . # optional + $ cmake -B build # Generate a build system in subdirectory "build" + $ cmake --build build # Run the actual build process + $ ctest --test-dir build # Run the test suite + $ sudo cmake --install build # Install the library into the system (optional) -To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake .. -LH` to see the full list of available flags. +To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake -B build -LH` or `ccmake -B build` to see the full list of available flags. ### Cross compiling To alleviate issues with cross compiling, preconfigured toolchain files are available in the `cmake` directory. For example, to cross compile for Windows: - $ cmake .. -DCMAKE_TOOLCHAIN_FILE=../cmake/x86_64-w64-mingw32.toolchain.cmake + $ cmake -B build -DCMAKE_TOOLCHAIN_FILE=cmake/x86_64-w64-mingw32.toolchain.cmake To cross compile for Android with [NDK](https://developer.android.com/ndk/guides/cmake) (using NDK's toolchain file, and assuming the `ANDROID_NDK_ROOT` environment variable has been set): - $ cmake .. -DCMAKE_TOOLCHAIN_FILE="${ANDROID_NDK_ROOT}/build/cmake/android.toolchain.cmake" -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=28 + $ cmake -B build -DCMAKE_TOOLCHAIN_FILE="${ANDROID_NDK_ROOT}/build/cmake/android.toolchain.cmake" -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=28 ### Building on Windows @@ -106,7 +105,7 @@ The following example assumes using of Visual Studio 2022 and CMake v3.21+. In "Developer Command Prompt for VS 2022": - >cmake -G "Visual Studio 17 2022" -A x64 -S . -B build + >cmake -G "Visual Studio 17 2022" -A x64 -B build >cmake --build build --config RelWithDebInfo Usage examples From a82287fb85746e0f7ea142a68a859e56ee86ee10 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 29 Jan 2025 03:59:32 +0100 Subject: [PATCH 17/21] schnorrsig: clear out masked secret key in BIP-340 nonce function --- src/modules/schnorrsig/main_impl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 82bba2f5..2ed7be67 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -94,6 +94,8 @@ 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)); + return 1; } From b682dbcf845d80f69c4e2d8243c90dd48006a4c1 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 11 Dec 2024 09:15:26 -0500 Subject: [PATCH 18/21] README: add instructions for verifying GPG signatures --- README.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/README.md b/README.md index 6a311aa6..f7a59b2b 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,42 @@ Implementation details * Optional runtime blinding which attempts to frustrate differential power analysis. * The precomputed tables add and eventually subtract points for which no known scalar (secret key) is known, preventing even an attacker with control over the secret key used to control the data internally. +Obtaining and verifying +----------------------- + +The git tag for each release (e.g. `v0.6.0`) is GPG-signed by one of the maintainers. +For a fully verified build of this project, it is recommended to obtain this repository +via git, obtain the GPG keys of the signing maintainer(s), and then verify the release +tag's signature using git. + +This can be done with the following steps: + +1. Obtain the GPG keys listed in [SECURITY.md](./SECURITY.md). +2. If possible, cross-reference these key IDs with another source controlled by its owner (e.g. + social media, personal website). This is to mitigate the unlikely case that incorrect + content is being presented by this repository. +3. Clone the repository: + ``` + git clone https://github.com/bitcoin-core/secp256k1 + ``` +4. Check out the latest release tag, e.g. + ``` + git checkout v0.6.0 + ``` +5. Use git to verify the GPG signature: + ``` + % git tag -v v0.6.0 | grep -C 3 'Good signature' + + gpg: Signature made Mon 04 Nov 2024 12:14:44 PM EST + gpg: using RSA key 4BBB845A6F5A65A69DFAEC234861DBF262123605 + gpg: Good signature from "Jonas Nick " [unknown] + gpg: aka "Jonas Nick " [unknown] + gpg: WARNING: This key is not certified with a trusted signature! + gpg: There is no indication that the signature belongs to the owner. + Primary key fingerprint: 36C7 1A37 C9D9 88BD E825 08D9 B1A7 0E4F 8DCD 0366 + Subkey fingerprint: 4BBB 845A 6F5A 65A6 9DFA EC23 4861 DBF2 6212 3605 + ``` + Building with Autotools ----------------------- From 965393fceadf8024c6b60dff59b2c30b61af28d2 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 14 Feb 2025 13:43:10 +0100 Subject: [PATCH 19/21] examples: use `EXIT_...` constants for `main` return values Fixes issue #1609. --- examples/ecdh.c | 9 +++++---- examples/ecdsa.c | 13 +++++++------ examples/ellswift.c | 11 ++++++----- examples/musig.c | 15 ++++++++------- examples/schnorr.c | 13 +++++++------ 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/examples/ecdh.c b/examples/ecdh.c index 13aa760b..67b8c204 100644 --- a/examples/ecdh.c +++ b/examples/ecdh.c @@ -8,6 +8,7 @@ *************************************************************************/ #include +#include #include #include @@ -33,7 +34,7 @@ int main(void) { secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); if (!fill_random(randomize, sizeof(randomize))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Randomizing the context is recommended to protect against side-channel * leakage See `secp256k1_context_randomize` in secp256k1.h for more @@ -44,14 +45,14 @@ int main(void) { /*** Key Generation ***/ if (!fill_random(seckey1, sizeof(seckey1)) || !fill_random(seckey2, sizeof(seckey2))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* If the secret key is zero or out of range (greater than secp256k1's * order), we fail. Note that the probability of this occurring is negligible * with a properly functioning random number generator. */ if (!secp256k1_ec_seckey_verify(ctx, seckey1) || !secp256k1_ec_seckey_verify(ctx, seckey2)) { printf("Generated secret key is invalid. This indicates an issue with the random number generator.\n"); - return 1; + return EXIT_FAILURE; } /* Public key creation using a valid context with a verified secret key should never fail */ @@ -116,5 +117,5 @@ int main(void) { secure_erase(shared_secret1, sizeof(shared_secret1)); secure_erase(shared_secret2, sizeof(shared_secret2)); - return 0; + return EXIT_SUCCESS; } diff --git a/examples/ecdsa.c b/examples/ecdsa.c index 80ae9d46..ae16c180 100644 --- a/examples/ecdsa.c +++ b/examples/ecdsa.c @@ -8,6 +8,7 @@ *************************************************************************/ #include +#include #include #include @@ -40,7 +41,7 @@ int main(void) { secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); if (!fill_random(randomize, sizeof(randomize))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Randomizing the context is recommended to protect against side-channel * leakage See `secp256k1_context_randomize` in secp256k1.h for more @@ -51,14 +52,14 @@ int main(void) { /*** Key Generation ***/ if (!fill_random(seckey, sizeof(seckey))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* If the secret key is zero or out of range (greater than secp256k1's * order), we fail. Note that the probability of this occurring is negligible * with a properly functioning random number generator. */ if (!secp256k1_ec_seckey_verify(ctx, seckey)) { printf("Generated secret key is invalid. This indicates an issue with the random number generator.\n"); - return 1; + return EXIT_FAILURE; } /* Public key creation using a valid context with a verified secret key should never fail */ @@ -92,13 +93,13 @@ int main(void) { /* Deserialize the signature. This will return 0 if the signature can't be parsed correctly. */ if (!secp256k1_ecdsa_signature_parse_compact(ctx, &sig, serialized_signature)) { printf("Failed parsing the signature\n"); - return 1; + return EXIT_FAILURE; } /* Deserialize the public key. This will return 0 if the public key can't be parsed correctly. */ if (!secp256k1_ec_pubkey_parse(ctx, &pubkey, compressed_pubkey, sizeof(compressed_pubkey))) { printf("Failed parsing the public key\n"); - return 1; + return EXIT_FAILURE; } /* Verify a signature. This will return 1 if it's valid and 0 if it's not. */ @@ -133,5 +134,5 @@ int main(void) { * will remove any writes that aren't used. */ secure_erase(seckey, sizeof(seckey)); - return 0; + return EXIT_SUCCESS; } diff --git a/examples/ellswift.c b/examples/ellswift.c index afb2fee4..d58e96b0 100644 --- a/examples/ellswift.c +++ b/examples/ellswift.c @@ -13,6 +13,7 @@ */ #include +#include #include #include @@ -38,7 +39,7 @@ int main(void) { ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); if (!fill_random(randomize, sizeof(randomize))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Randomizing the context is recommended to protect against side-channel * leakage. See `secp256k1_context_randomize` in secp256k1.h for more @@ -49,14 +50,14 @@ int main(void) { /*** Generate secret keys ***/ if (!fill_random(seckey1, sizeof(seckey1)) || !fill_random(seckey2, sizeof(seckey2))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* If the secret key is zero or out of range (greater than secp256k1's * order), we fail. Note that the probability of this occurring is negligible * with a properly functioning random number generator. */ if (!secp256k1_ec_seckey_verify(ctx, seckey1) || !secp256k1_ec_seckey_verify(ctx, seckey2)) { printf("Generated secret key is invalid. This indicates an issue with the random number generator.\n"); - return 1; + return EXIT_FAILURE; } /* Generate ElligatorSwift public keys. This should never fail with valid context and @@ -64,7 +65,7 @@ int main(void) { optional, but recommended. */ if (!fill_random(auxrand1, sizeof(auxrand1)) || !fill_random(auxrand2, sizeof(auxrand2))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } return_val = secp256k1_ellswift_create(ctx, ellswift_pubkey1, seckey1, auxrand1); assert(return_val); @@ -117,5 +118,5 @@ int main(void) { secure_erase(shared_secret1, sizeof(shared_secret1)); secure_erase(shared_secret2, sizeof(shared_secret2)); - return 0; + return EXIT_SUCCESS; } diff --git a/examples/musig.c b/examples/musig.c index 0352dc40..bdb8fced 100644 --- a/examples/musig.c +++ b/examples/musig.c @@ -12,6 +12,7 @@ */ #include +#include #include #include @@ -193,7 +194,7 @@ int main(void) { for (i = 0; i < N_SIGNERS; i++) { if (!create_keypair(ctx, &signer_secrets[i], &signers[i])) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } pubkeys_ptr[i] = &signers[i].pubkey; } @@ -208,7 +209,7 @@ int main(void) { fflush(stdout); if (!secp256k1_ec_pubkey_sort(ctx, pubkeys_ptr, N_SIGNERS)) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } printf("ok\n"); @@ -219,7 +220,7 @@ int main(void) { * while providing a non-NULL agg_pk argument. */ if (!secp256k1_musig_pubkey_agg(ctx, NULL, &cache, pubkeys_ptr, N_SIGNERS)) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } printf("ok\n"); printf("Tweaking................"); @@ -227,21 +228,21 @@ int main(void) { /* Optionally tweak the aggregate key */ if (!tweak(ctx, &agg_pk, &cache)) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } printf("ok\n"); printf("Signing message........."); fflush(stdout); if (!sign(ctx, signer_secrets, signers, &cache, msg, sig)) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } printf("ok\n"); printf("Verifying signature....."); fflush(stdout); if (!secp256k1_schnorrsig_verify(ctx, sig, msg, 32, &agg_pk)) { printf("FAILED\n"); - return 1; + return EXIT_FAILURE; } printf("ok\n"); @@ -256,5 +257,5 @@ int main(void) { secure_erase(&signer_secrets[i], sizeof(signer_secrets[i])); } secp256k1_context_destroy(ctx); - return 0; + return EXIT_SUCCESS; } diff --git a/examples/schnorr.c b/examples/schnorr.c index 909fcaa1..49baed24 100644 --- a/examples/schnorr.c +++ b/examples/schnorr.c @@ -8,6 +8,7 @@ *************************************************************************/ #include +#include #include #include @@ -34,7 +35,7 @@ int main(void) { secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); if (!fill_random(randomize, sizeof(randomize))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Randomizing the context is recommended to protect against side-channel * leakage See `secp256k1_context_randomize` in secp256k1.h for more @@ -45,7 +46,7 @@ int main(void) { /*** Key Generation ***/ if (!fill_random(seckey, sizeof(seckey))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Try to create a keypair with a valid context. This only fails if the * secret key is zero or out of range (greater than secp256k1's order). Note @@ -53,7 +54,7 @@ int main(void) { * functioning random number generator. */ if (!secp256k1_keypair_create(ctx, &keypair, seckey)) { printf("Generated secret key is invalid. This indicates an issue with the random number generator.\n"); - return 1; + return EXIT_FAILURE; } /* Extract the X-only public key from the keypair. We pass NULL for @@ -90,7 +91,7 @@ int main(void) { /* Generate 32 bytes of randomness to use with BIP-340 schnorr signing. */ if (!fill_random(auxiliary_rand, sizeof(auxiliary_rand))) { printf("Failed to generate randomness\n"); - return 1; + return EXIT_FAILURE; } /* Generate a Schnorr signature. @@ -110,7 +111,7 @@ int main(void) { * be parsed correctly */ if (!secp256k1_xonly_pubkey_parse(ctx, &pubkey, serialized_pubkey)) { printf("Failed parsing the public key\n"); - return 1; + return EXIT_FAILURE; } /* Compute the tagged hash on the received messages using the same tag as the signer. */ @@ -149,5 +150,5 @@ int main(void) { * Here we are preventing these writes from being optimized out, as any good compiler * will remove any writes that aren't used. */ secure_erase(seckey, sizeof(seckey)); - return 0; + return EXIT_SUCCESS; } From c8555817280d106a9f860462a4499f0eb1acd001 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 14 Feb 2025 13:43:29 +0100 Subject: [PATCH 20/21] test, bench, precompute_ecmult: use `EXIT_...` constants for `main` return values --- src/bench.c | 15 ++++++++------- src/bench.h | 2 +- src/bench_ecmult.c | 7 ++++--- src/bench_internal.c | 5 +++-- src/ctime_tests.c | 5 +++-- src/precompute_ecmult.c | 5 +++-- src/precompute_ecmult_gen.c | 5 +++-- src/tests.c | 2 +- src/tests_exhaustive.c | 4 ++-- 9 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/bench.c b/src/bench.c index 1127df67..149f25fc 100644 --- a/src/bench.c +++ b/src/bench.c @@ -5,6 +5,7 @@ ***********************************************************************/ #include +#include #include #include "../include/secp256k1.h" @@ -188,11 +189,11 @@ int main(int argc, char** argv) { || have_flag(argc, argv, "--help") || have_flag(argc, argv, "help")) { help(default_iters); - return 0; + return EXIT_SUCCESS; } else if (invalid_args) { fprintf(stderr, "./bench: unrecognized argument.\n\n"); help(default_iters); - return 1; + return EXIT_FAILURE; } } @@ -201,7 +202,7 @@ int main(int argc, char** argv) { if (have_flag(argc, argv, "ecdh")) { fprintf(stderr, "./bench: ECDH module not enabled.\n"); fprintf(stderr, "Use ./configure --enable-module-ecdh.\n\n"); - return 1; + return EXIT_FAILURE; } #endif @@ -209,7 +210,7 @@ int main(int argc, char** argv) { if (have_flag(argc, argv, "recover") || have_flag(argc, argv, "ecdsa_recover")) { fprintf(stderr, "./bench: Public key recovery module not enabled.\n"); fprintf(stderr, "Use ./configure --enable-module-recovery.\n\n"); - return 1; + return EXIT_FAILURE; } #endif @@ -217,7 +218,7 @@ int main(int argc, char** argv) { if (have_flag(argc, argv, "schnorrsig") || have_flag(argc, argv, "schnorrsig_sign") || have_flag(argc, argv, "schnorrsig_verify")) { fprintf(stderr, "./bench: Schnorr signatures module not enabled.\n"); fprintf(stderr, "Use ./configure --enable-module-schnorrsig.\n\n"); - return 1; + return EXIT_FAILURE; } #endif @@ -227,7 +228,7 @@ int main(int argc, char** argv) { have_flag(argc, argv, "ellswift_ecdh")) { fprintf(stderr, "./bench: ElligatorSwift module not enabled.\n"); fprintf(stderr, "Use ./configure --enable-module-ellswift.\n\n"); - return 1; + return EXIT_FAILURE; } #endif @@ -275,5 +276,5 @@ int main(int argc, char** argv) { run_ellswift_bench(iters, argc, argv); #endif - return 0; + return EXIT_SUCCESS; } diff --git a/src/bench.h b/src/bench.h index 1564b1a1..232fb35f 100644 --- a/src/bench.h +++ b/src/bench.h @@ -24,7 +24,7 @@ static int64_t gettime_i64(void) { struct timespec tv; if (!timespec_get(&tv, TIME_UTC)) { fputs("timespec_get failed!", stderr); - exit(1); + exit(EXIT_FAILURE); } return (int64_t)tv.tv_nsec / 1000 + (int64_t)tv.tv_sec * 1000000LL; #else diff --git a/src/bench_ecmult.c b/src/bench_ecmult.c index 3974af75..172292d5 100644 --- a/src/bench_ecmult.c +++ b/src/bench_ecmult.c @@ -4,6 +4,7 @@ * file COPYING or https://www.opensource.org/licenses/mit-license.php.* ***********************************************************************/ #include +#include #include "secp256k1.c" #include "../include/secp256k1.h" @@ -287,7 +288,7 @@ int main(int argc, char **argv) { || have_flag(argc, argv, "--help") || have_flag(argc, argv, "help")) { help(argv); - return 0; + return EXIT_SUCCESS; } else if(have_flag(argc, argv, "pippenger_wnaf")) { printf("Using pippenger_wnaf:\n"); data.ecmult_multi = secp256k1_ecmult_pippenger_batch_single; @@ -299,7 +300,7 @@ int main(int argc, char **argv) { } else { fprintf(stderr, "%s: unrecognized argument '%s'.\n\n", argv[0], argv[1]); help(argv); - return 1; + return EXIT_FAILURE; } } @@ -363,5 +364,5 @@ int main(int argc, char **argv) { free(data.output); free(data.expected_output); - return(0); + return EXIT_SUCCESS; } diff --git a/src/bench_internal.c b/src/bench_internal.c index a7006849..8688a4dc 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -4,6 +4,7 @@ * file COPYING or https://www.opensource.org/licenses/mit-license.php.* ***********************************************************************/ #include +#include #include "secp256k1.c" #include "../include/secp256k1.h" @@ -393,7 +394,7 @@ int main(int argc, char **argv) { || have_flag(argc, argv, "--help") || have_flag(argc, argv, "help")) { help(default_iters); - return 0; + return EXIT_SUCCESS; } } @@ -432,5 +433,5 @@ int main(int argc, char **argv) { if (d || have_flag(argc, argv, "context")) run_benchmark("context_create", bench_context, bench_setup, NULL, &data, 10, iters); - return 0; + return EXIT_SUCCESS; } diff --git a/src/ctime_tests.c b/src/ctime_tests.c index bbde863d..f81bdb92 100644 --- a/src/ctime_tests.c +++ b/src/ctime_tests.c @@ -5,6 +5,7 @@ ***********************************************************************/ #include +#include #include #include "../include/secp256k1.h" @@ -49,7 +50,7 @@ int main(void) { if (!SECP256K1_CHECKMEM_RUNNING()) { fprintf(stderr, "This test can only usefully be run inside valgrind because it was not compiled under msan.\n"); fprintf(stderr, "Usage: libtool --mode=execute valgrind ./ctime_tests\n"); - return 1; + return EXIT_FAILURE; } ctx = secp256k1_context_create(SECP256K1_CONTEXT_DECLASSIFY); /** In theory, testing with a single secret input should be sufficient: @@ -69,7 +70,7 @@ int main(void) { CHECK(ret); secp256k1_context_destroy(ctx); - return 0; + return EXIT_SUCCESS; } static void run_tests(secp256k1_context *ctx, unsigned char *key) { diff --git a/src/precompute_ecmult.c b/src/precompute_ecmult.c index 5ef198a7..021fe394 100644 --- a/src/precompute_ecmult.c +++ b/src/precompute_ecmult.c @@ -6,6 +6,7 @@ #include #include +#include #include "../include/secp256k1.h" @@ -62,7 +63,7 @@ int main(void) { fp = fopen(outfile, "w"); if (fp == NULL) { fprintf(stderr, "Could not open %s for writing!\n", outfile); - return -1; + return EXIT_FAILURE; } fprintf(fp, "/* This file was automatically generated by precompute_ecmult. */\n"); @@ -86,5 +87,5 @@ int main(void) { fprintf(fp, "#undef S\n"); fclose(fp); - return 0; + return EXIT_SUCCESS; } diff --git a/src/precompute_ecmult_gen.c b/src/precompute_ecmult_gen.c index 4d153a65..cd0fe70f 100644 --- a/src/precompute_ecmult_gen.c +++ b/src/precompute_ecmult_gen.c @@ -6,6 +6,7 @@ #include #include +#include #include "../include/secp256k1.h" @@ -64,7 +65,7 @@ int main(int argc, char **argv) { fp = fopen(outfile, "w"); if (fp == NULL) { fprintf(stderr, "Could not open %s for writing!\n", outfile); - return -1; + return EXIT_FAILURE; } fprintf(fp, "/* This file was automatically generated by precompute_ecmult_gen. */\n"); @@ -96,5 +97,5 @@ int main(int argc, char **argv) { fprintf(fp, "#undef S\n"); fclose(fp); - return 0; + return EXIT_SUCCESS; } diff --git a/src/tests.c b/src/tests.c index 78533b11..88628a14 100644 --- a/src/tests.c +++ b/src/tests.c @@ -7837,5 +7837,5 @@ int main(int argc, char **argv) { testrand_finish(); printf("no problems found\n"); - return 0; + return EXIT_SUCCESS; } diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index 6efa8898..f8bbcaaf 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -383,7 +383,7 @@ int main(int argc, char** argv) { this_core = strtol(argv[4], NULL, 0); if (num_cores < 1 || this_core >= num_cores) { fprintf(stderr, "Usage: %s [count] [seed] [numcores] [thiscore]\n", argv[0]); - return 1; + return EXIT_FAILURE; } printf("running tests for core %lu (out of [0..%lu])\n", (unsigned long)this_core, (unsigned long)num_cores - 1); } @@ -462,5 +462,5 @@ int main(int argc, char** argv) { testrand_finish(); printf("no problems found\n"); - return 0; + return EXIT_SUCCESS; } From 13d389629aba2c686798a856332c00c6934cacd5 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 14 Feb 2025 13:42:28 +0100 Subject: [PATCH 21/21] CONTRIBUTING: mention that `EXIT_` codes should be used --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a366d38b..80890fb7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,6 +74,7 @@ In addition, libsecp256k1 tries to maintain the following coding conventions: * User-facing comment lines in headers should be limited to 80 chars if possible. * All identifiers in file scope should start with `secp256k1_`. * Avoid trailing whitespace. +* Use the constants `EXIT_SUCCESS`/`EXIT_FAILURE` (defined in `stdlib.h`) to indicate program execution status for examples and other binaries. ### Tests