Merge remote-tracking branch 'dgoulet/bug24895_032_02' into maint-0.3.2

5 files changed, 35 insertions, 3 deletions

diff --git a/changes/bug24895 b/changes/bug24895
new file mode 100644
index 0000000000..7edde94a0b
--- /dev/null
+++ b/changes/bug24895
@@ -0,0 +1,8 @@
+  o Major bugfixes (onion services):
+    - Fix an "off by 2" error in counting rendezvous failures on the onion
+      service side. While we thought we would stop the rendezvous attempt
+      after one failed circuit, we were actually making three circuit attempts
+      before giving up. Now switch to a default of 2, and allow the consensus
+      parameter "hs_service_max_rdv_failures" to override. Fixes bug 24895;
+      bugfix on 0.0.6.
+
diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c
index 11ce2a1887..66c59e0dc7 100644
--- a/src/or/hs_circuit.c
+++ b/src/or/hs_circuit.c
@@ -484,9 +484,14 @@ can_relaunch_service_rendezvous_point(const origin_circuit_t *circ)
     goto disallow;
   }
 
+  /* We check failure_count >= hs_get_service_max_rend_failures()-1 below, and
+   * the -1 is because we increment the failure count for our current failure
+   * *after* this clause. */
+  int max_rend_failures = hs_get_service_max_rend_failures() - 1;
+
   /* A failure count that has reached maximum allowed or circuit that expired,
    * we skip relaunching. */
-  if (circ->build_state->failure_count > MAX_REND_FAILURES ||
+  if (circ->build_state->failure_count > max_rend_failures ||
       circ->build_state->expiry_time <= time(NULL)) {
     log_info(LD_REND, "Attempt to build a rendezvous circuit to %s has "
                       "failed with %d attempts and expiry time %ld. "
diff --git a/src/or/hs_common.c b/src/or/hs_common.c
index a5cfaf03a8..e9d7323316 100644
--- a/src/or/hs_common.c
+++ b/src/or/hs_common.c
@@ -210,6 +210,23 @@ hs_check_service_private_dir(const char *username, const char *path,
   return 0;
 }
 
+/* Default, minimum and maximum values for the maximum rendezvous failures
+ * consensus parameter. */
+#define MAX_REND_FAILURES_DEFAULT 2
+#define MAX_REND_FAILURES_MIN 1
+#define MAX_REND_FAILURES_MAX 10
+
+/** How many times will a hidden service operator attempt to connect to
+ * a requested rendezvous point before giving up? */
+int
+hs_get_service_max_rend_failures(void)
+{
+  return networkstatus_get_param(NULL, "hs_service_max_rdv_failures",
+                                 MAX_REND_FAILURES_DEFAULT,
+                                 MAX_REND_FAILURES_MIN,
+                                 MAX_REND_FAILURES_MAX);
+}
+
 /** Get the default HS time period length in minutes from the consensus. */
 STATIC uint64_t
 get_time_period_length(void)
diff --git a/src/or/hs_common.h b/src/or/hs_common.h
index b81c03d508..7c5ea4792c 100644
--- a/src/or/hs_common.h
+++ b/src/or/hs_common.h
@@ -166,6 +166,8 @@ void hs_cleanup_circ(circuit_t *circ);
 int hs_check_service_private_dir(const char *username, const char *path,
                                  unsigned int dir_group_readable,
                                  unsigned int create);
+int hs_get_service_max_rend_failures(void);
+
 char *hs_path_from_filename(const char *directory, const char *filename);
 void hs_build_address(const ed25519_public_key_t *key, uint8_t version,
                       char *addr_out);
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 2cdafa6666..2c5c5840a1 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -2042,7 +2042,8 @@ rend_service_receive_introduction(origin_circuit_t *circuit,
 
   /* Launch a circuit to the client's chosen rendezvous point.
    */
-  for (i=0;i<MAX_REND_FAILURES;i++) {
+  int max_rend_failures=hs_get_service_max_rend_failures();
+  for (i=0;i<max_rend_failures;i++) {
     int flags = CIRCLAUNCH_NEED_CAPACITY | CIRCLAUNCH_IS_INTERNAL;
     if (circ_needs_uptime) flags |= CIRCLAUNCH_NEED_UPTIME;
     /* A Single Onion Service only uses a direct connection if its
@@ -2938,7 +2939,6 @@ rend_service_relaunch_rendezvous(origin_circuit_t *oldcirc)
   cpath_build_state_t *newstate, *oldstate;
 
   tor_assert(oldcirc->base_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND);
-
   oldstate = oldcirc->build_state;
   tor_assert(oldstate);