Health Check mis-reports https functionality in certain situations

[Ipstenu]

The new HTTPS health check is over-aggressive when checking if HTTPS is working. In some cases, if you have the domain set up as HTTPS (that is, it's in the site and home URL) but you get an error saying WP can't verify that HTTPS is working.

After debugging with @clorith and @TimothyBlynJacobs we figured out it's due to plugins that strip some output used by that check to validate, which causes the check to false-flag a site as having a critical error.

$ wp option get https_detection_errors
array (
  'https_request_failed' =>
  array (
    0 => 'HTTPS request failed.',
  ),
)

This also pops up if you have a headless WP site that doesn't include all the WP info:

$ wp option get https_detection_errors
array (
  'bad_response_source' =>
  array (
    0 => 'It looks like the response did not come from this site.',
  ),
)

At this point, we all feel that considering most modern browsers (FF, Chrome, even Safari) will prevent you from visiting a site without valid HTTPs (yes, you can get around it), this part of the check is going to cause more drama than it's worth. We should remove that check, or change it from critical to an 'info'.

It's reasonable to say that if someone can log in to their site via https in the URL, then https is properly working as expected.

[bobbingwide]

Example of a plugin that strips output that's being checked is Fast Velocity Minify. This strips the output that's expected from rsd_link() when the Cleanup Header [ Removes resource hints, generator tag, shortlinks, manifest link, etc ] checkbox is selected.

Totally agree with removing these tests. If the request is being performed with https: and the response is 200 then I'd say it's pretty unlikely that it's not working.

[bobbingwide]

Additionally, it's bad advice to tell the user to contact hosting when it's nothing to do with hosting.

[Ipstenu]

TBF, if SSL really was broken then yes, talk to your host. But the check is flawed so (as a host) we were catching a lot of people freaking out for nothing.

So far we've found the following plugin types/situations can mess this up:

• Plugins that move/hide wp-admin (they also strip out things to hide it's WP)
• Plugins that legit hide all WP stuff on purpose for 'security' (YMMV, but they're used)
• Caching plugins/services
• Headless WordPress

[SergeyBiryukov]

#52805 was marked as a duplicate.

[dragongate]

Hello guys,

There is no problem with WP 5.7 , in fact I found out that WP site health was giving correct alert about HTTPS connection

This issue is related to firewall in my case was related to some manual wrong entries from my side in CloudFlare page rule settings

Please see reply in this thread

​https://wordpress.org/support/topic/false-detect-of-critical-issue-reported-by-wp-on-site-health/

thank you all

[mmuyskens]

Replying to dragongate:

Hello guys,

There is no problem with WP 5.7 , in fact I found out that WP site health was giving correct alert about HTTPS connection

This issue is related to firewall in my case was related to some manual wrong entries from my side in CloudFlare page rule settings

Please see reply in this thread

​https://wordpress.org/support/topic/false-detect-of-critical-issue-reported-by-wp-on-site-health/

thank you all

Glad you were able to fix it in your case but the issue does still remain. The health check that was implemented is 100% flawed as OP pointed out. The verbiage that was implemented for the message in this case was not appreciated.

[gab81]

hi,

i can confirm i am seeing this issue when everything is implemented properly - on an interesting note i do have Fast Velocity Minify, although i just tested disabling it and the same warning remains so indeed it checks this too "aggressively" - please roll it back.

thanks,
Gabrio

[eatsleepcode]

This appears to be happening with WP-Optimize also.

[k3nsai]

​Password Protected causes this too

[ayeshrajans]

#52837 was marked as a duplicate.

[peterwilsoncc]

In 52783.diff​, I've removed the bad_response_source portion of the check but kept everything leading up to that. At the moment this is just a POC/idea to allow for feedback.

@Ipstenu is this what you were thinking or were you suggesting that the SSL verification check be removed too?

[Ipstenu]

Honestly, I think the SSL verification should be pulled at the moment @peterwilsoncc, or modified to be a _warning_ instead of an error -- it's a fantastic *idea* but this is just the most common way it kicks back an incorrect response. We should look into not checking 'is WP returning properly with SSL' but maybe consider mimicking whatever browsers are doing to give us that happy lock in our address bars. If we leverage that, and not "Is WP..." I feel we'll have a better idea of what is coming back.

With the exception of a headless WP that pulls from an API on another server, it's unlikely a user would actually be able to get into their WP-admin anyway if SSL is borked. Most modern browsers warn you, some outright block you, and if you're intentionally pushing past that to log in, the odds are you already know and just need a warning. Basically, this is pretty rare, and in the majority of valid reports, they're being told already by the browser, so we shouldn't be hard error-ing, it gives the wrong impression when it's a false-flag.

And I have to stress... SSL is not an easy concept to grasp for a lot of new admins. I came 'round to this issue from customers at my host company who were freaking out because they could see the lock on their browser, they did SSL verify, but here's WP (who has a great track record for correct reports) telling them they were wrong. We have to consider WHO is being given this information and how we present it to them.

"WordPress is unable to verify that SSL is fully functional on your site. This can happen due to interference with plugins or proxy services. Here's how you can verify this for yourself." with a link to a doc on what could happen. We need to educate on this one a little more :)

https://core.trac.wordpress.org/ticket/52783

[peterwilsoncc]

In the ​linked pull request:

• Reduced the SSL tests to https_request_failed and bad_response_code
• Deleted related conditions in the site health class
• As tests are reduced to bad responses/connection errors, the error remains critical, text unchanged to play nice with the polyglots
• Unit tests for removed code deleted

"WordPress is unable to verify that SSL is fully functional on your site. This can happen due to interference with plugins or proxy services. Here's how you can verify this for yourself." with a link to a doc on what could happen. We need to educate on this one a little more :)

Site Health folk, are you happy to consider this for a follow up ticket so the docs can be prepared?

[TimothyBlynJacobs]

We should remove the test when your site is_ssl and has detected errors because of the reasons outlined.

But we should not remove the SSL verification from wp_update_https_detection_errors. Otherwise, a user may enable SSL on their site, thinking that their site supports it because we prompted them to in Site Health, but it in fact doesn't. And they'll wind up locked out of their site. A false negative is fine in this context, a false positive isn't.

[nicegamer7]

​Force Login also causes this.

[gab81]

hi all,

not sure what changed but i no longer have the warning and it's on "GOOD" now. i have just updated Yoast recently.

and WP is still 5.7 as far as i can see :).

