diff --git a/changes/ticket40586 b/changes/ticket40586 new file mode 100644 index 0000000000..a872ac2448 --- /dev/null +++ b/changes/ticket40586 @@ -0,0 +1,5 @@ + o Major bugfixes (onion service, congestion control): + - Fix the onion service upload case where the congestion control parameters + were not added to the right object. Fixes bug 40586; bugfix on + 0.4.7.4-alpha. + diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index f8a0e06d90..d253802f79 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -131,6 +131,12 @@ finalize_rend_circuit(origin_circuit_t *circ, crypt_path_t *hop, * so we can actually use it. */ circ->hs_circ_has_timed_out = 0; + /* If congestion control, transfer ccontrol onto the cpath. */ + if (TO_CIRCUIT(circ)->ccontrol) { + hop->ccontrol = TO_CIRCUIT(circ)->ccontrol; + TO_CIRCUIT(circ)->ccontrol = NULL; + } + /* Append the hop to the cpath of this circuit */ cpath_extend_linked_list(&circ->cpath, hop); @@ -410,25 +416,10 @@ launch_rendezvous_point_circuit,(const hs_service_t *service, tor_assert(circ->hs_ident); } + /* Setup congestion control if asked by the client from the INTRO cell. */ if (data->cc_enabled) { - circuit_params_t circ_params = { - .cc_enabled = data->cc_enabled, - .sendme_inc_cells = congestion_control_sendme_inc(), - }; - - /* Initialize ccontrol for appropriate path type */ - if (service->config.is_single_onion) { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_SOS); - } else { - if (get_options()->HSLayer3Nodes) { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_VG); - } else { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION); - } - } + hs_circ_setup_congestion_control(circ, congestion_control_sendme_inc(), + service->config.is_single_onion); } end: @@ -527,21 +518,13 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) new_circ->build_state->expiry_time = bstate->expiry_time; new_circ->hs_ident = hs_ident_circuit_dup(circ->hs_ident); - if (TO_CIRCUIT(circ)->ccontrol != NULL) { - circuit_params_t circ_params = { - .cc_enabled = 1, - .sendme_inc_cells = TO_CIRCUIT(circ)->ccontrol->sendme_inc, - }; - + /* Setup congestion control if asked by the client from the INTRO cell. */ + if (TO_CIRCUIT(circ)->ccontrol) { /* As per above, in this case, we are a full 3 hop rend, even if we're a - * single-onion service */ - if (get_options()->HSLayer3Nodes) { - TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_VG); - } else { - TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_SOS); - } + * single-onion service. */ + hs_circ_setup_congestion_control(new_circ, + TO_CIRCUIT(circ)->ccontrol->sendme_inc, + false); } done: @@ -630,6 +613,41 @@ cleanup_on_free_client_circ(circuit_t *circ) /* Public API */ /* ========== */ +/** Setup on the given circuit congestion control with the given parameters. + * + * This function assumes that congestion control is enabled on the network and + * so it is the caller responsability to make sure of it. */ +void +hs_circ_setup_congestion_control(origin_circuit_t *origin_circ, + uint8_t sendme_inc, bool is_single_onion) +{ + circuit_t *circ = NULL; + circuit_params_t circ_params = {0}; + + tor_assert(origin_circ); + + /* Ease our lives */ + circ = TO_CIRCUIT(origin_circ); + + circ_params.cc_enabled = true; + circ_params.sendme_inc_cells = sendme_inc; + + /* It is setup on the circuit in order to indicate that congestion control is + * enabled. It will be transferred to the RP crypt_path_t once the handshake + * is finalized in finalize_rend_circuit() for both client and service + * because the final hop is not available until then. */ + + if (is_single_onion) { + circ->ccontrol = congestion_control_new(&circ_params, CC_PATH_ONION_SOS); + } else { + if (get_options()->HSLayer3Nodes) { + circ->ccontrol = congestion_control_new(&circ_params, CC_PATH_ONION_VG); + } else { + circ->ccontrol = congestion_control_new(&circ_params, CC_PATH_ONION); + } + } +} + /** Return an introduction point circuit matching the given intro point object. * NULL is returned is no such circuit can be found. */ origin_circuit_t * diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index fbbd5f8f33..808e648951 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -69,6 +69,10 @@ int hs_circuit_setup_e2e_rend_circ_legacy_client(origin_circuit_t *circ, bool hs_circ_is_rend_sent_in_intro1(const origin_circuit_t *circ); +void hs_circ_setup_congestion_control(origin_circuit_t *origin_circ, + uint8_t sendme_inc, + bool is_single_onion); + #ifdef HS_CIRCUIT_PRIVATE struct hs_ntor_rend_cell_keys_t; diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index eb68adfd76..d08b518d94 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -801,8 +801,6 @@ client_intro_circ_has_opened(origin_circuit_t *circ) static void setup_rendezvous_circ_congestion_control(origin_circuit_t *circ) { - circuit_params_t circ_params = {0}; - tor_assert(circ); /* Setup congestion control parameters on the circuit. */ @@ -821,24 +819,13 @@ setup_rendezvous_circ_congestion_control(origin_circuit_t *circ) return; } - /* Take values from the consensus. */ - circ_params.cc_enabled = congestion_control_enabled(); - if (circ_params.cc_enabled) { - circ_params.sendme_inc_cells = desc->encrypted_data.sendme_inc; - - if (desc->encrypted_data.single_onion_service) { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_SOS); - } else { - if (get_options()->HSLayer3Nodes) { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION_VG); - } else { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, - CC_PATH_ONION); - } - } + /* If network doesn't enable it, do not setup. */ + if (!congestion_control_enabled()) { + return; } + + hs_circ_setup_congestion_control(circ, desc->encrypted_data.sendme_inc, + desc->encrypted_data.single_onion_service); } /** Called when a rendezvous circuit has opened. */