Merge bitcoin#708: Constant-time behaviour test using valgrind memtest.

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

Author: real-or-random

.gitignore

@@ -9,6 +9,7 @@ bench_internal
 tests
 exhaustive_tests
 gen_context
+valgrind_ctime_test
 *.exe
 *.so
 *.a

.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,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
@@ -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

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

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"

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 <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;
+}