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.

diff --git a/.gitignore b/.gitignore
index b141e80e89cdb3a5f4ebf0bf9e0f04aa08e02f58..7103dbed0af77b81b7b31ddd583943f080c58bb4 100644 (file)
--- 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 92ba2b868db0c7bdd16d19f0b855929379252723..e9abfc6b999d88e19080f68f916ea200092a92e0 100644 (file)
--- 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 7b1aab2f990f1f22f8218425860ee75b19f8e4d1..4b9f0953e9c1f7b011fbe0b933e45da44b15af9e 100644 (file)
--- 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,11 +64,20 @@ 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

diff --git a/changes/bug17983 b/changes/bug17983
new file mode 100644 (file)
index 0000000..db52a37
--- /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 4bdd2d39679b315ee947908df49babd6e795b2fa..626be6c1112bf4af049d52b204706474516fe076 100644 (file)
--- 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 5afb30da6a4d73b67e11589c3fba5bcd754fcc12..96fc329aae9284d5ee8893c771856ef6a45559ee 100644 (file)
--- 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 bf678f2c9d166b15649e716614fb07a60f7617ec..2a0227a85ae4f22473aa476cd11406f614f72c9c 100644 (file)
--- 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 712ae18406ad135805fe1084631a420eb7635008..19f1a7fe0a20d9ba6199d966f034c6748993a1c0 100644 (file)
--- 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 7d80fdf1524a1852499252c78dfebda11029419c..c4ef30fe0dfa47cf99b1d5294b6696610b09d372 100644 (file)
--- 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 38ed57546f49c28577d4831d53c481f2a40a1015..b4ea82d3557e2f40695c915f9a2c52aeba6cf3ff 100644 (file)
--- 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@ \