Closed Bug 1642723 Opened 4 years ago Closed 4 years ago

Pref-off automatic TRR-selection by default e.g. so it can be controlled by Normandy

Categories

(Firefox :: Security, task, P1)

task

Tracking

()

RESOLVED FIXED
Firefox 79
Tracking Status
relnote-firefox --- 77+
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 blocking fixed
firefox78 + fixed
firefox79 + fixed

People

(Reporter: nhnt11, Assigned: nhnt11)

References

Details

Attachments

(3 files)

We need to be able to roll this out gradually so that we don't overload any providers. Even the dry-run involves up to 10 requests per client which can be very significant when the entire release population updates.

Assignee: nobody → nhnt11
Status: NEW → ASSIGNED

The attached patch is built on mozilla-release. Patch for central/beta coming later.

Comment on attachment 9153521 [details]
Bug 1642723 - Pref-off automatic TRR-selection by default. r=johannh!,mythmon!,valentin!

Beta/Release Uplift Approval Request

  • User impact if declined: This prefs-off a feature that seems to be effectively DDoS'ing NextDNS, one of our DNS over HTTPs providers. This patch is blocking the rollout of Fx77.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This patch is not a simple visual change, which to me rules out the "Low" risk category. However, all it does is cause a code path to return early if a pref is false, which it is by default, and the code path in question is not responsible for anything that could cause breakage.
  • String changes made/needed: None
Attachment #9153521 - Flags: approval-mozilla-release?
Flags: qe-verify+

Note that the patch on Phabricator is based on mozilla-release and we still need separate patches for beta and central.

Comment on attachment 9153521 [details]
Bug 1642723 - Pref-off automatic TRR-selection by default. r=johannh!,mythmon!,valentin!

Approved for 77.0.1

Attachment #9153521 - Flags: approval-mozilla-release? → approval-mozilla-release+

Added to the 77.0.1 relnotes:

Disabled automatic selection of DNS over HTTPS providers during a test to enable wider deployment in a more controlled way

QA Whiteboard: [qa-triaged]

Pref-off automatic TRR-selection by default
Firefox Release 76.0.1, Firefox Release 77.0.1

We have finished verifying Bug 1642723.

Quality status: GREEN

Why is this Sign Off green?

  • During testing we have not encountered any issues.

Testing Summary:

  • Test Suite: TestRail
  • We did not manage to test the natural browser update flow from 76.0.1 or 77.0 to the 77.0.1 dot release. However, we tried enrolling in the Normandy Stage recipe in Firefox 76 and updating to 77.0.1 by manually overwriting the installation. Using this scenario after opening the profile using Firefox 77.0.1 the rollout was marked as graduated and the profile was unenrolled.
  • As a workaround we enrolled a profile in the rollout using Firefox Release 76.0.1 and then opened the same profile with Firefox Release 77.0.1 without overwriting the installation and managed to test the scenario.
  • We did not manage to capture relevant data by using Wireshark.

Tested Platforms and browser versions:

  • Firefox Release 77.0.1 en-US using Windows 10 x64
  • Firefox Release 77.0.1 en-US using macOS 10.15
  • Firefox Release 77.0.1 es-MX using Ubuntu Linux 18.04
  • Firefox Release 77.0.1 en-US using Windows 8 x64
  • Firefox Release 77.0.1 es-MX using Windows 7 x64
  • Firefox Release 76.0.1 en-US using Windows 10 x64
  • Firefox Release 76.0.1 en-US using macOS 10.15
  • Firefox Release 76.0.1 es-MX using Ubuntu Linux 18.04
  • Firefox Release 76.0.1 en-US using Windows 8 x64
  • Firefox Release 76.0.1 es-MX using Windows 7 x64
Attachment #9153936 - Attachment description: Bug 1642723 - Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=johannh!,mythmon,valentin → Bug 1642723 - Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=valentin!,johannh
Attachment #9153937 - Attachment description: Bug 1642723 - Also check doh-rollout.uri in browser_trrSelection_disable.js test. r=johannh! → Bug 1642723 - Also check doh-rollout.uri in browser_trrSelection_disable.js test. r=valentin!

Valentin can you get to this review today?

Flags: needinfo?(valentin.gosu)

(In reply to Julien Cristau [:jcristau] from comment #13)

Valentin can you get to this review today?

The patch for release was already r+
I just did the other patches too.

Flags: needinfo?(valentin.gosu)

Comment on attachment 9153936 [details]
Bug 1642723 - Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=valentin!,johannh

Beta/Release Uplift Approval Request

  • User impact if declined: Automatic TRR selection code can trigger a bug that can cause the client to DoS proxies, DoH providers, etc. in some scenarios. (Also, this patch is necessary to maintain compatibility across releases - it already landed in m-r)
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: I would say no because this has already been QA'd and dot-released in 77.0.1 and this patch simply back-ports to Nightly and Beta.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Already QA'd and dot-released in 77.0.1
  • String changes made/needed: none
Attachment #9153936 - Flags: approval-mozilla-beta?
Attachment #9153937 - Flags: approval-mozilla-beta?
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0fd2e1828e89
Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=valentin
https://hg.mozilla.org/integration/autoland/rev/2a72043f1fca
Also check doh-rollout.uri in browser_trrSelection_disable.js test. r=valentin

Comment on attachment 9153936 [details]
Bug 1642723 - Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=valentin!,johannh

this is already in 77.0.1, approved for 78.0b3

Attachment #9153936 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9153937 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79
Regressions: 1643520
You need to log in before you can comment on or make changes to this bug.