August 2018 - tor-commits - lists.torproject.org

[stem/master] Add decrypt instance method to BaseRelayCell
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 5baa2e37728ab50a5132d81225070e097e6bd058 Author: Dave Rolek <dmr-x(a)riseup.net> Date: Sat Aug 18 03:58:13 2018 +0000 Add decrypt instance method to BaseRelayCell Akin to encrypt(), this takes care of decryption and all the ancillary functionality related to it. It composes the functionality of the building blocks added in previous changes, namely: * check_recognized_field() * check_digest() * interpret_cell() - but not automatically done by default A consumer of this, especially in a multi-hop circuit, will only need to manage the digest/decryptor state (in the case of a cell that cannot be decrypted+recognized... fallback) and pay attention to the 'fully_decrypted' result. This should make a consumer's implementation pretty readable, something like: # relay_1 := nearest relay in circuit decryption_states = [ (relay_1, digest_1, decryptor_1), (relay_2, digest_2, decryptor_2), (relay_3, digest_3, decryptor_3), ] new_decryption_states = copy.deepcopy(decryption_states) from_relay = None for i in range(len(new_decryption_states)): relay, digest, decryptor = new_decryption_states[i] cell, fully_decrypted, digest, decryptor = cell.decrypt(digest, decryptor) new_decryption_states[i] = (relay, digest, decryptor) if fully_decrypted: from_relay = relay break if from_relay: decryption_states = new_decryption_states else: # handle this, since the cell couldn't be decrypted # probably raise Something() pass cell_and_hop = (cell, from_relay) # naming things is hard # do something with the decrypted cell # ^ probably feeding it into the queue/handler for the (circuit_id, relay, stream_id) --- stem/client/cell.py | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/stem/client/cell.py b/stem/client/cell.py index a96b2f7b..48dbcf36 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -443,6 +443,71 @@ class BaseRelayCell(CircuitCell): return new_cell + def decrypt(self, digest, decryptor, interpret = False): + """ + Decrypts a cell and checks whether it is fully decrypted, + returning a new (Cell, fully_decrypted, digest, decryptor) tuple. + Optionally also interprets the cell (not generally recommended). + + The method name is technically a misnomer, as it also checks whether the + cell has been fully decrypted (after decrypting), updating the digest if so. + However, these operations are defined per the spec as required for RELAY + cells, and ... + (1) it is a natural mental extension to include them here; + (2) it would be a bit pointless to require method consumers to manually + do all of that, for pedantry. + + :param HASH digest: running digest held with the relay + :param cryptography.hazmat.primitives.ciphers.CipherContext decryptor: + running stream cipher decryptor held with the relay + + :param bool interpret: (optional, defaults to **False**) Use **True** with + caution. The spec indicates that a fully decrypted cell should be + accounted for in digest and decryptor, independent of cell validity. Using + **True**, while convenient, may cause an exception for a NYI relay + command, a malformed cell, or some other reason. This option should only + be used when the consumer will consider the circuit to have a fatal error + in such cases, and catches/handles the exception accordingly (e.g. sending + a DestroyCell). + + :returns: (:class:`~stem.client.cell.Cell`, bool, HASH, CipherContext) tuple + of object copies updated as follows: + * Cell: either :class:`~stem.client.cell.RawRelayCell` with a decrypted + payload or :class:`~stem.client.cell.RelayCell` class or subclass, if + **interpret** is **True** and the cell was fully decrypted + * fully_decrypted: **bool** indicating whether the cell is fully + decrypted + * digest: updated via digest.update(payload), if the cell was fully + decrypted; otherwise a copy of the original + * decryptor: updated via decryptor.update(payload) + """ + + new_decryptor = copy.copy(decryptor) + + # actually decrypt + decrypted_payload = new_decryptor.update(self.payload) + new_cell = self.__class__(self.circ_id, decrypted_payload) + + # do post-decryption checks to ascertain whether cell is fully decrypted + if new_cell.check_recognized_field(): + digest_matches, new_digest = new_cell.check_digest(digest) + fully_decrypted = digest_matches + else: + new_digest = None + fully_decrypted = False + + # only return the new_digest if the digest check meant that the cell has been fully decrypted + # + # furthermore, even if the digest was not updated, return a copy + # this allows a consumer to always assume the returned digest is a different object + digest_to_return = new_digest if fully_decrypted else digest.copy() + + if interpret and fully_decrypted: + # this might raise an exception; oh well, we did warn about that + new_cell = new_cell.interpret_cell() + + return new_cell, fully_decrypted, digest_to_return, new_decryptor + def __hash__(self): return stem.util._hash_attr(self, 'circ_id', 'payload', cache = True)

