commit 7eb31ce4acfe000d475a76b0b6ea21187fabfed4 Author: Damian Johnson atagar@torproject.org Date: Sat Jun 23 12:17:21 2018 -0700

Adjust to use new 'unused' attribute

Great catch. Just adjusting this to populate the 'unused' attribute I added. --- stem/client/cell.py | 37 +++++++++++++++---------------------- test/unit/client/cell.py | 33 ++++++++++++++++----------------- 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/stem/client/cell.py b/stem/client/cell.py index 756788e9..f000347d 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -392,8 +392,8 @@ class DestroyCell(CircuitCell): VALUE = 4 IS_FIXED_SIZE = True

- def __init__(self, circ_id, reason = CloseReason.NONE): - super(DestroyCell, self).__init__(circ_id) + def __init__(self, circ_id, reason = CloseReason.NONE, unused = b''): + super(DestroyCell, self).__init__(circ_id, unused) self.reason, self.reason_int = CloseReason.get(reason)

def pack(self, link_protocol): @@ -401,11 +401,8 @@ class DestroyCell(CircuitCell):

@classmethod def _unpack(cls, content, circ_id, link_protocol): - reason, _ = Size.CHAR.pop(content) - - # remaining content (if any) is thrown out (ignored) - - return DestroyCell(circ_id, reason) + reason, unused = Size.CHAR.pop(content) + return DestroyCell(circ_id, reason, unused)

def __hash__(self): return _hash_attr(self, 'circ_id', 'reason_int') @@ -423,13 +420,13 @@ class CreateFastCell(CircuitCell): VALUE = 5 IS_FIXED_SIZE = True

- def __init__(self, circ_id, key_material = None): + def __init__(self, circ_id, key_material = None, unused = b''): if not key_material: key_material = os.urandom(HASH_LEN) elif len(key_material) != HASH_LEN: raise ValueError('Key material should be %i bytes, but was %i' % (HASH_LEN, len(key_material)))

- super(CreateFastCell, self).__init__(circ_id) + super(CreateFastCell, self).__init__(circ_id, unused) self.key_material = key_material

def pack(self, link_protocol): @@ -437,14 +434,12 @@ class CreateFastCell(CircuitCell):

@classmethod def _unpack(cls, content, circ_id, link_protocol): - key_material, _ = split(content, HASH_LEN) - - # remaining content (if any) is thrown out (ignored) + key_material, unused = split(content, HASH_LEN)

if len(key_material) != HASH_LEN: raise ValueError('Key material should be %i bytes, but was %i' % (HASH_LEN, len(key_material)))

- return CreateFastCell(circ_id, key_material) + return CreateFastCell(circ_id, key_material, unused)

def __hash__(self): return _hash_attr(self, 'circ_id', 'key_material') @@ -462,7 +457,7 @@ class CreatedFastCell(CircuitCell): VALUE = 6 IS_FIXED_SIZE = True

- def __init__(self, circ_id, derivative_key, key_material = None): + def __init__(self, circ_id, derivative_key, key_material = None, unused = b''): if not key_material: key_material = os.urandom(HASH_LEN) elif len(key_material) != HASH_LEN: @@ -471,7 +466,7 @@ class CreatedFastCell(CircuitCell): if len(derivative_key) != HASH_LEN: raise ValueError('Derivatived key should be %i bytes, but was %i' % (HASH_LEN, len(derivative_key)))

- super(CreatedFastCell, self).__init__(circ_id) + super(CreatedFastCell, self).__init__(circ_id, unused) self.key_material = key_material self.derivative_key = derivative_key

@@ -480,15 +475,13 @@ class CreatedFastCell(CircuitCell):

@classmethod def _unpack(cls, content, circ_id, link_protocol): - content_to_parse, _ = split(content, HASH_LEN * 2) - - # remaining content (if any) is thrown out (ignored) + if len(content) < HASH_LEN * 2: + raise ValueError('Key material and derivatived key should be %i bytes, but was %i' % (HASH_LEN * 2, len(content)))

