Browse Source

Merge branch 'ftrapv_v3'

There were some conflicts here, and some breakage to fix concerning
library link order in newer targets.
Nick Mathewson 10 years ago
parent
commit
607a9056d4
15 changed files with 212 additions and 119 deletions
  1. 3 0
      .gitignore
  2. 2 2
      Makefile.am
  3. 15 5
      acinclude.m4
  4. 11 0
      changes/bug17983
  5. 20 3
      configure.ac
  6. 45 0
      src/common/di_ops.c
  7. 2 0
      src/common/di_ops.h
  8. 23 12
      src/common/include.am
  9. 6 3
      src/ext/include.am
  10. 2 1
      src/or/include.am
  11. 35 56
      src/or/routerlist.c
  12. 3 10
      src/or/routerlist.h
  13. 13 3
      src/test/include.am
  14. 23 22
      src/test/test_dir.c
  15. 9 2
      src/tools/include.am

+ 3 - 0
.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

+ 2 - 2
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$(EXEEXT)

+ 15 - 5
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

+ 11 - 0
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.
+

+ 20 - 3
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", true)
+    TOR_TRY_COMPILE_WITH_CFLAGS(-fwrapv, , CFLAGS_FWRAPV="-fwrapv", 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", true)
+   TOR_TRY_COMPILE_WITH_CFLAGS([-fsanitize=undefined], , CFLAGS_UBSAN="-fsanitize=undefined", 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

+ 45 - 0
src/common/di_ops.c

@@ -226,3 +226,48 @@ safe_mem_is_zero(const void *mem, size_t sz)
   return 1 & ((total - 1) >> 8);
 }
 
+/** Time-invariant 64-bit greater-than; works on two integers in the range
+ * (0,INT64_MAX). */
+#if SIZEOF_VOID_P == 8
+#define gt_i64_timei(a,b) ((a) > (b))
+#else
+static inline int
+gt_i64_timei(uint64_t a, uint64_t b)
+{
+  int64_t diff = (int64_t) (b - a);
+  int res = diff >> 63;
+  return res & 1;
+}
+#endif
+
+/**
+ * Given an array of list of <b>n_entries</b> uint64_t values, whose sum is
+ * <b>total</b>, find the first i such that the total of all elements 0...i is
+ * greater than rand_val.
+ *
+ * Try to perform this operation in a constant-time way.
+ */
+int
+select_array_member_cumulative_timei(const uint64_t *entries, int n_entries,
+                                     uint64_t total, uint64_t rand_val)
+{
+  int i, i_chosen=-1, n_chosen=0;
+  uint64_t total_so_far = 0;
+
+  for (i = 0; i < n_entries; ++i) {
+    total_so_far += entries[i];
+    if (gt_i64_timei(total_so_far, rand_val)) {
+      i_chosen = i;
+      n_chosen++;
+      /* Set rand_val to INT64_MAX rather than stopping the loop. This way,
+       * the time we spend in the loop does not leak which element we chose. */
+      rand_val = INT64_MAX;
+    }
+  }
+  tor_assert(total_so_far == total);
+  tor_assert(n_chosen == 1);
+  tor_assert(i_chosen >= 0);
+  tor_assert(i_chosen < n_entries);
+
+  return i_chosen;
+}

+ 2 - 0
src/common/di_ops.h

@@ -42,6 +42,8 @@ void dimap_add_entry(di_digest256_map_t **map,
                      const uint8_t *key, void *val);
 void *dimap_search(const di_digest256_map_t *map, const uint8_t *key,
                    void *dflt_val);
+int select_array_member_cumulative_timei(const uint64_t *entries, int n_entries,
+                                         uint64_t total, uint64_t rand_val);
 
 #endif
 

+ 23 - 12
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_A_SOURCES = \
+LIBOR_CTIME_A_SRC = \
+   src/ext/csiphash.c   \
+   src/common/di_ops.c
+
+src_common_libor_ctime_a_SOURCES = $(LIBOR_CTIME_A_SRC)
+src_common_libor_ctime_testing_a_SOURCES = $(LIBOR_CTIME_A_SRC)
+src_common_libor_ctime_a_CFLAGS = @CFLAGS_CONSTTIME@
+src_common_libor_ctime_testing_a_CFLAGS = @CFLAGS_CONSTTIME@ $(TEST_CFLAGS)
+
+LIBOR_A_SRC = \
   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/pubsub.c					\
@@ -74,7 +86,6 @@ LIBOR_A_SOURCES = \
   src/common/util_process.c				\
   src/common/sandbox.c					\
   src/common/workqueue.c				\
-  src/ext/csiphash.c					\
   $(libor_extra_source)					\
   $(threads_impl_source)				\
   $(readpassphrase_source)
@@ -82,7 +93,7 @@ LIBOR_A_SOURCES = \
 src/common/src_common_libor_testing_a-log.$(OBJEXT) \
   src/common/log.$(OBJEXT): micro-revision.i
 
-LIBOR_CRYPTO_A_SOURCES = \
+LIBOR_CRYPTO_A_SRC = \
   src/common/aes.c		\
   src/common/crypto.c		\
   src/common/crypto_pwbox.c     \
@@ -93,19 +104,19 @@ LIBOR_CRYPTO_A_SOURCES = \
   src/common/crypto_curve25519.c \
   src/common/crypto_ed25519.c
 
-LIBOR_EVENT_A_SOURCES = \
+LIBOR_EVENT_A_SRC = \
 	src/common/compat_libevent.c \
 	src/common/procmon.c         \
 	src/common/timers.c          \
 	src/ext/timeouts/timeout.c
 
-src_common_libor_a_SOURCES = $(LIBOR_A_SOURCES)
-src_common_libor_crypto_a_SOURCES = $(LIBOR_CRYPTO_A_SOURCES)
-src_common_libor_event_a_SOURCES = $(LIBOR_EVENT_A_SOURCES)
+src_common_libor_a_SOURCES = $(LIBOR_A_SRC)
+src_common_libor_crypto_a_SOURCES = $(LIBOR_CRYPTO_A_SRC)
+src_common_libor_event_a_SOURCES = $(LIBOR_EVENT_A_SRC)
 
-src_common_libor_testing_a_SOURCES = $(LIBOR_A_SOURCES)
-src_common_libor_crypto_testing_a_SOURCES = $(LIBOR_CRYPTO_A_SOURCES)
-src_common_libor_event_testing_a_SOURCES = $(LIBOR_EVENT_A_SOURCES)
+src_common_libor_testing_a_SOURCES = $(LIBOR_A_SRC)
+src_common_libor_crypto_testing_a_SOURCES = $(LIBOR_CRYPTO_A_SRC)
+src_common_libor_event_testing_a_SOURCES = $(LIBOR_EVENT_A_SRC)
 
 src_common_libor_testing_a_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)
 src_common_libor_crypto_testing_a_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)

