commit 4502701f0ec8462c0848128f000ec18c14a94167 Author: Damian Johnson <atagar@torproject.org> Date: Sun Mar 25 14:52:43 2012 -0700 Unit tests for server descriptor parsing Borrowing heavily from Karsten's metrics-lib unit tests [1], plus a few of our own. This doesn't include everything, some are covered by util tests and others are effectively covered by parsing metrics and cached-descriptor files. That said, I wouldn't be against including more - my fingers are just getting numb... While looking through those tests I discovered these deltas... * When parsing the descriptor he validated that we had the correct first and last entry but we didn't. This was a bug, now fixed. * He checks that the 'protocols' line only contains numberic protocols but we accept any space separated content. The spec doesn't specifify what the protocols can be so I'm leaving this alone. * He had checks that a 'published' line with a year of '3012' or '1912' would fail. I can understand having a sanity check but the spec does not say that those dates are invalid so again leaving it as-is. * Metrics lib has validation for the exit policy but we don't. This is because we haven't yet implemented an ExitPolicy class - gsathya is currently working on that in... https://trac.torproject.org/projects/tor/ticket/5454 * Karsten is parsing and validating read/write-history lines. We, on the other hand, log an INFO level warning when these appear and assert in the integ tests that they (along with 'eventdns') do not exist in our cached-descriptor file. My understanding is that these are deprecated entries and should not appear outside of the extra-info descriptors. Maybe I'm misunderstanding something here... On everything else we either match or are maybe slightly more strict about only allowing content conforming to the spec. [1] https://gitweb.torproject.org/metrics-lib.git/blob/HEAD:/test/org/torproject... --- run_tests.py | 2 + stem/descriptor/server_descriptor.py | 8 +- test/unit/descriptor/server_descriptor.py | 276 +++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+), 1 deletions(-) diff --git a/run_tests.py b/run_tests.py index 3cb259d..2c36a04 100755 --- a/run_tests.py +++ b/run_tests.py @@ -18,6 +18,7 @@ import test.unit.connection.protocolinfo import test.unit.socket.control_line import test.unit.socket.control_message import test.unit.descriptor.reader +import test.unit.descriptor.server_descriptor import test.unit.util.conf import test.unit.util.connection import test.unit.util.enum @@ -88,6 +89,7 @@ UNIT_TESTS = ( test.unit.util.system.TestSystem, test.unit.util.tor_tools.TestTorTools, test.unit.descriptor.reader.TestDescriptorReader, + test.unit.descriptor.server_descriptor.TestServerDescriptor, test.unit.version.TestVersion, test.unit.socket.control_message.TestControlMessage, test.unit.socket.control_line.TestControlLine, diff --git a/stem/descriptor/server_descriptor.py b/stem/descriptor/server_descriptor.py index 899cfcc..a82a657 100644 --- a/stem/descriptor/server_descriptor.py +++ b/stem/descriptor/server_descriptor.py @@ -312,11 +312,13 @@ class ServerDescriptorV3(stem.descriptor.Descriptor): entries = {} remaining_contents = contents.split("\n") + first_entry, last_entry = remaining_contents[0], remaining_contents[0] while remaining_contents: line = remaining_contents.pop(0) # last line can be empty if not line and not remaining_contents: continue + last_entry = line # Some lines have an 'opt ' for backward compatability. They should be # ignored. This prefix is being removed in... @@ -355,7 +357,11 @@ class ServerDescriptorV3(stem.descriptor.Descriptor): if keyword in entries and len(entries[keyword]) > 1: raise ValueError("The '%s' entry can only appear once in a descriptor" % keyword) - if not self.exit_policy: + if not first_entry.startswith(ENTRY_START): + raise ValueError("Descriptor must start with a '%s' entry" % ENTRY_START) + elif not last_entry.startswith(ENTRY_END): + raise ValueError("Descriptor must end with a '%s' entry" % ENTRY_END) + elif not self.exit_policy: raise ValueError("Descriptor must have at least one 'accept' or 'reject' entry") # parse all the entries into our attributes diff --git a/test/unit/descriptor/server_descriptor.py b/test/unit/descriptor/server_descriptor.py new file mode 100644 index 0000000..cde6714 --- /dev/null +++ b/test/unit/descriptor/server_descriptor.py @@ -0,0 +1,276 @@ +""" +Unit tests for stem.descriptor.server_descriptor. +""" + +import datetime +import StringIO +import unittest + +import stem.descriptor.server_descriptor +from stem.descriptor.server_descriptor import ServerDescriptorV3 + +CRYPTO_BLOB = """ +MIGJAoGBAJv5IIWQ+WDWYUdyA/0L8qbIkEVH/cwryZWoIaPAzINfrw1WfNZGtBmg +skFtXhOHHqTRN4GPPrZsAIUOQGzQtGb66IQgT4tO/pj+P6QmSCCdTfhvGfgTCsC+ +WPi4Fl2qryzTb3QO5r5x7T8OsG2IBUET1bLQzmtbC560SYR49IvVAgMBAAE= +""" + +SAMPLE_DESCRIPTOR_ATTR = ( + ("router", "caerSidi 71.35.133.197 9001 0 0"), + ("published", "2012-03-01 17:15:27"), + ("bandwidth", "153600 256000 104590"), + ("reject", "*:*"), + ("onion-key", "\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----" % CRYPTO_BLOB), + ("signing-key", "\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----" % CRYPTO_BLOB), + ("router-signature", "\n-----BEGIN SIGNATURE-----%s-----END SIGNATURE-----" % CRYPTO_BLOB), +) + +def _make_descriptor(attr = None, exclude = None): + """ + Constructs a minimal server descriptor with the given attributes. + + Arguments: + attr (dict) - keyword/value mappings to be included in the descriptor + exclude (list) - mandatory keywords to exclude from the descriptor + + Returns: + str with customized descriptor content + """ + + descriptor_lines = [] + if attr == None: attr = {} + if exclude == None: exclude = [] + + for keyword, value in SAMPLE_DESCRIPTOR_ATTR: + if keyword in exclude: continue + elif keyword in attr: + value = attr[keyword] + del attr[keyword] + + # if this is the last entry then we should dump in any unused attributes + if keyword == "router-signature": + for attr_keyword, attr_value in attr.items(): + descriptor_lines.append("%s %s" % (attr_keyword, attr_value)) + + descriptor_lines.append("%s %s" % (keyword, value)) + + return "\n".join(descriptor_lines) + +class TestServerDescriptor(unittest.TestCase): + def test_minimal_descriptor(self): + """ + Basic sanity check that we can parse a descriptor with minimal attributes. + """ + + desc_text = _make_descriptor() + desc = ServerDescriptorV3(desc_text) + + self.assertEquals("caerSidi", desc.nickname) + self.assertEquals("71.35.133.197", desc.address) + self.assertEquals(None, desc.fingerprint) + self.assertTrue(CRYPTO_BLOB in desc.onion_key) + self.assertTrue(CRYPTO_BLOB in desc.signing_key) + self.assertTrue(CRYPTO_BLOB in desc.signature) + + def test_with_opt(self): + """ + Includes an 'opt <keyword> <value>' entry. + """ + + desc_text = _make_descriptor({"opt": "contact www.atagar.com/contact/"}) + desc = ServerDescriptorV3(desc_text) + self.assertEquals("www.atagar.com/contact/", desc.contact) + + def test_unrecognized_line(self): + """ + Includes unrecognized content in the descriptor. + """ + + desc_text = _make_descriptor({"pepperjack": "is oh so tasty!"}) + desc = ServerDescriptorV3(desc_text) + self.assertEquals(["pepperjack is oh so tasty!"], desc.get_unrecognized_lines()) + + def test_proceeding_line(self): + """ + Includes a line prior to the 'router' entry. + """ + + desc_text = "hibernate 1\n" + _make_descriptor() + self._expect_invalid_attr(desc_text) + + def test_trailing_line(self): + """ + Includes a line after the 'router-signature' entry. + """ + + desc_text = _make_descriptor() + "\nhibernate 1" + self._expect_invalid_attr(desc_text) + + def test_nickname_missing(self): + """ + Constructs with a malformed router entry. + """ + + desc_text = _make_descriptor({"router": " 71.35.133.197 9001 0 0"}) + self._expect_invalid_attr(desc_text, "nickname") + + def test_nickname_too_long(self): + """ + Constructs with a nickname that is an invalid length. + """ + + desc_text = _make_descriptor({"router": "saberrider2008ReallyLongNickname 71.35.133.197 9001 0 0"}) + self._expect_invalid_attr(desc_text, "nickname", "saberrider2008ReallyLongNickname") + + def test_nickname_invalid_char(self): + """ + Constructs with an invalid relay nickname. + """ + + desc_text = _make_descriptor({"router": "$aberrider2008 71.35.133.197 9001 0 0"}) + self._expect_invalid_attr(desc_text, "nickname", "$aberrider2008") + + def test_address_malformed(self): + """ + Constructs with an invalid ip address. + """ + + desc_text = _make_descriptor({"router": "caerSidi 371.35.133.197 9001 0 0"}) + self._expect_invalid_attr(desc_text, "address", "371.35.133.197") + + def test_port_too_high(self): + """ + Constructs with an ORPort that is too large. + """ + + desc_text = _make_descriptor({"router": "caerSidi 71.35.133.197 900001 0 0"}) + self._expect_invalid_attr(desc_text, "or_port", 900001) + + def test_port_malformed(self): + """ + Constructs with an ORPort that isn't numeric. + """ + + desc_text = _make_descriptor({"router": "caerSidi 71.35.133.197 900a1 0 0"}) + self._expect_invalid_attr(desc_text, "or_port") + + def test_port_newline(self): + """ + Constructs with a newline replacing the ORPort. + """ + + desc_text = _make_descriptor({"router": "caerSidi 71.35.133.197 \n 0 0"}) + self._expect_invalid_attr(desc_text, "or_port") + + def test_platform_empty(self): + """ + Constructs with an empty platform entry. + """ + + desc_text = _make_descriptor({"platform": ""}) + desc = ServerDescriptorV3(desc_text, validate = False) + self.assertEquals("", desc.platform) + + # does the same but with 'platform ' replaced with 'platform' + desc_text = desc_text.replace("platform ", "platform") + desc = ServerDescriptorV3(desc_text, validate = False) + self.assertEquals("", desc.platform) + + def test_protocols_no_circuit_versions(self): + """ + Constructs with a protocols line without circuit versions. + """ + + desc_text = _make_descriptor({"opt": "protocols Link 1 2"}) + self._expect_invalid_attr(desc_text, "circuit_protocols") + + def test_published_leap_year(self): + """ + Constructs with a published entry for a leap year, and when the date is + invalid. + """ + + desc_text = _make_descriptor({"published": "2011-02-29 04:03:19"}) + self._expect_invalid_attr(desc_text, "published") + + desc_text = _make_descriptor({"published": "2012-02-29 04:03:19"}) + expected_published = datetime.datetime(2012, 2, 29, 4, 3, 19) + self.assertEquals(expected_published, ServerDescriptorV3(desc_text).published) + + def test_published_no_time(self): + """ + Constructs with a published entry without a time component. + """ + + desc_text = _make_descriptor({"published": "2012-01-01"}) + self._expect_invalid_attr(desc_text, "published") + + def test_annotations(self): + """ + Checks that content before a descriptor are parsed as annotations. + """ + + desc_text = "@pepperjack very tasty\n@mushrooms not so much\n" + desc_text += _make_descriptor() + desc_text += "\ntrailing text that should be ignored, ho hum" + + # running parse_file_v3 should provide an iterator with a single descriptor + desc_iter = stem.descriptor.server_descriptor.parse_file_v3(StringIO.StringIO(desc_text)) + desc_entries = list(desc_iter) + self.assertEquals(1, len(desc_entries)) + desc = desc_entries[0] + + self.assertEquals("caerSidi", desc.nickname) + self.assertEquals("@pepperjack very tasty", desc.get_annotation_lines()[0]) + self.assertEquals("@mushrooms not so much", desc.get_annotation_lines()[1]) + self.assertEquals({"@pepperjack": "very tasty", "@mushrooms": "not so much"}, desc.get_annotations()) + self.assertEquals([], desc.get_unrecognized_lines()) + + def test_duplicate_field(self): + """ + Constructs with a field appearing twice. + """ + + desc_text = _make_descriptor({"<replace>": ""}) + desc_text = desc_text.replace("<replace>", "contact foo\ncontact bar") + self._expect_invalid_attr(desc_text, "contact", "foo") + + def test_missing_required_attr(self): + """ + Test making a descriptor with a missing required attribute. + """ + + for attr in stem.descriptor.server_descriptor.REQUIRED_FIELDS: + desc_text = _make_descriptor(exclude = [attr]) + self.assertRaises(ValueError, ServerDescriptorV3, desc_text) + + # check that we can still construct it without validation + desc = ServerDescriptorV3(desc_text, validate = False) + + # for one of them checks that the corresponding values are None + if attr == "router": + self.assertEquals(None, desc.nickname) + self.assertEquals(None, desc.address) + self.assertEquals(None, desc.or_port) + self.assertEquals(None, desc.socks_port) + self.assertEquals(None, desc.dir_port) + + def _expect_invalid_attr(self, desc_text, attr = None, expected_value = None): + """ + Asserts that construction will fail due to desc_text having a malformed + attribute. If an attr is provided then we check that it matches an expected + value when we're constructed without validation. + """ + + self.assertRaises(ValueError, ServerDescriptorV3, desc_text) + desc = ServerDescriptorV3(desc_text, validate = False) + + if attr: + # check that the invalid attribute matches the expected value when + # constructed without validation + + self.assertEquals(expected_value, getattr(desc, attr)) + else: + # check a default attribute + self.assertEquals("caerSidi", desc.nickname) +