[tor-commits] [tor/master] Bug 25870: Allow 4th hop of vanguard circuits to be the guard.

[nickm at torproject.org]

commit 289c04b065b7b36734d73fc11a516b3cdc9dd702
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Tue May 1 00:59:21 2018 +0000

    Bug 25870: Allow 4th hop of vanguard circuits to be the guard.
    
    This prevents a malicious RP/IP from learning the guard node in the case that
    we are using only one (because we aren't using two guards, or because one of
    those two guards is temporarily down).
    
    This ensures the "strong" version of Property #6 from
    https://lists.torproject.org/pipermail/tor-dev/2018-April/013098.html
    (Information about the guard(s) does not leak to the website/RP at all).
---
 src/or/circuitbuild.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 4bdc8a3ee..68ff964f6 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2447,8 +2447,10 @@ cpath_get_n_hops(crypt_path_t **head_ptr)
 /**
  * Build the exclude list for vanguard circuits.
  *
- * For vanguard circuits we exclude all the already chosen nodes (including
- * the exit) from being middle hops.
+ * For vanguard circuits we exclude all the already chosen nodes (including the
+ * exit) from being middle hops to prevent the creation of A - B - A subpaths.
+ * We also allow the 4th hop to be the same as the guard node so as to not leak
+ * guard information to RP/IP/HSDirs.
  *
  * For vanguard circuits, we don't apply any subnet or family restrictions.
  * This is to avoid impossible-to-build circuit paths, or just situations where
@@ -2483,6 +2485,19 @@ build_vanguard_middle_exclude_list(uint8_t purpose,
     smartlist_add(excluded, (node_t*)r);
   }
 
+  /* If we are picking the 4th hop, allow that node to be the guard too.
+   * This prevents us from avoiding the Guard for those hops, which
+   * gives the adversary information about our guard if they control
+   * the RP, IP, or HSDIR. We don't do this check based on purpose
+   * because we also want to allow HS_VANGUARDS pre-build circuits
+   * to use the guard for that last hop.
+   */
+  if (cur_len == DEFAULT_ROUTE_LEN+1) {
+    /* Skip the first hop for the exclude list below */
+    head = head->next;
+    cur_len--;
+  }
+
   for (i = 0, cpath = head; cpath && i < cur_len; ++i, cpath=cpath->next) {
     if ((r = node_get_by_id(cpath->extend_info->identity_digest))) {
       smartlist_add(excluded, (node_t*)r);