1 0

[stem/master] Simplify rollback of key/digest during errors
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 56f41833173aca0c5e78ca2095935f8a474c6430 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Aug 24 03:49:17 2018 -0700 Simplify rollback of key/digest during errors The reason I asked for our functions to return a new copy of our key/digest is because it lets us simplify this calling code a bit. Rather than explicitly reverting the key/digest we can simply wait to record the new values until cells are successfully sent and received. --- stem/client/__init__.py | 53 ++++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 8c96b8c2..0ee8cbf0 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -25,7 +25,6 @@ a wrapper for :class:`~stem.socket.RelaySocket`, much the same way as +- close - closes this circuit """ -import copy import hashlib import threading @@ -234,18 +233,16 @@ class Circuit(object): """ with self.relay._orport_lock: - orig_forward_digest = self.forward_digest.copy() - orig_forward_key = copy.copy(self.forward_key) + cell = stem.client.cell.RelayCell(self.id, command, data, stream_id = stream_id) + encrypted_cell, new_forward_digest, new_forward_key = cell.encrypt(self.forward_digest, self.forward_key) - try: - cell = stem.client.cell.RelayCell(self.id, command, data, stream_id = stream_id) - encrypted_cell, self.forward_digest, self.forward_key = cell.encrypt(self.forward_digest, self.forward_key) + self.relay._orport.send(encrypted_cell.pack(self.relay.link_protocol)) - self.relay._orport.send(encrypted_cell.pack(self.relay.link_protocol)) - except: - self.forward_digest = orig_forward_digest - self.forward_key = orig_forward_key - raise + # Only recoding our new digest/key if the cell's successfully sent. If + # the above raises we should leave them alone. + + self.forward_digest = new_forward_digest + self.forward_key = new_forward_key reply = self.relay._orport.recv() reply_cells = [] @@ -253,24 +250,22 @@ class Circuit(object): relay_cell_cmd = stem.client.cell.RelayCell.VALUE while reply: - orig_backward_digest = self.backward_digest.copy() - orig_backward_key = copy.copy(self.backward_key) - - try: - raw_cell, reply = stem.client.cell.Cell.pop(reply, self.relay.link_protocol) - - if raw_cell.VALUE != relay_cell_cmd: - raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (relay_cell_cmd, raw_cell.VALUE)) - elif raw_cell.circ_id != self.id: - raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, raw_cell.circ_id)) - - decrypted_cell, fully_decrypted, self.backward_digest, self.backward_key = raw_cell.decrypt(self.backward_digest, self.backward_key, interpret = True) - if not fully_decrypted: - raise stem.ProtocolError('Response for circuit id %i was not fully decrypted, when expected to be' % self.id) - except: - self.backward_digest = orig_backward_digest - self.backward_key = orig_backward_key - raise + raw_cell, reply = stem.client.cell.Cell.pop(reply, self.relay.link_protocol) + + if raw_cell.VALUE != relay_cell_cmd: + raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (relay_cell_cmd, raw_cell.VALUE)) + elif raw_cell.circ_id != self.id: + raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, raw_cell.circ_id)) + + decrypted_cell, fully_decrypted, new_backward_digest, new_backward_key = raw_cell.decrypt(self.backward_digest, self.backward_key, interpret = True) + + if not fully_decrypted: + raise stem.ProtocolError('Response for circuit id %i was not fully decrypted, when expected to be' % self.id) + + # Again, if the above raises the digest/key should remain unchanged. + + self.backward_digest = new_backward_digest + self.backward_key = new_backward_key reply_cells.append(decrypted_cell)

1 0