+ 6 - 3
src/ext/include.am

@@ -20,7 +20,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 \
@@ -97,7 +98,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
 
@@ -139,7 +141,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

+ 2 - 1
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@ \

+ 35 - 56
src/or/routerlist.c

@@ -67,7 +67,7 @@ typedef struct cert_list_t cert_list_t;
 /* static function prototypes */
 static int compute_weighted_bandwidths(const smartlist_t *sl,
                                        bandwidth_weight_rule_t rule,
-                                       u64_dbl_t **bandwidths_out);
+                                       double **bandwidths_out);
 static const routerstatus_t *router_pick_trusteddirserver_impl(
                 const smartlist_t *sourcelist, dirinfo_type_t auth,
                 int flags, int *n_busy_out);
@@ -1815,20 +1815,23 @@ dirserver_choose_by_weight(const smartlist_t *servers, double authority_weight)
 {
   int n = smartlist_len(servers);
   int i;
-  u64_dbl_t *weights;
+  double *weights_dbl;
+  uint64_t *weights_u64;
   const dir_server_t *ds;
 
-  weights = tor_calloc(n, sizeof(u64_dbl_t));
+  weights_dbl = tor_calloc(n, sizeof(double));
+  weights_u64 = tor_calloc(n, sizeof(uint64_t));
   for (i = 0; i < n; ++i) {
     ds = smartlist_get(servers, i);
-    weights[i].dbl = ds->weight;
+    weights_dbl[i] = ds->weight;
     if (ds->is_authority)
-      weights[i].dbl *= authority_weight;
+      weights_dbl[i] *= authority_weight;
   }
 
-  scale_array_elements_to_u64(weights, n, NULL);
-  i = choose_array_element_by_weight(weights, n);
-  tor_free(weights);
+  scale_array_elements_to_u64(weights_u64, weights_dbl, n, NULL);
+  i = choose_array_element_by_weight(weights_u64, n);
+  tor_free(weights_dbl);
+  tor_free(weights_u64);
   return (i < 0) ? NULL : smartlist_get(servers, i);
 }
 
