[tor-commits] [bridgedb/master] Filter returned bridges to avoid multiples from the same /16 or /32.

isis at torproject.org isis at torproject.org
Tue Jan 23 21:44:56 UTC 2018


commit ad269657fe16f2603f207bdb2a979266c3b918e8
Author: Isis Lovecruft <isis at torproject.org>
Date:   Fri Dec 22 02:34:32 2017 +0000

    Filter returned bridges to avoid multiples from the same /16 or /32.
    
     * FIXES #24704 https://bugs.torproject.org/24704
---
 bridgedb/Bridges.py                        | 52 ++++++++++++++++++++----
 bridgedb/distributors/email/distributor.py |  2 +-
 bridgedb/distributors/https/distributor.py |  2 +-
 bridgedb/test/test_Bridges.py              | 64 ++++++++++++++++++++++++++++++
 4 files changed, 111 insertions(+), 9 deletions(-)

diff --git a/bridgedb/Bridges.py b/bridgedb/Bridges.py
index 13bffd2..dfc6ac5 100644
--- a/bridgedb/Bridges.py
+++ b/bridgedb/Bridges.py
@@ -268,7 +268,39 @@ class BridgeRing(object):
         assert len(r) == N
         return r
 
-    def getBridges(self, pos, N=1):
+    def filterDistinctSubnets(self, fingerprints):
+        """Given a chosen set of ``fingerprints`` of bridges to distribute,
+        filter the bridges such that they are in distinct subnets.
+        """
+        logging.debug("Got %d possible bridges to filter" % len(fingerprints))
+
+        bridges = []
+        subnets = []
+
+        for fingerprint in fingerprints:
+            bridge = self.bridges[fingerprint]
+            jump = False
+            for subnet in subnets:
+                if bridge.address in subnet:
+                    jump = True
+                    logging.debug(
+                        ("Skipping distribution of bridge %s in a subnet which "
+                         "contains another bridge we're already distributing")
+                        % bridge)
+                    break
+            if jump:
+                continue
+
+            bridges.append(bridge)
+            if bridge.address.version == 4:
+                cidr = str(bridge.address) + "/16"
+            else:
+                cidr = str(bridge.address) + "/32"
+            subnets.append(ipaddr.IPNetwork(cidr))
+
+        return bridges
+
+    def getBridges(self, pos, N=1, filterBySubnet=False):
         """Return **N** bridges appearing in this hashring after a position.
 
         :param bytes pos: The position to jump to. Any bridges returned will
@@ -285,19 +317,25 @@ class BridgeRing(object):
                 count = len(subring)
             forced.extend(subring._getBridgeKeysAt(pos, count))
 
-        keys = [ ]
-        for k in forced + self._getBridgeKeysAt(pos, N):
+        keys = []
+
+        # Oversample double the number we need, in case we need to
+        # filter them and some are within the same subnet.
+        for k in forced + self._getBridgeKeysAt(pos, N + N):
             if k not in keys:
                 keys.append(k)
             else:
                 logging.debug(
                     "Got duplicate bridge %r in main hashring for position %r."
                     % (logSafely(k.encode('hex')), pos.encode('hex')))
-        keys = keys[:N]
         keys.sort()
 
-        #Do not return bridges from the same /16
-        bridges = [ self.bridges[k] for k in keys ]
+        if filterBySubnet:
+            bridges = self.filterDistinctSubnets(keys)
+        else:
+            bridges = [self.bridges[k] for k in keys]
+
+        bridges = bridges[:N]
 
         return bridges
 
@@ -551,7 +589,7 @@ class FilteredBridgeSplitter(object):
         For all sub-hashrings, the ``bridge`` will only be added iff it passes
         the filter functions for that sub-hashring.
 
-        :type bridge: :class:`~bridgedb.Bridges.Bridge`
+        :type bridge: :class:`~bridgedb.bridges.Bridge`
         :param bridge: The bridge to add.
         """
         # The bridge must be running to insert it:
diff --git a/bridgedb/distributors/email/distributor.py b/bridgedb/distributors/email/distributor.py
index b76c26c..fbf1a50 100644
--- a/bridgedb/distributors/email/distributor.py
+++ b/bridgedb/distributors/email/distributor.py
@@ -191,7 +191,7 @@ class EmailDistributor(Distributor):
                                       populate_from=self.hashring.bridges)
 
             returnNum = self.bridgesPerResponse(ring)
-            result = ring.getBridges(pos, returnNum)
+            result = ring.getBridges(pos, returnNum, filterBySubnet=False)
 
             db.setEmailTime(bridgeRequest.client, now)
             db.commit()
diff --git a/bridgedb/distributors/https/distributor.py b/bridgedb/distributors/https/distributor.py
index 5ff9d83..7bce8d9 100644
--- a/bridgedb/distributors/https/distributor.py
+++ b/bridgedb/distributors/https/distributor.py
@@ -342,6 +342,6 @@ class HTTPSDistributor(Distributor):
 
         # Determine the appropriate number of bridges to give to the client:
         returnNum = self.bridgesPerResponse(ring)
-        answer = ring.getBridges(position, returnNum)
+        answer = ring.getBridges(position, returnNum, filterBySubnet=True)
 
         return answer
diff --git a/bridgedb/test/test_Bridges.py b/bridgedb/test/test_Bridges.py
new file mode 100644
index 0000000..092b033
--- /dev/null
+++ b/bridgedb/test/test_Bridges.py
@@ -0,0 +1,64 @@
+# -*- coding: utf-8 -*-
+#_____________________________________________________________________________
+#
+# This file is part of BridgeDB, a Tor bridge distribution system.
+#
+# :authors: Isis Lovecruft 0xA3ADB67A2CDB8B35 <isis at torproject.org>
+# :copyright: (c) 2017, Isis Lovecruft
+#             (c) 2017, The Tor Project, Inc.
+# :license: see LICENSE for licensing information
+#_____________________________________________________________________________
+
+"""Unittests for :mod:`bridgedb.Bridges`."""
+
+from __future__ import print_function
+
+import copy
+import ipaddr
+import logging
+
+from twisted.trial import unittest
+
+from bridgedb import Bridges
+from bridgedb.test import util
+
+# For additional logger output for debugging, comment out the following:
+#logging.disable(50)
+# and then uncomment the following line:
+Bridges.logging.getLogger().setLevel(10)
+
+
+class BridgeRingTests(unittest.TestCase):
+    """Unittests for :class:`bridgedb.Bridges.BridgeRing`."""
+
+    def setUp(self):
+        self.ring = Bridges.BridgeRing('fake-hmac-key')
+
+    def addBridgesFromSameSubnet(self):
+        bridges = copy.deepcopy(util.generateFakeBridges())
+        subnet = "5.5.%d.%d"
+        i = 1
+        j = 1
+
+        for bridge in bridges:
+            bridge.address = ipaddr.IPAddress(subnet % (i, j))
+            self.ring.insert(bridge)
+
+            if j == 255:
+                j  = 1
+                i += 1
+            else:
+                j += 1
+
+    def test_filterDistinctSubnets(self):
+        """If there are bridges in the same subnet then they should be
+        filtered out of the results.
+        """
+        self.addBridgesFromSameSubnet()
+
+        chosen = self.ring.bridges.keys()[:10]
+        bridges = self.ring.filterDistinctSubnets(chosen)
+
+        # Since they're all in the same /16, we should only get one
+        # bridge back:
+        self.assertEqual(len(bridges), 1)





More information about the tor-commits mailing list