[stem/master] Actually make copies of the backward digest/key, too
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 03c115dba10de3aca5374ea61893189e5c8b46a1 Author: Dave Rolek <dmr-x(a)riseup.net> Date: Sat Aug 18 04:14:43 2018 +0000 Actually make copies of the backward digest/key, too And rename the other 'orig_' vars to 'orig_forward_' vars, for clarity. --- stem/client/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 33ac0d8d..91e8c4ca 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -234,8 +234,10 @@ class Circuit(object): """ with self.relay._orport_lock: - orig_digest = self.forward_digest.copy() - orig_key = copy.copy(self.forward_key) + orig_forward_digest = self.forward_digest.copy() + orig_forward_key = copy.copy(self.forward_key) + orig_backward_digest = self.backward_digest.copy() + orig_backward_key = copy.copy(self.backward_key) try: cell = stem.client.cell.RelayCell(self.id, command, data, stream_id = stream_id) @@ -263,8 +265,10 @@ class Circuit(object): return reply_cells except: - self.forward_digest = orig_digest - self.forward_key = orig_key + self.forward_digest = orig_forward_digest + self.forward_key = orig_forward_key + self.backward_digest = orig_backward_digest + self.backward_key = orig_backward_key raise def close(self):

1 0

[stem/master] Drop unsigned from Size class
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 575f1b6685f6b8be3b3801ff662a4d6d89db43c4 Author: Damian Johnson <atagar(a)torproject.org> Date: Thu Aug 16 10:33:47 2018 -0700 Drop unsigned from Size class Actually, it's already in our pydoc that these are all unsigned. Maybe we'll add a flag like this in the future if that changes but presently it doesn't provide any value. That said, I *do* like the idea of providing a better error message. Keeping that, and improving our performance a bit by only performing these checks if packing fails (this class is heavily used in IO so making this a tad extra performant is useful). --- stem/client/datatype.py | 16 +++++++++------- test/unit/client/size.py | 9 +-------- test/unit/control/controller.py | 1 - 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/stem/client/datatype.py b/stem/client/datatype.py index 1c7aec77..43bdfe9d 100644 --- a/stem/client/datatype.py +++ b/stem/client/datatype.py @@ -349,19 +349,21 @@ class Size(Field): self.name = name self.size = size self.format = pack_format - self.unsigned = pack_format.isupper() @staticmethod def pop(packed): raise NotImplementedError("Use our constant's unpack() and pop() instead") def pack(self, content): - if not stem.util._is_int(content): - raise ValueError('Size.pack encodes an integer, but was a %s' % type(content).__name__) - if content < 0 and self.unsigned: - raise ValueError('A %s field cannot pack negative values, but %i was tried' % (self.name, content)) - - packed = struct.pack(self.format, content) + try: + packed = struct.pack(self.format, content) + except struct.error: + if not stem.util._is_int(content): + raise ValueError('Size.pack encodes an integer, but was a %s' % type(content).__name__) + elif content < 0: + raise ValueError('Packed values must be positive (attempted to pack %i as a %s)' % (content, self.name)) + else: + raise # some other struct exception if self.size != len(packed): raise ValueError('%s is the wrong size for a %s field' % (repr(packed), self.name)) diff --git a/test/unit/client/size.py b/test/unit/client/size.py index 3d7d796f..733db0f4 100644 --- a/test/unit/client/size.py +++ b/test/unit/client/size.py @@ -7,22 +7,17 @@ import unittest from stem.client.datatype import Size -SIGNED_CHAR = Size('SIGNED_CHAR', 1, '!b') - class TestSize(unittest.TestCase): def test_attributes(self): self.assertEqual('CHAR', Size.CHAR.name) self.assertEqual('!B', Size.CHAR.format) - self.assertEqual(True, Size.CHAR.unsigned) self.assertEqual(1, Size.CHAR.size) self.assertEqual(2, Size.SHORT.size) self.assertEqual(4, Size.LONG.size) self.assertEqual(8, Size.LONG_LONG.size) - self.assertEqual(False, SIGNED_CHAR.unsigned) - def test_pack(self): self.assertEqual(b'\x12', Size.CHAR.pack(18)) self.assertEqual(b'\x00\x12', Size.SHORT.pack(18)) @@ -31,13 +26,11 @@ class TestSize(unittest.TestCase): self.assertRaisesWith(ValueError, 'Size.pack encodes an integer, but was a str', Size.CHAR.pack, 'hi') - self.assertRaisesWith(ValueError, 'A CHAR field cannot pack negative values, but -1 was tried', Size.CHAR.pack, -1) + self.assertRaisesWith(ValueError, 'Packed values must be positive (attempted to pack -1 as a CHAR)', Size.CHAR.pack, -1) bad_size = Size('BAD_SIZE', 1, '!H') self.assertRaisesRegexp(ValueError, re.escape("'\\x00\\x12' is the wrong size for a BAD_SIZE field"), bad_size.pack, 18) - self.assertEqual(b'\xFF', SIGNED_CHAR.pack(-1)) - def test_unpack(self): self.assertEqual(18, Size.CHAR.unpack(b'\x12')) self.assertEqual(18, Size.SHORT.unpack(b'\x00\x12')) diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index 7ee2ab70..ba55208a 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -5,7 +5,6 @@ integ tests, but a few bits lend themselves to unit testing. import datetime import io -import time import unittest import stem.descriptor.router_status_entry

1 0

[stem/master] Combine relay cell encrypting and packing
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 486a8d2e5c4724146d6647b7f69ffbd10adbfc1d Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 11:38:41 2018 -0700 Combine relay cell encrypting and packing Cells are only ever encrypted to be sent. I'm not seeing a purpose in having an intermediate step here. Our Circuit's send() method is simpler if the encrypt() method provides us the payload to be sent rather than an intermediate class. Are there any use cases where we want an 'encrypted cell' object? We can't do anything with it besides send it (if we made it) or decrypt it (if sent to us). Possible though I'm missing something. Presently I'm only looking at our Circuit's send() method. Our RelayCell's usage informs the API it should have, so I'm not worrying about the cell module until that's settled. --- stem/client/__init__.py | 15 +++++++-------- stem/client/cell.py | 10 +++++----- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 0ee8cbf0..46a82428 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -233,16 +233,15 @@ class Circuit(object): """ with self.relay._orport_lock: - cell = stem.client.cell.RelayCell(self.id, command, data, stream_id = stream_id) - encrypted_cell, new_forward_digest, new_forward_key = cell.encrypt(self.forward_digest, self.forward_key) - - self.relay._orport.send(encrypted_cell.pack(self.relay.link_protocol)) + # Encrypt and send the cell. Our digest/key only updates if the cell is + # successfully sent. - # Only recoding our new digest/key if the cell's successfully sent. If - # the above raises we should leave them alone. + cell = stem.client.cell.RelayCell(self.id, command, data, stream_id = stream_id) + payload, forward_digest, forward_key = cell.encrypt(self.relay.link_protocol, self.forward_digest, self.forward_key) + self.relay._orport.send(payload) - self.forward_digest = new_forward_digest - self.forward_key = new_forward_key + self.forward_digest = forward_digest + self.forward_key = forward_key reply = self.relay._orport.recv() reply_cells = [] diff --git a/stem/client/cell.py b/stem/client/cell.py index 48dbcf36..6587ebb5 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -654,7 +654,7 @@ class RelayCell(CircuitCell): return new_cell, new_digest - def encrypt(self, digest, encryptor, **kwargs): + def encrypt(self, link_protocol, digest, encryptor, **kwargs): """ Preps a cell payload, including calculating digest, and encrypts it, returning a new (RawRelayCell, digest, encryptor) tuple. @@ -666,6 +666,7 @@ class RelayCell(CircuitCell): (2) it would be a bit pointless to require method consumers to manually call both, for pedantry. + :param int link_protocol: link protocol version :param HASH digest: running digest held with the relay :param cryptography.hazmat.primitives.ciphers.CipherContext encryptor: running stream cipher encryptor held with the relay @@ -673,10 +674,9 @@ class RelayCell(CircuitCell): :param bool prep_cell: (optional, defaults to **True**) refer to :func:`~stem.client.cell.RelayCell.apply_digest` - :returns: (:class:`~stem.client.cell.RawRelayCell`, HASH, CipherContext) + :returns: (bytes, HASH, CipherContext) tuple of object copies updated as follows: - * RawRelayCell: updated as specified in - :func:`~stem.client.cell.RelayCell.apply_digest`, then encrypted + * bytes: encrypted cell payload * digest: updated via digest.update(payload) * encryptor: updated via encryptor.update(payload_with_digest) """ @@ -686,7 +686,7 @@ class RelayCell(CircuitCell): encrypted_payload = new_encryptor.update(unencrypted_cell.pack_payload()) encrypted_cell = RawRelayCell(unencrypted_cell.circ_id, encrypted_payload) - return encrypted_cell, new_digest, new_encryptor + return encrypted_cell.pack(link_protocol), new_digest, new_encryptor def pack_payload(self, **kwargs): """

