commit bbaa398d268cda00e1b52fc2ebbe28f038b7db8f Author: Mike Perry <mikeperry-git@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);