best,
Gabrio

[annalamprou]

I got the message when I updated to 5.7
I have a certificate and the connection is secure
I am not so technical so I have a question: If the connection is indeed secure can I ignore the message? I am afraid that if I pok around more I will mess up more
Any suggestions?
Many thanks in advanced
-anna

[AnotherDave]

This false-alert occurs with all popular "coming soon" and "maintenance mode" plugins as of WP 5.7

[annalamprou]

I only have a password protected plugin. I guess this counts as "maintenance mode" plugin.
Otherwise my website is super simple. So the suggestion is to remove the plugin??
This is a bit unfair because non technical people like me heavily depend on plugins.
So I am hoping that WP will turn this into a suggestion and not a critical issue.

[Clorith]

@peterwilsoncc I agree that we need some docs here on how to verify your site works on https in cases where it can't be automatically detected. I do agree with @TimothyBlynJacobs as well though, that we want to be careful to not put the user in a situation where they may lock them selves out by accident.

Not running the check if is_ssl is also a great idea, as @Ipstenu notes; browsers will handle the scenario if the SSL is incorrect in that case, so WordPress shouldn't worry about it.

I also think this is a scenario where we'd be OK to introduce some new strings if we need to make clarifications anywhere.

[geoffrey1963]

Hi All, I have been trying to work out where this error was originating from, even contacting my WP host who did tests only to say all was good. Like Anna, I am not technical and tend to agree with Anna that this should be listed as Non-Critical error so people like me don't unnecessarily freak out. I wasnt until I found this info that I now feel Ok not to worry about this too much.

I only have a password protected plugin. I guess this counts as "maintenance mode" plugin.
Otherwise my website is super simple. So the suggestion is to remove the plugin??
This is a bit unfair because non technical people like me heavily depend on plugins.
So I am hoping that WP will turn this into a suggestion and not a critical issue.

[Toru]

Seeing a similar issue for a site which require basic auth too (login and admin area).
Site Health tells me it is not https, and baisc auth modal pops up.

