816866 - Certificate errors frequently caused by captive portals should trigger captive portal detection

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Devdatta Akhawe [:devd]]

A cert error for HSTS site is likely a captive portal. Firefox should open and connect to a new tab. Chrome has this behavior, but I am sure they have a couple of other heuristics too.

The effort in implementing this should also be useful when we implement support for captive portal detection. (Unless that is high priority and going to get done soon).

Comment 1

[Gervase Markham [:gerv]]

Just for future reference, I think an initial CC list of 46 users might have been a little excessive...

Gerv

Comment 2

[Devdatta Akhawe [:devd]]

Yes. My apologies. I did not think "Clone Bug" would clone the CC list too: I only wanted to clone the product etc.

I am cutting down the CC list, those who care can add themselves back. I am afraid of the spam as everyone removes themselves from the CC list one by one.

Comment 3

[Ian Melven :imelven]

(In reply to Devdatta Akhawe [:devd] from comment #2)
> I am cutting down the CC list, those who care can add themselves back. I am
> afraid of the spam as everyone removes themselves from the CC list one by
> one.

excellent move, thanks.

We've had questions from people asking how to turn off the HSTS preload list or bypass the unbypassable cert exception dialog, due to them either being behind a MITM appliance/proxy or explicitly distrusting certain certs etc. I'm not convinced we can assume an HSTS error == captive portal. Also, I have heard that captive portal detection is something being worked on currently and may be forthcoming.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Devdatta Akhawe [:devd] from comment #0)
> A cert error for HSTS site is likely a captive portal. Firefox should open
> and connect to a new tab. Chrome has this behavior, but I am sure they have
> a couple of other heuristics too.

Shouldn't *every* cert error cause captive portal detection to run? Or at least every "untrusted issuer" error?

Comment 5

[Devdatta Akhawe [:devd]]

> Shouldn't *every* cert error cause captive portal detection to run? Or at
> least every "untrusted issuer" error?

No. The number of untrusted issuer errors are very high. Also, don't think captive portals will cause untrusted issuer errors: more likely a name validation error since the portal's login page, in my experience, is also often over HTTPS (to accept credit cards)

In the original captive portal detection bug, I recall getting stuck on "ok currently the browser assumes that network is ok, and never calls the network-link-status-service." Maybe a HSTS cert error could be a good reason to call the network status code.

Comment 6

[Devdatta Akhawe [:devd]]

> We've had questions from people asking how to turn off the HSTS preload list
> or bypass the unbypassable cert exception dialog, due to them either being
> behind a MITM appliance/proxy or explicitly distrusting certain certs etc.

These set of errors have to go away by them adding the issuer to their "trusted certs" list. I agree that HSTS errors can be due to MITM appliance/proxy. But those errors will need to be fixed anyways (since they can't browse the HSTS sites) Once they are fixed, HSTS errors will only occur for captive portals and actual MITM attempts, not due to appliances/proxies.

Comment 7

[Justin Dolske [:Dolske]]

The premise of this bug seems backwards to me. The browser should indeed do captive portal detection when (re)connecting to a network, and normal network loads should be suppressed until we think the network is good (not captive, or use signs in). [Insert some handwaving here about detecting when a network reverts to such mode, eg if your payment for 30 minutes of browsing expires.]

Things like HSTS errors seem like just one of many different kinds of clues that one may be connected to a captive portal. And it probably happens too late -- by the time a user happens to access a (rare) site that's using HSTS, they've most likely already tried to access other sites that the portal is intercepting. Or put another way, if captive portal detection is working well, one shouldn't get HSTS errors in the first place.

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Justin Dolske [:Dolske] from comment #7)
> portal is intercepting. Or put another way, if captive portal detection is
> working well, one shouldn't get HSTS errors in the first place.

The captive portal can "re-capture" the network at any time. For example, you might log into the captive portal and be happily surfing for a while, and then *bam* out of nowhere the captive portal decides you need to log in again. AFAICT, operating systems are OK at detecting captive portals at the time they establish the (Wifi) connection, but they don't help enough after the first check.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Devdatta Akhawe [:devd] from comment #5)
> > Shouldn't *every* cert error cause captive portal detection to run? Or at
> > least every "untrusted issuer" error?
> 
> No. The number of untrusted issuer errors are very high. Also, don't think
> captive portals will cause untrusted issuer errors: more likely a name
> validation error since the portal's login page, in my experience, is also
> often over HTTPS (to accept credit cards)
> 
> In the original captive portal detection bug, I recall getting stuck on "ok
> currently the browser assumes that network is ok, and never calls the
> network-link-status-service." Maybe a HSTS cert error could be a good reason
> to call the network status code.

I think I agree with you. My suggestion is that, whenever we encounter an untrusted issuer or domain mismatch certificate error, we run the captive portal detection logic, and if the captive portal detection logic returns "probably a captive portal" then we trigger the captive portal UI instead of the cert error UI. I don't understand why HSTS or not matters. Many sites are not HSTS and if we run captive portal detection logic before making a decision to show the captive portal UI, then we should be able to improve the captive portal UX in more cases (not just for HSTS sites) without significantly increasing the number of false positives.

The only downside I see is that doing captive portal detection for each cert error would significantly slow down navigation for sites that the user has added cert overrides for. We could mitigate that by skipping the captive portal detection logic for sites that have cert error overrides. That would work out well, especially if we fix bug 797678 to remove old overrides that users added to deal with captive portals.

Comment 10

[Damian Yerrick]

Firefox version: 45.0
Operating system: Xubuntu 14.04 LTS

Steps to reproduce:
1. Connect to a network using a captive portal that MITMs HTTPS connections made before authentication. Wi-Fi networks in Chick-fil-A restaurants make good test cases.
2. In Firefox, try to visit https://bugzilla.mozilla.org/ or any other HTTPS site.
3. On the error page, click Learn More.

Actual:
Another error page confusingly implying that support.mozilla.org itself is set up wrong.

Expect:
Display local help rather than remote help, explicitly mentioning the possibility of a captive portal and offering to open http://example.com/ in a new tab, bypassing cache. This should be done for the "https://support.mozilla.org" origin even if the operating system itself provides no standard way to detect captive portals.

Comment 11

[Damian Yerrick]

Attachment: Screenshot of message implying that support.mozilla.org is configured improperly

Comment 12

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

Attachment: Trigger a captive portal recheck when a certificate error occurs

Comment 13

[Patrick McManus [:mcmanus]]

Comment on attachment 8751677 [details] [diff] [review]
Trigger a captive portal recheck when a certificate error occurs

Review of attachment 8751677 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsIOService.cpp
@@ +377,5 @@
>  
>  nsresult
> +nsIOService::RecheckCaptivePortal()
> +{
> +  if (mCaptivePortalService) {

please assert main thread

Comment 14

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

Attachment: MozReview Request: Bug 816866 - Trigger a captive portal recheck when a certificate error occurs r=mcmanus

Review commit: https://reviewboard.mozilla.org/r/52786/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52786/

Comment 15

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

Attachment: MozReview Request: Bug 816866 - Add IsTrustedHTTPS to nsISiteSecurityService r=dkeeler

Review commit: https://reviewboard.mozilla.org/r/52788/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52788/

Comment 16

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8752760 [details]
MozReview Request: Bug 816866 - Add IsTrustedHTTPS to nsISiteSecurityService r=dkeeler

https://reviewboard.mozilla.org/r/52788/#review49802

I think you can get the functionality you want much more simply by calling mozilla::psm::ErrorIsOverridable(nsprError) in the other patch in this bug. Of course, that will only trigger the captive portal detection for overridable errors, which might not cover all TLS-related errors caused by a captive portal. To do that, you should have the check just be mozilla::psm::IsNSSErrorCode(nsprError).

Comment 17

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #16)
> Comment on attachment 8752760 [details]
> MozReview Request: Bug 816866 - Add IsTrustedHTTPS to nsISiteSecurityService
> r=dkeeler
> 
> https://reviewboard.mozilla.org/r/52788/#review49802
> 
> I think you can get the functionality you want much more simply by calling
> mozilla::psm::ErrorIsOverridable(nsprError) in the other patch in this bug.

Thanks, that's very good to know.

> Of course, that will only trigger the captive portal detection for
> overridable errors, which might not cover all TLS-related errors caused by a
> captive portal. To do that, you should have the check just be
> mozilla::psm::IsNSSErrorCode(nsprError).

I think you're right. Maybe we IsNSSErrorCode() is all we need here.

Comment 18

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

https://reviewboard.mozilla.org/r/52786/#review49978

Comment 19

[Patrick McManus [:mcmanus]]

Comment on attachment 8752759 [details]
MozReview Request: Bug 816866 - Trigger a captive portal recheck when a certificate error occurs r=mcmanus

https://reviewboard.mozilla.org/r/52786/#review50118

thanks all

Comment 20

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3701fc605cec7190dbd7e5c21d79ec81040de467
Bug 816866 - Trigger a captive portal recheck when a certificate error occurs r=mcmanus

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3701fc605cec