[tor-commits] [stem/master] Addressing issues spotted by integ tests

atagar at torproject.org atagar at torproject.org
Sat Oct 13 18:35:45 UTC 2012


commit c20cfdcd6d87a04d23c508b8b333086c77000d77
Author: Damian Johnson <atagar at torproject.org>
Date:   Tue Oct 9 09:15:41 2012 -0700

    Addressing issues spotted by integ tests
    
    Enough of this unit testing, time to run our new parser against actual network
    status content. Unsurprisingly this ran into a couple issues...
    
    - Microdescriptors have an extra field on their 'directory-signature' lines.
      This is undocumented so it'll also need a spec fix...
    
      https://trac.torproject.org/7072
    
    - Our parser for 'directory-signature' was only reading the first one, rather
      than iterating over all entries.
    
    Most of the rest of the changes are just revising the integ tests that Ravi
    wrote to accomidate changes I've made to the classes.
---
 stem/descriptor/networkstatus.py               |   35 ++++++++++++++++-----
 test/integ/descriptor/networkstatus.py         |   16 +++++----
 test/mocking.py                                |    8 ++++-
 test/unit/descriptor/networkstatus/document.py |   38 +++++++++++++++++++++++-
 4 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index ae0f0c3..ce3a00c 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -593,12 +593,25 @@ class _DocumentFooter(object):
             
             raise ValueError("A network status document's 'bandwidth-weights' entries should be '%s', got '%s'" % (expected_label, actual_label))
       elif keyword == "directory-signature":
-        if not " " in value or not block_contents:
-          if not validate: continue
-          raise ValueError("Authority signatures in a network status document are expected to be of the form 'directory-signature FINGERPRINT KEY_DIGEST\\nSIGNATURE', got:\n%s" % line)
-        
-        fingerprint, key_digest = value.split(" ", 1)
-        self.signatures.append(DocumentSignature(fingerprint, key_digest, block_contents, validate))
+        for sig_value, block_contents in values:
+          if not header.is_microdescriptor:
+            expected_spaces = 1
+            format_label = 'directory-signature FINGERPRINT KEY_DIGEST'
+          else:
+            expected_spaces = 2
+            format_label = 'directory-signature METHOD FINGERPRINT KEY_DIGEST'
+          
+          if sig_value.count(" ") != expected_spaces or not block_contents:
+            if not validate: continue
+            raise ValueError("Authority signatures in a network status document are expected to be of the form '%s\\nSIGNATURE', got:\n%s\n%s" % (format_label, sig_value, block_contents))
+          
+          if not header.is_microdescriptor:
+            method = None
+            fingerprint, key_digest = sig_value.split(" ", 1)
+          else:
+            method, fingerprint, key_digest = sig_value.split(" ", 2)
+          
+          self.signatures.append(DocumentSignature(method, fingerprint, key_digest, block_contents, validate))
 
 def _check_for_missing_and_disallowed_fields(header, entries, fields):
   """
@@ -1033,12 +1046,11 @@ class KeyCertificate(stem.descriptor.Descriptor):
     
     return str(self) > str(other)
 
-# TODO: microdescriptors have a slightly different format (including a
-# 'method') - should probably be a subclass
 class DocumentSignature(object):
   """
   Directory signature of a v3 network status document.
   
+  :var str method: method used to make the signature, this only appears in microdescriptor consensuses
   :var str identity: fingerprint of the authority that made the signature
   :var str key_digest: digest of the signing key
   :var str signature: document signature
