We should use Unix domain sockets by default in Tor Browser. The patch for #14272 (moved) takes care of doing that for the control port (via the extensions.torlauncher.control_port_use_socket = true default preference). For the SOCKS port we need additional changes in tor-browser and builders/tor-browser-bundle at least.
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Child items
0
Show closed items
No child items are currently assigned. Use child items to break down this issue into smaller parts.
Linked items
0
Link issues together to show that they're related.
Learn more.
Kathy and I looked at this and soon realized that we cannot simply modify torrc-defaults (SocksPort) and the browser default preferences (network.proxy.socks) to use a socket path. This approach will not work because we need to insert a full path, and we do not know what the path is until the browser is starting up (it won't work to use a relative path, at least not on OSX where the TorBrowser-Data directory may be located in one of two different locations). Possibly solutions:
Modify Tor Launcher to Do The Right Thing and configure tor and the browser correctly. This is how we handled the ControlPort.
Modify the browser and tor (possibly with help from Tor Launcher) so we can use a placeholder within network.proxy.socks and SocksPort, e.g.,
network.proxy.socks="file:///--PROFILEDIR--/../../Tor/socks.socket"
Comments? Do you have a better idea? Kathy and I favor the first approach.
Trac: Status: new to needs_information Cc: brade, gk to brade, gk, arthuredelstein
Kathy and I looked at this and soon realized that we cannot simply modify torrc-defaults (SocksPort) and the browser default preferences (network.proxy.socks) to use a socket path. This approach will not work because we need to insert a full path, and we do not know what the path is until the browser is starting up (it won't work to use a relative path, at least not on OSX where the TorBrowser-Data directory may be located in one of two different locations). Possibly solutions:
Modify Tor Launcher to Do The Right Thing and configure tor and the browser correctly. This is how we handled the ControlPort.
Modify the browser and tor (possibly with help from Tor Launcher) so we can use a placeholder within network.proxy.socks and SocksPort, e.g.,
network.proxy.socks="file:///--PROFILEDIR--/../../Tor/socks.socket"
Comments? Do you have a better idea? Kathy and I favor the first approach.
To use a ControlPort via a domain socket, Tor Launcher will need to specify the ControlPort socket path as a command argument, correct? So the first approach sounds pretty natural and simple to me, where you also specify a path for the SocksPort and then set the network.proxy.socks pref in the browser.
What will be the name of the environment variable to define the path to the socket file? TOR_SOCKS_SOCKET?
Yes, we will probably use TOR_SOCKS_SOCKET. We have not created a patch yet though, so suggestions are welcome. We will also add a Boolean preference which will be true by default, maybe extensions.torlauncher.socks_port_use_socket
Kathy and I propose that we leave the browser defaults as-is. Inside tor-browser/browser/app/profile/000-tor-browser.js, we have:
pref("network.proxy.socks", "127.0.0.1");
pref("network.proxy.socks_port", 9150);
That means that a TCP SOCKS port will be used if Tor Launcher is not installed, which seems okay since we do not know what path to use for the socket.
When testing your patches in a clean, new hardened build I got the following issue:
Sep 28 11:34:45.000 [warn] tor_bug_occurred_(): Bug: src/common/address.c:1119: tor_addr_compare_masked: This line should not have been reached. (Future instances of this warning will be silenced.) (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: Line unexpectedly reached at tor_addr_compare_masked at src/common/address.c:1119. Stack trace: (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/libasan.so.2(+0x4bc88) [0x7fa4d26eac88] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(log_backtrace+0x46) [0x560e0b946df6] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(tor_bug_occurred_+0x13b) [0x560e0b99423b] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(tor_addr_compare_masked+0x455) [0x560e0b941105] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(connection_edge_compatible_with_circuit+0x2a3) [0x560e0b866873] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(+0x73a276) [0x560e0b7d5276] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(+0x749a3a) [0x560e0b7e4a3a] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(connection_ap_handshake_attach_circuit+0x722) [0x560e0b7e6e22] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(connection_ap_attach_pending+0x4ac) [0x560e0b85707c] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(circuit_build_needed_circs+0xe7) [0x560e0b7e38a7] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(+0x552488) [0x560e0b5ed488] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/libevent-2.0.so.5(event_base_loop+0x937) [0x7fa4d1f488d7] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(do_main_loop+0x398) [0x560e0b5ee898] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(tor_main+0x140d) [0x560e0b5f3a3d] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(main+0x1c) [0x560e0b5e102c] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) [0x7fa4d0195700] (on Tor 0.2.9.2-alpha 00ec701f8343f552)Sep 28 11:34:45.000 [warn] Bug: /home/thomas/Arbeit/Tor/debugging/20111/tor-browser/Browser/TorBrowser/Tor/tor(+0x547dcd) [0x560e0b5e2dcd] (on Tor 0.2.9.2-alpha 00ec701f8343f552)
Do you see the same on your machines? It seems we are hitting a tor bug or is there something wrong with your patches (they looked reasonable to me after the first code-review pass).
Do you see the same on your machines? It seems we are hitting a tor bug or is there something wrong with your patches (they looked reasonable to me after the first code-review pass).
I see the same messages, although I do not remember seeing them before (maybe Kathy and I failed to notice them or maybe we tested with a different version or tor). This could be a tor bug; the tor_addr_compare_masked() function does not seem to support Unix domain sockets (but I have not had time to debug it yet).
Yes, and tor's a885271c08d2337b35c203c0b27509d0aa32dbf6 made it just visible... Want to file a bug against tor with the tbb-needs keyword once you debugged the issue?
Yes, and tor's a885271c08d2337b35c203c0b27509d0aa32dbf6 made it just visible... Want to file a bug against tor with the tbb-needs keyword once you debugged the issue?
I created #20261 (moved). I also talked to Yawning and teor about this issue.
We had to include a copy of Tor Launcher's string unescape code in Torbutton, which is a little annoying but seems like the best alternative for now (eventually, we should make it so everything uses Arthur's tor-control-port.js module or its successor).
"extensions.torlauncher.socks_port_use_socket" is a confusing name to me, because we are using a TCP socket when it is false. Maybe call it "socks_port_use_ipc", as this would also cover the named pipe scenario if we later get it working in Windows? Similar for the env var TOR_SOCKS_SOCKET.
_strUnescape: It would be nicer if this function simply returned the unescaped string, and threw an exception if it failed. (Although maybe you want to keep it this way to be consistent with the other copy in Tor launcher.)
unescapeTorString: I would suggest defining this after _strUnescape is defined in utils.js, because it refers to it.
"extensions.torlauncher.socks_port_use_socket" is a confusing name to me, because we are using a TCP socket when it is false. Maybe call it "socks_port_use_ipc", as this would also cover the named pipe scenario if we later get it working in Windows? Similar for the env var TOR_SOCKS_SOCKET.
Good point. Kathy and I cleaned this up. We renamed the env vars and preferences as well as various functions and variables. The changes are a little messy, but they seem worthwhile.
_strUnescape: It would be nicer if this function simply returned the unescaped string, and threw an exception if it failed. (Although maybe you want to keep it this way to be consistent with the other copy in Tor launcher.)
Done. While testing, we also found some errors in the implementation (oops).
unescapeTorString: I would suggest defining this after _strUnescape is defined in utils.js, because it refers to it.
After testing a bit it seems to me that we can't shut down the browser by clicking on the "x" in the upper right corner. At least not while I am loading a page. This is what I get before I have to kill Tor Browser if I e.g. go to cnn.com:
Those woff resources seem to be interesting in particular as they are put on the catch all circuit by Torbutton, probably because the windows is gone. Not sure whether network requests are still ongoing after that but I somehow doubt it.
This only happens when using unix domain sockets for the socks port.
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:
+ // If extensions.torlauncher.socks_ipc_path is empty, a default+ // default path is used (<tor-data-directory>/socks.socket).
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.
Trac: Keywords: TorBrowserTeam201610R deleted, TorBrowserTeam201610 added Status: needs_review to needs_revision
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:
{{{
// If extensions.torlauncher.socks_ipc_path is empty, a default
// default path is used (/socks.socket).
}}}
"default" once should be enough :)
}
}}}
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.
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.
Seems you didn't see ticket:18047#comment:3. You'll be more surprised to know that webpages continue to stay in a cache, so that it's possible to reopen them in a old state.
(also you've just disclosed your timezone ;)
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.
Woah, that was news to me. I actually am seeing this behavior for the first time. I opened #20411 (moved).