Merge branch 'tor-gitlab/mr/548'

4 files changed, 65 insertions, 51 deletions

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. */