[tor-bugs] #14271 [Applications/Tor Browser]: Make Torbutton work with Unix Domain Socket option

Tor Bug Tracker & Wiki blackhole at torproject.org
Thu Sep 1 15:47:40 UTC 2016


#14271: Make Torbutton work with Unix Domain Socket option
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  brade
     Type:  enhancement                          |         Status:
                                                 |  needs_information
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton, tbb-security,         |  Actual Points:
  TorBrowserTeam201609                           |
Parent ID:  #14270                               |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  SponsorU
-------------------------------------------------+-------------------------
Changes (by mcs):

 * status:  needs_revision => needs_information


Comment:

 Replying to [comment:12 gk]:
 > Here are some comments:
 >
 > 1)
 > {{{
 > +// given sockFile or host and port.
 > }}}
 > s/sockFile/socketFile/

 Good catch.


 > 2)
 > {{{
 > let tlps = Cc["@torproject.org/torlauncher-protocol-service;1"]
 >              .getService(Ci.nsISupports).wrappedJSObject;
 > }}}
 > We should do that once and not twice in `torbutton_init()`.

 Agreed.


 > 3) Checking whether we should call `torbutton_local_tor_check()` should
 check for
 > `m_tb_control_socket_file` as well (not only for `m_tb_control_port`) I
 guess?

 We did add that check, although if you look at the patch with git's
 default of three lines of context it is not so obvious.


 > 4) I am not so sure about
 > {{{
 > +            } catch(e) {
 > +              m_tb_control_port = 9151;
 > +            }
 > }}}
 > What was your rationale for adding that now (given you omitted it
 earlier)? For one, the logs might be misleading showing a probably wrong
 port (I mean the setup is seriously troubling if we need to assign `9151`
 in the catch clause at all) in an error with respect to the control
 connection. On the other hand, we might want to show that something is
 wrong with the help of `torbutton_do_tor_check()` which would update the
 toolbar button in this case (if we get that far at all with a broken setup
 like the one in question).

 Kathy and I added the 9151 default to be consistent with how
 m_tb_control_host is handled (it was already defaulting to 127.0.0.1).
 Thinking about this some more and looking at the existing Torbutton code,
 it seems like there is some effort to disable features (e.g., New
 Identity, the local Tor check) when the port is missing. So maybe we
 should put things back how there were and make sure we consistently check
 for port, password, etc. before trying to do things in Torbutton that
 require authenticated control port access?

 The circuit display code also includes code that defaults the port to
 9151, so if we decide to continue with the concept that a lack of port can
 be used to disable code, we should remove the `|| 9151` from this line in
 tor-circuit-display.js:
 `myController = controller(socketFile, host, port || 9151, password,`
 We can also add a check to skip the call to createTorCircuitDisplay() if
 port, password, etc. are missing (the existing code will log an error if
 initialization fails).

 What do you think?

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


More information about the tor-bugs mailing list