Add support for msan instead of valgrind (for memcheck and ctime test)

.cirrus.yml

@@ -23,7 +23,7 @@ env:
   SECP256K1_TEST_ITERS:
   BENCH: yes
   SECP256K1_BENCH_ITERS: 2
-  CTIMETEST: yes
+  CTIMETESTS: yes
   # Compile and run the tests
   EXAMPLES: yes
 
@@ -40,8 +40,8 @@ cat_logs_snippet: &CAT_LOGS
       - cat noverify_tests.log || true
     cat_exhaustive_tests_log_script:
       - cat exhaustive_tests.log || true
-    cat_valgrind_ctime_test_log_script:
-      - cat valgrind_ctime_test.log || true
+    cat_ctime_tests_log_script:
+      - cat ctime_tests.log || true
     cat_bench_log_script:
       - cat bench.log || true
     cat_config_log_script:
@@ -81,9 +81,9 @@ task:
     - env: {WIDEMUL: int128,                 ECDH: yes, SCHNORRSIG: yes}
     - env: {WIDEMUL: int128,  ASM: x86_64}
     - env: {                  RECOVERY: yes,            SCHNORRSIG: yes}
-    - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
+    - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETESTS: no, BENCH: no}
     - env: {CPPFLAGS: -DDETERMINISTIC}
-    - env: {CFLAGS: -O0, CTIMETEST: no}
+    - env: {CFLAGS: -O0, CTIMETESTS: no}
     - env: { ECMULTGENPRECISION: 2, ECMULTWINDOW: 2 }
     - env: { ECMULTGENPRECISION: 8, ECMULTWINDOW: 4 }
   matrix:
@@ -128,7 +128,7 @@ task:
   env:
     ASM: no
     WITH_VALGRIND: no
-    CTIMETEST: no
+    CTIMETESTS: no
   matrix:
     - env:
         CC: gcc
@@ -153,7 +153,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   << : *MERGE_BASE
   test_script:
     # https://sourceware.org/bugzilla/show_bug.cgi?id=27008
@@ -172,7 +172,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   matrix:
     - env: {}
     - env: {EXPERIMENTAL: yes, ASM: arm}
@@ -192,7 +192,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   << : *MERGE_BASE
   test_script:
     - ./ci/cirrus.sh
@@ -209,7 +209,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   << : *MERGE_BASE
   test_script:
     - ./ci/cirrus.sh
@@ -223,7 +223,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   matrix:
     - name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
       env:
@@ -246,7 +246,7 @@ task:
     RECOVERY: yes
     EXPERIMENTAL: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
     # Use a MinGW-w64 host to tell ./configure we're building for Windows.
     # This will detect some MinGW-w64 tools but then make will need only
     # the MSVC tools CC, AR and NM as specified below.
@@ -285,7 +285,7 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: no
   matrix:
     - name: "Valgrind (memcheck)"
       container:
@@ -330,10 +330,11 @@ task:
     ECDH: yes
     RECOVERY: yes
     SCHNORRSIG: yes
-    CTIMETEST: no
+    CTIMETESTS: yes
     CC: clang
     SECP256K1_TEST_ITERS: 32
     ASM: no
+    WITH_VALGRIND: no
   container:
     memory: 2G
   matrix:

.gitignore

@@ -5,7 +5,7 @@ tests
 exhaustive_tests
 precompute_ecmult_gen
 precompute_ecmult
-valgrind_ctime_test
+ctime_tests
 ecdh_example
 ecdsa_example
 schnorr_example

Makefile.am

@@ -47,6 +47,7 @@ noinst_HEADERS += src/modinv64_impl.h
 noinst_HEADERS += src/precomputed_ecmult.h
 noinst_HEADERS += src/precomputed_ecmult_gen.h
 noinst_HEADERS += src/assumptions.h
+noinst_HEADERS += src/checkmem.h
 noinst_HEADERS += src/util.h
 noinst_HEADERS += src/int128.h
 noinst_HEADERS += src/int128_impl.h