1 0

[stem/master] Stub minimal cell decryption function
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 52edd22ca10d559270ee240409447351bd8639f8 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 13:32:07 2018 -0700 Stub minimal cell decryption function Ok, now into the meat of this sucker. Dave's branch introduces nice additions such as validating our backward digest and a flag to indicate if we've fully decrypted the cell or not. For the moment moving to a minimal decryption function that only supports what we did previously. Next gonna try to integrate his new features to make a hybrid approach that hopefully takes the best of both. --- stem/client/__init__.py | 22 +++++++--------------- stem/client/cell.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index a216696a..19528d67 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -33,7 +33,7 @@ import stem.client.cell import stem.socket import stem.util.connection -from stem.client.datatype import ZERO, LinkProtocol, Address, KDF +from stem.client.datatype import ZERO, LinkProtocol, Address, KDF, split __all__ = [ 'cell', @@ -253,22 +253,14 @@ class Circuit(object): raise stem.ProtocolError('Circuit response should be a series of RELAY cells, but received an unexpected size for a response: %i' % len(reply)) while reply: - raw_cell, reply = stem.client.cell.Cell.pop(reply, self.relay.link_protocol) + encrypted_cell, reply = split(reply, self.relay.link_protocol.fixed_cell_length) + decrypted_cell, backward_key, backward_digest = stem.client.cell.RelayCell.decrypt(encrypted_cell, self.backward_key, self.backward_digest) - if raw_cell.VALUE != stem.client.cell.RelayCell.VALUE: - raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (stem.client.cell.RelayCell.VALUE, raw_cell.VALUE)) - elif raw_cell.circ_id != self.id: - raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, raw_cell.circ_id)) + if self.id != decrypted_cell.circ_id: + raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, decrypted_cell.circ_id)) - decrypted_cell, fully_decrypted, new_backward_digest, new_backward_key = raw_cell.decrypt(self.backward_digest, self.backward_key, interpret = True) - - if not fully_decrypted: - raise stem.ProtocolError('Response for circuit id %i was not fully decrypted, when expected to be' % self.id) - - # Again, if the above raises the digest/key should remain unchanged. - - self.backward_digest = new_backward_digest - self.backward_key = new_backward_key + self.backward_digest = backward_digest + self.backward_key = backward_key reply_cells.append(decrypted_cell) diff --git a/stem/client/cell.py b/stem/client/cell.py index 6587ebb5..3de1c731 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -551,6 +551,36 @@ class RelayCell(CircuitCell): elif stream_id and self.command in STREAM_ID_DISALLOWED: raise ValueError('%s relay cells concern the circuit itself and cannot have a stream id' % self.command) + @classmethod + def decrypt(link_protocol, content, digest, key): + """ + Parse the given content as an encrypted RELAY cell. + """ + + # TODO: Fill in the above pydocs, deduplicate with the other decrypt + # method, yadda yadda. Starting with a minimal stub to see if this makes + # the Circuit class better. I'll circle back to clean up this module if it + # works. + + if len(content) != link_protocol.fixed_cell_length: + raise stem.ProtocolError('RELAY cells should be %i bytes, but received %i' % (link_protocol.fixed_cell_length, len(content))) + + circ_id, content = link_protocol.circ_id_size.pop(content) + command, payload = Size.CHAR.pop(content) + + if command != RelayCell.VALUE: + raise stem.ProtocolError('Cannot decrypt as a RELAY cell. This had command %i instead.' % command) + + key = copy.copy(key) + decrypted = key.update(payload) + + # TODO: Integrate with check_digest() and flag for integrating if we're + # fully decrypted. For the moment we only handle direct responses (ie. all + # the cells we receive can be fully decrypted) but if we attempt to support + # relaying we'll need to pass along cells we can only partially decrypt. + + return RelayCell._unpack(decrypted, circ_id, link_protocol), key, digest + @staticmethod def _coerce_digest(digest): """

