1888494 - Increased Protection mode for DNS-over-HTTPS (DoH) breaks captive portals

Status: VERIFIED FIXED

(Core :: Networking: DNS, defect, P2)

Description

[Ted Campbell [:tcampbell]]

I've been having a lot of trouble with captive portals being detected by firefox, but when I click the button to view portal page it never loads. I've traced this down to having DoH configured to "Increased Protection" or higher. When Win11 captive portal detection asks to open in default browser (Firefox) I also get stuck.

The work around is to go to another browser to approve the portal which is a poor user experience. Perhaps we can have a better flow here similar to the work we did for HTTPS-Only mode.

Comment 1

[Sunil Mayya]

Valentin,
Are aware of any existing captive portal issues that might cause this?

Comment 2

[Ted Campbell [:tcampbell]]

STR:
0) Randomize your WiFi MAC in Windows to "forget" previous captive login

1. Enable "Increased Protection" DoH mode
2. Close Firefox (so that it bootstraps again on next startup)
3. Connect to "Hilton Honors" WiFi
4. Start Firefox and wait a few seconds for captive portal prompt in Firefox to appear under address bar
5. Click the button in prompt to login into captive portal

Expected:
A new tab with captive portal loads

Actual:
New tab is blank and loads forever

---

Additionally, after even longer waiting the Windows captive portal detection kicks in and it tries to use http://www.msftconnecttest.com/redirect as the portal which does actually resolve to the portal page. Perhaps the special handling of the Firefox Captive Portal in TRR is acting funny?

Comment 3

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

Theoretically, Increased protection should cause it to fallback to native DNS if it fails. And even if it doesn't, if you open the captive portal window via the notification banner, everything inside the window should not use DoH.

Could you maybe capture a profile in about:logging with the Networking preset and send the link to [e-mail]?
If you can do that with a new profile so no private information is captured it would be even better. Thanks!

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Ted, any chance of logs?

Comment 5

[Ted Campbell [:tcampbell]]

Sorry, I must not have finished capturing the process logs before I left the hotel and that laptop is no more.

Comment 6

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

I actually had the chance to test this in the airport before I left.
It seems that the mechanism to set the TRR mode for the captive portal login page got broken at some point in the past, but we didn't notice because we didn't have a test for it.
I managed to write one up, which I'll be posting soon. And I'll try to bisect to figure out when this got regressed.

Comment 7

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

I did a mozregression and it seems this broke quite a while ago:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a34695d9b99d1e9098e846b751c9adf1f52ee760&tochange=bc0932b38478abd600cae2b23c5f78381e80d0e8

Most likely in bug 1633310.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1602318

Comment 9

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

Attachment: Bug 1888494 - Make CanonicalBrowsingContext::FixupAndLoadURIString check set the correct TRR mode r=nika

It would seem sometimes in Firefox 77 we regressed the ability to set the TRR
mode for a browsing context when opening a new tab.
I confirmed that this bug didn't happen in Fx 77 when setting the
browser.tabs.documentchannel.parent-initiated pref to false.

The nsIWebNavigation::LOAD_FLAGS_DISABLE_TRR is correctly passed into
CanonicalBrowsingContext, but it doesn't end up getting used.

This sets the appropriate DefaultLoadFlags for the CanonicalBrowsingContext
when the LOAD_FLAGS_DISABLE_TRR or LOAD_TRR_ONLY_MODE flags are present.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b4b76ae15d7
Make CanonicalBrowsingContext::FixupAndLoadURIString check set the correct TRR mode r=nika

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1602318

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/9b4b76ae15d7

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta and ESR128 approval when you get a chance.

Comment 14

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

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)

Please nominate this for Beta and ESR128 approval when you get a chance.

I wrote a test for it in bug 1913692 which landed along with this patch.

Comment 15

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

Attachment: Bug 1888494 - Make CanonicalBrowsingContext::FixupAndLoadURIString check set the correct TRR mode r=nika

It would seem sometimes in Firefox 77 we regressed the ability to set the TRR
mode for a browsing context when opening a new tab.
I confirmed that this bug didn't happen in Fx 77 when setting the
browser.tabs.documentchannel.parent-initiated pref to false.

The nsIWebNavigation::LOAD_FLAGS_DISABLE_TRR is correctly passed into
CanonicalBrowsingContext, but it doesn't end up getting used.

This sets the appropriate DefaultLoadFlags for BrowsingContext
when the LOAD_FLAGS_DISABLE_TRR or LOAD_TRR_ONLY_MODE flags are present
in nsDocShellLoadState::LoadFlags()