@@ -96,10 +97,6 @@ libsecp256k1_la_CPPFLAGS = $(SECP_INCLUDES) $(SECP_CONFIG_DEFINES)
 libsecp256k1_la_LIBADD = $(SECP_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
 libsecp256k1_la_LDFLAGS = -no-undefined -version-info $(LIB_VERSION_CURRENT):$(LIB_VERSION_REVISION):$(LIB_VERSION_AGE)
 
-if VALGRIND_ENABLED
-libsecp256k1_la_CPPFLAGS += -DVALGRIND
-endif
-
 noinst_PROGRAMS =
 if USE_BENCHMARK
 noinst_PROGRAMS += bench bench_internal bench_ecmult
@@ -122,12 +119,6 @@ noverify_tests_SOURCES = src/tests.c
 noverify_tests_CPPFLAGS = $(SECP_INCLUDES) $(SECP_TEST_INCLUDES) $(SECP_CONFIG_DEFINES)
 noverify_tests_LDADD = $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
 noverify_tests_LDFLAGS = -static
-if VALGRIND_ENABLED
-noverify_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) $(COMMON_LIB)
-endif
 if !ENABLE_COVERAGE
 TESTS += tests
 noinst_PROGRAMS += tests
@@ -138,6 +129,13 @@ tests_LDFLAGS  = $(noverify_tests_LDFLAGS)
 endif
 endif
 
+if USE_CTIME_TESTS
+noinst_PROGRAMS += ctime_tests
+ctime_tests_SOURCES = src/ctime_tests.c
+ctime_tests_LDADD = libsecp256k1.la $(SECP_LIBS) $(COMMON_LIB)
+ctime_tests_CPPFLAGS = $(SECP_CONFIG_DEFINES)
+endif
+
 if USE_EXHAUSTIVE_TESTS
 noinst_PROGRAMS += exhaustive_tests
 exhaustive_tests_SOURCES = src/tests_exhaustive.c

ci/cirrus.sh

