hs: Don't BUG() when setting up RP congestion control

It is possible to not have the descriptor anymore by the time the
rendezvous circuit opens. Don't BUG() on that.

Instead, when sending the INTRODUCE1 cell, make sure the descriptor we
have (or have just fetched) matches what we setup in the rendezvous
circuit.

If not, the circuit is closed and another one is opened for a retry.

Fixes #40576

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/changes/ticket40576 b/changes/ticket40576
new file mode 100644 (file)
index 0000000..c9e3745
--- /dev/null
+++ b/changes/ticket40576
@@ -0,0 +1,5 @@
+  o Minor bugfixes (onion service congestion control):
+    - Avoid a non fatal assert in case we can't setup congestion control on the
+      rendezvous circuit after opening. Fixes bug 40576; bugfix on
+      0.4.7.4-alpha.
+

diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index 81c0459a8615c7f1ae59f54481da09ce5945ef09..f5891e6bdc163e4866553f688ec442a7eba6182b 100644 (file)
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -624,6 +624,16 @@ send_introduce1(origin_circuit_t *intro_circ,
     goto tran_err;
   }
 
+  /* Check if the rendevous circuit was setup WITHOUT congestion control but if
+   * it is enabled and the service supports it. This can happen, see
+   * setup_rendezvous_circ_congestion_control() and so close rendezvous circuit
+   * so another one can be created. */
+  if (TO_CIRCUIT(rend_circ)->ccontrol == NULL && congestion_control_enabled()
+      && hs_desc_supports_congestion_control(desc)) {
+    circuit_mark_for_close(TO_CIRCUIT(rend_circ), END_CIRC_REASON_INTERNAL);
+    goto tran_err;
+  }
+
   /* We need to find which intro point in the descriptor we are connected to
    * on intro_circ. */
   ip = find_desc_intro_point_by_ident(intro_circ->hs_ident, desc);
@@ -760,7 +770,14 @@ client_intro_circ_has_opened(origin_circuit_t *circ)
 }
 
 /** Setup the congestion control parameters on the given rendezvous circuit.
- * This looks at the service descriptor flow control line (if any). */
+ * This looks at the service descriptor flow control line (if any).
+ *
+ * It is possible that we are unable to set congestion control on the circuit
+ * if the descriptor can't be found. In that case, the introduction circuit
+ * can't be opened without it so a fetch will be triggered.
+ *
+ * However, if the descriptor asks for congestion control but the RP circuit
+ * doesn't have it, it will be closed and a new circuit will be opened. */
 static void
 setup_rendezvous_circ_congestion_control(origin_circuit_t *circ)
 {
@@ -771,16 +788,16 @@ setup_rendezvous_circ_congestion_control(origin_circuit_t *circ)
   /* Setup congestion control parameters on the circuit. */
   const hs_descriptor_t *desc =
     hs_cache_lookup_as_client(&circ->hs_ident->identity_pk);
-  if (BUG(desc == NULL)) {
-    /* This should really never happened but in case, scream and stop. */
+  if (desc == NULL) {
+    /* This is possible because between launching the circuit and the circuit
+     * ending in opened state, the descriptor could have been removed from the
+     * cache. In this case, we just can't setup congestion control. */
     return;
   }
 
   /* Check if the service lists support for congestion control in its
    * descriptor. If not, we don't setup congestion control. */
-  if (!desc->encrypted_data.flow_control_pv ||
-      !protocol_list_supports_protocol(desc->encrypted_data.flow_control_pv,
-                                       PRT_FLOWCTRL, PROTOVER_FLOWCTRL_CC)) {
+  if (!hs_desc_supports_congestion_control(desc)) {
     return;
   }
 

diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c
index 523ededf8cdf6264f949ad4f9d4679ec18ea9096..15ad9d8efb15a7d91039058d830f605c4cac10e9 100644 (file)
--- a/src/feature/hs/hs_descriptor.c
+++ b/src/feature/hs/hs_descriptor.c
@@ -2987,3 +2987,16 @@ hs_descriptor_clear_intro_points(hs_descriptor_t *desc)
     smartlist_clear(ips);
   }
 }
+
+/** Return true iff we support the given descriptor congestion control
+ * parameters. */
+bool
+hs_desc_supports_congestion_control(const hs_descriptor_t *desc)
+{
+  tor_assert(desc);
+
+  /* Validate that we support the protocol version in the descriptor. */
+  return desc->encrypted_data.flow_control_pv &&
+         protocol_list_supports_protocol(desc->encrypted_data.flow_control_pv,
+                                         PRT_FLOWCTRL, PROTOVER_FLOWCTRL_CC);
+}

diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h
index 8f5ee6a2f147505bfe3cd6dcafe9082339c6cef2..8f42b2138b11c47b497d3ffbb947ea9520b1ee4f 100644 (file)
--- a/src/feature/hs/hs_descriptor.h
+++ b/src/feature/hs/hs_descriptor.h
@@ -319,6 +319,8 @@ void hs_desc_superencrypted_data_free_contents(
                                         hs_desc_superencrypted_data_t *desc);
 void hs_desc_encrypted_data_free_contents(hs_desc_encrypted_data_t *desc);
 
+bool hs_desc_supports_congestion_control(const hs_descriptor_t *desc);
+
 #ifdef HS_DESCRIPTOR_PRIVATE
 
 /* Encoding. */