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

Status: RESOLVED FIXED

(Firefox :: Security, task, P1)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

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.

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1642723 - Pref-off automatic TRR-selection by default. r=johannh!,mythmon!,valentin!

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cd54615056efa57ca8d3ec0deddfb6c60133c94

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 77.0.1

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/b8d272601020250ac2897e113b3fc3b53b72cf4b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Patriciu Pop [:ppop], Ecosystem QA]

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

Comment 10

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1642723 - Pref-off automatic TRR-selection by default (back-port from mozilla-release). r=valentin!,johannh

Comment 11

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1642723 - Also check doh-rollout.uri in browser_trrSelection_disable.js test. r=valentin!

Depends on D78126

Comment 12

[(inactive) Nihanth Subramanya [:nhnt11]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59d1ba044cea650651a4067717dffe89922acf9f

Comment 13

[Julien Cristau [:jcristau]]

Valentin can you get to this review today?

Comment 14

[Valentin Gosu [:valentin] (he/him)]

(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.

Comment 15

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
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 17

[Julien Cristau [:jcristau]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a6f31328e670
https://hg.mozilla.org/releases/mozilla-beta/rev/a496c7db9cee

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/0fd2e1828e89
https://hg.mozilla.org/mozilla-central/rev/2a72043f1fca