Unverified Commit 8bd9b2a6 authored by teor's avatar teor
Browse files

Merge remote-tracking branch 'tor-github/pr/770' into maint-0.2.9

parents 90301247 72e30f26
o Major bugfix (Relay bandwidth statistics):
- When we close relayed circuits, report the data in the circuit queues
as being written in our relay bandwidth stats. This mitigates guard
discovery and other attacks that close circuits for the explicit purpose
of noticing this discrepancy in statistics. Fixes bug 23512; bugfix
on 0.0.8pre3.
......@@ -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)))
......
......@@ -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;
......
......@@ -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__)
......
......@@ -2839,6 +2839,18 @@ typedef struct testing_cell_stats_entry_t {
unsigned int exitward:1; /**< 0 for app-ward, 1 for exit-ward. */
} 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
......
......@@ -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);
......
......@@ -88,6 +88,11 @@
static void bw_arrays_init(void);
static void predicted_ports_init(void);
typedef struct bw_array_t bw_array_t;
STATIC uint64_t find_largest_max(bw_array_t *b);
STATIC void commit_max(bw_array_t *b);
STATIC void advance_obs(bw_array_t *b);
/** Total number of bytes currently allocated in fields used by rephist.c. */
uint64_t rephist_total_alloc=0;
/** Number of or_history_t objects currently allocated. */
......@@ -1206,7 +1211,7 @@ rep_hist_load_mtbf_data(time_t now)
/** Structure to track bandwidth use, and remember the maxima for a given
* time period.
*/
typedef struct bw_array_t {
struct bw_array_t {
/** Observation array: Total number of bytes transferred in each of the last
* NUM_SECS_ROLLING_MEASURE seconds. This is used as a circular array. */
uint64_t obs[NUM_SECS_ROLLING_MEASURE];
......@@ -1233,10 +1238,10 @@ typedef struct bw_array_t {
/** Circular array of the total bandwidth usage for the last NUM_TOTALS
* periods */
uint64_t totals[NUM_TOTALS];
} bw_array_t;
};
/** Shift the current period of b forward by one. */
static void
STATIC void
commit_max(bw_array_t *b)
{
/* Store total from current period. */
......@@ -1256,7 +1261,7 @@ commit_max(bw_array_t *b)
}
/** Shift the current observation time of <b>b</b> forward by one second. */
static inline void
STATIC void
advance_obs(bw_array_t *b)
{
int nextidx;
......@@ -1331,7 +1336,7 @@ bw_array_free(bw_array_t *b)
/** Recent history of bandwidth observations for read operations. */
static bw_array_t *read_array = NULL;
/** Recent history of bandwidth observations for write operations. */
static bw_array_t *write_array = NULL;
STATIC bw_array_t *write_array = NULL;
/** Recent history of bandwidth observations for read operations for the
directory protocol. */
static bw_array_t *dir_read_array = NULL;
......@@ -1363,7 +1368,7 @@ bw_arrays_init(void)
* earlier than the latest <b>when</b> you've heard of.
*/
void
rep_hist_note_bytes_written(size_t num_bytes, time_t when)
rep_hist_note_bytes_written(uint64_t num_bytes, time_t when)
{
/* Maybe a circular array for recent seconds, and step to a new point
* every time a new second shows up. Or simpler is to just to have
......@@ -1380,7 +1385,7 @@ rep_hist_note_bytes_written(size_t num_bytes, time_t when)
* (like rep_hist_note_bytes_written() above)
*/
void
rep_hist_note_bytes_read(size_t num_bytes, time_t when)
rep_hist_note_bytes_read(uint64_t num_bytes, time_t when)
{
/* if we're smart, we can make this func and the one above share code */
add_obs(read_array, when, num_bytes);
......@@ -1390,7 +1395,7 @@ rep_hist_note_bytes_read(size_t num_bytes, time_t when)
* <b>when</b>. (like rep_hist_note_bytes_written() above)
*/
void
rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when)
rep_hist_note_dir_bytes_written(uint64_t num_bytes, time_t when)
{
add_obs(dir_write_array, when, num_bytes);
}
......@@ -1399,7 +1404,7 @@ rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when)
* <b>when</b>. (like rep_hist_note_bytes_written() above)
*/
void
rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when)
rep_hist_note_dir_bytes_read(uint64_t num_bytes, time_t when)
{
add_obs(dir_read_array, when, num_bytes);
}
......@@ -1408,7 +1413,7 @@ rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when)
* most bandwidth used in any NUM_SECS_ROLLING_MEASURE period for the last
* NUM_SECS_BW_SUM_IS_VALID seconds.)
*/
static uint64_t
STATIC uint64_t
find_largest_max(bw_array_t *b)
{
int i;
......@@ -3292,4 +3297,3 @@ rep_hist_free_all(void)
tor_assert_nonfatal(rephist_total_alloc == 0);
tor_assert_nonfatal_once(rephist_total_num == 0);
}
......@@ -21,13 +21,13 @@ void rep_hist_note_extend_succeeded(const char *from_name,
const char *to_name);
void rep_hist_note_extend_failed(const char *from_name, const char *to_name);
void rep_hist_dump_stats(time_t now, int severity);
void rep_hist_note_bytes_read(size_t num_bytes, time_t when);
void rep_hist_note_bytes_written(size_t num_bytes, time_t when);
void rep_hist_note_bytes_read(uint64_t num_bytes, time_t when);
void rep_hist_note_bytes_written(uint64_t num_bytes, time_t when);
void rep_hist_make_router_pessimal(const char *id, time_t when);
void rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when);
void rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when);
void rep_hist_note_dir_bytes_read(uint64_t num_bytes, time_t when);
void rep_hist_note_dir_bytes_written(uint64_t num_bytes, time_t when);
MOCK_DECL(int, rep_hist_bandwidth_assess, (void));
char *rep_hist_get_bandwidth_lines(void);
......@@ -117,6 +117,8 @@ extern uint32_t rephist_total_num;
#ifdef TOR_UNIT_TESTS
extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1];
extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1];
typedef struct bw_array_t bw_array_t;
extern bw_array_t *write_array;
#endif
#endif
......
......@@ -4,6 +4,9 @@
#include "or.h"
#define CIRCUITBUILD_PRIVATE
#include "circuitbuild.h"
#include "circuitlist.h"
#include "rephist.h"
#include "channeltls.h"
#define RELAY_PRIVATE
#include "relay.h"
/* For init/free stuff */
......@@ -16,6 +19,9 @@
static or_circuit_t * new_fake_orcirc(channel_t *nchan, channel_t *pchan);
static void test_relay_append_cell_to_circuit_queue(void *arg);
uint64_t find_largest_max(bw_array_t *b);
void commit_max(bw_array_t *b);
void advance_obs(bw_array_t *b);
static or_circuit_t *
new_fake_orcirc(channel_t *nchan, channel_t *pchan)
......@@ -27,10 +33,9 @@ new_fake_orcirc(channel_t *nchan, channel_t *pchan)
circ = &(orcirc->base_);
circ->magic = OR_CIRCUIT_MAGIC;
circ->n_chan = nchan;
circ->n_circ_id = get_unique_circ_id_by_chan(nchan);
circ->n_mux = NULL; /* ?? */
circuit_set_n_circid_chan(circ, get_unique_circ_id_by_chan(nchan), nchan);
cell_queue_init(&(circ->n_chan_cells));
circ->n_hop = NULL;
circ->streams_blocked_on_n_chan = 0;
circ->streams_blocked_on_p_chan = 0;
......@@ -43,13 +48,108 @@ new_fake_orcirc(channel_t *nchan, channel_t *pchan)
circ->deliver_window = CIRCWINDOW_START_MAX;
circ->n_chan_create_cell = NULL;
orcirc->p_chan = pchan;
orcirc->p_circ_id = get_unique_circ_id_by_chan(pchan);
/* for assert_circ_ok */
orcirc->p_crypto = (void*)1;
orcirc->n_crypto = (void*)1;
orcirc->n_digest = (void*)1;
orcirc->p_digest = (void*)1;
circuit_set_p_circid_chan(orcirc, get_unique_circ_id_by_chan(pchan), pchan);
cell_queue_init(&(orcirc->p_chan_cells));
return orcirc;
}
static void
test_relay_close_circuit(void *arg)
{
channel_t *nchan = NULL, *pchan = NULL;
or_circuit_t *orcirc = NULL;
cell_t *cell = NULL;
int old_count, new_count;
(void)arg;
/* Make fake channels to be nchan and pchan for the circuit */
nchan = new_fake_channel();
tt_assert(nchan);
pchan = new_fake_channel();
tt_assert(pchan);
/* We'll need chans with working cmuxes */
nchan->cmux = circuitmux_alloc();
pchan->cmux = circuitmux_alloc();
/* Make a fake orcirc */
orcirc = new_fake_orcirc(nchan, pchan);
tt_assert(orcirc);
circuitmux_attach_circuit(nchan->cmux, TO_CIRCUIT(orcirc),
CELL_DIRECTION_OUT);
circuitmux_attach_circuit(pchan->cmux, TO_CIRCUIT(orcirc),
CELL_DIRECTION_IN);
/* Make a cell */
cell = tor_malloc_zero(sizeof(cell_t));
make_fake_cell(cell);
MOCK(scheduler_channel_has_waiting_cells,
scheduler_channel_has_waiting_cells_mock);
/* Append it */
old_count = get_mock_scheduler_has_waiting_cells_count();
append_cell_to_circuit_queue(TO_CIRCUIT(orcirc), nchan, cell,
CELL_DIRECTION_OUT, 0);
new_count = get_mock_scheduler_has_waiting_cells_count();
tt_int_op(new_count, OP_EQ, old_count + 1);
/* Now try the reverse direction */
old_count = get_mock_scheduler_has_waiting_cells_count();
append_cell_to_circuit_queue(TO_CIRCUIT(orcirc), pchan, cell,
CELL_DIRECTION_IN, 0);
new_count = get_mock_scheduler_has_waiting_cells_count();
tt_int_op(new_count, OP_EQ, old_count + 1);
/* Ensure our write totals are 0 */
tt_u64_op(find_largest_max(write_array), OP_EQ, 0);
/* Mark the circuit for close */
circuit_mark_for_close(TO_CIRCUIT(orcirc), 0);
/* Check our write totals. */
advance_obs(write_array);
commit_max(write_array);
/* Check for two cells plus overhead */
tt_u64_op(find_largest_max(write_array), OP_EQ,
2*(get_cell_network_size(nchan->wide_circ_ids)
+TLS_PER_CELL_OVERHEAD));
UNMOCK(scheduler_channel_has_waiting_cells);
/* Get rid of the fake channels */
MOCK(scheduler_release_channel, scheduler_release_channel_mock);
channel_mark_for_close(nchan);
channel_mark_for_close(pchan);
UNMOCK(scheduler_release_channel);
/* Shut down channels */
channel_free_all();
done:
tor_free(cell);
if (orcirc) {
circuitmux_detach_circuit(nchan->cmux, TO_CIRCUIT(orcirc));
circuitmux_detach_circuit(pchan->cmux, TO_CIRCUIT(orcirc));
cell_queue_clear(&orcirc->base_.n_chan_cells);
cell_queue_clear(&orcirc->p_chan_cells);
}
tor_free(orcirc);
free_fake_channel(nchan);
free_fake_channel(pchan);
return;
}
static void
test_relay_append_cell_to_circuit_queue(void *arg)
{
......@@ -129,6 +229,7 @@ test_relay_append_cell_to_circuit_queue(void *arg)
struct testcase_t relay_tests[] = {
{ "append_cell_to_circuit_queue", test_relay_append_cell_to_circuit_queue,
TT_FORK, NULL, NULL },
{ "close_circ_rephist", test_relay_close_circuit,
TT_FORK, NULL, NULL },
END_OF_TESTCASES
};
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment