From 9608dd5f69d56c776297a3006d5fc7b7629552fb Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Fri, 22 Sep 2023 19:06:45 +0000 Subject: [PATCH] Security: remove the cron event that checked for https support. Fix an issue where a cron job ran every 12 hours to check for https support - even when https support was already enabled. The check is now run only when the user visits the Site Health page. Reducing the unneeded requests lowers the impact and load of hosting WordPress sites. The `wp_update_https_detection_errors` function is deprecated and the `https_detection_errors` option that was previously set by the cron job is no longer maintained. The `pre_wp_update_https_detection_errors` filter is deprecated and replaced by the `pre_wp_get_https_detection_errors` filter which serves the same function. Props audrasjb, johnbillion, Michi91. Fixes #58494. git-svn-id: https://develop.svn.wordpress.org/trunk@56664 602fd350-edb4-49c9-b593-d223f7449a82 --- .../includes/class-wp-site-health.php | 5 +- src/wp-admin/includes/upgrade.php | 6 + src/wp-includes/default-filters.php | 5 - src/wp-includes/deprecated.php | 38 ++++++ src/wp-includes/https-detection.php | 48 ++------ tests/phpunit/tests/https-detection.php | 116 ------------------ 6 files changed, 53 insertions(+), 165 deletions(-) diff --git a/src/wp-admin/includes/class-wp-site-health.php b/src/wp-admin/includes/class-wp-site-health.php index 6d87edee1b..b73e1e78a0 100644 --- a/src/wp-admin/includes/class-wp-site-health.php +++ b/src/wp-admin/includes/class-wp-site-health.php @@ -1555,10 +1555,9 @@ class WP_Site_Health { */ public function get_test_https_status() { /* - * Enforce fresh HTTPS detection results. This is normally invoked by using cron, - * but for Site Health it should always rely on the latest results. + * Check HTTPS detection results. */ - wp_update_https_detection_errors(); + $errors = wp_get_https_detection_errors(); $default_update_url = wp_get_default_update_https_url(); diff --git a/src/wp-admin/includes/upgrade.php b/src/wp-admin/includes/upgrade.php index fc1ab3792f..6929570fac 100644 --- a/src/wp-admin/includes/upgrade.php +++ b/src/wp-admin/includes/upgrade.php @@ -2340,6 +2340,12 @@ function upgrade_640() { if ( $wp_current_db_version < 56657 ) { // Enable attachment pages. update_option( 'wp_attachment_pages_enabled', 1 ); + + // Remove the wp_https_detection cron. Https status is checked directly in an async Site Health check. + $scheduled = wp_get_scheduled_event( 'wp_https_detection' ); + if ( $scheduled ) { + wp_clear_scheduled_hook( 'wp_https_detection' ); + } } } diff --git a/src/wp-includes/default-filters.php b/src/wp-includes/default-filters.php index a3ea1537cb..5add466889 100644 --- a/src/wp-includes/default-filters.php +++ b/src/wp-includes/default-filters.php @@ -387,11 +387,6 @@ if ( ! defined( 'DOING_CRON' ) ) { add_action( 'init', 'wp_cron' ); } -// HTTPS detection. -add_action( 'init', 'wp_schedule_https_detection' ); -add_action( 'wp_https_detection', 'wp_update_https_detection_errors' ); -add_filter( 'cron_request', 'wp_cron_conditionally_prevent_sslverify', 9999 ); - // HTTPS migration. add_action( 'update_option_home', 'wp_update_https_migration_required', 10, 2 ); diff --git a/src/wp-includes/deprecated.php b/src/wp-includes/deprecated.php index 1d3ddd519e..c7b284cf5c 100644 --- a/src/wp-includes/deprecated.php +++ b/src/wp-includes/deprecated.php @@ -5870,3 +5870,41 @@ function _wp_theme_json_webfonts_handler() { add_action( 'wp_enqueue_scripts', $fn_generate_and_enqueue_styles ); add_action( 'admin_init', $fn_generate_and_enqueue_editor_styles ); } + +/** + * Runs a remote HTTPS request to detect whether HTTPS supported, and stores potential errors. + * + * This internal function is called by a regular Cron hook to ensure HTTPS support is detected and maintained. + * + * @since 5.7.0 + * @deprecated 6.4.0 The `wp_update_https_detection_errors()` function is no longer used and has been replaced by + * `wp_get_https_detection_errors()`. Previously the function was called by a regular Cron hook to + * update the `https_detection_errors` option, but this is no longer necessary as the errors are + * retrieved directly in Site Health and no longer used outside of Site Health. + * @access private + */ +function wp_update_https_detection_errors() { + _deprecated_function( __FUNCTION__, '6.4.0' ); + + /** + * Short-circuits the process of detecting errors related to HTTPS support. + * + * Returning a `WP_Error` from the filter will effectively short-circuit the default logic of trying a remote + * request to the site over HTTPS, storing the errors array from the returned `WP_Error` instead. + * + * @since 5.7.0 + * @deprecated 6.4.0 The `wp_update_https_detection_errors` filter is no longer used and has been replaced by `pre_wp_get_https_detection_errors`. + * + * @param null|WP_Error $pre Error object to short-circuit detection, + * or null to continue with the default behavior. + */ + $support_errors = apply_filters( 'pre_wp_update_https_detection_errors', null ); + if ( is_wp_error( $support_errors ) ) { + update_option( 'https_detection_errors', $support_errors->errors ); + return; + } + + $support_errors = wp_get_https_detection_errors(); + + update_option( 'https_detection_errors', $support_errors ); +} \ No newline at end of file diff --git a/src/wp-includes/https-detection.php b/src/wp-includes/https-detection.php index 878dd1aa38..f34d6a3a11 100644 --- a/src/wp-includes/https-detection.php +++ b/src/wp-includes/https-detection.php @@ -86,25 +86,25 @@ function wp_is_https_supported() { * * This internal function is called by a regular Cron hook to ensure HTTPS support is detected and maintained. * - * @since 5.7.0 + * @since 6.4.0 * @access private */ -function wp_update_https_detection_errors() { +function wp_get_https_detection_errors() { /** * Short-circuits the process of detecting errors related to HTTPS support. * * Returning a `WP_Error` from the filter will effectively short-circuit the default logic of trying a remote * request to the site over HTTPS, storing the errors array from the returned `WP_Error` instead. * - * @since 5.7.0 + * @since 6.4.0 * * @param null|WP_Error $pre Error object to short-circuit detection, * or null to continue with the default behavior. + * @return null|WP_Error Error object if HTTPS detection errors are found, null otherwise. */ - $support_errors = apply_filters( 'pre_wp_update_https_detection_errors', null ); + $support_errors = apply_filters( 'pre_wp_get_https_detection_errors', null ); if ( is_wp_error( $support_errors ) ) { - update_option( 'https_detection_errors', $support_errors->errors ); - return; + return $support_errors->errors; } $support_errors = new WP_Error(); @@ -153,41 +153,7 @@ function wp_update_https_detection_errors() { } } - update_option( 'https_detection_errors', $support_errors->errors ); -} - -/** - * Schedules the Cron hook for detecting HTTPS support. - * - * @since 5.7.0 - * @access private - */ -function wp_schedule_https_detection() { - if ( wp_installing() ) { - return; - } - - if ( ! wp_next_scheduled( 'wp_https_detection' ) ) { - wp_schedule_event( time(), 'twicedaily', 'wp_https_detection' ); - } -} - -/** - * Disables SSL verification if the 'cron_request' arguments include an HTTPS URL. - * - * This prevents an issue if HTTPS breaks, where there would be a failed attempt to verify HTTPS. - * - * @since 5.7.0 - * @access private - * - * @param array $request The cron request arguments. - * @return array The filtered cron request arguments. - */ -function wp_cron_conditionally_prevent_sslverify( $request ) { - if ( 'https' === wp_parse_url( $request['url'], PHP_URL_SCHEME ) ) { - $request['args']['sslverify'] = false; - } - return $request; + return $support_errors->errors; } /** diff --git a/tests/phpunit/tests/https-detection.php b/tests/phpunit/tests/https-detection.php index 80f1684699..da2fdc2d8a 100644 --- a/tests/phpunit/tests/https-detection.php +++ b/tests/phpunit/tests/https-detection.php @@ -54,122 +54,6 @@ class Tests_HTTPS_Detection extends WP_UnitTestCase { $this->assertFalse( wp_is_https_supported() ); } - /** - * @ticket 47577 - * @ticket 52484 - */ - public function test_wp_update_https_detection_errors() { - // Set HTTP URL, the request below should use its HTTPS version. - update_option( 'home', 'http://example.com/' ); - add_filter( 'pre_http_request', array( $this, 'record_request_url' ), 10, 3 ); - - // If initial request succeeds, all good. - add_filter( 'pre_http_request', array( $this, 'mock_success_with_sslverify' ), 10, 2 ); - wp_update_https_detection_errors(); - $this->assertSame( array(), get_option( 'https_detection_errors' ) ); - - // If initial request fails and request without SSL verification succeeds, - // return 'ssl_verification_failed' error. - add_filter( 'pre_http_request', array( $this, 'mock_error_with_sslverify' ), 10, 2 ); - add_filter( 'pre_http_request', array( $this, 'mock_success_without_sslverify' ), 10, 2 ); - wp_update_https_detection_errors(); - $this->assertSame( - array( 'ssl_verification_failed' => array( __( 'SSL verification failed.' ) ) ), - get_option( 'https_detection_errors' ) - ); - - // If both initial request and request without SSL verification fail, - // return 'https_request_failed' error. - add_filter( 'pre_http_request', array( $this, 'mock_error_with_sslverify' ), 10, 2 ); - add_filter( 'pre_http_request', array( $this, 'mock_error_without_sslverify' ), 10, 2 ); - wp_update_https_detection_errors(); - $this->assertSame( - array( 'https_request_failed' => array( __( 'HTTPS request failed.' ) ) ), - get_option( 'https_detection_errors' ) - ); - - // If request succeeds, but response is not 200, return error with - // 'bad_response_code' error code. - add_filter( 'pre_http_request', array( $this, 'mock_not_found' ), 10, 2 ); - wp_update_https_detection_errors(); - $this->assertSame( - array( 'bad_response_code' => array( 'Not Found' ) ), - get_option( 'https_detection_errors' ) - ); - - // If request succeeds, but response was not generated by this - // WordPress site, return error with 'bad_response_source' error code. - add_filter( 'pre_http_request', array( $this, 'mock_bad_source' ), 10, 2 ); - wp_update_https_detection_errors(); - $this->assertSame( - array( 'bad_response_source' => array( 'It looks like the response did not come from this site.' ) ), - get_option( 'https_detection_errors' ) - ); - - // Check that the requests are made to the correct URL. - $this->assertSame( 'https://example.com/', $this->last_request_url ); - } - - /** - * @ticket 47577 - */ - public function test_pre_wp_update_https_detection_errors() { - // Override to enforce no errors being detected. - add_filter( - 'pre_wp_update_https_detection_errors', - static function () { - return new WP_Error(); - } - ); - wp_update_https_detection_errors(); - $this->assertSame( array(), get_option( 'https_detection_errors' ) ); - - // Override to enforce an error being detected. - add_filter( - 'pre_wp_update_https_detection_errors', - static function () { - return new WP_Error( - 'ssl_verification_failed', - 'Bad SSL certificate.' - ); - } - ); - wp_update_https_detection_errors(); - $this->assertSame( - array( 'ssl_verification_failed' => array( 'Bad SSL certificate.' ) ), - get_option( 'https_detection_errors' ) - ); - } - - /** - * @ticket 47577 - */ - public function test_wp_schedule_https_detection() { - wp_schedule_https_detection(); - $this->assertSame( 'twicedaily', wp_get_schedule( 'wp_https_detection' ) ); - } - - /** - * @ticket 47577 - */ - public function test_wp_cron_conditionally_prevent_sslverify() { - // If URL is not using HTTPS, don't set 'sslverify' to false. - $request = array( - 'url' => 'http://example.com/', - 'args' => array( 'sslverify' => true ), - ); - $this->assertSame( $request, wp_cron_conditionally_prevent_sslverify( $request ) ); - - // If URL is using HTTPS, set 'sslverify' to false. - $request = array( - 'url' => 'https://example.com/', - 'args' => array( 'sslverify' => true ), - ); - $expected = $request; - $expected['args']['sslverify'] = false; - $this->assertSame( $expected, wp_cron_conditionally_prevent_sslverify( $request ) ); - } - /** * @ticket 47577 * @ticket 52542