- if len(content_to_parse) != HASH_LEN * 2: - raise ValueError('Key material and derivatived key should be %i bytes, but was %i' % (HASH_LEN * 2, len(content_to_parse))) + key_material, content = split(content, HASH_LEN) + derivative_key, content = split(content, HASH_LEN)

- key_material, derivative_key = split(content_to_parse, HASH_LEN) - return CreatedFastCell(circ_id, derivative_key, key_material) + return CreatedFastCell(circ_id, derivative_key, key_material, content)

def __hash__(self): return _hash_attr(self, 'circ_id', 'derivative_key', 'key_material') diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 7b5695a2..93468318 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -39,9 +39,9 @@ RELAY_CELLS = { }

DESTROY_CELLS = { - b'\x80\x00\x00\x00\x04\x00' + ZERO * 508: (2147483648, CloseReason.NONE, 0, True, True), - b'\x80\x00\x00\x00\x04\x03' + ZERO * 508: (2147483648, CloseReason.REQUESTED, 3, True, True), - b'\x80\x00\x00\x00\x04\x01' + b'\x01' + ZERO * 507: (2147483648, CloseReason.PROTOCOL, 1, True, False), + b'\x80\x00\x00\x00\x04\x00' + ZERO * 508: (2147483648, CloseReason.NONE, 0, ZERO * 508), + b'\x80\x00\x00\x00\x04\x03' + ZERO * 508: (2147483648, CloseReason.REQUESTED, 3, ZERO * 508), + b'\x80\x00\x00\x00\x04\x01' + b'\x01' + ZERO * 507: (2147483648, CloseReason.PROTOCOL, 1, b'\x01' + ZERO * 507), }

CREATE_FAST_CELLS = { @@ -189,7 +189,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) + self.assertEqual(ZERO * (498 - len(cell.data)), cell.unused)

digest = hashlib.sha1(b'hi') self.assertEqual(3257622417, RelayCell(5, 'RELAY_BEGIN_DIR', '', digest, 564346860).digest) @@ -199,20 +199,19 @@ class TestCell(unittest.TestCase): self.assertRaisesRegexp(ValueError, "Invalid enumeration 'NO_SUCH_COMMAND', options are RELAY_BEGIN, RELAY_DATA", RelayCell, 5, 'NO_SUCH_COMMAND', '', 5, 564346860)

def test_destroy_cell(self): - for cell_bytes, (circ_id, reason, reason_int, test_unpack, test_recreate_exactly) in DESTROY_CELLS.items(): - if test_recreate_exactly: + for cell_bytes, (circ_id, reason, reason_int, unused) in DESTROY_CELLS.items(): + # Packed cells always pad with zeros, so if we're testing something with + # non-zero padding then skip this check. + + if not unused.strip(ZERO): self.assertEqual(cell_bytes, DestroyCell(circ_id, reason).pack(5)) self.assertEqual(cell_bytes, DestroyCell(circ_id, reason_int).pack(5))

- if test_unpack: - cell = Cell.pop(cell_bytes, 5)[0] - self.assertEqual(circ_id, cell.circ_id) - self.assertEqual(reason, cell.reason) - self.assertEqual(reason_int, cell.reason_int) - self.assertEqual(b'', cell.unused) - - if not any((test_unpack, test_recreate_exactly)): - self.fail('Must test something!') + cell = Cell.pop(cell_bytes, 5)[0] + self.assertEqual(circ_id, cell.circ_id) + self.assertEqual(reason, cell.reason) + self.assertEqual(reason_int, cell.reason_int) + self.assertEqual(unused, cell.unused)

def test_create_fast_cell(self): for cell_bytes, (circ_id, key_material) in CREATE_FAST_CELLS.items(): @@ -221,7 +220,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.assertEqual(ZERO * 489, cell.unused)

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

@@ -233,7 +232,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.assertEqual(ZERO * 469, cell.unused)

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