1 0

[stem/master] Revert cell module hierarchy docs
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 347120a44614437f8e1642dc81676760f877c8be Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 13:45:47 2018 -0700 Revert cell module hierarchy docs Dropping BaseRelayCell and RawRelayCell from the hierarchy (reintegrating 'em into the RelayCell). Due to the whitespace changes this occupies a sizable chunk of the merge diff. --- stem/client/cell.py | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index 3de1c731..aec868c4 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -12,27 +12,25 @@ Messages communicated over a Tor relay's ORPort. Cell - Base class for ORPort messages. |- CircuitCell - Circuit management. - | |- CreateCell - Create a circuit. (section 5.1) - | |- CreatedCell - Acknowledge create. (section 5.1) - | |- BaseRelayCell - End-to-end data; abstract. (section 6.1) - | | +- RawRelayCell - End-to-end data. Payload not unpacked. (section 5.5.2.1, 5.5.3) - | |- RelayCell - End-to-end data. (section 6.1) - | |- DestroyCell - Stop using a circuit. (section 5.4) - | |- CreateFastCell - Create a circuit, no PK. (section 5.1) - | |- CreatedFastCell - Circuit created, no PK. (section 5.1) - | |- RelayEarlyCell - End-to-end data; limited. (section 5.6) - | |- Create2Cell - Extended CREATE cell. (section 5.1) - | +- Created2Cell - Extended CREATED cell. (section 5.1) + | |- CreateCell - Create a circuit. (section 5.1) + | |- CreatedCell - Acknowledge create. (section 5.1) + | |- RelayCell - End-to-end data. (section 6.1) + | |- DestroyCell - Stop using a circuit. (section 5.4) + | |- CreateFastCell - Create a circuit, no PK. (section 5.1) + | |- CreatedFastCell - Circuit created, no PK. (section 5.1) + | |- RelayEarlyCell - End-to-end data; limited. (section 5.6) + | |- Create2Cell - Extended CREATE cell. (section 5.1) + | +- Created2Cell - Extended CREATED cell. (section 5.1) | - |- PaddingCell - Padding negotiation. (section 7.2) - |- VersionsCell - Negotiate proto version. (section 4) - |- NetinfoCell - Time and address info. (section 4.5) - |- PaddingNegotiateCell - Padding negotiation. (section 7.2) - |- VPaddingCell - Variable-length padding. (section 7.2) - |- CertsCell - Relay certificates. (section 4.2) - |- AuthChallengeCell - Challenge value. (section 4.3) - |- AuthenticateCell - Client authentication. (section 4.5) - |- AuthorizeCell - Client authorization. (not yet used) + |- PaddingCell - Padding negotiation. (section 7.2) + |- VersionsCell - Negotiate proto version. (section 4) + |- NetinfoCell - Time and address info. (section 4.5) + |- PaddingNegotiateCell - Padding negotiation. (section 7.2) + |- VPaddingCell - Variable-length padding. (section 7.2) + |- CertsCell - Relay certificates. (section 4.2) + |- AuthChallengeCell - Challenge value. (section 4.3) + |- AuthenticateCell - Client authentication. (section 4.5) + |- AuthorizeCell - Client authorization. (not yet used) | |- pack - encodes cell into bytes |- unpack - decodes series of cells @@ -86,7 +84,6 @@ class Cell(object): The following cell types explicitly don't have *unused* content: * PaddingCell (we consider all content part of payload) * VersionsCell (all content is unpacked and treated as a version specification) - * BaseRelayCell (we don't parse cell beyond header/body) * VPaddingCell (we consider all content part of payload) :var bytes unused: unused filler that padded the cell to the expected size

