Open Bug 1315966 Opened 8 years ago Updated 2 years ago

CaptivePortalDetector should propagate login page URL when it's available

Categories

(Toolkit :: General, defect, P5)

defect

Tracking

()

Tracking Status
firefox51 --- unaffected
firefox52 --- fix-optional

People

(Reporter: nhnt11, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fxprivacy])

We check for a captive portal by attempting to load a canonical URL, and seeing if it redirects, or if we get an unexpected response. If it redirects, it's almost certainly to the login page - and the URL of this page is available to use via xhr.responseURL. We should propagate this in the captive-portal-login notification.
(In reply to Nihanth Subramanya [:nhnt11] from comment #0)
> We check for a captive portal by attempting to load a canonical URL, and
> seeing if it redirects, or if we get an unexpected response. If it
> redirects, it's almost certainly to the login page - and the URL of this
> page is available to use via xhr.responseURL. We should propagate this in
> the captive-portal-login notification.

I think this is making assumptions about how captive portals work so I would rather avoid this unless we really need it. What problem are you trying to solve?

One concern is that I've seen many captive portals redirect/load multiple URLs before and after login for things like SSO. An HTTP redirect isn't always used either.

Another problem I see is that the user may be held captive but not actually need to login due to the use of cookies so the last URL may not actually be a login page depending on whether the user already had an existing session.

e.g. canonicalURL => portal.a.com => login.b.com => (a.com or b.com indicated the user was already authorized and the user is no longer captive despite not taking any action) => airport.website

With your proposal we may report the airport website as a login page even when it's not. It was actually one of the intermediate redirects that freed the portal.
(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #1)
> I think this is making assumptions about how captive portals work so I would
> rather avoid this unless we really need it. What problem are you trying to
> solve?
> 
> One concern is that I've seen many captive portals redirect/load multiple
> URLs before and after login for things like SSO. An HTTP redirect isn't
> always used either.
> 
> Another problem I see is that the user may be held captive but not actually
> need to login due to the use of cookies so the last URL may not actually be
> a login page depending on whether the user already had an existing session.
> 
> e.g. canonicalURL => portal.a.com => login.b.com => (a.com or b.com
> indicated the user was already authorized and the user is no longer captive
> despite not taking any action) => airport.website
> 
> With your proposal we may report the airport website as a login page even
> when it's not. It was actually one of the intermediate redirects that freed
> the portal.

As someone implementing a captive portal and recently reviewing all the captive portal detection techniques in use, the most common technique is redirection. If your captive portal doesn't do a redirect, it's safe to assume it's not going to be detected. I know you are making the counter argument that redirection doesn't always equal captive portal. However I think that's also covered by the fact that Firefox would quickly detect a cookie-based authentication triggered by a series of redirects and then hide the notification bar (within 5 seconds). In any case, it's still a captive portal. I'd be happier that my Firefox users ask me what that bar that flashes up for a few seconds is about rather than complaining about SSL errors and the internet being broken.

As a user it would be nice safety feature to see the domain name in Location header of the response. In the case of a chain of redirects, it's probably not worth following it to the end. Waiting for a string of redirects to respond while also avoiding loops could be slow, and open up a can of worms. OSCP certificate checks could be broken in all manner of ways and if Firefox has to cope with that several times during the process due to EV certificates, the user experience could be terrible. I'm not actually sure why Firefox follows the redirect at all at present. Surely it's enough to look at the redirection response headers and stop there?

When the canonicalURL is requested and Firefox follows the redirect, does Firefox send cookies? I don't think it does in latest versions. In such cases the act of requesting canonicalURL in the background wouldn't be able to auto-login the user. Sending cookies with the canonicalURL requests and then checking once more at the end of the process if canonicalURL finally returns HTTP status 200 might be a way to resolve Matthew's concerns while also gaining access to the final login page destination.

On a side note, we had an issue with Firefox when our captive portal landing page actually created a new temporary session each time Firefox sent a request. Then we got effectively DDoS attacked by all the Firefox browsers on the network each creating a new session every 5 seconds that would never be used. We had to put a static page in front with a button to the actual login page just to cope with the fact the background request for canonicalURL didn't include cookies.

I think the moral of the story is that for simplicity, Firefox should just grab the Location header from the initial response in determining if there's a captive portal. Perhaps don't even follow the redirect unless you are prepared to send cookies and tackle all the associated issues highlighted here. There have been a lot of issues with Firefox captive portal detection crashing captive portal platforms, and the most common solution that network administrators seem to use to resolve it is simply to whitelist the cannonicalURL essentially breaking this otherwise really handy feature.
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.