[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:
-------------------------------------------------+-------------------------
Changes (by gk):

 * keywords:  tbb-torbutton, tbb-security, tbb-sandboxing,
     TorBrowserTeam201610R => tbb-torbutton, tbb-security, tbb-sandboxing,
     TorBrowserTeam201610
 * status:  needs_review => needs_revision


Comment:

 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). 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.

 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.

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