commit 72ba79d861972f848d6d777ed38c45663944b7ea Author: Damian Johnson atagar@torproject.org Date: Thu May 24 13:15:47 2018 -0700

Relay length attribute mangled by unpack/repack

Great catch from plcp!

https://trac.torproject.org/projects/tor/ticket/26060

When getting a circuit response we unpack then repack to get the data we just read from the socket. This is silly. We only did this to separate the cell headers from the payload but in doing so we corrupted the relay cell's length attribute.

Better to just read the raw encrypted data and only construct RelayCells *after* it has been decrypted. --- stem/client/__init__.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 281888e7..1d87966f 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -34,7 +34,7 @@ import stem.client.cell import stem.socket import stem.util.connection

-from stem.client.datatype import ZERO, Address, KDF, split +from stem.client.datatype import ZERO, Address, Size, KDF, split

__all__ = [ 'cell', @@ -250,14 +250,32 @@ class Circuit(object): header, payload = split(cell.pack(self.relay.link_protocol), header_size) encrypted_payload = header + self.forward_key.update(payload)

- reply = [] + reply_cells = [] self.relay._orport.send(encrypted_payload) + reply = self.relay._orport.recv()

- for cell in stem.client.cell.Cell.unpack(self.relay._orport.recv(), self.relay.link_protocol): - decrypted = self.backward_key.update(cell.pack(self.relay.link_protocol)[header_size:]) - reply.append(stem.client.cell.RelayCell._unpack(decrypted, self.id, self.relay.link_protocol)) + # Check that we got the correct number of bytes for a series of RELAY cells

- return reply + relay_cell_size = header_size + stem.client.cell.FIXED_PAYLOAD_LEN + relay_cell_cmd = stem.client.cell.RelayCell.VALUE + + if len(reply) % relay_cell_size != 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: + circ_id, reply = Size.SHORT.pop(reply) if self.relay.link_protocol < 4 else Size.LONG.pop(reply) + command, reply = Size.CHAR.pop(reply) + payload, reply = split(reply, stem.client.cell.FIXED_PAYLOAD_LEN) + + if command != relay_cell_cmd: + raise stem.ProtocolError('RELAY cell responses should be %i but was %i' % (relay_cell_cmd, command)) + elif circ_id != self.id: + raise stem.ProtocolError('Response should be for circuit id %i, not %i' % (self.id, circ_id)) + + decrypted = self.backward_key.update(payload) + reply_cells.append(stem.client.cell.RelayCell._unpack(decrypted, self.id, self.relay.link_protocol)) + + return reply_cells except: self.forward_digest = orig_digest self.forward_key = orig_key