commit 84e4e657b4785e4888e567fbc04c8ea29fd43cc4 Author: Damian Johnson atagar@torproject.org Date: Mon Jun 18 13:02:47 2018 -0700

Cell attribute for unused padding

I love our prior commit's effort to be more explicit when Cell data is explicitly ignored. Rather than a commment making this an 'unused' attribute of the base Cell class. This way our callers will have all the information necessary to reconstruct cells if they so desire. --- stem/client/cell.py | 67 ++++++++++++++++++++++++++++++++++-------------- test/unit/client/cell.py | 6 +++++ 2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/stem/client/cell.py b/stem/client/cell.py index 1b30db6c..788c4c41 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -75,12 +75,18 @@ STREAM_ID_DISALLOWED = ( class Cell(object): """ Metadata for ORPort cells. + + :var bytes unused: unused filler that padded the cell to the expected size """

NAME = 'UNKNOWN' VALUE = -1 IS_FIXED_SIZE = False

+ def __init__(self, unused = b''): + super(Cell, self).__init__() + self.unused = unused + @staticmethod def by_name(name): """ @@ -240,7 +246,8 @@ class CircuitCell(Cell): :var int circ_id: circuit id """

- def __init__(self, circ_id): + def __init__(self, circ_id, unused = b''): + super(CircuitCell, self).__init__(unused) self.circ_id = circ_id

@@ -261,6 +268,7 @@ class PaddingCell(Cell): elif len(payload) != FIXED_PAYLOAD_LEN: raise ValueError('Padding payload should be %i bytes, but was %i' % (FIXED_PAYLOAD_LEN, len(payload)))

+ super(PaddingCell, self).__init__() self.payload = payload

def pack(self, link_protocol): @@ -279,12 +287,18 @@ class CreateCell(CircuitCell): VALUE = 1 IS_FIXED_SIZE = True

+ def __init__(self): + super(CreateCell, self).__init__() # TODO: implement +

class CreatedCell(CircuitCell): NAME = 'CREATED' VALUE = 2 IS_FIXED_SIZE = True

+ def __init__(self): + super(CreatedCell, self).__init__() # TODO: implement +

class RelayCell(CircuitCell): """ @@ -302,7 +316,7 @@ class RelayCell(CircuitCell): VALUE = 3 IS_FIXED_SIZE = True

- def __init__(self, circ_id, command, data, digest = 0, stream_id = 0, recognized = 0): + def __init__(self, circ_id, command, data, digest = 0, stream_id = 0, recognized = 0, unused = b''): if 'HASH' in str(type(digest)): # Unfortunately hashlib generates from a dynamic private class so # isinstance() isn't such a great option. With python2/python3 the @@ -316,7 +330,7 @@ class RelayCell(CircuitCell): else: raise ValueError('RELAY cell digest must be a hash, string, or int but was a %s' % type(digest).__name__)

- super(RelayCell, self).__init__(circ_id) + super(RelayCell, self).__init__(circ_id, unused) self.command, self.command_int = RelayCommand.get(command) self.data = str_tools._to_bytes(data) self.recognized = recognized @@ -347,11 +361,9 @@ class RelayCell(CircuitCell): stream_id, content = Size.SHORT.pop(content) digest, content = Size.LONG.pop(content) data_len, content = Size.SHORT.pop(content) - data, _ = split(content, data_len) + data, unused = split(content, data_len)

- # remaining content (if any) is thrown out (ignored) - - return RelayCell(circ_id, command, data, digest, stream_id, recognized) + return RelayCell(circ_id, command, data, digest, stream_id, recognized, unused)

def __hash__(self): return _hash_attr(self, 'command_int', 'stream_id', 'digest', 'data') @@ -481,6 +493,7 @@ class VersionsCell(Cell): IS_FIXED_SIZE = False

def __init__(self, versions): + super(VersionsCell, self).__init__() self.versions = versions

def pack(self, link_protocol = None): @@ -517,7 +530,8 @@ class NetinfoCell(Cell): VALUE = 8 IS_FIXED_SIZE = True

- def __init__(self, receiver_address, sender_addresses, timestamp = None): + def __init__(self, receiver_address, sender_addresses, timestamp = None, unused = b''): + super(NetinfoCell, self).__init__(unused) self.timestamp = timestamp if timestamp else datetime.datetime.now() self.receiver_address = receiver_address self.sender_addresses = sender_addresses @@ -548,9 +562,7 @@ class NetinfoCell(Cell): addr, content = Address.pop(content) sender_addresses.append(addr)

- # remaining content (if any) is thrown out (ignored) - - return NetinfoCell(receiver_address, sender_addresses, datetime.datetime.utcfromtimestamp(timestamp)) + return NetinfoCell(receiver_address, sender_addresses, datetime.datetime.utcfromtimestamp(timestamp), unused = b'')

def __hash__(self): return _hash_attr(self, 'timestamp', 'receiver_address', 'sender_addresses') @@ -561,24 +573,36 @@ class RelayEarlyCell(CircuitCell): VALUE = 9 IS_FIXED_SIZE = True

+ def __init__(self): + super(RelayEarlyCell, self).__init__() # TODO: implement +

class Create2Cell(CircuitCell): NAME = 'CREATE2' VALUE = 10 IS_FIXED_SIZE = True

+ def __init__(self): + super(Create2Cell, self).__init__() # TODO: implement +

class Created2Cell(Cell): NAME = 'CREATED2' VALUE = 11 IS_FIXED_SIZE = True

+ def __init__(self): + super(Created2Cell, self).__init__() # TODO: implement +

class PaddingNegotiateCell(Cell): NAME = 'PADDING_NEGOTIATE' VALUE = 12 IS_FIXED_SIZE = True

+ def __init__(self): + super(PaddingNegotiateCell, self).__init__() # TODO: implement +

class VPaddingCell(Cell): """ @@ -599,6 +623,7 @@ class VPaddingCell(Cell): elif size is not None and payload is not None and size != len(payload): raise ValueError('VPaddingCell constructor specified both a size of %i bytes and payload of %i bytes' % (size, len(payload)))

