From dd63e8cf9dd12677ba1396f3b8f697718538d9bf Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 16 Mar 2022 11:01:56 -0400 Subject: [PATCH 1/2] hs: Transfer ccontrol from circuit to cpath Once the cpath is finalized, e2e encryption setup, transfer the ccontrol from the rendezvous circuit to the cpath. This allows the congestion control subsystem to properly function for both upload and download side of onion services. Closes #40586 Signed-off-by: David Goulet --- changes/ticket40586 | 5 +++++ src/feature/hs/hs_circuit.c | 16 ++++++++++++++++ src/feature/hs/hs_client.c | 5 +++++ 3 files changed, 26 insertions(+) create mode 100644 changes/ticket40586 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..6c4e315e4e 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); @@ -416,6 +422,11 @@ launch_rendezvous_point_circuit,(const hs_service_t *service, .sendme_inc_cells = congestion_control_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() because the final hop + * is not available until then. */ + /* Initialize ccontrol for appropriate path type */ if (service->config.is_single_onion) { TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, @@ -533,6 +544,11 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) .sendme_inc_cells = TO_CIRCUIT(circ)->ccontrol->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() because the final hop + * is not available until then. */ + /* 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) { diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index eb68adfd76..c845a5a945 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -826,6 +826,11 @@ setup_rendezvous_circ_congestion_control(origin_circuit_t *circ) if (circ_params.cc_enabled) { circ_params.sendme_inc_cells = desc->encrypted_data.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() because the final hop + * is not available until then. */ + if (desc->encrypted_data.single_onion_service) { TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, CC_PATH_ONION_SOS); From 32400b5688b0f48e5d52910f8403c2aa17b8484a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 16 Mar 2022 13:11:34 -0400 Subject: [PATCH 2/2] hs: Helper function to setup congestion control We had 3 callsites setting up the circuit congestion control and so this commit consolidates all 3 calls into 1 function. Related to #40586 Signed-off-by: David Goulet --- src/feature/hs/hs_circuit.c | 86 +++++++++++++++++++------------------ src/feature/hs/hs_circuit.h | 4 ++ src/feature/hs/hs_client.c | 30 +++---------- 3 files changed, 54 insertions(+), 66 deletions(-) diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 6c4e315e4e..d253802f79 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -416,30 +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(), - }; - - /* 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() because the final hop - * is not available until then. */ - - /* 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: @@ -538,26 +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, - }; - - /* 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() because the final hop - * is not available until then. */ - + /* 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: @@ -646,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 c845a5a945..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,29 +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; - - /* 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() because the final hop - * is not available until then. */ - - 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. */