[tor-commits] [Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 3 commits: fixup! TB 40597: Implement TorSettings module

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser

Commits:

0b42b720

by Henry Wilkes at 2025-01-20T17:57:59+00:00

fixup! TB 40597: Implement TorSettings module

TB 41921: Return only the bridge settings from Moat.

Rather than return { bridges: enabled: true, ... } from MoatRPC API, we
just return the relevant parts. This should make it clearer that Moat
can *only* change TorSettings.bridges, and nothing else.

7129f408

by Henry Wilkes at 2025-01-20T17:57:59+00:00

fixup! TB 40597: Implement TorSettings module

TB 41921: Have Moat bridges pass through TorSettings rather than using
TorProvider directly.

TorSettings should be the only caller to TorProvider.writeSettings, so
we can establish more control over which bridges are in use at any given
moment in one place.

Also add some sanitation to the Moat response.

6ea6bd74

by Henry Wilkes at 2025-01-20T17:57:59+00:00

fixup! TB 40933: Add tor-launcher functionality

TB 41921: Use temporary bridges for TorProvider initialisation.

It is unlikely that temporary bridges would be present when TorProvider
is initialised, but it should be using whatever
TorSettings.applySettings uses.

4 changed files:

toolkit/components/tor-launcher/TorProvider.sys.mjs
toolkit/modules/Moat.sys.mjs
toolkit/modules/TorConnect.sys.mjs
toolkit/modules/TorSettings.sys.mjs

Changes:

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

@@ -227,7 +227,7 @@ export class TorProvider {
      if (this.ownsTorDaemon) {
        try {
          await lazy.TorSettings.initializedPromise;
 -        await this.writeSettings(lazy.TorSettings.getSettings());
 +        await this.writeSettings();
        } catch (e) {
          logger.warn(
            "Failed to initialize TorSettings or to write our initial settings. Continuing the initialization anyway.",
@@ -269,11 +269,13 @@ export class TorProvider {
    /**
     * 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.
 +   * This should only be called internally or by the TorSettings module.
     */
 -  async writeSettings(settings) {
 +  async writeSettings() {
 +    // Fetch the current settings.
 +    // We set the useTemporary parameter since we want to apply temporary
 +    // bridges if they are available.
 +    const settings = lazy.TorSettings.getSettings(true);
      logger.debug("TorProvider.writeSettings", settings);
      const torSettings = new Map();

toolkit/modules/Moat.sys.mjs

@@ -13,7 +13,6 @@ 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({
@@ -73,6 +72,25 @@ class InternetTestResponseListener {
+   }
+ }
 +/**
 + * @typedef {Object} MoatBridges
 + *
 + * Bridge settings that can be passed to TorSettings.bridges.
 + *
 + * @property {number} source - The `TorBridgeSource` type.
 + * @property {string} [builtin_type] - The built-in bridge type.
 + * @property {string[]} [bridge_strings] - The bridge lines.
 + */
++
 +/**
 + * @typedef {Object} MoatSettings
 + *
 + * The settings returned by Moat.
 + *
 + * @property {MoatBridges[]} bridgesList - The list of bridges found.
 + * @property {string} [country] - The detected country (region).
 + */
++
  /**
   * Constructs JSON objects and sends requests over Moat.
   * The documentation about the JSON schemas to use are available at
@@ -213,37 +231,31 @@ export class MoatRPC {
      return { bridges, qrcode: qrcodeImg };
+   }
 -  // Convert received settings object to format used by TorSettings module.
 -  #fixupSettings(settings) {
 +  /**
 +   * Extract bridges from the received Moat settings object.
 +   *
 +   * @param {Object} settings - The received settings.
 +   * @return {MoatBridge} The extracted bridges.
 +   */
 +  #extractBridges(settings) {
      if (!("bridges" in settings)) {
        throw new Error("Expected to find `bridges` in the settings object.");
+     }
 -    const retval = {
 -      bridges: {
 -        enabled: true,
 -      },
 -    };
 +    const bridges = {};
      switch (settings.bridges.source) {
        case "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. However, when we try these settings
 -        // in the connect assist, we skip TorSettings. Therefore, we set the
 -        // lines also here (the ones we already know, 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
 -        );
 +        bridges.source = lazy.TorBridgeSource.BuiltIn;
 +        bridges.builtin_type = String(settings.bridges.type);
 +        // Ignore the bridge_strings argument since we will use our built-in
 +        // bridge strings instead.
          break;
        case "bridgedb":
 -        retval.bridges.source = lazy.TorBridgeSource.BridgeDB;
 -        if (settings.bridges.bridge_strings) {
 -          retval.bridges.bridge_strings = settings.bridges.bridge_strings;
 +        bridges.source = lazy.TorBridgeSource.BridgeDB;
 +        if (settings.bridges.bridge_strings?.length) {
 +          bridges.bridge_strings = Array.from(
 +            settings.bridges.bridge_strings,
 +            item => String(item)
 +          );
          } else {
            throw new Error(
              "Received no bridge-strings for BridgeDB bridge source"
@@ -255,37 +267,38 @@ export class MoatRPC {
            `Unexpected bridge source '${settings.bridges.source}'`
          );
+     }
 -    return retval;
 +    return bridges;
+   }
 -  // Converts a list of settings objects received from BridgeDB to a list of
 -  // settings objects understood by the TorSettings module.
 -  // In the event of error, returns an empty list.
 -  #fixupSettingsList(settingsList) {
 -    const retval = [];
 +  /**
 +   * Extract a list of bridges from the received Moat settings object.
 +   *
 +   * @param {Object} settings - The received settings.
 +   * @return {MoatBridge[]} The list of extracted bridges.
 +   */
 +  #extractBridgesList(settingsList) {
 +    const bridgesList = [];
      for (const settings of settingsList) {
        try {
 -        retval.push(this.#fixupSettings(settings));
 +        bridgesList.push(this.#extractBridges(settings));
        } catch (ex) {
          log.error(ex);
+       }
+     }
 -    return retval;
 +    return bridgesList;
+   }
 -  // Request tor settings for the user optionally based on their location
 -  // (derived from their IP). Takes the following parameters:
 -  // - transports: optional, an array of transports available to the client; if
 -  //   empty (or not given) returns settings using all working transports known
 -  //   to the server
 -  // - country: optional, an ISO 3166-1 alpha-2 country code to request settings
 -  //   for; if not provided the country is determined by the user's IP address
 -  //
 -  // Returns an object with the detected country code and an array of settings
 -  // in a format that can be passed to the TorSettings module. This array might
 -  // be empty if the country has no associated settings.
 -  // If the server cannot determine the user's country (and no country code is
 -  // provided), then null is returned instead of the object.
 +  /**
 +   * Request tor settings for the user optionally based on their location
 +   * (derived from their IP). Takes the following parameters:
 +   *
 +   * @param {string[]} transports - A list of transports we support.
 +   * @param {?string} country - The region to request bridges for, as an
 +   *   ISO 3166-1 alpha-2 region code, or `null` to have the server
 +   *   automatically determine the region.
 +   * @returns {?MoatSettings} - The returned settings from the server, or `null`
 +   *   if the region could not be determined by the server.
 +   */
    async circumvention_settings(transports, country) {
      const args = {
        transports: transports ? transports : [],
@@ -306,7 +319,7 @@ export class MoatRPC {
        throw new Error(`MoatRPC: ${detail} (${code})`);
      } else if ("settings" in response) {
 -      settings.settings = this.#fixupSettingsList(response.settings);
 +      settings.bridgesList = this.#extractBridgesList(response.settings);
+     }
      if ("country" in response) {
        settings.country = response.country;
@@ -349,14 +362,12 @@ export class MoatRPC {
      return map;
+   }
 -  // Request a copy of the defaul/fallback bridge settings, takes the following
 -  // parameters:
 -  // - transports: optional, an array of transports available to the client; if
 -  //   empty (or not given) returns settings using all working transports known
 -  //   to the server
 -  //
 -  // returns an array of settings objects in roughly the same format as the
 -  // _settings object on the TorSettings module
 +  /**
 +   * Request a copy of the default/fallback bridge settings.
 +   *
 +   * @param {string[]} transports - A list of transports we support.
 +   * @returns {MoatBridges[]} - The list of bridges found.
 +   */
    async circumvention_defaults(transports) {
      const args = {
        transports: transports ? transports : [],
@@ -367,7 +378,7 @@ export class MoatRPC {
        const detail = response.errors[0].detail;
        throw new Error(`MoatRPC: ${detail} (${code})`);
      } else if ("settings" in response) {
 -      return this.#fixupSettingsList(response.settings);
 +      return this.#extractBridgesList(response.settings);
+     }
      return [];
+   }

toolkit/modules/TorConnect.sys.mjs

@@ -103,11 +103,12 @@ export const TorConnectTopics = Object.freeze({
   *   failing bootstrap.
   * @property {integer} [options.simulateDelay] - The delay in microseconds to
   *   apply to simulated bootstraps.
 - * @property {object} [options.simulateMoatResponse] - Simulate a Moat response
 - *   for circumvention settings. Should include a "settings" property, and
 - *   optionally a "country" property. You may add a "simulateCensorship"
 - *   property to some of the settings to make only their bootstrap attempts
 - *   fail.
 + * @property {MoatSettings} [options.simulateMoatResponse] - Simulate a Moat
 + *   response for circumvention settings. Should include a "bridgesList"
 + *   property, and optionally a "country" property. The "bridgesList" property
 + *   should be an Array of MoatBridges objects that match the bridge settings
 + *   accepted by TorSettings.bridges, plus you may add a "simulateCensorship"
 + *   property to make only their bootstrap attempts fail.
   * @property {boolean} [options.testInternet] - Whether to also test the
   *   internet connection.
   * @property {boolean} [options.simulateOffline] - Whether to simulate an
@@ -214,6 +215,14 @@ class BootstrapAttempt {
      return promise;
+   }
 +  /**
 +   * Method to call just after the Bootstrapped stage is set in response to this
 +   * bootstrap attempt.
 +   */
 +  postBootstrapped() {
 +    // Nothing to do.
 +  }
++
    /**
     * Run the attempt.
+    *
@@ -395,17 +404,11 @@ class AutoBootstrapAttempt {
     */
    #cancelledPromise = null;
    /**
 -   * The found settings from Moat.
 -   *
 -   * @type {?object[]}
 -   */
 -  #settings = null;
 -  /**
 -   * The last settings that have been applied to the TorProvider, if any.
 +   * The list of bridge configurations from Moat.
+    *
 -   * @type {?object}
 +   * @type {?MoatBridges[]}
     */
 -  #changedSetting = null;
 +  #bridgesList = null;
    /**
     * The detected region code returned by Moat, if any.
+    *
@@ -438,13 +441,8 @@ class AutoBootstrapAttempt {
          // Run cleanup before we resolve the promise to ensure two instances
          // of AutoBootstrapAttempt are not trying to change the settings at
          // the same time.
 -        if (this.#changedSetting) {
 -          if (arg.result === "complete") {
 -            // Persist the current settings to preferences.
 -            lazy.TorSettings.setSettings(this.#changedSetting);
 -            lazy.TorSettings.saveToPrefs();
 -          } // else, applySettings will restore the current settings.
 -          await lazy.TorSettings.applySettings();
 +        if (arg.result !== "complete") {
 +          await lazy.TorSettings.clearTemporaryBridges();
+         }
        } catch (error) {
          lazy.logger.error("Unexpected error in auto-bootstrap cleanup", error);
@@ -466,6 +464,15 @@ class AutoBootstrapAttempt {
      return promise;
+   }
 +  /**
 +   * Method to call just after the Bootstrapped stage is set in response to this
 +   * bootstrap attempt.
 +   */
 +  postBootstrapped() {
 +    // Persist the current settings to preferences.
 +    lazy.TorSettings.saveTemporaryBridges();
 +  }
++
    /**
     * Run the attempt.
+    *
@@ -477,12 +484,12 @@ class AutoBootstrapAttempt {
     * @param {BootstrapOptions} options - Options to apply to the bootstrap.
     */
    async #runInternal(progressCallback, options) {
 -    await this.#fetchSettings(options);
 +    await this.#fetchBridges(options);
      if (this.#cancelled || this.#resolved) {
        return;
+     }
 -    if (!this.#settings?.length) {
 +    if (!this.#bridgesList?.length) {
        this.#resolveRun({
          error: new TorConnectError(
            options.regionCode === "automatic" && !this.detectedRegion
@@ -493,14 +500,14 @@ class AutoBootstrapAttempt {
+     }
      // Apply each of our settings and try to bootstrap with each.
 -    for (const [index, currentSetting] of this.#settings.entries()) {
 +    for (const [index, bridges] of this.#bridgesList.entries()) {
        lazy.logger.info(
          `Attempting Bootstrap with configuration ${index + 1}/${
 -          this.#settings.length
 +          this.#bridgesList.length
          }`
        );
 -      await this.#trySetting(currentSetting, progressCallback, options);
 +      await this.#tryBridges(bridges, progressCallback, options);
        if (this.#cancelled || this.#resolved) {
          return;
@@ -518,7 +525,7 @@ class AutoBootstrapAttempt {
+    *
     * @param {BootstrapOptions} options - Options to apply to the bootstrap.
     */
 -  async #fetchSettings(options) {
 +  async #fetchBridges(options) {
      if (options.simulateMoatResponse) {
        await Promise.race([
          new Promise(res => setTimeout(res, options.simulateDelay || 0)),
@@ -530,7 +537,7 @@ class AutoBootstrapAttempt {
+       }
        this.detectedRegion = options.simulateMoatResponse.country || null;
 -      this.#settings = options.simulateMoatResponse.settings ?? null;
 +      this.#bridgesList = options.simulateMoatResponse.bridgesList ?? null;
        return;
+     }
@@ -564,16 +571,16 @@ class AutoBootstrapAttempt {
        this.detectedRegion = maybeSettings?.country || null;
 -      if (maybeSettings?.settings?.length) {
 -        this.#settings = maybeSettings.settings;
 +      if (maybeSettings?.bridgesList?.length) {
 +        this.#bridgesList = maybeSettings.bridgesList;
        } else {
          // Keep consistency with the other call.
 -        this.#settings = await Promise.race([
 +        this.#bridgesList = await Promise.race([
            moat.circumvention_defaults([
              ...lazy.TorSettings.builtinBridgeTypes,
              "vanilla",
            ]),
 -          // This might set this.#settings to undefined.
 +          // This might set this.#bridgesList to undefined.
            this.#cancelledPromise,
          ]);
+       }
@@ -586,21 +593,21 @@ class AutoBootstrapAttempt {
    /**
     * Try to apply the settings we fetched.
+    *
 -   * @param {object} setting - The setting to try.
 +   * @param {MoatBridges} bridges - The bridges to try.
     * @param {ProgressCallback} progressCallback - The callback to invoke with
     *   the bootstrap progress.
     * @param {BootstrapOptions} options - Options to apply to the bootstrap.
     */
 -  async #trySetting(setting, progressCallback, options) {
 +  async #tryBridges(bridges, progressCallback, options) {
      if (this.#cancelled || this.#resolved) {
        return;
+     }
 -    if (options.simulateMoatResponse && setting.simulateCensorship) {
 +    if (options.simulateMoatResponse && bridges.simulateCensorship) {
        // Move the simulateCensorship option to the options for the next
        // BootstrapAttempt.
 -      setting = structuredClone(setting);
 -      delete setting.simulateCensorship;
 +      bridges = structuredClone(bridges);
 +      delete bridges.simulateCensorship;
        options = { ...options, simulateCensorship: true };
+     }
@@ -616,14 +623,7 @@ class AutoBootstrapAttempt {
      // 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();
 -    this.#changedSetting = setting;
 -    // We need to merge with old settings, in case the user is using a proxy
 -    // or is behind a firewall.
 -    await provider.writeSettings({
 -      ...lazy.TorSettings.getSettings(),
 -      ...setting,
 -    });
 +    await lazy.TorSettings.applyTemporaryBridges(bridges);
      if (this.#cancelled || this.#resolved) {
        return;
@@ -642,7 +642,7 @@ class AutoBootstrapAttempt {
          error instanceof TorConnectError &&
          error.code === TorConnectError.BootstrapError
        ) {
 -        lazy.logger.info("TorConnect setting failed", setting, error);
 +        lazy.logger.info("TorConnect setting failed", bridges, error);
          // Try with the next settings.
          // NOTE: We do not restore the user settings in between these runs.
          // Instead we wait for #resolveRun callback to do so.
@@ -1459,6 +1459,12 @@ export const TorConnect = {
+       }
        this._setStage(TorConnectStage.Bootstrapped);
        Services.obs.notifyObservers(null, TorConnectTopics.BootstrapComplete);
++
 +      // Now call the postBootstrapped method. We do this after changing the
 +      // stage to ensure that AutoBootstrapAttempt.postBootstrapped call to
 +      // saveTemporaryBridges does not trigger SettingsChanged too early and
 +      // cancel itself.
 +      bootstrapAttempt.postBootstrapped();
        return;
+     }

toolkit/modules/TorSettings.sys.mjs

@@ -198,6 +198,14 @@ class TorSettingsImpl {
        allowed_ports: [],
      },
    };
++
 +  /**
 +   * Temporary bridge settings to apply instead of #settings.bridges.
 +   *
 +   * @type {?Object}
 +   */
 +  #temporaryBridgeSettings = null;
++
    /**
     * Accumulated errors from trying to set settings.
+    *
@@ -713,6 +721,8 @@ class TorSettingsImpl {
        try {
          this.#allowUninitialized = true;
          this.#loadFromPrefs();
 +        // We do not pass on the loaded settings to the TorProvider yet. Instead
 +        // TorProvider will ask for these once it has initialised.
        } finally {
          this.#allowUninitialized = false;
          this.#notificationQueue.clear();
@@ -971,7 +981,7 @@ class TorSettingsImpl {
    async applySettings() {
      this.#checkIfInitialized();
      const provider = await lazy.TorProviderBuilder.build();
 -    await provider.writeSettings(this.getSettings());
 +    await provider.writeSettings();
+   }
    /**
@@ -1073,12 +1083,20 @@ class TorSettingsImpl {
    /**
     * Get a copy of all our settings.
+    *
 +   * @param {boolean} [useTemporary=false] - Whether the returned settings
 +   *   should use the temporary bridge settings, if any, instead.
 +   *
     * @returns {object} A copy of the settings object
     */
 -  getSettings() {
 +  getSettings(useTemporary = false) {
      lazy.logger.debug("getSettings()");
      this.#checkIfInitialized();
 -    return structuredClone(this.#settings);
 +    const settings = structuredClone(this.#settings);
 +    if (useTemporary && this.#temporaryBridgeSettings) {
 +      // Override the bridge settings with our temporary ones.
 +      settings.bridges = structuredClone(this.#temporaryBridgeSettings);
 +    }
 +    return settings;
+   }
    /**
@@ -1097,6 +1115,83 @@ class TorSettingsImpl {
+     }
      return types;
+   }
++
 +  /**
 +   * Apply some Moat bridges temporarily.
 +   *
 +   * These bridges will not yet be saved to settings.
 +   *
 +   * @param {MoatBridges} bridges - The bridges to apply.
 +   */
 +  async applyTemporaryBridges(bridges) {
 +    this.#checkIfInitialized();
++
 +    if (
 +      bridges.source !== TorBridgeSource.BuiltIn &&
 +      bridges.source !== TorBridgeSource.BridgeDB
 +    ) {
 +      throw new Error(`Invalid bridge source ${bridges.source}`);
 +    }
++
 +    const bridgeSettings = {
 +      enabled: true,
 +      source: bridges.source,
 +    };
++
 +    if (bridges.source === TorBridgeSource.BuiltIn) {
 +      if (!bridges.builtin_type) {
 +        throw Error("Missing a built-in type");
 +      }
 +      bridgeSettings.builtin_type = String(bridges.builtin_type);
 +      const bridgeStrings = this.getBuiltinBridges(bridgeSettings.builtin_type);
 +      if (!bridgeStrings.length) {
 +        throw new Error(`No builtin bridges for type ${bridges.builtin_type}`);
 +      }
 +      bridgeSettings.bridge_strings = bridgeStrings;
 +    } else {
 +      // BridgeDB.
 +      if (!bridges.bridge_strings?.length) {
 +        throw new Error("Missing bridges strings");
 +      }
 +      // TODO: Can we safely verify the format of the bridge addresses sent from
 +      // Moat?
 +      bridgeSettings.bridge_strings = Array.from(bridges.bridge_strings, item =>
 +        String(item)
 +      );
 +    }
++
 +    // After checks are complete, we commit them.
 +    this.#temporaryBridgeSettings = bridgeSettings;
 +    await this.applySettings();
 +  }
++
 +  /**
 +   * Save to current temporary bridges to be permanent instead.
 +   */
 +  async saveTemporaryBridges() {
 +    this.#checkIfInitialized();
 +    if (!this.#temporaryBridgeSettings) {
 +      lazy.logger.warn("No temporary bridges to save");
 +      return;
 +    }
 +    this.setSettings({ bridges: this.#temporaryBridgeSettings });
 +    this.#temporaryBridgeSettings = null;
 +    this.saveToPrefs();
 +    await this.applySettings();
 +  }
++
 +  /**
 +   * Clear the current temporary bridges.
 +   */
 +  async clearTemporaryBridges() {
 +    this.#checkIfInitialized();
 +    if (!this.#temporaryBridgeSettings) {
 +      lazy.logger.debug("No temporary bridges to clear");
 +      return;
 +    }
 +    this.#temporaryBridgeSettings = null;
 +    await this.applySettings();
 +  }
+ }
  export const TorSettings = new TorSettingsImpl();