Skip to content
Snippets Groups Projects

TorSettings handles failures to apply Tor settings

Merge Info

Issues

Resolves

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
    • :warning: 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 (fixes CVEs, 0-days, etc)
  • 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

  • Security update: patchset contains a security fix (be sure to select the correct item in Timeline)
  • 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

Uplifting

  • Patchset is a candidate for uplift to Firefox

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, 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

TorProvider now exposes three separate methods for applying bridge settings, proxy settings and firewall settings. TorSettings now sends out an ApplyError signal whenever one of these methods fails.

On desktop we set up a listener for the ApplyError signal. This will show a modal dialog for the application window. We do not show the notification in the about:preferences page because we can still get errors outside of about:preferences.

TorSettings also keeps track of whether we have successfully applied any settings to the provider in this session. If we have, the user will have the option to reset their settings to the last successfully applied settings. Otherwise they will have the option to clear their bad settings.

In addition, we also add some further verification to the "Advanced > Settings" dialog to further avoid these errors. This should be further addressed in #43467. As part of this, we no longer accept string values for the proxy.port and firewall.allowed_ports settings. And we require that the proxy usernanme and password are either both empty or both filled.

How Tested

Testing notification

I know of two ways to get the notification:

  1. Enter a bridge 0:0.
  2. Enter a proxy address a.

I don't know of a way to have the firewall settings be rejected. I've artificially caused this part to throw to test the UI.

If this occurs mid-session you will be shown the "Undo changes" option. If it occurs at startup you will be shown the "Clear" option instead.

Testing Moat bridges

Set torbrowser.debug.censorship_level to 1. Enter a bridge line 0:0 and select "Fix myself" in the dialog.

Then start auto bootstrapping and cancel. TorSettings should send out a warning about clearing the temporary bridges and the dialog should re-show.

The reason we re-show the notification is because the user should have addressed the original issue before they bootstrapped again. This is only meant to capture the case where we fail to re-apply our bridge settings for whatever other reason.

Testing switching providers
  1. Kill and restart the tor process with good settings. Will not show anything.
  2. Kill the tor process, change to some good settings and restart the tor process. Will not show anything and the new settings will be shown in torrc.
  3. Kill the tor process, change to some bad settings and restart the tor process. No notification will be shown. Restart the tor process and the notification will show "Clear" rather than "Undo".
  4. Start with bad settings and kill and restart the tor process. On restart the notification will be shown.
  5. Start with bad settings and kill the tor process. Fix the settings. Restart the tor process and no notification will be shown.
Testing advanced settings dialog
  1. Proxy inputs are disabled until the checkbox is ticked and a type is selected.
  2. Firewall input is disabled until the checkbox is ticked.
  3. Unchecking the box will not clear the inputs.
  4. Selecting "SOCKS4" will clear the username and password inputs and disable them.
  5. Entering "example.org:443" in the proxy address and pressing Tab will move the "443" part to the port input.
  6. Cannot press "OK" if a required input is empty.
  7. Cannot press "OK" if the ports are out of range. "0" or "70000".
  8. Cannot press "OK" if a username or password is given, but not the other.
  9. Cannot press "OK" if the firewall ports entry has the wrong format. E.g. 9,d or 9 8.
  10. Settings are saved between sessions.
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
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading