Merge remote-tracking branch 'dgoulet/bug24895_029_02' into maint-0.2.9

2 files changed, 32 insertions, 5 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/rendservice.c b/src/or/rendservice.c
index a8c383444d..da200d1381 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -108,12 +108,25 @@ struct rend_service_port_config_s {
 /** Don't try to build more than this many circuits before giving up
  * for a while.*/
 #define MAX_INTRO_CIRCS_PER_PERIOD 10
-/** How many times will a hidden service operator attempt to connect to
- * a requested rendezvous point before giving up? */
-#define MAX_REND_FAILURES 1
 /** How many seconds should we spend trying to connect to a requested
  * rendezvous point before giving up? */
 #define MAX_REND_TIMEOUT 30
+/* 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? */
+static int
+get_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);
+}
 
 /* Hidden service directory file names:
  * new file names should be added to rend_service_add_filenames_to_list()
@@ -2028,7 +2041,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=get_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
@@ -2930,8 +2944,13 @@ rend_service_relaunch_rendezvous(origin_circuit_t *oldcirc)
   }
   oldcirc->hs_service_side_rend_circ_has_been_relaunched = 1;
 
+  /* We check failure_count >= get_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 = get_max_rend_failures() - 1;
+
   if (!oldcirc->build_state ||
-      oldcirc->build_state->failure_count > MAX_REND_FAILURES ||
+      oldcirc->build_state->failure_count >= max_rend_failures ||
       oldcirc->build_state->expiry_time < time(NULL)) {
     log_info(LD_REND,
              "Attempt to build circuit to %s for rendezvous has failed "