1608713 - Captive portal login tab should setTRRMode(Ci.nsIRequest.TRR_DISABLED_MODE)

Status: VERIFIED FIXED

(Firefox :: Untriaged, defect)

Description

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

In Bug 1552176 we added a mechanism to nsIRequest to have a different TRR mode for each individual HTTP request.
This should prove useful for people running TRR in mode3, but who connect to a WiFi network that requires login and is hosted on a domain such as captive.domain.com instead of a network ip 10.0.5.2.

While for an IP we don't need to resolve a domain name, if the captive portal server is hosted on a certain domain (even if it's located in the local network but requires DNS), what would happen is that the captive portal detector sees a captive portal, we open a new tab to the captive portal endpoint, but since the new tab uses a regular channel with TRR mode 3 it will fail to resolve.

So we need that the tab that is specifically opened for the captive portal login to be in TRR mode 0. I initially designed the API so that we could setTRRMode on all necessary channels but it seems to me that it's easier just to set the nsIDocShell.defaultLoadFlags |= 1 << 3 (maybe I should add a named constant for this in the IDL) and it should be applied to all requests within the docshell.

Nihanth, what do you think?

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

I agree that we should do this. As for the implementation strategy - can you describe the mechanism a bit more? When do we set defaultLoadFlags? Do you have an example of setting a bit on the DocShell like this when opening a new tab and before the load happens?

Comment 2

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

Whoops, I think I had an answer written in some tab, but I forgot to submit it?

In any case, here I go again:
The defaultLoadFlags from the docShell get passed down to the loadGroup and end up being set on all channels in that loadGroup: [1]
The defaultLoadFlags should be set before we load the page. Example usage: [2]

I've been looking at browser-captivePortal.js, and I think adding another param to addWebTab then doing something like in addTab:

if (disableTRR) {
  b.docShell.defaultLoadFLags = 1 << 3;
}

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1608713 - Add nsIWebNavigation load flags to force TRR mode and use them to load captive portal tabs with TRR disabled. r=valentin!,Gijs!,baku!

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

Having made this patch, I am no longer sure this is the right approach, from a privacy perspective. If the user has enabled TRR mode 3 and we disrespect it for the captive portal tab, with the current patch, any loads in that tab within the lifecycle of the docshell will leak to the "default" DNS provider.

I wonder if a more appropriate solution is to put the captive portal tab in a throwaway container with TRR disabled on it. This will lead to UX problems though - how do we tell the user not to keep using that tab once they're logged in? etc.

I wonder if this patch needs to do some more magic to ensure that only the initial load obeys the directive to disable TRR. Maybe this is already the case, but I'm realizing that I don't actually know the lifecycle of the docshell and the default load flags well enough to make that assertion.

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Hmm, it might be viable to wait until the portal is unlocked and then reset the flag on the docshell.

Comment 6

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

(In reply to Nihanth Subramanya [:nhnt11] from comment #5)

Hmm, it might be viable to wait until the portal is unlocked and then reset the flag on the docshell.

If that's possible I think it would be really great! It seems like it should be. The code that normally closes the tab should be able to do that.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Let's land this for now.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb13b5c6742c
Add nsIWebNavigation load flags to force TRR mode and use them to load captive portal tabs with TRR disabled. r=valentin,Gijs,baku

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/eb13b5c6742c

Comment 10

[Daniel Cicas [:dcicas], Release QA]

Hello,

I can confirm that this issue is is fixed on the latest Firefox Nightly!