@@ -13,7 +13,7 @@ print_environment() {
     for var in WERROR_CFLAGS MAKEFLAGS BUILD \
             ECMULTWINDOW ECMULTGENPRECISION ASM WIDEMUL WITH_VALGRIND EXTRAFLAGS \
             EXPERIMENTAL ECDH RECOVERY SCHNORRSIG \
-            SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETEST\
+            SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS\
             EXAMPLES \
             HOST WRAPPER_CMD \
             CC CFLAGS CPPFLAGS AR NM
@@ -62,6 +62,7 @@ fi
     --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
     --enable-module-schnorrsig="$SCHNORRSIG" \
     --enable-examples="$EXAMPLES" \
+    --enable-ctime-tests="$CTIMETESTS" \
     --with-valgrind="$WITH_VALGRIND" \
     --host="$HOST" $EXTRAFLAGS
 
@@ -78,24 +79,29 @@ export LOG_COMPILER="$WRAPPER_CMD"
 
 make "$BUILD"
 
+# Using the local `libtool` because on macOS the system's libtool has nothing to do with GNU libtool
+EXEC='./libtool --mode=execute'
+if [ -n "$WRAPPER_CMD" ]
+then
+    EXEC="$EXEC $WRAPPER_CMD"
+fi
+
 if [ "$BENCH" = "yes" ]
 then
-    # Using the local `libtool` because on macOS the system's libtool has nothing to do with GNU libtool
-    EXEC='./libtool --mode=execute'
-    if [ -n "$WRAPPER_CMD" ]
-    then
-        EXEC="$EXEC $WRAPPER_CMD"
-    fi
     {
         $EXEC ./bench_ecmult
         $EXEC ./bench_internal
         $EXEC ./bench
     } >> bench.log 2>&1
 fi
 
-if [ "$CTIMETEST" = "yes" ]
+if [ "$CTIMETESTS" = "yes" ]
 then
-    ./libtool --mode=execute valgrind --error-exitcode=42 ./valgrind_ctime_test > valgrind_ctime_test.log 2>&1
+    if [ "$WITH_VALGRIND" = "yes" ]; then
+        ./libtool --mode=execute valgrind --error-exitcode=42 ./ctime_tests > ctime_tests.log 2>&1
+    else
+        $EXEC ./ctime_tests > ctime_tests.log 2>&1
+    fi
 fi
 
 # Rebuild precomputed files (if not cross-compiling).

configure.ac

@@ -142,6 +142,10 @@ AC_ARG_ENABLE(tests,
     AS_HELP_STRING([--enable-tests],[compile tests [default=yes]]), [],
     [SECP_SET_DEFAULT([enable_tests], [yes], [yes])])
 
+AC_ARG_ENABLE(ctime_tests,
+    AS_HELP_STRING([--enable-ctime-tests],[compile constant-time tests [default=yes if valgrind enabled]]), [],
+    [SECP_SET_DEFAULT([enable_ctime_tests], [auto], [auto])])
+
 AC_ARG_ENABLE(experimental,
     AS_HELP_STRING([--enable-experimental],[allow experimental configure options [default=no]]), [],
     [SECP_SET_DEFAULT([enable_experimental], [no], [yes])])
@@ -225,7 +229,10 @@ else
     enable_valgrind=yes
   fi
 fi
-AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"])
+
+if test x"$enable_ctime_tests" = x"auto"; then
+    enable_ctime_tests=$enable_valgrind
+fi
 
 if test x"$enable_coverage" = x"yes"; then
     SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOVERAGE=1"
@@ -344,7 +351,7 @@ case $set_ecmult_gen_precision in
 esac
 
 if test x"$enable_valgrind" = x"yes"; then
-  SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
+  SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES $VALGRIND_CPPFLAGS -DVALGRIND"
 fi
 
 # Add -Werror and similar flags passed from the outside (for testing, e.g., in CI).
@@ -407,6 +414,7 @@ AC_SUBST(SECP_CFLAGS)
 AC_SUBST(SECP_CONFIG_DEFINES)
 AM_CONDITIONAL([ENABLE_COVERAGE], [test x"$enable_coverage" = x"yes"])
 AM_CONDITIONAL([USE_TESTS], [test x"$enable_tests" != x"no"])
+AM_CONDITIONAL([USE_CTIME_TESTS], [test x"$enable_ctime_tests" = x"yes"])
 AM_CONDITIONAL([USE_EXHAUSTIVE_TESTS], [test x"$enable_exhaustive_tests" != x"no"])
 AM_CONDITIONAL([USE_EXAMPLES], [test x"$enable_examples" != x"no"])
 AM_CONDITIONAL([USE_BENCHMARK], [test x"$enable_benchmark" = x"yes"])
@@ -428,6 +436,7 @@ echo "Build Options:"
 echo "  with external callbacks = $enable_external_default_callbacks"
 echo "  with benchmarks         = $enable_benchmark"
 echo "  with tests              = $enable_tests"
+echo "  with ctime tests        = $enable_ctime_tests"
 echo "  with coverage           = $enable_coverage"
 echo "  with examples           = $enable_examples"
 echo "  module ecdh             = $enable_module_ecdh"

doc/safegcd_implementation.md

@@ -410,7 +410,7 @@ sufficient even. Given that every loop iteration performs *N* divsteps, it will
 
 To deal with the branches in `divsteps_n_matrix` we will replace them with constant-time bitwise
 operations (and hope the C compiler isn't smart enough to turn them back into branches; see
-`valgrind_ctime_test.c` for automated tests that this isn't the case). To do so, observe that a
+`ctime_tests.c` for automated tests that this isn't the case). To do so, observe that a
 divstep can be written instead as (compare to the inner loop of `gcd` in section 1).
 
 ```python

src/checkmem.h

@@ -0,0 +1,88 @@
+/***********************************************************************
+ * Copyright (c) 2022 Pieter Wuille                                    *
+ * Distributed under the MIT software license, see the accompanying    *
+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
+ ***********************************************************************/
+
+/* The code here is inspired by Kris Kwiatkowski's approach in
+ * https://github.com/kriskwiatkowski/pqc/blob/main/src/common/ct_check.h
+ * to provide a general interface for memory-checking mechanisms, primarily
+ * for constant-time checking.
+ */
+
+/* These macros are defined by this header file:
+ *
+ * - SECP256K1_CHECKMEM_ENABLED:
+ *   - 1 if memory-checking integration is available, 0 otherwise.
+ *     This is just a compile-time macro. Use the next macro to check it is actually
+ *     available at runtime.
+ * - SECP256K1_CHECKMEM_RUNNING():
+ *   - Acts like a function call, returning 1 if memory checking is available
+ *     at runtime.
+ * - SECP256K1_CHECKMEM_CHECK(p, len):
+ *   - Assert or otherwise fail in case the len-byte memory block pointed to by p is
+ *     not considered entirely defined.
+ * - SECP256K1_CHECKMEM_CHECK_VERIFY(p, len):
+ *   - Like SECP256K1_CHECKMEM_CHECK, but only works in VERIFY mode.
+ * - SECP256K1_CHECKMEM_UNDEFINE(p, len):
+ *   - marks the len-byte memory block pointed to by p as undefined data (secret data,
+ *     in the context of constant-time checking).
+ * - SECP256K1_CHECKMEM_DEFINE(p, len):
+ *   - marks the len-byte memory pointed to by p as defined data (public data, in the
+ *     context of constant-time checking).
+ *
+ */
+
+#ifndef SECP256K1_CHECKMEM_H
+#define SECP256K1_CHECKMEM_H
+
+/* Define a statement-like macro that ignores the arguments. */
+#define SECP256K1_CHECKMEM_NOOP(p, len) do { (void)(p); (void)(len); } while(0)
+
+/* If compiling under msan, map the SECP256K1_CHECKMEM_* functionality to msan.
+ * Choose this preferentially, even when VALGRIND is defined, as msan-compiled
+ * binaries can't be run under valgrind anyway. */
+#if defined(__has_feature)
+#  if __has_feature(memory_sanitizer)
+#    include <sanitizer/msan_interface.h>
+#    define SECP256K1_CHECKMEM_ENABLED 1
+#    define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len))
+#    define SECP256K1_CHECKMEM_DEFINE(p, len) __msan_unpoison((p), (len))
+#    define SECP256K1_CHECKMEM_CHECK(p, len) __msan_check_mem_is_initialized((p), (len))
+#    define SECP256K1_CHECKMEM_RUNNING() (1)
+#  endif
+#endif
+
+/* If valgrind integration is desired (through the VALGRIND define), implement the
+ * SECP256K1_CHECKMEM_* macros using valgrind. */
+#if !defined SECP256K1_CHECKMEM_ENABLED
+#  if defined VALGRIND
+#    include <stddef.h>
+#    include <valgrind/memcheck.h>
+#    define SECP256K1_CHECKMEM_ENABLED 1
+#    define SECP256K1_CHECKMEM_UNDEFINE(p, len) VALGRIND_MAKE_MEM_UNDEFINED((p), (len))
+#    define SECP256K1_CHECKMEM_DEFINE(p, len) VALGRIND_MAKE_MEM_DEFINED((p), (len))
+#    define SECP256K1_CHECKMEM_CHECK(p, len) VALGRIND_CHECK_MEM_IS_DEFINED((p), (len))
+     /* VALGRIND_MAKE_MEM_DEFINED returns 0 iff not running on memcheck.
+      * This is more precise than the RUNNING_ON_VALGRIND macro, which
+      * checks for valgrind in general instead of memcheck specifically. */
+#    define SECP256K1_CHECKMEM_RUNNING() (VALGRIND_MAKE_MEM_DEFINED(NULL, 0) != 0)
+#  endif
+#endif
+
+/* As a fall-back, map these macros to dummy statements. */
+#if !defined SECP256K1_CHECKMEM_ENABLED
+#  define SECP256K1_CHECKMEM_ENABLED 0
+#  define SECP256K1_CHECKMEM_UNDEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_CHECK(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#  define SECP256K1_CHECKMEM_RUNNING() (0)
+#endif
+
+#if defined VERIFY
+#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_CHECK((p), (len))
+#else
+#define SECP256K1_CHECKMEM_CHECK_VERIFY(p, len) SECP256K1_CHECKMEM_NOOP((p), (len))
+#endif
+
+#endif /* SECP256K1_CHECKMEM_H */