January 2014 - tor-commits - lists.torproject.org

[bridgedb/master] Fix a log message in networkstatus.parseSLine() function.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit 2a1e5dad2e70ba2e09a6b9480880f08aab408c67 Author: Isis Lovecruft <isis(a)torproject.org> Date: Fri Nov 22 01:23:23 2013 +0000 Fix a log message in networkstatus.parseSLine() function. Before, it was only printing whatever was originally found in the networkstatus line, *not* which flags we had successfully parsed. Since we only actually care that we are correctly able to parse relevant flags, that is what the log message should contain. --- lib/bridgedb/parse/networkstatus.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index 1cb54d3..a367b03 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -256,8 +256,14 @@ def parseSLine(line): stable = 'Stable' in flags guard = 'Guard' in flags valid = 'Valid' in flags - - logging.debug("Parsed Flags: %s" % flags) + + if (fast or running or stable or guard or valid): + logging.debug("Parsed Flags: %s%s%s%s%s" + % ('Fast ' if fast else '', + 'Running ' if running else '', + 'Stable ' if stable else '', + 'Guard ' if guard else '', + 'Valid ' if valid else '')) # Right now, we only care about 'Running' and 'Stable' return running, stable

1 0

[bridgedb/master] Refactor networkstatus.parseALine() raise/return more sane values.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit df4b5a48abba5b6987070894230182018af09596 Author: Isis Lovecruft <isis(a)torproject.org> Date: Fri Nov 22 01:15:11 2013 +0000 Refactor networkstatus.parseALine() raise/return more sane values. There were some cases raised within the unittests where, when the IP address had failed to parse, `networkstatus.parseALine()` would return an `addr.PortList` and `None` (for the IP). Having a PortList without an IP address is useless, so instead we should deal with parsing the IP first, and checking its validity. Then, if we have the IP, we should parse and check the ports, then create the PortList. * CHANGE logic of the bridgedb.parse.networkstatus.parseALine() function so that all of the parsing takes place in one `try:` block, and the `except`s catch any and all raised errors, logging messages and reseting variables to None as appropriate. * ADD a FutureWarning if we ever see a networkstatus document with a valid IPc4 address as one of its `ORAddress`es, because this means that tor/torspec and the descriptors have changed format. * CHANGE networkstatus.parseALine() to only return a PortList if an IP address was successfully parsed. --- lib/bridgedb/parse/networkstatus.py | 55 ++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index 294e465..cb08bef 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -174,38 +174,53 @@ def parseALine(line, fingerprint=None): :class:`bridgedb.parse.addr.PortList`. """ ip = None - address = None portlist = None if not line.startswith('a '): - logging.error("Networkstatus parser received non 'a'-line for %r:" - % (fingerprint or 'Unknown')) - logging.error("\t%r" % line) - return address, portlist + logging.error("Networkstatus parser received non 'a'-line for %r:"\ + " %r" % (fingerprint or 'Unknown', line)) + return ip, portlist line = line[2:] # Chop off the 'a ' try: ip, portlist = line.rsplit(':', 1) - except (IndexError, ValueError, addr.InvalidPort) as error: - logging.exception(error) - raise NetworkstatusParsingError( - "Parsing networkstatus 'a'-line for %r failed! Line: %r" - %(fingerprint, line)) - else: - ip = ip.strip('[]') - address = addr.isIPAddress(ip) - if not ip: + except ValueError as error: + logging.error("Bad separator in networkstatus 'a'-line: %r" % line) + return (None, None) + + if ip.startswith('[') and ip.endswith(']'): + ip = ip.strip('[]') + + try: + if not addr.isIPAddress(ip): raise NetworkstatusParsingError( "Got invalid IP Address in networkstatus 'a'-line for %r: %r" - % (fingerprint, ip)) - - portlist = addr.PortList(portlist) + % (fingerprint or 'Unknown', line)) - logging.debug("Parsed networkstatus ORAddress line for %r:" % fingerprint) - logging.debug("\tAddress: %s \tPorts: %s" % (address, portlist)) + if addr.isIPv4(ip): + warnings.warn(FutureWarning( + "Got IPv4 address in networkstatus 'a'-line! "\ + "Networkstatus document format may have changed!")) + except NetworkstatusParsingError as error: + logging.error(error) + ip, portlist = None, None - return address, portlist + try: + portlist = addr.PortList(portlist) + if not portlist: + raise NetworkstatusParsingError( + "Got invalid portlist in 'a'-line for %r!\n Line: %r" + % (fingerprint or 'Unknown', line)) + except (addr.InvalidPort, NetworkstatusParsingError) as error: + logging.exception(error) + portlist = None + else: + logging.debug("Parsed networkstatus ORAddress line for %r:"\ + "\n Address: %s \tPorts: %s" + % (fingerprint or 'Unknown', ip, portlist)) + finally: + return (ip, portlist) def parseSLine(line): """Parse an 's'-line from a bridge networkstatus document.

