Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 weeks ago

#47957 closed enhancement (fixed)

Don't verify SSL certificate for loopback test

Reported by: ocean90 Owned by: ocean90
Priority: normal Milestone: 5.3
Component: Site Health Version: 5.2
Severity: normal Keywords: has-patch commit
Cc: Focuses:

Description

If a site uses a self-signed certificate the loopback test will report an error. Since spawn_cron() doesn't verify the certificate the test shouldn't either to replicate core's behaviour.

Attachments (4)

47957.diff (886 bytes ) - added by ocean90 7 years ago.
47957.2.diff (1.5 KB ) - added by ocean90 7 years ago.
Disable sslverify for get_test_rest_availability()
47957.3.diff (2.4 KB ) - added by ocean90 7 years ago.
Disable sslverify for test_wp_version_check_attached()
47957.patch (3.7 KB ) - added by Clorith 7 years ago.

Download all attachments as: .zip

Change History (10)

@ocean90
7 years ago

@ocean90
7 years ago

Disable sslverify for get_test_rest_availability()

@ocean90
7 years ago

Disable sslverify for test_wp_version_check_attached()

#1 @Clorith
7 years ago

Hmm, we definitely need a more reliable solution overall. The reason the Site Health check does it this way is because it was copied from how the plugin/theme editors do them (this was the basis for creating the test, as a lot of users had issues saving after the changes to t hem were implemented, all due to loopback failures).

There's now 3 places (likely to be more) that do loopbacks, so having a fixed loopback function is probably a more sustainable approach?

I'm thinking along the lines of function loopback( $target_url ) { return $WP_Http } which all the places that do loopbacks can use, and by returning the WP_Http object those places can then check whatever they need, if that is the content body, headers etc.

This would also be handy for #47954 which looks to do loopback calls to verify URLs are reachable before breaking a users access to their site.

@Clorith
7 years ago

#2 @Clorith
7 years ago

  • Keywords commit added

47957.patch includes the recommendations from 47957.3.diff, but also implements them for the theme/plugin editors so that it's consistent in all the places that currently do loopbacks.

Let's get this ticket fixed for now, as it's a simple fix, and we can keep looking into the deeper needs depending on the needs of #47954.

#3 @ocean90
7 years ago

In 46230:

File Editor: Don't verify SSL certificate when doing loopback requests for checking for fatal errors.

Props Clorith.
See #47957.

#4 @ocean90
7 years ago

  • Owner set to ocean90
  • Resolutionfixed
  • Status newclosed

In 46231:

Site Health: Don't verify SSL certificate when testing the local site.

The SSL certificate may be self-signed which prevents various tests from returning proper results. Since the Cron API and file editors don't verify the certificate the tests shouldn't either.

Props Clorith, ocean90.
Fixes #47957.

#5 @SergeyBiryukov
6 years ago

In 47896:

File Editor: Don't verify SSL certificate when doing loopback requests for checking for fatal errors.

Previously, verification was disabled for the loopback request to the admin URL, but not to the home URL.

Follow-up to [46230].

Props munyagu.
Fixes #50309. See #47957.

#6 @westonruter
7 weeks ago

In 62894:

HTTP API: Pass the missing URL to the https_local_ssl_verify filter.

The filter has documented a $url parameter since 5.1.0, when r42682 added it to the transports in WP_Http_Streams and WP_Http_Curl. However, this new parameter was not passed to all instances of the filter being applied, resulting in a possible fatal error if a callback is expecting it.

These call sites were surfaced by the PHPStan extensions under development for this ticket, which are committed separately.

Developed as subset of https://github.com/WordPress/wordpress-develop/pull/12022.
Follow-up to r28781, r42682, r46230, r46231, r54043, r58128.

See #65376, #42186, #47957, #56041, #58281.

Note: See TracTickets for help on using tickets.