Consider the exit family when applying guard restrictions.

When the new path selection logic went into place, I accidentally
dropped the code that considered the _family_ of the exit node when
deciding if the guard was usable, and we didn't catch that during
code review.

This patch makes the guard_restriction_t code consider the exit
family as well, and adds some (hopefully redundant) checks for the
case where we lack a node_t for a guard but we have a bridge_info_t
for it.

Fixes bug 22753; bugfix on 0.3.0.1-alpha. Tracked as TROVE-2016-006
and CVE-2017-0377.

1 files changed, 38 insertions, 1 deletions

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index aba35e69f7..ccb080880c 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -1428,6 +1428,38 @@ entry_guard_passes_filter(const or_options_t *options, guard_selection_t *gs,
   }
 }
 
+/** Return true iff <b>guard</b> is in the same family as <b>node</b>.
+ */
+static int
+guard_in_node_family(const entry_guard_t *guard, const node_t *node)
+{
+  const node_t *guard_node = node_get_by_id(guard->identity);
+  if (guard_node) {
+    return nodes_in_same_family(guard_node, node);
+  } else {
+    /* If we don't have a node_t for the guard node, we might have
+     * a bridge_info_t for it. So let's check to see whether the bridge
+     * address matches has any family issues.
+     *
+     * (Strictly speaking, I believe this check is unnecessary, since we only
+     * use it to avoid the exit's family when building circuits, and we don't
+     * build multihop circuits until we have a routerinfo_t for the
+     * bridge... at which point, we'll also have a node_t for the
+     * bridge. Nonetheless, it seems wise to include it, in case our
+     * assumptions change down the road.  -nickm.)
+     */
+    if (get_options()->EnforceDistinctSubnets && guard->bridge_addr) {
+      tor_addr_t node_addr;
+      node_get_addr(node, &node_addr);
+      if (addrs_in_same_network_family(&node_addr,
+                                       &guard->bridge_addr->addr)) {
+        return 1;
+      }
+    }
+    return 0;
+  }
+}
+
 /**
  * Return true iff <b>guard</b> obeys the restrictions defined in <b>rst</b>.
  * (If <b>rst</b> is NULL, there are no restrictions.)
@@ -1440,7 +1472,12 @@ entry_guard_obeys_restriction(const entry_guard_t *guard,
   if (! rst)
     return 1; // No restriction?  No problem.
 
-  // Only one kind of restriction exists right now
+  // Only one kind of restriction exists right now: excluding an exit
+  // ID and all of its family.
+  const node_t *node = node_get_by_id((const char*)rst->exclude_id);
+  if (node && guard_in_node_family(guard, node))
+    return 0;
+
   return tor_memneq(guard->identity, rst->exclude_id, DIGEST_LEN);
 }