+ super(VPaddingCell, self).__init__() self.payload = payload if payload is not None else os.urandom(size)

def pack(self, link_protocol): @@ -623,7 +648,8 @@ class CertsCell(Cell): VALUE = 129 IS_FIXED_SIZE = False

- def __init__(self, certs): + def __init__(self, certs, unused = b''): + super(CertsCell, self).__init__(unused) self.certificates = certs

def pack(self, link_protocol): @@ -641,9 +667,7 @@ class CertsCell(Cell): cert, content = Certificate.pop(content) certs.append(cert)

- # remaining content (if any) is thrown out (ignored) - - return CertsCell(certs) + return CertsCell(certs, unused = content)

def __hash__(self): return _hash_attr(self, 'certificates') @@ -662,12 +686,13 @@ class AuthChallengeCell(Cell): VALUE = 130 IS_FIXED_SIZE = False

- def __init__(self, methods, challenge = None): + def __init__(self, methods, challenge = None, unused = b''): if not challenge: challenge = os.urandom(AUTH_CHALLENGE_SIZE) elif len(challenge) != AUTH_CHALLENGE_SIZE: raise ValueError('AUTH_CHALLENGE must be %i bytes, but was %i' % (AUTH_CHALLENGE_SIZE, len(challenge)))

+ super(AuthChallengeCell, self).__init__(unused) self.challenge = challenge self.methods = methods

@@ -698,9 +723,7 @@ class AuthChallengeCell(Cell): method, content = Size.SHORT.pop(content) methods.append(method)

- # remaining content (if any) is thrown out (ignored) - - return AuthChallengeCell(methods, challenge) + return AuthChallengeCell(methods, challenge, unused = content)

def __hash__(self): return _hash_attr(self, 'challenge', 'methods') @@ -711,8 +734,14 @@ class AuthenticateCell(Cell): VALUE = 131 IS_FIXED_SIZE = False

+ def __init__(self): + super(AuthenticateCell, self).__init__() # TODO: implement +

class AuthorizeCell(Cell): NAME = 'AUTHORIZE' VALUE = 132 IS_FIXED_SIZE = False + + def __init__(self): + super(AuthorizeCell, self).__init__() # TODO: implement diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 3d054757..b6734fe7 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -173,6 +173,7 @@ class TestCell(unittest.TestCase): self.assertEqual(data, cell.data) self.assertEqual(digest, cell.digest) self.assertEqual(stream_id, cell.stream_id) + self.assertEqual(b'\x00' * (498 - len(cell.data)), cell.unused)

digest = hashlib.sha1(b'hi') self.assertEqual(3257622417, RelayCell(5, 'RELAY_BEGIN_DIR', '', digest, 564346860).digest) @@ -190,6 +191,7 @@ class TestCell(unittest.TestCase): self.assertEqual(circ_id, cell.circ_id) self.assertEqual(reason, cell.reason) self.assertEqual(reason_int, cell.reason_int) + self.assertEqual(b'', cell.unused)

self.assertRaisesRegexp(ValueError, 'Circuit closure reason should be a single byte, but was 2', Cell.pop, b'\x80\x00\x00\x00\x04\x01\x01' + ZERO * 507, 5)

@@ -200,6 +202,7 @@ class TestCell(unittest.TestCase): cell = Cell.pop(cell_bytes, 5)[0] self.assertEqual(circ_id, cell.circ_id) self.assertEqual(key_material, cell.key_material) + self.assertEqual(b'', cell.unused)

self.assertRaisesRegexp(ValueError, 'Key material should be 20 bytes, but was 3', CreateFastCell, 5, 'boo')

@@ -211,6 +214,7 @@ class TestCell(unittest.TestCase): self.assertEqual(circ_id, cell.circ_id) self.assertEqual(key_material, cell.key_material) self.assertEqual(derivative_key, cell.derivative_key) + self.assertEqual(b'', cell.unused)

self.assertRaisesRegexp(ValueError, 'Key material should be 20 bytes, but was 3', CreateFastCell, 5, 'boo')

@@ -227,6 +231,7 @@ class TestCell(unittest.TestCase): self.assertEqual(timestamp, cell.timestamp) self.assertEqual(receiver_address, cell.receiver_address) self.assertEqual(sender_addresses, cell.sender_addresses) + self.assertEqual(b'', cell.unused)

def test_vpadding_cell(self): for cell_bytes, payload in VPADDING_CELLS.items(): @@ -262,6 +267,7 @@ class TestCell(unittest.TestCase): cell = Cell.pop(cell_bytes, 2)[0] self.assertEqual(challenge, cell.challenge) self.assertEqual(methods, cell.methods) + self.assertEqual(b'', cell.unused)

self.assertRaisesRegexp(ValueError, 'AUTH_CHALLENGE cell should have a payload of 38 bytes, but only had 16', Cell.pop, b'\x00\x00\x82\x00&' + CHALLENGE[:10] + b'\x00\x02\x00\x01\x00\x03', 2) self.assertRaisesRegexp(ValueError, 'AUTH_CHALLENGE should have 3 methods, but only had 4 bytes for it', Cell.pop, b'\x00\x00\x82\x00&' + CHALLENGE + b'\x00\x03\x00\x01\x00\x03', 2)