Skip to content
Snippets Groups Projects

Manage the shown user stage in TorConnect module

Merge Info

Related Issues

Backporting

Timeline

  • Immediate: patchset needed as soon as possible
  • Next Minor Stable Release: patchset that needs to be verified in nightly before backport
  • Eventually: patchset that needs to be verified in alpha before backport
  • No Backport (preferred): patchset for the next major stable

(Optional) Justification

  • Emergency security update: patchset fixes CVEs, 0-days, etc
  • Censorship event: patchset enables censorship circumvention
  • Critical bug-fix: patchset fixes a bug in core-functionality
  • Consistency: patchset which would make development easier if it were in both the alpha and release branches; developer tools, build system changes, etc
  • Sponsor required: patchset required for sponsor
  • Localization: typos and other localization changes that should be also in the release branch
  • Other: please explain

Merging

  • Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports
  • Merge to base-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
    • NOTE: if your changeset includes patches to both base-browser and tor-browser please clearly label in the change description which commits should be cherry-picked to base-browser after merging

Issue Tracking

Review

Request Reviewer

  • Request review from an applications developer depending on modified system:
    • NOTE: if the MR modifies multiple areas, please /cc all the relevant reviewers (since gitlab only allows 1 reviewer)
    • accessibility : henry
    • android : clairehurst, dan
    • build system : boklm
    • extensions : ma1
    • firefox internals (XUL/JS/XPCOM) : jwilde, ma1
    • fonts : pierov
    • frontend (implementation) : henry
    • frontend (review) : donuts, richard
    • localization : henry, pierov
    • macOS : clairehurst, dan
    • nightly builds : boklm
    • rebases/release-prep : dan, ma1, pierov, richard
    • security : jwilde, ma1
    • signing : boklm, richard
    • updater : pierov
    • windows : jwilde, richard
    • misc/other : pierov, richard

Change Description

1. Move bootstrapping attempts into a new class.

We copy the logic from "BootstrappingState" and "AutoBootstrappingState" into "BootstrappingAttempt" and "AutoBootstrappingAttempt". The main difference is that we can do the following:

bootstrapAttempt = new BootstrapAttempt();
bootstrapResult = await bootstrapAttempt.run();
// ...
bootstrapAttempt.cancel();

rather than using the "StateCallback" class, which requires some complicated state management.

Moreover, "AutoBootstrappingAttempt" will use "BootstrappingAttempt" for each of its attempts. So the logic for bootstrapping can be kept in one place.

Some other changes:

  1. Censorship simulation will no longer necessarily avoid the Moat calls, so these can be tested.
  2. It would be possible to perform the internet test when auto-bootstrapping as well.
  3. When auto-bootstrapping, if "Bootstrapping" produces an error other than a "BootstrapError", we can end it early.
  4. No longer set TorConnect internals.
  5. More fine-grained control over censorship simulation and offline simulation.
2. Drop StateCallback from TorConnect.

Instead of managing the abstract "State" we directly manager the user "Stage". We provide backward compatibility with the "State" for android and about:torconnect. Eventually this logic can be dropped from these endpoints and they can listen for changes in the "Stage" instead.

The behaviour for about:torconnect is mostly the same as before, with some exceptions:

  1. If the user sees the "Offline" state, and starts and cancels the bootstrap, they should return the to the "Offline" state, rather than "ConnectToTor".
  2. Trying to start a bootstrap via the UI before the settings have loaded will do nothing.
  3. Pressing a breadcrumb whilst bootstrapping will now also cancel the bootstrap.
3. Switch about:torconnect to use TorConnect.stage to control

the shown stage and sync pages.

Now TorConnect entirely controls which stage should be shown to the user, and "about:torconnect" simply relays the user actions up to TorConnect to handle. In particular, we stop sending out "torconnect:broadcast-user-action" to sync pages.

We also do not try and sync the selected region between pages. However all pages should still show the actually submitted region after a bootstrap fails. E.g. to confirm their location.

We also allow the user to re-select "Automatic" when they use breadcrumbs to go back a stage.

How Tested

