diff --git a/.travis.yml b/.travis.yml index a1f247021a0ba..da4254cb9f8f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,9 @@ language: c os: linux addons: apt: - packages: libgmp-dev + packages: + - libgmp-dev + - valgrind compiler: - clang - gcc @@ -60,18 +62,10 @@ matrix: env: - BIGNUM=no ENDOMORPHISM=yes ASM=x86_64 EXPERIMENTAL=yes ECDH=yes RECOVERY=yes - VALGRIND=yes EXTRAFLAGS="--disable-openssl-tests CPPFLAGS=-DVALGRIND" BUILD= - addons: - apt: - packages: - - valgrind - compiler: gcc env: # The same as above but without endomorphism. - BIGNUM=no ENDOMORPHISM=no ASM=x86_64 EXPERIMENTAL=yes ECDH=yes RECOVERY=yes - VALGRIND=yes EXTRAFLAGS="--disable-openssl-tests CPPFLAGS=-DVALGRIND" BUILD= - addons: - apt: - packages: - - valgrind before_script: ./autogen.sh diff --git a/Makefile.am b/Makefile.am index de56754bb2111..79dcf6e2f7c87 100644 --- a/Makefile.am +++ b/Makefile.am @@ -69,6 +69,10 @@ libsecp256k1_la_SOURCES = src/secp256k1.c libsecp256k1_la_CPPFLAGS = -DSECP256K1_BUILD -I$(top_srcdir)/include -I$(top_srcdir)/src $(SECP_INCLUDES) libsecp256k1_la_LIBADD = $(SECP_LIBS) $(COMMON_LIB) +if VALGRIND_ENABLED +libsecp256k1_la_CPPFLAGS += -DVALGRIND +endif + noinst_PROGRAMS = if USE_BENCHMARK noinst_PROGRAMS += bench_verify bench_sign bench_internal bench_ecmult diff --git a/configure.ac b/configure.ac index 50210405cd345..a2023aba43611 100644 --- a/configure.ac +++ b/configure.ac @@ -170,6 +170,9 @@ AC_ARG_WITH([ecmult-gen-precision], [AS_HELP_STRING([--with-ecmult-gen-precision AC_CHECK_TYPES([__int128]) +AC_CHECK_HEADER([valgrind/memcheck.h], [enable_valgrind=yes], [enable_valgrind=no], []) +AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"]) + if test x"$enable_coverage" = x"yes"; then AC_DEFINE(COVERAGE, 1, [Define this symbol to compile out all VERIFY code]) CFLAGS="$CFLAGS -O0 --coverage" diff --git a/include/secp256k1.h b/include/secp256k1.h index 36020e5164677..1678406610e68 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -162,12 +162,14 @@ typedef int (*secp256k1_nonce_function)( /** The higher bits contain the actual data. Do not use directly. */ #define SECP256K1_FLAGS_BIT_CONTEXT_VERIFY (1 << 8) #define SECP256K1_FLAGS_BIT_CONTEXT_SIGN (1 << 9) +#define SECP256K1_FLAGS_BIT_CONTEXT_DECLASSIFY (1 << 10) #define SECP256K1_FLAGS_BIT_COMPRESSION (1 << 8) /** Flags to pass to secp256k1_context_create, secp256k1_context_preallocated_size, and * secp256k1_context_preallocated_create. */ #define SECP256K1_CONTEXT_VERIFY (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) #define SECP256K1_CONTEXT_SIGN (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_SIGN) +#define SECP256K1_CONTEXT_DECLASSIFY (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_DECLASSIFY) #define SECP256K1_CONTEXT_NONE (SECP256K1_FLAGS_TYPE_CONTEXT) /** Flag to pass to secp256k1_ec_pubkey_serialize. */ diff --git a/src/secp256k1.c b/src/secp256k1.c index de4eea36816fa..c10ac4cda546a 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -20,6 +20,10 @@ #include "hash_impl.h" #include "scratch_impl.h" +#if defined(VALGRIND) +# include +#endif + #define ARG_CHECK(cond) do { \ if (EXPECT(!(cond), 0)) { \ secp256k1_callback_call(&ctx->illegal_callback, #cond); \ @@ -66,13 +70,15 @@ struct secp256k1_context_struct { secp256k1_ecmult_gen_context ecmult_gen_ctx; secp256k1_callback illegal_callback; secp256k1_callback error_callback; + int declassify; }; static const secp256k1_context secp256k1_context_no_precomp_ = { { 0 }, { 0 }, { secp256k1_default_illegal_callback_fn, 0 }, - { secp256k1_default_error_callback_fn, 0 } + { secp256k1_default_error_callback_fn, 0 }, + 0 }; const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_precomp_; @@ -132,6 +138,7 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne if (flags & SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) { secp256k1_ecmult_context_build(&ret->ecmult_ctx, &prealloc); } + ret->declassify = !!(flags & SECP256K1_FLAGS_BIT_CONTEXT_DECLASSIFY); return (secp256k1_context*) ret; } @@ -215,6 +222,20 @@ void secp256k1_scratch_space_destroy(const secp256k1_context *ctx, secp256k1_scr secp256k1_scratch_destroy(&ctx->error_callback, scratch); } +/* Mark memory as no-longer-secret for the purpose of analysing constant-time behaviour + * of the software. This is setup for use with valgrind but could be substituted with + * the appropriate instrumentation for other analysis tools. + */ +static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx, void *p, size_t len) { +#if defined(VALGRIND) + if (EXPECT(ctx->declassify,0)) VALGRIND_MAKE_MEM_DEFINED(p, len); +#else + (void)ctx; + (void)p; + (void)len; +#endif +} + static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) { if (sizeof(secp256k1_ge_storage) == 64) { /* When the secp256k1_ge_storage type is exactly 64 byte, use its @@ -474,8 +495,14 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature break; } secp256k1_scalar_set_b32(&non, nonce32, &koverflow); - if (!koverflow && !secp256k1_scalar_is_zero(&non)) { - if (secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL)) { + koverflow |= secp256k1_scalar_is_zero(&non); + /* The nonce is still secret here, but it overflowing or being zero is is less likely than 1:2^255. */ + secp256k1_declassify(ctx, &koverflow, sizeof(koverflow)); + if (!koverflow) { + ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL); + /* The final signature is no longer a secret, nor is the fact that we were successful or not. */ + secp256k1_declassify(ctx, &ret, sizeof(ret)); + if (ret) { break; } }