Add -ftrapv to gcc-hardening ... mostly!

We know there are overflows in curve25519-donna-c32, so we'll have
to have that one be fwrapv.

Only apply the asan, ubsan, and trapv options to the code that does
not need to run in constant time.  Those options introduce branches
to the code they instrument.

(These introduced branches should never actually be taken, so it
might _still_ be constant time after all, but branch predictors are
complicated enough that I'm not really confident here. Let's aim for
safety.)

Closes 17983.

10 files changed, 88 insertions, 19 deletions

diff --git a/.gitignore b/.gitignore
index b141e80e89..7103dbed0a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -132,6 +132,9 @@ uptime-*.json
 /src/common/libor.a
 /src/common/libor-testing.a
 /src/common/libor.lib
+/src/common/libor-ctime.a
+/src/common/libor-ctime-testing.a
+/src/common/libor-ctime.lib
 /src/common/libor-crypto.a
 /src/common/libor-crypto-testing.a
 /src/common/libor-crypto.lib
diff --git a/Makefile.am b/Makefile.am
index 92ba2b868d..e9abfc6b99 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -15,8 +15,8 @@ noinst_PROGRAMS=
 DISTCLEANFILES=
 bin_SCRIPTS=
 AM_CPPFLAGS=
-AM_CFLAGS = @TOR_SYSTEMD_CFLAGS@
-SHELL = @SHELL@
+AM_CFLAGS=@TOR_SYSTEMD_CFLAGS@ @CFLAGS_BUGTRAP@
+SHELL=@SHELL@
 
 if COVERAGE_ENABLED
 TESTING_TOR_BINARY="$(top_builddir)/src/or/tor-cov"
diff --git a/acinclude.m4 b/acinclude.m4
index 7b1aab2f99..4b9f0953e9 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -42,10 +42,11 @@ AC_DEFUN([TOR_DEFINE_CODEPATH],
   AC_SUBST(TOR_LDFLAGS_$2)
 ])
 
