diff --git a/changes/bug1297a b/changes/bug1297a new file mode 100644 index 0000000000..140b94e3b0 --- /dev/null +++ b/changes/bug1297a @@ -0,0 +1,16 @@ + o Major bugfixes: + - Apply circuit timeouts to opened hidden-service-related circuits + based on the correct start time. Previously, we would apply the + circuit build timeout based on time since the circuit's + creation; it was supposed to be applied based on time since the + circuit entered its current state. Bugfix on 0.0.6; fixes part + of bug 1297. + - Use the same circuit timeout for client-side introduction + circuits as for other four-hop circuits. Previously, + client-side introduction circuits were closed after the same + timeout as single-hop directory-fetch circuits; this was + appropriate with the static circuit build timeout in 0.2.1.x and + earlier, but caused many hidden service access attempts to fail + with the adaptive CBT introduced in 0.2.2.2-alpha. Bugfix on + 0.2.2.2-alpha; fixes another part of bug 1297. + diff --git a/src/or/circuituse.c b/src/or/circuituse.c index f176f346f8..f7274bd161 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -289,7 +289,6 @@ circuit_expire_building(void) struct timeval general_cutoff, begindir_cutoff, fourhop_cutoff, cannibalize_cutoff, close_cutoff, extremely_old_cutoff; struct timeval now; - struct timeval introcirc_cutoff; cpath_build_state_t *build_state; tor_gettimeofday(&now); @@ -308,8 +307,6 @@ circuit_expire_building(void) SET_CUTOFF(close_cutoff, circ_times.close_ms); SET_CUTOFF(extremely_old_cutoff, circ_times.close_ms*2 + 1000); - introcirc_cutoff = begindir_cutoff; - while (next_circ) { struct timeval cutoff; victim = next_circ; @@ -326,8 +323,6 @@ circuit_expire_building(void) cutoff = fourhop_cutoff; else if (TO_ORIGIN_CIRCUIT(victim)->has_opened) cutoff = cannibalize_cutoff; - else if (victim->purpose == CIRCUIT_PURPOSE_C_INTRODUCING) - cutoff = introcirc_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) cutoff = close_cutoff; else @@ -338,12 +333,6 @@ circuit_expire_building(void) #if 0 /* some debug logs, to help track bugs */ - if (victim->purpose == CIRCUIT_PURPOSE_C_INTRODUCING && - victim->timestamp_created.tv_sec <= introcirc_cutoff && - victim->timestamp_created.tv_sec > general_cutoff) - log_info(LD_REND|LD_CIRC, "Timing out introduction circuit which we " - "would not have done if it had been a general circuit."); - if (victim->purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && victim->purpose <= CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED) { if (!victim->timestamp_dirty) diff --git a/src/or/or.h b/src/or/or.h index 4b20b57d1e..4aa1ba2d27 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2308,6 +2308,11 @@ typedef struct circuit_t { * in time in order to indicate that a circuit shouldn't be used for new * streams, but that it can stay alive as long as it has streams on it. * That's a kludge we should fix. + * + * XXX023 The CBT code uses this field to record when HS-related + * circuits entered certain states. This usage probably won't + * interfere with this field's primary purpose, but we should + * document it more thoroughly to make sure of that. */ time_t timestamp_dirty; diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 3a2aaf1c41..d135dc53f6 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -276,6 +276,10 @@ rend_client_send_introduction(origin_circuit_t *introcirc, /* Now, we wait for an ACK or NAK on this circuit. */ introcirc->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT; + /* Set timestamp_dirty, because circuit_expire_building expects it + * to specify when a circuit entered the _C_INTRODUCE_ACK_WAIT + * state. */ + introcirc->_base.timestamp_dirty = time(NULL); return 0; perm_err: @@ -330,6 +334,10 @@ rend_client_introduction_acked(origin_circuit_t *circ, circ->rend_data->onion_address, CIRCUIT_PURPOSE_C_REND_READY); if (rendcirc) { /* remember the ack */ rendcirc->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED; + /* Set timestamp_dirty, because circuit_expire_building expects + * it to specify when a circuit entered the + * _C_REND_READY_INTRO_ACKED state. */ + rendcirc->_base.timestamp_dirty = time(NULL); } else { log_info(LD_REND,"...Found no rend circ. Dropping on the floor."); } @@ -677,6 +685,9 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, log_info(LD_REND,"Got rendezvous ack. This circuit is now ready for " "rendezvous."); circ->_base.purpose = CIRCUIT_PURPOSE_C_REND_READY; + /* Set timestamp_dirty, because circuit_expire_building expects it + * to specify when a circuit entered the _C_REND_READY state. */ + circ->_base.timestamp_dirty = time(NULL); /* XXXX023 This is a pretty brute-force approach. It'd be better to * attach only the connections that are waiting on this circuit, rather * than trying to attach them all. See comments bug 743. */