1 0

[stem/master] Resume size check for circuit responses
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 2e51a01c1fa96f24f44b744d37915ef2ffd7762d Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 12:39:44 2018 -0700 Resume size check for circuit responses Maybe this check is redundant with the cell module additions? Not sure. Regardless, unlike the hamhanded 'relay_cell_size' calculation I did prior to Dave's branch our LinkProtocol's 'fixed_cell_length' now makes this check trivial to do upfront. --- stem/client/__init__.py | 10 +++++++--- stem/client/datatype.py | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 46a82428..a216696a 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -243,16 +243,20 @@ class Circuit(object): self.forward_digest = forward_digest self.forward_key = forward_key + # Decrypt relay cells received in response. Again, our digest/key only + # updates when handled successfully. + reply = self.relay._orport.recv() reply_cells = [] - relay_cell_cmd = stem.client.cell.RelayCell.VALUE + if len(reply) % self.relay.link_protocol.fixed_cell_length != 0: + raise stem.ProtocolError('Circuit response should be a series of RELAY cells, but received an unexpected size for a response: %i' % len(reply)) while reply: raw_cell, reply = stem.client.cell.Cell.pop(reply, self.relay.link_protocol) - if raw_cell.VALUE != relay_cell_cmd: - raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (relay_cell_cmd, raw_cell.VALUE)) + if raw_cell.VALUE != stem.client.cell.RelayCell.VALUE: + raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (stem.client.cell.RelayCell.VALUE, raw_cell.VALUE)) elif raw_cell.circ_id != self.id: raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, raw_cell.circ_id)) diff --git a/stem/client/datatype.py b/stem/client/datatype.py index 43bdfe9d..644d4993 100644 --- a/stem/client/datatype.py +++ b/stem/client/datatype.py @@ -116,6 +116,7 @@ import collections import hashlib import struct +import stem.client.cell import stem.prereq import stem.util import stem.util.connection @@ -246,9 +247,11 @@ class LinkProtocol(int): protocol = int.__new__(cls, version) protocol.version = version protocol.circ_id_size = Size.LONG if version > 3 else Size.SHORT - protocol.fixed_cell_length = 514 if version > 3 else 512 protocol.first_circ_id = 0x80000000 if version > 3 else 0x01 + cell_header_size = protocol.circ_id_size.size + 1 # circuit id (2 or 4 bytes) + command (1 byte) + protocol.fixed_cell_length = cell_header_size + stem.client.cell.FIXED_PAYLOAD_LEN + return protocol def __hash__(self):

