Merge branch 'tor-github/pr/1256' into maint-0.4.1

changes/bug23507

@@ -0,0 +1,5 @@
+  o Minor bugfixes (v3 single onion services):
+    - Make v3 single onion services fall back to a 3-hop intro, when there
+      all intro points are unreachable via a 1-hop path. Previously, v3
+      single onion services failed when all intro nodes were unreachable
+      via a 1-hop path. Fixes bug 23507; bugfix on 0.3.2.1-alpha.

changes/bug23818_v3

@@ -0,0 +1,6 @@
+  o Minor bugfixes (v3 single onion services):
+    - Always retry v3 single onion service intro and rend circuits with a
+      3-hop path. Previously, v3 single onion services used a 3-hop path
+      when rend circuits were retried after a remote or delayed failure,
+      but a 1-hop path for immediate retries. Fixes bug 23818;
+      bugfix on 0.3.2.1-alpha.

src/feature/hs/hs_circuit.c

@@ -406,8 +406,12 @@ launch_rendezvous_point_circuit(const hs_service_t *service,
     if (circ_needs_uptime) {
       circ_flags |= CIRCLAUNCH_NEED_UPTIME;
     }
-    /* Firewall and policies are checked when getting the extend info. */
-    if (service->config.is_single_onion) {
+    /* Firewall and policies are checked when getting the extend info.
+     *
+     * We only use a one-hop path on the first attempt. If the first attempt
+     * fails, we use a 3-hop path for reachability / reliability.
+     * See the comment in retry_service_rendezvous_point() for details. */
+    if (service->config.is_single_onion && i == 0) {
       circ_flags |= CIRCLAUNCH_ONEHOP_TUNNEL;
     }
 
@@ -679,13 +683,16 @@ hs_circ_retry_service_rendezvous_point(origin_circuit_t *circ)
 }
 
 /* For a given service and a service intro point, launch a circuit to the
- * extend info ei. If the service is a single onion, a one-hop circuit will be
- * requested. Return 0 if the circuit was successfully launched and tagged
+ * extend info ei. If the service is a single onion, and direct_conn is true,
+ * a one-hop circuit will be requested.
+ *
+ * Return 0 if the circuit was successfully launched and tagged
  * with the correct identifier. On error, a negative value is returned. */
 int
 hs_circ_launch_intro_point(hs_service_t *service,
                            const hs_service_intro_point_t *ip,
-                           extend_info_t *ei)
+                           extend_info_t *ei,
+                           bool direct_conn)
 {
   /* Standard flags for introduction circuit. */
   int ret = -1, circ_flags = CIRCLAUNCH_NEED_UPTIME | CIRCLAUNCH_IS_INTERNAL;
@@ -697,7 +704,16 @@ hs_circ_launch_intro_point(hs_service_t *service,
 
   /* Update circuit flags in case of a single onion service that requires a
    * direct connection. */
-  if (service->config.is_single_onion) {
+  tor_assert_nonfatal(ip->circuit_retries > 0);
+  /* Only single onion services can make direct conns */
+  if (BUG(!service->config.is_single_onion && direct_conn)) {
+    goto end;
+  }
+  /* We only use a one-hop path on the first attempt. If the first attempt
+   * fails, we use a 3-hop path for reachability / reliability.
+   * (Unlike v2, retries is incremented by the caller before it calls this
+   * function.) */
+  if (direct_conn && ip->circuit_retries == 1) {
     circ_flags |= CIRCLAUNCH_ONEHOP_TUNNEL;
   }
 

src/feature/hs/hs_circuit.h

@@ -26,7 +26,8 @@ void hs_circ_service_rp_has_opened(const hs_service_t *service,
                                    origin_circuit_t *circ);
 int hs_circ_launch_intro_point(hs_service_t *service,
                                const hs_service_intro_point_t *ip,
-                               extend_info_t *ei);
+                               extend_info_t *ei,
+                               bool direct_conn);
 int hs_circ_launch_rendezvous_point(const hs_service_t *service,
                                     const curve25519_public_key_t *onion_key,
                                     const uint8_t *rendezvous_cookie);

src/feature/hs/hs_service.c

@@ -2071,6 +2071,7 @@ build_all_descriptors(time_t now)
 static hs_service_intro_point_t *
 pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes)
 {
+  const or_options_t *options = get_options();
   const node_t *node;
   hs_service_intro_point_t *ip = NULL;
   /* Normal 3-hop introduction point flags. */
@@ -2078,11 +2079,19 @@ pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes)
   /* Single onion flags. */
   router_crn_flags_t direct_flags = flags | CRN_PREF_ADDR | CRN_DIRECT_CONN;
 
-  node = router_choose_random_node(exclude_nodes, get_options()->ExcludeNodes,
+  node = router_choose_random_node(exclude_nodes, options->ExcludeNodes,
                                    direct_conn ? direct_flags : flags);
-  /* Unable to find a node. When looking for a node for a direct connection,
-   * we could try a 3-hop path instead. We'll add support for this in a later
-   * release. */
+
+  /* If we are in single onion mode, retry node selection for a 3-hop
+   * path */
+  if (direct_conn && !node) {
+    log_info(LD_REND,
+             "Unable to find an intro point that we can connect to "
+             "directly, falling back to a 3-hop path.");
+    node = router_choose_random_node(exclude_nodes, options->ExcludeNodes,
+                                     flags);
+  }
+
   if (!node) {
     goto err;
   }
@@ -2583,7 +2592,7 @@ launch_intro_point_circuits(hs_service_t *service)
    * circuits using the current map. */
   FOR_EACH_DESCRIPTOR_BEGIN(service, desc) {
     /* Keep a ref on if we need a direct connection. We use this often. */
-    unsigned int direct_conn = service->config.is_single_onion;
+    bool direct_conn = service->config.is_single_onion;
 
     DIGEST256MAP_FOREACH_MODIFY(desc->intro_points.map, key,
                                 hs_service_intro_point_t *, ip) {
@@ -2594,8 +2603,15 @@ launch_intro_point_circuits(hs_service_t *service)
       if (hs_circ_service_get_intro_circ(ip)) {
         continue;
       }
-
       ei = get_extend_info_from_intro_point(ip, direct_conn);
+
+      /* If we can't connect directly to the intro point, get an extend_info
+       * for a multi-hop path instead. */
+      if (ei == NULL && direct_conn) {
+        direct_conn = false;
+        ei = get_extend_info_from_intro_point(ip, 0);
+      }
+
       if (ei == NULL) {
         /* This is possible if we can get a node_t but not the extend info out
          * of it. In this case, we remove the intro point and a new one will
@@ -2607,7 +2623,7 @@ launch_intro_point_circuits(hs_service_t *service)
 
       /* Launch a circuit to the intro point. */
       ip->circuit_retries++;
-      if (hs_circ_launch_intro_point(service, ip, ei) < 0) {
+      if (hs_circ_launch_intro_point(service, ip, ei, direct_conn) < 0) {
         log_info(LD_REND, "Unable to launch intro circuit to node %s "
                           "for service %s.",
                  safe_str_client(extend_info_describe(ei)),