diff --git a/changes/bug32040 b/changes/bug32040 new file mode 100644 index 00000000000..1cdc0bec9a5 --- /dev/null +++ b/changes/bug32040 @@ -0,0 +1,7 @@ + o Minor bugfixes (circuitpadding): + - Add the abilility to keep circuit padding machines if they match a set + of circuit state or purposes. This allows us to have machines that start + up under some conditions but don't shut down under others. We now + use this mask to avoid starting up introduction circuit padding + again after the machines have already completed. Fixes bug 32040; + bugfix on 0.4.1.1-alpha. diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 7c8a17001e5..25568f03f03 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -108,10 +108,10 @@ problem dependency-violation /src/core/or/circuitlist.h 1 problem function-size /src/core/or/circuitmux.c:circuitmux_set_policy() 109 problem function-size /src/core/or/circuitmux.c:circuitmux_attach_circuit() 113 problem dependency-violation /src/core/or/circuitmux_ewma.c 2 -problem file-size /src/core/or/circuitpadding.c 3101 +problem file-size /src/core/or/circuitpadding.c 3183 problem function-size /src/core/or/circuitpadding.c:circpad_machine_schedule_padding() 113 problem dependency-violation /src/core/or/circuitpadding.c 6 -problem file-size /src/core/or/circuitpadding.h 813 +problem file-size /src/core/or/circuitpadding.h 832 problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_relay_hide_intro_circuits() 103 problem function-size /src/core/or/circuitpadding_machines.c:circpad_machine_client_hide_rend_circuits() 112 problem dependency-violation /src/core/or/circuitpadding_machines.c 1 diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index b958fec4f91..889ffb03f1e 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -2008,7 +2008,7 @@ circpad_internal_event_state_length_up(circpad_machine_runtime_t *mi) * Returns true if the circuit matches the conditions. */ static inline bool -circpad_machine_conditions_met(origin_circuit_t *circ, +circpad_machine_conditions_apply(origin_circuit_t *circ, const circpad_machine_spec_t *machine) { /* If padding is disabled, no machines should match/apply. This has @@ -2025,7 +2025,7 @@ circpad_machine_conditions_met(origin_circuit_t *circ, } if (!(circpad_circ_purpose_to_mask(TO_CIRCUIT(circ)->purpose) - & machine->conditions.purpose_mask)) + & machine->conditions.apply_purpose_mask)) return 0; if (machine->conditions.requires_vanguards) { @@ -2041,7 +2041,7 @@ circpad_machine_conditions_met(origin_circuit_t *circ, * "I want to apply to circuits with either streams or no streams"; OR * "I only want to apply to circuits with streams"; OR * "I only want to apply to circuits without streams". */ - if (!(circpad_circuit_state(circ) & machine->conditions.state_mask)) + if (!(circpad_circuit_state(circ) & machine->conditions.apply_state_mask)) return 0; if (circuit_get_cpath_opened_len(circ) < machine->conditions.min_hops) @@ -2050,6 +2050,26 @@ circpad_machine_conditions_met(origin_circuit_t *circ, return 1; } +/** + * Check to see if any of the keep conditions still apply to this circuit. + * + * These conditions keep the machines active if they match, but do not + * cause new machines to start up. + */ +static inline bool +circpad_machine_conditions_keep(origin_circuit_t *circ, + const circpad_machine_spec_t *machine) +{ + if ((circpad_circ_purpose_to_mask(TO_CIRCUIT(circ)->purpose) + & machine->conditions.keep_purpose_mask)) + return 1; + + if ((circpad_circuit_state(circ) & machine->conditions.keep_state_mask)) + return 1; + + return 0; +} + /** * Returns a minimized representation of the circuit state. * @@ -2115,7 +2135,12 @@ circpad_shutdown_old_machines(origin_circuit_t *on_circ) circuit_t *circ = TO_CIRCUIT(on_circ); FOR_EACH_ACTIVE_CIRCUIT_MACHINE_BEGIN(i, circ) { - if (!circpad_machine_conditions_met(on_circ, + /* We shut down a machine if neither the apply conditions + * nor the keep conditions match. If either set of conditions match, + * keep it around. */ + if (!circpad_machine_conditions_apply(on_circ, + circ->padding_machine[i]) && + !circpad_machine_conditions_keep(on_circ, circ->padding_machine[i])) { uint32_t machine_ctr = circ->padding_info[i]->machine_ctr; // Clear machineinfo (frees timers) @@ -2174,7 +2199,7 @@ circpad_add_matching_machines(origin_circuit_t *on_circ, * machines installed on a circuit. Make sure we only * add this machine if its target machine index is free. */ if (machine->machine_index == i && - circpad_machine_conditions_met(on_circ, machine)) { + circpad_machine_conditions_apply(on_circ, machine)) { // We can only replace this machine if the target hopnum // is the same, otherwise we'll get invalid data @@ -2587,9 +2612,9 @@ circpad_circ_client_machine_init(void) = tor_malloc_zero(sizeof(circpad_machine_spec_t)); circ_client_machine->conditions.min_hops = 2; - circ_client_machine->conditions.state_mask = + circ_client_machine->conditions.apply_state_mask = CIRCPAD_CIRC_BUILDING|CIRCPAD_CIRC_OPENED|CIRCPAD_CIRC_HAS_RELAY_EARLY; - circ_client_machine->conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; + circ_client_machine->conditions.apply_purpose_mask = CIRCPAD_PURPOSE_ALL; circ_client_machine->conditions.reduced_padding_ok = 1; circ_client_machine->target_hopnum = 2; diff --git a/src/core/or/circuitpadding.h b/src/core/or/circuitpadding.h index 4fadcb742a9..3d2929cf74d 100644 --- a/src/core/or/circuitpadding.h +++ b/src/core/or/circuitpadding.h @@ -173,11 +173,21 @@ typedef struct circpad_machine_conditions_t { /** Only apply the machine *if* the circuit's state matches any of * the bits set in this bitmask. */ - circpad_circuit_state_t state_mask; + circpad_circuit_state_t apply_state_mask; /** Only apply a machine *if* the circuit's purpose matches one * of the bits set in this bitmask */ - circpad_purpose_mask_t purpose_mask; + circpad_purpose_mask_t apply_purpose_mask; + + /** Keep a machine if any of the circuits's state machine's match + * the bits set in this bitmask, but don't apply new machines if + * they match this mask. */ + circpad_circuit_state_t keep_state_mask; + + /** Keep a machine if any of the circuits's state machine's match + * the bits set in this bitmask, but don't apply new machines if + * they match this mask. */ + circpad_purpose_mask_t keep_purpose_mask; } circpad_machine_conditions_t; diff --git a/src/core/or/circuitpadding_machines.c b/src/core/or/circuitpadding_machines.c index 98767f9e8f2..1e6b580f5be 100644 --- a/src/core/or/circuitpadding_machines.c +++ b/src/core/or/circuitpadding_machines.c @@ -67,7 +67,7 @@ circpad_machine_client_hide_intro_circuits(smartlist_t *machines_sl) client_machine->name = "client_ip_circ"; - client_machine->conditions.state_mask = CIRCPAD_CIRC_OPENED; + client_machine->conditions.apply_state_mask = CIRCPAD_CIRC_OPENED; client_machine->target_hopnum = 2; /* This is a client machine */ @@ -102,9 +102,18 @@ circpad_machine_client_hide_intro_circuits(smartlist_t *machines_sl) * INTRO_MACHINE_MAXIMUM_PADDING cells, to match the "...(inbound data cells * continue)" portion of the trace (aka the rest of an HTTPS response body). */ - client_machine->conditions.purpose_mask = - circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT)| - circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)| + + /* Start the machine on fresh intro circs. */ + client_machine->conditions.apply_purpose_mask = + circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT); + + /* If the client purpose changes back to CIRCUIT_PURPOSE_C_INTRODUCING, + * or transitions to CIRCUIT_PURPOSE_C_INTRODUCE_ACKED, keep the machine + * alive, but do not launch new machines for these purposes. Also + * keep the machine around if it is in the CIRCUIT_PADDING purpose + * (but do not try to take over other machines in that purpose). */ + client_machine->conditions.keep_purpose_mask = + circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_INTRODUCE_ACKED) | circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_CIRCUIT_PADDING); /* Keep the circuit alive even after the introduction has been finished, @@ -152,7 +161,7 @@ circpad_machine_relay_hide_intro_circuits(smartlist_t *machines_sl) relay_machine->name = "relay_ip_circ"; - relay_machine->conditions.state_mask = CIRCPAD_CIRC_OPENED; + relay_machine->conditions.apply_state_mask = CIRCPAD_CIRC_OPENED; /* This is a relay-side machine */ relay_machine->is_origin_side = 0; @@ -263,7 +272,7 @@ circpad_machine_client_hide_rend_circuits(smartlist_t *machines_sl) client_machine->name = "client_rp_circ"; /* Only pad after the circuit has been built and pad to the middle */ - client_machine->conditions.state_mask = CIRCPAD_CIRC_OPENED; + client_machine->conditions.apply_state_mask = CIRCPAD_CIRC_OPENED; client_machine->target_hopnum = 2; /* This is a client machine */ @@ -299,7 +308,7 @@ circpad_machine_client_hide_rend_circuits(smartlist_t *machines_sl) * * Hence this way we make rendezvous circuits look like general circuits up * till the end of the circuit setup. */ - client_machine->conditions.purpose_mask = + client_machine->conditions.apply_purpose_mask = circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_REND_JOINED)| circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_REND_READY)| circpad_circ_purpose_to_mask(CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED); @@ -383,7 +392,7 @@ circpad_machine_relay_hide_rend_circuits(smartlist_t *machines_sl) /* Only pad after the circuit has been built and pad to the middle */ relay_machine->conditions.min_hops = 2; - relay_machine->conditions.state_mask = CIRCPAD_CIRC_OPENED; + relay_machine->conditions.apply_state_mask = CIRCPAD_CIRC_OPENED; /* This is a relay-side machine */ relay_machine->is_origin_side = 0; diff --git a/src/test/test_circuitpadding.c b/src/test/test_circuitpadding.c index cc8c63d6082..c3346e2e03d 100644 --- a/src/test/test_circuitpadding.c +++ b/src/test/test_circuitpadding.c @@ -1733,9 +1733,9 @@ helper_create_conditional_machines(void) add->conditions.requires_vanguards = 0; add->conditions.min_hops = 2; - add->conditions.state_mask = CIRCPAD_CIRC_BUILDING| + add->conditions.apply_state_mask = CIRCPAD_CIRC_BUILDING| CIRCPAD_CIRC_NO_STREAMS|CIRCPAD_CIRC_HAS_RELAY_EARLY; - add->conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; + add->conditions.apply_purpose_mask = CIRCPAD_PURPOSE_ALL; circpad_register_padding_machine(add, origin_padding_machines); add = helper_create_conditional_machine(); @@ -1752,9 +1752,9 @@ helper_create_conditional_machines(void) add->conditions.requires_vanguards = 1; add->conditions.min_hops = 3; - add->conditions.state_mask = CIRCPAD_CIRC_OPENED| + add->conditions.apply_state_mask = CIRCPAD_CIRC_OPENED| CIRCPAD_CIRC_STREAMS|CIRCPAD_CIRC_HAS_NO_RELAY_EARLY; - add->conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; + add->conditions.apply_purpose_mask = CIRCPAD_PURPOSE_ALL; circpad_register_padding_machine(add, origin_padding_machines); add = helper_create_conditional_machine(); @@ -2728,8 +2728,8 @@ helper_create_ender_machine(void) circ_client_machine.states[CIRCPAD_STATE_START]. next_state[CIRCPAD_EVENT_NONPADDING_RECV] = CIRCPAD_STATE_END; - circ_client_machine.conditions.state_mask = CIRCPAD_STATE_ALL; - circ_client_machine.conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; + circ_client_machine.conditions.apply_state_mask = CIRCPAD_STATE_ALL; + circ_client_machine.conditions.apply_purpose_mask = CIRCPAD_PURPOSE_ALL; } static time_t mocked_timeofday;