@@ -1047,7 +1059,7 @@ class DocumentSignature(object):
   :raises: ValueError if a validity check fails
   """
   
-  def __init__(self, identity, key_digest, signature, validate = True):
+  def __init__(self, method, identity, key_digest, signature, validate = True):
     # Checking that these attributes are valid. Technically the key
     # digest isn't a fingerprint, but it has the same characteristics.
     
@@ -1058,6 +1070,11 @@ class DocumentSignature(object):
       if not stem.util.tor_tools.is_valid_fingerprint(key_digest):
         raise ValueError("Malformed key digest (%s) in the document signature" % (key_digest))
     
+    # TODO: The method field is undocumented so I'm just guessing how we should
+    # handle it. Ticket for clarification...
+    # https://trac.torproject.org/7072
+    
+    self.method = method
     self.identity = identity
     self.key_digest = key_digest
     self.signature = signature
diff --git a/test/integ/descriptor/networkstatus.py b/test/integ/descriptor/networkstatus.py
index aa91ab3..31f2c73 100644
--- a/test/integ/descriptor/networkstatus.py
+++ b/test/integ/descriptor/networkstatus.py
@@ -82,11 +82,12 @@ class TestNetworkStatus(unittest.TestCase):
     descriptor_path = test.integ.descriptor.get_resource("cached-consensus")
     
     descriptor_file = file(descriptor_path)
-    desc = stem.descriptor.networkstatus.NetworkStatusDocument(descriptor_file.read())
+    desc = stem.descriptor.networkstatus.NetworkStatusDocument(descriptor_file.read(), default_params = False)
     router1 = desc.routers[0]
     descriptor_file.close()
     
-    self.assertEquals("3", desc.version)
+    self.assertEquals(3, desc.version)
+    self.assertEquals(None, desc.version_flavor)
     self.assertEquals(True, desc.is_consensus)
     self.assertEquals(False, desc.is_vote)
     self.assertEquals([], desc.consensus_methods)
@@ -123,8 +124,8 @@ class TestNetworkStatus(unittest.TestCase):
     self.assertEquals(8, len(desc.directory_authorities))
     self.assertEquals("tor26", desc.directory_authorities[0].nickname)
     self.assertEquals("14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4", desc.directory_authorities[0].fingerprint)
+    self.assertEquals("86.59.21.38", desc.directory_authorities[0].hostname)
     self.assertEquals("86.59.21.38", desc.directory_authorities[0].address)
-    self.assertEquals("86.59.21.38", desc.directory_authorities[0].ip)
     self.assertEquals(80, desc.directory_authorities[0].dir_port)
     self.assertEquals(443, desc.directory_authorities[0].or_port)
     self.assertEquals("Peter Palfrader", desc.directory_authorities[0].contact)
@@ -175,11 +176,12 @@ I/TJmV928na7RLZe2mGHCAW3VQOvV+QkCfj05VZ8CsY=
     descriptor_path = test.integ.descriptor.get_resource("vote")
     
     descriptor_file = file(descriptor_path)
-    desc = stem.descriptor.networkstatus.NetworkStatusDocument(descriptor_file.read())
+    desc = stem.descriptor.networkstatus.NetworkStatusDocument(descriptor_file.read(), default_params = False)
     router1 = desc.routers[0]
     descriptor_file.close()
     
-    self.assertEquals("3", desc.version)
+    self.assertEquals(3, desc.version)
+    self.assertEquals(None, desc.version_flavor)
     self.assertEquals(False, desc.is_consensus)
     self.assertEquals(True, desc.is_vote)
     self.assertEquals(range(1, 13), desc.consensus_methods)
@@ -207,8 +209,8 @@ I/TJmV928na7RLZe2mGHCAW3VQOvV+QkCfj05VZ8CsY=
     self.assertEquals(1, len(desc.directory_authorities))
     self.assertEquals("turtles", desc.directory_authorities[0].nickname)
     self.assertEquals("27B6B5996C426270A5C95488AA5BCEB6BCC86956", desc.directory_authorities[0].fingerprint)
+    self.assertEquals("76.73.17.194", desc.directory_authorities[0].hostname)
     self.assertEquals("76.73.17.194", desc.directory_authorities[0].address)
-    self.assertEquals("76.73.17.194", desc.directory_authorities[0].ip)
     self.assertEquals(9030, desc.directory_authorities[0].dir_port)
     self.assertEquals(9090, desc.directory_authorities[0].or_port)
     self.assertEquals("Mike Perry <email>", desc.directory_authorities[0].contact)
@@ -245,7 +247,7 @@ KG2OUeQUNoCck4nDpsZwFqPlrWCHcHfTV2iDYFV1HQWDTtZz/qf+GtB8NXsq+I1w
 brADmvReM2BD6p/13h0QURCI5hq7ZYlIKcKrBa0jn1d9cduULl7vgKsRCJDls/ID
 emBZ6pUxMpBmV0v+PrA3v9w4DlE7GHAq61FF/zju2kpqj6MInbEvI/E+e438sWsL
 -----END SIGNATURE-----"""
