Control: tag -1 + patch

Hi intrigeri, Micah,

On Sun, Aug 30, 2015 at 13:30:49 +0200, intrigeri wrote:
> Michael Gold wrote (29 Aug 2015 16:55:28 GMT) :
> > It is inappropriate to assume Tor is running on this port, as any local
> > user could be running a service there (Debian bug #797335), possibly to
> > interfere with torbrowser-launcher.
> 
> Note that torbrowser-launcher Depends: tor, so this is correct if, and
> only if, the system administrator has disabled the tor service, or
> configured it to not listen on the default SOCKS port.

This would also be true if another user had caused Tor to crash or shut
down.  Tor is full of assertions (enabled in release builds) that could
cause it to intentionally shut down, leaving its SOCKS port available to
others.

> > torbrowser-launcher should allow the user to select an alternate TCP or
> > Unix-domain SOCKS address, and shouldn't connect to an unprivileged one
> > without confirmation.
> 
> Given 1. the design goals of torbrowser-launcher (that is: working
> out-of-the-box for non-technical users, AIUI); 2. the current state of
> things as described above; and 3. the fact that in practice this is
> a problem only if the system administrator tweaked torrc in a way that
> ignores #2, I beg to disagree that this would be a good solution to
> the (real) problem we have here.

Here's a patch to add it to the --settings screen.  I'm not sure whether
it's meant to be accessible to non-technical users (the mirror selection
box suggests it's assuming some technical ability).  Maybe it could be
moved to a command line option if the GUI is inappropriate.

> Now, *if* the quick fix I'm suggesting above works (checking who
> opened the SOCKs port before connecting to it), then it could be an
> acceptable temporary fix for users who get torbrowser-launcher from
> their distro.

I'm not aware of any good way to do this.  getpeereid only works on Unix
sockets (according to the man page; plus, if the socket were in a system
directory it would be unnecessary).  IDENT (rfc1413) is the standard way
way to check for a TCP connection, but it's rarely available.

Parsing /proc/self/net/tcp would work on Linux but it's non-portable and
ugly.  It would need to be done after connecting to avoid a race.

-- Michael
diff --git a/torbrowser_launcher/common.py b/torbrowser_launcher/common.py
index e5844b974eb1..9c80def37b81 100644
--- a/torbrowser_launcher/common.py
+++ b/torbrowser_launcher/common.py
@@ -194,6 +194,7 @@ class Common:
             'installed_version': False,
             'latest_version': '0',
             'update_over_tor': True,
+            'tor_socks_address': 'tcp:127.0.0.1:9050',
             'check_for_updates': False,
             'modem_sound': False,
             'last_update_check_timestamp': 0,
diff --git a/torbrowser_launcher/launcher.py b/torbrowser_launcher/launcher.py
index 698949d929ac..e88a9f0ca87d 100644
--- a/torbrowser_launcher/launcher.py
+++ b/torbrowser_launcher/launcher.py
@@ -444,10 +444,10 @@ class Launcher:
         self.refresh_gtk()
 
         if self.common.settings['update_over_tor']:
-            from twisted.internet.endpoints import TCP4ClientEndpoint
+            from twisted.internet.endpoints import clientFromString
             from txsocksx.http import SOCKS5Agent
 
-            torEndpoint = TCP4ClientEndpoint(reactor, '127.0.0.1', 9050)
+            torEndpoint = clientFromString(reactor, self.common.settings['tor_socks_address'])
 
             # default mirror gets certificate pinning, only for requests that use the mirror
             if self.common.settings['mirror'] == self.common.default_mirror and '{0}' in url:
diff --git a/torbrowser_launcher/settings.py b/torbrowser_launcher/settings.py
index 5bd505ea8041..3aacc33962c1 100644
--- a/torbrowser_launcher/settings.py
+++ b/torbrowser_launcher/settings.py
@@ -85,6 +85,21 @@ class Settings:
 
         self.tor_update_checkbox.show()
 
+        # Tor SOCKS address
+        self.tor_addr_box = gtk.HBox(False, 10)
+        self.settings_box.pack_start(self.tor_addr_box, True, True, 0)
+        self.tor_addr_box.show()
+
+        self.tor_addr_label = gtk.Label(_('Tor server'))
+        self.tor_addr_label.set_line_wrap(True)
+        self.tor_addr_box.pack_start(self.tor_addr_label, True, True, 0)
+        self.tor_addr_label.show()
+
+        self.tor_addr = gtk.Entry()
+        self.tor_addr.set_text(self.common.settings['tor_socks_address'])
+        self.tor_addr_box.pack_start(self.tor_addr, True, True, 0)
+        self.tor_addr.show()
+
         # check for updates
         self.update_checkbox = gtk.CheckButton(_("Check for updates next launch"))
         self.settings_box.pack_start(self.update_checkbox, True, True, 0)
@@ -213,6 +228,7 @@ class Settings:
         self.common.settings['update_over_tor'] = self.tor_update_checkbox.get_active()
         self.common.settings['check_for_updates'] = self.update_checkbox.get_active()
         self.common.settings['modem_sound'] = self.modem_checkbox.get_active()
+        self.common.settings['tor_socks_address'] = self.tor_addr.get_text()
 
         # figure out the selected mirror
         self.common.settings['mirror'] = self.common.mirrors[self.mirrors.get_active()]

Attachment: signature.asc
Description: PGP signature

Reply via email to