-dnl 1:flags
-dnl 2:also try to link (yes: non-empty string)
-dnl   will set yes or no in $tor_can_link_$1 (as modified by AS_VAR_PUSHDEF)
-AC_DEFUN([TOR_CHECK_CFLAGS], [
+dnl 1: flags
+dnl 2: try to link too if this is nonempty.
+dnl 3: what to do on success compiling
+dnl 4: what to do on failure compiling
+AC_DEFUN([TOR_TRY_COMPILE_WITH_CFLAGS], [
   AS_VAR_PUSHDEF([VAR],[tor_cv_cflags_$1])
   AC_CACHE_CHECK([whether the compiler accepts $1], VAR, [
     tor_saved_CFLAGS="$CFLAGS"
@@ -63,12 +64,21 @@ AC_DEFUN([TOR_CHECK_CFLAGS], [
     CFLAGS="$tor_saved_CFLAGS"
   ])
   if test x$VAR = xyes; then
-    CFLAGS="$CFLAGS $1"
+     $3
+  else
+     $4
   fi
   AS_VAR_POPDEF([VAR])
 ])
 
 dnl 1:flags
+dnl 2:also try to link (yes: non-empty string)
+dnl   will set yes or no in $tor_can_link_$1 (as modified by AS_VAR_PUSHDEF)
+AC_DEFUN([TOR_CHECK_CFLAGS], [
+  TOR_TRY_COMPILE_WITH_CFLAGS($1, $2, CFLAGS="$CFLAGS $1", /bin/true)
+])
+
+dnl 1:flags
 dnl 2:extra ldflags
 dnl 3:extra libraries
 AC_DEFUN([TOR_CHECK_LDFLAGS], [
diff --git a/changes/bug17983 b/changes/bug17983
new file mode 100644
index 0000000000..db52a37615
--- /dev/null
+++ b/changes/bug17983
@@ -0,0 +1,11 @@
+  o Minor features (bug-finding):
+    - Tor now builds with -ftrapv by default on compilers that support it.
+      This option detects signed integer overflow, and turns it into a
+      hard-failure.  We do not apply this option to code that needs to run
+      in constant time to avoid side-channels; instead, we use -fwrapv.
+      Closes ticket 17983.
+    - When --enable-expensive-hardening is selected, stop applying the clang/gcc
+      sanitizers to code that needs to run in constant-time to avoid side
+      channels: although we are aware of no introduced side-channels, we
+      are not able to prove that this is safe. Related to ticket 17983.
+
diff --git a/configure.ac b/configure.ac
index 4bdd2d3967..626be6c111 100644
--- a/configure.ac
+++ b/configure.ac
@@ -755,6 +755,11 @@ dnl use it with a build of a library.
 all_ldflags_for_check="$TOR_LDFLAGS_zlib $TOR_LDFLAGS_openssl $TOR_LDFLAGS_libevent"
 all_libs_for_check="$TOR_ZLIB_LIBS $TOR_LIB_MATH $TOR_LIBEVENT_LIBS $TOR_OPENSSL_LIBS $TOR_SYSTEMD_LIBS $TOR_LIB_WS32 $TOR_LIB_GDI $TOR_CAP_LIBS"
 
+CFLAGS_FTRAPV=
+CFLAGS_FWRAPV=
+CFLAGS_ASAN=
+CFLAGS_UBSAN=
+
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 #if !defined(__clang__)
 #error
@@ -777,20 +782,32 @@ m4_ifdef([AS_VAR_IF],[
     AS_VAR_POPDEF([can_link])
     AS_VAR_POPDEF([can_compile])
     TOR_CHECK_CFLAGS(-Wstack-protector)
-    TOR_CHECK_CFLAGS(-fwrapv)
     TOR_CHECK_CFLAGS(--param ssp-buffer-size=1)
     if test "$bwin32" = "false"; then
        TOR_CHECK_CFLAGS(-fPIE)
        TOR_CHECK_LDFLAGS(-pie, "$all_ldflags_for_check", "$all_libs_for_check")
     fi
+    TOR_TRY_COMPILE_WITH_CFLAGS(-ftrapv, , CFLAGS_FTRAPV="-ftrapv", /bin/true)
+    TOR_TRY_COMPILE_WITH_CFLAGS(-fwrapv, , CFLAGS_FWRAPV="-fwrapv", /bin/true)
 fi
 
 if test "x$enable_expensive_hardening" = "xyes"; then
-   TOR_CHECK_CFLAGS([-fsanitize=address])
-   TOR_CHECK_CFLAGS([-fsanitize=undefined])
+   TOR_TRY_COMPILE_WITH_CFLAGS([-fsanitize=address], , CFLAGS_ASAN="-fsanitize=address", /bin/true)
+   TOR_TRY_COMPILE_WITH_CFLAGS([-fsanitize=undefined], , CFLAGS_UBSAN="-fsanitize=undefined", /bin/true)
    TOR_CHECK_CFLAGS([-fno-omit-frame-pointer])
 fi
 
+CFLAGS_BUGTRAP="$CFLAGS_FTRAPV $CFLAGS_ASAN $CFLAGS_UBSAN"
+CFLAGS_CONSTTIME="$CFLAGS_FWRAPV"
+
+dnl These cflags add bunches of branches, and we haven't been able to
+dnl persuade ourselves that they're suitable for code that needs to be
+dnl constant time.
+AC_SUBST(CFLAGS_BUGTRAP)
+dnl These cflags are variant ones sutable for code that needs to be
+dnl constant-time.
+AC_SUBST(CFLAGS_CONSTTIME)
+
 if test "x$enable_linker_hardening" != "xno"; then
     TOR_CHECK_LDFLAGS(-z relro -z now, "$all_ldflags_for_check", "$all_libs_for_check")
 fi
diff --git a/src/common/include.am b/src/common/include.am
index 5afb30da6a..96fc329aae 100644
--- a/src/common/include.am
+++ b/src/common/include.am
@@ -1,12 +1,14 @@
 
 noinst_LIBRARIES += \
 	src/common/libor.a \
+        src/common/libor-ctime.a \
 	src/common/libor-crypto.a \
 	src/common/libor-event.a
 
 if UNITTESTS_ENABLED
 noinst_LIBRARIES += \
 	src/common/libor-testing.a \
+        src/common/libor-ctime-testing.a \
 	src/common/libor-crypto-testing.a \
 	src/common/libor-event-testing.a
 endif
@@ -27,12 +29,14 @@ src_common_libcurve25519_donna_a_CFLAGS=
 if BUILD_CURVE25519_DONNA
 src_common_libcurve25519_donna_a_SOURCES=\
 	src/ext/curve25519_donna/curve25519-donna.c
+# See bug 13538 -- this code is known to have signed overflow issues.
 src_common_libcurve25519_donna_a_CFLAGS+=\
-	@F_OMIT_FRAME_POINTER@
+	@F_OMIT_FRAME_POINTER@ @CFLAGS_CONSTTIME@
 noinst_LIBRARIES+=src/common/libcurve25519_donna.a
 LIBDONNA=src/common/libcurve25519_donna.a
 else
 if BUILD_CURVE25519_DONNA_C64
+src_common_libcurve25519_donna_a_CFLAGS+=@CFLAGS_CONSTTIME@
 src_common_libcurve25519_donna_a_SOURCES=\
 	src/ext/curve25519_donna/curve25519-donna-c64.c
 noinst_LIBRARIES+=src/common/libcurve25519_donna.a
@@ -58,13 +62,21 @@ else
 readpassphrase_source=
 endif
 
+LIBOR_CTIME_A_SOURCES = \
+   src/ext/csiphash.c   \
+   src/common/di_ops.c
+
+src_common_libor_ctime_a_SOURCES = $(LIBOR_CTIME_A_SOURCES)
+src_common_libor_ctime_testing_a_SOURCES = $(LIBOR_CTIME_A_SOURCES)
+src_common_libor_ctime_a_CFLAGS = @CFLAGS_CONSTTIME@
+src_common_libor_ctime_testing_a_CFLAGS = @CFLAGS_CONSTTIME@ $(TEST_CFLAGS)
+
 LIBOR_A_SOURCES = \
   src/common/address.c					\
   src/common/backtrace.c				\
   src/common/compat.c					\
   src/common/compat_threads.c				\
   src/common/container.c				\
-  src/common/di_ops.c					\
   src/common/log.c					\
   src/common/memarea.c					\
   src/common/util.c					\
@@ -72,7 +84,6 @@ LIBOR_A_SOURCES = \
   src/common/util_process.c				\
   src/common/sandbox.c					\
   src/common/workqueue.c				\
-  src/ext/csiphash.c					\
   src/ext/trunnel/trunnel.c				\
   $(libor_extra_source)					\
   $(threads_impl_source)				\
diff --git a/src/ext/include.am b/src/ext/include.am
index bf678f2c9d..2a0227a85a 100644
--- a/src/ext/include.am
+++ b/src/ext/include.am
@@ -16,7 +16,8 @@ EXTHEADERS = \
 
 noinst_HEADERS+= $(EXTHEADERS)
 
-src_ext_ed25519_ref10_libed25519_ref10_a_CFLAGS=
+src_ext_ed25519_ref10_libed25519_ref10_a_CFLAGS=\
+  @CFLAGS_CONSTTIME@
 
 src_ext_ed25519_ref10_libed25519_ref10_a_SOURCES= \
 	src/ext/ed25519/ref10/fe_0.c \
@@ -93,7 +94,8 @@ noinst_HEADERS += $(ED25519_REF10_HDRS)
 LIBED25519_REF10=src/ext/ed25519/ref10/libed25519_ref10.a
 noinst_LIBRARIES += $(LIBED25519_REF10)
 
-src_ext_ed25519_donna_libed25519_donna_a_CFLAGS= \
+src_ext_ed25519_donna_libed25519_donna_a_CFLAGS=\
+  @CFLAGS_CONSTTIME@ \
   -DED25519_CUSTOMRANDOM \
   -DED25519_SUFFIX=_donna
 
@@ -135,7 +137,8 @@ noinst_HEADERS += $(ED25519_DONNA_HDRS)
 LIBED25519_DONNA=src/ext/ed25519/donna/libed25519_donna.a
 noinst_LIBRARIES += $(LIBED25519_DONNA)
 
-src_ext_keccak_tiny_libkeccak_tiny_a_CFLAGS=
+src_ext_keccak_tiny_libkeccak_tiny_a_CFLAGS=\
+  @CFLAGS_CONSTTIME@
 
 src_ext_keccak_tiny_libkeccak_tiny_a_SOURCES= \
 	src/ext/keccak-tiny/keccak-tiny-unrolled.c
diff --git a/src/or/include.am b/src/or/include.am
index 712ae18406..19f1a7fe0a 100644
--- a/src/or/include.am
+++ b/src/or/include.am
@@ -109,7 +109,7 @@ src_or_libtor_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 
 
 src_or_tor_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ @TOR_LDFLAGS_libevent@
-src_or_tor_LDADD = src/or/libtor.a src/common/libor.a \
+src_or_tor_LDADD = src/or/libtor.a src/common/libor.a src/common/libor-ctime.a \
 	src/common/libor-crypto.a $(LIBKECCAK_TINY) $(LIBDONNA) \
 	src/common/libor-event.a src/trunnel/libor-trunnel.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ @TOR_OPENSSL_LIBS@ \
@@ -121,6 +121,7 @@ src_or_tor_cov_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)
 src_or_tor_cov_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 src_or_tor_cov_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ @TOR_LDFLAGS_libevent@
 src_or_tor_cov_LDADD = src/or/libtor-testing.a src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	src/common/libor-crypto-testing.a $(LIBKECCAK_TINY) $(LIBDONNA) \
 	src/common/libor-event-testing.a src/trunnel/libor-trunnel-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ @TOR_OPENSSL_LIBS@ \
diff --git a/src/test/include.am b/src/test/include.am
index 7d80fdf152..c4ef30fe0d 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -147,6 +147,7 @@ src_test_test_switch_id_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 src_test_test_switch_id_LDFLAGS = @TOR_LDFLAGS_zlib@
 src_test_test_switch_id_LDADD = \
 	src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@
 
 src_test_test_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \
@@ -156,6 +157,7 @@ src_test_test_LDADD = src/or/libtor-testing.a \
 	$(LIBKECCAK_TINY) \
 	$(LIBDONNA) \
 	src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	src/common/libor-event-testing.a \
 	src/trunnel/libor-trunnel-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \
@@ -175,6 +177,7 @@ src_test_test_memwipe_LDFLAGS = $(src_test_test_LDFLAGS)
 src_test_bench_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \
         @TOR_LDFLAGS_libevent@
 src_test_bench_LDADD = src/or/libtor.a src/common/libor.a \
+	src/common/libor-ctime.a \
 	src/common/libor-crypto.a $(LIBKECCAK_TINY) $(LIBDONNA) \
 	src/common/libor-event.a src/trunnel/libor-trunnel.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \
@@ -185,6 +188,7 @@ src_test_test_workqueue_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \
         @TOR_LDFLAGS_libevent@
 src_test_test_workqueue_LDADD = src/or/libtor-testing.a \
 	src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	src/common/libor-crypto-testing.a $(LIBKECCAK_TINY) $(LIBDONNA) \
 	src/common/libor-event-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \
@@ -208,6 +212,7 @@ noinst_PROGRAMS+= src/test/test-ntor-cl
 src_test_test_ntor_cl_SOURCES  = src/test/test_ntor_cl.c
 src_test_test_ntor_cl_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@
 src_test_test_ntor_cl_LDADD = src/or/libtor.a src/common/libor.a \
+	src/common/libor-ctime.a \
 	src/common/libor-crypto.a $(LIBKECCAK_TINY) $(LIBDONNA) \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ \
 	@TOR_OPENSSL_LIBS@ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@
@@ -217,6 +222,7 @@ src_test_test_ntor_cl_AM_CPPFLAGS =	       \
 noinst_PROGRAMS += src/test/test-bt-cl
 src_test_test_bt_cl_SOURCES = src/test/test_bt_cl.c
 src_test_test_bt_cl_LDADD = src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	@TOR_LIB_MATH@ \
 	@TOR_LIB_WS32@ @TOR_LIB_GDI@
 src_test_test_bt_cl_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
diff --git a/src/tools/include.am b/src/tools/include.am
index 38ed57546f..b4ea82d355 100644
--- a/src/tools/include.am
+++ b/src/tools/include.am
@@ -7,19 +7,23 @@ endif
 
 src_tools_tor_resolve_SOURCES = src/tools/tor-resolve.c
 src_tools_tor_resolve_LDFLAGS =
-src_tools_tor_resolve_LDADD = src/common/libor.a @TOR_LIB_MATH@ @TOR_LIB_WS32@
+src_tools_tor_resolve_LDADD = src/common/libor.a \
+	src/common/libor-ctime.a \
+	@TOR_LIB_MATH@ @TOR_LIB_WS32@
 
 if COVERAGE_ENABLED
 src_tools_tor_cov_resolve_SOURCES = src/tools/tor-resolve.c
 src_tools_tor_cov_resolve_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)
 src_tools_tor_cov_resolve_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 src_tools_tor_cov_resolve_LDADD = src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
         @TOR_LIB_MATH@ @TOR_LIB_WS32@
 endif
 
 src_tools_tor_gencert_SOURCES = src/tools/tor-gencert.c
 src_tools_tor_gencert_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@
 src_tools_tor_gencert_LDADD = src/common/libor.a src/common/libor-crypto.a \
+    src/common/libor-ctime.a \
     $(LIBKECCAK_TINY) \
     $(LIBDONNA) \
         @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ @TOR_OPENSSL_LIBS@ \
@@ -31,6 +35,7 @@ src_tools_tor_cov_gencert_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)
 src_tools_tor_cov_gencert_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 src_tools_tor_cov_gencert_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@
 src_tools_tor_cov_gencert_LDADD = src/common/libor-testing.a \
+    src/common/libor-ctime-testing.a \
     src/common/libor-crypto-testing.a \
     $(LIBKECCAK_TINY) \
     $(LIBDONNA) \
@@ -40,7 +45,9 @@ endif
 
 src_tools_tor_checkkey_SOURCES = src/tools/tor-checkkey.c
 src_tools_tor_checkkey_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@
-src_tools_tor_checkkey_LDADD = src/common/libor.a src/common/libor-crypto.a \
+src_tools_tor_checkkey_LDADD = src/common/libor.a \
+    src/common/libor-ctime.a \
+    src/common/libor-crypto.a \
     $(LIBKECCAK_TINY) \
     $(LIBDONNA) \
         @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ @TOR_OPENSSL_LIBS@ \