-    self.assertEquals("3", desc.directory_authorities[0].key_certificate.key_certificate_version)
+    self.assertEquals(3, desc.directory_authorities[0].key_certificate.version)
     self.assertEquals("27B6B5996C426270A5C95488AA5BCEB6BCC86956", desc.directory_authorities[0].key_certificate.fingerprint)
     self.assertEquals(_strptime("2011-11-28 21:51:04"), desc.directory_authorities[0].key_certificate.published)
     self.assertEquals(_strptime("2012-11-28 21:51:04"), desc.directory_authorities[0].key_certificate.expires)
diff --git a/test/mocking.py b/test/mocking.py
index ae74166..ada34aa 100644
--- a/test/mocking.py
+++ b/test/mocking.py
@@ -75,6 +75,7 @@ WPi4Fl2qryzTb3QO5r5x7T8OsG2IBUET1bLQzmtbC560SYR49IvVAgMBAAE=
 """
 
 DOC_SIG = stem.descriptor.networkstatus.DocumentSignature(
+  None,
   "14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4",
   "BF112F1C6D5543CFD0A32215ACABD4197B5279AD",
   "-----BEGIN SIGNATURE-----%s-----END SIGNATURE-----" % CRYPTO_BLOB)
@@ -671,7 +672,7 @@ def get_network_status_document(attr = None, exclude = (), authorities = None, r
   if attr is None:
     attr = {}
   
-  # add defaults only found in a vote or consensus
+  # add defaults only found in a vote, consensus, or microdescriptor
   
   if attr.get("vote-status") == "vote":
     extra_defaults = {
@@ -683,6 +684,11 @@ def get_network_status_document(attr = None, exclude = (), authorities = None, r
       "consensus-method": "9",
     }
   
+  if "microdesc" in attr.get("network-status-version", ""):
+    extra_defaults.update({
+      "directory-signature": "sha256 " + NETWORK_STATUS_DOCUMENT_FOOTER[2][1],
+    })
+  
   for k, v in extra_defaults.items():
     if not (k in attr or (exclude and k in exclude)):
       attr[k] = v
diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py
index 246f7aa..7539b63 100644
--- a/test/unit/descriptor/networkstatus/document.py
+++ b/test/unit/descriptor/networkstatus/document.py
@@ -12,7 +12,7 @@ import stem.version
 from stem.descriptor import Flag
 from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, BANDWIDTH_WEIGHT_ENTRIES, DirectoryAuthority, NetworkStatusDocument, parse_file
 from stem.descriptor.router_status_entry import RouterStatusEntryV3, RouterStatusEntryMicroV3
-from test.mocking import support_with, get_router_status_entry_v3, get_router_status_entry_micro_v3, get_directory_authority, get_network_status_document, CRYPTO_BLOB, DOC_SIG
+from test.mocking import support_with, get_router_status_entry_v3, get_router_status_entry_micro_v3, get_directory_authority, get_network_status_document, CRYPTO_BLOB, DOC_SIG, NETWORK_STATUS_DOCUMENT_FOOTER
 
 class TestNetworkStatusDocument(unittest.TestCase):
   def test_minimal_consensus(self):
@@ -633,6 +633,42 @@ class TestNetworkStatusDocument(unittest.TestCase):
       document = NetworkStatusDocument(content, False)
       self.assertEquals(expected, document.bandwidth_weights)
   
+  def test_microdescriptor_signature(self):
+    """
+    The 'directory-signature' lines for normal and microdescriptor conensuses
+    differ slightly in their format.
+    """
+    
+    # including a microdescriptor flavored 'directory-signature' line should work
+    
+    document = get_network_status_document({"network-status-version": "3 microdesc"})
+    self.assertEqual('sha256', document.signatures[0].method)
+    
+    # include a standard 'directory-signature' line in a microdescriptor
+    # consensus
+    
+    content = get_network_status_document({
+      "network-status-version": "3 microdesc",
+      "directory-signature": NETWORK_STATUS_DOCUMENT_FOOTER[2][1],
+    }, content = True)
+    
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    
+    document = NetworkStatusDocument(content, validate = False)
+    self.assertEqual([], document.signatures)
+    
+    # includes a microdescriptor flavored 'directory-signature' line in a
+    # normal consensus
+    
+    content = get_network_status_document({
+      "directory-signature": "sha256 " + NETWORK_STATUS_DOCUMENT_FOOTER[2][1],
+    }, content = True)
+    
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    
+    document = NetworkStatusDocument(content, validate = False)
+    self.assertEqual([], document.signatures)
+  
   def test_malformed_signature(self):
     """
     Provides malformed or missing content in the 'directory-signature' line.





More information about the tor-commits mailing list