$ wp option get https_detection_errors
array (
  'bad_response_code' =>
  array (
    0 => 'Unauthorized',
  ),
)

[peterwilsoncc]

I've updated the ​linked pull request with the following:

• Remove ! wp_is_https_supported() branch of health check per @TimothyBlynJacobs' suggestion
• Downgraded failing check to warning
• (As a maybe) Send basic auth headers in wp_update_https_detection_errors() if the site is currently been accessed behind basic auth.

I'm a little confused by the code here so feel free to tell me if I have it completely incorrect. I'm on leave next week so won't have much of a chance to refresh the PR prior to the 5.7.1 RC I am afraid, if I am completely missing the mark then please feel free to replace my pull request.

[mmuyskens]

Just pull the check completely if you have to. A broken padlock or non padlock is visual enough.

[pwallner]

I have the same error on my both sites. and after downgrading to 5.6.2 the error disappears. both sites running on https for 100%, dont use CDN, only wprocket.

[SergeyBiryukov]

Replying to peterwilsoncc:

(As a maybe) Send basic auth headers in wp_update_https_detection_errors() if the site is currently been accessed behind basic auth.

I'm not too sure about this last part at a glance, would need to take a deeper look. The rest of the PR looks good to me :)

[peterwilsoncc]

Replying to SergeyBiryukov:

Replying to peterwilsoncc:

(As a maybe) Send basic auth headers in wp_update_https_detection_errors() if the site is currently been accessed behind basic auth.

I'm not too sure about this last part at a glance, would need to take a deeper look. The rest of the PR looks good to me :)

Thanks Sergey, I've reverted the basic auth portion of the linked pull request in order to get this in without been side-tracked. I've opened #52977 as a follow up ticket for whether basic auth headers ought to be included.

[TimothyBlynJacobs]

Patch looks good to me. I don't think we necessarily need to drop the error level to a warning if they aren't using SSL, but I don't feel strongly about it.

[peterwilsoncc]

In 50659:

Site Health: Reduce false reports of HTTPS failures.

Reduce severity of failing HTTPS tests from critical to warning. Stop reporting failures if the site is being accessed over HTTPS but wp_is_https_supported() indicates a lack of support.

Props annalamprou, AnotherDave, ayeshrajans, bobbingwide, Clorith, dragongate, eatsleepcode, gab81, geoffrey1963, Ipstenu, k3nsai, mmuyskens, nicegamer7, peterwilsoncc, pwallner, SergeyBiryukov, TimothyBlynJacobs, Toru.
Fixes #52783.

[peterwilsoncc]

Reopening for backporting to the 5.7 branch.

[peterwilsoncc]

In 50660:

Site Health: Reduce false reports of HTTPS failures.

Reduce severity of failing HTTPS tests from critical to warning. Stop reporting failures if the site is being accessed over HTTPS but wp_is_https_supported() indicates a lack of support.

Props annalamprou, AnotherDave, ayeshrajans, bobbingwide, Clorith, dragongate, eatsleepcode, gab81, geoffrey1963, Ipstenu, k3nsai, mmuyskens, nicegamer7, peterwilsoncc, pwallner, SergeyBiryukov, TimothyBlynJacobs, Toru.
Merges [50659] to the 5.7 branch.
Fixes #52783.

[ripperj7]

This error is still showing for me. I checked with my host and there's no issue on their end.

[flixos90]

Sorry for being late here, but I haven't seen this issue until now. :/

I understand the reasoning behind removing the somewhat unreliable logic for when the site is already being accessed over HTTPS. However, what does that have to do with reducing the severity of the entire check?

I'd like to better understand why the decision was made to reduce it to "warning". Not using HTTPS today should be considered severe, and a goal of the original issue #47577 (which would have been good to review here) was that users should be informed about it in their WP dashboard so that WordPress can raise better awareness of the problem.

[SergeyBiryukov]

Replying to flixos90:

I understand the reasoning behind removing the somewhat unreliable logic for when the site is already being accessed over HTTPS. However, what does that have to do with reducing the severity of the entire check?

Good point, I think we can revert that part of [50660].

[peterwilsoncc]

