[tor-commits] [tor/maint-0.3.4] Bug 23512: Report queued cells on or circs as written.

[teor at torproject.org]

commit bbaa398d268cda00e1b52fc2ebbe28f038b7db8f
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Fri Sep 14 18:50:40 2018 +0000

    Bug 23512: Report queued cells on or circs as written.
    
    This avoids asymmetry in our public relay stats, which can be exploited for
    guard discovery and other attacks.
---
 src/or/channeltls.h  |  2 ++
 src/or/circuitlist.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/or/circuitlist.h |  2 ++
 src/or/or.h          | 12 +++++++++++
 src/or/relay.c       |  1 +
 5 files changed, 76 insertions(+)

diff --git a/src/or/channeltls.h b/src/or/channeltls.h
index 8b5863a46..463f7d928 100644
--- a/src/or/channeltls.h
+++ b/src/or/channeltls.h
@@ -12,6 +12,8 @@
 #include "or.h"
 #include "channel.h"
 
+#define TLS_PER_CELL_OVERHEAD 29
+
 #define BASE_CHAN_TO_TLS(c) (channel_tls_from_base((c)))
 #define TLS_CHAN_TO_BASE(c) (channel_tls_to_base((c)))
 
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 977afca18..8cfdd3bb9 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -12,6 +12,7 @@
 #define CIRCUITLIST_PRIVATE
 #include "or.h"
 #include "channel.h"
+#include "channeltls.h"
 #include "circpathbias.h"
 #include "circuitbuild.h"
 #include "circuitlist.h"
@@ -1680,6 +1681,61 @@ circuit_mark_all_dirty_circs_as_unusable(void)
   SMARTLIST_FOREACH_END(circ);
 }
 
+/**
+ * Report any queued cells on or_circuits as written in our bandwidth
+ * totals, for the specified channel direction.
+ *
+ * When we close a circuit or clear its cell queues, we've read
+ * data and recorded those bytes in our read statistics, but we're
+ * not going to write it. This discrepancy can be used by an adversary
+ * to infer information from our public relay statistics and perform
+ * attacks such as guard discovery.
+ *
+ * This function is in the critical path of circuit_mark_for_close().
+ * It must be (and is) O(1)!
+ *
+ * See https://trac.torproject.org/projects/tor/ticket/23512.
+ */
+void
+circuit_synchronize_written_or_bandwidth(const circuit_t *c,
+                                         circuit_channel_direction_t dir)
+{
+  uint64_t cells;
+  uint64_t cell_size;
+  uint64_t written_sync;
+  const channel_t *chan = NULL;
+  const or_circuit_t *or_circ;
+
+  if (!CIRCUIT_IS_ORCIRC(c))
+    return;
+
+  or_circ = CONST_TO_OR_CIRCUIT(c);
+
+  if (dir == CIRCUIT_N_CHAN) {
+    chan = c->n_chan;
+    cells = c->n_chan_cells.n;
+  } else {
+    chan = or_circ->p_chan;
+    cells = or_circ->p_chan_cells.n;
+  }
+
+  /* If we still know the chan, determine real cell size. Otherwise,
+   * assume it's a wide circid channel */
+  if (chan)
+    cell_size = get_cell_network_size(chan->wide_circ_ids);
+  else
+    cell_size = CELL_MAX_NETWORK_SIZE;
+
+  /* The missing written bytes are the cell counts times their cell
+   * size plus TLS per cell overhead */
+  written_sync = cells*(cell_size+TLS_PER_CELL_OVERHEAD);
+
+  /* Report the missing bytes as written, to avoid asymmetry.
+   * We must use time() for consistency with rephist, even though on
+   * some very old rare platforms, approx_time() may be faster. */
+  rep_hist_note_bytes_written(written_sync, time(NULL));
+}
+
 /** Mark <b>circ</b> to be closed next time we call
  * circuit_close_all_marked(). Do any cleanup needed:
  *   - If state is onionskin_pending, remove circ from the onion_pending
@@ -1732,6 +1788,9 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
     reason = END_CIRC_REASON_NONE;
   }
 
+  circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_N_CHAN);
+  circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_P_CHAN);
+
   if (reason & END_CIRC_REASON_FLAG_REMOTE)
     reason &= ~END_CIRC_REASON_FLAG_REMOTE;
 
diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h
index 2707b426a..2ede6f76c 100644
--- a/src/or/circuitlist.h
+++ b/src/or/circuitlist.h
@@ -62,6 +62,8 @@ crypt_path_t *circuit_get_cpath_hop(origin_circuit_t *circ, int hopnum);
 void circuit_get_all_pending_on_channel(smartlist_t *out,
                                         channel_t *chan);
 int circuit_count_pending_on_channel(channel_t *chan);
+void circuit_synchronize_written_or_bandwidth(const circuit_t *c,
+                                              circuit_channel_direction_t dir);
 
 #define circuit_mark_for_close(c, reason)                               \
   circuit_mark_for_close_((c), (reason), __LINE__, SHORT_FILE__)
diff --git a/src/or/or.h b/src/or/or.h
index 024a9cff0..9f53c8064 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2840,6 +2840,18 @@ typedef struct testing_cell_stats_entry_t {
 } testing_cell_stats_entry_t;
 
 /**
+ * An enum to allow us to specify which channel in a circuit
+ * we're interested in.
+ *
+ * This is needed because our data structures and other fields
+ * for channel delivery are disassociated from the channel.
+ */
+typedef enum {
+  CIRCUIT_N_CHAN = 0,
+  CIRCUIT_P_CHAN = 1
+} circuit_channel_direction_t;
+
+/**
  * A circuit is a path over the onion routing
  * network. Applications can connect to one end of the circuit, and can
  * create exit connections at the other end of the circuit. AP and exit
diff --git a/src/or/relay.c b/src/or/relay.c
index 1c791e02c..d1c7820c7 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -1682,6 +1682,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       }
       if (circ->n_chan) {
         uint8_t trunc_reason = get_uint8(cell->payload + RELAY_HEADER_SIZE);
+        circuit_synchronize_written_or_bandwidth(circ, CIRCUIT_N_CHAN);
         circuit_clear_cell_queue(circ, circ->n_chan);
         channel_send_destroy(circ->n_circ_id, circ->n_chan,
                              trunc_reason);