Original Revision: https://phabricator.services.mozilla.com/D220550

Comment 16

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

Steps to reproduce:

• Set network.trr.exclude-etc-hosts to false.
• Set network.trr.mode to 2 or 3
• Start a local server to simulate the captive portal on port 80
• add 127.0.0.1 detectportal.firefox.com to /etc/hosts
• restart firefox
• click on the captive portal banner
• check that the captive portal opens a page from the local server (the public one redirects to https://support.mozilla.org/en-US/kb/captive-portal)
• remember to clear the prefs and /etc/hosts after the test

Comment 17

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Users using DoH in increased/max protection modes (2 and 3) might have difficulty in logging into the captive portal.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: See comment 16
• Risk associated with taking this patch: Low
• Explanation of risk level: Fixes regressed behaviour.
• String changes made/needed: no
• Is Android affected?: no

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/448346682db7

Comment 19

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! I could not reproduce the issue by following the steps from comment 16. By adding 127.0.0.1 detectportal.firefox.com to the /etc/hosts file, starting a Python HTTP server on port 80, and adjusting the prefs inside Firefox, Firefox will not display the captive portal banner after a restart.However I have a router set with captive portal and I could reproduce the issue with Firefox 130.0 on Windows 10x64 but only by setting DoH to Max Protection (network.trr.mode to 3). After Firefox is reopened clicking the Open network login page button from the captive portal banner will open a new blank tab with infinite loading. Note that the issue does not reproduce for me with Increased Protection (network.trr.mode to 2) on an affected build.

I followed the next steps to reproduce/ verify the issue (Windows 10x64 and Ubuntu 24.04 are desktop machines with USB Wifi stick):

1. Set Firefox DoH to Max Protection in about:preferences#privacy (network.trr.mode:3)
2. Set the captive portal router and disable the ethernet adapter.
3. Connect to Captive portal Wifi and close Firefox.
4. Open Firefox and click the Open network login page button from the captive portal banner.
   Expected Result: The captive portal page is opened (no infinite loading in a blank tab)

Following the above steps I can no longer reproduce the issue with Firefox 131.0b4 and 132.0a1 (2024-09-09) on Windows 10x64, macOS 12 and Ubuntu 24. Verified with Increase Protection and Max Protection. Just to be sure is the above verification correct giving the fact that I could only reproduce the issue with network.trr.mode:3? Thank you!

Comment 20

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

Hi Alexandru, thank you for checking.

I think what was missing from my steps list was making sure there's a canonical.html file served by the webserver with arbitrary content.

I think testing with a real captive portal device in mode3 is enough to confirm this is fixed. Thanks!

Comment 21

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #20)

Hi Alexandru, thank you for checking.

I think what was missing from my steps list was making sure there's a canonical.html file served by the webserver with arbitrary content.

I think testing with a real captive portal device in mode3 is enough to confirm this is fixed. Thanks!

Thank you! Setting flags accordingly.

Comment 22

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

Attachment: Bug 1888494 - Make CanonicalBrowsingContext::FixupAndLoadURIString check set the correct TRR mode r=nika

It would seem sometimes in Firefox 77 we regressed the ability to set the TRR
mode for a browsing context when opening a new tab.
I confirmed that this bug didn't happen in Fx 77 when setting the
browser.tabs.documentchannel.parent-initiated pref to false.

The nsIWebNavigation::LOAD_FLAGS_DISABLE_TRR is correctly passed into
CanonicalBrowsingContext, but it doesn't end up getting used.

This sets the appropriate DefaultLoadFlags for BrowsingContext
when the LOAD_FLAGS_DISABLE_TRR or LOAD_TRR_ONLY_MODE flags are present
in nsDocShellLoadState::LoadFlags()

Original Revision: https://phabricator.services.mozilla.com/D220550

Comment 23

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Users using DoH in increased/max protection modes (2 and 3) might have difficulty in logging into the captive portal.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: See comment 16 and 20
• Risk associated with taking this patch: low.
• Explanation of risk level: The code has been on Nightly/Beta for more than a month. No regressions seen in Release 131.
• String changes made/needed: None
• Is Android affected?: no

Comment 24

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/6b7ed074fcee

Comment 25

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Verified fixed with Firefox 128.4.0esr by using steps from comment 19 on Windows 10x64, macOS 12 and Ubuntu 24.04 and on Windows 10x64 by using steps from comment 16 (network.trr.mode 2 and 3). The captive portal page is correctly opened.