GitLab

at 2025-01-28T10:06:19+00:00

fixup! TB 40597: Implement TorSettings module

TB 43254: Allow some DomainFrontedRequests to be cancelled.

This is used by some of the Moat calls to stop abandoned requests
running in the background:

1. Cancelled AutoBootstraps.
2. Cancelled CAPTCHA request/submissions.

@@ -143,6 +143,7 @@ pref("browser.tor_provider.cp_log_level", "Warn");
  pref("lox.log_level", "Warn");
  pref("torbrowser.bootstrap.log_level", "Info");
  pref("browser.torsettings.log_level", "Warn");
 +pref("browser.torMoat.loglevel", "Warn");
  pref("browser.tordomainisolator.loglevel", "Warn");
  pref("browser.torcircuitpanel.loglevel", "Log");
  pref("browser.tor_android.log_level", "Info");

@@ -119,6 +119,9 @@ const gRequestBridgeDialog = {
    },
    _setcaptchaImage(uri) {
 +    if (!uri) {
 +      return;
 +    }
      if (uri != this._captchaImage.src) {
        this._captchaImage.src = uri;
        this._dialogHeader.setAttribute(

@@ -13,6 +13,16 @@ export var BridgeDB = {
    _challenge: null,
    _image: null,
    _bridges: null,
 +  /**
 +   * A collection of controllers to abort any ongoing Moat requests if the
 +   * dialog is closed.
 +   *
 +   * NOTE: We do not expect this set to ever contain more than one instance.
 +   * However the public API has no assurances to prevent multiple calls.
 +   *
 +   * @type {Set<AbortController>}
 +   */
 +  _moatAbortControllers: new Set(),
    get currentCaptchaImage() {
      return this._image;
@@ -28,13 +38,18 @@ export var BridgeDB = {
        await this._moatRPC.init();
+     }
 -    const response = await this._moatRPC.check(
 -      "obfs4",
 -      this._challenge,
 -      solution,
 -      false
 -    );
 -    this._bridges = response?.bridges;
 +    const abortController = new AbortController();
 +    this._moatAbortControllers.add(abortController);
 +    try {
 +      this._bridges = await this._moatRPC.check(
 +        "obfs4",
 +        this._challenge,
 +        solution,
 +        abortController.signal
 +      );
 +    } finally {
 +      this._moatAbortControllers.delete(abortController);
 +    }
      return this._bridges;
    },
@@ -45,10 +60,20 @@ export var BridgeDB = {
          await this._moatRPC.init();
+       }
 -      const response = await this._moatRPC.fetch(["obfs4"]);
 -      this._challenge = response.challenge;
 -      this._image =
 -        "data:image/jpeg;base64," + encodeURIComponent(response.image);
 +      const abortController = new AbortController();
 +      this._moatAbortControllers.add(abortController);
 +      let response;
 +      try {
 +        response = await this._moatRPC.fetch(["obfs4"], abortController.signal);
 +      } finally {
 +        this._moatAbortControllers.delete(abortController);
 +      }
 +      if (response) {
 +        // Not cancelled.
 +        this._challenge = response.challenge;
 +        this._image =
 +          "data:image/jpeg;base64," + encodeURIComponent(response.image);
 +      }
      } catch (err) {
        console.error("Could not request a captcha image", err);
+     }
@@ -56,6 +81,11 @@ export var BridgeDB = {
    },
    close() {
 +    // Abort any ongoing requests.
 +    for (const controller of this._moatAbortControllers) {
 +      controller.abort();
 +    }
 +    this._moatAbortControllers.clear();
      this._moatRPC?.uninit();
      this._moatRPC = null;
      this._challenge = null;

@@ -377,6 +377,15 @@ export class DomainFrontRequestResponseError extends Error {
+   }
+ }
 +/**
 + * Thrown when the caller cancels the request.
 + */
 +export class DomainFrontRequestCancelledError extends Error {
 +  constructor(url) {
 +    super(`Cancelled request to ${url}`);
 +  }
 +}
++
  /**
   * Callback object to promisify the XPCOM request.
   */
@@ -397,8 +406,12 @@ class ResponseListener {
      });
+   }
 -  // callers wait on this for final response
 -  response() {
 +  /**
 +   * A promise that resolves to the response body from the request.
 +   *
 +   * @type {Promise<string>}
 +   */
 +  get response() {
      return this.#responsePromise;
+   }
@@ -530,24 +543,74 @@ export class DomainFrontRequestBuilder {
     * @param {string} args.method The request method.
     * @param {string} args.body The request body.
     * @param {string} args.contentType The "Content-Type" header to set.
 -   * @returns {string} The response body.
 +   * @param {AbortSignal} [signal] args.signal An optional means of cancelling
 +   *   the request early. Will throw DomainFrontRequestCancelledError if
 +   *   aborted.
 +   * @returns {Promise<string>} A promise that resolves to the response body.
     */
 -  async buildRequest(url, args) {
 -    const ch = this.buildHttpHandler(url);
+-
 -    const inStream = Cc["@mozilla.org/io/string-input-stream;1"].createInstance(
 -      Ci.nsIStringInputStream
 -    );
 -    inStream.setData(args.body, args.body.length);
 -    const upChannel = ch.QueryInterface(Ci.nsIUploadChannel);
 -    upChannel.setUploadStream(inStream, args.contentType, args.body.length);
 -    ch.requestMethod = args.method;
 +  buildRequest(url, args) {
 +    // Pre-fetch the argument values from `args` so the caller cannot change the
 +    // parameters mid-call.
 +    const { body, method, contentType, signal } = args;
 +    let cancel = null;
 +    const promise = new Promise((resolve, reject) => {
 +      if (signal?.aborted) {
 +        // Unexpected, cancel immediately.
 +        reject(new DomainFrontRequestCancelledError(url));
 +        return;
 +      }
 -    // Make request
 -    const listener = new ResponseListener();
 -    await ch.asyncOpen(listener, ch);
 +      let ch = null;
++
 +      if (signal) {
 +        cancel = () => {
 +          // Reject prior to calling cancel, since we want to ignore any error
 +          // responses from ResponseListener.
 +          // NOTE: In principle we could let ResponseListener throw this error
 +          // when it receives NS_ERROR_ABORT, but that would rely on mozilla
 +          // never calling this error either.
 +          reject(new DomainFrontRequestCancelledError(url));
 +          ch?.cancel(Cr.NS_ERROR_ABORT);
 +        };
 +        signal.addEventListener("abort", cancel);
 +      }
 -    // wait for response
 -    return listener.response();
 +      ch = this.buildHttpHandler(url);
++
 +      const inStream = Cc[
 +        "@mozilla.org/io/string-input-stream;1"
 +      ].createInstance(Ci.nsIStringInputStream);
 +      inStream.setData(body, body.length);
 +      const upChannel = ch.QueryInterface(Ci.nsIUploadChannel);
 +      upChannel.setUploadStream(inStream, contentType, body.length);
 +      ch.requestMethod = method;
++
 +      // Make request
 +      const listener = new ResponseListener();
 +      ch.asyncOpen(listener);
 +      listener.response.then(
 +        body => {
 +          resolve(body);
 +        },
 +        error => {
 +          reject(error);
 +        }
 +      );
 +    });
 +    // Clean up. Do not return this `Promise.finally` since the caller should
 +    // not depend on it.
 +    // We pre-catch and suppress all errors for this `.finally` to stop the
 +    // errors from being duplicated in the console log.
 +    promise
 +      .catch(() => {})
 +      .finally(() => {
 +        // Remove the callback for the AbortSignal so that it doesn't hold onto
 +        // our channel reference if the caller continues to hold a reference to
 +        // AbortSignal.
 +        if (cancel) {
 +          signal.removeEventListener("abort", cancel);
 +        }
 +      });
 +    return promise;
+   }
+ }

@@ -570,6 +570,7 @@ class AutoBootstrapAttempt {
          return;
+       }
 +      let moatAbortController = new AbortController();
        // For now, throw any errors we receive from the backend, except when it
        // was unable to detect user's country/region.
        // If we use specialized error objects, we could pass the original errors
@@ -577,11 +578,20 @@ class AutoBootstrapAttempt {
        const maybeSettings = await Promise.race([
          moat.circumvention_settings(
            [...lazy.TorSettings.builtinBridgeTypes, "vanilla"],
 -          options.regionCode === "automatic" ? null : options.regionCode
 +          options.regionCode === "automatic" ? null : options.regionCode,
 +          moatAbortController.signal
          ),
          // This might set maybeSettings to undefined.
          this.#cancelledPromise,
        ]);
 +      if (this.#cancelled) {
 +        // Ended early due to being cancelled. Abort the ongoing Moat request so
 +        // that it does not continue unnecessarily in the background.
 +        // NOTE: We do not care about circumvention_settings return value or
 +        // errors at this point. Nor do we need to await its return. We just
 +        // want it to resolve quickly.
 +        moatAbortController.abort();
 +      }
        if (this.#cancelled || this.#resolved) {
          return;
+       }
@@ -591,15 +601,25 @@ class AutoBootstrapAttempt {
        if (maybeSettings?.bridgesList?.length) {
          this.#bridgesList = maybeSettings.bridgesList;
        } else {
 +        // In principle we could reuse the existing moatAbortController
 +        // instance, since its abort method has not been called. But it is
 +        // cleaner to use a new instance to avoid triggering any potential
 +        // lingering callbacks attached to the AbortSignal.
 +        moatAbortController = new AbortController();
          // Keep consistency with the other call.
          this.#bridgesList = await Promise.race([
 -          moat.circumvention_defaults([
 -            ...lazy.TorSettings.builtinBridgeTypes,
 -            "vanilla",
 -          ]),
 +          moat.circumvention_defaults(
 +            [...lazy.TorSettings.builtinBridgeTypes, "vanilla"],
 +            moatAbortController.signal
 +          ),
            // This might set this.#bridgesList to undefined.
            this.#cancelledPromise,
          ]);
 +        if (this.#cancelled) {
 +          // Ended early due to being cancelled. Abort the ongoing Moat request
 +          // so that it does not continue in the background.
 +          moatAbortController.abort();
 +        }
+       }
      } finally {
        // Do not await the uninit.

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

Commits:

6 changed files:

Changes:

@@ -5,13 +5,15 @@
  const lazy = {};
  const log = console.createInstance({
 -  maxLogLevel: "Warn",
    prefix: "Moat",
 +  maxLogLevelPref: "browser.torMoat.loglevel",
  });
  ChromeUtils.defineESModuleGetters(lazy, {
    DomainFrontRequestBuilder:
      "resource://gre/modules/DomainFrontedRequests.sys.mjs",
 +  DomainFrontRequestCancelledError:
 +    "resource://gre/modules/DomainFrontedRequests.sys.mjs",
    TorBridgeSource: "resource://gre/modules/TorSettings.sys.mjs",
  });
@@ -91,6 +93,19 @@ class InternetTestResponseListener {
   * @property {string} [country] - The detected country (region).
   */
 +/**
 + * @typedef {Object} CaptchaChallenge
 + *
 + * The details for a captcha challenge.
 + *
 + * @property {string} transport - The transport type selected by the Moat
 + *   server.
 + * @property {string} image - A base64 encoded jpeg with the captcha to
 + *   complete.
 + * @property {string} challenge - A nonce/cookie string associated with this
 + * request.
 + */
++
  /**
   * Constructs JSON objects and sends requests over Moat.
   * The documentation about the JSON schemas to use are available at
@@ -122,17 +137,51 @@ export class MoatRPC {
      this.#requestBuilder = null;
+   }
 -  async #makeRequest(procedure, args) {
 +  /**
 +   * @typedef {Object} MoatResult
 +   *
 +   * The result of a Moat request.
 +   *
 +   * @property {any} response - The parsed JSON response from the Moat server,
 +   *   or `undefined` if the request was cancelled.
 +   * @property {boolean} cancelled - Whether the request was cancelled.
 +   */
++
 +  /**
 +   * Make a request to Moat.
 +   *
 +   * @param {string} procedure - The name of the procedure.
 +   * @param {object} args - The arguments to pass in as a JSON string.
 +   * @param {AbortSignal} [abortSignal] - An optional signal to be able to abort
 +   * the request early.
 +   * @returns {MoatResult} - The result of the request.
 +   */
 +  async #makeRequest(procedure, args, abortSignal) {
      const procedureURIString = `${Services.prefs.getStringPref(
        TorLauncherPrefs.moat_service
      )}/${procedure}`;
 -    return JSON.parse(
 -      await this.#requestBuilder.buildRequest(procedureURIString, {
 -        method: "POST",
 -        contentType: "application/vnd.api+json",
 -        body: JSON.stringify(args),
 -      })
 -    );
 +    log.info(`Making request to ${procedureURIString}:`, args);
 +    let response = undefined;
 +    let cancelled = false;
 +    try {
 +      response = JSON.parse(
 +        await this.#requestBuilder.buildRequest(procedureURIString, {
 +          method: "POST",
 +          contentType: "application/vnd.api+json",
 +          body: JSON.stringify(args),
 +          signal: abortSignal,
 +        })
 +      );
 +      log.info(`Response to ${procedureURIString}:`, response);
 +    } catch (e) {
 +      if (abortSignal && e instanceof lazy.DomainFrontRequestCancelledError) {
 +        log.info(`Request to ${procedureURIString} cancelled`);
 +        cancelled = true;
 +      } else {
 +        throw e;
 +      }
 +    }
 +    return { response, cancelled };
