Draft: Allow for TorConnect bootstrap prior to TorSettings initialisation, and cancel bootstraps when bridges change

Merge Info

Issues

Resolves

• #41921 (closed)

Related

Merging

Target Branches

• tor-browser - !fixups to tor-browser-specific commits, new features, security backports
• base-browser and mullvad-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
  • ⚠️ IMPORTANT: Please list the base-browser-specific commits which need to be cherry-picked to the base-browser and mullvad-browser branches here

Target Channels

• Alpha: esr128-14.5
• Stable: esr128-14.0
• Legacy: esr115-13.5

Backporting

Timeline

• No Backport (preferred): patchset for the next major stable
• 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

(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

Issue Tracking

• Link resolved issues with appropriate Release Prep issue for changelog generation

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, morgan
  • localization : henry, pierov
  • macOS : clairehurst, dan
  • nightly builds : boklm
  • rebases/release-prep : dan, ma1, pierov, morgan
  • security : jwilde, ma1
  • signing : boklm, morgan
  • updater : pierov
  • windows : jwilde, morgan
  • misc/other : pierov, morgan

Change Description

This is a large set of changes to achieve two results:

1. TorConnect no longer needs to wait for TorSettings to be able to start a bootstrap. The TorBootstrapRequest already waits for TorProviderBuilder.build, which will wait for TorSettings to be initialised and applied.
2. If a user changes any setting under TorSettings.bridges then we cancel any ongoing bootstrap. More specifically, we request the Start stage, which has the side effect or cancelling any bootstraps.

To achieve this, I made sure that only TorSettings could trigger TorProvider.writeSettings, making it the only caller of this method. In particular, instead of TorConnect calling writeSettings to apply temporary Moat bridges, this is performed in TorSettings instead using the applyTemporaryBridges method. This also allows TorSettings to control the temporary bridges, and drop them if the user applies there own bridges in the mean time.

Moreover, to simplify the implementation in TorSettings, which needed to know when the bridge settings had been changed, I changed the TorSettings API surface to only allow editing the settings via a changeSettings method. By placing all the change logic in a single method we could greatly simplify the module.

In addition to the above, I also made some minor improvements to TorConnect and TorSettings, which aren't really related. They are isolated to their own commits, but they could be split into a separate merge request if desired:

1. Only accept an Array of strings for the bridge_strings property. There was only one existing place that still passed in a plain string value.
2. Add TorSettings.enabled property, similar to TorConnect.enabled, and check this for public API.
3. Allow TorSettings.quickstart.enabled to be read early (prior to TorSettings.init completing). This means that about:torconnect and TorConnect can both access this property early.
4. Fix typo in TorConnectParent signal name. Was torconnect:quickstart-changed, when it should have been torconnect:quickstart-change.
5. Move the prompt_at_startup preference out of TorProvider, and only set it in TorConnect. This means it is only used in one place, and its behaviour is simpler: if bootstrap fails or tor exits we set it to true, and we only set it to false when a bootstrap succeeds.
6. TorProvider.flushSettings is now only called by TorSettings, rather than via about:preferences callback which had no guarentees of being called and did not work for Moat bridges.

How Tested

Since this effects a central component, this will need further testing and re-testing before the final merge. What I've tested so far:

• Normal bootstrap without bridges.
• Normal bootstrap with bridges.
• Toggling the quickstart setting in about:torconnect and about:preferences effects the other.
• Changing bridge settings in one about:preferences tab will trigger changes in the other.
• Bridge dialogs only show "Connect" before bootstrapping.
• Removing a single bridge.
• Removing all bridges.
• Editing bridges.
• Editing advanced connection preferences.
• Reselecting built-in bridges results in the same ordering per session. Changes at the next restart.
• Changing bridge settings during bootstrap attempt or "ChooseRegion" resets the UI to "Start".
• If previous session's bootstrap failed quickstart for the next session will not trigger. Similarly if the tor process was killed mid-session.
• Auto bootstrap will save its settings after it succeeds. For both "Builtin" and "BridgeDB" sources (depending on the selected region).
• Auto bootstrap settings will not be saved if they are cancelled or fail.
• Changing settings will be passed to torrc file.
• With quickstart enabled, about:torconnect immediately shows the "Bootstrapping" stage.
• Editing bridges will cancel normal bootstraps and auto bootstraps.