1 0

[bridgedb/master] Remove line that duplicated setting the flags in parseSLine().
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit 9fc6aaed3140020cce4f42b06b23d7b533c5aa93 Author: Isis Lovecruft <isis(a)torproject.org> Date: Fri Nov 22 01:19:58 2013 +0000 Remove line that duplicated setting the flags in parseSLine(). The lines following this one (i.e. ``stable = 'Stable' in flags``) ensure that if a particular flags isn't present in the flags, then the variable is false; there is no need to set each variable twice. * REMOVE duplicate variable instantiation/setting from bridgedb.parse.networkstatus.parseSLine() function. --- lib/bridgedb/parse/networkstatus.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index a367b03..a20ddb4 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -246,8 +246,6 @@ def parseSLine(line): :returns: A 2-tuple of booleans, the first is True if the bridge has the "Running" flag, and the second is True if it has the "Stable" flag. """ - fast, running, stable, guard, valid = False - line = line[2:] flags = [x.capitalize() for x in line.split()]

1 0

[bridgedb/master] Catch all parser exceptions in parse.networkstatus.parseRLine().
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit aa5d17329201a9723d14b71d050c34228a3f2886 Author: Isis Lovecruft <isis(a)torproject.org> Date: Fri Nov 22 00:38:01 2013 +0000 Catch all parser exceptions in parse.networkstatus.parseRLine(). Rather than raising exceptions during parsing, we should try to log appropriate messages. If the error had a known cause, i.e. one that is due to a badly formatted descriptor, then we should make note of the relevant information from the bad descriptor, and reset any affected variables to None. If the error is possibly do to a newly implemented change in the descriptor format in tor, we should likewise log the event, generate a loud warning about a possible descriptor format change, and continue as before. In networkstatus documents and [bridge-]server-descriptors, it is possible that the `ORAddress` lines may soon change (#9729): 1. to allow more than one ORAddress line, and 2. to allow extra IPv4 addresses in the ORAddress lines. Additionally, remove exception and handlers which are never raised in parseRLine(). 1. The catch for `IndexError`s in networkstatus.parseRLine() is removed: At the beginning of `parseRLine()`, a `NetworkstatusParsingError` is raised if there are less than eight fields in the descriptor, so there is no way that an `IndexError` would ever get raised. Coverage branch reports also show that this `except:` block is never touched. 2. Remove the `InvalidNetworkstatusDescriptorDigest` exception class: …as well as all corresponding code for raising and handling it. The way it was used, there is no possible way that these lines will get executed, because the descriptor digest will either be a str or None. If None, then an error is raised earlier for having too few fields in the 'r'-line. If a str, then `binascii.a2b_base64() will always produce some kind of string, and its exceptions are handled separate. Therefore, there is no way for the descriptor digest to be 0, None, or False. In `bridgedb.parse.networkstatus.parseRLine()` function: * CHANGE to catch and handle all `InvalidNetworkstatusRouterIdentity`, and `NetworkstatusParsingErrors`, exceptions raised during parsing. * REMOVE unnecessary `except IndexError` block. * REMOVE `InvalidNetworkstatusDescriptorDigest` exception class. * CHANGE logging of caught exceptions to use `exc`, not `exc.message`. The later has been deprecated since Python2.6. --- lib/bridgedb/parse/networkstatus.py | 81 ++++++++++++++++------------------- 1 file changed, 38 insertions(+), 43 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index a20ddb4..294e465 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -27,6 +27,9 @@ import binascii import logging import string import time +import warnings + +from twisted.python.log import showwarning from bridgedb.parse import addr from bridgedb.parse import padBase64 @@ -38,9 +41,6 @@ class NetworkstatusParsingError(Exception): class InvalidNetworkstatusRouterIdentity(ValueError): """The ID field of a networkstatus document 'r'-line is invalid.""" -class InvalidNetworkstatusDescriptorDigest(ValueError): - """Descriptor digest of a networkstatus document 'r'-line is invalid.""" - class InvalidRouterNickname(ValueError): """Router nickname doesn't follow tor-spec.""" @@ -94,60 +94,55 @@ def parseRLine(line): (nickname, ID, descDigest, timestamp, ORaddr, ORport, dirport) = (None for x in xrange(7)) - if not line.startswith('r '): - raise NetworkstatusParsingError( - "Networkstatus parser received non 'r'-line: %r" % line) + try: + if not line.startswith('r '): + raise NetworkstatusParsingError( + "Networkstatus parser received non 'r'-line: %r" % line) - line = line[2:] # Chop of the 'r ' + line = line[2:] # Chop of the 'r ' + fields = line.split() - fields = line.split() - if len(fields) != 8: - raise NetworkstatusParsingError( - "Wrong number of fields in networkstatus 'r'-line: %r" % line) + if len(fields) != 8: + raise NetworkstatusParsingError( + "Wrong number of fields in networkstatus 'r'-line: %r" % line) - try: nickname, ID = fields[:2] - isValidRouterNickname(nickname) if ID.endswith('='): raise InvalidNetworkstatusRouterIdentity( - "Skipping networkstatus parsing for router with nickname %r:"\ - "\n\tUnpadded, base64-encoded networkstatus router identity "\ + "Skipping networkstatus parsing for router with nickname "\ + "'%s':\n Unpadded, base64-encoded networkstatus router identity "\ "string ends with '=': %r" % (nickname, ID)) - try: - ID = padBase64(ID) # Add the trailing equals sign back in - except (AttributeError, ValueError) as error: - raise InvalidNetworkstatusRouterIdentity(error.message) - - ID = binascii.a2b_base64(ID) - if not ID: + paddedID = padBase64(ID) # Add the trailing equals sign back in + debasedID = binascii.a2b_base64(paddedID) + if not debasedID: raise InvalidNetworkstatusRouterIdentity( - "Skipping networkstatus parsing for router with nickname %r:"\ - "\n\tBase64-encoding for networkstatus router identity string"\ - "is invalid!\n\tLine: %r" % (nickname, line)) - - except IndexError as error: - logging.error(error.message) + "Skipping networkstatus parsing for router with nickname "\ + "'%s':\n Base64-encoding for networkstatus router identity "\ + "string is invalid!\n Line: %r" % (nickname, line)) + ID = debasedID + + except NetworkstatusParsingError as error: + logging.error(error) + nickname, ID = None, None except InvalidRouterNickname as error: - logging.error(error.message) + logging.error(error) nickname = None except InvalidNetworkstatusRouterIdentity as error: - logging.error(error.message) + logging.error(error) ID = None - - try: - descDigest = binascii.a2b_base64(fields[2]) - except (AttributeError, ValueError) as error: - raise InvalidNetworkstatusDescriptorDigest(error.message) - - - timestamp = time.mktime(time.strptime(" ".join(fields[3:5]), - "%Y-%m-%d %H:%M:%S")) - ORaddr = fields[5] - ORport = fields[6] - dirport = fields[7] - + else: + try: + descDigest = binascii.a2b_base64(fields[2]) + timestamp = time.mktime(time.strptime(" ".join(fields[3:5]), + "%Y-%m-%d %H:%M:%S")) + ORaddr = fields[5] + ORport = fields[6] + dirport = fields[7] + except (AttributeError, ValueError, IndexError) as error: + logging.error(error) + descDigest = None finally: return (nickname, ID, descDigest, timestamp, ORaddr, ORport, dirport)

1 0

[bridgedb/master] Do not catch general Exceptions in isValidRouterNickname.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit b385af36198afc2adc897e94858132fc36a761d6 Author: Isis Lovecruft <isis(a)torproject.org> Date: Sat Dec 7 13:03:44 2013 +0000 Do not catch general Exceptions in isValidRouterNickname. General exceptions here are never raised, and all exceptions which are raised are handled by the calling function, parseRLine(). --- lib/bridgedb/parse/networkstatus.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index cb08bef..8a03af8 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -52,20 +52,14 @@ def isValidRouterNickname(nickname): """ ALPHANUMERIC = string.letters + string.digits - try: - if not (1 <= len(nickname) <= 19): + if not (1 <= len(nickname) <= 19): + raise InvalidRouterNickname( + "Nicknames must be between 1 and 19 characters: %r" % nickname) + for letter in nickname: + if not letter in ALPHANUMERIC: raise InvalidRouterNickname( - "Nicknames must be between 1 and 19 characters: %r" % nickname) - for letter in nickname: - if not letter in ALPHANUMERIC: - raise InvalidRouterNickname( - "Nicknames must only use [A-Za-z0-9]: %r" % nickname) - except Exception as error: - logging.exception(error) - else: - return True - - raise InvalidRouterNickname + "Nicknames must only use [A-Za-z0-9]: %r" % nickname) + return True def parseRLine(line): """Parse an 'r'-line from a networkstatus document.

1 0

[bridgedb/master] Move Bridges.getConfigLine() documentation into function docstring.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit bd526bf5a007d923b11337f4987153a8983ca0f7 Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Dec 10 11:17:57 2013 +0000 Move Bridges.getConfigLine() documentation into function docstring. --- lib/bridgedb/Bridges.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index a99860b..e27c287 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -150,13 +150,16 @@ class Bridge: def getConfigLine(self, includeFingerprint=False, addressClass=None, request=None, transport=None): - """Returns a valid bridge line for inclusion in a torrc""" - #arguments: - # includeFingerprint - # addressClass - type of address to choose - # request - a string unique to this request - # e.g. email-address or uniformMap(ip) - # transport - a pluggable transport method name + """Returns a valid bridge line for inclusion in a torrc. + + :param bool includeFingerprint: If ``True``, include the + ``fingerprint`` of this :class:`Bridge` in the returned bridge + line. + :param DOCDOC addressClass: Type of address to choose. + :param str request: A string unique to this request e.g. email-address + or ``uniformMap(ip)`` or ``'default'``. + :param str transport: A pluggable transport method name. + """ if not request: request = 'default' digest = get_hmac_fn('Order-Or-Addresses')(request)

1 0

[bridgedb/master] Fix an iter-over-null-value bug in Dist.IPBasedDistributor class.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit 328caedefcd701c4280ebae79bd3a5fc8a8c6109 Author: Isis Lovecruft <isis(a)torproject.org> Date: Fri Nov 22 00:32:14 2013 +0000 Fix an iter-over-null-value bug in Dist.IPBasedDistributor class. In the initialisor of Dist.IPBasedDistributor, the `ipCatagories` and `answerParameters` both default to `None` and then they are immediately iterated over. --- lib/bridgedb/Dist.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/bridgedb/Dist.py b/lib/bridgedb/Dist.py index f599e47..cd34dcb 100644 --- a/lib/bridgedb/Dist.py +++ b/lib/bridgedb/Dist.py @@ -89,6 +89,10 @@ class IPBasedDistributor(bridgedb.Bridges.BridgeHolder): self.nClusters = nClusters self.answerParameters = answerParameters + if not ipCategories: + ipCategories = [] + if not answerParameters: + answerParameters = [] self.rings = [] self.categories = []

1 0

[bridgedb/master] Change `PortList` to `addr.PortList` in bridgedb.Bridges module.
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit 879ee6bb0b7700bf9486869fdeaee937f3b5d240 Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Dec 10 11:16:46 2013 +0000 Change `PortList` to `addr.PortList` in bridgedb.Bridges module. --- lib/bridgedb/Bridges.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index 158c980..a99860b 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -19,6 +19,7 @@ import random import bridgedb.Storage import bridgedb.Bucket +from bridgedb.parse import addr from bridgedb.parse import networkstatus @@ -182,7 +183,7 @@ class Bridge: # default ip, orport should get a chance at being selected if isinstance(self.ip, addressClass): - addresses.insert(0,(self.ip, PortList(self.orport))) + addresses.insert(0,(self.ip, addr.PortList(self.orport))) if addresses: address,portlist = addresses[pos % len(addresses)] @@ -543,7 +544,7 @@ def parseCountryBlockFile(f): m = regex.match(addrspec) if m: address = ipaddr.IPAddress(m.group(1)) - portlist = PortList(m.group(2)) + portlist = addr.PortList(m.group(2)) countries = countries.split(',') logging.debug("Parsed address: %s", address) logging.debug("Parsed portlist: %s", portlist)

1 0

[bridgedb/master] Condense two log calls into one in Bridges.parseExtraInfo().
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit d58ab9fe0ab56bc77d27a25b4d2c87ec96010634 Author: Isis Lovecruft <isis(a)torproject.org> Date: Tue Dec 10 11:19:35 2013 +0000 Condense two log calls into one in Bridges.parseExtraInfo(). --- lib/bridgedb/Bridges.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index e27c287..a080331 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -471,8 +471,8 @@ def parseExtraInfoFile(f): m = regex.match(fields[1]) address = ipaddr.IPAddress(m.group(1)) port = int(m.group(2)) - logging.debug(" Parsed Transport: %s", method_name) - logging.debug(" Parsed Transport Address: %s:%d", address, port) + logging.debug(" Parsed Transport: %s %s:%d" + % (method_name, address, port)) yield ID, method_name, address, port, argdict except (IndexError, ValueError, AttributeError): # skip this line

1 0

[bridgedb/master] Fix typo in inline comment in bridgedb.parse.networkstatus.parseRLine().
by isis＠torproject.org 12 Jan '14

12 Jan '14

commit 70dc78323efce1b5a756130f0686d509f963ec8c Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed Dec 11 00:55:26 2013 +0000 Fix typo in inline comment in bridgedb.parse.networkstatus.parseRLine(). --- lib/bridgedb/parse/networkstatus.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bridgedb/parse/networkstatus.py b/lib/bridgedb/parse/networkstatus.py index 8a03af8..293fdd0 100644 --- a/lib/bridgedb/parse/networkstatus.py +++ b/lib/bridgedb/parse/networkstatus.py @@ -93,7 +93,7 @@ def parseRLine(line): raise NetworkstatusParsingError( "Networkstatus parser received non 'r'-line: %r" % line) - line = line[2:] # Chop of the 'r ' + line = line[2:] # Chop off the 'r ' fields = line.split() if len(fields) != 8:

1 0