Bug 40933: Move tor-launcher to tor-browser.git

Reviewer's guide for this MR

What does tor-launcher do?

1. Do things at the Firefox start (launch tor/connect to a provided tor instance)
2. Monitor the Tor status, to then provide information to the connection page (in particular, handle the events)
3. Provide an interface to allow configuration from the preferences page
4. Cleanup things when Firefox exits (+ as a bonus, clean the old meek profiles)

What was the previous architecture?

tor-launcher does that in three files: tl-protocol.js, tl-process.js, tl-util.jsm, plus a shared logger that I replaced with the console.

tl-util.jsm contains the TorLauncherUtil module, the other two are components, which can be accessed through XPCOM wizardry (Cc["@torproject.org/whateveritis"]).

tl-process contains both the logic to start a tor process, plus part of the logic to monitor it.

tl-protocol contains the other part of monitoring, plus a part that checks how tor should be reached (which address, or IPC path, etc etc), and should be used in general to send commands to Tor (even though the controller implementation is in torbutton).

In addition to that, there are localization files (moved with torbutton!95 (merged) and !369 (merged)) and the old connection wizard. The localization files should be moved to Fluent, and it's time to retire completely the old connection wizard (i.e., this MR doesn't contain it).

Many parts are implemented in torbutton, and in general, torbutton references tor-launcher. That's why I have also torbutton!96 (merged).

For tor-browser, we tried not to use them directly, but only through the XPCOM facilities (actually, we use XPCOM only to get a magic object, and then get the raw JS object with .wrappedJSObject).

What is the new architecture?

I have tried to divide the files more, accordingly to my understanding of the functions of tor-launcher.

TorProtocolService.jsm

This was the most difficult file to deal with.

It had already some wrappers for tl-protocol.js, so I've tried to move the actual implementation in these wrappers.

However, eventually I split the tl-protocol.js in 2/3 parts:

1. The interface to help with the configuration (it contained and still contains a map to prevent sending the same configuration to Tor twice)
2. The event monitor: I moved it to its own file
3. The part to query the address/path to use for SOCKS and for the control port: I've kept it here, because I think it partially overlaps with torbutton, so we should also move torbutton before moving this part.

TorProcess.jsm

Previously, we had TorProcessService.jsm, which only exposed the status of the tor process to TorConnect.jsm (mainly to handle a case in which the process was started before the TorConnect logic was initialized, not sure it was actually possible).

This only feature isn't part of the new TorProcess, because I feel that we're not interested in knowing whether the process has been started, but to know whether we are monitoring any Tor process already.

So, I took the parts from tl-process.js that actually dealt with starting a new process with the XPCOM facilities (in particular, nsIProcess).

Everything else from tl-process.js was moved to the event monitor.

Also, we don't have a singleton anymore, but a normal class, which is instanced only when we need to start tor (which is most of the cases, actually, but you know). It reports events like Tor has exited, or Tor has been restarted through callbacks, instead of observers (these callbacks are implemented in the monitor, which actually notify observers again 😄 ).

Also, TorProcess still contains the code to make the "Do you want to restart Tor?" prompt appear. We might want to move it somewhere else, because it's a mix of frontend and backend 🙁 .

TorMonitorService.jsm

This is a new file.

I moved the event parsing here, and it is also responsible of telling whether the bootstrap has been completed, or what was last error, etc...

It starts the Tor process, if needed, and it's notified of its events.

tor ownership

This is very important, so it deserves its own section.

We don't kill tor, but we set it to self-kill when the "main" control port connection has been closed. According to the comments I found, this has been done to prevent Firefox from ramping the CPU up because its connections are closed unexpectedly.

I thought that it made more sense to make the connection that this module keeps the one that makes tor close. This is different from the previous implementation, in which the connection that is used to send the configuration was also the owner of the daemon.

The rationale is that this might simplify the management of the connection on TorProtocolService.

However, we might try to switch to a single connection to the control port, too, but will see in the next phase.

TorParsers.jsm

tl-protocol.js contained some text-processing functions. I preferred moving them to their own file, to reuse them wherever needed.

IIRC, torbutton also has many additional parsers, that we should move here when the time comes.

Also, this was helpful to create TorMonitorService.jsm.

TorLauncherUtil.jsm

This is the previous tl-util.jsm, without the stuff we don't need anymore.

I have to rework a lot the getTorFile functions... which I transformed from a single function to an entire class.