1 0

[stem/master] Fork new and old RelayCell implementations
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 04c2c9f0e33055dd40c9117314287077c9c22aee Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 14:16:35 2018 -0700 Fork new and old RelayCell implementations Dave's branch makes quite a few RelayCell modifications in support of its new BaseRelayCell class. That class is no longer used by the new Circuit send method, but there's quite a few things we want to integrate from it. Making a total mess, but for the moment placing side-by-side copies of both our new and old RelayCell class in this module. AlternateRelayCell is Dave's version, and RelayCell is what we had previously (plus the decrypt stub from earlier). This does *not* presently work, as the old RelayCell class lack the encrypt() and decrypt() methods the Circuit class now excepts. Doing this so we have a clean starting point to see how we integrate. --- stem/client/cell.py | 84 +++++++++++++++++++++++++++++++++++++++++++++--- test/unit/client/cell.py | 3 +- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index aec868c4..3f247af4 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -364,7 +364,7 @@ class BaseRelayCell(CircuitCell): :func:`~stem.client.cell.BaseRelayCell.check_digest` """ - _, recognized_from_cell, _, _, _, _, _ = RelayCell._unpack_payload(self.payload) + _, recognized_from_cell, _, _, _, _, _ = AlternateRelayCell._unpack_payload(self.payload) return recognized_from_cell == 0 def check_digest(self, digest): @@ -383,10 +383,10 @@ class BaseRelayCell(CircuitCell): :raises: **ValueError** if payload is the wrong size """ - command, recognized, stream_id, digest_from_cell, data_len, data, unused = RelayCell._unpack_payload(self.payload) + command, recognized, stream_id, digest_from_cell, data_len, data, unused = AlternateRelayCell._unpack_payload(self.payload) # running digest is calculated using a zero'd digest field in the payload - prepared_payload = RelayCell._pack_payload(command, recognized, stream_id, 0, data_len, data, unused, pad_remainder = False) + prepared_payload = AlternateRelayCell._pack_payload(command, recognized, stream_id, 0, data_len, data, unused, pad_remainder = False) if len(prepared_payload) != FIXED_PAYLOAD_LEN: # this should never fail @@ -396,7 +396,7 @@ class BaseRelayCell(CircuitCell): new_digest = digest.copy() new_digest.update(prepared_payload) - digest_matches = (RelayCell._coerce_digest(new_digest) == digest_from_cell) + digest_matches = (AlternateRelayCell._coerce_digest(new_digest) == digest_from_cell) # only return the new_digest if the digest check passed # even if not, return a copy of the original @@ -518,6 +518,82 @@ class RelayCell(CircuitCell): """ Command concerning a relay circuit. + :var stem.client.RelayCommand command: command to be issued + :var int command_int: integer value of our command + :var bytes data: payload of the cell + :var int recognized: zero if cell is decrypted, non-zero otherwise + :var int digest: running digest held with the relay + :var int stream_id: specific stream this concerns + """ + + NAME = 'RELAY' + VALUE = 3 + IS_FIXED_SIZE = True + + 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 + # name is 'hashlib.HASH' whereas PyPy calls it just 'HASH'. + + digest_packed = digest.digest()[:RELAY_DIGEST_SIZE.size] + digest = RELAY_DIGEST_SIZE.unpack(digest_packed) + elif stem.util._is_str(digest): + digest_packed = digest[:RELAY_DIGEST_SIZE.size] + digest = RELAY_DIGEST_SIZE.unpack(digest_packed) + elif stem.util._is_int(digest): + pass + 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, unused) + self.command, self.command_int = RelayCommand.get(command) + self.recognized = recognized + self.stream_id = stream_id + self.digest = digest + self.data = str_tools._to_bytes(data) + + if digest == 0: + if not stream_id and self.command in STREAM_ID_REQUIRED: + raise ValueError('%s relay cells require a stream id' % self.command) + elif stream_id and self.command in STREAM_ID_DISALLOWED: + raise ValueError('%s relay cells concern the circuit itself and cannot have a stream id' % self.command) + + def pack(self, link_protocol): + payload = bytearray() + payload += Size.CHAR.pack(self.command_int) + payload += Size.SHORT.pack(self.recognized) + payload += Size.SHORT.pack(self.stream_id) + payload += Size.LONG.pack(self.digest) + payload += Size.SHORT.pack(len(self.data)) + payload += self.data + + return RelayCell._pack(link_protocol, bytes(payload), self.unused, self.circ_id) + + @classmethod + def _unpack(cls, content, circ_id, link_protocol): + command, content = Size.CHAR.pop(content) + recognized, content = Size.SHORT.pop(content) # 'recognized' field + stream_id, content = Size.SHORT.pop(content) + digest, content = Size.LONG.pop(content) + data_len, content = Size.SHORT.pop(content) + data, unused = split(content, data_len) + + if len(data) != data_len: + raise ValueError('%s cell said it had %i bytes of data, but only had %i' % (cls.NAME, data_len, len(data))) + + return RelayCell(circ_id, command, data, digest, stream_id, recognized, unused) + + def __hash__(self): + return stem.util._hash_attr(self, 'command_int', 'stream_id', 'digest', 'data', cache = True) + + +# TODO: merge the below with the RelayCell + +class AlternateRelayCell(CircuitCell): + """ + Command concerning a relay circuit. + :var stem.client.datatype.RelayCommand command: command to be issued :var int command_int: integer value of our command :var bytes data: payload of the cell diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index b4c0bd94..74d664d4 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -16,6 +16,7 @@ from stem.client.cell import ( PaddingCell, BaseRelayCell, RawRelayCell, + AlternateRelayCell, RelayCell, DestroyCell, CreateFastCell, @@ -225,7 +226,7 @@ class TestCell(unittest.TestCase): self.assertRaisesWith(ValueError, expected_message_format % payload_len, BaseRelayCell, arbitrary_circ_id, ZERO * payload_len) def test_relay_cell(self): - self.assertEquals(True, RelayCell.CANNOT_DIRECTLY_UNPACK) + self.assertEquals(True, AlternateRelayCell.CANNOT_DIRECTLY_UNPACK) for cell_bytes, (command, command_int, circ_id, stream_id, data, digest, unused, link_protocol) in RELAY_CELLS.items(): if not unused.strip(ZERO):

1 0

[stem/master] Further explain our RelayCell's 'recognized' field
by atagar＠torproject.org 26 Aug '18

26 Aug '18

commit 1adde73d6cd05c253346abdaad199b9337cefc68 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Aug 25 14:28:54 2018 -0700 Further explain our RelayCell's 'recognized' field Few months back this field throughly confused me so I asked Roger about it. Passing on the helpful explanation I got from him. --- stem/client/cell.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index 3f247af4..b40a30e1 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -518,10 +518,16 @@ class RelayCell(CircuitCell): """ Command concerning a relay circuit. + Our 'recognized' attribute provides a cheap (but incomplete) check for if our + cell payload is encrypted. If non-zero our payload *IS* encrypted, but if + zero we're *PROBABLY* fully decrypted. This uncertainty is because encrypted + cells have a small chance of coincidently producing zero for this value as + well. + :var stem.client.RelayCommand command: command to be issued :var int command_int: integer value of our command :var bytes data: payload of the cell - :var int recognized: zero if cell is decrypted, non-zero otherwise + :var int recognized: non-zero if payload is encrypted :var int digest: running digest held with the relay :var int stream_id: specific stream this concerns """

1 0