The downgrade in severity came from comment 15. The documentation needs to be improved so end users know what to do and what questions to ask to improve the website.

Without docs improvements it's kind of like a warning sign on highway without context: is the road slippery, wildlife unpredictable or something else.

[flixos90]

Replying to peterwilsoncc:

The downgrade in severity came from comment 15. The documentation needs to be improved so end users know what to do and what questions to ask to improve the website.

Without docs improvements it's kind of like a warning sign on highway without context: is the road slippery, wildlife unpredictable or something else.

That comment to me still reads mostly as focused on the scenario where a site already has HTTPS. In that case, now the check that was causing problems has been removed.

If your site is not using HTTPS at all, that is a severe issue the user needs to be informed about. Whether the site in that case supports SSL or not is of rather secondary nature I'd say, so even if that check is unreliable, not using HTTPS is a valid and critical error either way.

So I think reducing the severity was done a bit out of context, I suggest to revert that bit due to the above.

[Ipstenu]

That comment to me still reads mostly as focused on the scenario where a site already has HTTPS. In that case, now the check that was causing problems has been removed.

No, it wasn't.

If a site has https, the warning is unneeded, obviously. If the site does not have https, the odds of them getting to wp-admin's health check _without_ their browser yelling at them about security, is so high, it puts into question the efficacy of us saying "Hey, this is bad."

Coupled with the reality that we know it's throwing false-positives, and we lack any proper documentation on what it means, it seems fair to have it be a warning "Hey, your site doesn't appear to have HTTPS working. if you meant to do that, cool. if not, you should know..."

Is it severe? Yes. But is it a severe _WordPress_ issue? Well that's a different question and it depends.

[TimothyBlynJacobs]

If the site does not have https, the odds of them getting to wp-admin's health check _without_ their browser yelling at them about security, is so high, it puts into question the efficacy of us saying "Hey, this is bad."

This is the warning that has been removed. @flixos90 is talking about the warning when a site doesn't have SSL configured period.

[Ipstenu]

Okay so... how do we know that's not on purpose?

Yes, I know google etc is all "Https or die" but having spent an hour with a customer today about that, some people don't care.

[peterwilsoncc]

Replying to Ipstenu:

Okay so... how do we know that's not on purpose?

Yes, I know google etc is all "Https or die" but having spent an hour with a customer today about that, some people don't care.

This is very much a part of my point: without improved docs WordPress is offloading support issues to hosts by reporting the issue as critical rather than a warning.

At some stage soon, I think it ought to be bumped up to critical again but it needs to be at WordPress's expense rather than hosting companies' support teams.

[flixos90]

If the site does not have https, the odds of them getting to wp-admin's health check _without_ their browser yelling at them about security, is so high, it puts into question the efficacy of us saying "Hey, this is bad."

This only applies if they have a browser that is strict about this. Users on older browsers (which are also more likely to not use HTTPS on their own websites) will not be informed about this by their browsers.

Even if these users don't care as much about security, more and more of their audience is using browsers that warn about insecure websites and may even prevent them from visiting them. So at this point it becomes a more severe issue even from the point of losing potential audience.

without improved docs WordPress is offloading support issues to hosts by reporting the issue as critical rather than a warning.

At some stage soon, I think it ought to be bumped up to critical again but it needs to be at WordPress's expense rather than hosting companies' support teams.

I don't fully agree, but I get the point; instead of discarding this, it would be great to define what kind of documentation is expected so that we can point users to it. What do you suggest?

[pwallner]

Maybe I misunderstand the comments above, but I have https connection and receive this error. And yes, if u not have https, error or warning are ok because more and more browsers request https.

[knutsp]

Replying to pwallner:

Maybe I misunderstand the comments above, but I have https connection and receive this error.

That issue is the main point for this ticket. Fixed.

And yes, if u not have https, error or warning are ok because more and more browsers request https.

Recommendation or Critical issue is discussed, since this was altered in the same changeset.

Talking about front end:
I say critical in case HTTPS is not available, since one (browser) cannot upgrade the request. But when available, but not set in home_url, it could be a recommendation, at least for now.

The reason for my view is that Site Health does not check if plain HTTP is available (status 200 on home), so in that case, using old links/bookmarks, the site can still be accessed by plain HTTP. For me, it' seems a bit pointless to call something critical when the insecure way (HTTP) may still be used.