This could be further improved (e.g., don't use strings, but create an object to use as an enum, etc...).

But the truth is that I'd like to get rid of this file. Many functions can be moved together with the part of TorProtocolService.jsm that detects how Tor should be reached.

So, I kept it, but I think we should at least rename it after merging torbutton.

TorStartupService.jsm

We still needed a way to start/initialize things at the right moment.

So, instead of registering each module as a component, I have registered only this components, which calls the needed function on the other modules.

Phase 2/3 + Arti

This MR is phase 1, but I expect we'll have a phase 2 for tor-launcher. Or phase 2 is torbutton, and phase 3 is review of the whole.

My objective is to make TorProcotolService.jsm a configuration utility, that will become a TorSettings backend, rather than what it is now.

Anything else should be moved to another module.

I don't know how Arti works already, but my idea is that we'll have a check that tells that we're using Arti, and that we don't need to initialize the modules of this MR.

Also, I'd like to make TorService more abstract and focused on the functionalities (e.g., tell its backends that the user wants to use bridges, not that Tor should be configured with an array of bridge strings, if it makes sense).

But again, I think that we should port torbutton to tor-browser, first.

---

Closes #40853 (closed), #40993.

Should be merged with torbutton!96 (merged).

Previous description

I'm trying to follow the strategy I described in #40933 (closed).

Setting as a draft because I'm still working on it, but I could use an early review.

TL;DR:

• tl-process.js ➡️ TorProcessService.jsm (mostly done)
• tl-protocol.js ➡️ TorProtocolService.jsm (work in progress)
• tl-util.jsm copy&paste&rework (not done yet)
• l10n ➡️ torbutton, until we approve tor-browser-build!503 (merged)?

Component analysis (WIP)

tl-process.js/TorProcessService

• Public interface:
  • only the possibility to get the status of the tor process.
    • I renamed the getter to status only, because I think that adding tor to every method/getter is redundant 🙂
    • This can be tested by querying the value in the browser console
    • To make things more spicy, we can kill tor
  • tl-process.js had some public interfaces that were accessed through TorProtocolService, instead. So, I moved them tl-protocol.js in the tor-launcher branch
    • isBootstrapDone, clearBootstrapError, torBootstrapErrorOccurred, _torBootstrapDebugSetError
    • I'm not sure on how to test them. The preference to debug about:torconnect could be handy.
  • other "public" communication happens through observers (see later, but I have tried to keep all the topics in TorProcessTopics)
• Custom logger, instead of using the old logger
  • I think using a standard API is better than maintaining our log code
  • It has its own nice label in the Firefox console 😄
  • No way to customize the level, yet, but we can add a preference
  • Way to test: open the console and see that we can still see logs (increase the maxLogLevel to all, if needed)
• Startup observer (profile-after-change)
  • This needs a component, so I have used a mix between the previous code, and the SecurityLevel code.
    • This is the reason for which TorProcessService needs to be a class instanced by Firefox now, rather than a plain old object.
    • I think we could use a single startup/quit observer in a new module, in a new revision
  • Test: without this, tor won't be started.
• Quit observer: kills the the process in a smart way (the code hasn't been touched, mostly)
  • Test way: I haven't thought to it, yet.
• Process observer: it is notified if tor exits
  • Way to test: killall tor and see Tor Browser reacting
• tor starter
  • Tests:
    • check on the system monitor that the tor process is started correctly, and with the expected arguments
      • arguments are also dumped to the console
    • It includes extensions.torlauncher.launch_delay. We can add a very high value, and in any case check that the process is started as expected even when this preference is set
    • Start TBB when another tor/another TBB is already running to trigger a failure
• start a tor controller: calls TorProtocolService.torStartEventMonitor(), TorProtocolService.torRetrieveBootstrapStatus() and notifies the observers that the process is ready (or if it timed out)
  • This was based on a nsITimer, and was handled in the observer, I switched to a setTimeout
  • Tests:
    • we should actually think on how to test the other methods
    • add a custom observer, just to check that it is notified
    • hijack tor to some sleep process, just to see that the ProcessDidNotStart observer is notified, instead
    • try with an already-running tor, and see that it works like the previous implementation (critical for Tails)
• fix torrc: I have copied and pasted the old functions, but haven't analyzed them too much
  • It was the only method using _valueIsUnixDomainSocket and _valueContainsPort, so I changed them to closures inside fixupTorrc
  • Tests: analyze the code, reverse engineer what we expect it to do, modify the torrc and see that is gets fixed as we expect
• remove Meek and Moat helper profiles
  • can we remove this, actually?
  • If we can't, we need to test this on macOS: create some directories, and check that they're deleted

Removed code 😈️

• messing with (default) bridges
  • From what I could gather, this code isn't actually used
• helpers to open the old launcher/settings/locale picker
  • The locale picker maybe could be still useful... But until then, let's keep it outta here!
• helper to remove the program
  • I think it isn't used anymore
• filesystem helpers: use IOUtils instead
  • they are used to fix torrc, so we will test IOUtils while testing it
• helper to get Firefox's PID: I yoloed and used Services.appinfo.processID only