browser/components/torpreferences/content/connectionPane.js

@@ -114,10 +114,9 @@ async function setTorSettings(changes) {
     // This will trigger TorSettings.#cleanupSettings()
     TorSettings.saveToPrefs();
     try {
-      // May throw.
       await TorSettings.applySettings();
     } catch (e) {
-      console.error("Failed to save Tor settings", e);
+      console.error("Failed to apply Tor settings", e);
     }
   } finally {
     TorSettings.thawNotifications();

browser/components/torpreferences/content/connectionSettingsDialog.js

@@ -362,6 +362,8 @@ const gConnectionSettingsDialog = {
     }
 
     TorSettings.saveToPrefs();
+    // FIXME: What if this fails? Should we prevent the dialog to close and show
+    // an error?
     TorSettings.applySettings();
   },
 };

toolkit/components/tor-launcher/TorControlPort.sys.mjs

@@ -838,10 +838,12 @@ export class TorController {
   /**
    * Send multiple configuration values to tor.
    *
-   * @param {object} values The values to set
+   * @param {Array} values The values to set. It should be an array of
+   * [key, value] pairs to pass to SETCONF. Keys can be repeated, and array
+   * values will be automatically unrolled.
    */
   async setConf(values) {
-    const args = Object.entries(values)
+    const args = values
       .flatMap(([key, value]) => {
         if (value === undefined || value === null) {
           return [key];
@@ -871,7 +873,7 @@ export class TorController {
    * @param {boolean} enabled Tell whether the network should be enabled
    */
   async setNetworkEnabled(enabled) {
-    return this.setConf({ DisableNetwork: !enabled });
+    return this.setConf([["DisableNetwork", !enabled]]);
   }
 
   /**

toolkit/components/tor-launcher/TorProvider.sys.mjs

@@ -15,6 +15,8 @@ ChromeUtils.defineESModuleGetters(lazy, {
   TorController: "resource://gre/modules/TorControlPort.sys.mjs",
   TorProcess: "resource://gre/modules/TorProcess.sys.mjs",
   TorProcessAndroid: "resource://gre/modules/TorProcessAndroid.sys.mjs",
+  TorProxyType: "resource://gre/modules/TorSettings.sys.mjs",
+  TorSettings: "resource://gre/modules/TorSettings.sys.mjs",
 });
 
 const logger = new ConsoleAPI({
@@ -73,6 +75,20 @@ const Preferences = Object.freeze({
   PromptAtStartup: "extensions.torlauncher.prompt_at_startup",
 });
 
+/* Config Keys used to configure tor daemon */
+const TorConfigKeys = Object.freeze({
+  useBridges: "UseBridges",
+  bridgeList: "Bridge",
+  socks4Proxy: "Socks4Proxy",
+  socks5Proxy: "Socks5Proxy",
+  socks5ProxyUsername: "Socks5ProxyUsername",
+  socks5ProxyPassword: "Socks5ProxyPassword",
+  httpsProxy: "HTTPSProxy",
+  httpsProxyAuthenticator: "HTTPSProxyAuthenticator",
+  reachableAddresses: "ReachableAddresses",
+  clientTransportPlugin: "ClientTransportPlugin",
+});
+
 /**
  * This is a Tor provider for the C Tor daemon.
  *
@@ -166,15 +182,6 @@ export class TorProvider {
    */
   #currentBridge = null;
 
-  /**
-   * Maintain a map of tor settings set by Tor Browser so that we don't
-   * repeatedly set the same key/values over and over.
-   * This map contains string keys to primitives or array values.
-   *
-   * @type {Map<string, any>}
-   */
-  #settingsCache = new Map();
-
   /**
    * Starts a new tor process and connect to its control port, or connect to the
    * control port of an existing tor daemon.
@@ -219,13 +226,22 @@ export class TorProvider {
       throw e;
     }
 
+    try {
+      await lazy.TorSettings.initializedPromise;
+      await this.writeSettings(lazy.TorSettings.getSettings());
+    } catch (e) {
+      logger.warn(
+        "Failed to initialize TorSettings or to write our settings, so uninitializing.",
+        e
+      );
+      this.uninit();
+      throw e;
+    }
+
     TorLauncherUtil.setProxyConfiguration(this.#socksSettings);
 
     logger.info("The Tor provider is ready.");
 
-    logger.debug(`Notifying ${TorProviderTopics.ProcessIsReady}`);
-    Services.obs.notifyObservers(null, TorProviderTopics.ProcessIsReady);
-
     // If we are using an external Tor daemon, we might need to fetch circuits
     // already, in case streams use them. Do not await because we do not want to
     // block the intialization on this (it should not fail anyway...).
@@ -252,42 +268,74 @@ export class TorProvider {
 
   // Provider API
 
-  async writeSettings(settingsObj) {
-    // TODO: Move the translation from settings object to settings understood by
-    // tor here.
-    const entries =
-      settingsObj instanceof Map
-        ? Array.from(settingsObj.entries())
-        : Object.entries(settingsObj);
-    // only write settings that have changed
-    const newSettings = entries.filter(([setting, value]) => {
-      if (!this.#settingsCache.has(setting)) {
-        // no cached setting, so write
-        return true;
-      }
+  /**
+   * Send settings to the tor daemon.
+   *
+   * @param {object} settings A settings object, as returned by
+   * TorSettings.getSettings(). This allow to try settings without passing
+   * through TorSettings.
+   */
+  async writeSettings(settings) {
+    logger.debug("TorProvider.writeSettings", settings);
+    const torSettings = new Map();
+
+    // Bridges
+    const haveBridges =
+      settings.bridges?.enabled && !!settings.bridges.bridge_strings.length;
+    torSettings.set(TorConfigKeys.useBridges, haveBridges);
+    if (haveBridges) {
+      torSettings.set(
+        TorConfigKeys.bridgeList,
+        settings.bridges.bridge_strings
+      );
+    } else {
+      torSettings.set(TorConfigKeys.bridgeList, null);
+    }
 
-      const cachedValue = this.#settingsCache.get(setting);
-      // Arrays are the only special case for which === could fail.
-      // The other values we accept (strings, booleans, numbers, null and
-      // undefined) work correctly with ===.
-      if (Array.isArray(value) && Array.isArray(cachedValue)) {
-        return (
-          value.length !== cachedValue.length ||
-          value.some((val, idx) => val !== cachedValue[idx])
+    // Proxy
+    torSettings.set(TorConfigKeys.socks4Proxy, null);
+    torSettings.set(TorConfigKeys.socks5Proxy, null);
+    torSettings.set(TorConfigKeys.socks5ProxyUsername, null);
+    torSettings.set(TorConfigKeys.socks5ProxyPassword, null);
+    torSettings.set(TorConfigKeys.httpsProxy, null);
+    torSettings.set(TorConfigKeys.httpsProxyAuthenticator, null);
+    if (settings.proxy && !settings.proxy.enabled) {
+      settings.proxy.type = null;
+    }
+    const address = settings.proxy?.address;
+    const port = settings.proxy?.port;
+    const username = settings.proxy?.username;
+    const password = settings.proxy?.password;
+    switch (settings.proxy?.type) {
+      case lazy.TorProxyType.Socks4:
+        torSettings.set(TorConfigKeys.socks4Proxy, `${address}:${port}`);
+        break;
+      case lazy.TorProxyType.Socks5:
+        torSettings.set(TorConfigKeys.socks5Proxy, `${address}:${port}`);
+        torSettings.set(TorConfigKeys.socks5ProxyUsername, username);
+        torSettings.set(TorConfigKeys.socks5ProxyPassword, password);
+        break;
+      case lazy.TorProxyType.HTTPS:
+        torSettings.set(TorConfigKeys.httpsProxy, `${address}:${port}`);
+        torSettings.set(
+          TorConfigKeys.httpsProxyAuthenticator,
+          `${username}:${password}`
         );
-      }
-      return value !== cachedValue;
-    });
-
-    // only write if new setting to save
-    if (newSettings.length) {
-      await this.#controller.setConf(Object.fromEntries(newSettings));
+        break;
+    }
 
-      // save settings to cache after successfully writing to Tor
-      for (const [setting, value] of newSettings) {
-        this.#settingsCache.set(setting, value);
-      }
+    // Firewall
+    if (settings.firewall?.enabled) {
+      const reachableAddresses = settings.firewall.allowed_ports
+        .map(port => `*:${port}`)
+        .join(",");
+      torSettings.set(TorConfigKeys.reachableAddresses, reachableAddresses);
+    } else {
+      torSettings.set(TorConfigKeys.reachableAddresses, null);
     }
+
+    logger.debug("Mapped settings object", settings, torSettings);
+    await this.#controller.setConf(Array.from(torSettings));
   }
 
   async flushSettings() {

toolkit/components/tor-launcher/TorProviderBuilder.sys.mjs

@@ -9,7 +9,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
 });
 
 export const TorProviderTopics = Object.freeze({
-  ProcessIsReady: "TorProcessIsReady",
   ProcessExited: "TorProcessExited",
   BootstrapStatus: "TorBootstrapStatus",
   BootstrapError: "TorBootstrapError",

toolkit/components/tor-launcher/TorStartupService.sys.mjs

@@ -31,15 +31,16 @@ export class TorStartupService {
     }
   }
 
-  async #init() {
+  #init() {
     Services.obs.addObserver(this, BrowserTopics.QuitApplicationGranted);
 
-    // Do not await on this init. build() is expected to await the
-    // initialization, so anything that should need the Tor Provider should
-    // block there, instead.
+    lazy.TorSettings.init();
+
+    // Theoretically, build() is expected to await the initialization of the
+    // provider, and anything needing the Tor Provider should be able to just
+    // await on TorProviderBuilder.build().
     lazy.TorProviderBuilder.init();
 
-    await lazy.TorSettings.init();
     lazy.TorConnect.init();
 
     lazy.TorDomainIsolator.init();

toolkit/modules/Moat.sys.mjs

@@ -2,13 +2,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import { TorBridgeSource } from "resource://gre/modules/TorSettings.sys.mjs";
-
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
   DomainFrontRequestBuilder:
     "resource://gre/modules/DomainFrontedRequests.sys.mjs",
+  TorBridgeSource: "resource://gre/modules/TorSettings.sys.mjs",
+  TorSettings: "resource://gre/modules/TorSettings.sys.mjs",
 });
 
 const TorLauncherPrefs = Object.freeze({
@@ -211,13 +211,23 @@ export class MoatRPC {
     };
     switch (settings.bridges.source) {
       case "builtin":
-        retval.bridges.source = TorBridgeSource.BuiltIn;
+        retval.bridges.source = lazy.TorBridgeSource.BuiltIn;
         retval.bridges.builtin_type = settings.bridges.type;
         // TorSettings will ignore strings for built-in bridges, and use the
-        // ones it already knows, instead.
+        // ones it already knows, instead. However, when we try these settings
+        // in the connect assist, we skip TorSettings. Therefore, we set the
+        // lines also here (the ones we already known, not the ones we receive
+        // from Moat). This needs TorSettings to be initialized, which by now
+        // should have already happened (this method is used only by TorConnect,
+        // that needs TorSettings to be initialized).
+        // In any case, getBuiltinBridges will throw if the data is not ready,
+        // yet.
+        retval.bridges.bridge_strings = lazy.TorSettings.getBuiltinBridges(
+          settings.bridges.type
+        );
         break;
       case "bridgedb":
-        retval.bridges.source = TorBridgeSource.BridgeDB;
+        retval.bridges.source = lazy.TorBridgeSource.BridgeDB;
         if (settings.bridges.bridge_strings) {
           retval.bridges.bridge_strings = settings.bridges.bridge_strings;
         } else {

toolkit/modules/TorAndroidIntegration.sys.mjs

@@ -150,7 +150,7 @@ class TorAndroidIntegrationImpl {
             lazy.TorSettings.saveToPrefs();
           }
           if (data.apply) {
-            lazy.TorSettings.applySettings();
+            await lazy.TorSettings.applySettings();
           }
           break;
         case ListenedEvents.settingsApply:

toolkit/modules/TorConnect.sys.mjs

@@ -2,14 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
 import { setTimeout, clearTimeout } from "resource://gre/modules/Timer.sys.mjs";
 
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
+  ConsoleAPI: "resource://gre/modules/Console.sys.mjs",
   EventDispatcher: "resource://gre/modules/Messaging.sys.mjs",
   MoatRPC: "resource://gre/modules/Moat.sys.mjs",
   TorBootstrapRequest: "resource://gre/modules/TorBootstrapRequest.sys.mjs",
+  TorProviderBuilder: "resource://gre/modules/TorProviderBuilder.sys.mjs",
 });
 
 // TODO: Should we move this to the about:torconnect actor?
@@ -20,10 +23,7 @@ ChromeUtils.defineModuleGetter(
 );
 
 import { TorLauncherUtil } from "resource://gre/modules/TorLauncherUtil.sys.mjs";
-import {
-  TorSettings,
-  TorSettingsTopics,
-} from "resource://gre/modules/TorSettings.sys.mjs";
+import { TorSettings } from "resource://gre/modules/TorSettings.sys.mjs";
 
 import { TorStrings } from "resource://gre/modules/TorStrings.sys.mjs";
 
@@ -40,6 +40,7 @@ const TorLauncherPrefs = Object.freeze({
 const TorConnectPrefs = Object.freeze({
   censorship_level: "torbrowser.debug.censorship_level",
   allow_internet_test: "torbrowser.bootstrap.allow_internet_test",
+  log_level: "torbrowser.bootstrap.log_level",
 });
 
 export const TorConnectState = Object.freeze({
@@ -59,6 +60,17 @@ export const TorConnectState = Object.freeze({
   Disabled: "Disabled",
 });
 
+XPCOMUtils.defineLazyGetter(
+  lazy,
+  "logger",
+  () =>
+    new lazy.ConsoleAPI({
+      maxLogLevel: "info",
+      maxLogLevelPref: TorConnectPrefs.log_level,
+      prefix: "TorConnect",
+    })
+);
+
 /*
                              TorConnect State Transitions
 
@@ -194,12 +206,12 @@ class StateCallback {
   }
 
   async begin(...args) {
-    console.log(`TorConnect: Entering ${this._state} state`);
+    lazy.logger.trace(`Entering ${this._state} state`);
     this._init();
     try {
       // this Promise will block until this StateCallback has completed its work
       await Promise.resolve(this._callback.call(this._context, ...args));
-      console.log(`TorConnect: Exited ${this._state} state`);
+      lazy.logger.info(`Exited ${this._state} state`);
 
       // handled state transition
       Services.obs.notifyObservers(
@@ -267,16 +279,14 @@ class InternetTest {
     this.cancel();
     this._pending = true;
 
-    console.log("TorConnect: starting the Internet test");
+    lazy.logger.info("Starting the Internet test");
     this._testAsync()
       .then(status => {
         this._pending = false;
         this._status = status.successful
           ? InternetStatus.Online
           : InternetStatus.Offline;
-        console.log(
-          `TorConnect: performed Internet test, outcome ${this._status}`
-        );
+        lazy.logger.info(`Performed Internet test, outcome ${this._status}`);
         this.onResult(this.status, status.date);
       })
       .catch(error => {
@@ -305,7 +315,7 @@ class InternetTest {
       await mrpc.init();
       status = await mrpc.testInternetConnection();
     } catch (err) {
-      console.error("Error while checking the Internet connection", err);
+      lazy.logger.error("Error while checking the Internet connection", err);
       error = err;
     } finally {
       mrpc.uninit();
@@ -523,8 +533,8 @@ export const TorConnect = (() => {
                   // get "Building circuits: Establishing a Tor circuit failed".
                   // TODO: Maybe move this logic deeper in the process to know
                   // when to filter out such errors triggered by cancelling.
-                  console.log(
-                    `TorConnect: Post-cancel error => ${message}; ${details}`
+                  lazy.logger.warn(
+                    `Post-cancel error => ${message}; ${details}`
                   );
                   return;
                 }
@@ -628,7 +638,7 @@ export const TorConnect = (() => {
                       "vanilla",
                     ]);
                   } catch (err) {
-                    console.error(
+                    lazy.logger.error(
                       "We did not get localized settings, and default settings failed as well",
                       err
                     );
@@ -651,10 +661,19 @@ export const TorConnect = (() => {
                   }
                 }
 
+                const restoreOriginalSettings = async () => {
+                  try {
+                    await TorSettings.applySettings();
+                  } catch (e) {
+                    // We cannot do much if the original settings were bad or
+                    // if the connection closed, so just report it in the
+                    // console.
+                    lazy.logger.warn("Failed to restore original settings.", e);
+                  }
+                };
+
                 // apply each of our settings and try to bootstrap with each
                 try {
-                  this.originalSettings = TorSettings.getSettings();
-
                   for (const [
                     index,
                     currentSetting,
@@ -664,14 +683,32 @@ export const TorConnect = (() => {
                       break;
                     }
 
-                    console.log(
-                      `TorConnect: Attempting Bootstrap with configuration ${
-                        index + 1
-                      }/${this.settings.length}`
+                    lazy.logger.info(
+                      `Attempting Bootstrap with configuration ${index + 1}/${
+                        this.settings.length
+                      }`
                     );
 
-                    TorSettings.setSettings(currentSetting);
-                    await TorSettings.applySettings();
+                    // Send the new settings directly to the provider. We will
+                    // save them only if the bootstrap succeeds.
+                    // FIXME: We should somehow signal TorSettings users that we
+                    // have set custom settings, and they should not apply
+                    // theirs until we are done with trying ours.
+                    // Otherwise, the new settings provided by the user while we
+                    // were bootstrapping could be the ones that cause the
+                    // bootstrap to succeed, but we overwrite them (unless we
+                    // backup the original settings, and then save our new
+                    // settings only if they have not changed).
+                    // Another idea (maybe easier to implement) is to disable
+                    // the settings UI while *any* bootstrap is going on.
+                    // This is also documented in tor-browser#41921.
+                    const provider = await lazy.TorProviderBuilder.build();
+                    // We need to merge with old settings, in case the user is
+                    // using a proxy or is behind a firewall.
+                    await provider.writeSettings({
+                      ...TorSettings.getSettings(),
+                      ...currentSetting,
+                    });
 
                     // build out our bootstrap request
                     const tbr = new lazy.TorBootstrapRequest();
@@ -679,8 +716,8 @@ export const TorConnect = (() => {
                       TorConnect._updateBootstrapStatus(progress, status);
                     };
                     tbr.onbootstraperror = (message, details) => {
-                      console.log(
-                        `TorConnect: Auto-Bootstrap error => ${message}; ${details}`
+                      lazy.logger.error(
+                        `Auto-Bootstrap error => ${message}; ${details}`
                       );
                     };
 
@@ -688,6 +725,7 @@ export const TorConnect = (() => {
                     this.on_transition = async nextState => {
                       if (nextState === TorConnectState.Configuring) {
                         await tbr.cancel();
+                        await restoreOriginalSettings();
                       }
                       resolve();
                     };
@@ -695,23 +733,20 @@ export const TorConnect = (() => {
                     // begin bootstrap
                     if (await tbr.bootstrap()) {
                       // persist the current settings to preferences
+                      TorSettings.setSettings(currentSetting);
                       TorSettings.saveToPrefs();
+                      await TorSettings.applySettings();
                       TorConnect._changeState(TorConnectState.Bootstrapped);
                       return;
                     }
                   }
 
-                  // bootstrapped failed for all potential settings, so reset daemon to use original
-                  TorSettings.setSettings(this.originalSettings);
-                  // The original settings should be good, so we save them to
-                  // preferences before trying to apply them, as it might fail
-                  // if the actual problem is with the connection to the control
-                  // port.
-                  // FIXME: We should handle this case in a better way.
-                  TorSettings.saveToPrefs();
-                  await TorSettings.applySettings();
-
-                  // only explicitly change state here if something else has not transitioned us
+                  // Bootstrap failed for all potential settings, so restore the
+                  // original settings the provider.
+                  await restoreOriginalSettings();
+
+                  // Only explicitly change state here if something else has not
+                  // transitioned us.
                   if (!this.transitioning) {
                     throw_error(
                       TorStrings.torConnect.autoBootstrappingFailed,
@@ -720,18 +755,8 @@ export const TorConnect = (() => {
                   }
                   return;
                 } catch (err) {
-                  // restore original settings in case of error
-                  try {
-                    TorSettings.setSettings(this.originalSettings);
-                    // As above
-                    TorSettings.saveToPrefs();
-                    await TorSettings.applySettings();
-                  } catch (errRestore) {
-                    console.log(
-                      `TorConnect: Failed to restore original settings => ${errRestore}`
-                    );
-                  }
-                  // throw to outer catch to transition us
+                  await restoreOriginalSettings();
+                  // throw to outer catch to transition us.
                   throw err;
                 }
               } catch (err) {
@@ -748,8 +773,8 @@ export const TorConnect = (() => {
                     true
                   );
                 } else {
-                  console.error(
-                    "TorConnect: Received AutoBootstrapping error after transitioning",
+                  lazy.logger.error(
+                    "Received AutoBootstrapping error after transitioning",
                     err
                   );
                 }
@@ -793,8 +818,8 @@ export const TorConnect = (() => {
 
               TorConnect._errorMessage = errorMessage;
               TorConnect._errorDetails = errorDetails;
-              console.error(
-                `[TorConnect] Entering error state (${errorMessage}, ${errorDetails})`
+              lazy.logger.error(
+                `Entering error state (${errorMessage}, ${errorDetails})`
               );
 
               Services.obs.notifyObservers(
@@ -835,9 +860,7 @@ export const TorConnect = (() => {
         );
       }
 
-      console.log(
-        `TorConnect: Try transitioning from ${prevState} to ${newState}`
-      );
+      lazy.logger.trace(`Try transitioning from ${prevState} to ${newState}`);
 
       // set our new state first so that state transitions can themselves trigger
       // a state transition
@@ -851,8 +874,8 @@ export const TorConnect = (() => {
       this._bootstrapProgress = progress;
       this._bootstrapStatus = status;
 
-      console.log(
-        `TorConnect: Bootstrapping ${this._bootstrapProgress}% complete (${this._bootstrapStatus})`
+      lazy.logger.info(
+        `Bootstrapping ${this._bootstrapProgress}% complete (${this._bootstrapStatus})`
       );
       Services.obs.notifyObservers(
         {
@@ -866,7 +889,7 @@ export const TorConnect = (() => {
 
     // init should be called by TorStartupService
     init() {
-      console.log("TorConnect: init()");
+      lazy.logger.debug("TorConnect.init()");
       this._callback(TorConnectState.Initial).begin();
 
       if (!this.enabled) {
@@ -875,9 +898,17 @@ export const TorConnect = (() => {
       } else {
         let observeTopic = addTopic => {
           Services.obs.addObserver(this, addTopic);
-          console.log(`TorConnect: Observing topic '${addTopic}'`);
+          lazy.logger.debug(`Observing topic '${addTopic}'`);
         };
 
+        // Wait for TorSettings, as we will need it.
+        // We will wait for a TorProvider only after TorSettings is ready,
+        // because the TorProviderBuilder initialization might not have finished
+        // at this point, and TorSettings initialization is a prerequisite for
+        // having a provider.
+        // So, we prefer initializing TorConnect as soon as possible, so that
+        // the UI will be able to detect it is in the Initializing state and act
+        // consequently.
         TorSettings.initializedPromise.then(() => this._settingsInitialized());
 
         // register the Tor topics we always care about
@@ -887,7 +918,7 @@ export const TorConnect = (() => {
     },
 
     async observe(subject, topic, data) {
-      console.log(`TorConnect: Observed ${topic}`);
+      lazy.logger.debug(`Observed ${topic}`);
 
       switch (topic) {
         case TorTopics.LogHasWarnOrErr: {
@@ -919,19 +950,25 @@ export const TorConnect = (() => {
       }
     },
 
-    _settingsInitialized() {
+    async _settingsInitialized() {
+      // TODO: Handle failures here, instead of the prompt to restart the
+      // daemon when it exits (tor-browser#21053, tor-browser#41921).
+      await lazy.TorProviderBuilder.build();
+
       // tor-browser#41907: This is only a workaround to avoid users being
       // bounced back to the initial panel without any explanation.
       // Longer term we should disable the clickable elements, or find a UX
       // to prevent this from happening (e.g., allow buttons to be clicked,
       // but show an intermediate starting state, or a message that tor is
       // starting while the butons are disabled, etc...).
+      // See also tor-browser#41921.
       if (this.state !== TorConnectState.Initial) {
-        console.warn(
-          "TorConnect: Seen the torsettings:ready after the state has already changed, ignoring the notification."
+        lazy.logger.warn(
+          "The TorProvider was built after the state had already changed."
         );
         return;
       }
+      lazy.logger.debug("The TorProvider is ready, changing state.");
       if (this.shouldQuickStart) {
         // Quickstart
         this._changeState(TorConnectState.Bootstrapping);
@@ -1074,17 +1111,17 @@ export const TorConnect = (() => {
         */
 
     beginBootstrap() {
-      console.log("TorConnect: beginBootstrap()");
+      lazy.logger.debug("TorConnect.beginBootstrap()");
       this._changeState(TorConnectState.Bootstrapping);
     },
 
     cancelBootstrap() {
-      console.log("TorConnect: cancelBootstrap()");
+      lazy.logger.debug("TorConnect.cancelBootstrap()");
       this._changeState(TorConnectState.Configuring);
     },
 
     beginAutoBootstrap(countryCode) {
-      console.log("TorConnect: beginAutoBootstrap()");
+      lazy.logger.debug("TorConnect.beginAutoBootstrap()");
       this._changeState(TorConnectState.AutoBootstrapping, countryCode);
     },
 
@@ -1154,7 +1191,10 @@ export const TorConnect = (() => {
         await mrpc.init();
         this._countryCodes = await mrpc.circumvention_countries();
       } catch (err) {
-        console.log("An error occurred while fetching country codes", err);
+        lazy.logger.error(
+          "An error occurred while fetching country codes",
+          err
+        );
       } finally {
         mrpc.uninit();
       }
@@ -1187,8 +1227,8 @@ export const TorConnect = (() => {
         uriArray = uriVariant;
       } else {
         // about:tor as safe fallback
-        console.error(
-          `TorConnect: received unknown variant '${JSON.stringify(uriVariant)}'`
+        lazy.logger.error(
+          `Received unknown variant '${JSON.stringify(uriVariant)}'`
         );
         uriArray = ["about:tor"];
       }
@@ -1209,9 +1249,7 @@ export const TorConnect = (() => {
     // which redirect after bootstrapping
     getURIsToLoad(uriVariant) {
       const uris = this.fixupURIs(uriVariant);
-      console.log(
-        `TorConnect: Will load after bootstrap => [${uris.join(", ")}]`
-      );
+      lazy.logger.debug(`Will load after bootstrap => [${uris.join(", ")}]`);
       return uris.map(uri => this.getRedirectURL(uri));
     },
   };

toolkit/modules/TorSettings.sys.mjs

@@ -6,10 +6,9 @@ const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
   TorLauncherUtil: "resource://gre/modules/TorLauncherUtil.sys.mjs",
-  TorProviderBuilder: "resource://gre/modules/TorProviderBuilder.sys.mjs",
-  TorProviderTopics: "resource://gre/modules/TorProviderBuilder.sys.mjs",
   Lox: "resource://gre/modules/Lox.sys.mjs",
   TorParsers: "resource://gre/modules/TorParsers.sys.mjs",
+  TorProviderBuilder: "resource://gre/modules/TorProviderBuilder.sys.mjs",
 });
 
 ChromeUtils.defineLazyGetter(lazy, "logger", () => {
@@ -71,20 +70,6 @@ const TorSettingsPrefs = Object.freeze({
   },
 });
 
-/* Config Keys used to configure tor daemon */
-const TorConfigKeys = Object.freeze({
-  useBridges: "UseBridges",
-  bridgeList: "Bridge",
-  socks4Proxy: "Socks4Proxy",
-  socks5Proxy: "Socks5Proxy",
-  socks5ProxyUsername: "Socks5ProxyUsername",
-  socks5ProxyPassword: "Socks5ProxyPassword",
-  httpsProxy: "HTTPSProxy",
-  httpsProxyAuthenticator: "HTTPSProxyAuthenticator",
-  reachableAddresses: "ReachableAddresses",
-  clientTransportPlugin: "ClientTransportPlugin",
-});
-
 export const TorBridgeSource = Object.freeze({
   Invalid: -1,
   BuiltIn: 0,
@@ -322,7 +307,7 @@ class TorSettingsImpl {
           if (!val) {
             return;
           }
-          const bridgeStrings = this.#getBuiltinBridges(val);
+          const bridgeStrings = this.getBuiltinBridges(val);
           if (bridgeStrings.length) {
             this.bridges.bridge_strings = bridgeStrings;
             return;
@@ -659,14 +644,17 @@ class TorSettingsImpl {
    * @param {string} pt The pluggable transport to return the lines for
    * @returns {string[]} The bridge lines in random order
    */
-  #getBuiltinBridges(pt) {
+  getBuiltinBridges(pt) {
+    if (!this.#allowUninitialized) {
+      this.#checkIfInitialized();
+    }
     // Shuffle so that Tor Browser users do not all try the built-in bridges in
     // the same order.
     return arrayShuffle(this.#builtinBridges[pt] ?? []);
   }
 
   /**
-   * Load or init our settings, and register observers.
+   * Load or init our settings.
    */
   async init() {
     if (this.#initialized) {
@@ -677,6 +665,7 @@ class TorSettingsImpl {
       await this.#initInternal();
       this.#initialized = true;
       this.#initComplete();
+      Services.obs.notifyObservers(null, TorSettingsTopics.Ready);
     } catch (e) {
       this.#initFailed(e);
       throw e;
@@ -698,45 +687,35 @@ class TorSettingsImpl {
       lazy.logger.error("Could not load the built-in PT config.", e);
     }
 
-    // Initialize this before loading from prefs because we need Lox initialized before
-    // any calls to Lox.getBridges()
+    // Initialize this before loading from prefs because we need Lox initialized
+    // before any calls to Lox.getBridges().
     try {
       await lazy.Lox.init();
     } catch (e) {
       lazy.logger.error("Could not initialize Lox.", e.type);
     }
 
-    // TODO: We could use a shared promise, and wait for it to be fullfilled
-    // instead of Service.obs.
-    if (lazy.TorLauncherUtil.shouldStartAndOwnTor) {
-      // if the settings branch exists, load settings from prefs
-      if (Services.prefs.getBoolPref(TorSettingsPrefs.enabled, false)) {
-        // Do not want notifications for initially loaded prefs.
-        this.freezeNotifications();
-        try {
-          this.#allowUninitialized = true;
-          this.#loadFromPrefs();
-        } finally {
-          this.#allowUninitialized = false;
-          this.#notificationQueue.clear();
-          this.thawNotifications();
-        }
-      }
+    if (
+      lazy.TorLauncherUtil.shouldStartAndOwnTor &&
+      Services.prefs.getBoolPref(TorSettingsPrefs.enabled, false)
+    ) {
+      // Do not want notifications for initially loaded prefs.
+      this.freezeNotifications();
       try {
-        const provider = await lazy.TorProviderBuilder.build();
-        if (provider.isRunning) {
-          this.#handleProcessReady();
-          // No need to add an observer to call this again.
-          return;
-        }
-      } catch {}
-
-      Services.obs.addObserver(this, lazy.TorProviderTopics.ProcessIsReady);
+        this.#allowUninitialized = true;
+        this.#loadFromPrefs();
+      } finally {
+        this.#allowUninitialized = false;
+        this.#notificationQueue.clear();
+        this.thawNotifications();
+      }
     }
+
+    lazy.logger.info("Ready");
   }
 
   /**
-   * Unload or uninit our settings, and unregister observers.
+   * Unload or uninit our settings.
    */
   async uninit() {
     await lazy.Lox.uninit();
@@ -764,34 +743,6 @@ class TorSettingsImpl {
     return this.#initialized;
   }
 
-  /**
-   * Wait for relevant life-cycle events to apply saved settings.
-   */
-  async observe(subject, topic, data) {
-    lazy.logger.debug(`Observed ${topic}`);
-
-    switch (topic) {
-      case lazy.TorProviderTopics.ProcessIsReady:
-        Services.obs.removeObserver(
-          this,
-          lazy.TorProviderTopics.ProcessIsReady
-        );
-        await this.#handleProcessReady();
-        break;
-    }
-  }
-
-  /**
-   * Apply the settings once the tor provider is ready and notify any observer
-   * that the settings can be used.
-   */
-  async #handleProcessReady() {
-    // push down settings to tor
-    await this.#applySettings(true);
-    lazy.logger.info("Ready");
-    Services.obs.notifyObservers(null, TorSettingsTopics.Ready);
-  }
-
   /**
    * Load our settings from prefs.
    */
@@ -972,85 +923,14 @@ class TorSettingsImpl {
 
   /**
    * Push our settings down to the tor provider.
+   *
+   * Even though this introduces a circular depdency, it makes the API nicer for
+   * frontend consumers.
    */
   async applySettings() {
     this.#checkIfInitialized();
-    return this.#applySettings(false);
-  }
-
-  /**
-   * Internal implementation of applySettings that does not check if we are
-   * initialized.
-   */
-  async #applySettings(allowUninitialized) {
-    lazy.logger.debug("#applySettings()");
-
-    this.#cleanupSettings();
-
-    const settingsMap = new Map();
-
-    // #applySettings can be called only when #allowUninitialized is false
-    this.#allowUninitialized = allowUninitialized;
-
-    try {
-      /* Bridges */
-      const haveBridges =
-        this.bridges.enabled && !!this.bridges.bridge_strings.length;
-      settingsMap.set(TorConfigKeys.useBridges, haveBridges);
-      if (haveBridges) {
-        settingsMap.set(TorConfigKeys.bridgeList, this.bridges.bridge_strings);
-      } else {
-        settingsMap.set(TorConfigKeys.bridgeList, null);
-      }
-
-      /* Proxy */
-      settingsMap.set(TorConfigKeys.socks4Proxy, null);
-      settingsMap.set(TorConfigKeys.socks5Proxy, null);
-      settingsMap.set(TorConfigKeys.socks5ProxyUsername, null);
-      settingsMap.set(TorConfigKeys.socks5ProxyPassword, null);
-      settingsMap.set(TorConfigKeys.httpsProxy, null);
-      settingsMap.set(TorConfigKeys.httpsProxyAuthenticator, null);
-      if (this.proxy.enabled) {
-        const address = this.proxy.address;
-        const port = this.proxy.port;
-        const username = this.proxy.username;
-        const password = this.proxy.password;
-
-        switch (this.proxy.type) {
-          case TorProxyType.Socks4:
-            settingsMap.set(TorConfigKeys.socks4Proxy, `${address}:${port}`);
-            break;
-          case TorProxyType.Socks5:
-            settingsMap.set(TorConfigKeys.socks5Proxy, `${address}:${port}`);
-            settingsMap.set(TorConfigKeys.socks5ProxyUsername, username);
-            settingsMap.set(TorConfigKeys.socks5ProxyPassword, password);
-            break;
-          case TorProxyType.HTTPS:
-            settingsMap.set(TorConfigKeys.httpsProxy, `${address}:${port}`);
-            settingsMap.set(
-              TorConfigKeys.httpsProxyAuthenticator,
-              `${username}:${password}`
-            );
-            break;
-        }
-      }
-
-      /* Firewall */
-      if (this.firewall.enabled) {
-        const reachableAddresses = this.firewall.allowed_ports
-          .map(port => `*:${port}`)
-          .join(",");
-        settingsMap.set(TorConfigKeys.reachableAddresses, reachableAddresses);
-      } else {
-        settingsMap.set(TorConfigKeys.reachableAddresses, null);
-      }
-    } finally {
-      this.#allowUninitialized = false;
-    }
-
-    /* Push to Tor */
     const provider = await lazy.TorProviderBuilder.build();
-    await provider.writeSettings(settingsMap);
+    await provider.writeSettings(this.getSettings());
   }
 
   /**