But when HTTP only check is also made in Site Health, both checks may emit a critical issue. (I have plugin that makes such test, but when it fails I classify it as a recommendation - for now.) Ticket with request to implement it will arrive soon.

Finally, having wp-admin accessible without HTTPS, no FORCE_SSL_ADMIN and no https configured, is critical.

[peterwilsoncc]

Replying to flixos90:

I don't fully agree, but I get the point; instead of discarding this, it would be great to define what kind of documentation is expected so that we can point users to it. What do you suggest?

As 5.7.1 is due in a couple of days I think this is best figured out with the docs team on a follow up ticket. I've created #53024 for the purpose. In the ticket I have suggested the following but all is up for discussion:

• Fine tune when failures should report warnings or critical errors
• Improve documentation on WordPress.org to avoid overburdening hosting support teams
• Link to WordPress.org documentation as a primary call to action rather than contacting the user's host.

I've put this ticket against the 5.8 milestone but it may be able to be brought forward to 5.7.2 if everything can be discussed and documented in time.

As a direct answer to the question of what the documentation contains, I haven't had a chance to consider that but am willing to assist on the ticket once 5.7.1 has shipped.

I'm going to reclose this ticket as I don't think we'll get the questions above solved in time for the release.

[TimothyBlynJacobs]

Looking at the description of #53024 I'm confused as to what docs we are trying to create. The only warnings we generate now are if the site URL or home URL are not set to https. The false positives will no longer occur, because we don't run those tests at all.

So what would this documentation concretely be? The way to resolve the issue is to enable HTTPS for your site, which is a host-specific process.

If we disagree about whether a site not using https is a critical issue, like in comment:47, ok. But it seems like we are closing this because of a lack of documentation about the issue we've fixed in this ticket?

[Ipstenu]

But it seems like we are closing this because of a lack of documentation about the issue we've fixed in this ticket?

I'm totally fine with the documentation being a separate ticket, on the basis of these fixes. But I strongly feel it's our responsibility to educate users as to WHY this matters. it's the same reason why we have to be careful when we tell people "Hey, your PHP is old." Yes, it matters greatly, but at the same time, if we don't explain why and where to go, we're failing at the ultimate purpose of the health check!

To whit: If people can't solve the problems on the Health Check page, we have failed to properly explain the issue and direct them forward.

Since you asked this:

So what would this documentation concretely be?

The simplest is "Why this matters..."

If someone gets a message on our check-health pages, it's our responsibility to make sure they understand what the message means and why it matters,

For example, a document that does something like this...

"As of [year], it has become imperative that all websites use HTTPS for all communications, even something as straightforward as delivering a static about page, in order to protect visitors. Search Engines actively downgrade you, and many browsers will block users from accessing your website. As such, WordPress feels it's highly important that all websites can support HTTPS connections. While WordPress itself can change the URLs on your site to use that protocol, you will need to add an SSL certificate to your site. In many cases, this will require you to contact your webhost directly."

Essentially we're telling them "This is the reason this matters, here's how you can go about fixing it."

That way they're educated going forward, we're giving them the tools to get it fixed, and hopefully everyone moves to HTTPS :)

[TimothyBlynJacobs]

I agree that documentation would make sense, and we do already have this and link to it from the Site Health test: ​https://wordpress.org/support/article/why-should-i-use-https/

I'm reopening this because one thing I noticed we do have to fix regardless is that we are setting the status to warning which is invalid. It should be recommended if we don't want to go with critical. Otherwise, the test results don't appear at all.

[SergeyBiryukov]

In 50710:

Site Health: Correct test result status for the HTTPS test.

The supported status values for Site Health tests are good, recommended, and critical.

Follow-up to [50660].

Props TimothyBlynJacobs.
Fixes #52783.

[SergeyBiryukov]

In 50711:

Site Health: Correct test result status for the HTTPS test.

The supported status values for Site Health tests are good, recommended, and critical.

Follow-up to [50660].

Props TimothyBlynJacobs.
Merges [50710] to the 5.7 branch.
Fixes #52783.

https://core.trac.wordpress.org/changeset/50659