[tor-commits] [bridgedb/develop] Validate extrainfo descriptors by default.

[isis at torproject.org]

commit babbede52344e6ddf51dea2bcc61b688a7aa8c0f
Author: Isis Lovecruft <isis at torproject.org>
Date:   Wed Nov 5 01:45:29 2014 +0000

    Validate extrainfo descriptors by default.
    
    Also update the documentation of `b.p.d.parseBridgeExtraInfoFiles` to
    mention a method for ``router-signature`` verification, and the reasons
    why we use Stem's ``RelayExtraInfoDescriptor`` class, rather than the
    ``BridgeExtraInfoDescriptor`` class.
---
 lib/bridgedb/parse/descriptors.py           |   29 ++++++++++++++++++++++-----
 lib/bridgedb/test/test_parse_descriptors.py |    4 ++--
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/lib/bridgedb/parse/descriptors.py b/lib/bridgedb/parse/descriptors.py
index a7cb202..03b3f55 100644
--- a/lib/bridgedb/parse/descriptors.py
+++ b/lib/bridgedb/parse/descriptors.py
@@ -164,22 +164,41 @@ def deduplicate(descriptors):
 def parseBridgeExtraInfoFiles(*filenames, **kwargs):
     """Parse files which contain ``@type bridge-extrainfo-descriptor``s.
 
+    .. warning:: This function will *not* check that the ``router-signature``
+        at the end of the extrainfo descriptor is valid. See
+        ``bridgedb.bridges.Bridge._verifyExtraInfoSignature`` for a method for
+        checking the signature.
+
     .. note:: This function will call :func:`deduplicate` to deduplicate the
         extrainfo descriptors parsed from all **filenames**.
 
     :kwargs validate: If there is a ``'validate'`` keyword argument, its value
         will be passed along as the ``'validate'`` argument to
         :api:`stem.descriptor.extrainfo_descriptor.BridgeExtraInfoDescriptor`.
+        The ``'validate'`` keyword argument defaults to ``True``, meaning that
+        the hash digest stored in the ``router-digest`` line will be checked
+        against the actual contents of the descriptor and the extrainfo
+        document's signature will be verified.
     :rtype: dict
     :returns: A dictionary mapping bridge fingerprints to deduplicated
-        :api:`stem.descriptor.extrainfo_descriptor.BridgeExtraInfoDescriptor`s.
+        :api:`stem.descriptor.extrainfo_descriptor.RelayExtraInfoDescriptor`s.
     """
     descriptors = []
-    descriptorType = 'bridge-extra-info 1.1'
 
-    validate = False
-    if ('validate' in kwargs) and (kwargs['validate'] is True):
-        validate = True
+    # The ``stem.descriptor.extrainfo_descriptor.BridgeExtraInfoDescriptor``
+    # class (with ``descriptorType = 'bridge-extra-info 1.1``) is unsuitable
+    # for our purposes for the following reasons:
+    #
+    #   1. It expects a ``router-digest`` line, which is only present in
+    #      sanitised bridge extrainfo descriptors.
+    #
+    #   2. It doesn't check the ``router-signature`` (nor does it expect there
+    #      to be a signature).
+    descriptorType = 'extra-info 1.0'
+
+    validate = True
+    if ('validate' in kwargs) and (kwargs['validate'] is False):
+        validate = False
 
     for filename in filenames:
         logging.info("Parsing %s descriptors with Stem: %s"
diff --git a/lib/bridgedb/test/test_parse_descriptors.py b/lib/bridgedb/test/test_parse_descriptors.py
index 953f371..c3772c7 100644
--- a/lib/bridgedb/test/test_parse_descriptors.py
+++ b/lib/bridgedb/test/test_parse_descriptors.py
@@ -21,7 +21,7 @@ HAS_STEM = False
 
 try:
     from stem.descriptor.server_descriptor import RelayDescriptor
-    from stem.descriptor.extrainfo_descriptor import BridgeExtraInfoDescriptor
+    from stem.descriptor.extrainfo_descriptor import RelayExtraInfoDescriptor
     from stem.descriptor.router_status_entry import RouterStatusEntryV3
     from bridgedb.parse import descriptors
 except (ImportError, NameError), error:
@@ -252,7 +252,7 @@ class ParseDescriptorsTests(unittest.TestCase):
         descFile = io.BytesIO(BRIDGE_EXTRA_INFO_DESCRIPTOR)
         routers = descriptors.parseBridgeExtraInfoFiles(descFile)
         bridge = routers.values()[0]
-        self.assertIsInstance(bridge, BridgeExtraInfoDescriptor)
+        self.assertIsInstance(bridge, RelayExtraInfoDescriptor)
 
     def test_parse_descriptors_parseBridgeExtraInfoFiles_one_file(self):
         """Test for ``b.p.descriptors.parseBridgeExtraInfoFiles`` with only one