lighthouse: Don't fail localhost pages with INSECURE

@brendankenny this is not the situation we are in though. IIUC, I think @jimmydief is saying that even though --ignore-certificate-errors is being used and the bad certificate page is not being shown. Lighthouse still throws. This would also explain the explicit workaround behavior in puppeteer.

Example: lighthouse https://expired.badssl.com --chrome-flags="--ignore-certificate-errors" exits immediately with a fatal error.

I confirmed that with and without that flag, the same security states are seen from the protocol.

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
index 299a8c78..fe2653e9 100644
--- a/lighthouse-core/gather/driver.js
+++ b/lighthouse-core/gather/driver.js
@@ -780,6 +780,7 @@ class Driver {
        * @param {LH.Crdp.Security.SecurityStateChangedEvent} event
        */
       const securityStateChangedListener = ({securityState, explanations}) => {
+        console.log({securityState, explanations});
         if (securityState === 'insecure') {
           cancel();
           const insecureDescriptions = explanations

Seems --ignore-certificate-errors disables interstitials and removes feature gating for things like WebSockets, but does not modify how the protocol sees the security state.

Adding an option to disable that check (so it would hit the timeout instead) seems fine, but I’m loath to have some automated way of circumventing Chrome’s security protections.

This approach would require a flag for the DevTools client too btw. Which would probably lead to checking it for the first time it’s needed, and then never ever unchecking it, which is scary.

People use LH to test sites with authentication, for instance, and I don’t want to be responsible for folks XSSing themselves 😃

I’m wondering if we don’t really need to concern ourselves about that? If a machine is compromised such that a malicious website is being served at localhost, it would seem that the entire machine is compromised anyways. (EDIT: Actually, I’m failing to realize the attack could also come from the network level, if the DNS had been spoofed or something? I have a sketchy understanding of security in regards to this.)

–

I’m leaning towards total disabling of the security check for localhost. A special audit just for localhost seems fine too, although at first glance I’m unsure how that audit would be made with our current setup (how do we move the current code to a gatherer, and still early exit for non-local sites?).

There is another complication for any automated approach, which is that not all local sites use the hostname localhost. What if the local site uses any other hostname? Seem an opt in for “this is a local site” flag would be necessary to handle this completely.

@MichaelJCole the version of Lighthouse in which this has been fixed has not yet been released.

h2 is indeed our reason for running localhost via https.

I have tried launching Chrome independently via chrome-launcher and passing ignore-certificate-errors and/or allow-insecure-localhost but Lighthouse itself still throws an INSECURE_DOCUMENT_REQUEST error.

Happy to spend some time on this if there’s interest.

I believe this is already possible, provided you add your private keys to your local keychain. Without that step the local, self-signed certificate is deemed insecure.

Reviewing the history here:

• In https://github.com/GoogleChrome/lighthouse/pull/6300 when we found an insecure state, we treated it as a fatal error. It was to avoid a case where ERR_CERT_SYMANTEC_LEGACY meant subsequent protocol commands would hang indefinitely.
• Lots of folks use HTTPS on localhost for h2 testing (and other stuff) and were inconvenienced by this change. There are a few current resolutions
  • Self-sign your certs and hook up the private key. (ref)
  • Copy system certs into the docker container (ref)
  • Use an older version of Lighthouse (ref)
• https://github.com/GoogleChrome/lighthouse/pull/7574 was opened to add a CLI option to bypass these issues.

---

My take:

Looking back, we changed behavior not because we wanted to throw on insecure sites but because some of them triggered protocol_timeout situations (before we had implemented that).

It seems like the PROTOCOL_TIMEOUT failure would now actually catch the ERR_CERT_SYMANTEC_LEGACY cases, though admittedly the error message would be less useful.

In the last PR, @Janpot advocated for a different approach than what he implemented:

IMO lighthouse should treat security errors in a similar way as the http-status-code audit. Basically, accept insecure urls and use the Security.securityStateChanged to craft an audit that fails on those.

I agree that we shouldn’t throw if we can technically finish loading the page (Seems like our new NO_FCP stance aligns 😃.

I propose we remove our fatal INSECURE error and rely on PROTOCOL_TIMEOUT for the few cases where it’d apply. (The securitystate listeners should log.error in case of a problem.) And we should add an audit for valid & happy certificate–that’ll be a great place to put the documentation we’ve been discussing.

How bout dat? +@patrickhulce, @hoten, folks.

I got feedback from an MSC that our change here broke his flow. He’s using a self-signed cert for localhost and not added it properly.

I suspect that is more common than people trusting the cert on their machine. I think we should allow this behavior… but perhaps add an audit that flags this localhost/untrusted cert issue. That way, they’d at least get results.

@brendankenny this is not the situation we are in though. IIUC, I think @jimmydief is saying that even though --ignore-certificate-errors is being used and the bad certificate page is not being shown. Lighthouse still throws. This would also explain the explicit workaround behavior in puppeteer.

Example: lighthouse https://expired.badssl.com --chrome-flags="--ignore-certificate-errors" exits immediately with a fatal error.