[tbb-bugs] #20111 [Applications/Tor Browser]: use Unix domain sockets for SOCKS port by default

[Tor Bug Tracker & Wiki]

#20111: use Unix domain sockets for SOCKS port by default
-------------------------------------------------+-------------------------
 Reporter:  mcs                                  |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton, tbb-security, tbb-    |  Actual Points:
  sandboxing, TorBrowserTeam201610               |
Parent ID:  #14270                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by mcs):

 Replying to [comment:25 gk]:
 > The Torbutton patch looks good to me (although I am not happy either
 with having a copy of `_strUnescape()` there as well; I thought a bit
 about using the one from TorLauncher there, too, but that is probably a
 bad idea as we want to support the no-TorLauncher case as well).

 Yes, that is why we did not rely on making a call into Tor Launcher.

 > Just two minor nits for the TorLauncher one:
 >
 > 1)
 > {{{
 > +    // If extensions.torlauncher.socks_ipc_path is empty, a default
 > +    // default path is used (<tor-data-directory>/socks.socket).
 > }}}
 > "default" once should be enough :)
 >
 > 2)
 > {{{
 > +    if (useIPC)
 > +      TorLauncherLogger.log(3, "ipcFile: " +
 this.mSOCKSPortInfo.ipcFile.path);
 > +    else
 > +    {
 > +      TorLauncherLogger.log(3, "SOCKS host: " +
 this.mSOCKSPortInfo.host);
 > +      TorLauncherLogger.log(3, "SOCKS port: " +
 this.mSOCKSPortInfo.port);
 > +    }
 > }}}
 > Please don't mix code parts with and without curly braces in one if-else
 construct. This is too error-prone for my taste.

 Thanks for the review. We will fix both of these issue and post a new
 patch.

 > That said I tested the patches again with the proposed fix for bug
 1311044 and there is not shutdown hang anymore. However, seeing all the
 output after `tor` is supposedly shut down still makes me nervous. I think
 we should have the behavior we currently have in this regard (at least it
 seems to me we have it that way at the moment): first no traffic anymore,
 then `tor` gets shut down and then the browser goes away.

 This is not new behavior. We were able to reproduce it using TB 6.5a3 when
 running with TCP control and SOCKS ports. I will attach a log.

 It surprises me that Firefox does not cancel all network activity as a
 browser tab is closed, but maybe that hurts performance. I don't think any
 harm is done in this case because, even though the catch all circuit would
 be used, there is no tor to talk to any more. But I wonder if this same
 situation can happen when closing a window or tab without exiting the
 browser. Probably we should open a new ticket for this issue.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20111#comment:26>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online