From fe1c16e35f9ca3709e6b731fc96a221797767600 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:23:24 +0100 Subject: [PATCH 01/46] Begin switching the password hashing mechanism from phpass to bcrypt. --- src/wp-includes/pluggable.php | 96 +++++++------- tests/phpunit/tests/auth.php | 126 ++++++++++++++++++- tests/phpunit/tests/pluggable/signatures.php | 3 + tests/phpunit/tests/user/passwordHash.php | 4 + 4 files changed, 175 insertions(+), 54 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 690c0fe4bca47..4903c777b3f28 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2570,22 +2570,13 @@ function wp_hash( $data, $scheme = 'auth' ) { * instead use the other package password hashing algorithm. * * @since 2.5.0 - * - * @global PasswordHash $wp_hasher PHPass object. + * @since x.y.z The password is now hashed using bcrypt instead of phpass. * * @param string $password Plain text user password to hash. * @return string The hash string of the password. */ function wp_hash_password( $password ) { - global $wp_hasher; - - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - // By default, use the portable hash from phpass. - $wp_hasher = new PasswordHash( 8, true ); - } - - return $wp_hasher->HashPassword( trim( $password ) ); + return password_hash( trim( $password ), PASSWORD_BCRYPT ); } endif; @@ -2593,19 +2584,15 @@ function wp_hash_password( $password ) { /** * Checks a plaintext password against a hashed password. * - * Maintains compatibility between old version and the new cookie authentication - * protocol using PHPass library. The $hash parameter is the encrypted password - * and the function compares the plain text password when encrypted similarly - * against the already encrypted password to see if they match. - * * For integration with other applications, this function can be overwritten to * instead use the other package password hashing algorithm. * * @since 2.5.0 + * @since x.y.z Passwords in WordPress are now hashed with bcrypt by default. A + * password that wasn't hashed with bcrypt will be checked with phpass. * - * @global PasswordHash $wp_hasher PHPass object used for checking the password - * against the $hash + $password. - * @uses PasswordHash::CheckPassword + * @global PasswordHash $wp_hasher phpass object. Used as a fallback for verifying + * passwords that were hashed with phpass. * * @param string $password Plaintext user's password. * @param string $hash Hash of the user's password to check against. @@ -2615,45 +2602,57 @@ function wp_hash_password( $password ) { function wp_check_password( $password, $hash, $user_id = '' ) { global $wp_hasher; - // If the hash is still md5... + // If the hash is still md5 or otherwise truncated then invalidate it. if ( strlen( $hash ) <= 32 ) { - $check = hash_equals( $hash, md5( $password ) ); - if ( $check && $user_id ) { - // Rehash using new hash. - wp_set_password( $password, $user_id ); - $hash = wp_hash_password( $password ); + return false; + } + + if ( wp_password_needs_rehash( $hash ) ) { + if ( empty( $wp_hasher ) ) { + require_once ABSPATH . WPINC . '/class-phpass.php'; + // Use the portable hash from phpass. + $wp_hasher = new PasswordHash( 8, true ); } - /** - * Filters whether the plaintext password matches the encrypted password. - * - * @since 2.5.0 - * - * @param bool $check Whether the passwords match. - * @param string $password The plaintext password. - * @param string $hash The hashed password. - * @param string|int $user_id User ID. Can be empty. - */ - return apply_filters( 'check_password', $check, $password, $hash, $user_id ); + $check = $wp_hasher->CheckPassword( $password, $hash ); + } else { + $check = password_verify( $password, $hash ); } - /* - * If the stored hash is longer than an MD5, - * presume the new style phpass portable hash. + /** + * Filters whether the plaintext password matches the encrypted password. + * + * @since 2.5.0 + * @since x.y.z Passwords are now hashed with bcrypt by default. + * Old passwords may still be hashed with phpass. + * + * @param bool $check Whether the passwords match. + * @param string $password The plaintext password. + * @param string $hash The hashed password. + * @param string|int $user_id User ID. Can be empty. */ - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - // By default, use the portable hash from phpass. - $wp_hasher = new PasswordHash( 8, true ); - } - - $check = $wp_hasher->CheckPassword( $password, $hash ); - - /** This filter is documented in wp-includes/pluggable.php */ return apply_filters( 'check_password', $check, $password, $hash, $user_id ); } endif; +if ( ! function_exists( 'wp_password_needs_rehash' ) ) : + /** + * Checks whether a password hash needs to be rehashed. + * + * Passwords are hashed with bcrypt using the default cost. A password hashed in a prior version + * of WordPress may still be hashed with phpass and will need to be rehashed. If the default cost + * gets updated then a password hashed in a previous version of PHP will need to be rehashed. + * + * @since x.y.z + * + * @param string $hash Hash of a password to check. + * @return bool Whether the hash needs to be rehashed. + */ + function wp_password_needs_rehash( $hash ) { + return password_needs_rehash( $hash, PASSWORD_BCRYPT ); + } +endif; + if ( ! function_exists( 'wp_generate_password' ) ) : /** * Generates a random password drawn from the defined set of characters. @@ -2802,6 +2801,7 @@ function wp_rand( $min = null, $max = null ) { * of password resets if precautions are not taken to ensure it does not execute on every page load. * * @since 2.5.0 + * @since x.y.z The password is now hashed using bcrypt instead of phpass. * * @global wpdb $wpdb WordPress database abstraction object. * diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 5c196b499fcce..5029b283a5abb 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -10,13 +10,24 @@ class Tests_Auth extends WP_UnitTestCase { const USER_LOGIN = 'password-user'; const USER_PASS = 'password'; + /** + * @var WP_User + */ protected $user; /** * @var WP_User */ protected static $_user; + + /** + * @var int + */ protected static $user_id; + + /** + * @var PasswordHash + */ protected static $wp_hasher; /** @@ -159,6 +170,30 @@ public function test_wp_hash_password_trimming() { $this->assertTrue( wp_check_password( 'pass with vertical tab o_O', wp_hash_password( $password ) ) ); } + /** + * @ticket 21022 + */ + public function test_wp_check_password_supports_fallback_phpass_hashes() { + $password = 'password'; + $this->assertTrue( wp_check_password( $password, self::$wp_hasher->HashPassword( $password ) ) ); + } + + /** + * @ticket 21022 + */ + public function test_wp_check_password_does_not_support_md5_hashes() { + $password = 'password'; + $this->assertFalse( wp_check_password( $password, md5( $password ) ) ); + } + + /** + * @ticket 21022 + */ + public function test_wp_check_password_does_not_support_plain_text() { + $password = 'password'; + $this->assertFalse( wp_check_password( $password, $password ) ); + } + /** * @ticket 29217 */ @@ -235,16 +270,24 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() unset( $_REQUEST['_wpnonce'] ); } + /** + * See https://core.trac.wordpress.org/changeset/30466 . + * + * @TODO bcrypt has a password length limit of 72, need to decide what our approach is. + * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 + * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html + */ public function test_password_length_limit() { - $limit = str_repeat( 'a', 4096 ); + $limit = str_repeat( 'a', 72 ); wp_set_password( $limit, self::$user_id ); - // phpass hashed password. - $this->assertStringStartsWith( '$P$', $this->user->data->user_pass ); + // Ensure the password is hashed with bcrypt. + $this->assertStringStartsWith( '$2y$', $this->user->data->user_pass ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, $limit ); $this->assertInstanceOf( 'WP_User', $user ); @@ -254,6 +297,7 @@ public function test_password_length_limit() { $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); wp_set_password( $limit . 'a', self::$user_id ); $user = get_user_by( 'id', self::$user_id ); @@ -262,24 +306,30 @@ public function test_password_length_limit() { $user = wp_authenticate( $this->user->user_login, '*' ); $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, '*0' ); $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, '*1' ); $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, $limit ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); // Password broken by setting it to be too long. $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); } /** @@ -306,7 +356,7 @@ public function test_user_activation_key_is_checked() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-1 hour' ) . ':' . self::$wp_hasher->HashPassword( $key ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash_password( $key ), ), array( 'ID' => $this->user->ID, @@ -344,7 +394,7 @@ public function test_expired_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . self::$wp_hasher->HashPassword( $key ), + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash_password( $key ), ), array( 'ID' => $this->user->ID, @@ -355,6 +405,7 @@ public function test_expired_user_activation_key_is_rejected() { // An expired but otherwise valid key should be rejected. $check = check_password_reset_key( $key, $this->user->user_login ); $this->assertInstanceOf( 'WP_Error', $check ); + $this->assertSame( 'expired_key', $check->get_error_code() ); } /** @@ -382,7 +433,7 @@ public function test_legacy_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => self::$wp_hasher->HashPassword( $key ), + 'user_activation_key' => wp_hash_password( $key ), ), array( 'ID' => $this->user->ID, @@ -393,10 +444,73 @@ public function test_legacy_user_activation_key_is_rejected() { // A legacy user_activation_key should not be accepted. $check = check_password_reset_key( $key, $this->user->user_login ); $this->assertInstanceOf( 'WP_Error', $check ); + $this->assertSame( 'expired_key', $check->get_error_code() ); // An empty key with a legacy user_activation_key should be rejected. $check = check_password_reset_key( '', $this->user->user_login ); $this->assertInstanceOf( 'WP_Error', $check ); + $this->assertSame( 'invalid_key', $check->get_error_code() ); + } + + /** + * @ticket 21022 + * @ticket 32429 + */ + public function test_phpass_user_activation_key_is_rejected() { + global $wpdb; + + // A legacy user_activation_key is one hashed using phpass between WordPress 4.3 and x.y.z. + + $key = wp_generate_password( 20, false ); + $wpdb->update( + $wpdb->users, + array( + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . self::$wp_hasher->HashPassword( $key ), + ), + array( + 'ID' => $this->user->ID, + ) + ); + clean_user_cache( $this->user ); + + // A legacy phpass user_activation_key should not be accepted even when it is otherwise valid. + $check = check_password_reset_key( $key, $this->user->user_login ); + $this->assertWPError( $check ); + $this->assertSame( 'invalid_key', $check->get_error_code() ); + + // An empty key with a legacy user_activation_key should be rejected. + $check = check_password_reset_key( '', $this->user->user_login ); + $this->assertWPError( $check ); + $this->assertSame( 'invalid_key', $check->get_error_code() ); + } + + /** + * The `wp_password_needs_rehash()` function is just a wrapper around `password_needs_rehash()`, but this ensures + * that it works as expected. + * + * @ticket 21022 + */ + public function check_password_needs_rehashing() { + $password = 'password'; + + // Current password hashing algorithm. + $hash = wp_hash_password( $password ); + $this->assertFalse( wp_password_needs_rehash( $hash ) ); + + // A future upgrade from a previously lower cost. + $opts = array( + 'cost' => 8, + ); + $hash = password_hash( $password, PASSWORD_BCRYPT, $opts ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); + + // Previous phpass algorithm. + $hash = self::$wp_hasher->HashPassword( $password ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); + + // o_O md5. + $hash = md5( $password ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); } /** diff --git a/tests/phpunit/tests/pluggable/signatures.php b/tests/phpunit/tests/pluggable/signatures.php index 81fd079621916..b4ecae0f522e8 100644 --- a/tests/phpunit/tests/pluggable/signatures.php +++ b/tests/phpunit/tests/pluggable/signatures.php @@ -216,6 +216,9 @@ public function get_pluggable_function_signatures() { 'hash', 'user_id' => '', ), + 'wp_password_needs_rehash' => array( + 'hash', + ), 'wp_generate_password' => array( 'length' => 12, 'special_chars' => true, diff --git a/tests/phpunit/tests/user/passwordHash.php b/tests/phpunit/tests/user/passwordHash.php index db34969c71bb3..13efdb9367ad0 100644 --- a/tests/phpunit/tests/user/passwordHash.php +++ b/tests/phpunit/tests/user/passwordHash.php @@ -3,6 +3,10 @@ /** * Tests for the PasswordHash external library. * + * PasswordHash is no longer used to hash passwords, but it is still used as a fallback + * to verify passwords that were hashed by it. The library therefore needs to remain + * compatible with the latest versions of PHP. + * * @covers PasswordHash */ class Tests_User_PasswordHash extends WP_UnitTestCase { From 82d937f5401dc49fac381c0b40e3ec05017556b2 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:28:40 +0100 Subject: [PATCH 02/46] Update the handling of user passwords, password reset keys, and user request keys. --- src/wp-includes/class-wp-user-request.php | 2 + src/wp-includes/class-wp-user.php | 1 + src/wp-includes/user.php | 47 +++-------------------- 3 files changed, 8 insertions(+), 42 deletions(-) diff --git a/src/wp-includes/class-wp-user-request.php b/src/wp-includes/class-wp-user-request.php index 8c66dcdf8189e..82f079f40b799 100644 --- a/src/wp-includes/class-wp-user-request.php +++ b/src/wp-includes/class-wp-user-request.php @@ -92,6 +92,8 @@ final class WP_User_Request { * Key used to confirm this request. * * @since 4.9.6 + * @since x.y.z The key is now hashed using bcrypt instead of phpass. + * * @var string */ public $confirm_key = ''; diff --git a/src/wp-includes/class-wp-user.php b/src/wp-includes/class-wp-user.php index 0be1b3ed02e86..051f583627298 100644 --- a/src/wp-includes/class-wp-user.php +++ b/src/wp-includes/class-wp-user.php @@ -11,6 +11,7 @@ * Core class used to implement the WP_User object. * * @since 2.0.0 + * @since x.y.z The `user_pass` property is now hashed using bcrypt instead of phpass. * * @property string $nickname * @property string $description diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index c234c495dfe3d..fa3787884f373 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2377,6 +2377,7 @@ function wp_insert_user( $userdata ) { * * @since 4.9.0 * @since 5.8.0 The `$userdata` parameter was added. + * @since x.y.z The user's password is now hashed using bcrypt instead of phpass. * * @param array $data { * Values and keys for the user. @@ -2914,14 +2915,10 @@ function wp_get_password_hint() { * * @since 4.4.0 * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. - * * @param WP_User $user User to retrieve password reset key for. * @return string|WP_Error Password reset key on success. WP_Error on error. */ function get_password_reset_key( $user ) { - global $wp_hasher; - if ( ! ( $user instanceof WP_User ) ) { return new WP_Error( 'invalidcombo', __( 'Error: There is no account with that username or email address.' ) ); } @@ -2967,13 +2964,7 @@ function get_password_reset_key( $user ) { */ do_action( 'retrieve_password_key', $user->user_login, $key ); - // Now insert the key, hashed, into the DB. - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - - $hashed = time() . ':' . $wp_hasher->HashPassword( $key ); + $hashed = time() . ':' . wp_hash_password( $key ); $key_saved = wp_update_user( array( @@ -2999,15 +2990,11 @@ function get_password_reset_key( $user ) { * * @since 3.1.0 * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. - * * @param string $key Hash to validate sending user's password. * @param string $login The user login. * @return WP_User|WP_Error WP_User object on success, WP_Error object for invalid or expired keys. */ function check_password_reset_key( $key, $login ) { - global $wp_hasher; - $key = preg_replace( '/[^a-z0-9]/i', '', $key ); if ( empty( $key ) || ! is_string( $key ) ) { @@ -3024,11 +3011,6 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - /** * Filters the expiration time of password reset keys. * @@ -3050,7 +3032,7 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - $hash_is_correct = $wp_hasher->CheckPassword( $key, $pass_key ); + $hash_is_correct = password_verify( $key, $pass_key ); if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { return $user; @@ -3087,7 +3069,6 @@ function check_password_reset_key( $key, $login ) { * @since 5.7.0 Added `$user_login` parameter. * * @global wpdb $wpdb WordPress database abstraction object. - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. * * @param string $user_login Optional. Username to send a password retrieval email for. * Defaults to `$_POST['user_login']` if not set. @@ -4864,28 +4845,19 @@ function wp_send_user_request( $request_id ) { * * @since 4.9.6 * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. - * * @param int $request_id Request ID. * @return string Confirmation key. */ function wp_generate_user_request_key( $request_id ) { - global $wp_hasher; - // Generate something random for a confirmation key. $key = wp_generate_password( 20, false ); // Return the key, hashed. - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - wp_update_post( array( 'ID' => $request_id, 'post_status' => 'request-pending', - 'post_password' => $wp_hasher->HashPassword( $key ), + 'post_password' => wp_hash_password( $key ), ) ); @@ -4897,15 +4869,11 @@ function wp_generate_user_request_key( $request_id ) { * * @since 4.9.6 * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. - * * @param string $request_id ID of the request being confirmed. * @param string $key Provided key to validate. * @return true|WP_Error True on success, WP_Error on failure. */ function wp_validate_user_request_key( $request_id, $key ) { - global $wp_hasher; - $request_id = absint( $request_id ); $request = wp_get_user_request( $request_id ); $saved_key = $request->confirm_key; @@ -4923,11 +4891,6 @@ function wp_validate_user_request_key( $request_id, $key ) { return new WP_Error( 'missing_key', __( 'The confirmation key is missing from this personal data request.' ) ); } - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - /** * Filters the expiration time of confirm keys. * @@ -4938,7 +4901,7 @@ function wp_validate_user_request_key( $request_id, $key ) { $expiration_duration = (int) apply_filters( 'user_request_key_expiration', DAY_IN_SECONDS ); $expiration_time = $key_request_time + $expiration_duration; - if ( ! $wp_hasher->CheckPassword( $key, $saved_key ) ) { + if ( ! password_verify( $key, $saved_key ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } From f12e93d49ea77b21c53f2062896f341f0109770e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:31:47 +0100 Subject: [PATCH 03/46] Update the handling of the recovery mode key. --- .../class-wp-recovery-mode-key-service.php | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/wp-includes/class-wp-recovery-mode-key-service.php b/src/wp-includes/class-wp-recovery-mode-key-service.php index 38d5730f85bf4..e245f3cddf701 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -37,24 +37,15 @@ public function generate_recovery_mode_token() { * Creates a recovery mode key. * * @since 5.2.0 - * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. + * @since x.y.z The stored key is now hashed using bcrypt instead of phpass. * * @param string $token A token generated by {@see generate_recovery_mode_token()}. * @return string Recovery mode key. */ public function generate_and_store_recovery_mode_key( $token ) { - - global $wp_hasher; - $key = wp_generate_password( 22, false ); - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - - $hashed = $wp_hasher->HashPassword( $key ); + $hashed = wp_hash_password( $key ); $records = $this->get_keys(); @@ -85,16 +76,12 @@ public function generate_and_store_recovery_mode_key( $token ) { * * @since 5.2.0 * - * @global PasswordHash $wp_hasher Portable PHP password hashing framework instance. - * * @param string $token The token used when generating the given key. - * @param string $key The unhashed key. + * @param string $key The plain text key. * @param int $ttl Time in seconds for the key to be valid for. * @return true|WP_Error True on success, error object on failure. */ public function validate_recovery_mode_key( $token, $key, $ttl ) { - global $wp_hasher; - $records = $this->get_keys(); if ( ! isset( $records[ $token ] ) ) { @@ -109,12 +96,7 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - $wp_hasher = new PasswordHash( 8, true ); - } - - if ( ! $wp_hasher->CheckPassword( $key, $record['hashed_key'] ) ) { + if ( ! password_verify( $key, $record['hashed_key'] ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } @@ -169,9 +151,20 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 + * @since x.y.z Each key should now be hashed using bcrypt instead of phpass, but + * existing keys may still be hashed using phpass. + * + * @return array { + * Associative array of token => data pairs, where the data is an associative + * array of information about the key. * - * @return array Associative array of $token => $data pairs, where $data has keys 'hashed_key' - * and 'created_at'. + * @type array ...$0 { + * Information about the key. + * + * @type string $hashed_key The hashed value of the key. + * @type int $created_at The timestamp when the key was created. + * } + * } */ private function get_keys() { return (array) get_option( $this->option_name, array() ); @@ -181,9 +174,19 @@ private function get_keys() { * Updates the recovery key records. * * @since 5.2.0 + * @since x.y.z Each key should now be hashed using bcrypt instead of phpass. + * + * @param array $keys { + * Associative array of token => data pairs, where the data is an associative + * array of information about the key. + * + * @type array ...$0 { + * Information about the key. * - * @param array $keys Associative array of $token => $data pairs, where $data has keys 'hashed_key' - * and 'created_at'. + * @type string $hashed_key The hashed value of the key. + * @type int $created_at The timestamp when the key was created. + * } + * } * @return bool True on success, false on failure. */ private function update_keys( array $keys ) { From 5a468099a4a92ea1d16200a224e5351c2094e5cd Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:32:08 +0100 Subject: [PATCH 04/46] Update the handling of post passwords. --- src/wp-includes/post-template.php | 8 +++----- src/wp-login.php | 7 +++---- tests/phpunit/tests/comment/wpHandleCommentSubmission.php | 3 +-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/wp-includes/post-template.php b/src/wp-includes/post-template.php index 4f3bfbef0cd7e..4c787a21f17c5 100644 --- a/src/wp-includes/post-template.php +++ b/src/wp-includes/post-template.php @@ -865,6 +865,7 @@ function get_body_class( $css_class = '' ) { * Determines whether the post requires password and whether a correct password has been provided. * * @since 2.7.0 + * @TODO test coverage * * @param int|WP_Post|null $post An optional post. Global $post used if not provided. * @return bool false if a password is not required or the correct password cookie is present, true otherwise. @@ -882,14 +883,11 @@ function post_password_required( $post = null ) { return apply_filters( 'post_password_required', true, $post ); } - require_once ABSPATH . WPINC . '/class-phpass.php'; - $hasher = new PasswordHash( 8, true ); - $hash = wp_unslash( $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] ); - if ( ! str_starts_with( $hash, '$P$B' ) ) { + if ( ! str_starts_with( $hash, '$' ) ) { $required = true; } else { - $required = ! $hasher->CheckPassword( $post->post_password, $hash ); + $required = ! wp_check_password( $post->post_password, $hash ); } /** diff --git a/src/wp-login.php b/src/wp-login.php index 45d7794c84ed7..cf9a705853898 100644 --- a/src/wp-login.php +++ b/src/wp-login.php @@ -762,9 +762,6 @@ function wp_login_viewport_meta() { exit; } - require_once ABSPATH . WPINC . '/class-phpass.php'; - $hasher = new PasswordHash( 8, true ); - /** * Filters the life span of the post password cookie. * @@ -784,7 +781,9 @@ function wp_login_viewport_meta() { $secure = false; } - setcookie( 'wp-postpass_' . COOKIEHASH, $hasher->HashPassword( wp_unslash( $_POST['post_password'] ) ), $expire, COOKIEPATH, COOKIE_DOMAIN, $secure ); + $hashed = wp_hash_password( wp_unslash( $_POST['post_password'] ) ); + + setcookie( 'wp-postpass_' . COOKIEHASH, $hashed, $expire, COOKIEPATH, COOKIE_DOMAIN, $secure ); wp_safe_redirect( wp_get_referer() ); exit; diff --git a/tests/phpunit/tests/comment/wpHandleCommentSubmission.php b/tests/phpunit/tests/comment/wpHandleCommentSubmission.php index bbba0735795fc..1668e7eecd3ed 100644 --- a/tests/phpunit/tests/comment/wpHandleCommentSubmission.php +++ b/tests/phpunit/tests/comment/wpHandleCommentSubmission.php @@ -197,9 +197,8 @@ public function test_submitting_comment_to_password_required_post_returns_error( public function test_submitting_comment_to_password_protected_post_succeeds() { $password = 'password'; - $hasher = new PasswordHash( 8, true ); - $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = $hasher->HashPassword( $password ); + $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = wp_hash_password( $password ); $post = self::factory()->post->create_and_get( array( From d4e4e473c5f63348bd041711baa96c08b8cecae4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:36:29 +0100 Subject: [PATCH 05/46] Automatically rehash user passwords and application passwords after they are validated. --- src/wp-includes/user.php | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index fa3787884f373..f4aa4cab614d8 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -200,7 +200,14 @@ function wp_authenticate_username_password( $user, $username, $password ) { return $user; } - if ( ! wp_check_password( $password, $user->user_pass, $user->ID ) ) { + $valid = wp_check_password( $password, $user->user_pass, $user->ID ); + + if ( $valid && wp_password_needs_rehash( $user->user_pass ) ) { + // @TODO test coverage + wp_set_password( $password, $user->ID ); + } + + if ( ! $valid ) { return new WP_Error( 'incorrect_password', sprintf( @@ -272,7 +279,14 @@ function wp_authenticate_email_password( $user, $email, $password ) { return $user; } - if ( ! wp_check_password( $password, $user->user_pass, $user->ID ) ) { + $valid = wp_check_password( $password, $user->user_pass, $user->ID ); + + if ( $valid && wp_password_needs_rehash( $user->user_pass ) ) { + // @TODO test coverage + wp_set_password( $password, $user->ID ); + } + + if ( ! $valid ) { return new WP_Error( 'incorrect_password', sprintf( @@ -425,10 +439,18 @@ function wp_authenticate_application_password( $input_user, $username, $password $hashed_passwords = WP_Application_Passwords::get_user_application_passwords( $user->ID ); foreach ( $hashed_passwords as $key => $item ) { - if ( ! wp_check_password( $password, $item['password'], $user->ID ) ) { + $valid = wp_check_password( $password, $item['password'], $user->ID ); + + if ( ! $valid ) { continue; } + if ( wp_password_needs_rehash( $item['password'] ) ) { + // @TODO test coverage + $item['password'] = wp_hash_password( $password ); + WP_Application_Passwords::update_application_password( $user->ID, $item['uuid'], $item ); + } + $error = new WP_Error(); /** From 638421df33fbd6f71a5649d388ade89e303339fc Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:38:24 +0100 Subject: [PATCH 06/46] Docs. --- src/wp-includes/class-wp-application-passwords.php | 3 +++ src/wp-includes/pluggable.php | 3 ++- tests/phpunit/tests/auth.php | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index 38ec4915dea48..f3f2dc2bdc64b 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -253,6 +253,7 @@ public static function application_name_exists_for_user( $user_id, $name ) { * Updates an application password. * * @since 5.6.0 + * @since x.y.z The actual password should now be hashed using bcrypt instead of phpass. See wp_hash_password(). * * @param int $user_id User ID. * @param string $uuid The password's UUID. @@ -300,6 +301,8 @@ public static function update_application_password( $user_id, $uuid, $update = a * Fires when an application password is updated. * * @since 5.6.0 + * @since x.y.z The password is now hashed using bcrypt instead of phpass, but + * existing passwords may still be hashed using phpass. * * @param int $user_id The user ID. * @param array $item { diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 4903c777b3f28..c45f65e43d701 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2641,7 +2641,8 @@ function wp_check_password( $password, $hash, $user_id = '' ) { * * Passwords are hashed with bcrypt using the default cost. A password hashed in a prior version * of WordPress may still be hashed with phpass and will need to be rehashed. If the default cost - * gets updated then a password hashed in a previous version of PHP will need to be rehashed. + * gets updated in PHP or the algorithm is changed in WordPress then a password hashed in a + * previous version of PHP or WordPress will need to be rehashed. * * @since x.y.z * diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 5029b283a5abb..3004069bfb685 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -172,6 +172,7 @@ public function test_wp_hash_password_trimming() { /** * @ticket 21022 + * @ticket 50027 */ public function test_wp_check_password_supports_fallback_phpass_hashes() { $password = 'password'; @@ -180,6 +181,7 @@ public function test_wp_check_password_supports_fallback_phpass_hashes() { /** * @ticket 21022 + * @ticket 50027 */ public function test_wp_check_password_does_not_support_md5_hashes() { $password = 'password'; @@ -188,6 +190,7 @@ public function test_wp_check_password_does_not_support_md5_hashes() { /** * @ticket 21022 + * @ticket 50027 */ public function test_wp_check_password_does_not_support_plain_text() { $password = 'password'; @@ -275,6 +278,7 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() * * @TODO bcrypt has a password length limit of 72, need to decide what our approach is. * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 + * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket 50027 * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html */ public function test_password_length_limit() { @@ -454,6 +458,7 @@ public function test_legacy_user_activation_key_is_rejected() { /** * @ticket 21022 + * @ticket 50027 * @ticket 32429 */ public function test_phpass_user_activation_key_is_rejected() { @@ -489,6 +494,7 @@ public function test_phpass_user_activation_key_is_rejected() { * that it works as expected. * * @ticket 21022 + * @ticket 50027 */ public function check_password_needs_rehashing() { $password = 'password'; From 316cb41eed3f601951adbe2097026ceb8dc2dbac Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 20:05:43 +0100 Subject: [PATCH 07/46] Juggle this a bit. --- tests/phpunit/tests/auth.php | 69 ++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 3004069bfb685..b406d72984212 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -274,14 +274,12 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() } /** - * See https://core.trac.wordpress.org/changeset/30466 . - * * @TODO bcrypt has a password length limit of 72, need to decide what our approach is. * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket 50027 * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html */ - public function test_password_length_limit() { + public function test_password_length_limit_with_bcrypt() { $limit = str_repeat( 'a', 72 ); wp_set_password( $limit, self::$user_id ); @@ -308,6 +306,54 @@ public function test_password_length_limit() { // Password broken by setting it to be too long. $this->assertSame( '*', $user->data->user_pass ); + $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); + // Wrong password. + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + + $user = wp_authenticate( $this->user->user_login, $limit ); + // Wrong password. + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + + $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); + // Password broken by setting it to be too long. + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + /** + * See https://core.trac.wordpress.org/changeset/30466 . + */ + public function test_password_length_limit_with_phpass() { + $limit = str_repeat( 'a', 4096 ); + + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( self::$user_id, $limit ); + + // phpass hashed password. + $this->assertStringStartsWith( '$P$', $this->user->data->user_pass ); + + $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); + // Wrong password. + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + + $user = wp_authenticate( $this->user->user_login, $limit ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + + // One char too many. + $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); + // Wrong password. + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + + wp_set_password( $limit . 'a', self::$user_id ); + $user = get_user_by( 'id', self::$user_id ); + // Password broken by setting it to be too long. + $this->assertSame( '*', $user->data->user_pass ); + $user = wp_authenticate( $this->user->user_login, '*' ); $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); @@ -493,6 +539,8 @@ public function test_phpass_user_activation_key_is_rejected() { * The `wp_password_needs_rehash()` function is just a wrapper around `password_needs_rehash()`, but this ensures * that it works as expected. * + * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . + * * @ticket 21022 * @ticket 50027 */ @@ -1065,4 +1113,19 @@ public function tests_basic_http_authentication_with_colon_in_password() { $this->assertSame( $_SERVER['PHP_AUTH_USER'], 'username' ); $this->assertSame( $_SERVER['PHP_AUTH_PW'], 'pass:word' ); } + + private static function set_password_with_phpass( string $password, int $user_id ) { + global $wpdb; + + $wpdb->update( + $wpdb->users, + array( + 'user_pass' => self::$wp_hasher->HashPassword( $password ), + ), + array( + 'ID' => $user_id, + ) + ); + clean_user_cache( $user_id ); + } } From 76fa51855b3b70c9c06f42b3286f0acedd3bc2ff Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 20:56:09 +0100 Subject: [PATCH 08/46] Trying to get these tests in order. --- tests/phpunit/tests/auth.php | 50 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index b406d72984212..b058fdc5c2028 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -274,6 +274,9 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() } /** + * @ticket 21022 + * @ticket 50027 + * * @TODO bcrypt has a password length limit of 72, need to decide what our approach is. * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket 50027 @@ -282,74 +285,81 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() public function test_password_length_limit_with_bcrypt() { $limit = str_repeat( 'a', 72 ); + // Set the user password. wp_set_password( $limit, self::$user_id ); + // Ensure the password is hashed with bcrypt. - $this->assertStringStartsWith( '$2y$', $this->user->data->user_pass ); + $this->assertStringStartsWith( '$2y$', get_userdata( self::$user_id )->user_pass ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. - $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertWPError( $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + // Correct password. $user = wp_authenticate( $this->user->user_login, $limit ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); - // One char too many. + // One char too many, still accepted by bcrypt. $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); - // Wrong password. - $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); + // Correct password. + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + // Set the password longer than the bcrypt limit. wp_set_password( $limit . 'a', self::$user_id ); - $user = get_user_by( 'id', self::$user_id ); - // Password broken by setting it to be too long. - $this->assertSame( '*', $user->data->user_pass ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. - $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertWPError( $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); $user = wp_authenticate( $this->user->user_login, $limit ); - // Wrong password. - $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); + // Correct password. + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); - // Password broken by setting it to be too long. - $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); + // Correct password. + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); } /** + * @ticket 21022 + * @ticket 50027 + * * See https://core.trac.wordpress.org/changeset/30466 . */ public function test_password_length_limit_with_phpass() { $limit = str_repeat( 'a', 4096 ); // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( self::$user_id, $limit ); + self::set_password_with_phpass( $limit, self::$user_id ); // phpass hashed password. - $this->assertStringStartsWith( '$P$', $this->user->data->user_pass ); + $this->assertStringStartsWith( '$P$', get_userdata( self::$user_id )->user_pass ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + // Correct password is accepted even though its hash in the database was hashed with phpass. $user = wp_authenticate( $this->user->user_login, $limit ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); // One char too many. - $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); + $long = $limit . 'a'; + $this->assertTrue( wp_password_needs_rehash( $long ) ); + $user = wp_authenticate( $this->user->user_login, $long ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); - wp_set_password( $limit . 'a', self::$user_id ); + self::set_password_with_phpass( $long, self::$user_id ); $user = get_user_by( 'id', self::$user_id ); // Password broken by setting it to be too long. $this->assertSame( '*', $user->data->user_pass ); From d7079398d300f61818622e605a797cf5b2a4bed1 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 22:41:43 +0100 Subject: [PATCH 09/46] More docs. --- src/wp-includes/user.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index f4aa4cab614d8..946f13dc17b10 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -3012,7 +3012,7 @@ function get_password_reset_key( $user ) { * * @since 3.1.0 * - * @param string $key Hash to validate sending user's password. + * @param string $key The password reset key. * @param string $login The user login. * @return WP_User|WP_Error WP_User object on success, WP_Error object for invalid or expired keys. */ @@ -3069,7 +3069,7 @@ function check_password_reset_key( $key, $login ) { /** * Filters the return value of check_password_reset_key() when an - * old-style key is used. + * old-style key or an expired key is used. * * @since 3.7.0 Previously plain-text keys were stored in the database. * @since 4.3.0 Previously key hashes were stored without an expiration time. @@ -3090,7 +3090,7 @@ function check_password_reset_key( $key, $login ) { * @since 2.5.0 * @since 5.7.0 Added `$user_login` parameter. * - * @global wpdb $wpdb WordPress database abstraction object. + * @global wpdb $wpdb WordPress database abstraction object. * * @param string $user_login Optional. Username to send a password retrieval email for. * Defaults to `$_POST['user_login']` if not set. From 4a796b7e2c3466a19b7ddca3a8c455d3e553ae31 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 23:43:55 +0100 Subject: [PATCH 10/46] Retain validity of phpass hashed password reset keys. --- src/wp-includes/user.php | 2 +- tests/phpunit/tests/auth.php | 40 +++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 946f13dc17b10..641825cbaa107 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -3054,7 +3054,7 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - $hash_is_correct = password_verify( $key, $pass_key ); + $hash_is_correct = wp_check_password( $key, $pass_key ); if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { return $user; diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index b058fdc5c2028..66e15f1512b0e 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -517,7 +517,7 @@ public function test_legacy_user_activation_key_is_rejected() { * @ticket 50027 * @ticket 32429 */ - public function test_phpass_user_activation_key_is_rejected() { + public function test_phpass_user_activation_key_is_allowed() { global $wpdb; // A legacy user_activation_key is one hashed using phpass between WordPress 4.3 and x.y.z. @@ -526,7 +526,7 @@ public function test_phpass_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . self::$wp_hasher->HashPassword( $key ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . self::$wp_hasher->HashPassword( $key ), ), array( 'ID' => $this->user->ID, @@ -534,10 +534,44 @@ public function test_phpass_user_activation_key_is_rejected() { ); clean_user_cache( $this->user ); - // A legacy phpass user_activation_key should not be accepted even when it is otherwise valid. + // A legacy phpass user_activation_key should remain valid. $check = check_password_reset_key( $key, $this->user->user_login ); + $this->assertNotWPError( $check ); + $this->assertInstanceOf( 'WP_User', $check ); + $this->assertSame( $this->user->ID, $check->ID ); + + // An empty key with a legacy user_activation_key should be rejected. + $check = check_password_reset_key( '', $this->user->user_login ); $this->assertWPError( $check ); $this->assertSame( 'invalid_key', $check->get_error_code() ); + } + + /** + * @ticket 21022 + * @ticket 50027 + * @ticket 32429 + */ + public function test_expired_phpass_user_activation_key_is_rejected() { + global $wpdb; + + // A legacy user_activation_key is one hashed using phpass between WordPress 4.3 and x.y.z. + + $key = wp_generate_password( 20, false ); + $wpdb->update( + $wpdb->users, + array( + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . self::$wp_hasher->HashPassword( $key ), + ), + array( + 'ID' => $this->user->ID, + ) + ); + clean_user_cache( $this->user ); + + // A legacy phpass user_activation_key should still be subject to an expiry check. + $check = check_password_reset_key( $key, $this->user->user_login ); + $this->assertWPError( $check ); + $this->assertSame( 'expired_key', $check->get_error_code() ); // An empty key with a legacy user_activation_key should be rejected. $check = check_password_reset_key( '', $this->user->user_login ); From 64dfd57dbe86aaabaf478394dc81cd1d3c0743db Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 23:46:56 +0100 Subject: [PATCH 11/46] Retain validity of a phpass hashed recovery mode key. --- src/wp-includes/class-wp-recovery-mode-key-service.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-recovery-mode-key-service.php b/src/wp-includes/class-wp-recovery-mode-key-service.php index e245f3cddf701..b993a283c41a0 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -96,7 +96,7 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - if ( ! password_verify( $key, $record['hashed_key'] ) ) { + if ( ! wp_check_password( $key, $record['hashed_key'] ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } From e161994921fc0a5b88e0534666ae00d0570df2a1 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 23:48:11 +0100 Subject: [PATCH 12/46] Retain validity of phpass hashed user request keys. --- src/wp-includes/user.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 641825cbaa107..fb6f2addaf915 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -4923,7 +4923,7 @@ function wp_validate_user_request_key( $request_id, $key ) { $expiration_duration = (int) apply_filters( 'user_request_key_expiration', DAY_IN_SECONDS ); $expiration_time = $key_request_time + $expiration_duration; - if ( ! password_verify( $key, $saved_key ) ) { + if ( ! wp_check_password( $key, $saved_key ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } From 548f937907b61f8a95234eda579c3f31681ab8d8 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 23:48:22 +0100 Subject: [PATCH 13/46] More docs. --- src/wp-includes/pluggable.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index c45f65e43d701..d19e2d66dc42b 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2584,6 +2584,11 @@ function wp_hash_password( $password ) { /** * Checks a plaintext password against a hashed password. * + * Note that this function is used for checking more than just user passwords, for + * example it's used for checking password reset keys, application passwords, post + * passwords, recovery mode keys, and more. There is not always a user ID associated + * with the password. + * * For integration with other applications, this function can be overwritten to * instead use the other package password hashing algorithm. * @@ -2594,9 +2599,9 @@ function wp_hash_password( $password ) { * @global PasswordHash $wp_hasher phpass object. Used as a fallback for verifying * passwords that were hashed with phpass. * - * @param string $password Plaintext user's password. - * @param string $hash Hash of the user's password to check against. - * @param string|int $user_id Optional. User ID. + * @param string $password Plaintext password. + * @param string $hash Hash of the password to check against. + * @param string|int $user_id Optional. ID of a user associated with the password. * @return bool False, if the $password does not match the hashed password. */ function wp_check_password( $password, $hash, $user_id = '' ) { @@ -2620,7 +2625,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { } /** - * Filters whether the plaintext password matches the encrypted password. + * Filters whether the plaintext password matches the hashed password. * * @since 2.5.0 * @since x.y.z Passwords are now hashed with bcrypt by default. @@ -2629,7 +2634,8 @@ function wp_check_password( $password, $hash, $user_id = '' ) { * @param bool $check Whether the passwords match. * @param string $password The plaintext password. * @param string $hash The hashed password. - * @param string|int $user_id User ID. Can be empty. + * @param string|int $user_id Optional ID of a user associated with the password. + * Can be empty. */ return apply_filters( 'check_password', $check, $password, $hash, $user_id ); } From fe9b053e67e2873648f3e5b348758cfda59d03c2 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 01:40:59 +0100 Subject: [PATCH 14/46] Add tests for password rehashing when signing in with a username or email address. --- src/wp-includes/user.php | 2 -- tests/phpunit/tests/auth.php | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index fb6f2addaf915..d2b7e2dfe2e64 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -203,7 +203,6 @@ function wp_authenticate_username_password( $user, $username, $password ) { $valid = wp_check_password( $password, $user->user_pass, $user->ID ); if ( $valid && wp_password_needs_rehash( $user->user_pass ) ) { - // @TODO test coverage wp_set_password( $password, $user->ID ); } @@ -282,7 +281,6 @@ function wp_authenticate_email_password( $user, $email, $password ) { $valid = wp_check_password( $password, $user->user_pass, $user->ID ); if ( $valid && wp_password_needs_rehash( $user->user_pass ) ) { - // @TODO test coverage wp_set_password( $password, $user->ID ); } diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 66e15f1512b0e..4e12e21f275f9 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -669,6 +669,53 @@ public function test_user_activation_key_after_successful_login() { $this->assertEmpty( $activation_key_from_database, 'The `user_activation_key` was not empty in the database.' ); } + /** + * @dataProvider data_usernames + */ + public function test_phpass_password_is_rehashed_after_successful_login( $username_or_email ) { + $password = 'password'; + + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( $password, self::$user_id ); + + // Verify that the password is hashed with phpass. + $hash = get_userdata( self::$user_id )->user_pass; + $this->assertStringStartsWith( '$P$', $hash ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); + + // Authenticate. + $user = wp_authenticate( $username_or_email, $password ); + + // Verify that the phpass password hash was valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + + // Verify that the password has been rehashed with bcrypt. + $hash = get_userdata( self::$user_id )->user_pass; + $this->assertStringStartsWith( '$2y$', $hash ); + $this->assertFalse( wp_password_needs_rehash( $hash ) ); + + // Authenticate a second time to ensure the new hash is valid. + $user = wp_authenticate( $username_or_email, $password ); + + // Verify that the bcrypt password hash is valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + } + + public function data_usernames() { + return array( + array( + self::USER_LOGIN, + ), + array( + self::USER_EMAIL, + ), + ); + } + /** * Ensure users can log in using both their username and their email address. * From 33cf000ce11b46521a0a8471c301d116882e305b Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 10:25:03 +0100 Subject: [PATCH 15/46] Docs. --- src/wp-includes/class-wp-recovery-mode-key-service.php | 8 ++++---- src/wp-includes/pluggable.php | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/class-wp-recovery-mode-key-service.php b/src/wp-includes/class-wp-recovery-mode-key-service.php index b993a283c41a0..114fa68e2ce9b 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -155,8 +155,8 @@ private function remove_key( $token ) { * existing keys may still be hashed using phpass. * * @return array { - * Associative array of token => data pairs, where the data is an associative - * array of information about the key. + * Associative array of token => data pairs, where the data is an associative + * array of information about the key. * * @type array ...$0 { * Information about the key. @@ -177,8 +177,8 @@ private function get_keys() { * @since x.y.z Each key should now be hashed using bcrypt instead of phpass. * * @param array $keys { - * Associative array of token => data pairs, where the data is an associative - * array of information about the key. + * Associative array of token => data pairs, where the data is an associative + * array of information about the key. * * @type array ...$0 { * Information about the key. diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index d19e2d66dc42b..d989cd1730450 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2595,6 +2595,7 @@ function wp_hash_password( $password ) { * @since 2.5.0 * @since x.y.z Passwords in WordPress are now hashed with bcrypt by default. A * password that wasn't hashed with bcrypt will be checked with phpass. + * Passwords hashed with md5 are no longer supported. * * @global PasswordHash $wp_hasher phpass object. Used as a fallback for verifying * passwords that were hashed with phpass. From d51cbc29f44b0cc5bf14d19219cf8ee39d2925b4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 10:26:46 +0100 Subject: [PATCH 16/46] Ensure `wp_check_password()` remains compatible with changes to the default bcrypt cost. --- src/wp-includes/pluggable.php | 4 +- tests/phpunit/tests/auth.php | 81 ++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index d989cd1730450..1ae2c9cc6185a 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2613,7 +2613,9 @@ function wp_check_password( $password, $hash, $user_id = '' ) { return false; } - if ( wp_password_needs_rehash( $hash ) ) { + $needs_rehash = wp_password_needs_rehash( $hash ); + + if ( $needs_rehash && str_starts_with( $hash, '$P$' ) ) { if ( empty( $wp_hasher ) ) { require_once ABSPATH . WPINC . '/class-phpass.php'; // Use the portable hash from phpass. diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 4e12e21f275f9..69036ac886972 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -117,6 +117,7 @@ public function test_password_trimming() { wp_set_password( $password_to_test, $this->user->ID ); $authed_user = wp_authenticate( $this->user->user_login, $password_to_test ); + $this->assertNotWPError( $authed_user ); $this->assertInstanceOf( 'WP_User', $authed_user ); $this->assertSame( $this->user->ID, $authed_user->ID ); } @@ -176,7 +177,66 @@ public function test_wp_hash_password_trimming() { */ public function test_wp_check_password_supports_fallback_phpass_hashes() { $password = 'password'; - $this->assertTrue( wp_check_password( $password, self::$wp_hasher->HashPassword( $password ) ) ); + $hash = self::$wp_hasher->HashPassword( $password ); + $this->assertTrue( wp_check_password( $password, $hash ) ); + } + + /** + * Ensure wp_check_password() remains compatible with increases to the default bcrypt cost. + * + * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . + * + * It does this by reducing the cost used to generate the hash, therefore mimicing a hash which + * was generated prior to the default cost being increased. + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost() { + $password = 'password'; + $default = self::get_default_bcrypt_cost(); + $options = array( + // Reducing the cost mimics an increase in the default cost. + 'cost' => $default - 1, + ); + $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + $this->assertTrue( wp_check_password( $password, $hash ) ); + } + + /** + * Ensure wp_check_password() remains compatible with decreases to the default bcrypt cost. + * + * This is unlikely to occur but is fully supported. + * + * It does this by increasing the cost used to generate the hash, therefore mimicing a hash which + * was generated prior to the default cost being reduced. + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_hash_with_reduced_bcrypt_cost() { + $password = 'password'; + $default = self::get_default_bcrypt_cost(); + $options = array( + // Increasing the cost mimics a decrease in the default cost. + 'cost' => $default + 1, + ); + $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + $this->assertTrue( wp_check_password( $password, $hash ) ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_hash_with_default_bcrypt_cost() { + $password = 'password'; + $default = self::get_default_bcrypt_cost(); + $options = array( + 'cost' => $default, + ); + $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + $this->assertTrue( wp_check_password( $password, $hash ) ); } /** @@ -185,7 +245,8 @@ public function test_wp_check_password_supports_fallback_phpass_hashes() { */ public function test_wp_check_password_does_not_support_md5_hashes() { $password = 'password'; - $this->assertFalse( wp_check_password( $password, md5( $password ) ) ); + $hash = md5( $password ); + $this->assertFalse( wp_check_password( $password, $hash ) ); } /** @@ -194,7 +255,8 @@ public function test_wp_check_password_does_not_support_md5_hashes() { */ public function test_wp_check_password_does_not_support_plain_text() { $password = 'password'; - $this->assertFalse( wp_check_password( $password, $password ) ); + $hash = $password; + $this->assertFalse( wp_check_password( $password, $hash ) ); } /** @@ -596,8 +658,10 @@ public function check_password_needs_rehashing() { $this->assertFalse( wp_password_needs_rehash( $hash ) ); // A future upgrade from a previously lower cost. - $opts = array( - 'cost' => 8, + $default = self::get_default_bcrypt_cost(); + $opts = array( + // Reducing the cost mimics an increase in the default cost. + 'cost' => $default - 1, ); $hash = password_hash( $password, PASSWORD_BCRYPT, $opts ); $this->assertTrue( wp_password_needs_rehash( $hash ) ); @@ -1219,4 +1283,11 @@ private static function set_password_with_phpass( string $password, int $user_id ); clean_user_cache( $user_id ); } + + private static function get_default_bcrypt_cost() { + $hash = password_hash( 'password', PASSWORD_BCRYPT ); + $matches = array(); + preg_match( '/^\$2y\$(\d+)\$/', $hash, $matches ); + return (int) $matches[1]; + } } From 6b5441cc24ba208fbff45b2a7c1aa66f634be6f9 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 10:39:40 +0100 Subject: [PATCH 17/46] Simplify this logic. --- src/wp-includes/pluggable.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 1ae2c9cc6185a..34165e00068c4 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2613,9 +2613,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { return false; } - $needs_rehash = wp_password_needs_rehash( $hash ); - - if ( $needs_rehash && str_starts_with( $hash, '$P$' ) ) { + if ( str_starts_with( $hash, '$P$' ) ) { if ( empty( $wp_hasher ) ) { require_once ABSPATH . WPINC . '/class-phpass.php'; // Use the portable hash from phpass. From 89e0645380a236851ac96c4a2f5735bfe63e396d Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 11:50:49 +0100 Subject: [PATCH 18/46] Add tests for Argon2 support. --- tests/phpunit/tests/auth.php | 40 +++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 69036ac886972..8bc190a09401d 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -175,14 +175,14 @@ public function test_wp_hash_password_trimming() { * @ticket 21022 * @ticket 50027 */ - public function test_wp_check_password_supports_fallback_phpass_hashes() { + public function test_wp_check_password_supports_phpass_hash() { $password = 'password'; $hash = self::$wp_hasher->HashPassword( $password ); $this->assertTrue( wp_check_password( $password, $hash ) ); } /** - * Ensure wp_check_password() remains compatible with increases to the default bcrypt cost. + * Ensure wp_check_password() remains compatible with an increase to the default bcrypt cost. * * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . * @@ -204,7 +204,7 @@ public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost( } /** - * Ensure wp_check_password() remains compatible with decreases to the default bcrypt cost. + * Ensure wp_check_password() remains compatible with a decrease to the default bcrypt cost. * * This is unlikely to occur but is fully supported. * @@ -239,6 +239,40 @@ public function test_wp_check_password_supports_hash_with_default_bcrypt_cost() $this->assertTrue( wp_check_password( $password, $hash ) ); } + /** + * Ensure wp_check_password() is compatible with Argon2i hashes. + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_argon2i_hash() { + if ( ! defined( 'PASSWORD_ARGON2I' ) ) { + $this->fail( 'Argon2i is not supported.' ); + } + + $password = 'password'; + $hash = password_hash( trim( $password ), PASSWORD_ARGON2I ); + $this->assertTrue( wp_check_password( $password, $hash ) ); + } + + /** + * Ensure wp_check_password() is compatible with Argon2id hashes. + * + * @requires PHP >= 7.3 + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_argon2id_hash() { + if ( ! defined( 'PASSWORD_ARGON2ID' ) ) { + $this->fail( 'Argon2id is not supported.' ); + } + + $password = 'password'; + $hash = password_hash( trim( $password ), PASSWORD_ARGON2ID ); + $this->assertTrue( wp_check_password( $password, $hash ) ); + } + /** * @ticket 21022 * @ticket 50027 From 085b73ccdda7c6ff90f3c2bf5e726c9d257ed0ac Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 12:01:45 +0100 Subject: [PATCH 19/46] Always pass the return value of `wp_check_password()` through the `check_password` filter. --- src/wp-includes/pluggable.php | 31 +++++++++++++++++-------------- tests/phpunit/tests/auth.php | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 34165e00068c4..369000eb210c2 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2610,7 +2610,22 @@ function wp_check_password( $password, $hash, $user_id = '' ) { // If the hash is still md5 or otherwise truncated then invalidate it. if ( strlen( $hash ) <= 32 ) { - return false; + $check = false; + + /** + * Filters whether the plaintext password matches the hashed password. + * + * @since 2.5.0 + * @since x.y.z Passwords are now hashed with bcrypt by default. + * Old passwords may still be hashed with phpass. + * + * @param bool $check Whether the passwords match. + * @param string $password The plaintext password. + * @param string $hash The hashed password. + * @param string|int $user_id Optional ID of a user associated with the password. + * Can be empty. + */ + return apply_filters( 'check_password', $check, $password, $hash, $user_id ); } if ( str_starts_with( $hash, '$P$' ) ) { @@ -2625,19 +2640,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { $check = password_verify( $password, $hash ); } - /** - * Filters whether the plaintext password matches the hashed password. - * - * @since 2.5.0 - * @since x.y.z Passwords are now hashed with bcrypt by default. - * Old passwords may still be hashed with phpass. - * - * @param bool $check Whether the passwords match. - * @param string $password The plaintext password. - * @param string $hash The hashed password. - * @param string|int $user_id Optional ID of a user associated with the password. - * Can be empty. - */ + /** This filter is documented in wp-includes/pluggable.php */ return apply_filters( 'check_password', $check, $password, $hash, $user_id ); } endif; diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 8bc190a09401d..19a4d72a69889 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -179,6 +179,7 @@ public function test_wp_check_password_supports_phpass_hash() { $password = 'password'; $hash = self::$wp_hasher->HashPassword( $password ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -201,6 +202,7 @@ public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost( ); $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -223,6 +225,7 @@ public function test_wp_check_password_supports_hash_with_reduced_bcrypt_cost() ); $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -237,6 +240,7 @@ public function test_wp_check_password_supports_hash_with_default_bcrypt_cost() ); $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -253,6 +257,7 @@ public function test_wp_check_password_supports_argon2i_hash() { $password = 'password'; $hash = password_hash( trim( $password ), PASSWORD_ARGON2I ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -271,6 +276,7 @@ public function test_wp_check_password_supports_argon2id_hash() { $password = 'password'; $hash = password_hash( trim( $password ), PASSWORD_ARGON2ID ); $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -281,6 +287,7 @@ public function test_wp_check_password_does_not_support_md5_hashes() { $password = 'password'; $hash = md5( $password ); $this->assertFalse( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** @@ -291,6 +298,18 @@ public function test_wp_check_password_does_not_support_plain_text() { $password = 'password'; $hash = $password; $this->assertFalse( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_does_not_support_empty_value() { + $password = 'password'; + $hash = ''; + $this->assertFalse( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); } /** From 3e078a767624aa44810641554aa32013ad1ac7c9 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 15:15:18 +0100 Subject: [PATCH 20/46] Reintroduce support for the `$wp_hasher` global if it's set. --- src/wp-includes/pluggable.php | 29 +++++++++---- tests/phpunit/includes/bootstrap.php | 1 + .../phpunit/includes/class-wp-fake-hasher.php | 41 +++++++++++++++++++ tests/phpunit/tests/auth.php | 22 ++++++++++ 4 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 tests/phpunit/includes/class-wp-fake-hasher.php diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 369000eb210c2..7c00c92bd4815 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2572,10 +2572,18 @@ function wp_hash( $data, $scheme = 'auth' ) { * @since 2.5.0 * @since x.y.z The password is now hashed using bcrypt instead of phpass. * + * @global PasswordHash $wp_hasher phpass object. + * * @param string $password Plain text user password to hash. * @return string The hash string of the password. */ function wp_hash_password( $password ) { + global $wp_hasher; + + if ( ! empty( $wp_hasher ) ) { + return $wp_hasher->HashPassword( trim( $password ) ); + } + return password_hash( trim( $password ), PASSWORD_BCRYPT ); } endif; @@ -2628,14 +2636,13 @@ function wp_check_password( $password, $hash, $user_id = '' ) { return apply_filters( 'check_password', $check, $password, $hash, $user_id ); } - if ( str_starts_with( $hash, '$P$' ) ) { - if ( empty( $wp_hasher ) ) { - require_once ABSPATH . WPINC . '/class-phpass.php'; - // Use the portable hash from phpass. - $wp_hasher = new PasswordHash( 8, true ); - } - + if ( ! empty( $wp_hasher ) ) { $check = $wp_hasher->CheckPassword( $password, $hash ); + } elseif ( str_starts_with( $hash, '$P$' ) ) { + require_once ABSPATH . WPINC . '/class-phpass.php'; + // Use the portable hash from phpass. + $hasher = new PasswordHash( 8, true ); + $check = $hasher->CheckPassword( $password, $hash ); } else { $check = password_verify( $password, $hash ); } @@ -2656,10 +2663,18 @@ function wp_check_password( $password, $hash, $user_id = '' ) { * * @since x.y.z * + * @global PasswordHash $wp_hasher phpass object. + * * @param string $hash Hash of a password to check. * @return bool Whether the hash needs to be rehashed. */ function wp_password_needs_rehash( $hash ) { + global $wp_hasher; + + if ( ! empty( $wp_hasher ) ) { + return false; + } + return password_needs_rehash( $hash, PASSWORD_BCRYPT ); } endif; diff --git a/tests/phpunit/includes/bootstrap.php b/tests/phpunit/includes/bootstrap.php index 15ec06b081440..27213c1656745 100644 --- a/tests/phpunit/includes/bootstrap.php +++ b/tests/phpunit/includes/bootstrap.php @@ -328,6 +328,7 @@ function wp_tests_options( $value ) { require __DIR__ . '/class-wp-rest-test-search-handler.php'; require __DIR__ . '/class-wp-rest-test-configurable-controller.php'; require __DIR__ . '/class-wp-fake-block-type.php'; +require __DIR__ . '/class-wp-fake-hasher.php'; require __DIR__ . '/class-wp-sitemaps-test-provider.php'; require __DIR__ . '/class-wp-sitemaps-empty-test-provider.php'; require __DIR__ . '/class-wp-sitemaps-large-test-provider.php'; diff --git a/tests/phpunit/includes/class-wp-fake-hasher.php b/tests/phpunit/includes/class-wp-fake-hasher.php new file mode 100644 index 0000000000000..733e03e5e20af --- /dev/null +++ b/tests/phpunit/includes/class-wp-fake-hasher.php @@ -0,0 +1,41 @@ +hash = str_repeat( 'a', 36 ); + } + + /** + * Hashes a password. + * + * @param string $password Password to hash. + * @return string Hashed password. + */ + public function HashPassword( string $password ) { + return $this->hash; + } + + /** + * Checks the password hash. + * + * @param string $password Password to check. + * @param string $hash Hash to check against. + * @return bool Whether the password hash is valid. + */ + public function CheckPassword( string $password, string $hash ) { + return $hash == $this->hash; + } +} diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 19a4d72a69889..df2104fe0eb8e 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -833,6 +833,28 @@ public function data_usernames() { ); } + public function test_password_checks_support_wp_hasher_fallback() { + global $wp_hasher; + + $password = 'password'; + + // Ensure the global $wp_hasher is set. + $wp_hasher = new WP_Fake_Hasher(); + + $hasher_hash = $wp_hasher->HashPassword( $password ); + $wp_hash = wp_hash_password( $password ); + $valid = wp_check_password( $password, $wp_hash ); + $needs_rehash = wp_password_needs_rehash( $wp_hash ); + + // Reset the global $wp_hasher. + $wp_hasher = null; + + $this->assertSame( $hasher_hash, $wp_hash ); + $this->assertTrue( $valid ); + $this->assertFalse( $needs_rehash ); + $this->assertSame( 1, did_filter( 'check_password' ) ); + } + /** * Ensure users can log in using both their username and their email address. * From be636be817777e00b1666d22b79bb92628268c8b Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 21:23:45 +0100 Subject: [PATCH 21/46] Test the tests. --- tests/phpunit/tests/auth.php | 47 +++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index df2104fe0eb8e..6c340a1ccad2f 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -388,6 +388,28 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() unset( $_REQUEST['_wpnonce'] ); } + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_password_is_hashed_and_verified_with_bcrypt() { + $password = 'password'; + + // Set the user password. + wp_set_password( $password, self::$user_id ); + + // Ensure the password is hashed with bcrypt. + $this->assertStringStartsWith( '$2y$', get_userdata( self::$user_id )->user_pass ); + + // Authenticate. + $user = wp_authenticate( $this->user->user_login, $password ); + + // Verify correct password. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + } + /** * @ticket 21022 * @ticket 50027 @@ -396,6 +418,7 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket 50027 * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html + * @TODO split up this test */ public function test_password_length_limit_with_bcrypt() { $limit = str_repeat( 'a', 72 ); @@ -403,9 +426,6 @@ public function test_password_length_limit_with_bcrypt() { // Set the user password. wp_set_password( $limit, self::$user_id ); - // Ensure the password is hashed with bcrypt. - $this->assertStringStartsWith( '$2y$', get_userdata( self::$user_id )->user_pass ); - $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertWPError( $user ); @@ -453,10 +473,8 @@ public function test_password_length_limit_with_phpass() { // Set the user password with the old phpass algorithm. self::set_password_with_phpass( $limit, self::$user_id ); - // phpass hashed password. - $this->assertStringStartsWith( '$P$', get_userdata( self::$user_id )->user_pass ); - $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); + // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); @@ -1344,6 +1362,23 @@ public function tests_basic_http_authentication_with_colon_in_password() { $this->assertSame( $_SERVER['PHP_AUTH_PW'], 'pass:word' ); } + /** + * Test the tests + * + * @covers Tests_Auth::set_password_with_phpass + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_set_password_with_phpass() { + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( 'password', self::$user_id ); + + // Ensure the password is hashed with phpass. + $hash = get_userdata( self::$user_id )->user_pass; + $this->assertStringStartsWith( '$P$', $hash ); + } + private static function set_password_with_phpass( string $password, int $user_id ) { global $wpdb; From 4265787bea3ae7f140caa4208ee31cfbfd871257 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 21:36:43 +0100 Subject: [PATCH 22/46] Docs. --- tests/phpunit/tests/auth.php | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 6c340a1ccad2f..19ca279b1bc6d 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -185,10 +185,10 @@ public function test_wp_check_password_supports_phpass_hash() { /** * Ensure wp_check_password() remains compatible with an increase to the default bcrypt cost. * - * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . + * The test verifies this by reducing the cost used to generate the hash, therefore mimicing a hash + * which was generated prior to the default cost being increased. * - * It does this by reducing the cost used to generate the hash, therefore mimicing a hash which - * was generated prior to the default cost being increased. + * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . * * @ticket 21022 * @ticket 50027 @@ -197,7 +197,7 @@ public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost( $password = 'password'; $default = self::get_default_bcrypt_cost(); $options = array( - // Reducing the cost mimics an increase in the default cost. + // Reducing the cost mimics an increase to the default cost. 'cost' => $default - 1, ); $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); @@ -206,12 +206,12 @@ public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost( } /** - * Ensure wp_check_password() remains compatible with a decrease to the default bcrypt cost. + * Ensure wp_check_password() remains compatible with a reduction of the default bcrypt cost. * - * This is unlikely to occur but is fully supported. + * The test verifies this by increasing the cost used to generate the hash, therefore mimicing a hash + * which was generated prior to the default cost being reduced. * - * It does this by increasing the cost used to generate the hash, therefore mimicing a hash which - * was generated prior to the default cost being reduced. + * A reduction of the cost is unlikely to occur but is fully supported. * * @ticket 21022 * @ticket 50027 @@ -220,7 +220,7 @@ public function test_wp_check_password_supports_hash_with_reduced_bcrypt_cost() $password = 'password'; $default = self::get_default_bcrypt_cost(); $options = array( - // Increasing the cost mimics a decrease in the default cost. + // Increasing the cost mimics a reduction of the default cost. 'cost' => $default + 1, ); $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); @@ -392,7 +392,7 @@ public function test_check_ajax_referer_with_no_action_triggers_doing_it_wrong() * @ticket 21022 * @ticket 50027 */ - public function test_password_is_hashed_and_verified_with_bcrypt() { + public function test_password_is_hashed_with_bcrypt() { $password = 'password'; // Set the user password. @@ -423,7 +423,7 @@ public function test_password_is_hashed_and_verified_with_bcrypt() { public function test_password_length_limit_with_bcrypt() { $limit = str_repeat( 'a', 72 ); - // Set the user password. + // Set the user password to the bcrypt limit. wp_set_password( $limit, self::$user_id ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); @@ -462,10 +462,7 @@ public function test_password_length_limit_with_bcrypt() { } /** - * @ticket 21022 - * @ticket 50027 - * - * See https://core.trac.wordpress.org/changeset/30466 . + * @see https://core.trac.wordpress.org/changeset/30466 */ public function test_password_length_limit_with_phpass() { $limit = str_repeat( 'a', 4096 ); @@ -473,6 +470,7 @@ public function test_password_length_limit_with_phpass() { // Set the user password with the old phpass algorithm. self::set_password_with_phpass( $limit, self::$user_id ); + // Authenticate. $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. @@ -648,7 +646,6 @@ public function test_legacy_user_activation_key_is_rejected() { /** * @ticket 21022 * @ticket 50027 - * @ticket 32429 */ public function test_phpass_user_activation_key_is_allowed() { global $wpdb; @@ -682,7 +679,6 @@ public function test_phpass_user_activation_key_is_allowed() { /** * @ticket 21022 * @ticket 50027 - * @ticket 32429 */ public function test_expired_phpass_user_activation_key_is_rejected() { global $wpdb; @@ -807,7 +803,7 @@ public function test_user_activation_key_after_successful_login() { /** * @dataProvider data_usernames */ - public function test_phpass_password_is_rehashed_after_successful_login( $username_or_email ) { + public function test_phpass_password_is_rehashed_after_successful_authentication( $username_or_email ) { $password = 'password'; // Set the user password with the old phpass algorithm. From fd6bcdd8531623fd4a82e74d036e08904b2433f6 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 21:38:33 +0100 Subject: [PATCH 23/46] Start splitting up and fixing these tests. --- tests/phpunit/tests/auth.php | 109 +++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 24 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 19ca279b1bc6d..3fbff9a8192b4 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -30,6 +30,10 @@ class Tests_Auth extends WP_UnitTestCase { */ protected static $wp_hasher; + protected static $bcrypt_length_limit = 72; + + protected static $phpass_length_limit = 4096; + /** * Action hook. */ @@ -420,8 +424,8 @@ public function test_password_is_hashed_with_bcrypt() { * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html * @TODO split up this test */ - public function test_password_length_limit_with_bcrypt() { - $limit = str_repeat( 'a', 72 ); + public function test_invalid_password_at_bcrypt_length_limit_is_rejected() { + $limit = str_repeat( 'a', self::$bcrypt_length_limit ); // Set the user password to the bcrypt limit. wp_set_password( $limit, self::$user_id ); @@ -430,33 +434,46 @@ public function test_password_length_limit_with_bcrypt() { // Wrong password. $this->assertWPError( $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } - // Correct password. - $user = wp_authenticate( $this->user->user_login, $limit ); - $this->assertInstanceOf( 'WP_User', $user ); - $this->assertSame( self::$user_id, $user->ID ); - - // One char too many, still accepted by bcrypt. - $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); - // Correct password. - $this->assertInstanceOf( 'WP_User', $user ); - $this->assertSame( self::$user_id, $user->ID ); + public function test_invalid_password_beyond_bcrypt_length_limit_is_rejected() { + $limit = str_repeat( 'a', self::$bcrypt_length_limit + 1 ); - // Set the password longer than the bcrypt limit. - wp_set_password( $limit . 'a', self::$user_id ); + // Set the user password beyond the bcrypt limit. + wp_set_password( $limit, self::$user_id ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertWPError( $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_valid_password_at_bcrypt_length_limit_is_accepted() { + $limit = str_repeat( 'a', self::$bcrypt_length_limit ); + + // Set the user password to the bcrypt limit. + wp_set_password( $limit, self::$user_id ); + // Authenticate. $user = wp_authenticate( $this->user->user_login, $limit ); + // Correct password. + $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); + } - $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); - // Correct password. + public function test_valid_password_beyond_bcrypt_length_limit_is_accepted() { + $limit = str_repeat( 'a', self::$bcrypt_length_limit + 1 ); + + // Set the user password beyond the bcrypt limit. + wp_set_password( $limit, self::$user_id ); + + // Authenticate. + $user = wp_authenticate( $this->user->user_login, $limit ); + + // Correct password depite its length. + $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); } @@ -464,8 +481,8 @@ public function test_password_length_limit_with_bcrypt() { /** * @see https://core.trac.wordpress.org/changeset/30466 */ - public function test_password_length_limit_with_phpass() { - $limit = str_repeat( 'a', 4096 ); + public function test_invalid_password_at_phpass_length_limit_is_rejected() { + $limit = str_repeat( 'a', self::$phpass_length_limit ); // Set the user password with the old phpass algorithm. self::set_password_with_phpass( $limit, self::$user_id ); @@ -476,46 +493,90 @@ public function test_password_length_limit_with_phpass() { // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } - // Correct password is accepted even though its hash in the database was hashed with phpass. + public function test_valid_password_at_phpass_length_limit_is_accepted() { + $limit = str_repeat( 'a', self::$phpass_length_limit ); + + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( $limit, self::$user_id ); + + // Authenticate. $user = wp_authenticate( $this->user->user_login, $limit ); + + // Correct password. + $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); + } + + public function test_too_long_password_at_phpass_length_limit_is_rejected() { + $limit = str_repeat( 'a', self::$phpass_length_limit ); + + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( $limit, self::$user_id ); + + // Authenticate with a password that is one character too long. + $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); - // One char too many. - $long = $limit . 'a'; - $this->assertTrue( wp_password_needs_rehash( $long ) ); - $user = wp_authenticate( $this->user->user_login, $long ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_too_long_password_beyond_phpass_length_limit_is_rejected() { + // One char too many. + $too_long = str_repeat( 'a', self::$phpass_length_limit + 1 ); + + // Set the user password with the old phpass algorithm. + self::set_password_with_phpass( $too_long, self::$user_id ); - self::set_password_with_phpass( $long, self::$user_id ); $user = get_user_by( 'id', self::$user_id ); // Password broken by setting it to be too long. $this->assertSame( '*', $user->data->user_pass ); + // Password is not accepted. $user = wp_authenticate( $this->user->user_login, '*' ); $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_f() { + $this->markTestIncomplete(); $user = wp_authenticate( $this->user->user_login, '*0' ); $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_g() { + $this->markTestIncomplete(); $user = wp_authenticate( $this->user->user_login, '*1' ); $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_h() { + $this->markTestIncomplete(); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_i() { + $this->markTestIncomplete(); $user = wp_authenticate( $this->user->user_login, $limit ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + public function test_j() { + $this->markTestIncomplete(); $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); // Password broken by setting it to be too long. From 392e61211396d52b05e218bdffedaa543c3fbc8d Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Sep 2024 21:43:59 +0100 Subject: [PATCH 24/46] Coding standards. --- tests/phpunit/includes/class-wp-fake-hasher.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/includes/class-wp-fake-hasher.php b/tests/phpunit/includes/class-wp-fake-hasher.php index 733e03e5e20af..573362e375fb5 100644 --- a/tests/phpunit/includes/class-wp-fake-hasher.php +++ b/tests/phpunit/includes/class-wp-fake-hasher.php @@ -24,7 +24,7 @@ public function __construct() { * @param string $password Password to hash. * @return string Hashed password. */ - public function HashPassword( string $password ) { + public function HashPassword( string $password ) { // phpcs:ignore WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid return $this->hash; } @@ -35,7 +35,7 @@ public function HashPassword( string $password ) { * @param string $hash Hash to check against. * @return bool Whether the password hash is valid. */ - public function CheckPassword( string $password, string $hash ) { - return $hash == $this->hash; + public function CheckPassword( string $password, string $hash ) { // phpcs:ignore WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid + return $hash === $this->hash; } } From 904019333251d1f999cbf27d01810feaaef91e86 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Fri, 13 Sep 2024 14:33:04 +0100 Subject: [PATCH 25/46] Add a todo. --- src/wp-includes/pluggable.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 7c00c92bd4815..aed92c713399a 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2584,6 +2584,7 @@ function wp_hash_password( $password ) { return $wp_hasher->HashPassword( trim( $password ) ); } + // @TODO this can return false, null, or throw an exception return password_hash( trim( $password ), PASSWORD_BCRYPT ); } endif; From bc99ca16123cc48fd80acf9dd205d29d4ddeb0d1 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 10:19:20 -0700 Subject: [PATCH 26/46] Add some more assertions to the application password auth tests. --- tests/phpunit/tests/auth.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 3fbff9a8192b4..4185adbe73819 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1154,7 +1154,9 @@ public function test_application_password_authentication() { * @ticket 42790 */ public function test_authenticate_application_password_respects_existing_user() { - $this->assertSame( self::$_user, wp_authenticate_application_password( self::$_user, self::$_user->user_login, 'password' ) ); + $user = wp_authenticate_application_password( self::$_user, self::$_user->user_login, 'password' ); + $this->assertNotWPError( $user ); + $this->assertSame( self::$_user, $user ); } /** @@ -1163,7 +1165,9 @@ public function test_authenticate_application_password_respects_existing_user() public function test_authenticate_application_password_is_rejected_if_not_api_request() { add_filter( 'application_password_is_api_request', '__return_false' ); - $this->assertNull( wp_authenticate_application_password( null, self::$_user->user_login, 'password' ) ); + $user = wp_authenticate_application_password( null, self::$_user->user_login, 'password' ); + $this->assertNotWPError( $user ); + $this->assertNull( $user ); } /** @@ -1256,6 +1260,7 @@ public function test_authenticate_application_password_by_username() { list( $password ) = WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => 'phpunit' ) ); $user = wp_authenticate_application_password( null, self::$_user->user_login, $password ); + $this->assertNotWPError( $user ); $this->assertInstanceOf( WP_User::class, $user ); $this->assertSame( self::$user_id, $user->ID ); } @@ -1270,6 +1275,7 @@ public function test_authenticate_application_password_by_email() { list( $password ) = WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => 'phpunit' ) ); $user = wp_authenticate_application_password( null, self::$_user->user_email, $password ); + $this->assertNotWPError( $user ); $this->assertInstanceOf( WP_User::class, $user ); $this->assertSame( self::$user_id, $user->ID ); } @@ -1284,6 +1290,7 @@ public function test_authenticate_application_password_chunked() { list( $password ) = WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => 'phpunit' ) ); $user = wp_authenticate_application_password( null, self::$_user->user_email, WP_Application_Passwords::chunk_password( $password ) ); + $this->assertNotWPError( $user ); $this->assertInstanceOf( WP_User::class, $user ); $this->assertSame( self::$user_id, $user->ID ); } @@ -1295,6 +1302,7 @@ public function test_authenticate_application_password_returns_null_if_not_in_us delete_site_option( 'using_application_passwords' ); $authenticated = wp_authenticate_application_password( null, 'idonotexist', 'password' ); + $this->assertNotWPError( $authenticated ); $this->assertNull( $authenticated ); } From 58a89e20867fd9b6554388d41b963c7f8b7d4683 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 11:31:42 -0700 Subject: [PATCH 27/46] Correct and add tests for the application password rehashing. --- .../class-wp-application-passwords.php | 17 ++- tests/phpunit/tests/auth.php | 102 ++++++++++++++++-- 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index f3f2dc2bdc64b..81d8045eda2d7 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -279,22 +279,17 @@ public static function update_application_password( $user_id, $uuid, $update = a } if ( ! empty( $update['name'] ) ) { - $update['name'] = sanitize_text_field( $update['name'] ); + $item['name'] = sanitize_text_field( $update['name'] ); } - $save = false; - - if ( ! empty( $update['name'] ) && $item['name'] !== $update['name'] ) { - $item['name'] = $update['name']; - $save = true; + if ( ! empty( $update['password'] ) ) { + $item['password'] = $update['password']; } - if ( $save ) { - $saved = static::set_user_application_passwords( $user_id, $passwords ); + $saved = static::set_user_application_passwords( $user_id, $passwords ); - if ( ! $saved ) { - return new WP_Error( 'db_error', __( 'Could not save application password.' ) ); - } + if ( ! $saved ) { + return new WP_Error( 'db_error', __( 'Could not save application password.' ) ); } /** diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 4185adbe73819..27f320e8c46dd 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -485,7 +485,7 @@ public function test_invalid_password_at_phpass_length_limit_is_rejected() { $limit = str_repeat( 'a', self::$phpass_length_limit ); // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( $limit, self::$user_id ); + self::set_user_password_with_phpass( $limit, self::$user_id ); // Authenticate. $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); @@ -499,7 +499,7 @@ public function test_valid_password_at_phpass_length_limit_is_accepted() { $limit = str_repeat( 'a', self::$phpass_length_limit ); // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( $limit, self::$user_id ); + self::set_user_password_with_phpass( $limit, self::$user_id ); // Authenticate. $user = wp_authenticate( $this->user->user_login, $limit ); @@ -514,7 +514,7 @@ public function test_too_long_password_at_phpass_length_limit_is_rejected() { $limit = str_repeat( 'a', self::$phpass_length_limit ); // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( $limit, self::$user_id ); + self::set_user_password_with_phpass( $limit, self::$user_id ); // Authenticate with a password that is one character too long. $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); @@ -529,7 +529,7 @@ public function test_too_long_password_beyond_phpass_length_limit_is_rejected() $too_long = str_repeat( 'a', self::$phpass_length_limit + 1 ); // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( $too_long, self::$user_id ); + self::set_user_password_with_phpass( $too_long, self::$user_id ); $user = get_user_by( 'id', self::$user_id ); // Password broken by setting it to be too long. @@ -861,14 +861,52 @@ public function test_user_activation_key_after_successful_login() { $this->assertEmpty( $activation_key_from_database, 'The `user_activation_key` was not empty in the database.' ); } + public function test_phpass_password_is_rehashed_after_successful_application_password_authentication() { + add_filter( 'application_password_is_api_request', '__return_true' ); + add_filter( 'wp_is_application_passwords_available', '__return_true' ); + + $password = 'password'; + + // Set an application password with the old phpass algorithm. + $uuid = self::set_application_password_with_phpass( $password, self::$user_id ); + + // Verify that the password is hashed with phpass. + $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; + $this->assertStringStartsWith( '$P$', $hash ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); + $this->assertTrue( WP_Application_Passwords::is_in_use() ); + + // Authenticate. + $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); + + // Verify that the phpass password hash was valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + + // Verify that the password has been rehashed with bcrypt. + $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; + $this->assertStringStartsWith( '$2y$', $hash ); + $this->assertFalse( wp_password_needs_rehash( $hash ) ); + $this->assertTrue( WP_Application_Passwords::is_in_use() ); + + // Authenticate a second time to ensure the new hash is valid. + $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); + + // Verify that the bcrypt password hash is valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + } + /** * @dataProvider data_usernames */ - public function test_phpass_password_is_rehashed_after_successful_authentication( $username_or_email ) { + public function test_phpass_password_is_rehashed_after_successful_user_password_authentication( $username_or_email ) { $password = 'password'; // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( $password, self::$user_id ); + self::set_user_password_with_phpass( $password, self::$user_id ); // Verify that the password is hashed with phpass. $hash = get_userdata( self::$user_id )->user_pass; @@ -1430,21 +1468,21 @@ public function tests_basic_http_authentication_with_colon_in_password() { /** * Test the tests * - * @covers Tests_Auth::set_password_with_phpass + * @covers Tests_Auth::set_user_password_with_phpass * * @ticket 21022 * @ticket 50027 */ - public function test_set_password_with_phpass() { + public function test_set_user_password_with_phpass() { // Set the user password with the old phpass algorithm. - self::set_password_with_phpass( 'password', self::$user_id ); + self::set_user_password_with_phpass( 'password', self::$user_id ); // Ensure the password is hashed with phpass. $hash = get_userdata( self::$user_id )->user_pass; $this->assertStringStartsWith( '$P$', $hash ); } - private static function set_password_with_phpass( string $password, int $user_id ) { + private static function set_user_password_with_phpass( string $password, int $user_id ) { global $wpdb; $wpdb->update( @@ -1459,6 +1497,50 @@ private static function set_password_with_phpass( string $password, int $user_id clean_user_cache( $user_id ); } + /** + * Test the tests + * + * @covers Tests_Auth::set_application_password_with_phpass + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_set_application_password_with_phpass() { + // Set an application password with the old phpass algorithm. + $uuid = self::set_application_password_with_phpass( 'password', self::$user_id ); + + // Ensure the password is hashed with phpass. + $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; + $this->assertStringStartsWith( '$P$', $hash ); + } + + private static function set_application_password_with_phpass( string $password, int $user_id ) { + $uuid = wp_generate_uuid4(); + $item = array( + 'uuid' => $uuid, + 'app_id' => '', + 'name' => 'Test', + 'password' => self::$wp_hasher->HashPassword( $password ), + 'created' => time(), + 'last_used' => null, + 'last_ip' => null, + ); + + $saved = update_user_meta( + $user_id, + WP_Application_Passwords::USERMETA_KEY_APPLICATION_PASSWORDS, + array( $item ), + ); + + if ( ! $saved ) { + throw new Exception( 'Could not save application password.' ); + } + + update_network_option( get_main_network_id(), WP_Application_Passwords::OPTION_KEY_IN_USE, true ); + + return $uuid; + } + private static function get_default_bcrypt_cost() { $hash = password_hash( 'password', PASSWORD_BCRYPT ); $matches = array(); From 49871d01586916da2438a1364e5c61eb8ebe3b52 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 11:37:08 -0700 Subject: [PATCH 28/46] Done. --- src/wp-includes/user.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index d2b7e2dfe2e64..0888418741890 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -444,7 +444,6 @@ function wp_authenticate_application_password( $input_user, $username, $password } if ( wp_password_needs_rehash( $item['password'] ) ) { - // @TODO test coverage $item['password'] = wp_hash_password( $password ); WP_Application_Passwords::update_application_password( $user->ID, $item['uuid'], $item ); } From b42e57bb5e7f29338da47cbb7c82de47cbddd6e3 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 11:37:13 -0700 Subject: [PATCH 29/46] Coding standards. --- tests/phpunit/tests/auth.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 27f320e8c46dd..340894f1375d7 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -791,7 +791,7 @@ public function check_password_needs_rehashing() { // Reducing the cost mimics an increase in the default cost. 'cost' => $default - 1, ); - $hash = password_hash( $password, PASSWORD_BCRYPT, $opts ); + $hash = password_hash( $password, PASSWORD_BCRYPT, $opts ); $this->assertTrue( wp_password_needs_rehash( $hash ) ); // Previous phpass algorithm. @@ -1542,7 +1542,7 @@ private static function set_application_password_with_phpass( string $password, } private static function get_default_bcrypt_cost() { - $hash = password_hash( 'password', PASSWORD_BCRYPT ); + $hash = password_hash( 'password', PASSWORD_BCRYPT ); $matches = array(); preg_match( '/^\$2y\$(\d+)\$/', $hash, $matches ); return (int) $matches[1]; From 2baeb1990dfc0447a2e424688e497fa9862bc11d Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 13:19:45 -0700 Subject: [PATCH 30/46] Allow either the name or password to change in order to allow an application password to get updated. --- .../class-wp-application-passwords.php | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index 81d8045eda2d7..c2a0dff3ca18f 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -279,17 +279,27 @@ public static function update_application_password( $user_id, $uuid, $update = a } if ( ! empty( $update['name'] ) ) { - $item['name'] = sanitize_text_field( $update['name'] ); + $update['name'] = sanitize_text_field( $update['name'] ); } - if ( ! empty( $update['password'] ) ) { + $save = false; + + if ( ! empty( $update['name'] ) && $item['name'] !== $update['name'] ) { + $item['name'] = $update['name']; + $save = true; + } + + if ( ! empty( $update['password'] ) && $item['password'] !== $update['password'] ) { $item['password'] = $update['password']; + $save = true; } - $saved = static::set_user_application_passwords( $user_id, $passwords ); + if ( $save ) { + $saved = static::set_user_application_passwords( $user_id, $passwords ); - if ( ! $saved ) { - return new WP_Error( 'db_error', __( 'Could not save application password.' ) ); + if ( ! $saved ) { + return new WP_Error( 'db_error', __( 'Could not save application password.' ) ); + } } /** From 95f6d948ca354bfb443a625b0dbd0ad33c9ec7fa Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 17 Sep 2024 13:19:49 -0700 Subject: [PATCH 31/46] Docs. --- src/wp-includes/class-wp-application-passwords.php | 4 ++-- src/wp-includes/class-wp-recovery-mode-key-service.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index c2a0dff3ca18f..8f40a562a5658 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -306,8 +306,8 @@ public static function update_application_password( $user_id, $uuid, $update = a * Fires when an application password is updated. * * @since 5.6.0 - * @since x.y.z The password is now hashed using bcrypt instead of phpass, but - * existing passwords may still be hashed using phpass. + * @since x.y.z The password is now hashed using bcrypt instead of phpass. + * Existing passwords may still be hashed using phpass. * * @param int $user_id The user ID. * @param array $item { diff --git a/src/wp-includes/class-wp-recovery-mode-key-service.php b/src/wp-includes/class-wp-recovery-mode-key-service.php index 114fa68e2ce9b..1850b7cbcf2f3 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -151,8 +151,8 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key should now be hashed using bcrypt instead of phpass, but - * existing keys may still be hashed using phpass. + * @since x.y.z Each key is now hashed using bcrypt instead of phpass. + * Existing keys may still be hashed using phpass. * * @return array { * Associative array of token => data pairs, where the data is an associative From f8974eb6b25bce8a401847bc69c418c011692bc3 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 8 Oct 2024 11:46:59 +0100 Subject: [PATCH 32/46] PHP 7.2 compatibility. --- tests/phpunit/tests/auth.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 340894f1375d7..e8dd9ba7672cf 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1529,7 +1529,7 @@ private static function set_application_password_with_phpass( string $password, $saved = update_user_meta( $user_id, WP_Application_Passwords::USERMETA_KEY_APPLICATION_PASSWORDS, - array( $item ), + array( $item ) ); if ( ! $saved ) { From ff85df225fce70d5d0174a11529d8f13e5f2db26 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 8 Oct 2024 15:25:30 +0100 Subject: [PATCH 33/46] Docs. --- src/wp-includes/pluggable.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index aed92c713399a..a32e7aa1d207d 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2584,7 +2584,6 @@ function wp_hash_password( $password ) { return $wp_hasher->HashPassword( trim( $password ) ); } - // @TODO this can return false, null, or throw an exception return password_hash( trim( $password ), PASSWORD_BCRYPT ); } endif; @@ -2594,8 +2593,8 @@ function wp_hash_password( $password ) { * Checks a plaintext password against a hashed password. * * Note that this function is used for checking more than just user passwords, for - * example it's used for checking password reset keys, application passwords, post - * passwords, recovery mode keys, and more. There is not always a user ID associated + * example it's used for checking application passwords, post passwords, recovery mode + * keys, user password reset keys, and more. There is not always a user ID associated * with the password. * * For integration with other applications, this function can be overwritten to From ebcdd26fa4a4f251ee6f91bfc971fb4542fbce83 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 8 Oct 2024 15:25:55 +0100 Subject: [PATCH 34/46] Add a test to ensure the user's account password isn't touched when rehashing an application password. --- tests/phpunit/tests/auth.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index e8dd9ba7672cf..858896e287bd2 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -865,12 +865,13 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa add_filter( 'application_password_is_api_request', '__return_true' ); add_filter( 'wp_is_application_passwords_available', '__return_true' ); - $password = 'password'; + $password = 'password'; + $user_pass = get_userdata( self::$user_id )->user_pass; // Set an application password with the old phpass algorithm. $uuid = self::set_application_password_with_phpass( $password, self::$user_id ); - // Verify that the password is hashed with phpass. + // Verify that the application password is hashed with phpass. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; $this->assertStringStartsWith( '$P$', $hash ); $this->assertTrue( wp_password_needs_rehash( $hash ) ); @@ -879,21 +880,24 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa // Authenticate. $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); - // Verify that the phpass password hash was valid. + // Verify that the phpass hash for the application password was valid. $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); - // Verify that the password has been rehashed with bcrypt. + // Verify that the application password has been rehashed with bcrypt. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; $this->assertStringStartsWith( '$2y$', $hash ); $this->assertFalse( wp_password_needs_rehash( $hash ) ); $this->assertTrue( WP_Application_Passwords::is_in_use() ); + // Verify that the user's password has not been touched. + $this->assertSame( $user_pass, get_userdata( self::$user_id )->user_pass ); + // Authenticate a second time to ensure the new hash is valid. $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); - // Verify that the bcrypt password hash is valid. + // Verify that the bcrypt hashed application password is valid. $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); From ea8c56c872cebfcac0f4d440608f1a52290bd2c4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 8 Oct 2024 23:46:19 +0100 Subject: [PATCH 35/46] One fewer regexes in the world. --- tests/phpunit/tests/auth.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 858896e287bd2..07042a88aea24 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1546,9 +1546,9 @@ private static function set_application_password_with_phpass( string $password, } private static function get_default_bcrypt_cost() { - $hash = password_hash( 'password', PASSWORD_BCRYPT ); - $matches = array(); - preg_match( '/^\$2y\$(\d+)\$/', $hash, $matches ); - return (int) $matches[1]; + $hash = password_hash( 'password', PASSWORD_BCRYPT ); + $info = password_get_info( $hash ); + + return $info['options']['cost']; } } From 3a951ef457d8fe5415779aa1fe344ed2d74cca34 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Fri, 11 Oct 2024 22:43:54 +0100 Subject: [PATCH 36/46] Allow the password hashing options to be filtered. --- src/wp-includes/pluggable.php | 18 ++++++++++++++++-- tests/phpunit/tests/auth.php | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 5246f5f0619ab..3b2aef5e598f8 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2617,7 +2617,18 @@ function wp_hash_password( $password ) { return $wp_hasher->HashPassword( trim( $password ) ); } - return password_hash( trim( $password ), PASSWORD_BCRYPT ); + /** + * Filters the options passed to the password_hash() and password_needs_rehash() functions. + * + * @since x.y.z + * + * @param array $options Array of options to pass to the password hashing functions. + * By default this is an empty array which means the default + * options will be used. + */ + $options = apply_filters( 'wp_hash_password_options', array() ); + + return password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); } endif; @@ -2708,7 +2719,10 @@ function wp_password_needs_rehash( $hash ) { return false; } - return password_needs_rehash( $hash, PASSWORD_BCRYPT ); + /** This filter is documented in wp-includes/pluggable.php */ + $options = apply_filters( 'wp_hash_password_options', array() ); + + return password_needs_rehash( $hash, PASSWORD_BCRYPT, $options ); } endif; diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 07042a88aea24..724ef21fe1c9b 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -950,9 +950,40 @@ public function data_usernames() { ); } + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_password_hashing_options_can_be_filtered() { + $password = 'password'; + + add_filter( + 'wp_hash_password_options', + static function ( $options ) { + $options['cost'] = 5; + return $options; + } + ); + + $filter_count_before = did_filter( 'wp_hash_password_options' ); + + $wp_hash = wp_hash_password( $password ); + $valid = wp_check_password( $password, $wp_hash ); + $needs_rehash = wp_password_needs_rehash( $wp_hash ); + $info = password_get_info( $wp_hash ); + $cost = $info['options']['cost']; + + $this->assertTrue( $valid ); + $this->assertFalse( $needs_rehash ); + $this->assertSame( $filter_count_before + 2, did_filter( 'wp_hash_password_options' ) ); + $this->assertSame( 5, $cost ); + } + public function test_password_checks_support_wp_hasher_fallback() { global $wp_hasher; + $filter_count_before = did_filter( 'wp_hash_password_options' ); + $password = 'password'; // Ensure the global $wp_hasher is set. @@ -970,6 +1001,7 @@ public function test_password_checks_support_wp_hasher_fallback() { $this->assertTrue( $valid ); $this->assertFalse( $needs_rehash ); $this->assertSame( 1, did_filter( 'check_password' ) ); + $this->assertSame( $filter_count_before, did_filter( 'wp_hash_password_options' ) ); } /** From 53acf70fdad75ecbd955eecf18d61bc4322ee37e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Fri, 11 Oct 2024 22:44:11 +0100 Subject: [PATCH 37/46] Docs. --- src/wp-includes/pluggable.php | 4 ++-- tests/phpunit/tests/auth.php | 22 ++++++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 3b2aef5e598f8..431655b879bdf 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2702,8 +2702,8 @@ function wp_check_password( $password, $hash, $user_id = '' ) { * * Passwords are hashed with bcrypt using the default cost. A password hashed in a prior version * of WordPress may still be hashed with phpass and will need to be rehashed. If the default cost - * gets updated in PHP or the algorithm is changed in WordPress then a password hashed in a - * previous version of PHP or WordPress will need to be rehashed. + * or algorithm is changed in PHP or WordPress then a password hashed in a previous version will + * need to be rehashed. * * @since x.y.z * diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 724ef21fe1c9b..110fd0c1e937b 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -417,12 +417,6 @@ public function test_password_is_hashed_with_bcrypt() { /** * @ticket 21022 * @ticket 50027 - * - * @TODO bcrypt has a password length limit of 72, need to decide what our approach is. - * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket/21022 - * @TODO See the discussion comments on https://core.trac.wordpress.org/ticket 50027 - * @TODO Reminder: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html - * @TODO split up this test */ public function test_invalid_password_at_bcrypt_length_limit_is_rejected() { $limit = str_repeat( 'a', self::$bcrypt_length_limit ); @@ -436,6 +430,10 @@ public function test_invalid_password_at_bcrypt_length_limit_is_rejected() { $this->assertSame( 'incorrect_password', $user->get_error_code() ); } + /** + * @ticket 21022 + * @ticket 50027 + */ public function test_invalid_password_beyond_bcrypt_length_limit_is_rejected() { $limit = str_repeat( 'a', self::$bcrypt_length_limit + 1 ); @@ -448,6 +446,10 @@ public function test_invalid_password_beyond_bcrypt_length_limit_is_rejected() { $this->assertSame( 'incorrect_password', $user->get_error_code() ); } + /** + * @ticket 21022 + * @ticket 50027 + */ public function test_valid_password_at_bcrypt_length_limit_is_accepted() { $limit = str_repeat( 'a', self::$bcrypt_length_limit ); @@ -463,6 +465,10 @@ public function test_valid_password_at_bcrypt_length_limit_is_accepted() { $this->assertSame( self::$user_id, $user->ID ); } + /** + * @ticket 21022 + * @ticket 50027 + */ public function test_valid_password_beyond_bcrypt_length_limit_is_accepted() { $limit = str_repeat( 'a', self::$bcrypt_length_limit + 1 ); @@ -979,6 +985,10 @@ static function ( $options ) { $this->assertSame( 5, $cost ); } + /** + * @ticket 21022 + * @ticket 50027 + */ public function test_password_checks_support_wp_hasher_fallback() { global $wp_hasher; From 820f48fc1ef23b718e691686a6e246d6dd3c927e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 21 Oct 2024 12:08:44 +0100 Subject: [PATCH 38/46] Cease hashing passwords as md5 during the upgrade routine and/or the repair routine. This prevents passwords from being hashed using md5 when they're already hashed with phpass or bcrypt. --- src/wp-admin/includes/upgrade.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/wp-admin/includes/upgrade.php b/src/wp-admin/includes/upgrade.php index 090a5f1853e6f..bb054d43eb1b3 100644 --- a/src/wp-admin/includes/upgrade.php +++ b/src/wp-admin/includes/upgrade.php @@ -965,6 +965,7 @@ function upgrade_101() { * * @ignore * @since 1.2.0 + * Since x.y.z User passwords are no longer hashed with md5. * * @global wpdb $wpdb WordPress database abstraction object. */ @@ -980,13 +981,6 @@ function upgrade_110() { } } - $users = $wpdb->get_results( "SELECT ID, user_pass from $wpdb->users" ); - foreach ( $users as $row ) { - if ( ! preg_match( '/^[A-Fa-f0-9]{32}$/', $row->user_pass ) ) { - $wpdb->update( $wpdb->users, array( 'user_pass' => md5( $row->user_pass ) ), array( 'ID' => $row->ID ) ); - } - } - // Get the GMT offset, we'll use that later on. $all_options = get_alloptions_110(); From c07e618b9fef755d40c80c495291b17b16cccd06 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 21 Oct 2024 12:08:50 +0100 Subject: [PATCH 39/46] More tests for empty values. --- tests/phpunit/tests/auth.php | 62 ++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 110fd0c1e937b..5e716484ec289 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -308,14 +308,48 @@ public function test_wp_check_password_does_not_support_plain_text() { /** * @ticket 21022 * @ticket 50027 + * + * @dataProvider data_empty_values + * @param mixed $value */ - public function test_wp_check_password_does_not_support_empty_value() { + public function test_wp_check_password_does_not_support_empty_hash( $value ) { $password = 'password'; - $hash = ''; + $hash = $value; $this->assertFalse( wp_check_password( $password, $hash ) ); $this->assertSame( 1, did_filter( 'check_password' ) ); } + /** + * @ticket 21022 + * @ticket 50027 + * + * @dataProvider data_empty_values + * @param mixed $value + */ + public function test_wp_check_password_does_not_support_empty_password( $value ) { + $password = $value; + $hash = $value; + $this->assertFalse( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); + } + + public function data_empty_values() { + return array( + // Integer zero: + array( 0 ), + // String zero: + array( '0' ), + // Zero-length string: + array( '' ), + // Null byte character: + array( "\0" ), + // Asterisk values: + array( '*' ), + array( '*0' ), + array( '*1' ), + ); + } + /** * @ticket 29217 */ @@ -547,20 +581,28 @@ public function test_too_long_password_beyond_phpass_length_limit_is_rejected() $this->assertSame( 'incorrect_password', $user->get_error_code() ); } - public function test_f() { - $this->markTestIncomplete(); + /** + * @dataProvider data_empty_values + * @param mixed $value + */ + public function test_empty_password_is_rejected_by_bcrypt( $value ) { + // Set the user password. + wp_set_password( 'password', self::$user_id ); - $user = wp_authenticate( $this->user->user_login, '*0' ); + $user = wp_authenticate( $this->user->user_login, $value ); $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); } - public function test_g() { - $this->markTestIncomplete(); + /** + * @dataProvider data_empty_values + * @param mixed $value + */ + public function test_empty_password_is_rejected_by_phpass( $value ) { + // Set the user password with the old phpass algorithm. + self::set_user_password_with_phpass( 'password', self::$user_id ); - $user = wp_authenticate( $this->user->user_login, '*1' ); + $user = wp_authenticate( $this->user->user_login, $value ); $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); } public function test_h() { From 5d3362191806860bb49d7641fbf8ac52f1076539 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 21 Oct 2024 14:45:58 +0100 Subject: [PATCH 40/46] More updates to the tests. --- tests/phpunit/tests/auth.php | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 5e716484ec289..524520d1a8162 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -605,28 +605,25 @@ public function test_empty_password_is_rejected_by_phpass( $value ) { $this->assertInstanceOf( 'WP_Error', $user ); } - public function test_h() { - $this->markTestIncomplete(); + public function test_incorrect_password_is_rejected_by_phpass() { + // Set the user password with the old phpass algorithm. + self::set_user_password_with_phpass( 'password', self::$user_id ); $user = wp_authenticate( $this->user->user_login, 'aaaaaaaa' ); - // Wrong password. - $this->assertInstanceOf( 'WP_Error', $user ); - $this->assertSame( 'incorrect_password', $user->get_error_code() ); - } - - public function test_i() { - $this->markTestIncomplete(); - $user = wp_authenticate( $this->user->user_login, $limit ); // Wrong password. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); } - public function test_j() { - $this->markTestIncomplete(); + public function test_too_long_password_is_rejected_by_phpass() { + $limit = str_repeat( 'a', self::$phpass_length_limit ); + + // Set the user password with the old phpass algorithm. + self::set_user_password_with_phpass( 'password', self::$user_id ); $user = wp_authenticate( $this->user->user_login, $limit . 'a' ); + // Password broken by setting it to be too long. $this->assertInstanceOf( 'WP_Error', $user ); $this->assertSame( 'incorrect_password', $user->get_error_code() ); From 00631546b07fd466dffbd428aa088b0488974b5e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 22 Oct 2024 11:56:00 +0100 Subject: [PATCH 41/46] Add a test to verify that a password gets rehashed when the default cost is increased. --- tests/phpunit/tests/auth.php | 44 +++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 524520d1a8162..45e3c69ffeb3f 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -984,6 +984,48 @@ public function test_phpass_password_is_rehashed_after_successful_user_password_ $this->assertSame( self::$user_id, $user->ID ); } + /** + * @dataProvider data_usernames + */ + public function test_bcrypt_password_is_rehashed_with_new_cost_after_successful_user_password_authentication( $username_or_email ) { + $password = 'password'; + + // Hash the user password with a lower cost than default to mimic a cost upgrade. + add_filter( 'wp_hash_password_options', array( $this, 'reduce_hash_cost' ) ); + wp_set_password( $password, self::$user_id ); + remove_filter( 'wp_hash_password_options', array( $this, 'reduce_hash_cost' ) ); + + // Verify that the password needs rehashing. + $hash = get_userdata( self::$user_id )->user_pass; + $this->assertTrue( wp_password_needs_rehash( $hash ) ); + + // Authenticate. + $user = wp_authenticate( $username_or_email, $password ); + + // Verify that the reduced cost password hash was valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + + // Verify that the password has been rehashed with the increased cost. + $hash = get_userdata( self::$user_id )->user_pass; + $this->assertFalse( wp_password_needs_rehash( $hash ) ); + $this->assertSame( self::get_default_bcrypt_cost(), password_get_info( $hash )['options']['cost'] ); + + // Authenticate a second time to ensure the new hash is valid. + $user = wp_authenticate( $username_or_email, $password ); + + // Verify that the password hash is valid. + $this->assertNotWPError( $user ); + $this->assertInstanceOf( 'WP_User', $user ); + $this->assertSame( self::$user_id, $user->ID ); + } + + public function reduce_hash_cost( array $options ): array { + $options['cost'] = self::get_default_bcrypt_cost() - 1; + return $options; + } + public function data_usernames() { return array( array( @@ -1626,7 +1668,7 @@ private static function set_application_password_with_phpass( string $password, return $uuid; } - private static function get_default_bcrypt_cost() { + private static function get_default_bcrypt_cost(): int { $hash = password_hash( 'password', PASSWORD_BCRYPT ); $info = password_get_info( $hash ); From 1aea250483222bfc3679b36a23df098480dba2ee Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 22 Oct 2024 22:54:55 +0100 Subject: [PATCH 42/46] This might as well go here. --- src/wp-includes/pluggable.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 431655b879bdf..4c7b1bb85ce53 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2660,10 +2660,10 @@ function wp_hash_password( $password ) { function wp_check_password( $password, $hash, $user_id = '' ) { global $wp_hasher; + $check = false; + // If the hash is still md5 or otherwise truncated then invalidate it. if ( strlen( $hash ) <= 32 ) { - $check = false; - /** * Filters whether the plaintext password matches the hashed password. * From d6c7837e8e72abaa8e69cae3467893cd539b739e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 22 Oct 2024 23:09:01 +0100 Subject: [PATCH 43/46] Add tests for post password handling. --- src/wp-includes/post-template.php | 1 - .../tests/post/postPasswordRequired.php | 69 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/tests/post/postPasswordRequired.php diff --git a/src/wp-includes/post-template.php b/src/wp-includes/post-template.php index 4c787a21f17c5..176bf35ec3c34 100644 --- a/src/wp-includes/post-template.php +++ b/src/wp-includes/post-template.php @@ -865,7 +865,6 @@ function get_body_class( $css_class = '' ) { * Determines whether the post requires password and whether a correct password has been provided. * * @since 2.7.0 - * @TODO test coverage * * @param int|WP_Post|null $post An optional post. Global $post used if not provided. * @return bool false if a password is not required or the correct password cookie is present, true otherwise. diff --git a/tests/phpunit/tests/post/postPasswordRequired.php b/tests/phpunit/tests/post/postPasswordRequired.php new file mode 100644 index 0000000000000..e95aa21a4eb42 --- /dev/null +++ b/tests/phpunit/tests/post/postPasswordRequired.php @@ -0,0 +1,69 @@ +post->create( + array( + 'post_password' => $password, + ) + ); + + // Password is required: + $this->assertTrue( post_password_required( $post_id ) ); + } + + public function test_post_password_not_required_with_valid_cookie() { + $password = 'password'; + + // Create a post with a password: + $post_id = self::factory()->post->create( + array( + 'post_password' => $password, + ) + ); + + // Set the cookie: + $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = wp_hash_password( $password ); + + // Password is not required: + $this->assertFalse( post_password_required( $post_id ) ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_post_password_hashed_with_phpass_remains_valid() { + $password = 'password'; + + // Create a post with a password: + $post_id = self::factory()->post->create( + array( + 'post_password' => $password, + ) + ); + + // Set the cookie with the phpass hash: + $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = self::$wp_hasher->HashPassword( $password ); + + // Password is not required as it remains valid when hashed with phpass: + $this->assertFalse( post_password_required( $post_id ) ); + } +} From 5824f4cbba0b13d043f03f9eeff7c4a4197a5006 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 23 Oct 2024 00:44:34 +0100 Subject: [PATCH 44/46] Clear the cookie value before performing the assertions. --- .../phpunit/tests/post/postPasswordRequired.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/post/postPasswordRequired.php b/tests/phpunit/tests/post/postPasswordRequired.php index e95aa21a4eb42..cb74606817f2e 100644 --- a/tests/phpunit/tests/post/postPasswordRequired.php +++ b/tests/phpunit/tests/post/postPasswordRequired.php @@ -42,8 +42,14 @@ public function test_post_password_not_required_with_valid_cookie() { // Set the cookie: $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = wp_hash_password( $password ); + // Check if the password is required: + $required = post_password_required( $post_id ); + + // Clear the cookie: + unset( $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] ); + // Password is not required: - $this->assertFalse( post_password_required( $post_id ) ); + $this->assertFalse( $required ); } /** @@ -63,7 +69,13 @@ public function test_post_password_hashed_with_phpass_remains_valid() { // Set the cookie with the phpass hash: $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] = self::$wp_hasher->HashPassword( $password ); + // Check if the password is required: + $required = post_password_required( $post_id ); + + // Clear the cookie: + unset( $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] ); + // Password is not required as it remains valid when hashed with phpass: - $this->assertFalse( post_password_required( $post_id ) ); + $this->assertFalse( $required ); } } From d86ef6d6fac7472e1c5507da357676e2deedcda7 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 25 Nov 2024 17:23:42 +0100 Subject: [PATCH 45/46] Remove assertions that are unnecessarily specific to bcrypt. --- tests/phpunit/tests/auth.php | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 45e3c69ffeb3f..700e5083d89ab 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -916,11 +916,9 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa // Set an application password with the old phpass algorithm. $uuid = self::set_application_password_with_phpass( $password, self::$user_id ); - // Verify that the application password is hashed with phpass. + // Verify that the application password needs rehashing. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertStringStartsWith( '$P$', $hash ); $this->assertTrue( wp_password_needs_rehash( $hash ) ); - $this->assertTrue( WP_Application_Passwords::is_in_use() ); // Authenticate. $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); @@ -930,11 +928,9 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); - // Verify that the application password has been rehashed with bcrypt. + // Verify that the application password no longer needs rehashing. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertStringStartsWith( '$2y$', $hash ); $this->assertFalse( wp_password_needs_rehash( $hash ) ); - $this->assertTrue( WP_Application_Passwords::is_in_use() ); // Verify that the user's password has not been touched. $this->assertSame( $user_pass, get_userdata( self::$user_id )->user_pass ); @@ -957,9 +953,8 @@ public function test_phpass_password_is_rehashed_after_successful_user_password_ // Set the user password with the old phpass algorithm. self::set_user_password_with_phpass( $password, self::$user_id ); - // Verify that the password is hashed with phpass. + // Verify that the password needs rehashing. $hash = get_userdata( self::$user_id )->user_pass; - $this->assertStringStartsWith( '$P$', $hash ); $this->assertTrue( wp_password_needs_rehash( $hash ) ); // Authenticate. @@ -970,9 +965,8 @@ public function test_phpass_password_is_rehashed_after_successful_user_password_ $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); - // Verify that the password has been rehashed with bcrypt. + // Verify that the password no longer needs rehashing. $hash = get_userdata( self::$user_id )->user_pass; - $this->assertStringStartsWith( '$2y$', $hash ); $this->assertFalse( wp_password_needs_rehash( $hash ) ); // Authenticate a second time to ensure the new hash is valid. From 09ddc5e12e02db5a1d1e79086510415b1ef68bb9 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 25 Nov 2024 17:24:00 +0100 Subject: [PATCH 46/46] The default bcrypt cost was increased in PHP 8.4. --- tests/phpunit/tests/auth.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 700e5083d89ab..01b7ea989203b 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -192,7 +192,7 @@ public function test_wp_check_password_supports_phpass_hash() { * The test verifies this by reducing the cost used to generate the hash, therefore mimicing a hash * which was generated prior to the default cost being increased. * - * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . + * Notably the bcrypt cost was increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . * * @ticket 21022 * @ticket 50027 @@ -818,7 +818,7 @@ public function test_expired_phpass_user_activation_key_is_rejected() { * The `wp_password_needs_rehash()` function is just a wrapper around `password_needs_rehash()`, but this ensures * that it works as expected. * - * Notably the bcrypt cost may get increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . + * Notably the bcrypt cost was increased in PHP 8.4: https://wiki.php.net/rfc/bcrypt_cost_2023 . * * @ticket 21022 * @ticket 50027