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

commit c20cfdcd6d87a04d23c508b8b333086c77000d77 Author: Damian Johnson atagar@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.