From ed420e0001e1afec2d69f688ce7f56edab4d712c Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 14 Sep 2017 15:48:35 -0700 Subject: [PATCH 1/2] tor_manager: pass endpoints to txtorcon.connect(), not descriptors This was breaking any uses of --tor-control-port=: the client would always fall back to using the default SOCKS port. --- src/wormhole/test/test_tor_manager.py | 30 +++++++++++++++++---------- src/wormhole/tor_manager.py | 4 ++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/wormhole/test/test_tor_manager.py b/src/wormhole/test/test_tor_manager.py index f14e97c..1f04354 100644 --- a/src/wormhole/test/test_tor_manager.py +++ b/src/wormhole/test/test_tor_manager.py @@ -51,29 +51,37 @@ class Tor(unittest.TestCase): reactor = object() my_tor = X() # object() didn't like providedBy() tcp = "port" + ep = object() connect_d = defer.Deferred() stderr = io.StringIO() with mock.patch("wormhole.tor_manager.txtorcon.connect", side_effect=connect_d) as connect: - d = get_tor(reactor, tor_control_port=tcp, stderr=stderr) - self.assertNoResult(d) - self.assertEqual(connect.mock_calls, [mock.call(reactor, tcp)]) - connect_d.callback(my_tor) - tor = self.successResultOf(d) - self.assertIs(tor, my_tor) - self.assert_(ITorManager.providedBy(tor)) - self.assertEqual(stderr.getvalue(), " using Tor via control port\n") + with mock.patch("wormhole.tor_manager.clientFromString", + side_effect=[ep]) as sfs: + d = get_tor(reactor, tor_control_port=tcp, stderr=stderr) + self.assertEqual(sfs.mock_calls, [mock.call(reactor, tcp)]) + self.assertNoResult(d) + self.assertEqual(connect.mock_calls, [mock.call(reactor, ep)]) + connect_d.callback(my_tor) + tor = self.successResultOf(d) + self.assertIs(tor, my_tor) + self.assert_(ITorManager.providedBy(tor)) + self.assertEqual(stderr.getvalue(), " using Tor via control port\n") def test_connect_fails(self): reactor = object() tcp = "port" + ep = object() connect_d = defer.Deferred() stderr = io.StringIO() with mock.patch("wormhole.tor_manager.txtorcon.connect", side_effect=connect_d) as connect: - d = get_tor(reactor, tor_control_port=tcp, stderr=stderr) - self.assertNoResult(d) - self.assertEqual(connect.mock_calls, [mock.call(reactor, tcp)]) + with mock.patch("wormhole.tor_manager.clientFromString", + side_effect=[ep]) as sfs: + d = get_tor(reactor, tor_control_port=tcp, stderr=stderr) + self.assertEqual(sfs.mock_calls, [mock.call(reactor, tcp)]) + self.assertNoResult(d) + self.assertEqual(connect.mock_calls, [mock.call(reactor, ep)]) connect_d.errback(ConnectError()) tor = self.successResultOf(d) diff --git a/src/wormhole/tor_manager.py b/src/wormhole/tor_manager.py index 3aa2a11..f92a627 100644 --- a/src/wormhole/tor_manager.py +++ b/src/wormhole/tor_manager.py @@ -3,6 +3,7 @@ import sys from attr import attrs, attrib from zope.interface.declarations import directlyProvides from twisted.internet.defer import inlineCallbacks, returnValue +from twisted.internet.endpoints import clientFromString try: import txtorcon except ImportError: @@ -86,6 +87,9 @@ def get_tor(reactor, launch_tor=False, tor_control_port=None, # If tor_control_port is None (the default), txtorcon # will look through a list of usual places. If it is set, # it will look only in the place we tell it to. + if tor_control_port is not None: + tor_control_port = clientFromString(reactor, + tor_control_port) tor = yield txtorcon.connect(reactor, tor_control_port) print(" using Tor via control port", file=stderr) except Exception: From 0aeae9ce10902fee83c226405a6f70a3d5be7a15 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 14 Sep 2017 16:46:26 -0700 Subject: [PATCH 2/2] tor_manager: expose errors when --tor-control-port= is provided If you pass --tor-control-port= and we can't use it, throw an error that will kill the whole process, instead of falling back to the default SOCKS port. If you omit --tor-control-port=, then if all default control port connections fail, we'll fall back to the default SOCKS port. Also, test each combination separately, and improve the status messages. --- src/wormhole/test/test_tor_manager.py | 56 ++++++++++++++++++++++----- src/wormhole/tor_manager.py | 20 +++++----- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/wormhole/test/test_tor_manager.py b/src/wormhole/test/test_tor_manager.py index 1f04354..b758e71 100644 --- a/src/wormhole/test/test_tor_manager.py +++ b/src/wormhole/test/test_tor_manager.py @@ -50,7 +50,48 @@ class Tor(unittest.TestCase): def test_connect(self): reactor = object() my_tor = X() # object() didn't like providedBy() - tcp = "port" + connect_d = defer.Deferred() + stderr = io.StringIO() + with mock.patch("wormhole.tor_manager.txtorcon.connect", + side_effect=connect_d) as connect: + with mock.patch("wormhole.tor_manager.clientFromString", + side_effect=["foo"]) as sfs: + d = get_tor(reactor, stderr=stderr) + self.assertEqual(sfs.mock_calls, []) + self.assertNoResult(d) + self.assertEqual(connect.mock_calls, [mock.call(reactor)]) + connect_d.callback(my_tor) + tor = self.successResultOf(d) + self.assertIs(tor, my_tor) + self.assert_(ITorManager.providedBy(tor)) + self.assertEqual(stderr.getvalue(), + " using Tor via default control port\n") + + def test_connect_fails(self): + reactor = object() + connect_d = defer.Deferred() + stderr = io.StringIO() + with mock.patch("wormhole.tor_manager.txtorcon.connect", + side_effect=connect_d) as connect: + with mock.patch("wormhole.tor_manager.clientFromString", + side_effect=["foo"]) as sfs: + d = get_tor(reactor, stderr=stderr) + self.assertEqual(sfs.mock_calls, []) + self.assertNoResult(d) + self.assertEqual(connect.mock_calls, [mock.call(reactor)]) + + connect_d.errback(ConnectError()) + tor = self.successResultOf(d) + self.assertIsInstance(tor, SocksOnlyTor) + self.assert_(ITorManager.providedBy(tor)) + self.assertEqual(tor._reactor, reactor) + self.assertEqual(stderr.getvalue(), + " unable to find default Tor control port, using SOCKS\n") + + def test_connect_custom_control_port(self): + reactor = object() + my_tor = X() # object() didn't like providedBy() + tcp = "PORT" ep = object() connect_d = defer.Deferred() stderr = io.StringIO() @@ -66,9 +107,10 @@ class Tor(unittest.TestCase): tor = self.successResultOf(d) self.assertIs(tor, my_tor) self.assert_(ITorManager.providedBy(tor)) - self.assertEqual(stderr.getvalue(), " using Tor via control port\n") + self.assertEqual(stderr.getvalue(), + " using Tor via control port at PORT\n") - def test_connect_fails(self): + def test_connect_custom_control_port_fails(self): reactor = object() tcp = "port" ep = object() @@ -84,12 +126,8 @@ class Tor(unittest.TestCase): self.assertEqual(connect.mock_calls, [mock.call(reactor, ep)]) connect_d.errback(ConnectError()) - tor = self.successResultOf(d) - self.assertIsInstance(tor, SocksOnlyTor) - self.assert_(ITorManager.providedBy(tor)) - self.assertEqual(tor._reactor, reactor) - self.assertEqual(stderr.getvalue(), - " unable to find Tor control port, using SOCKS\n") + self.failureResultOf(d, ConnectError) + self.assertEqual(stderr.getvalue(), "") class SocksOnly(unittest.TestCase): def test_tor(self): diff --git a/src/wormhole/tor_manager.py b/src/wormhole/tor_manager.py index f92a627..0f8a449 100644 --- a/src/wormhole/tor_manager.py +++ b/src/wormhole/tor_manager.py @@ -81,22 +81,24 @@ def get_tor(reactor, launch_tor=False, tor_control_port=None, #data_directory=, #tor_binary=, ) + elif tor_control_port: + with timing.add("find tor"): + control_ep = clientFromString(reactor, tor_control_port) + tor = yield txtorcon.connect(reactor, control_ep) # might raise + print(" using Tor via control port at %s" % tor_control_port, + file=stderr) else: + # Let txtorcon look through a list of usual places. If that fails, + # we'll arrange to attempt the default SOCKS port with timing.add("find tor"): try: - # If tor_control_port is None (the default), txtorcon - # will look through a list of usual places. If it is set, - # it will look only in the place we tell it to. - if tor_control_port is not None: - tor_control_port = clientFromString(reactor, - tor_control_port) - tor = yield txtorcon.connect(reactor, tor_control_port) - print(" using Tor via control port", file=stderr) + tor = yield txtorcon.connect(reactor) + print(" using Tor via default control port", file=stderr) except Exception: # TODO: make this more specific. I think connect() is # likely to throw a reactor.connectTCP -type error, like # ConnectionFailed or ConnectionRefused or something - print(" unable to find Tor control port, using SOCKS", + print(" unable to find default Tor control port, using SOCKS", file=stderr) tor = SocksOnlyTor(reactor) directlyProvides(tor, _interfaces.ITorManager)