+   }
    async testInternetConnection() {
@@ -147,15 +196,16 @@ export class MoatRPC {
      return listener.status;
+   }
 -  // Receive a CAPTCHA challenge, takes the following parameters:
 -  // - transports: array of transport strings available to us eg: ["obfs4", "meek"]
 -  //
 -  // returns an object with the following fields:
 -  // - transport: a transport string the moat server decides it will send you selected
 -  //   from the list of provided transports
 -  // - image: a base64 encoded jpeg with the captcha to complete
 -  // - challenge: a nonce/cookie string associated with this request
 -  async fetch(transports) {
 +  /**
 +   * Request a CAPTCHA challenge.
 +   *
 +   * @param {string[]} transports - List of transport strings available to us
 +   *   eg: ["obfs4", "meek"].
 +   * @param {AbortSignal} abortSignal - A signal to abort the request early.
 +   * @returns {?CaptchaChallenge} - The captcha challenge, or `null` if the
 +   *   request was aborted by the caller.
 +   */
 +  async fetch(transports, abortSignal) {
      if (
        // ensure this is an array
        Array.isArray(transports) &&
@@ -173,7 +223,15 @@ export class MoatRPC {
            },
          ],
        };
 -      const response = await this.#makeRequest("fetch", args);
 +      const { response, cancelled } = await this.#makeRequest(
 +        "fetch",
 +        args,
 +        abortSignal
 +      );
 +      if (cancelled) {
 +        return null;
 +      }
++
        if ("errors" in response) {
          const code = response.errors[0].code;
          const detail = response.errors[0].detail;
@@ -189,18 +247,17 @@ export class MoatRPC {
      throw new Error("MoatRPC: fetch() expects a non-empty array of strings");
+   }
 -  // Submit an answer for a CAPTCHA challenge and get back bridges, takes the following
 -  // parameters:
 -  // - transport: the transport string associated with a previous fetch request
 -  // - challenge: the nonce string associated with the fetch request
 -  // - solution: solution to the CAPTCHA associated with the fetch request
 -  // - qrcode: true|false whether we want to get back a qrcode containing the bridge strings
 -  //
 -  // returns an object with the following fields:
 -  // - bridges: an array of bridge line strings
 -  // - qrcode: base64 encoded jpeg of bridges if requested, otherwise null
 -  // if the provided solution is incorrect, returns an empty object
 -  async check(transport, challenge, solution, qrcode) {
 +  /**
 +   * Submit an answer for a previous CAPTCHA fetch to get bridges.
 +   *
 +   * @param {string} transport - The transport associated with the fetch.
 +   * @param {string} challenge - The nonce string associated with the fetch.
 +   * @param {string} solution - The solution to the CAPTCHA.
 +   * @param {AbortSignal} abortSignal - A signal to abort the request early.
 +   * @returns {?string[]} - The bridge lines for a correct solution, or `null`
 +   *   if the solution was incorrect or the request was aborted by the caller.
 +   */
 +  async check(transport, challenge, solution, abortSignal) {
      const args = {
        data: [
+         {
@@ -210,25 +267,30 @@ export class MoatRPC {
            transport,
            challenge,
            solution,
 -          qrcode: qrcode ? "true" : "false",
 +          qrcode: "false",
          },
        ],
      };
 -    const response = await this.#makeRequest("check", args);
 +    const { response, cancelled } = await this.#makeRequest(
 +      "check",
 +      args,
 +      abortSignal
 +    );
 +    if (cancelled) {
 +      return null;
 +    }
++
      if ("errors" in response) {
        const code = response.errors[0].code;
        const detail = response.errors[0].detail;
        if (code == 419 && detail === "The CAPTCHA solution was incorrect.") {
 -        return {};
 +        return null;
+       }
        throw new Error(`MoatRPC: ${detail} (${code})`);
+     }
 -    const bridges = response.data[0].bridges;
 -    const qrcodeImg = qrcode ? response.data[0].qrcode : null;
+-
 -    return { bridges, qrcode: qrcodeImg };
 +    return response.data[0].bridges;
+   }
    /**
@@ -296,15 +358,24 @@ export class MoatRPC {
     * @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.
 +   * @param {AbortSignal} abortSignal - A signal to abort the request early.
     * @returns {?MoatSettings} - The returned settings from the server, or `null`
 -   *   if the region could not be determined by the server.
 +   *   if the region could not be determined by the server or the caller
 +   *   cancelled the request.
     */
 -  async circumvention_settings(transports, country) {
 +  async circumvention_settings(transports, country, abortSignal) {
      const args = {
        transports: transports ? transports : [],
        country,
      };
 -    const response = await this.#makeRequest("circumvention/settings", args);
 +    const { response, cancelled } = await this.#makeRequest(
 +      "circumvention/settings",
 +      args,
 +      abortSignal
 +    );
 +    if (cancelled) {
 +      return null;
 +    }
      let settings = {};
      if ("errors" in response) {
        const code = response.errors[0].code;
@@ -334,7 +405,11 @@ export class MoatRPC {
    // settings for.
    async circumvention_countries() {
      const args = {};
 -    return this.#makeRequest("circumvention/countries", args);
 +    const { response } = await this.#makeRequest(
 +      "circumvention/countries",
 +      args
 +    );
 +    return response;
+   }
    // Request a copy of the builtin bridges, takes the following parameters:
@@ -347,7 +422,7 @@ export class MoatRPC {
      const args = {
        transports: transports ? transports : [],
      };
 -    const response = await this.#makeRequest("circumvention/builtin", args);
 +    const { response } = await this.#makeRequest("circumvention/builtin", args);
      if ("errors" in response) {
        const code = response.errors[0].code;
        const detail = response.errors[0].detail;
@@ -366,13 +441,22 @@ export class MoatRPC {
     * 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.
 +   * @param {AbortSignal} abortSignal - A signal to abort the request early.
 +   * @returns {?MoatBridges[]} - The list of bridges found, or `null` if the
 +   *   caller cancelled the request.
     */
 -  async circumvention_defaults(transports) {
 +  async circumvention_defaults(transports, abortSignal) {
      const args = {
        transports: transports ? transports : [],
      };
 -    const response = await this.#makeRequest("circumvention/defaults", args);
 +    const { response, cancelled } = await this.#makeRequest(
 +      "circumvention/defaults",
 +      args,
 +      abortSignal
 +    );
 +    if (cancelled) {
 +      return null;
 +    }
      if ("errors" in response) {
        const code = response.errors[0].code;
        const detail = response.errors[0].detail;