Skip to content

Commit

Permalink
Merge bitcoin#708: Constant-time behaviour test using valgrind memtest.
Browse files Browse the repository at this point in the history
08fb6c4 Run valgrind_ctime_test in travis (Jonas Nick)
3d23022 Constant-time behaviour test using valgrind memtest. (Gregory Maxwell)

Pull request description:

  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

ACKs for top commit:
  sipa:
    ACK 08fb6c4
  real-or-random:
    ACK 08fb6c4
  jonasnick:
    ACK 08fb6c4

Tree-SHA512: d2eb829fb09f43ad1af70898e0eb9cf3f002c6bc418eca9e3e01a9c2c6e87c092aed23d6b0f311ddccbce1cce5f8ef39162cf9b2e68b83d160bc3d249e881493
  • Loading branch information
real-or-random committed Mar 3, 2020
2 parents 96d8ccb + 08fb6c4 commit e9fccd4
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ bench_internal
tests
exhaustive_tests
gen_context
valgrind_ctime_test
*.exe
*.so
*.a
Expand Down
21 changes: 19 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -39,25 +40,37 @@ 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:
apt:
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
Expand All @@ -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
6 changes: 6 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
100 changes: 100 additions & 0 deletions src/valgrind_ctime_test.c
Original file line number Diff line number Diff line change
@@ -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 <valgrind/memcheck.h>
#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;
}

0 comments on commit e9fccd4

Please sign in to comment.