commit 662825474cb7d5fb0e713844aef5e912edd6ed8a Author: Mike Perry <mikeperry-git@torproject.org> Date: Thu Apr 18 20:40:55 2019 +0000 Bug 28780: Make use of purpose to keep padding circuits open. When a circuit is marked for close, check to see if any of our padding machines want to take ownership of it and continue padding until the machine hits the END state. For safety, we also ensure that machines that do not terminate are still closed as follows: Because padding machine timers are UINT32_MAX in size, if some sort of network event doesn't happen on a padding-only circuit within that time, we can conclude it is deadlocked and allow circuit_expire_old_circuits_clientside() to close it. If too much network activity happens, then per-machine padding limits can be used to cease padding, which will cause network cell events to cease, on the circuit, which will cause circpad to abandon the circuit as per the above time limit. --- src/core/or/circuitlist.c | 5 ++ src/core/or/circuitpadding.c | 119 ++++++++++++++++++++++++++++++++++++++++++- src/core/or/circuitpadding.h | 31 +++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c index 91a02ce56..72952a8a5 100644 --- a/src/core/or/circuitlist.c +++ b/src/core/or/circuitlist.c @@ -2195,6 +2195,11 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line, tor_assert(line); tor_assert(file); + /* Check whether the circuitpadding subsystem wants to block this close */ + if (circpad_marked_circuit_for_padding(circ, reason)) { + return; + } + if (circ->marked_for_close) { log_warn(LD_BUG, "Duplicate call to circuit_mark_for_close at %s:%d" diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index 58e8e053c..68140356c 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -163,6 +163,116 @@ circpad_circuit_machineinfo_free_idx(circuit_t *circ, int idx) } } +/** + * Return true if circpad has decided to hold the circuit open for additional + * padding. This function is used to take and retain ownership of certain + * types of circuits that have padding machines on them, that have been passed + * to circuit_mark_for_close(). + * + * circuit_mark_for_close() calls this function to ask circpad if any padding + * machines want to keep the circuit open longer to pad. + * + * Any non-measurement circuit that was closed for a normal, non-error reason + * code may be held open for up to CIRCPAD_DELAY_INFINITE microseconds between + * network-driven cell events. + * + * After CIRCPAD_DELAY_INFINITE microseconds of silence on a circuit, this + * function will no longer hold it open (it will return 0 regardless of + * what the machines ask for, and thus circuit_expire_old_circuits_clientside() + * will close the circuit after roughly 1.25hr of idle time, maximum, + * regardless of the padding machine state. + */ +int +circpad_marked_circuit_for_padding(circuit_t *circ, int reason) +{ + /* If the circuit purpose is measurement or path bias, don't + * hold it open */ + if (circ->purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING || + circ->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { + return 0; + } + + /* If the circuit is closed for any reason other than these three valid, + * client-side close reasons, do not try to keep it open. It is probably + * damaged or unusable. Note this is OK with vanguards because + * controller-closed circuits have REASON=REQUESTED, so vanguards-closed + * circuits will not be held open (we want them to close ASAP). */ + if (!(reason == END_CIRC_REASON_NONE || + reason == END_CIRC_REASON_FINISHED || + reason == END_CIRC_REASON_IP_NOW_REDUNDANT)) { + return 0; + } + + FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, circ) { + circpad_machine_runtime_t *mi = circ->padding_info[i]; + if (!mi) { + continue; // No padding runtime info; check next machine + } + + const circpad_state_t *state = circpad_machine_current_state(mi); + + /* If we're in END state (NULL here), then check next machine */ + if (!state) { + continue; // check next machine + } + + /* If the machine does not want to control the circuit close itself, then + * check the next machine */ + if (!circ->padding_machine[i]->manage_circ_lifetime) { + continue; // check next machine + } + + /* If the machine has reached the END state, we can close. Check next + * machine. */ + if (mi->current_state == CIRCPAD_STATE_END) { + continue; // check next machine + } + + log_info(LD_CIRC, "Circuit %d is not marked for close because of a " + " pending padding machine.", CIRCUIT_IS_ORIGIN(circ) ? + TO_ORIGIN_CIRCUIT(circ)->global_identifier : 0); + + /* If the machine has had no network events at all within the + * last circpad_delay_t timespan, it's in some deadlock state. + * Tell circuit_mark_for_close() that we don't own it anymore. + * This will allow circuit_expire_old_circuits_clientside() to + * close it. + */ + if (circ->padding_info[i]->last_cell_time_sec + + (time_t)CIRCPAD_DELAY_MAX_SECS < approx_time()) { + log_notice(LD_BUG, "Circuit %d was not marked for close because of a " + " pending padding machine for over an hour. Circuit is a %s", + CIRCUIT_IS_ORIGIN(circ) ? + TO_ORIGIN_CIRCUIT(circ)->global_identifier : 0, + circuit_purpose_to_string(circ->purpose)); + + return 0; // abort timer reached; mark the circuit for close now + } + + /* If we weren't marked dirty yet, let's pretend we're dirty now. + * ("Dirty" means that a circuit has been used for application traffic + * by Tor.. Dirty circuits have different expiry times, and are not + * considered in counts of built circuits, etc. By claiming that we're + * dirty, the rest of Tor will make decisions as if we were actually + * used by application data. + * + * This is most important for circuit_expire_old_circuits_clientside(), + * where we want that function to expire us after the padding machine + * has shut down, but using the MaxCircuitDirtiness timer instead of + * the idle circuit timer (again, we want this because we're not + * supposed to look idle to Guard nodes that can see our lifespan). */ + if (!circ->timestamp_dirty) + circ->timestamp_dirty = approx_time(); + + /* Take ownership of the circuit */ + circuit_change_purpose(circ, CIRCUIT_PURPOSE_C_CIRCUIT_PADDING); + + return 1; + } FOR_EACH_ACTIVE_CIRCUIT_MACHINE_END; + + return 0; // No machine wanted to keep the circuit open; mark for close +} + /** Free all the machineinfos in <b>circ</b> that match <b>machine_num</b>. */ static void free_circ_machineinfos_with_machine_num(circuit_t *circ, int machine_num) @@ -197,6 +307,7 @@ circpad_circuit_machineinfo_new(circuit_t *on_circ, int machine_index) tor_malloc_zero(sizeof(circpad_machine_runtime_t)); mi->machine_index = machine_index; mi->on_circ = on_circ; + mi->last_cell_time_sec = approx_time(); return mi; } @@ -1538,7 +1649,8 @@ circpad_cell_event_nonpadding_sent(circuit_t *on_circ) /* If there are no machines then this loop should not iterate */ FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, on_circ) { - /* First, update any RTT estimate */ + /* First, update any timestamps */ + on_circ->padding_info[i]->last_cell_time_sec = approx_time(); circpad_estimate_circ_rtt_on_send(on_circ, on_circ->padding_info[i]); /* Remove a token: this is the idea of adaptive padding, since we have an @@ -1564,7 +1676,8 @@ void circpad_cell_event_nonpadding_received(circuit_t *on_circ) { FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, on_circ) { - /* First, update any RTT estimate */ + /* First, update any timestamps */ + on_circ->padding_info[i]->last_cell_time_sec = approx_time(); circpad_estimate_circ_rtt_on_received(on_circ, on_circ->padding_info[i]); circpad_machine_spec_transition(on_circ->padding_info[i], @@ -1584,6 +1697,7 @@ void circpad_cell_event_padding_sent(circuit_t *on_circ) { FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, on_circ) { + on_circ->padding_info[i]->last_cell_time_sec = approx_time(); circpad_machine_spec_transition(on_circ->padding_info[i], CIRCPAD_EVENT_PADDING_SENT); } FOR_EACH_ACTIVE_CIRCUIT_MACHINE_END; @@ -1602,6 +1716,7 @@ circpad_cell_event_padding_received(circuit_t *on_circ) { /* identical to padding sent */ FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, on_circ) { + on_circ->padding_info[i]->last_cell_time_sec = approx_time(); circpad_machine_spec_transition(on_circ->padding_info[i], CIRCPAD_EVENT_PADDING_RECV); } FOR_EACH_ACTIVE_CIRCUIT_MACHINE_END; diff --git a/src/core/or/circuitpadding.h b/src/core/or/circuitpadding.h index f00369eb0..56ed0a346 100644 --- a/src/core/or/circuitpadding.h +++ b/src/core/or/circuitpadding.h @@ -73,6 +73,7 @@ typedef uint64_t circpad_time_t; /** The type for timer delays, in microseconds */ typedef uint32_t circpad_delay_t; +#define CIRCPAD_DELAY_UNITS_PER_SECOND (1000*1000) /** * An infinite padding cell delay means don't schedule any padding -- @@ -86,6 +87,13 @@ typedef uint32_t circpad_delay_t; #define CIRCPAD_DELAY_INFINITE (UINT32_MAX) /** + * This is the maximum delay that the circuit padding system can have, in + * seconds. + */ +#define CIRCPAD_DELAY_MAX_SECS \ + ((CIRCPAD_DELAY_INFINITE/CIRCPAD_DELAY_UNITS_PER_SECOND)+1) + +/** * Macro to clarify when we're checking the infinity bin. * * Works with either circpad_state_t or circpad_machine_runtime_t @@ -525,6 +533,14 @@ typedef struct circpad_machine_runtime_t { uint16_t nonpadding_sent; /** + * Timestamp of the most recent cell event (sent, received, padding, + * non-padding), in seconds from approx_time(). + * + * Used as an emergency break to stop holding padding circuits open. + */ + time_t last_cell_time_sec; + + /** * EWMA estimate of the RTT of the circuit from this hop * to the exit end, in microseconds. */ circpad_delay_t rtt_estimate_usec; @@ -605,6 +621,19 @@ typedef struct circpad_machine_spec_t { * 1-indexed (ie: hop #1 is guard, #2 middle, #3 exit). */ unsigned target_hopnum : 3; + /** If this flag is enabled, don't close circuits that use this machine even + * if another part of Tor wants to close this circuit. + * + * If this flag is set, the circuitpadding subsystem will close circuits the + * moment the machine transitions to the END state, and only if the circuit + * has already been asked to be closed by another part of Tor. + * + * Circuits that should have been closed but were kept open by a padding + * machine are re-purposed to CIRCUIT_PURPOSE_C_CIRCUIT_PADDING, hence + * machines should take that purpose into account if they are filtering + * circuits by purpose. */ + unsigned manage_circ_lifetime : 1; + /** This machine only kills fascists if the following conditions are met. */ circpad_machine_conditions_t conditions; @@ -630,6 +659,8 @@ typedef struct circpad_machine_spec_t { void circpad_new_consensus_params(const networkstatus_t *ns); +int circpad_marked_circuit_for_padding(circuit_t *circ, int reason); + /** * The following are event call-in points that are of interest to * the state machines. They are called during cell processing. */