Edited by henry

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • assigned to @henry

    • Author Maintainer
      Resolved by henry

      @clairehurst @morgan @pierov here is a preview of the work I have done so far on the TorConnect refactor.

      I'm sharing it now in case you want to see the details of the implementation, but it does not need a review right now.

      Aside from the details within the TorConnect module, from a consumer point of view ("about:torconnect" pages, and android), here's an overview of how it worked before, and how it would ideally work after.

      You might also want to look at the changes to toolkit/components/torconnect/content/aboutTorConnect.js and toolkit/components/torconnect/TorConnectParent.sys.mjs since they contain the changes that the consumers can make. And it hopefully demonstrates that the new approach is simpler for consumers.

      Before

      TorConnect manages its TorConnect.state, which is one of the values in TorConnectState:

        Initial,
        Configuring,
        AutoBootstrapping,
        Bootstrapping,
        Error,
        Bootstrapped,
        Disabled,

      NOTE: Each state may last for a few cycles, but the "Error" state is immediately replaced with "Configuring".

      Each consumer individually manages what user "stage" should be shown. The consumer listens for changes in torconnect:state-change and torconnect:error. The consumer will change the displayed "stage" depending on the current TorConnect.state, sometimes depending on what stage was shown before, or if we got an error signal.

      Each consumer also sometimes makes requests to TorConnect, such as TorConnect.beginBootstrap, which may change the TorConnect.state, and therefore the shown stage.

      On desktop, each "about:torconnect" tab also sends out torconnect:broadcast-user-action to try and sync the shown stage on different tabs. From memory, there were still ways to make to tabs fall out of sync.

      After

      TorConnect manages the user "stage" directly. The stages are named in TorConnectStage:

        Disabled,
        Loading,
        Start,
        Bootstrapping,
        Offline,
        ChooseRegion,
        RegionNotFound,
        ConfirmRegion,
        FinalError,
        Bootstrapped,

      It also exposes TorConnect.stage, which contains the stage name, plus other details that would be useful for displaying the stage:

       * @property {string} name - The name of the stage.
       * @property {number} id - The ID of the stage.
       * @property {string} defaultRegion - The default region to show in the UI.
       * @property {?string} bootstrapTrigger - The trigger that started a bootstrap.
       *   Only set during the "Bootstrapping" stage.
       * @property {?Error} error - The last bootstrapping error.
       * @property {boolean} hasEverFailed - Whether connecting to tor has ever
       *   failed.
       * @property {BootstrappingStatus} bootstrappingStatus - The current
       *   bootstrapping status.

      Consumers should listen to torconnect:stage-change to update the shown stage in the UI.

      Moreover, consumers should not change the shown stage directly for any other reason. Instead, they can make requests to TorConnect (like beginBootstrapping), which will correspondingly handle the change in stage instead, which will then be passed on to the consumer.

      This ensures:

      1. Each consumer easily stays in sync ("about:torconnect" tabs).
      2. The logic of which stage to show is entirely handled within TorConnect, so all consumers should have the same UI flow.

      Ideally, consumers would also pass in the stage id to the public TorConnect methods. This helps ensure that a delayed user action won't effect TorConnect after it has already moved onto another stage.

      Whilst in the "Bootstrapping" stage, the consumer will also receive bootstrap progress updates from torconnect:bootstrap-progress. This should be used to update the progress bar.

      Compatibility

      The new implementation of TorConnect still exposes TorConnect.state and sends out torconnect:state-change and torconnect:error. This should allow consumers to continue to function without switching over to the "stage" instead. I tested this on desktop before switching "about:torconnect" and it worked. I imagine it would be the same on Android, but I've not investigated this thoroughly yet.

  • henry changed title from Draft: Manager the shown user stage in TorConnect module to Draft: Manage the shown user stage in TorConnect module

    changed title from Draft: Manager the shown user stage in TorConnect module to Draft: Manage the shown user stage in TorConnect module

  • This is a great idea. This should've been the way to solve the problem when I realized that the UI and TorConnect couldn't be overlapped.

    I only quickly skimmed through this, and here are some minor details:

    • I'd like to remove/rework the current bootstrap request class, which is a wrapper around Services.obs from before TorProvider. I think the bootstrap workflow should be inside the provider itself (the callback way to handle changes sounds a lot pre-promises and pre-async, but if it's helpful maybe we can just remove the Services.obs part). Anyway, it can be a following change.
    • What about circumvention_countries/getCountryCodes? Is it going to remain as it is? I think that and the localized names are the only thing that will be then missing from the state update (and it's a good idea, since this data is unlikely to change in a session).
    • The third is mostly a curiosity. We aren't consistent in our pages with IPC communication: about:tor uses custom events, whereas for other pages we use RPM functions. Is there a preferred one? Or does it only depend on the needs of the specific page?
  • henry added 356 commits

    added 356 commits

    • 581261c6...9d515b7c - 346 earlier commits
    • 547f60d2 - fixup! Bug 42660: Disable ProxySelector.openConnectionWithProxy and NOPify...
    • 0dfa7bee - Bug 43006: Disable RFP for Font Visibility on Android
    • 8213fc9e - Bug 43098: Year End Campaign 2024.
    • 758ee17f - fixup! Bug 43098: Year End Campaign 2024.
    • dba47080 - Bug_43099: 2024 YEC
    • 947e6879 - fixup! Add CI for Base Browser
    • c86d56c2 - fixup! Add CI for Base Browser
    • afb24cd3 - fixup! Bug 40597: Implement TorSettings module
    • 4e721fec - fixup! Bug 40597: Implement TorSettings module
    • bff2ae60 - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser

    Compare with previous version

  • henry changed target branch from tor-browser-128.3.0esr-14.0-1 to tor-browser-128.4.0esr-14.5-1

    changed target branch from tor-browser-128.3.0esr-14.0-1 to tor-browser-128.4.0esr-14.5-1

  • henry added 15 commits

    added 15 commits

    • bff2ae60...f8491f19 - 5 earlier commits
    • 215c4e03 - fixup! Bug 40597: Implement TorSettings module
    • e5ef4714 - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • 7ae91b4a - fixup! Bug 40597: Implement TorSettings module
    • 8898c3c1 - fixup! Bug 42247: Android helpers for the TorProvider
    • 5343bfd1 - fixup! Lox integration
    • 73a53c94 - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • 3d52c4a8 - fixup! Bug 40597: Implement TorSettings module
    • 65f288d8 - fixup! [android] Enable the connect assist experiments on alpha
    • ecef80e9 - fixup! [android] Add Tor integration and UI
    • 07c94ece - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry added 10 commits

    added 10 commits

    • 43f531fe - fixup! Bug 40597: Implement TorSettings module
    • 48102455 - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • 798b4a31 - fixup! Bug 40597: Implement TorSettings module
    • db1cf7d6 - fixup! Bug 42247: Android helpers for the TorProvider
    • c06467a2 - fixup! Lox integration
    • 74ea398d - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • 77cc621c - fixup! Bug 40597: Implement TorSettings module
    • c4171098 - fixup! [android] Enable the connect assist experiments on alpha
    • 913bf446 - fixup! [android] Add Tor integration and UI
    • 50bcd836 - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry requested review from @pierov

    requested review from @pierov

  • henry marked the checklist item Request review from an applications developer depending on modified system: as completed

    marked the checklist item Request review from an applications developer depending on modified system: as completed

  • henry added 15 commits

    added 15 commits

    • 50bcd836...991439ef - 5 earlier commits
    • 6318a79b - fixup! Bug 40597: Implement TorSettings module
    • cb7b86b1 - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • 9f468884 - fixup! Bug 40597: Implement TorSettings module
    • e9982d69 - fixup! Bug 42247: Android helpers for the TorProvider
    • dc9738b1 - fixup! Lox integration
    • 4a350d6a - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • ccc9efc4 - fixup! Bug 40597: Implement TorSettings module
    • 85c2a5a3 - fixup! [android] Enable the connect assist experiments on alpha
    • a2ffb9b4 - fixup! [android] Add Tor integration and UI
    • f9ad2e43 - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry added 12 commits

    added 12 commits

    • f9ad2e43...50b97d81 - 2 earlier commits
    • d6745e80 - fixup! Bug 40597: Implement TorSettings module
    • d02ceb25 - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • c6676964 - fixup! Bug 40597: Implement TorSettings module
    • 355ffc9f - fixup! Bug 42247: Android helpers for the TorProvider
    • 5644581a - fixup! Lox integration
    • 398cf3fb - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • a80d03f6 - fixup! Bug 40597: Implement TorSettings module
    • e4847c6b - fixup! [android] Enable the connect assist experiments on alpha
    • 958c8622 - fixup! [android] Add Tor integration and UI
    • e691c119 - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry added 12 commits

    added 12 commits

    • e691c119...8284c765 - 2 earlier commits
    • 5b0ceaa0 - fixup! Bug 40597: Implement TorSettings module
    • 600abfcc - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • 781cf5e7 - fixup! Bug 40597: Implement TorSettings module
    • fe770e71 - fixup! Bug 42247: Android helpers for the TorProvider
    • a33a5889 - fixup! Lox integration
    • 7f057354 - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • fc4aed46 - fixup! Bug 40597: Implement TorSettings module
    • 9f530b8c - fixup! [android] Enable the connect assist experiments on alpha
    • 63cc09c8 - fixup! [android] Add Tor integration and UI
    • 60f49b88 - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry added 16 commits

    added 16 commits

    • cc1f52a5 - 1 commit from branch tpo/applications:tor-browser-128.4.0esr-14.5-1
    • cc1f52a5...42484367 - 5 earlier commits
    • ea391ae7 - fixup! Bug 40597: Implement TorSettings module
    • e22374e7 - fixup! Bug 31286: Implementation of bridge, proxy, and firewall settings in...
    • 1cad82e7 - fixup! Bug 40597: Implement TorSettings module
    • bc0bdc93 - fixup! Bug 42247: Android helpers for the TorProvider
    • 85cc2018 - fixup! Lox integration
    • 212ad735 - fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
    • 00a133de - fixup! Bug 40597: Implement TorSettings module
    • ad04bfdc - fixup! [android] Enable the connect assist experiments on alpha
    • 7087f308 - fixup! [android] Add Tor integration and UI
    • 144edec6 - fixup! Temporary changes to about:torconnect for Android.

    Compare with previous version

  • henry marked this merge request as ready

    marked this merge request as ready

  • Pier Angelo Vendrame
  • Pier Angelo Vendrame
  • Pier Angelo Vendrame
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading