GitLab

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.

@@ -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();

@@ -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 [];
+   }

@@ -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();

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

Commits:

4 changed files:

Changes:

@@ -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;
+     }