[tor-commits] [tor/maint-0.4.1] Avoid using labs() on time_t in channeltls.c

[nickm at torproject.org]

commit 0849d2a2fdaeea2871f32bed35d410f19703aae1
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Aug 6 11:11:06 2019 -0400

    Avoid using labs() on time_t in channeltls.c
    
    On some windows builds, time_t is 64 bits but long is not.  This is
    causing appveyor builds to fail.
    
    Also, one of our uses of labs() on time_t was logically incorrect:
    it was telling us to accept NETINFO cells up to three minutes
    _before_ the message they were responding to, which doesn't make
    sense.
    
    This patch adds a time_abs() function that we should eventually move
    to intmath.h or something.  For now, though, it will make merges
    easier to have it file-local in channeltls.c.
    
    Fixes bug 31343; bugfix on 0.2.4.4-alpha.
---
 changes/bug31343    |  9 +++++++++
 src/or/channeltls.c | 23 +++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/changes/bug31343 b/changes/bug31343
new file mode 100644
index 000000000..17a8057ea
--- /dev/null
+++ b/changes/bug31343
@@ -0,0 +1,9 @@
+  o Minor bugfixes (compilation):
+    - Avoid using labs() on time_t, which can cause compilation warnings
+      on 64-bit Windows builds.  Fixes bug 31343; bugfix on 0.2.4.4-alpha.
+
+  o Minor bugfixes (clock skew detection):
+    - Don't believe clock skew results from NETINFO cells that appear to
+      arrive before the VERSIONS cells they are responding to were sent.
+      Previously, we would accept them up to 3 minutes "in the past".
+      Fixes bug 31343; bugfix on 0.2.4.4-alpha.
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index 3a352d47f..ea69792f1 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -1584,6 +1584,18 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan)
 }
 
 /**
+ * Helper: compute the absolute value of a time_t.
+ *
+ * (we need this because labs() doesn't always work for time_t, since
+ * long can be shorter than time_t.)
+ */
+static inline time_t
+time_abs(time_t val)
+{
+  return (val < 0) ? -val : val;
+}
+
+/**
  * Process a 'netinfo' cell
  *
  * This function is called to handle an incoming NETINFO cell; read and act
@@ -1601,7 +1613,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   uint8_t n_other_addrs;
   time_t now = time(NULL);
 
-  long apparent_skew = 0;
+  time_t apparent_skew = 0;
   tor_addr_t my_apparent_addr = TOR_ADDR_NULL;
 
   tor_assert(cell);
@@ -1659,7 +1671,11 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
 
   /* Decode the cell. */
   timestamp = ntohl(get_uint32(cell->payload));
-  if (labs(now - chan->conn->handshake_state->sent_versions_at) < 180) {
+  const time_t sent_versions_at =
+    chan->conn->handshake_state->sent_versions_at;
+  if (now > sent_versions_at && (now - sent_versions_at) < 180) {
+    /* If we have gotten the NETINFO cell reasonably soon after having
+     * sent our VERSIONS cell, maybe we can learn skew information from it. */
     apparent_skew = now - timestamp;
   }
 
@@ -1705,7 +1721,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   /* Act on apparent skew. */
   /** Warn when we get a netinfo skew with at least this value. */
 #define NETINFO_NOTICE_SKEW 3600
-  if (labs(apparent_skew) > NETINFO_NOTICE_SKEW &&
+  if (time_abs(apparent_skew) &&
       router_get_by_id_digest(chan->conn->identity_digest)) {
     int trusted = router_digest_is_trusted_dir(chan->conn->identity_digest);
     clock_skew_warning(TO_CONN(chan->conn), apparent_skew, trusted, LD_GENERAL,
@@ -2182,4 +2198,3 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
 
 #undef ERR
 }
-