From 3d2302257f19533932cd53547e9745b6283a907d Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Wed, 8 Jan 2020 11:56:15 +0000 Subject: [PATCH 1/2] Constant-time behaviour test using valgrind memtest. Valgrind does bit-level tracking of the "uninitialized" status of memory, property tracks memory which is tainted by any uninitialized memory, and warns if any branch or array access depends on an uninitialized bit. That is exactly the verification we need on secret data to test for constant-time behaviour. All we need to do is tell valgrind our secret key is actually uninitialized memory. This adds a valgrind_ctime_test which is compiled if valgrind is installed: Run it with libtool --mode=execute: $ libtool --mode=execute valgrind ./valgrind_ctime_test --- .gitignore | 1 + Makefile.am | 6 +++ configure.ac | 1 + src/valgrind_ctime_test.c | 100 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 src/valgrind_ctime_test.c diff --git a/.gitignore b/.gitignore index 55d325aeefa9c..cb4331aa90a2a 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ bench_internal tests exhaustive_tests gen_context +valgrind_ctime_test *.exe *.so *.a diff --git a/Makefile.am b/Makefile.am index 79dcf6e2f7c87..e73b1baf3815e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -93,6 +93,12 @@ if USE_TESTS noinst_PROGRAMS += tests tests_SOURCES = src/tests.c tests_CPPFLAGS = -DSECP256K1_BUILD -I$(top_srcdir)/src -I$(top_srcdir)/include $(SECP_INCLUDES) $(SECP_TEST_INCLUDES) +if VALGRIND_ENABLED +tests_CPPFLAGS += -DVALGRIND +noinst_PROGRAMS += valgrind_ctime_test +valgrind_ctime_test_SOURCES = src/valgrind_ctime_test.c +valgrind_ctime_test_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB) +endif if !ENABLE_COVERAGE tests_CPPFLAGS += -DVERIFY endif diff --git a/configure.ac b/configure.ac index a2023aba43611..e4929d6604c8c 100644 --- a/configure.ac +++ b/configure.ac @@ -556,6 +556,7 @@ echo " scalar = $set_scalar" echo " ecmult window size = $set_ecmult_window" echo " ecmult gen prec. bits = $set_ecmult_gen_precision" echo +echo " valgrind = $enable_valgrind" echo " CC = $CC" echo " CFLAGS = $CFLAGS" echo " CPPFLAGS = $CPPFLAGS" diff --git a/src/valgrind_ctime_test.c b/src/valgrind_ctime_test.c new file mode 100644 index 0000000000000..04c06d498fe58 --- /dev/null +++ b/src/valgrind_ctime_test.c @@ -0,0 +1,100 @@ +/********************************************************************** + * Copyright (c) 2020 Gregory Maxwell * + * 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" + +#if ENABLE_MODULE_ECDH +# include "include/secp256k1_ecdh.h" +#endif + +int main(void) { + secp256k1_context* ctx; + secp256k1_ecdsa_signature signature; + secp256k1_pubkey pubkey; + size_t siglen = 74; + size_t outputlen = 33; + int i; + int ret; + unsigned char msg[32]; + unsigned char key[32]; + unsigned char sig[74]; + unsigned char spubkey[33]; + + if (!RUNNING_ON_VALGRIND) { + fprintf(stderr, "This test can only usefully be run inside valgrind.\n"); + fprintf(stderr, "Usage: libtool --mode=execute valgrind ./valgrind_ctime_test\n"); + exit(1); + } + + /** In theory, testing with a single secret input should be sufficient: + * If control flow depended on secrets the tool would generate an error. + */ + for (i = 0; i < 32; i++) { + key[i] = i + 65; + } + for (i = 0; i < 32; i++) { + msg[i] = i + 1; + } + + ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_DECLASSIFY); + + /* Test keygen. */ + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_ec_pubkey_create(ctx, &pubkey, key); + VALGRIND_MAKE_MEM_DEFINED(&pubkey, sizeof(secp256k1_pubkey)); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret); + CHECK(secp256k1_ec_pubkey_serialize(ctx, spubkey, &outputlen, &pubkey, SECP256K1_EC_COMPRESSED) == 1); + + /* Test signing. */ + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_ecdsa_sign(ctx, &signature, msg, key, NULL, NULL); + VALGRIND_MAKE_MEM_DEFINED(&signature, sizeof(secp256k1_ecdsa_signature)); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret); + CHECK(secp256k1_ecdsa_signature_serialize_der(ctx, sig, &siglen, &signature)); + +#if ENABLE_MODULE_ECDH + /* Test ECDH. */ + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_ecdh(ctx, msg, &pubkey, key, NULL, NULL); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret == 1); +#endif + + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_ec_seckey_verify(ctx, key); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret == 1); + + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_ec_privkey_negate(ctx, key); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret == 1); + + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + VALGRIND_MAKE_MEM_UNDEFINED(msg, 32); + ret = secp256k1_ec_privkey_tweak_add(ctx, key, msg); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret == 1); + + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + VALGRIND_MAKE_MEM_UNDEFINED(msg, 32); + ret = secp256k1_ec_privkey_tweak_mul(ctx, key, msg); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret == 1); + + /* Test context randomisation. Do this last because it leaves the context tainted. */ + VALGRIND_MAKE_MEM_UNDEFINED(key, 32); + ret = secp256k1_context_randomize(ctx, key); + VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); + CHECK(ret); + + secp256k1_context_destroy(ctx); + return 0; +} From 08fb6c49261f8aefaaa8ea2ca6d84a53e037e812 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Wed, 12 Feb 2020 10:20:38 +0000 Subject: [PATCH 2/2] Run valgrind_ctime_test in travis --- .travis.yml | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index da4254cb9f8f2..ff4a6d2bc9663 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,12 +5,13 @@ addons: packages: - libgmp-dev - valgrind + - libtool-bin compiler: - clang - gcc env: global: - - FIELD=auto BIGNUM=auto SCALAR=auto ENDOMORPHISM=no STATICPRECOMPUTATION=yes ECMULTGENPRECISION=auto ASM=no BUILD=check EXTRAFLAGS= HOST= ECDH=no RECOVERY=no EXPERIMENTAL=no + - FIELD=auto BIGNUM=auto SCALAR=auto ENDOMORPHISM=no STATICPRECOMPUTATION=yes ECMULTGENPRECISION=auto ASM=no BUILD=check EXTRAFLAGS= HOST= ECDH=no RECOVERY=no EXPERIMENTAL=no CTIMETEST=yes matrix: - SCALAR=32bit RECOVERY=yes - SCALAR=32bit FIELD=32bit ECDH=yes EXPERIMENTAL=yes @@ -24,7 +25,7 @@ env: - BIGNUM=no - BIGNUM=no ENDOMORPHISM=yes RECOVERY=yes EXPERIMENTAL=yes - BIGNUM=no STATICPRECOMPUTATION=no - - BUILD=distcheck + - BUILD=distcheck CTIMETEST= - EXTRAFLAGS=CPPFLAGS=-DDETERMINISTIC - EXTRAFLAGS=CFLAGS=-O0 - ECMULTGENPRECISION=2 @@ -39,18 +40,27 @@ matrix: packages: - gcc-multilib - libgmp-dev:i386 + - valgrind + - libtool-bin + - libc6-dbg:i386 - compiler: clang env: HOST=i686-linux-gnu addons: apt: packages: - gcc-multilib + - valgrind + - libtool-bin + - libc6-dbg:i386 - compiler: gcc env: HOST=i686-linux-gnu ENDOMORPHISM=yes addons: apt: packages: - gcc-multilib + - valgrind + - libtool-bin + - libc6-dbg:i386 - compiler: gcc env: HOST=i686-linux-gnu addons: @@ -58,6 +68,9 @@ matrix: packages: - gcc-multilib - libgmp-dev:i386 + - valgrind + - libtool-bin + - libc6-dbg:i386 - compiler: gcc env: - BIGNUM=no ENDOMORPHISM=yes ASM=x86_64 EXPERIMENTAL=yes ECDH=yes RECOVERY=yes @@ -81,7 +94,11 @@ script: travis_wait 30 valgrind --error-exitcode=42 ./tests 16 && travis_wait 30 valgrind --error-exitcode=42 ./exhaustive_tests; fi + - if [ -n "$CTIMETEST" ]; then + libtool --mode=execute valgrind ./valgrind_ctime_test &> valgrind_ctime_test.log; + fi after_script: - cat ./tests.log - cat ./exhaustive_tests.log + - cat ./valgrind_ctime_test.log