@@ -2090,7 +2093,8 @@ router_get_advertised_bandwidth_capped(const routerinfo_t *router)
  * much of the range of uint64_t. If <b>total_out</b> is provided, set it to
  * the sum of all elements in the array _before_ scaling. */
 STATIC void
-scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
+scale_array_elements_to_u64(uint64_t *entries_out, const double *entries_in,
+                            int n_entries,
                             uint64_t *total_out)
 {
   double total = 0.0;
@@ -2100,13 +2104,13 @@ scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
 #define SCALE_TO_U64_MAX ((int64_t) (INT64_MAX / 4))
 
   for (i = 0; i < n_entries; ++i)
-    total += entries[i].dbl;
+    total += entries_in[i];
 
   if (total > 0.0)
     scale_factor = SCALE_TO_U64_MAX / total;
 
   for (i = 0; i < n_entries; ++i)
-    entries[i].u64 = tor_llround(entries[i].dbl * scale_factor);
+    entries_out[i] = tor_llround(entries_in[i] * scale_factor);
 
   if (total_out)
     *total_out = (uint64_t) total;
@@ -2114,35 +2118,20 @@ scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
 #undef SCALE_TO_U64_MAX
 }
 
-/** Time-invariant 64-bit greater-than; works on two integers in the range
- * (0,INT64_MAX). */
-#if SIZEOF_VOID_P == 8
-#define gt_i64_timei(a,b) ((a) > (b))
-#else
-static inline int
-gt_i64_timei(uint64_t a, uint64_t b)
-{
-  int64_t diff = (int64_t) (b - a);
-  int res = diff >> 63;
-  return res & 1;
-}
-#endif
-
 /** Pick a random element of <b>n_entries</b>-element array <b>entries</b>,
  * choosing each element with a probability proportional to its (uint64_t)
  * value, and return the index of that element.  If all elements are 0, choose
  * an index at random. Return -1 on error.
  */
 STATIC int
-choose_array_element_by_weight(const u64_dbl_t *entries, int n_entries)
+choose_array_element_by_weight(const uint64_t *entries, int n_entries)
 {
-  int i, i_chosen=-1, n_chosen=0;
-  uint64_t total_so_far = 0;
+  int i;
   uint64_t rand_val;
   uint64_t total = 0;
 
   for (i = 0; i < n_entries; ++i)
-    total += entries[i].u64;
+    total += entries[i];
 
   if (n_entries < 1)
     return -1;
@@ -2154,22 +2143,8 @@ choose_array_element_by_weight(const u64_dbl_t *entries, int n_entries)
 
   rand_val = crypto_rand_uint64(total);
 
-  for (i = 0; i < n_entries; ++i) {
-    total_so_far += entries[i].u64;
-    if (gt_i64_timei(total_so_far, rand_val)) {
-      i_chosen = i;
-      n_chosen++;
-      /* Set rand_val to INT64_MAX rather than stopping the loop. This way,
-       * the time we spend in the loop does not leak which element we chose. */
-      rand_val = INT64_MAX;
-    }
-  }
-  tor_assert(total_so_far == total);
-  tor_assert(n_chosen == 1);
-  tor_assert(i_chosen >= 0);
-  tor_assert(i_chosen < n_entries);
-
-  return i_chosen;
+  return select_array_member_cumulative_timei(
+                           entries, n_entries, total, rand_val);
 }
 
 /** When weighting bridges, enforce these values as lower and upper
@@ -2221,17 +2196,21 @@ static const node_t *
 smartlist_choose_node_by_bandwidth_weights(const smartlist_t *sl,
                                            bandwidth_weight_rule_t rule)
 {
-  u64_dbl_t *bandwidths=NULL;
+  double *bandwidths_dbl=NULL;
+  uint64_t *bandwidths_u64=NULL;
 
-  if (compute_weighted_bandwidths(sl, rule, &bandwidths) < 0)
+  if (compute_weighted_bandwidths(sl, rule, &bandwidths_dbl) < 0)
     return NULL;
 
-  scale_array_elements_to_u64(bandwidths, smartlist_len(sl), NULL);
+  bandwidths_u64 = tor_calloc(smartlist_len(sl), sizeof(uint64_t));
+  scale_array_elements_to_u64(bandwidths_u64, bandwidths_dbl,
+                              smartlist_len(sl), NULL);
 
   {
-    int idx = choose_array_element_by_weight(bandwidths,
+    int idx = choose_array_element_by_weight(bandwidths_u64,
                                              smartlist_len(sl));
-    tor_free(bandwidths);
+    tor_free(bandwidths_dbl);
+    tor_free(bandwidths_u64);
     return idx < 0 ? NULL : smartlist_get(sl, idx);
   }
 }
@@ -2244,14 +2223,14 @@ smartlist_choose_node_by_bandwidth_weights(const smartlist_t *sl,
 static int
 compute_weighted_bandwidths(const smartlist_t *sl,
                             bandwidth_weight_rule_t rule,
-                            u64_dbl_t **bandwidths_out)
+                            double **bandwidths_out)
 {
   int64_t weight_scale;
   double Wg = -1, Wm = -1, We = -1, Wd = -1;
   double Wgb = -1, Wmb = -1, Web = -1, Wdb = -1;
   uint64_t weighted_bw = 0;
   guardfraction_bandwidth_t guardfraction_bw;
-  u64_dbl_t *bandwidths;
+  double *bandwidths;
 
   /* Can't choose exit and guard at same time */
   tor_assert(rule == NO_WEIGHTING ||
@@ -2333,7 +2312,7 @@ compute_weighted_bandwidths(const smartlist_t *sl,
   Web /= weight_scale;
   Wdb /= weight_scale;
 
-  bandwidths = tor_calloc(smartlist_len(sl), sizeof(u64_dbl_t));
+  bandwidths = tor_calloc(smartlist_len(sl), sizeof(double));
 
   // Cycle through smartlist and total the bandwidth.
   static int warned_missing_bw = 0;
@@ -2420,7 +2399,7 @@ compute_weighted_bandwidths(const smartlist_t *sl,
       final_weight = weight*this_bw;
     }
 
-    bandwidths[node_sl_idx].dbl = final_weight + 0.5;
+    bandwidths[node_sl_idx] = final_weight + 0.5;
   } SMARTLIST_FOREACH_END(node);
 
   log_debug(LD_CIRC, "Generated weighted bandwidths for rule %s based "
@@ -2441,7 +2420,7 @@ double
 frac_nodes_with_descriptors(const smartlist_t *sl,
                             bandwidth_weight_rule_t rule)
 {
-  u64_dbl_t *bandwidths = NULL;
+  double *bandwidths = NULL;
   double total, present;
 
   if (smartlist_len(sl) == 0)
@@ -2458,7 +2437,7 @@ frac_nodes_with_descriptors(const smartlist_t *sl,
 
   total = present = 0.0;
   SMARTLIST_FOREACH_BEGIN(sl, const node_t *, node) {
-    const double bw = bandwidths[node_sl_idx].dbl;
+    const double bw = bandwidths[node_sl_idx];
     total += bw;
     if (node_has_descriptor(node))
       present += bw;

+ 3 - 10
src/or/routerlist.h

@@ -217,17 +217,10 @@ int hex_digest_nickname_matches(const char *hexdigest,
                                 const char *nickname, int is_named);
 
 #ifdef ROUTERLIST_PRIVATE
-/** Helper type for choosing routers by bandwidth: contains a union of
- * double and uint64_t. Before we call scale_array_elements_to_u64, it holds
- * a double; after, it holds a uint64_t. */
-typedef union u64_dbl_t {
-  uint64_t u64;
-  double dbl;
-} u64_dbl_t;
-
-STATIC int choose_array_element_by_weight(const u64_dbl_t *entries,
+STATIC int choose_array_element_by_weight(const uint64_t *entries,
                                           int n_entries);
-STATIC void scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
+STATIC void scale_array_elements_to_u64(uint64_t *entries_out,
+                                        const double *entries_in, int n_entries,
                                         uint64_t *total_out);
 STATIC const routerstatus_t *router_pick_directory_server_impl(
                                            dirinfo_type_t auth, int flags,

+ 13 - 3
src/test/include.am

@@ -153,6 +153,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@ \
@@ -162,6 +163,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@ \
@@ -174,13 +176,17 @@ src_test_test_slow_LDADD = $(src_test_test_LDADD)
 src_test_test_slow_LDFLAGS = $(src_test_test_LDFLAGS)
 
 src_test_test_memwipe_CPPFLAGS = $(src_test_test_CPPFLAGS)
-src_test_test_memwipe_CFLAGS = $(src_test_test_CFLAGS)
+# Don't use bugtrap cflags here: memwipe tests require memory violations.
+src_test_test_memwipe_CFLAGS = $(TEST_CFLAGS)
 src_test_test_memwipe_LDADD = $(src_test_test_LDADD)
-src_test_test_memwipe_LDFLAGS = $(src_test_test_LDFLAGS)
+# The LDFLAGS need to include the bugtrap cflags, or else we won't link
+# successfully with the libraries built with them.
+src_test_test_memwipe_LDFLAGS = $(src_test_test_LDFLAGS) @CFLAGS_BUGTRAP@
 
 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@ \
@@ -191,6 +197,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@ \
@@ -199,9 +206,10 @@ src_test_test_workqueue_LDADD = src/or/libtor-testing.a \
 src_test_test_timers_CPPFLAGS = $(src_test_test_CPPFLAGS)
 src_test_test_timers_CFLAGS = $(src_test_test_CFLAGS)
 src_test_test_timers_LDADD = \
+	src/common/libor-testing.a \
+	src/common/libor-ctime-testing.a \
 	src/common/libor-event-testing.a \
 	src/common/libor-crypto-testing.a $(LIBKECCAK_TINY) $(LIBDONNA) \
-	src/common/libor-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \
 	@TOR_OPENSSL_LIBS@ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@
 src_test_test_timers_LDFLAGS = $(src_test_test_LDFLAGS)
@@ -224,6 +232,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@
@@ -233,6 +242,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)

+ 23 - 22
src/test/test_dir.c

@@ -2149,56 +2149,57 @@ test_dir_scale_bw(void *testdata)
                   1.0/7,
                   12.0,
                   24.0 };
-  u64_dbl_t vals[8];
+  double vals_dbl[8];
+  uint64_t vals_u64[8];
   uint64_t total;
   int i;
 
   (void) testdata;
 
   for (i=0; i<8; ++i)
-    vals[i].dbl = v[i];
+    vals_dbl[i] = v[i];
 
-  scale_array_elements_to_u64(vals, 8, &total);
+  scale_array_elements_to_u64(vals_u64, vals_dbl, 8, &total);
 
   tt_int_op((int)total, OP_EQ, 48);
   total = 0;
   for (i=0; i<8; ++i) {
-    total += vals[i].u64;
+    total += vals_u64[i];
   }
   tt_assert(total >= (U64_LITERAL(1)<<60));
   tt_assert(total <= (U64_LITERAL(1)<<62));
 
   for (i=0; i<8; ++i) {
     /* vals[2].u64 is the scaled value of 1.0 */
-    double ratio = ((double)vals[i].u64) / vals[2].u64;
+    double ratio = ((double)vals_u64[i]) / vals_u64[2];
     tt_double_op(fabs(ratio - v[i]), OP_LT, .00001);
   }
 
   /* test handling of no entries */
   total = 1;
-  scale_array_elements_to_u64(vals, 0, &total);
+  scale_array_elements_to_u64(vals_u64, vals_dbl, 0, &total);
   tt_assert(total == 0);
 
   /* make sure we don't read the array when we have no entries
    * may require compiler flags to catch NULL dereferences */
   total = 1;
-  scale_array_elements_to_u64(NULL, 0, &total);
+  scale_array_elements_to_u64(NULL, NULL, 0, &total);
   tt_assert(total == 0);
 
-  scale_array_elements_to_u64(NULL, 0, NULL);
+  scale_array_elements_to_u64(NULL, NULL, 0, NULL);
 
   /* test handling of zero totals */
   total = 1;
-  vals[0].dbl = 0.0;
-  scale_array_elements_to_u64(vals, 1, &total);
+  vals_dbl[0] = 0.0;
+  scale_array_elements_to_u64(vals_u64, vals_dbl, 1, &total);
   tt_assert(total == 0);
-  tt_assert(vals[0].u64 == 0);
+  tt_assert(vals_u64[0] == 0);
 
-  vals[0].dbl = 0.0;
-  vals[1].dbl = 0.0;
-  scale_array_elements_to_u64(vals, 2, NULL);
-  tt_assert(vals[0].u64 == 0);
-  tt_assert(vals[1].u64 == 0);
+  vals_dbl[0] = 0.0;
+  vals_dbl[1] = 0.0;
+  scale_array_elements_to_u64(vals_u64, vals_dbl, 2, NULL);
+  tt_assert(vals_u64[0] == 0);
+  tt_assert(vals_u64[1] == 0);
 
  done:
   ;
@@ -2209,7 +2210,7 @@ test_dir_random_weighted(void *testdata)
 {
   int histogram[10];
   uint64_t vals[10] = {3,1,2,4,6,0,7,5,8,9}, total=0;
-  u64_dbl_t inp[10];
+  uint64_t inp_u64[10];
   int i, choice;
   const int n = 50000;
   double max_sq_error;
@@ -2219,12 +2220,12 @@ test_dir_random_weighted(void *testdata)
    * in a scrambled order to make sure we don't depend on order. */
   memset(histogram,0,sizeof(histogram));
   for (i=0; i<10; ++i) {
-    inp[i].u64 = vals[i];
+    inp_u64[i] = vals[i];
     total += vals[i];
   }
   tt_u64_op(total, OP_EQ, 45);
   for (i=0; i<n; ++i) {
-    choice = choose_array_element_by_weight(inp, 10);
+    choice = choose_array_element_by_weight(inp_u64, 10);
     tt_int_op(choice, OP_GE, 0);
     tt_int_op(choice, OP_LT, 10);
     histogram[choice]++;
@@ -2251,16 +2252,16 @@ test_dir_random_weighted(void *testdata)
 
   /* Now try a singleton; do we choose it? */
   for (i = 0; i < 100; ++i) {
-    choice = choose_array_element_by_weight(inp, 1);
+    choice = choose_array_element_by_weight(inp_u64, 1);
     tt_int_op(choice, OP_EQ, 0);
   }
 
   /* Now try an array of zeros.  We should choose randomly. */
   memset(histogram,0,sizeof(histogram));
   for (i = 0; i < 5; ++i)
-    inp[i].u64 = 0;
+    inp_u64[i] = 0;
   for (i = 0; i < n; ++i) {
-    choice = choose_array_element_by_weight(inp, 5);
+    choice = choose_array_element_by_weight(inp_u64, 5);
     tt_int_op(choice, OP_GE, 0);
     tt_int_op(choice, OP_LT, 5);
     histogram[choice]++;

+ 9 - 2
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@ \
@@ -32,6 +36,7 @@ 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-crypto-testing.a \
+    src/common/libor-ctime-testing.a \
     $(LIBKECCAK_TINY) \
     $(LIBDONNA) \
         @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ @TOR_OPENSSL_LIBS@ \
@@ -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@ \