From 7a0ff5beb21e6485bda44c0545036bb2883f643d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 13 Sep 2018 20:47:41 -0400 Subject: [PATCH 01/11] In conditionvar_timeout test, wait for threads to timeout Previously we just waited 200msec, which was not enough on slow/busy systems. Fixes bug 27073; bugfix on 0.2.6.3-alpha when the test was introduced. --- changes/bug27073 | 4 ++++ src/test/test_threads.c | 30 +++++++++++++++++++----------- 2 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 changes/bug27073 diff --git a/changes/bug27073 b/changes/bug27073 new file mode 100644 index 0000000000..851e7f0df6 --- /dev/null +++ b/changes/bug27073 @@ -0,0 +1,4 @@ + o Minor bugfixes (testing): + - Revise the "conditionvar_timeout" test so that it succeeds even + on heavily loaded systems where the test threads are not scheduled + within 200 msec. Fixes bug 27073; bugfix on 0.2.6.3-alpha. diff --git a/src/test/test_threads.c b/src/test/test_threads.c index ebbc95c7ca..448ab2034b 100644 --- a/src/test/test_threads.c +++ b/src/test/test_threads.c @@ -234,25 +234,33 @@ test_threads_conditionvar(void *arg) if (timeout) { ti->tv = &msec100; } + +#define SPIN_UNTIL(condition,sleep_msec) \ + while (1) { \ + tor_mutex_acquire(ti->mutex); \ + if (condition) { \ + break; \ + } \ + tor_mutex_release(ti->mutex); \ + tor_sleep_msec(sleep_msec); \ + } + spawn_func(cv_test_thr_fn_, ti); spawn_func(cv_test_thr_fn_, ti); spawn_func(cv_test_thr_fn_, ti); spawn_func(cv_test_thr_fn_, ti); - tor_mutex_acquire(ti->mutex); + SPIN_UNTIL(ti->n_threads == 4, 10); + + time_t started_at = time(NULL); + ti->addend = 7; ti->shutdown = 1; tor_cond_signal_one(ti->cond); tor_mutex_release(ti->mutex); #define SPIN() \ - while (1) { \ - tor_mutex_acquire(ti->mutex); \ - if (ti->addend == 0) { \ - break; \ - } \ - tor_mutex_release(ti->mutex); \ - } + SPIN_UNTIL(ti->addend == 0, 0) SPIN(); @@ -279,8 +287,9 @@ test_threads_conditionvar(void *arg) if (!timeout) { tt_int_op(ti->n_shutdown, ==, 4); } else { - tor_sleep_msec(200); - tor_mutex_acquire(ti->mutex); + const int GIVE_UP_AFTER_SEC = 30; + SPIN_UNTIL((ti->n_timeouts == 2 || + time(NULL) >= started_at + GIVE_UP_AFTER_SEC), 10); tt_int_op(ti->n_shutdown, ==, 2); tt_int_op(ti->n_timeouts, ==, 2); tor_mutex_release(ti->mutex); @@ -301,4 +310,3 @@ struct testcase_t thread_tests[] = { &passthrough_setup, (void*)"tv" }, END_OF_TESTCASES }; - From 7dc435e6bc4361ab52eac834eac5d30351f4f7b7 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 14 Sep 2018 18:35:44 +0000 Subject: [PATCH 02/11] Promote rep_hist bw functions to uint64_t. The rest of rephist uses uint64_t internally. Let's make these take it too, so we don't have to worry about overflowing size_t on 32 bit systems. --- src/or/rephist.c | 8 ++++---- src/or/rephist.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/or/rephist.c b/src/or/rephist.c index f0bac57898..7223c3759b 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1363,7 +1363,7 @@ bw_arrays_init(void) * earlier than the latest when 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 +1380,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 +1390,7 @@ rep_hist_note_bytes_read(size_t num_bytes, time_t when) * when. (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 +1399,7 @@ rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when) * when. (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); } diff --git a/src/or/rephist.h b/src/or/rephist.h index ff4810a56d..c1fc7ef1aa 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -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); int rep_hist_bandwidth_assess(void); char *rep_hist_get_bandwidth_lines(void); From bbaa398d268cda00e1b52fc2ebbe28f038b7db8f Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 14 Sep 2018 18:50:40 +0000 Subject: [PATCH 03/11] 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 8b5863a461..463f7d928c 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 977afca18d..8cfdd3bb94 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 circ 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 2707b426ab..2ede6f76cd 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 024a9cff0f..9f53c80644 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -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 diff --git a/src/or/relay.c b/src/or/relay.c index 1c791e02cc..d1c7820c7c 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); From 2ceda207d3343f4a3d148ffb2e9543c39a3a97ee Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Thu, 13 Sep 2018 19:19:03 +0000 Subject: [PATCH 04/11] Bug 23512: Changes file. --- changes/bug23512 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/bug23512 diff --git a/changes/bug23512 b/changes/bug23512 new file mode 100644 index 0000000000..91b2786de4 --- /dev/null +++ b/changes/bug23512 @@ -0,0 +1,6 @@ + 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. From 6af352172d3dbf1571c8cb62cd0a618f4e2355d1 Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Thu, 13 Sep 2018 23:03:32 +0000 Subject: [PATCH 05/11] Bug 23512: Test recording bytes in circ queues. --- src/or/rephist.c | 13 +++-- src/or/rephist.h | 2 + src/test/test_relay.c | 112 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 9 deletions(-) diff --git a/src/or/rephist.c b/src/or/rephist.c index 7223c3759b..211480284a 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -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. */ @@ -1236,7 +1241,7 @@ typedef struct bw_array_t { } 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 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; @@ -1408,7 +1413,7 @@ rep_hist_note_dir_bytes_read(uint64_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; diff --git a/src/or/rephist.h b/src/or/rephist.h index c1fc7ef1aa..837bb8d043 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -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 diff --git a/src/test/test_relay.c b/src/test/test_relay.c index 4713c79ea5..1ec538395b 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -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_int_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_int_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,8 @@ 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 }; From dfd3823047d05a878a7a559ab6fc539dcad240fc Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 14 Sep 2018 20:28:47 +0000 Subject: [PATCH 06/11] Bug 23512: Mock assert_circuit_ok in tests. This mocking was not available in 0.2.9. --- src/test/test_relay.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/test/test_relay.c b/src/test/test_relay.c index ce8ff1a917..f18de805f6 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -48,18 +48,19 @@ new_fake_orcirc(channel_t *nchan, channel_t *pchan) circ->deliver_window = CIRCWINDOW_START_MAX; circ->n_chan_create_cell = NULL; - /* 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 +assert_circuit_ok_mock(const circuit_t *c) +{ + (void) c; + return; +} + static void test_relay_close_circuit(void *arg) { @@ -95,6 +96,8 @@ test_relay_close_circuit(void *arg) MOCK(scheduler_channel_has_waiting_cells, scheduler_channel_has_waiting_cells_mock); + MOCK(assert_circuit_ok, + assert_circuit_ok_mock); /* Append it */ old_count = get_mock_scheduler_has_waiting_cells_count(); @@ -146,6 +149,7 @@ test_relay_close_circuit(void *arg) tor_free(orcirc); free_fake_channel(nchan); free_fake_channel(pchan); + UNMOCK(assert_circuit_ok); return; } From dd15998d289e849d8524b70f2edeb5a92ece971d Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Mon, 17 Sep 2018 22:08:45 +0000 Subject: [PATCH 07/11] Remove duplicate TLS define from kist code. Duplicate comes from introducing this define into 0.2.9, which did not yet have KIST. --- src/or/scheduler_kist.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index c79b413b88..392a2545d7 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -16,8 +16,6 @@ #define SCHEDULER_PRIVATE_ #include "scheduler.h" -#define TLS_PER_CELL_OVERHEAD 29 - #ifdef HAVE_KIST_SUPPORT /* Kernel interface needed for KIST. */ #include From ad10cafd9f0157e6aaa6f1f68ab7d3ef9b8b1b2e Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Fri, 14 Sep 2018 20:45:21 +0000 Subject: [PATCH 08/11] Bug 23512: Test fix: cmux is now allocated by new_fake_channel() --- src/test/test_relay.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/test_relay.c b/src/test/test_relay.c index 5e226ade54..ffb0134426 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -78,10 +78,6 @@ test_relay_close_circuit(void *arg) 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); From 72e30f26ec291baba202b71e121c3211af2db371 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 20 Sep 2018 14:18:09 -0400 Subject: [PATCH 09/11] Fix a pair of errors in bug23512 --- src/or/rephist.c | 5 ++--- src/test/test_relay.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/or/rephist.c b/src/or/rephist.c index 211480284a..20f9148f03 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1211,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]; @@ -1238,7 +1238,7 @@ 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 @@ -3297,4 +3297,3 @@ rep_hist_free_all(void) tor_assert_nonfatal(rephist_total_alloc == 0); tor_assert_nonfatal_once(rephist_total_num == 0); } - diff --git a/src/test/test_relay.c b/src/test/test_relay.c index 1ec538395b..57dcb2406a 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -111,7 +111,7 @@ test_relay_close_circuit(void *arg) tt_int_op(new_count, OP_EQ, old_count + 1); /* Ensure our write totals are 0 */ - tt_int_op(find_largest_max(write_array), OP_EQ, 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); @@ -120,7 +120,7 @@ test_relay_close_circuit(void *arg) advance_obs(write_array); commit_max(write_array); /* Check for two cells plus overhead */ - tt_int_op(find_largest_max(write_array), OP_EQ, + tt_u64_op(find_largest_max(write_array), OP_EQ, 2*(get_cell_network_size(nchan->wide_circ_ids) +TLS_PER_CELL_OVERHEAD)); @@ -233,4 +233,3 @@ struct testcase_t relay_tests[] = { TT_FORK, NULL, NULL }, END_OF_TESTCASES }; - From 2fbc58cf07fd7367ddaf81b82868b5f37d7883ae Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 22 Oct 2018 14:02:43 +1000 Subject: [PATCH 10/11] Windows: fix uname on recent Windows versions Correctly identify Windows 8.1, Windows 10, and Windows Server 2008 and later from their NT versions. On recent Windows versions, the GetVersionEx() function may report an earlier Windows version than the running OS. To avoid user confusion, add "[or later]" to Tor's version string on affected versions of Windows. Remove Windows versions that were never supported by the GetVersionEx() function. Stop duplicating the latest Windows version in get_uname(). Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly. --- changes/bug28096 | 13 +++++++ src/common/compat.c | 95 ++++++++++++++++++++++++++++++--------------- 2 files changed, 77 insertions(+), 31 deletions(-) create mode 100644 changes/bug28096 diff --git a/changes/bug28096 b/changes/bug28096 new file mode 100644 index 0000000000..6847df9798 --- /dev/null +++ b/changes/bug28096 @@ -0,0 +1,13 @@ + o Minor bugfixes (Windows): + - Correctly identify Windows 8.1, Windows 10, and Windows Server 2008 + and later from their NT versions. + Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly. + - On recent Windows versions, the GetVersionEx() function may report + an earlier Windows version than the running OS. To avoid user + confusion, add "[or later]" to Tor's version string on affected + versions of Windows. + Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly. + - Remove Windows versions that were never supported by the + GetVersionEx() function. Stop duplicating the latest Windows + version in get_uname(). + Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly. diff --git a/src/common/compat.c b/src/common/compat.c index 4ac443c134..9758751122 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -2689,22 +2689,33 @@ MOCK_IMPL(const char *, get_uname, (void)) #ifdef _WIN32 OSVERSIONINFOEX info; int i; + int is_client = 0; + int is_server = 0; const char *plat = NULL; static struct { - unsigned major; unsigned minor; const char *version; + unsigned major; unsigned minor; + const char *client_version; const char *server_version; } win_version_table[] = { - { 6, 2, "Windows 8" }, - { 6, 1, "Windows 7" }, - { 6, 0, "Windows Vista" }, - { 5, 2, "Windows Server 2003" }, - { 5, 1, "Windows XP" }, - { 5, 0, "Windows 2000" }, - /* { 4, 0, "Windows NT 4.0" }, */ - { 4, 90, "Windows Me" }, - { 4, 10, "Windows 98" }, - /* { 4, 0, "Windows 95" } */ - { 3, 51, "Windows NT 3.51" }, - { 0, 0, NULL } + /* This table must be sorted in descending order. + * Sources: + * https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions + * https://docs.microsoft.com/en-us/windows/desktop/api/winnt/ + * ns-winnt-_osversioninfoexa#remarks + */ + /* Windows Server 2019 is indistinguishable from Windows Server 2016 + * using GetVersionEx(). + { 10, 0, NULL, "Windows Server 2019" }, */ + { 10, 0, "Windows 10", "Windows Server 2016" }, + { 6, 3, "Windows 8.1", "Windows Server 2012 R2" }, + { 6, 2, "Windows 8", "Windows Server 2012" }, + { 6, 1, "Windows 7", "Windows Server 2008 R2" }, + { 6, 0, "Windows Vista", "Windows Server 2008" }, + { 5, 2, "Windows XP Professional", "Windows Server 2003" }, + /* Windows XP did not have a server version, but we need something here */ + { 5, 1, "Windows XP", "Windows XP Server" }, + { 5, 0, "Windows 2000 Professional", "Windows 2000 Server" }, + /* Earlier versions are not supported by GetVersionEx(). */ + { 0, 0, NULL, NULL } }; memset(&info, 0, sizeof(info)); info.dwOSVersionInfoSize = sizeof(info); @@ -2714,25 +2725,34 @@ MOCK_IMPL(const char *, get_uname, (void)) uname_result_is_set = 1; return uname_result; } - if (info.dwMajorVersion == 4 && info.dwMinorVersion == 0) { - if (info.dwPlatformId == VER_PLATFORM_WIN32_NT) - plat = "Windows NT 4.0"; - else - plat = "Windows 95"; +#ifdef VER_NT_SERVER + if (info.wProductType == VER_NT_SERVER || + info.wProductType == VER_NT_DOMAIN_CONTROLLER) { + is_server = 1; } else { - for (i=0; win_version_table[i].major>0; ++i) { - if (win_version_table[i].major == info.dwMajorVersion && - win_version_table[i].minor == info.dwMinorVersion) { - plat = win_version_table[i].version; - break; + is_client = 1; + } +#endif + /* Search the version table for a matching version */ + for (i=0; win_version_table[i].major>0; ++i) { + if (win_version_table[i].major == info.dwMajorVersion && + win_version_table[i].minor == info.dwMinorVersion) { + if (is_server) { + plat = win_version_table[i].server_version; + } else { + /* Use client versions for clients, and when we don't know if it + * is a client or a server. */ + plat = win_version_table[i].client_version; } + break; } } if (plat) { strlcpy(uname_result, plat, sizeof(uname_result)); } else { - if (info.dwMajorVersion > 6 || - (info.dwMajorVersion==6 && info.dwMinorVersion>2)) + if (info.dwMajorVersion > win_version_table[0].major || + (info.dwMajorVersion == win_version_table[0].major && + info.dwMinorVersion > win_version_table[0].minor)) tor_snprintf(uname_result, sizeof(uname_result), "Very recent version of Windows [major=%d,minor=%d]", (int)info.dwMajorVersion,(int)info.dwMinorVersion); @@ -2741,12 +2761,25 @@ MOCK_IMPL(const char *, get_uname, (void)) "Unrecognized version of Windows [major=%d,minor=%d]", (int)info.dwMajorVersion,(int)info.dwMinorVersion); } -#ifdef VER_NT_SERVER - if (info.wProductType == VER_NT_SERVER || - info.wProductType == VER_NT_DOMAIN_CONTROLLER) { - strlcat(uname_result, " [server]", sizeof(uname_result)); - } -#endif + /* Now append extra information to the name. + * + * Microsoft's API documentation says that on Windows 8.1 and later, + * GetVersionEx returns Windows 8 (6.2) for applications without an + * app compatibility manifest (including tor's default build). + * + * But in our testing, we have seen the actual Windows version on + * Windows Server 2012 R2, even without a manifest. */ + if (info.dwMajorVersion > 6 || + (info.dwMajorVersion == 6 && info.dwMinorVersion >= 2)) { + /* When GetVersionEx() returns Windows 8, the actual OS may be any + * later version. */ + strlcat(uname_result, " [or later]", sizeof(uname_result)); + } + /* When we don't know if the OS is a client or server version, we use + * the client version, and this qualifier. */ + if (!is_server && !is_client) { + strlcat(uname_result, " [client or server]", sizeof(uname_result)); + } #else /* LCOV_EXCL_START -- can't provoke uname failure */ strlcpy(uname_result, "Unknown platform", sizeof(uname_result)); From e8d84b18aabf6a3d32a41700142048224e83ed8b Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Mon, 16 Apr 2018 22:49:27 +0000 Subject: [PATCH 11/11] Bug 25733: Avoid assert failure if all circuits time out. Prior to #23100, we were not counting HS circuit build times in our calculation of the timeout. This could lead to a condition where our timeout was set too low, based on non HS circuit build times, and then we would abandon all HS circuits, storing no valid timeouts in the histogram. This commit avoids the assert. --- changes/bug25733 | 4 ++++ src/or/circuitstats.c | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 changes/bug25733 diff --git a/changes/bug25733 b/changes/bug25733 new file mode 100644 index 0000000000..775c1ae00e --- /dev/null +++ b/changes/bug25733 @@ -0,0 +1,4 @@ + o Minor bugfixes (Assert crash): + - Avoid an assert in the circuit build timeout code if we fail to + allow any circuits to actually complete. Fixes bug 25733; + bugfix on 0.2.2.2-alpha. diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 418acc0024..735b3f0fc2 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -753,11 +753,23 @@ circuit_build_times_get_xm(circuit_build_times_t *cbt) histogram[nth_max_bin[n]]); } - /* The following assert is safe, because we don't get called when we - * haven't observed at least CBT_MIN_MIN_CIRCUITS_TO_OBSERVE circuits. */ + /* bin_counts can become zero if all of our last CBT_NCIRCUITS_TO_OBSERVE + * circuits were abandoned before they completed. This shouldn't happen, + * though. We should have reset/re-learned a lower timeout first. */ + if (bin_counts == 0) { + ret = 0; + log_warn(LD_CIRC, + "No valid circuit build time data out of %d times, %u modes, " + "have_timeout=%d, %lfms", cbt->total_build_times, num_modes, + cbt->have_computed_timeout, cbt->timeout_ms); + goto done; + } + tor_assert(bin_counts > 0); ret /= bin_counts; + + done: tor_free(histogram); tor_free(nth_max_bin); @@ -1043,6 +1055,10 @@ circuit_build_times_update_alpha(circuit_build_times_t *cbt) * and less frechet-like. */ cbt->Xm = circuit_build_times_get_xm(cbt); + /* If Xm came back 0, then too many circuits were abandoned. */ + if (cbt->Xm == 0) + return 0; + tor_assert(cbt->Xm > 0); for (i=0; i< CBT_NCIRCUITS_TO_OBSERVE; i++) {