From e1546bf03fc404ecdb6543f57835ec77a609c247 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 22 Dec 2016 18:17:05 -0500 Subject: [PATCH 1/5] Transit: send new (sided) handshakes --- src/wormhole/test/test_transit.py | 34 +++++++++++++++++++++++++++++++ src/wormhole/transit.py | 15 ++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/wormhole/test/test_transit.py b/src/wormhole/test/test_transit.py index 9f76e63..65bdf18 100644 --- a/src/wormhole/test/test_transit.py +++ b/src/wormhole/test/test_transit.py @@ -1,6 +1,7 @@ from __future__ import print_function, unicode_literals import io import gc +import mock from binascii import hexlify, unhexlify from twisted.trial import unittest from twisted.internet import defer, task, endpoints, protocol, address, error @@ -9,6 +10,7 @@ from twisted.python import log, failure from twisted.test import proto_helpers from ..errors import InternalError from .. import transit +from ..server import transit_server from .common import ServerBase from nacl.secret import SecretBox from nacl.exceptions import CryptoError @@ -1320,6 +1322,38 @@ class Transit(unittest.TestCase): relay_connectors[0].callback("winner") self.assertEqual(results, ["winner"]) +class RelayHandshake(unittest.TestCase): + def old_build_relay_handshake(self, key): + token = transit.HKDF(key, 32, CTXinfo=b"transit_relay_token") + return (token, b"please relay "+hexlify(token)+b"\n") + + def test_old(self): + key = b"\x00" + token, old_handshake = self.old_build_relay_handshake(key) + tc = transit_server.TransitConnection() + tc.factory = mock.Mock() + tc.factory.connection_got_token = mock.Mock() + tc.dataReceived(old_handshake[:-1]) + self.assertEqual(tc.factory.connection_got_token.mock_calls, []) + tc.dataReceived(old_handshake[-1:]) + self.assertEqual(tc.factory.connection_got_token.mock_calls, + [mock.call(hexlify(token), None, tc)]) + + def test_new(self): + c = transit.Common(None) + c.set_transit_key(b"\x00") + new_handshake = c._build_relay_handshake() + token, old_handshake = self.old_build_relay_handshake(b"\x00") + + tc = transit_server.TransitConnection() + tc.factory = mock.Mock() + tc.factory.connection_got_token = mock.Mock() + tc.dataReceived(new_handshake[:-1]) + self.assertEqual(tc.factory.connection_got_token.mock_calls, []) + tc.dataReceived(new_handshake[-1:]) + self.assertEqual(tc.factory.connection_got_token.mock_calls, + [mock.call(hexlify(token), c._side.encode("ascii"), tc)]) + class Full(ServerBase, unittest.TestCase): def doBoth(self, d1, d2): diff --git a/src/wormhole/transit.py b/src/wormhole/transit.py index 308e92f..d851070 100644 --- a/src/wormhole/transit.py +++ b/src/wormhole/transit.py @@ -1,6 +1,6 @@ # no unicode_literals, revisit after twisted patch from __future__ import print_function, absolute_import -import re, sys, time, socket +import os, re, sys, time, socket from collections import namedtuple, deque from binascii import hexlify, unhexlify import six @@ -15,6 +15,7 @@ from nacl.secret import SecretBox from hkdf import Hkdf from .errors import InternalError from .timing import DebugTiming +from .util import bytes_to_hexstr from . import ipaddrs def HKDF(skm, outlen, salt=None, CTXinfo=b""): @@ -76,9 +77,11 @@ def build_sender_handshake(key): hexid = HKDF(key, 32, CTXinfo=b"transit_sender") return b"transit sender "+hexlify(hexid)+b" ready\n\n" -def build_relay_handshake(key): +def build_sided_relay_handshake(key, side): + assert isinstance(side, type(u"")) + assert len(side) == 8*2 token = HKDF(key, 32, CTXinfo=b"transit_relay_token") - return b"please relay "+hexlify(token)+b"\n" + return b"please relay "+hexlify(token)+b" for side "+side.encode("ascii")+b"\n" # These namedtuples are "hint objects". The JSON-serializable dictionaries @@ -575,6 +578,7 @@ class Common: def __init__(self, transit_relay, no_listen=False, tor_manager=None, reactor=reactor, timing=None): + self._side = bytes_to_hexstr(os.urandom(8)) # unicode if transit_relay: if not isinstance(transit_relay, type(u"")): raise InternalError @@ -830,11 +834,14 @@ class Common: d.addBoth(_done) return d + def _build_relay_handshake(self): + return build_sided_relay_handshake(self._transit_key, self._side) + def _start_connector(self, ep, description, is_relay=False): relay_handshake = None if is_relay: assert self._transit_key - relay_handshake = build_relay_handshake(self._transit_key) + relay_handshake = self._build_relay_handshake() f = OutboundConnectionFactory(self, relay_handshake, description) d = ep.connect(f) # fires with protocol, or ConnectError From 80ae9236df63bf872d4f4a12d73aad1a84e6afad Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 22 Dec 2016 18:28:26 -0500 Subject: [PATCH 2/5] make RelayV1Hint objects hashable/comparable --- src/wormhole/test/test_transit.py | 11 +++++++++++ src/wormhole/transit.py | 11 +++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/wormhole/test/test_transit.py b/src/wormhole/test/test_transit.py index 65bdf18..8d49994 100644 --- a/src/wormhole/test/test_transit.py +++ b/src/wormhole/test/test_transit.py @@ -139,6 +139,17 @@ class Hints(unittest.TestCase): ep = c._endpoint_from_hint_obj("unknown:stuff:yowza:pivlor") self.assertEqual(ep, None) + def test_comparable(self): + h1 = transit.DirectTCPV1Hint("hostname", "port1") + h1b = transit.DirectTCPV1Hint("hostname", "port1") + h2 = transit.DirectTCPV1Hint("hostname", "port2") + r1 = transit.RelayV1Hint(tuple(sorted([h1, h2]))) + r2 = transit.RelayV1Hint(tuple(sorted([h2, h1]))) + r3 = transit.RelayV1Hint(tuple(sorted([h1b, h2]))) + self.assertEqual(r1, r2) + self.assertEqual(r2, r3) + self.assertEqual(len(set([r1, r2, r3])), 1) + class Basic(unittest.TestCase): @inlineCallbacks diff --git a/src/wormhole/transit.py b/src/wormhole/transit.py index d851070..eaf81a6 100644 --- a/src/wormhole/transit.py +++ b/src/wormhole/transit.py @@ -95,9 +95,10 @@ def build_sided_relay_handshake(key, side): # * the rest of the connection contains transit data DirectTCPV1Hint = namedtuple("DirectTCPV1Hint", ["hostname", "port"]) TorTCPV1Hint = namedtuple("TorTCPV1Hint", ["hostname", "port"]) -# RelayV1Hint contains a list of DirectTCPV1Hint and TorTCPV1Hint hints. For -# each one, make the TCP connection, send the relay handshake, then complete -# the rest of the V1 protocol. Only one hint per relay is useful. +# RelayV1Hint contains a tuple of DirectTCPV1Hint and TorTCPV1Hint hints (we +# use a tuple rather than a list so they'll be hashable into a set). For each +# one, make the TCP connection, send the relay handshake, then complete the +# rest of the V1 protocol. Only one hint per relay is useful. RelayV1Hint = namedtuple("RelayV1Hint", ["hints"]) def describe_hint_obj(hint): @@ -582,7 +583,9 @@ class Common: if transit_relay: if not isinstance(transit_relay, type(u"")): raise InternalError - relay = RelayV1Hint(hints=[parse_hint_argv(transit_relay)]) + # TODO: allow multiple hints for a single relay + relay_hint = parse_hint_argv(transit_relay) + relay = RelayV1Hint(hints=(relay_hint,)) self._transit_relays = [relay] else: self._transit_relays = [] From b8313b45954fd329ad4464dc89d2bf36b61eb974 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 22 Dec 2016 18:28:58 -0500 Subject: [PATCH 3/5] dedup relays, include our own relay when connecting * Previously, we only connected to the relay supplied by our partner, which meant that if our relay differed from theirs, we'd never connect * But we must de-duplicate the relays because when our relay *is* the same as theirs, we'd have two copies, which means two connections. Now that we deliver sided handshakes, we can tolerate that (previously, our two connections would be matched with each other), but it's still wasteful. This also fixes our handling of relay hints to accept multiple specific endpoints in each RelayHint. The idea here is that we might know multiple addresses for a single relay (maybe one IPv4, one IPv6, a Tor .onion, and an I2P address). Any one connection is good enough, and the connections we can try depend upon what local interfaces we discover. So a clever implementation could refrain from making some of those connections when it knows the sibling hints are just as good. However we might still have multiple relays entirely, for which it is *not* sufficient to connect to just one. The change is to create and process RelayV1Hint objects properly, and to set the connection loop to try every endpoint inside each RelayV1Hint. This is not "clever" (we could nominally make fewer connection attempts), but it's plenty good for now. refs #115 fix relay hints --- src/wormhole/test/test_transit.py | 2 +- src/wormhole/transit.py | 35 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/wormhole/test/test_transit.py b/src/wormhole/test/test_transit.py index 8d49994..5ecad4c 100644 --- a/src/wormhole/test/test_transit.py +++ b/src/wormhole/test/test_transit.py @@ -176,7 +176,7 @@ class Basic(unittest.TestCase): c.add_connection_hints([{"type": "relay-v1", "hints": [{"type": "unknown"}]}]) self.assertEqual(c._their_direct_hints, []) - self.assertEqual(c._their_relay_hints, []) + self.assertEqual(c._our_relay_hints, set()) def test_ignore_localhost_hint(self): # this actually starts the listener diff --git a/src/wormhole/transit.py b/src/wormhole/transit.py index eaf81a6..b9d887d 100644 --- a/src/wormhole/transit.py +++ b/src/wormhole/transit.py @@ -590,7 +590,7 @@ class Common: else: self._transit_relays = [] self._their_direct_hints = [] # hintobjs - self._their_relay_hints = [] + self._our_relay_hints = set(self._transit_relays) self._tor_manager = tor_manager self._transit_key = None self._no_listen = no_listen @@ -710,10 +710,14 @@ class Common: # with a set of equally-valid ways to connect to it. Treat # them as separate relays, instead of merging them all # together like this. + relay_hints = [] for rhs in h.get(u"hints", []): - rh = self._parse_tcp_v1_hint(rhs) - if rh: - self._their_relay_hints.append(rh) + h = self._parse_tcp_v1_hint(rhs) + if h: + relay_hints.append(h) + if relay_hints: + rh = RelayV1Hint(hints=tuple(sorted(relay_hints))) + self._our_relay_hints.add(rh) else: log.msg("unknown hint type: %r" % (h,)) @@ -804,21 +808,22 @@ class Common: contenders.append(d) relay_delay = self.RELAY_DELAY - # Start trying the relay a few seconds after we start to try the + # Start trying the relays a few seconds after we start to try the # direct hints. The idea is to prefer direct connections, but not be - # afraid of using the relay when we have direct hints that don't + # afraid of using a relay when we have direct hints that don't # resolve quickly. Many direct hints will be to unused local-network # IP addresses, which won't answer, and would take the full TCP # timeout (30s or more) to fail. - for hint_obj in self._their_relay_hints: - ep = self._endpoint_from_hint_obj(hint_obj) - if not ep: - continue - description = "->relay:%s" % describe_hint_obj(hint_obj) - d = task.deferLater(self._reactor, relay_delay, - self._start_connector, ep, description, - is_relay=True) - contenders.append(d) + for rh in self._our_relay_hints: + for hint_obj in rh.hints: + ep = self._endpoint_from_hint_obj(hint_obj) + if not ep: + continue + description = "->relay:%s" % describe_hint_obj(hint_obj) + d = task.deferLater(self._reactor, relay_delay, + self._start_connector, ep, description, + is_relay=True) + contenders.append(d) if not contenders: raise TransitError("No contenders for connection") From db968900d9c981df82dfce9eec2af4a4ce38b08e Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 23 Dec 2016 13:41:00 -0500 Subject: [PATCH 4/5] test_server: improve coverage --- src/wormhole/test/test_server.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/wormhole/test/test_server.py b/src/wormhole/test/test_server.py index 2d67ab2..cf1a5f7 100644 --- a/src/wormhole/test/test_server.py +++ b/src/wormhole/test/test_server.py @@ -1334,7 +1334,7 @@ class Transit(ServerBase, unittest.TestCase): a1.transport.loseConnection() @defer.inlineCallbacks - def test_impatience(self): + def test_impatience_old(self): ep = clientFromString(reactor, self.transit) a1 = yield connectProtocol(ep, Accumulator()) @@ -1347,3 +1347,20 @@ class Transit(ServerBase, unittest.TestCase): self.assertEqual(a1.data, exp) a1.transport.loseConnection() + + @defer.inlineCallbacks + def test_impatience_new(self): + ep = clientFromString(reactor, self.transit) + a1 = yield connectProtocol(ep, Accumulator()) + + token1 = b"\x00"*32 + side1 = b"\x01"*8 + # sending too many bytes is impatience. + a1.transport.write(b"please relay " + hexlify(token1) + + b" for side " + hexlify(side1) + b"\nNOWNOWNOW") + + exp = b"impatient\n" + yield a1.waitForBytes(len(exp)) + self.assertEqual(a1.data, exp) + + a1.transport.loseConnection() From fde98b7c7e532a149d45e73d81e55847d1b5d757 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 23 Dec 2016 21:31:19 -0500 Subject: [PATCH 5/5] more coverage --- src/wormhole/test/test_transit.py | 47 ++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/wormhole/test/test_transit.py b/src/wormhole/test/test_transit.py index 5ecad4c..7f63eac 100644 --- a/src/wormhole/test/test_transit.py +++ b/src/wormhole/test/test_transit.py @@ -1212,14 +1212,18 @@ DIRECT_HINT = {"type": "direct-tcp-v1", RELAY_HINT = {"type": "relay-v1", "hints": [{"type": "direct-tcp-v1", "hostname": "relay", "port": 1234}]} -UNUSABLE_HINT = {"type": "unknown"} +UNRECOGNIZED_HINT = {"type": "unknown"} +UNAVAILABLE_HINT = {"type": "direct-tcp-v1", # e.g. Tor without txtorcon + "hostname": "unavailable", "port": 1234} RELAY_HINT2 = {"type": "relay-v1", "hints": [{"type": "direct-tcp-v1", "hostname": "relay", "port": 1234}, - UNUSABLE_HINT]} + UNRECOGNIZED_HINT]} +UNAVAILABLE_RELAY_HINT = {"type": "relay-v1", + "hints": [UNAVAILABLE_HINT]} DIRECT_HINT_INTERNAL = transit.DirectTCPV1Hint("direct", 1234) RELAY_HINT_FIRST = transit.DirectTCPV1Hint("relay", 1234) -RELAY_HINT_INTERNAL = transit.RelayV1Hint([RELAY_HINT_FIRST]) +RELAY_HINT_INTERNAL = transit.RelayV1Hint((RELAY_HINT_FIRST,)) class Transit(unittest.TestCase): @inlineCallbacks @@ -1229,7 +1233,7 @@ class Transit(unittest.TestCase): s.set_transit_key(b"key") hints = yield s.get_connection_hints() # start the listener del hints - s.add_connection_hints([DIRECT_HINT, UNUSABLE_HINT]) + s.add_connection_hints([DIRECT_HINT, UNRECOGNIZED_HINT]) connectors = [] def _start_connector(ep, description, is_relay=False): @@ -1253,7 +1257,7 @@ class Transit(unittest.TestCase): elif hint == RELAY_HINT_FIRST: return "relay" else: - return None + return None # e.g. UNAVAILABLE_HINT @inlineCallbacks def test_wait_for_relay(self): @@ -1262,7 +1266,7 @@ class Transit(unittest.TestCase): s.set_transit_key(b"key") hints = yield s.get_connection_hints() # start the listener del hints - s.add_connection_hints([DIRECT_HINT, UNUSABLE_HINT, RELAY_HINT]) + s.add_connection_hints([DIRECT_HINT, UNRECOGNIZED_HINT, RELAY_HINT]) direct_connectors = [] relay_connectors = [] @@ -1301,7 +1305,9 @@ class Transit(unittest.TestCase): s.set_transit_key(b"key") hints = yield s.get_connection_hints() # start the listener del hints - s.add_connection_hints([UNUSABLE_HINT, RELAY_HINT2]) + # include hints that can't be turned into an endpoint at runtime + s.add_connection_hints([UNRECOGNIZED_HINT, UNAVAILABLE_HINT, + RELAY_HINT2, UNAVAILABLE_RELAY_HINT]) direct_connectors = [] relay_connectors = [] @@ -1333,6 +1339,33 @@ class Transit(unittest.TestCase): relay_connectors[0].callback("winner") self.assertEqual(results, ["winner"]) + @inlineCallbacks + def test_no_contenders(self): + clock = task.Clock() + s = transit.TransitSender("", reactor=clock, no_listen=True) + s.set_transit_key(b"key") + hints = yield s.get_connection_hints() # start the listener + del hints + s.add_connection_hints([]) # no hints at all + + direct_connectors = [] + relay_connectors = [] + s._endpoint_from_hint_obj = self._endpoint_from_hint_obj + def _start_connector(ep, description, is_relay=False): + d = defer.Deferred() + if ep == "direct": + direct_connectors.append(d) + elif ep == "relay": + relay_connectors.append(d) + else: + raise ValueError + return d + s._start_connector = _start_connector + + d = s.connect() + f = self.failureResultOf(d, transit.TransitError) + self.assertEqual(str(f.value), "No contenders for connection") + class RelayHandshake(unittest.TestCase): def old_build_relay_handshake(self, key): token = transit.HKDF(key, 32, CTXinfo=b"transit_relay_token")