From 3b326847083025535e9acd0a7057ec7f59bc6ddf Mon Sep 17 00:00:00 2001 From: isle2983 Date: Tue, 14 Mar 2017 12:48:43 -0600 Subject: [PATCH 01/14] Use AC_USE_SYSTEM_EXTENSIONS to define _GNU_SOURCE in libsecp256k1-config.h In glibc 2.25, the new explicit_bzero() call is guarded by #ifdef __USE_MISC in string.h. Via glibc 2.25's features.h, the selectors for __USE_MISC are set under #ifdef _GNU_SOURCE. Without this set, the linker still figures out how to link explicit_bzero(), but it throws a bunch of warnings about a missing prototype for explicit_bzero(). By setting AC_USE_SYSTEM_EXTENSIONS, the system is detected and libsecp256k1-config.h is generated with the correct setting for _GNU_SOURCE It was found that even after this addition, the compile for the bench_internal executable was still throwing the warning. This was because the was included before libsec256k1-config.h (via including src/util.h) and confusing the preprocessing. It turns out that this include is redundant since it is included later from src/util.h in the correct order. --- configure.ac | 1 + src/bench_internal.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index e5fcbcb4ed..278524fd59 100644 --- a/configure.ac +++ b/configure.ac @@ -3,6 +3,7 @@ AC_INIT([libsecp256k1],[0.1]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_MACRO_DIR([build-aux/m4]) AC_CANONICAL_HOST +AC_USE_SYSTEM_EXTENSIONS AH_TOP([#ifndef LIBSECP256K1_CONFIG_H]) AH_TOP([#define LIBSECP256K1_CONFIG_H]) AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/]) diff --git a/src/bench_internal.c b/src/bench_internal.c index 0809f77bda..f46975223b 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -3,8 +3,6 @@ * Distributed under the MIT software license, see the accompanying * * file COPYING or http://www.opensource.org/licenses/mit-license.php.* **********************************************************************/ -#include - #include "include/secp256k1.h" #include "util.h" From fa474312e84cb76308b329d5f6e166c3bc100ecb Mon Sep 17 00:00:00 2001 From: isle2983 Date: Thu, 9 Mar 2017 15:25:59 -0700 Subject: [PATCH 02/14] Introduce SECP256K1_CLEANSE() explicit_bzero() is newly available in glibc 2.25 (released 2017-02-05; current stable). The fall-back implementation uses the 'volatile' keyword to signal the compiler to not optimize away the zeroing in the same way as explicit_bzero(). The wrapper macro SECP256K1_CLEANSE() borrows its name from OPENSSL_cleanse() in the openssl codebase which serves the same purpose and looks reasonably clear in code context. --- configure.ac | 2 ++ src/util.h | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/configure.ac b/configure.ac index 278524fd59..17994f8c44 100644 --- a/configure.ac +++ b/configure.ac @@ -154,6 +154,8 @@ AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto] AC_CHECK_TYPES([__int128]) +AC_CHECK_FUNCS([explicit_bzero]) + AC_MSG_CHECKING([for __builtin_expect]) AC_COMPILE_IFELSE([AC_LANG_SOURCE([[void myfunc() {__builtin_expect(0,0);}]])], [ AC_MSG_RESULT([yes]);AC_DEFINE(HAVE_BUILTIN_EXPECT,1,[Define this symbol if __builtin_expect is available]) ], diff --git a/src/util.h b/src/util.h index 4092a86c91..bd8ead9186 100644 --- a/src/util.h +++ b/src/util.h @@ -11,6 +11,7 @@ #include "libsecp256k1-config.h" #endif +#include #include #include #include @@ -76,6 +77,17 @@ static SECP256K1_INLINE void *checked_malloc(const secp256k1_callback* cb, size_ return ret; } +/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */ +#ifdef HAVE_EXPLICIT_BZERO +#define SECP256K1_CLEANSE(v) explicit_bzero(&v, sizeof(v)) +#else +static SECP256K1_INLINE void secp256k1_cleanse(void *r, size_t n) { + void *(*volatile const volatile_memset)(void *, int, size_t) = memset; + volatile_memset(r, 0, n); +} +#define SECP256K1_CLEANSE(v) secp256k1_cleanse(&(v), sizeof(v)) +#endif + /* Macro for restrict, when available and not in a VERIFY build. */ #if defined(SECP256K1_BUILD) && defined(VERIFY) # define SECP256K1_RESTRICT From 407cd76dcd0d260aa1e70bd2af97dca69dfef88b Mon Sep 17 00:00:00 2001 From: isle2983 Date: Thu, 9 Mar 2017 15:34:08 -0700 Subject: [PATCH 03/14] Replace secp256k1_scalar_clear() calls with SECP256K1_CLEANSE() --- src/ecdsa_impl.h | 2 +- src/ecmult_gen_impl.h | 6 +++--- src/modules/ecdh/main_impl.h | 2 +- src/modules/recovery/main_impl.h | 6 +++--- src/scalar.h | 3 --- src/scalar_4x64_impl.h | 7 ------- src/scalar_8x32_impl.h | 11 ----------- src/scalar_low_impl.h | 1 - src/secp256k1.c | 18 +++++++++--------- 9 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 453bb11880..4cd033168a 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -297,7 +297,7 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec secp256k1_scalar_add(&n, &n, message); secp256k1_scalar_inverse(sigs, nonce); secp256k1_scalar_mul(sigs, sigs, &n); - secp256k1_scalar_clear(&n); + SECP256K1_CLEANSE(n); secp256k1_gej_clear(&rp); secp256k1_ge_clear(&r); if (secp256k1_scalar_is_zero(sigs)) { diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 35f2546077..33358cabad 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -116,7 +116,7 @@ static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx #ifndef USE_ECMULT_STATIC_PRECOMPUTATION free(ctx->prec); #endif - secp256k1_scalar_clear(&ctx->blind); + SECP256K1_CLEANSE(ctx->blind); secp256k1_gej_clear(&ctx->initial); ctx->prec = NULL; } @@ -152,7 +152,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 } bits = 0; secp256k1_ge_clear(&add); - secp256k1_scalar_clear(&gnb); + SECP256K1_CLEANSE(gnb); } /* Setup blinding values for secp256k1_ecmult_gen. */ @@ -203,7 +203,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_scalar_negate(&b, &b); ctx->blind = b; ctx->initial = gb; - secp256k1_scalar_clear(&b); + SECP256K1_CLEANSE(b); secp256k1_gej_clear(&gb); } diff --git a/src/modules/ecdh/main_impl.h b/src/modules/ecdh/main_impl.h index 9e30fb73dd..2aa45b5f6e 100644 --- a/src/modules/ecdh/main_impl.h +++ b/src/modules/ecdh/main_impl.h @@ -47,7 +47,7 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *result, const se ret = 1; } - secp256k1_scalar_clear(&s); + SECP256K1_CLEANSE(s); return ret; } diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index c6fbe23981..de991e4a38 100755 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -155,9 +155,9 @@ int secp256k1_ecdsa_sign_recoverable(const secp256k1_context* ctx, secp256k1_ecd count++; } memset(nonce32, 0, 32); - secp256k1_scalar_clear(&msg); - secp256k1_scalar_clear(&non); - secp256k1_scalar_clear(&sec); + SECP256K1_CLEANSE(msg); + SECP256K1_CLEANSE(non); + SECP256K1_CLEANSE(sec); } if (ret) { secp256k1_ecdsa_recoverable_signature_save(signature, &r, &s, recid); diff --git a/src/scalar.h b/src/scalar.h index 27e9d8375e..a754013578 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -23,9 +23,6 @@ #error "Please select scalar implementation" #endif -/** Clear a scalar to prevent the leak of sensitive data. */ -static void secp256k1_scalar_clear(secp256k1_scalar *r); - /** Access bits from a scalar. All requested bits must belong to the same 32-bit limb. */ static unsigned int secp256k1_scalar_get_bits(const secp256k1_scalar *a, unsigned int offset, unsigned int count); diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 56e7bd82af..4b26623d44 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -24,13 +24,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 aae4f35c08..e3d81d5b6f 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -34,17 +34,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_low_impl.h b/src/scalar_low_impl.h index 4f94441f49..26e276e6d5 100644 --- a/src/scalar_low_impl.h +++ b/src/scalar_low_impl.h @@ -15,7 +15,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; } SECP256K1_INLINE static unsigned int secp256k1_scalar_get_bits(const secp256k1_scalar *a, unsigned int offset, unsigned int count) { diff --git a/src/secp256k1.c b/src/secp256k1.c index a709bea33d..0a4f51198c 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -376,9 +376,9 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature count++; } memset(nonce32, 0, 32); - secp256k1_scalar_clear(&msg); - secp256k1_scalar_clear(&non); - secp256k1_scalar_clear(&sec); + SECP256K1_CLEANSE(msg); + SECP256K1_CLEANSE(non); + SECP256K1_CLEANSE(sec); } if (ret) { secp256k1_ecdsa_signature_save(signature, &r, &s); @@ -397,7 +397,7 @@ int secp256k1_ec_seckey_verify(const secp256k1_context* ctx, const unsigned char secp256k1_scalar_set_b32(&sec, seckey, &overflow); ret = !overflow && !secp256k1_scalar_is_zero(&sec); - secp256k1_scalar_clear(&sec); + SECP256K1_CLEANSE(sec); return ret; } @@ -420,7 +420,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p secp256k1_ge_set_gej(&p, &pj); secp256k1_pubkey_save(pubkey, &p); } - secp256k1_scalar_clear(&sec); + SECP256K1_CLEANSE(sec); return ret; } @@ -469,8 +469,8 @@ int secp256k1_ec_privkey_tweak_add(const secp256k1_context* ctx, unsigned char * secp256k1_scalar_get_b32(seckey, &sec); } - secp256k1_scalar_clear(&sec); - secp256k1_scalar_clear(&term); + SECP256K1_CLEANSE(sec); + SECP256K1_CLEANSE(term); return ret; } @@ -515,8 +515,8 @@ int secp256k1_ec_privkey_tweak_mul(const secp256k1_context* ctx, unsigned char * secp256k1_scalar_get_b32(seckey, &sec); } - secp256k1_scalar_clear(&sec); - secp256k1_scalar_clear(&factor); + SECP256K1_CLEANSE(sec); + SECP256K1_CLEANSE(factor); return ret; } From d743f2fa97f04aa894e57d3f72d0d736130a092b Mon Sep 17 00:00:00 2001 From: isle2983 Date: Mon, 27 Mar 2017 13:24:54 -0600 Subject: [PATCH 04/14] Separate secp256k1_fe_set_zero() 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)zero() 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. --- src/ecmult_const_impl.h | 4 ++-- src/field.h | 3 +++ src/field_10x26_impl.h | 13 ++++++++++++- src/field_5x52_impl.h | 13 ++++++++++++- src/group_impl.h | 6 +++--- src/tests.c | 2 +- 6 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/ecmult_const_impl.h b/src/ecmult_const_impl.h index 0db314c48e..b8e412e6c1 100644 --- a/src/ecmult_const_impl.h +++ b/src/ecmult_const_impl.h @@ -28,8 +28,8 @@ VERIFY_CHECK(((n) & 1) == 1); \ VERIFY_CHECK((n) >= -((1 << ((w)-1)) - 1)); \ VERIFY_CHECK((n) <= ((1 << ((w)-1)) - 1)); \ - VERIFY_SETUP(secp256k1_fe_clear(&(r)->x)); \ - VERIFY_SETUP(secp256k1_fe_clear(&(r)->y)); \ + VERIFY_SETUP(secp256k1_fe_set_zero(&(r)->x)); \ + VERIFY_SETUP(secp256k1_fe_set_zero(&(r)->y)); \ for (m = 0; m < ECMULT_TABLE_SIZE(w); m++) { \ /* This loop is used to avoid secret data in array indices. See * the comment in ecmult_gen_impl.h for rationale. */ \ diff --git a/src/field.h b/src/field.h index bbb1ee866c..bdf952d4eb 100644 --- a/src/field.h +++ b/src/field.h @@ -53,6 +53,9 @@ static int secp256k1_fe_normalizes_to_zero_var(secp256k1_fe *r); static void secp256k1_fe_set_int(secp256k1_fe *r, int a); /** Sets a field element equal to zero, initializing all fields. */ +static void secp256k1_fe_set_zero(secp256k1_fe *a); + +/** Clear a field element to prevent leaking sensitive information. */ static void secp256k1_fe_clear(secp256k1_fe *a); /** Verify whether a field element is zero. Requires the input to be normalized. */ diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index 5fb092f1be..dd668c3b44 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -290,7 +290,7 @@ SECP256K1_INLINE static int secp256k1_fe_is_odd(const secp256k1_fe *a) { return a->n[0] & 1; } -SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { +SECP256K1_INLINE static void secp256k1_fe_set_zero(secp256k1_fe *a) { int i; #ifdef VERIFY a->magnitude = 0; @@ -301,6 +301,17 @@ SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { } } +SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { + int i; +#ifdef VERIFY + a->magnitude = 0; + a->normalized = 0; +#endif + for (i=0; i<10; i++) { + a->n[i] = 0; + } +} + static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; #ifdef VERIFY diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index dd88f38c77..06d36a6228 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -253,7 +253,7 @@ SECP256K1_INLINE static int secp256k1_fe_is_odd(const secp256k1_fe *a) { return a->n[0] & 1; } -SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { +SECP256K1_INLINE static void secp256k1_fe_set_zero(secp256k1_fe *a) { int i; #ifdef VERIFY a->magnitude = 0; @@ -264,6 +264,17 @@ SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { } } +SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { + int i; +#ifdef VERIFY + a->magnitude = 0; + a->normalized = 0; +#endif + for (i=0; i<5; i++) { + a->n[i] = 0; + } +} + static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; #ifdef VERIFY diff --git a/src/group_impl.h b/src/group_impl.h index 7d723532ff..d05d7ac6fc 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -195,9 +195,9 @@ static void secp256k1_ge_globalz_set_table_gej(size_t len, secp256k1_ge *r, secp 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_zero(&r->x); + secp256k1_fe_set_zero(&r->y); + secp256k1_fe_set_zero(&r->z); } static void secp256k1_gej_clear(secp256k1_gej *r) { diff --git a/src/tests.c b/src/tests.c index c05abb1e1d..fd256576b1 100644 --- a/src/tests.c +++ b/src/tests.c @@ -75,7 +75,7 @@ void random_field_element_magnitude(secp256k1_fe *fe) { if (n == 0) { return; } - secp256k1_fe_clear(&zero); + secp256k1_fe_set_zero(&zero); secp256k1_fe_negate(&zero, &zero, 0); secp256k1_fe_mul_int(&zero, n - 1); secp256k1_fe_add(fe, &zero); From e270ef017bb8051ce5045e2ec601b758fc829e0e Mon Sep 17 00:00:00 2001 From: isle2983 Date: Mon, 27 Mar 2017 13:12:42 -0600 Subject: [PATCH 05/14] Replace secp256k1_fe_clear() with SECP256K1_CLEANSE() --- src/ecmult_gen_impl.h | 2 +- src/field.h | 3 --- src/field_10x26_impl.h | 11 ----------- src/field_5x52_impl.h | 11 ----------- src/group_impl.h | 10 +++++----- 5 files changed, 6 insertions(+), 31 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 33358cabad..3ea172b3ff 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -190,7 +190,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const } while (retry); /* This branch true is cryptographically unreachable. Requires sha256_hmac output > Fp. */ /* Randomize the projection to defend against multiplier sidechannels. */ secp256k1_gej_rescale(&ctx->initial, &s); - secp256k1_fe_clear(&s); + SECP256K1_CLEANSE(s); do { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); secp256k1_scalar_set_b32(&b, nonce32, &retry); diff --git a/src/field.h b/src/field.h index bdf952d4eb..9b367e4a73 100644 --- a/src/field.h +++ b/src/field.h @@ -55,9 +55,6 @@ static void secp256k1_fe_set_int(secp256k1_fe *r, int a); /** Sets a field element equal to zero, initializing all fields. */ static void secp256k1_fe_set_zero(secp256k1_fe *a); -/** Clear a field element to prevent leaking sensitive information. */ -static void secp256k1_fe_clear(secp256k1_fe *a); - /** Verify whether a field element is zero. Requires the input to be normalized. */ static int secp256k1_fe_is_zero(const secp256k1_fe *a); diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index dd668c3b44..dd052ee7f0 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -301,17 +301,6 @@ SECP256K1_INLINE static void secp256k1_fe_set_zero(secp256k1_fe *a) { } } -SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { - int i; -#ifdef VERIFY - a->magnitude = 0; - a->normalized = 0; -#endif - for (i=0; i<10; i++) { - a->n[i] = 0; - } -} - static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; #ifdef VERIFY diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 06d36a6228..669a08200a 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -264,17 +264,6 @@ SECP256K1_INLINE static void secp256k1_fe_set_zero(secp256k1_fe *a) { } } -SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { - int i; -#ifdef VERIFY - a->magnitude = 0; - a->normalized = 0; -#endif - for (i=0; i<5; i++) { - a->n[i] = 0; - } -} - static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { int i; #ifdef VERIFY diff --git a/src/group_impl.h b/src/group_impl.h index d05d7ac6fc..3fcb0de74a 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -202,15 +202,15 @@ static void secp256k1_gej_set_infinity(secp256k1_gej *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_CLEANSE(r->x); + SECP256K1_CLEANSE(r->y); + SECP256K1_CLEANSE(r->z); } static void secp256k1_ge_clear(secp256k1_ge *r) { r->infinity = 0; - secp256k1_fe_clear(&r->x); - secp256k1_fe_clear(&r->y); + SECP256K1_CLEANSE(r->x); + SECP256K1_CLEANSE(r->y); } static int secp256k1_ge_set_xquad(secp256k1_ge *r, const secp256k1_fe *x) { From d056957095d8153d7b29bb2e30125e7341b763f1 Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sun, 26 Mar 2017 19:11:45 -0600 Subject: [PATCH 06/14] Setup test ge from the infinity gej ...rather than relying on zero assignment to set the initial state. Suggested by Pieter Wuille to avoid using secp256k1_ge_clear() for this initialization need. --- src/tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests.c b/src/tests.c index fd256576b1..e022c9b8ea 100644 --- a/src/tests.c +++ b/src/tests.c @@ -1898,7 +1898,7 @@ void test_ge(void) { secp256k1_fe zfi2, zfi3; secp256k1_gej_set_infinity(&gej[0]); - secp256k1_ge_clear(&ge[0]); + secp256k1_ge_set_gej(&ge[0], &gej[0]); secp256k1_ge_set_gej_var(&ge[0], &gej[0]); for (i = 0; i < runs; i++) { int j; From 6e8cb68a3a5ea853257156f56f8b2fb15667068e Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sun, 26 Mar 2017 19:20:05 -0600 Subject: [PATCH 07/14] Replace secp256k1_ge_clear with SECP256K1_CLEANSE() --- src/ecdsa_impl.h | 2 +- src/ecmult_gen_impl.h | 2 +- src/group.h | 3 --- src/group_impl.h | 6 ------ src/secp256k1.c | 2 +- 5 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 4cd033168a..f89feda9df 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -299,7 +299,7 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec secp256k1_scalar_mul(sigs, sigs, &n); SECP256K1_CLEANSE(n); secp256k1_gej_clear(&rp); - secp256k1_ge_clear(&r); + SECP256K1_CLEANSE(r); if (secp256k1_scalar_is_zero(sigs)) { return 0; } diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 3ea172b3ff..25d1005122 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -151,7 +151,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 secp256k1_gej_add_ge(r, r, &add); } bits = 0; - secp256k1_ge_clear(&add); + SECP256K1_CLEANSE(add); SECP256K1_CLEANSE(gnb); } diff --git a/src/group.h b/src/group.h index 4957b248fe..e7432a4eb0 100644 --- a/src/group.h +++ b/src/group.h @@ -126,9 +126,6 @@ static void secp256k1_ge_mul_lambda(secp256k1_ge *r, const secp256k1_ge *a); /** Clear a secp256k1_gej to prevent leaking sensitive information. */ static void secp256k1_gej_clear(secp256k1_gej *r); -/** Clear a secp256k1_ge to prevent leaking sensitive information. */ -static void secp256k1_ge_clear(secp256k1_ge *r); - /** Convert a group element to the storage type. */ static void secp256k1_ge_to_storage(secp256k1_ge_storage *r, const secp256k1_ge *a); diff --git a/src/group_impl.h b/src/group_impl.h index 3fcb0de74a..78739e1a00 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -207,12 +207,6 @@ static void secp256k1_gej_clear(secp256k1_gej *r) { SECP256K1_CLEANSE(r->z); } -static void secp256k1_ge_clear(secp256k1_ge *r) { - r->infinity = 0; - SECP256K1_CLEANSE(r->x); - SECP256K1_CLEANSE(r->y); -} - static int secp256k1_ge_set_xquad(secp256k1_ge *r, const secp256k1_fe *x) { secp256k1_fe x2, x3, c; r->x = *x; diff --git a/src/secp256k1.c b/src/secp256k1.c index 0a4f51198c..13a65d1921 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -158,7 +158,7 @@ int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pu return 0; } secp256k1_pubkey_save(pubkey, &Q); - secp256k1_ge_clear(&Q); + SECP256K1_CLEANSE(Q); return 1; } From ad225e8487b721b93d6bd8f180df2c5cc84439cd Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sun, 26 Mar 2017 19:25:35 -0600 Subject: [PATCH 08/14] Replace secp256k1_gej_clear() with SECP256K1_CLEANSE() --- src/ecdsa_impl.h | 2 +- src/ecmult_gen_impl.h | 4 ++-- src/group.h | 3 --- src/group_impl.h | 7 ------- 4 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index f89feda9df..3bfe465902 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -298,7 +298,7 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec secp256k1_scalar_inverse(sigs, nonce); secp256k1_scalar_mul(sigs, sigs, &n); SECP256K1_CLEANSE(n); - secp256k1_gej_clear(&rp); + SECP256K1_CLEANSE(rp); SECP256K1_CLEANSE(r); if (secp256k1_scalar_is_zero(sigs)) { return 0; diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 25d1005122..4eb54c32aa 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -117,7 +117,7 @@ static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx free(ctx->prec); #endif SECP256K1_CLEANSE(ctx->blind); - secp256k1_gej_clear(&ctx->initial); + SECP256K1_CLEANSE(ctx->initial); ctx->prec = NULL; } @@ -204,7 +204,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const ctx->blind = b; ctx->initial = gb; SECP256K1_CLEANSE(b); - secp256k1_gej_clear(&gb); + SECP256K1_CLEANSE(gb); } #endif diff --git a/src/group.h b/src/group.h index e7432a4eb0..6605893950 100644 --- a/src/group.h +++ b/src/group.h @@ -123,9 +123,6 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, static void secp256k1_ge_mul_lambda(secp256k1_ge *r, const secp256k1_ge *a); #endif -/** Clear a secp256k1_gej to prevent leaking sensitive information. */ -static void secp256k1_gej_clear(secp256k1_gej *r); - /** Convert a group element to the storage type. */ static void secp256k1_ge_to_storage(secp256k1_ge_storage *r, const secp256k1_ge *a); diff --git a/src/group_impl.h b/src/group_impl.h index 78739e1a00..fd4bae0b9f 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -200,13 +200,6 @@ static void secp256k1_gej_set_infinity(secp256k1_gej *r) { secp256k1_fe_set_zero(&r->z); } -static void secp256k1_gej_clear(secp256k1_gej *r) { - r->infinity = 0; - SECP256K1_CLEANSE(r->x); - SECP256K1_CLEANSE(r->y); - SECP256K1_CLEANSE(r->z); -} - static int secp256k1_ge_set_xquad(secp256k1_ge *r, const secp256k1_fe *x) { secp256k1_fe x2, x3, c; r->x = *x; From 059012ff039440372bed8d9d692110c3b0ffd226 Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 13:48:44 -0700 Subject: [PATCH 09/14] Split secp256k1_ecmult_context_teardown() out of secp256k1_ecmult_context_clear The 'teardown' naming is to complement the 'build' function name. To 'build' allocates and sets up the memory for pre_g and pre_g_128. To 'teardown' frees the memory. --- src/ecmult.h | 1 + src/ecmult_impl.h | 5 ++++- src/secp256k1.c | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ecmult.h b/src/ecmult.h index 20484134f5..c065c645e2 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -22,6 +22,7 @@ static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx); static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const secp256k1_callback *cb); static void secp256k1_ecmult_context_clone(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src, const secp256k1_callback *cb); +static void secp256k1_ecmult_context_teardown(secp256k1_ecmult_context *ctx); static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx); static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx); diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 4e40104ad4..a88dfb8647 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -217,11 +217,14 @@ static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx return ctx->pre_g != NULL; } -static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx) { +static void secp256k1_ecmult_context_teardown(secp256k1_ecmult_context *ctx) { free(ctx->pre_g); #ifdef USE_ENDOMORPHISM free(ctx->pre_g_128); #endif +} + +static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx) { secp256k1_ecmult_context_init(ctx); } diff --git a/src/secp256k1.c b/src/secp256k1.c index 13a65d1921..d8a1326b16 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -91,6 +91,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { void secp256k1_context_destroy(secp256k1_context* ctx) { if (ctx != NULL) { + secp256k1_ecmult_context_teardown(&ctx->ecmult_ctx); secp256k1_ecmult_context_clear(&ctx->ecmult_ctx); secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx); From 2f05e69300823a32c858aa2f7b0bd52142eb0e0a Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 13:52:41 -0700 Subject: [PATCH 10/14] Replace secp256k1_ecmult_context_clear() with SECP256K1_CLEANSE() --- src/ecmult.h | 1 - src/ecmult_impl.h | 4 ---- src/secp256k1.c | 2 +- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/ecmult.h b/src/ecmult.h index c065c645e2..1c03abefea 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -23,7 +23,6 @@ static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const static void secp256k1_ecmult_context_clone(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src, const secp256k1_callback *cb); static void secp256k1_ecmult_context_teardown(secp256k1_ecmult_context *ctx); -static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx); static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx); /** Double multiply: R = na*A + ng*G */ diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index a88dfb8647..22ff3d75a2 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -224,10 +224,6 @@ static void secp256k1_ecmult_context_teardown(secp256k1_ecmult_context *ctx) { #endif } -static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx) { - secp256k1_ecmult_context_init(ctx); -} - /** Convert a number to WNAF notation. The number becomes represented by sum(2^i * wnaf[i], i=0..bits), * with the following guarantees: * - each wnaf[i] is either 0, or an odd integer between -(1<<(w-1) - 1) and (1<<(w-1) - 1) diff --git a/src/secp256k1.c b/src/secp256k1.c index d8a1326b16..6f73a26b34 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -92,7 +92,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { void secp256k1_context_destroy(secp256k1_context* ctx) { if (ctx != NULL) { secp256k1_ecmult_context_teardown(&ctx->ecmult_ctx); - secp256k1_ecmult_context_clear(&ctx->ecmult_ctx); + SECP256K1_CLEANSE(ctx->ecmult_ctx); secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx); free(ctx); From 0edecfd377a3181fa86c3e71e6d45f559642dbd4 Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 14:04:47 -0700 Subject: [PATCH 11/14] Split secp256k1_ecmult_gen_context_teardown() out of secp256k1_ecmult_gen_context_clear The 'teardown' naming is to complement the 'build' function name. To 'build' allocates and sets up the memory for ctx->prec. To 'teardown' frees the memory. --- src/ecmult_gen.h | 3 +++ src/ecmult_gen_impl.h | 5 ++++- src/gen_context.c | 3 +++ src/secp256k1.c | 3 +++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h index eb2cc9ead6..f7ca2ed5ef 100644 --- a/src/ecmult_gen.h +++ b/src/ecmult_gen.h @@ -32,6 +32,9 @@ static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context* ctx) static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, const secp256k1_callback* cb); static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context* src, const secp256k1_callback* cb); +#ifndef USE_ECMULT_STATIC_PRECOMPUTATION +static void secp256k1_ecmult_gen_context_teardown(secp256k1_ecmult_gen_context* ctx); +#endif static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx); static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_context* ctx); diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 4eb54c32aa..b10639b212 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -112,10 +112,13 @@ static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst } } -static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) { #ifndef USE_ECMULT_STATIC_PRECOMPUTATION +static void secp256k1_ecmult_gen_context_teardown(secp256k1_ecmult_gen_context *ctx) { free(ctx->prec); +} #endif + +static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) { SECP256K1_CLEANSE(ctx->blind); SECP256K1_CLEANSE(ctx->initial); ctx->prec = NULL; diff --git a/src/gen_context.c b/src/gen_context.c index 1835fd491d..5af41d3460 100644 --- a/src/gen_context.c +++ b/src/gen_context.c @@ -64,6 +64,9 @@ int main(int argc, char **argv) { } } fprintf(fp,"};\n"); +#ifndef USE_ECMULT_STATIC_PRECOMPUTATION + secp256k1_ecmult_gen_context_teardown(&ctx); +#endif secp256k1_ecmult_gen_context_clear(&ctx); fprintf(fp, "#undef SC\n"); diff --git a/src/secp256k1.c b/src/secp256k1.c index 6f73a26b34..154a983e9b 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -93,6 +93,9 @@ void secp256k1_context_destroy(secp256k1_context* ctx) { if (ctx != NULL) { secp256k1_ecmult_context_teardown(&ctx->ecmult_ctx); SECP256K1_CLEANSE(ctx->ecmult_ctx); +#ifndef USE_ECMULT_STATIC_PRECOMPUTATION + secp256k1_ecmult_gen_context_teardown(&ctx->ecmult_gen_ctx); +#endif secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx); free(ctx); From 8feb9fb352796788920e1f658433d4ded6771964 Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 14:10:32 -0700 Subject: [PATCH 12/14] Replace secp256k1_ecmult_gen_context_clear() with SECP256K1_CLEANSE() --- src/ecmult_gen.h | 1 - src/ecmult_gen_impl.h | 6 ------ src/gen_context.c | 4 ++-- src/secp256k1.c | 2 +- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h index f7ca2ed5ef..2ca6bee9e2 100644 --- a/src/ecmult_gen.h +++ b/src/ecmult_gen.h @@ -35,7 +35,6 @@ static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst #ifndef USE_ECMULT_STATIC_PRECOMPUTATION static void secp256k1_ecmult_gen_context_teardown(secp256k1_ecmult_gen_context* ctx); #endif -static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx); static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_context* ctx); /** Multiply with the generator: R = a*G */ diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index b10639b212..62d82eefcf 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -118,12 +118,6 @@ static void secp256k1_ecmult_gen_context_teardown(secp256k1_ecmult_gen_context * } #endif -static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) { - SECP256K1_CLEANSE(ctx->blind); - SECP256K1_CLEANSE(ctx->initial); - ctx->prec = NULL; -} - static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp256k1_gej *r, const secp256k1_scalar *gn) { secp256k1_ge add; secp256k1_ge_storage adds; diff --git a/src/gen_context.c b/src/gen_context.c index 5af41d3460..4944c369e7 100644 --- a/src/gen_context.c +++ b/src/gen_context.c @@ -67,8 +67,8 @@ int main(int argc, char **argv) { #ifndef USE_ECMULT_STATIC_PRECOMPUTATION secp256k1_ecmult_gen_context_teardown(&ctx); #endif - secp256k1_ecmult_gen_context_clear(&ctx); - + SECP256K1_CLEANSE(ctx); + fprintf(fp, "#undef SC\n"); fprintf(fp, "#endif\n"); fclose(fp); diff --git a/src/secp256k1.c b/src/secp256k1.c index 154a983e9b..0280ae51ae 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -96,7 +96,7 @@ void secp256k1_context_destroy(secp256k1_context* ctx) { #ifndef USE_ECMULT_STATIC_PRECOMPUTATION secp256k1_ecmult_gen_context_teardown(&ctx->ecmult_gen_ctx); #endif - secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx); + SECP256K1_CLEANSE(ctx->ecmult_gen_ctx); free(ctx); } From 95824cd2e678b41b03f4410979328ff9f986625c Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 15:04:22 -0700 Subject: [PATCH 13/14] Cleanse 'bits' variable such that it doesn't get optimized away. Fixes clang static analysis issue: -------------------------------------------------------------------------------- An issue has been found in ./src/ecmult_gen_impl.h:150:5 Type: Dead assignment Description: Value stored to 'bits' is never read 0: ./src/ecmult_gen_impl.h:153:5 - Value stored to 'bits' is never read -------------------------------------------------------------------------------- --- src/ecmult_gen_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 62d82eefcf..cbe846324b 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -147,7 +147,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 secp256k1_ge_from_storage(&add, &adds); secp256k1_gej_add_ge(r, r, &add); } - bits = 0; + SECP256K1_CLEANSE(bits); SECP256K1_CLEANSE(add); SECP256K1_CLEANSE(gnb); } From 9acb45db560760635268deb71374e8d0c9aeff6d Mon Sep 17 00:00:00 2001 From: isle2983 Date: Sat, 11 Mar 2017 16:52:51 -0700 Subject: [PATCH 14/14] Use SECP256K1_CLEANSE() to zero stack memory instead of memset() All of these conversions: 1) operate on stack memory. 2) happen after the function is done with the variable 3) had an existing memset() action to be replaced These were found by visual inspection and may not be the total set of places where SECP256K1_CLEANSE should ideally be applied. --- src/ecmult_gen_impl.h | 4 ++-- src/hash_impl.h | 4 ++-- src/modules/recovery/main_impl.h | 2 +- src/num_gmp_impl.h | 12 ++++++------ src/secp256k1.c | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index cbe846324b..b4f6b2adb0 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -178,7 +178,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const memcpy(keydata + 32, seed32, 32); } secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, seed32 ? 64 : 32); - memset(keydata, 0, sizeof(keydata)); + SECP256K1_CLEANSE(keydata); /* Retry for out of range results to achieve uniformity. */ do { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); @@ -195,7 +195,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const retry |= secp256k1_scalar_is_zero(&b); } while (retry); /* This branch true is cryptographically unreachable. Requires sha256_hmac output > order. */ secp256k1_rfc6979_hmac_sha256_finalize(&rng); - memset(nonce32, 0, 32); + SECP256K1_CLEANSE(nonce32); secp256k1_ecmult_gen(ctx, &gb, &b); secp256k1_scalar_negate(&b, &b); ctx->blind = b; diff --git a/src/hash_impl.h b/src/hash_impl.h index b47e65f830..24e4809ed0 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -186,7 +186,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256_t *hash, cons rkey[n] ^= 0x5c ^ 0x36; } secp256k1_sha256_write(&hash->inner, rkey, 64); - memset(rkey, 0, 64); + SECP256K1_CLEANSE(rkey); } static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256_t *hash, const unsigned char *data, size_t size) { @@ -197,7 +197,7 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256_t *hash, unsign unsigned char temp[32]; secp256k1_sha256_finalize(&hash->inner, temp); secp256k1_sha256_write(&hash->outer, temp, 32); - memset(temp, 0, 32); + SECP256K1_CLEANSE(temp); secp256k1_sha256_finalize(&hash->outer, out32); } diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index de991e4a38..42ff470358 100755 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -154,7 +154,7 @@ int secp256k1_ecdsa_sign_recoverable(const secp256k1_context* ctx, secp256k1_ecd } count++; } - memset(nonce32, 0, 32); + SECP256K1_CLEANSE(nonce32); SECP256K1_CLEANSE(msg); SECP256K1_CLEANSE(non); SECP256K1_CLEANSE(sec); diff --git a/src/num_gmp_impl.h b/src/num_gmp_impl.h index 3a46495eea..f781f4473e 100644 --- a/src/num_gmp_impl.h +++ b/src/num_gmp_impl.h @@ -39,7 +39,7 @@ static void secp256k1_num_get_bin(unsigned char *r, unsigned int rlen, const sec if (len > shift) { memcpy(r + rlen - len + shift, tmp + shift, len - shift); } - memset(tmp, 0, sizeof(tmp)); + SECP256K1_CLEANSE(tmp); } static void secp256k1_num_set_bin(secp256k1_num *r, const unsigned char *a, unsigned int alen) { @@ -85,7 +85,7 @@ static void secp256k1_num_mod(secp256k1_num *r, const secp256k1_num *m) { if (r->limbs >= m->limbs) { mp_limb_t t[2*NUM_LIMBS]; mpn_tdiv_qr(t, r->data, 0, r->data, r->limbs, m->data, m->limbs); - memset(t, 0, sizeof(t)); + SECP256K1_CLEANSE(t); r->limbs = m->limbs; while (r->limbs > 1 && r->data[r->limbs-1]==0) { r->limbs--; @@ -139,9 +139,9 @@ static void secp256k1_num_mod_inverse(secp256k1_num *r, const secp256k1_num *a, } else { r->limbs = sn; } - memset(g, 0, sizeof(g)); - memset(u, 0, sizeof(u)); - memset(v, 0, sizeof(v)); + SECP256K1_CLEANSE(g); + SECP256K1_CLEANSE(u); + SECP256K1_CLEANSE(v); } static int secp256k1_num_jacobi(const secp256k1_num *a, const secp256k1_num *b) { @@ -256,7 +256,7 @@ static void secp256k1_num_mul(secp256k1_num *r, const secp256k1_num *a, const se VERIFY_CHECK(r->limbs <= 2*NUM_LIMBS); mpn_copyi(r->data, tmp, r->limbs); r->neg = a->neg ^ b->neg; - memset(tmp, 0, sizeof(tmp)); + SECP256K1_CLEANSE(tmp); } static void secp256k1_num_shift(secp256k1_num *r, int bits) { diff --git a/src/secp256k1.c b/src/secp256k1.c index 0280ae51ae..6a64b99a3e 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -335,7 +335,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m keylen += 16; } secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, keylen); - memset(keydata, 0, sizeof(keydata)); + SECP256K1_CLEANSE(keydata); for (i = 0; i <= counter; i++) { secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); } @@ -379,7 +379,7 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature } count++; } - memset(nonce32, 0, 32); + SECP256K1_CLEANSE(nonce32); SECP256K1_CLEANSE(msg); SECP256K1_CLEANSE(non); SECP256K1_CLEANSE(sec);