From fe1c16e35f9ca3709e6b731fc96a221797767600 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 11 Sep 2024 18:23:24 +0100 Subject: [PATCH 01/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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/73] 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 250217989e828b8846c745fba8ff7376e466c604 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 5 Nov 2024 00:52:25 +0000 Subject: [PATCH 45/73] Implement pre-hashing with sha384 to retain entropy of passwords over 72 bytes. --- src/wp-includes/pluggable.php | 26 +++++++- tests/phpunit/tests/auth.php | 116 +++++++++++++++++++++++++++------- 2 files changed, 116 insertions(+), 26 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 4c7b1bb85ce53..dd80f8d1dda37 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2617,6 +2617,10 @@ function wp_hash_password( $password ) { return $wp_hasher->HashPassword( trim( $password ) ); } + if ( strlen( $password ) > 4096 ) { + return '*'; + } + /** * Filters the options passed to the password_hash() and password_needs_rehash() functions. * @@ -2628,7 +2632,11 @@ function wp_hash_password( $password ) { */ $options = apply_filters( 'wp_hash_password_options', array() ); - return password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + // Use sha384 to retain entropy from a password that's longer than 72 bytes. + $password_to_hash = base64_encode( hash( 'sha384', trim( $password ), true ) ); + + // Add a `wp-` prefix to facilitate distinguishing vanilla bcrypt hashes. + return 'wp-' . password_hash( $password_to_hash, PASSWORD_BCRYPT, $options ); } endif; @@ -2681,13 +2689,21 @@ function wp_check_password( $password, $hash, $user_id = '' ) { } if ( ! empty( $wp_hasher ) ) { + // Check the password using the overridden hasher. $check = $wp_hasher->CheckPassword( $password, $hash ); + } elseif ( strlen( $password ) > 4096 ) { + $check = false; + } elseif ( str_starts_with( $hash, 'wp-' ) ) { + // Check the password using the current `wp-` prefixed hash. + $password_to_verify = base64_encode( hash( 'sha384', $password, true ) ); + $check = password_verify( $password_to_verify, substr( $hash, 3 ) ); } elseif ( str_starts_with( $hash, '$P$' ) ) { + // Check the password using phpass. 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 the password using compat support for any non-prefixed hash. $check = password_verify( $password, $hash ); } @@ -2719,10 +2735,14 @@ function wp_password_needs_rehash( $hash ) { return false; } + if ( ! str_starts_with( $hash, 'wp-' ) ) { + return true; + } + /** 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 ); + return password_needs_rehash( substr( $hash, 3 ), PASSWORD_BCRYPT, $options ); } endif; diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 45e3c69ffeb3f..266b57d717d65 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -34,6 +34,8 @@ class Tests_Auth extends WP_UnitTestCase { protected static $phpass_length_limit = 4096; + protected static $password_length_limit = 4096; + /** * Action hook. */ @@ -199,14 +201,15 @@ public function test_wp_check_password_supports_phpass_hash() { */ 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 to the default cost. - 'cost' => $default - 1, - ); - $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + + // Reducing the cost mimics an increase to the default cost. + add_filter( 'wp_hash_password_options', array( $this, 'reduce_hash_cost' ) ); + $hash = wp_hash_password( $password, PASSWORD_BCRYPT ); + remove_filter( 'wp_hash_password_options', array( $this, 'reduce_hash_cost' ) ); + $this->assertTrue( wp_check_password( $password, $hash ) ); $this->assertSame( 1, did_filter( 'check_password' ) ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); } /** @@ -222,29 +225,43 @@ public function test_wp_check_password_supports_hash_with_increased_bcrypt_cost( */ 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 reduction of the default cost. - 'cost' => $default + 1, - ); - $hash = password_hash( trim( $password ), PASSWORD_BCRYPT, $options ); + + // Increasing the cost mimics a reduction of the default cost. + add_filter( 'wp_hash_password_options', array( $this, 'increase_hash_cost' ) ); + $hash = wp_hash_password( $password, PASSWORD_BCRYPT ); + remove_filter( 'wp_hash_password_options', array( $this, 'increase_hash_cost' ) ); + $this->assertTrue( wp_check_password( $password, $hash ) ); $this->assertSame( 1, did_filter( 'check_password' ) ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); } /** * @ticket 21022 * @ticket 50027 */ - public function test_wp_check_password_supports_hash_with_default_bcrypt_cost() { + public function test_wp_check_password_supports_wp_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 ); + + $hash = wp_hash_password( $password, PASSWORD_BCRYPT ); + + $this->assertTrue( wp_check_password( $password, $hash ) ); + $this->assertSame( 1, did_filter( 'check_password' ) ); + $this->assertFalse( wp_password_needs_rehash( $hash ) ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_wp_check_password_supports_plain_bcrypt_hash_with_default_bcrypt_cost() { + $password = 'password'; + + $hash = password_hash( $password, PASSWORD_BCRYPT ); + $this->assertTrue( wp_check_password( $password, $hash ) ); $this->assertSame( 1, did_filter( 'check_password' ) ); + $this->assertTrue( wp_password_needs_rehash( $hash ) ); } /** @@ -437,7 +454,7 @@ public function test_password_is_hashed_with_bcrypt() { wp_set_password( $password, self::$user_id ); // Ensure the password is hashed with bcrypt. - $this->assertStringStartsWith( '$2y$', get_userdata( self::$user_id )->user_pass ); + $this->assertStringStartsWith( 'wp-$2y$', get_userdata( self::$user_id )->user_pass ); // Authenticate. $user = wp_authenticate( $this->user->user_login, $password ); @@ -518,6 +535,54 @@ public function test_valid_password_beyond_bcrypt_length_limit_is_accepted() { $this->assertSame( self::$user_id, $user->ID ); } + /** + * A password beyond 72 bytes will be truncated by bcrypt by default and still be accepted. + * + * This ensures that a truncated password is not accepted by WordPress. + * + * @ticket 21022 + * @ticket 50027 + */ + public function test_long_truncated_password_is_rejected() { + $at_limit = str_repeat( 'a', self::$bcrypt_length_limit ); + $beyond_limit = str_repeat( 'a', self::$bcrypt_length_limit + 1 ); + + // Set the user password beyond the bcrypt limit. + wp_set_password( $beyond_limit, self::$user_id ); + + // Authenticate using a truncated password. + $user = wp_authenticate( $this->user->user_login, $at_limit ); + + // Incorrect password. + $this->assertWPError( $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_setting_password_beyond_bcrypt_length_limit_is_rejected() { + $beyond_limit = str_repeat( 'a', self::$password_length_limit + 1 ); + + // Set the user password beyond the limit. + wp_set_password( $beyond_limit, self::$user_id ); + + // Password broken by setting it to be too long. + $user = get_user_by( 'id', self::$user_id ); + $this->assertSame( '*', $user->data->user_pass ); + + // Password is not accepted. + $user = wp_authenticate( $this->user->user_login, $beyond_limit ); + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + + // Placeholder is not accepted. + $user = wp_authenticate( $this->user->user_login, '*' ); + $this->assertInstanceOf( 'WP_Error', $user ); + $this->assertSame( 'incorrect_password', $user->get_error_code() ); + } + /** * @see https://core.trac.wordpress.org/changeset/30466 */ @@ -932,7 +997,7 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa // 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->assertStringStartsWith( 'wp-$2y$', $hash ); $this->assertFalse( wp_password_needs_rehash( $hash ) ); $this->assertTrue( WP_Application_Passwords::is_in_use() ); @@ -972,7 +1037,7 @@ public function test_phpass_password_is_rehashed_after_successful_user_password_ // Verify that the password has been rehashed with bcrypt. $hash = get_userdata( self::$user_id )->user_pass; - $this->assertStringStartsWith( '$2y$', $hash ); + $this->assertStringStartsWith( 'wp-$2y$', $hash ); $this->assertFalse( wp_password_needs_rehash( $hash ) ); // Authenticate a second time to ensure the new hash is valid. @@ -1010,7 +1075,7 @@ public function test_bcrypt_password_is_rehashed_with_new_cost_after_successful_ // 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'] ); + $this->assertSame( self::get_default_bcrypt_cost(), password_get_info( substr( $hash, 3 ) )['options']['cost'] ); // Authenticate a second time to ensure the new hash is valid. $user = wp_authenticate( $username_or_email, $password ); @@ -1026,6 +1091,11 @@ public function reduce_hash_cost( array $options ): array { return $options; } + public function increase_hash_cost( array $options ): array { + $options['cost'] = self::get_default_bcrypt_cost() + 1; + return $options; + } + public function data_usernames() { return array( array( @@ -1057,7 +1127,7 @@ static function ( $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 ); + $info = password_get_info( substr( $wp_hash, 3 ) ); $cost = $info['options']['cost']; $this->assertTrue( $valid ); From 2321412f435c79aa75869dd1475d8d04f6774dd5 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 11 Nov 2024 15:47:34 +0100 Subject: [PATCH 46/73] Implement domain separation for the password to protect against password shucking. --- src/wp-includes/pluggable.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index dd80f8d1dda37..9654f09a12c83 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2632,8 +2632,8 @@ function wp_hash_password( $password ) { */ $options = apply_filters( 'wp_hash_password_options', array() ); - // Use sha384 to retain entropy from a password that's longer than 72 bytes. - $password_to_hash = base64_encode( hash( 'sha384', trim( $password ), true ) ); + // Use sha384 to retain entropy from a password that's longer than 72 bytes, and a wp-sha384- prefix for domain separation. + $password_to_hash = base64_encode( hash( 'sha384', 'wp-sha384-' . trim( $password ), true ) ); // Add a `wp-` prefix to facilitate distinguishing vanilla bcrypt hashes. return 'wp-' . password_hash( $password_to_hash, PASSWORD_BCRYPT, $options ); @@ -2695,7 +2695,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { $check = false; } elseif ( str_starts_with( $hash, 'wp-' ) ) { // Check the password using the current `wp-` prefixed hash. - $password_to_verify = base64_encode( hash( 'sha384', $password, true ) ); + $password_to_verify = base64_encode( hash( 'sha384', 'wp-sha384-' . $password, true ) ); $check = password_verify( $password_to_verify, substr( $hash, 3 ) ); } elseif ( str_starts_with( $hash, '$P$' ) ) { // Check the password using phpass. From d86ef6d6fac7472e1c5507da357676e2deedcda7 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 25 Nov 2024 17:23:42 +0100 Subject: [PATCH 47/73] 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 48/73] 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 From 1cebd80c28cf5b537b4aabad392cbf5f879aa020 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Dec 2024 12:53:03 +0100 Subject: [PATCH 49/73] Switch to HMAC in place of manually prepending the domain separation key. --- src/wp-includes/pluggable.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 77277d7bc8893..c5a566e486bbc 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2633,8 +2633,8 @@ function wp_hash_password( $password ) { */ $options = apply_filters( 'wp_hash_password_options', array() ); - // Use sha384 to retain entropy from a password that's longer than 72 bytes, and a wp-sha384- prefix for domain separation. - $password_to_hash = base64_encode( hash( 'sha384', 'wp-sha384-' . trim( $password ), true ) ); + // Use sha384 to retain entropy from a password that's longer than 72 bytes, and a wp-sha384 key for domain separation. + $password_to_hash = base64_encode( hash_hmac( 'sha384', trim( $password ), 'wp-sha384', true ) ); // Add a `wp-` prefix to facilitate distinguishing vanilla bcrypt hashes. return 'wp-' . password_hash( $password_to_hash, PASSWORD_BCRYPT, $options ); @@ -2696,7 +2696,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { $check = false; } elseif ( str_starts_with( $hash, 'wp-' ) ) { // Check the password using the current `wp-` prefixed hash. - $password_to_verify = base64_encode( hash( 'sha384', 'wp-sha384-' . $password, true ) ); + $password_to_verify = base64_encode( hash_hmac( 'sha384', $password, 'wp-sha384', true ) ); $check = password_verify( $password_to_verify, substr( $hash, 3 ) ); } elseif ( str_starts_with( $hash, '$P$' ) ) { // Check the password using phpass. From 537ee4aede5a230333abc2259e19415aca244849 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Dec 2024 14:20:05 +0000 Subject: [PATCH 50/73] Introduce the `wp_hash_password_algorithm` filter for controlling the password hashing algorithm. --- src/wp-includes/pluggable.php | 55 ++++++++++++++++++++++++++++------- tests/phpunit/tests/auth.php | 14 +++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index c5a566e486bbc..70a31b8c2f0ab 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2622,22 +2622,54 @@ function wp_hash_password( $password ) { return '*'; } + /** + * Filters the hashing algorithm to use in the password_hash() and password_needs_rehash() functions. + * + * The default is the value of the `PASSWORD_BCRYPT` constant which means bcrypt is used. + * + * **Important:** The only password hashing algorithm that is guaranteed to be available across PHP + * installations is bcrypt. If you use any other algorithm you must make sure that it is available on + * the server. The `password_algos()` function can be used to check which hashing algorithms are available. + * + * The hashing options can be controlled via the {@see 'wp_hash_password_options'} filter. + * + * Other available constants include: + * + * - `PASSWORD_ARGON2I` + * - `PASSWORD_ARGON2ID` + * - `PASSWORD_DEFAULT` + * + * @since x.y.z + * + * @param string $algorithm The hashing algorithm. Default is the value of the `PASSWORD_BCRYPT` constant. + */ + $algorithm = apply_filters( 'wp_hash_password_algorithm', PASSWORD_BCRYPT ); + /** * Filters the options passed to the password_hash() and password_needs_rehash() functions. * + * The default hashing algorithm is bcrypt, but this can be changed via the {@see 'wp_hash_password_algorithm'} + * filter. You must ensure that the options are appropriate for the algorithm in use. + * * @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. + * @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. + * @param string $algorithm The hashing algorithm in use. */ - $options = apply_filters( 'wp_hash_password_options', array() ); + $options = apply_filters( 'wp_hash_password_options', array(), $algorithm ); + + // Algorithms other than bcrypt don't need to use pre-hashing. + if ( $algorithm !== PASSWORD_BCRYPT ) { + return password_hash( $password, $algorithm, $options ); + } // Use sha384 to retain entropy from a password that's longer than 72 bytes, and a wp-sha384 key for domain separation. $password_to_hash = base64_encode( hash_hmac( 'sha384', trim( $password ), 'wp-sha384', true ) ); // Add a `wp-` prefix to facilitate distinguishing vanilla bcrypt hashes. - return 'wp-' . password_hash( $password_to_hash, PASSWORD_BCRYPT, $options ); + return 'wp-' . password_hash( $password_to_hash, $algorithm, $options ); } endif; @@ -2736,14 +2768,17 @@ function wp_password_needs_rehash( $hash ) { return false; } - if ( ! str_starts_with( $hash, 'wp-' ) ) { - return true; - } + /** This filter is documented in wp-includes/pluggable.php */ + $algorithm = apply_filters( 'wp_hash_password_algorithm', PASSWORD_BCRYPT ); /** This filter is documented in wp-includes/pluggable.php */ - $options = apply_filters( 'wp_hash_password_options', array() ); + $options = apply_filters( 'wp_hash_password_options', array(), $algorithm ); + + if ( str_starts_with( $hash, 'wp-' ) ) { + $hash = substr( $hash, 3 ); + } - return password_needs_rehash( substr( $hash, 3 ), PASSWORD_BCRYPT, $options ); + return password_needs_rehash( $hash, $algorithm, $options ); } endif; diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 66e6c75ee4579..88fe38c69c6aa 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1101,6 +1101,20 @@ public function data_usernames() { ); } + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_password_hashing_algorithm_can_be_filtered() { + $password = 'password'; + + $filter_count_before = did_filter( 'wp_hash_password_algorithm' ); + + wp_check_password( $password, wp_hash_password( $password ) ); + + $this->assertSame( $filter_count_before + 2, did_filter( 'wp_hash_password_algorithm' ) ); + } + /** * @ticket 21022 * @ticket 50027 From 8a99eddf6a1245483475c0bf4b59d9d8e614d676 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Dec 2024 14:38:19 +0000 Subject: [PATCH 51/73] Why. --- src/wp-includes/pluggable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 70a31b8c2f0ab..0c1605f509836 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2661,7 +2661,7 @@ function wp_hash_password( $password ) { $options = apply_filters( 'wp_hash_password_options', array(), $algorithm ); // Algorithms other than bcrypt don't need to use pre-hashing. - if ( $algorithm !== PASSWORD_BCRYPT ) { + if ( PASSWORD_BCRYPT !== $algorithm ) { return password_hash( $password, $algorithm, $options ); } From 68634f01dec634160fe2e0db496304e6fa57f703 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Dec 2024 16:59:02 +0000 Subject: [PATCH 52/73] Vanilla bcrypt hashes should be rehashed to use pre-hashing. --- src/wp-includes/pluggable.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 0c1605f509836..f2c798f4ac189 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2776,6 +2776,9 @@ function wp_password_needs_rehash( $hash ) { if ( str_starts_with( $hash, 'wp-' ) ) { $hash = substr( $hash, 3 ); + } else if ( PASSWORD_BCRYPT === $algorithm ) { + // Vanilla bcrypt hashes should be rehashed to use pre-hashing. + return true; } return password_needs_rehash( $hash, $algorithm, $options ); From adf983f783e04fa5f61167a123d51f4ff82efb08 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Dec 2024 17:16:17 +0000 Subject: [PATCH 53/73] Let's bring this more inline with the other tests. --- tests/phpunit/tests/auth.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 88fe38c69c6aa..a4cc05f4510b2 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1110,7 +1110,10 @@ public function test_password_hashing_algorithm_can_be_filtered() { $filter_count_before = did_filter( 'wp_hash_password_algorithm' ); - wp_check_password( $password, wp_hash_password( $password ) ); + $wp_hash = wp_hash_password( $password ); + + wp_check_password( $password, $wp_hash ); + wp_password_needs_rehash( $wp_hash ); $this->assertSame( $filter_count_before + 2, did_filter( 'wp_hash_password_algorithm' ) ); } From 754519b42b3d901b95936d8d4d528abeb35f3921 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 18 Dec 2024 18:49:45 +0000 Subject: [PATCH 54/73] No need to perform a prefix check here, just let `wp_check_password()` handle everything. --- src/wp-includes/post-template.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/wp-includes/post-template.php b/src/wp-includes/post-template.php index 176bf35ec3c34..f8ba56c0de6b8 100644 --- a/src/wp-includes/post-template.php +++ b/src/wp-includes/post-template.php @@ -883,11 +883,7 @@ function post_password_required( $post = null ) { } $hash = wp_unslash( $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] ); - if ( ! str_starts_with( $hash, '$' ) ) { - $required = true; - } else { - $required = ! wp_check_password( $post->post_password, $hash ); - } + $required = ! wp_check_password( $post->post_password, $hash ); /** * Filters whether a post requires the user to supply a password. From 0c49b94e66d975e51a7a9a96ac4fd404e1e5bcb9 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 6 Jan 2025 13:48:20 +0100 Subject: [PATCH 55/73] Docs. --- tests/phpunit/tests/auth.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index a4cc05f4510b2..a714b7ef41269 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -971,6 +971,10 @@ 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.' ); } + /** + * @ticket 21022 + * @ticket 50027 + */ 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' ); @@ -1011,6 +1015,9 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa /** * @dataProvider data_usernames + * + * @ticket 21022 + * @ticket 50027 */ public function test_phpass_password_is_rehashed_after_successful_user_password_authentication( $username_or_email ) { $password = 'password'; @@ -1045,6 +1052,9 @@ public function test_phpass_password_is_rehashed_after_successful_user_password_ /** * @dataProvider data_usernames + * + * @ticket 21022 + * @ticket 50027 */ public function test_bcrypt_password_is_rehashed_with_new_cost_after_successful_user_password_authentication( $username_or_email ) { $password = 'password'; From 413e81f1ecf0d69dde0a9c1a6432f26e75ab5f91 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Mon, 6 Jan 2025 22:41:59 +0100 Subject: [PATCH 56/73] Switch to using a fast sha1 hash for password reset keys, user request keys, and the recovery mode key, which have a known high level of entropy. --- .../class-wp-recovery-mode-key-service.php | 11 ++-- src/wp-includes/class-wp-user-request.php | 2 +- src/wp-includes/pluggable.php | 7 +-- src/wp-includes/user.php | 24 ++++++-- tests/phpunit/tests/auth.php | 58 ++++++++++++++++++- tests/phpunit/tests/user/passwordHash.php | 6 +- 6 files changed, 87 insertions(+), 21 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 1850b7cbcf2f3..2539f5d10702b 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -37,7 +37,7 @@ public function generate_recovery_mode_token() { * Creates a recovery mode key. * * @since 5.2.0 - * @since x.y.z The stored key is now hashed using bcrypt instead of phpass. + * @since x.y.z The stored key is now hashed using sha1 via wp_hash() instead of phpass. * * @param string $token A token generated by {@see generate_recovery_mode_token()}. * @return string Recovery mode key. @@ -45,7 +45,7 @@ public function generate_recovery_mode_token() { public function generate_and_store_recovery_mode_key( $token ) { $key = wp_generate_password( 22, false ); - $hashed = wp_hash_password( $key ); + $hashed = wp_hash( $key, 'auth', 'sha1' ); $records = $this->get_keys(); @@ -96,7 +96,8 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - if ( ! wp_check_password( $key, $record['hashed_key'] ) ) { + // @TODO check this function is always available when this is called + if ( ! hash_equals( $record['hashed_key'], wp_hash( $key, 'auth', 'sha1' ) ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } @@ -151,7 +152,7 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key is now hashed using bcrypt instead of phpass. + * @since x.y.z Each key is now hashed using sha1 via wp_hash() instead of phpass. * Existing keys may still be hashed using phpass. * * @return array { @@ -174,7 +175,7 @@ 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. + * @since x.y.z Each key should now be hashed using sha1 via wp_hash() instead of phpass. * * @param array $keys { * Associative array of token => data pairs, where the data is an associative diff --git a/src/wp-includes/class-wp-user-request.php b/src/wp-includes/class-wp-user-request.php index 82f079f40b799..6b33ad2a87d1a 100644 --- a/src/wp-includes/class-wp-user-request.php +++ b/src/wp-includes/class-wp-user-request.php @@ -92,7 +92,7 @@ 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. + * @since x.y.z The key is now hashed using sha1 via wp_hash() instead of phpass. * * @var string */ diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index f5451d2b6e5d4..27210a44ddce6 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2697,10 +2697,9 @@ 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 application passwords, post passwords, recovery mode - * keys, user password reset keys, and more. There is not always a user ID associated - * with the password. + * Note that this function is used to check more than just user passwords, for + * example it's also used to check application passwords and post passwords. + * 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. diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 09fe2c3b2a957..0c07973c1f649 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2996,7 +2996,7 @@ function get_password_reset_key( $user ) { */ do_action( 'retrieve_password_key', $user->user_login, $key ); - $hashed = time() . ':' . wp_hash_password( $key ); + $hashed = time() . ':' . wp_hash( $key, 'auth', 'sha1' ); $key_saved = wp_update_user( array( @@ -3064,7 +3064,14 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - $hash_is_correct = wp_check_password( $key, $pass_key ); + if ( str_starts_with( $pass_key, '$P$B' ) ) { + // Back-compat for old phpass hashes. + require_once ABSPATH . WPINC . '/class-phpass.php'; + + $hash_is_correct = ( new PasswordHash( 8, true ) )->CheckPassword( $key, $pass_key ); + } else { + $hash_is_correct = hash_equals( $pass_key, wp_hash( $key, 'auth', 'sha1' ) ); + } if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { return $user; @@ -4884,12 +4891,12 @@ function wp_generate_user_request_key( $request_id ) { // Generate something random for a confirmation key. $key = wp_generate_password( 20, false ); - // Return the key, hashed. + // Save the key, hashed. wp_update_post( array( 'ID' => $request_id, 'post_status' => 'request-pending', - 'post_password' => wp_hash_password( $key ), + 'post_password' => wp_hash( $key, 'auth', 'sha1' ), ) ); @@ -4933,7 +4940,14 @@ 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_check_password( $key, $saved_key ) ) { + if ( str_starts_with( $saved_key, '$P$B' ) ) { + // Back-compat for old phpass hashes. + require_once ABSPATH . WPINC . '/class-phpass.php'; + + if ( ! ( new PasswordHash( 8, true ) )->CheckPassword( $key, $saved_key ) ) { + return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); + } + } elseif ( ! hash_equals( $saved_key, wp_hash( $key, 'auth', 'sha1' ) ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index a714b7ef41269..5dfb0243c6037 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -718,7 +718,7 @@ public function test_user_activation_key_is_checked() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash_password( $key ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash( $key, 'auth', 'sha1' ), ), array( 'ID' => $this->user->ID, @@ -756,7 +756,7 @@ public function test_expired_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash_password( $key ), + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash( $key, 'auth', 'sha1' ), ), array( 'ID' => $this->user->ID, @@ -795,7 +795,7 @@ public function test_legacy_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => wp_hash_password( $key ), + 'user_activation_key' => self::$wp_hasher->HashPassword( $key ), ), array( 'ID' => $this->user->ID, @@ -879,6 +879,58 @@ public function test_expired_phpass_user_activation_key_is_rejected() { $this->assertSame( 'invalid_key', $check->get_error_code() ); } + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_user_request_key_handling() { + $request_id = wp_create_user_request( 'test@example.com', 'remove_personal_data' ); + $key = wp_generate_user_request_key( $request_id ); + + // A valid key should be accepted. + $check = wp_validate_user_request_key( $request_id, $key ); + $this->assertNotWPError( $check ); + $this->assertTrue( $check ); + + // An invalid key should rejected. + $check = wp_validate_user_request_key( $request_id, 'invalid' ); + $this->assertWPError( $check ); + $this->assertSame( 'invalid_key', $check->get_error_code() ); + + // An empty key should be rejected. + $check = wp_validate_user_request_key( $request_id, '' ); + $this->assertWPError( $check ); + $this->assertSame( 'missing_key', $check->get_error_code() ); + } + + /** + * @ticket 21022 + * @ticket 50027 + */ + public function test_phpass_user_request_key_is_allowed() { + // A legacy user request key is one hashed using phpass between WordPress 4.3 and x.y.z. + + $request_id = wp_create_user_request( 'test@example.com', 'remove_personal_data' ); + $key = wp_generate_password( 20, false ); + + wp_update_post( + array( + 'ID' => $request_id, + 'post_password' => self::$wp_hasher->HashPassword( $key ), + ) + ); + + // A legacy phpass key should remain valid. + $check = wp_validate_user_request_key( $request_id, $key ); + $this->assertNotWPError( $check ); + $this->assertTrue( $check ); + + // An empty key with a legacy key should be rejected. + $check = wp_validate_user_request_key( $request_id, '' ); + $this->assertWPError( $check ); + $this->assertSame( 'missing_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. diff --git a/tests/phpunit/tests/user/passwordHash.php b/tests/phpunit/tests/user/passwordHash.php index 13efdb9367ad0..35a37df9902f6 100644 --- a/tests/phpunit/tests/user/passwordHash.php +++ b/tests/phpunit/tests/user/passwordHash.php @@ -3,9 +3,9 @@ /** * 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. + * PasswordHash is no longer used to hash passwords, but it is still used to hash security keys + * that don't need to use bcrypt, and as a fallback to verify old passwords that were hashed by + * phpass. The library therefore needs to remain compatible with the latest versions of PHP. * * @covers PasswordHash */ From 4880c6e8eb3974ca095d528fac50eb4e8fe38675 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 01:28:28 +0100 Subject: [PATCH 57/73] Switch from SHA-1 to SHA-256 in the security key HMACs. --- .../class-wp-recovery-mode-key-service.php | 17 +++++++++++------ src/wp-includes/class-wp-user-request.php | 2 +- src/wp-includes/user.php | 15 +++++++++++---- 3 files changed, 23 insertions(+), 11 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 2539f5d10702b..17766b8ebb775 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -37,7 +37,7 @@ public function generate_recovery_mode_token() { * Creates a recovery mode key. * * @since 5.2.0 - * @since x.y.z The stored key is now hashed using sha1 via wp_hash() instead of phpass. + * @since x.y.z The stored key is now hashed using SHA-256 instead of phpass. * * @param string $token A token generated by {@see generate_recovery_mode_token()}. * @return string Recovery mode key. @@ -45,7 +45,9 @@ public function generate_recovery_mode_token() { public function generate_and_store_recovery_mode_key( $token ) { $key = wp_generate_password( 22, false ); - $hashed = wp_hash( $key, 'auth', 'sha1' ); + // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + $hashed = wp_hash( $key, 'auth', $algo ); $records = $this->get_keys(); @@ -96,8 +98,11 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - // @TODO check this function is always available when this is called - if ( ! hash_equals( $record['hashed_key'], wp_hash( $key, 'auth', 'sha1' ) ) ) { + // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + + // @TODO check `wp_hash()` is always available when this is called + if ( ! hash_equals( $record['hashed_key'], wp_hash( $key, 'auth', $algo ) ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } @@ -152,7 +157,7 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key is now hashed using sha1 via wp_hash() instead of phpass. + * @since x.y.z Each key is now hashed using SHA-256 instead of phpass. * Existing keys may still be hashed using phpass. * * @return array { @@ -175,7 +180,7 @@ private function get_keys() { * Updates the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key should now be hashed using sha1 via wp_hash() instead of phpass. + * @since x.y.z Each key should now be hashed using SHA-256 instead of phpass. * * @param array $keys { * Associative array of token => data pairs, where the data is an associative diff --git a/src/wp-includes/class-wp-user-request.php b/src/wp-includes/class-wp-user-request.php index 6b33ad2a87d1a..2d1e9bc699ef4 100644 --- a/src/wp-includes/class-wp-user-request.php +++ b/src/wp-includes/class-wp-user-request.php @@ -92,7 +92,7 @@ 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 sha1 via wp_hash() instead of phpass. + * @since x.y.z The key is now hashed using SHA-256 instead of phpass. * * @var string */ diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 0c07973c1f649..a25e9b3f7aa2b 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2996,7 +2996,8 @@ function get_password_reset_key( $user ) { */ do_action( 'retrieve_password_key', $user->user_login, $key ); - $hashed = time() . ':' . wp_hash( $key, 'auth', 'sha1' ); + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + $hashed = time() . ':' . wp_hash( $key, 'auth', $algo ); $key_saved = wp_update_user( array( @@ -3070,7 +3071,9 @@ function check_password_reset_key( $key, $login ) { $hash_is_correct = ( new PasswordHash( 8, true ) )->CheckPassword( $key, $pass_key ); } else { - $hash_is_correct = hash_equals( $pass_key, wp_hash( $key, 'auth', 'sha1' ) ); + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + + $hash_is_correct = hash_equals( $pass_key, wp_hash( $key, 'auth', $algo ) ); } if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { @@ -4891,12 +4894,14 @@ function wp_generate_user_request_key( $request_id ) { // Generate something random for a confirmation key. $key = wp_generate_password( 20, false ); + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + // Save the key, hashed. wp_update_post( array( 'ID' => $request_id, 'post_status' => 'request-pending', - 'post_password' => wp_hash( $key, 'auth', 'sha1' ), + 'post_password' => wp_hash( $key, 'auth', $algo ), ) ); @@ -4940,6 +4945,8 @@ 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; + $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; + if ( str_starts_with( $saved_key, '$P$B' ) ) { // Back-compat for old phpass hashes. require_once ABSPATH . WPINC . '/class-phpass.php'; @@ -4947,7 +4954,7 @@ function wp_validate_user_request_key( $request_id, $key ) { if ( ! ( new PasswordHash( 8, true ) )->CheckPassword( $key, $saved_key ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } - } elseif ( ! hash_equals( $saved_key, wp_hash( $key, 'auth', 'sha1' ) ) ) { + } elseif ( ! hash_equals( $saved_key, wp_hash( $key, 'auth', $algo ) ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } From cb460779ec31d0bf043a5f21a6b3b169f659c0f7 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 18:28:00 +0100 Subject: [PATCH 58/73] Use a more fitting hash prefix. --- src/wp-includes/pluggable.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 27210a44ddce6..82d54b93f1257 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2685,11 +2685,11 @@ function wp_hash_password( $password ) { return password_hash( $password, $algorithm, $options ); } - // Use sha384 to retain entropy from a password that's longer than 72 bytes, and a wp-sha384 key for domain separation. + // Use SHA-384 to retain entropy from a password that's longer than 72 bytes, and a `wp-sha384` key for domain separation. $password_to_hash = base64_encode( hash_hmac( 'sha384', trim( $password ), 'wp-sha384', true ) ); - // Add a `wp-` prefix to facilitate distinguishing vanilla bcrypt hashes. - return 'wp-' . password_hash( $password_to_hash, $algorithm, $options ); + // Add a prefix to facilitate distinguishing vanilla bcrypt hashes. + return '$wp' . password_hash( $password_to_hash, $algorithm, $options ); } endif; @@ -2745,8 +2745,8 @@ function wp_check_password( $password, $hash, $user_id = '' ) { $check = $wp_hasher->CheckPassword( $password, $hash ); } elseif ( strlen( $password ) > 4096 ) { $check = false; - } elseif ( str_starts_with( $hash, 'wp-' ) ) { - // Check the password using the current `wp-` prefixed hash. + } elseif ( str_starts_with( $hash, '$wp' ) ) { + // Check the password using the current prefixed hash. $password_to_verify = base64_encode( hash_hmac( 'sha384', $password, 'wp-sha384', true ) ); $check = password_verify( $password_to_verify, substr( $hash, 3 ) ); } elseif ( str_starts_with( $hash, '$P$' ) ) { @@ -2793,9 +2793,9 @@ function wp_password_needs_rehash( $hash ) { /** This filter is documented in wp-includes/pluggable.php */ $options = apply_filters( 'wp_hash_password_options', array(), $algorithm ); - if ( str_starts_with( $hash, 'wp-' ) ) { + if ( str_starts_with( $hash, '$wp' ) ) { $hash = substr( $hash, 3 ); - } else if ( PASSWORD_BCRYPT === $algorithm ) { + } elseif ( PASSWORD_BCRYPT === $algorithm ) { // Vanilla bcrypt hashes should be rehashed to use pre-hashing. return true; } From a9a1c1703a00480e2690277493876fd66e04ee18 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 23:27:34 +0100 Subject: [PATCH 59/73] Update some more tests. --- tests/phpunit/tests/auth.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 5dfb0243c6037..90fa5c9fe38b8 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -454,7 +454,7 @@ public function test_password_is_hashed_with_bcrypt() { wp_set_password( $password, self::$user_id ); // Ensure the password is hashed with bcrypt. - $this->assertStringStartsWith( 'wp-$2y$', get_userdata( self::$user_id )->user_pass ); + $this->assertStringStartsWith( '$wp$2y$', get_userdata( self::$user_id )->user_pass ); // Authenticate. $user = wp_authenticate( $this->user->user_login, $password ); @@ -718,7 +718,7 @@ public function test_user_activation_key_is_checked() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash( $key, 'auth', 'sha1' ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash( $key, 'auth', 'sha256' ), ), array( 'ID' => $this->user->ID, @@ -756,7 +756,7 @@ public function test_expired_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash( $key, 'auth', 'sha1' ), + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash( $key, 'auth', 'sha256' ), ), array( 'ID' => $this->user->ID, From 8b4d40e7d3748da12dfd9b907acecfcc273c4e2a Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 23:32:30 +0100 Subject: [PATCH 60/73] Introduce wrapper functions for hashing and checking a value with BLAKE2b via Sodium. --- src/wp-includes/functions.php | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 7e46dd53cab2d..a60c817ffd055 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9085,3 +9085,41 @@ function wp_is_heic_image_mime_type( $mime_type ) { return in_array( $mime_type, $heic_mime_types, true ); } + +/** + * Returns a cryptographically secure hash of the message. + * + * The BLAKE2b algorithm is used by Sodium to hash the message. + * + * @TODO explain when to use this instead of wp_hash(). + * + * @since x.y.z + * + * @param string $message The message to hash. + * @return string The hash of the message. + */ +function wp_hash_value( $message ) { + return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message, wp_salt() ) ); +} + +/** + * Checks whether a plaintext message matches the hashed value. + * + * The function uses Sodium to hash the message and compare it to the hashed value. If the hash + * is not a generic hash, the hash is treated as a phpass portable hash. + * + * @since x.y.z + * + * @param string $message The plaintext message. + * @param string $hash Hash of the message to check against. + * @return bool Whether the message matches the hashed message. + */ +function wp_verify_hashed_value( $message, $hash ) { + if ( ! str_starts_with( $hash, '$generic$' ) ) { + // Back-compat for old phpass hashes. + require_once ABSPATH . WPINC . '/class-phpass.php'; + return ( new PasswordHash( 8, true ) )->CheckPassword( $message, $hash ); + } + + return hash_equals( substr( $hash, 9 ), wp_hash_value( $message ) ); +} From 9994922b8b6ee243bfb8ed4894f2af370b4e8edf Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 23:34:03 +0100 Subject: [PATCH 61/73] Replace SHA-256 and SHA-1 with BLAKE2b when hashing of password reset keys, user request keys, and the recovery mode key. --- .../class-wp-recovery-mode-key-service.php | 18 ++++-------- src/wp-includes/class-wp-user-request.php | 2 +- src/wp-includes/user.php | 29 +++---------------- tests/phpunit/tests/auth.php | 4 +-- 4 files changed, 12 insertions(+), 41 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 17766b8ebb775..52dfcc876d267 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -37,7 +37,7 @@ public function generate_recovery_mode_token() { * Creates a recovery mode key. * * @since 5.2.0 - * @since x.y.z The stored key is now hashed using SHA-256 instead of phpass. + * @since x.y.z The stored key is now hashed using wp_hash_value() instead of phpass. * * @param string $token A token generated by {@see generate_recovery_mode_token()}. * @return string Recovery mode key. @@ -45,14 +45,10 @@ public function generate_recovery_mode_token() { public function generate_and_store_recovery_mode_key( $token ) { $key = wp_generate_password( 22, false ); - // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - $hashed = wp_hash( $key, 'auth', $algo ); - $records = $this->get_keys(); $records[ $token ] = array( - 'hashed_key' => $hashed, + 'hashed_key' => wp_hash_value( $key ), 'created_at' => time(), ); @@ -98,11 +94,7 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - // If ext/hash is not present, compat.php's hash_hmac() does not support sha256. - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - - // @TODO check `wp_hash()` is always available when this is called - if ( ! hash_equals( $record['hashed_key'], wp_hash( $key, 'auth', $algo ) ) ) { + if ( ! wp_verify_hashed_value( $key, $record['hashed_key'] ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } @@ -157,7 +149,7 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key is now hashed using SHA-256 instead of phpass. + * @since x.y.z Each key is now hashed using wp_hash_value() instead of phpass. * Existing keys may still be hashed using phpass. * * @return array { @@ -180,7 +172,7 @@ private function get_keys() { * Updates the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key should now be hashed using SHA-256 instead of phpass. + * @since x.y.z Each key should now be hashed using wp_hash_value() instead of phpass. * * @param array $keys { * Associative array of token => data pairs, where the data is an associative diff --git a/src/wp-includes/class-wp-user-request.php b/src/wp-includes/class-wp-user-request.php index 2d1e9bc699ef4..cfeacd7bd553c 100644 --- a/src/wp-includes/class-wp-user-request.php +++ b/src/wp-includes/class-wp-user-request.php @@ -92,7 +92,7 @@ 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 SHA-256 instead of phpass. + * @since x.y.z The key is now hashed using wp_hash_value() instead of phpass. * * @var string */ diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index a25e9b3f7aa2b..d77f2e375a7b0 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2996,8 +2996,7 @@ function get_password_reset_key( $user ) { */ do_action( 'retrieve_password_key', $user->user_login, $key ); - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - $hashed = time() . ':' . wp_hash( $key, 'auth', $algo ); + $hashed = time() . ':' . wp_hash_value( $key ); $key_saved = wp_update_user( array( @@ -3065,16 +3064,7 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - if ( str_starts_with( $pass_key, '$P$B' ) ) { - // Back-compat for old phpass hashes. - require_once ABSPATH . WPINC . '/class-phpass.php'; - - $hash_is_correct = ( new PasswordHash( 8, true ) )->CheckPassword( $key, $pass_key ); - } else { - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - - $hash_is_correct = hash_equals( $pass_key, wp_hash( $key, 'auth', $algo ) ); - } + $hash_is_correct = wp_verify_hashed_value( $key, $pass_key ); if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { return $user; @@ -4894,14 +4884,12 @@ function wp_generate_user_request_key( $request_id ) { // Generate something random for a confirmation key. $key = wp_generate_password( 20, false ); - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - // Save the key, hashed. wp_update_post( array( 'ID' => $request_id, 'post_status' => 'request-pending', - 'post_password' => wp_hash( $key, 'auth', $algo ), + 'post_password' => wp_hash_value( $key ), ) ); @@ -4945,16 +4933,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; - $algo = function_exists( 'hash' ) ? 'sha256' : 'sha1'; - - if ( str_starts_with( $saved_key, '$P$B' ) ) { - // Back-compat for old phpass hashes. - require_once ABSPATH . WPINC . '/class-phpass.php'; - - if ( ! ( new PasswordHash( 8, true ) )->CheckPassword( $key, $saved_key ) ) { - return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); - } - } elseif ( ! hash_equals( $saved_key, wp_hash( $key, 'auth', $algo ) ) ) { + if ( ! wp_verify_hashed_value( $key, $saved_key ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index 90fa5c9fe38b8..bc77132e94c70 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -718,7 +718,7 @@ public function test_user_activation_key_is_checked() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash( $key, 'auth', 'sha256' ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash_value( $key ), ), array( 'ID' => $this->user->ID, @@ -756,7 +756,7 @@ public function test_expired_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash( $key, 'auth', 'sha256' ), + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash_value( $key ), ), array( 'ID' => $this->user->ID, From d80491dd02d4c3be3e851cc899267d78910c3129 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 7 Jan 2025 23:38:18 +0100 Subject: [PATCH 62/73] Switch application passwords over to BLAKE2b instead of phpass. --- .../class-wp-application-passwords.php | 49 +++++++++++++++++-- src/wp-includes/user.php | 14 +++--- tests/phpunit/tests/auth.php | 4 +- 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index 50d9060dfdff5..dde7ce1e983aa 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -60,6 +60,7 @@ public static function is_in_use() { * * @since 5.6.0 * @since 5.7.0 Returns WP_Error if application name already exists. + * @since x.y.z The hashed password value now uses wp_hash_value() instead of phpass. * * @param int $user_id User ID. * @param array $args { @@ -95,7 +96,7 @@ public static function create_new_application_password( $user_id, $args = array( } $new_password = wp_generate_password( static::PW_LENGTH, false ); - $hashed_password = wp_hash_password( $new_password ); + $hashed_password = self::hash_password( $new_password ); $new_item = array( 'uuid' => wp_generate_uuid4(), @@ -124,6 +125,7 @@ public static function create_new_application_password( $user_id, $args = array( * Fires when an application password is created. * * @since 5.6.0 + * @since x.y.z @since x.y.z The hashed password value now uses wp_hash_value() instead of phpass. * * @param int $user_id The user ID. * @param array $new_item { @@ -249,7 +251,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(). + * @since x.y.z The actual password should now be hashed using wp_hash_value(). * * @param int $user_id User ID. * @param string $uuid The password's UUID. @@ -302,7 +304,7 @@ 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. + * @since x.y.z The password is now hashed using wp_hash_value() instead of phpass. * Existing passwords may still be hashed using phpass. * * @param int $user_id The user ID. @@ -472,4 +474,45 @@ public static function chunk_password( $raw_password ) { return trim( chunk_split( $raw_password, 4, ' ' ) ); } + + /** + * Hashes a plaintext application password. + * + * @since x.y.z + * + * @param string $password Plaintext password. + * @return string Hashed password. + */ + public static function hash_password( $password ) { + return wp_hash_value( $password ); + } + + /** + * Checks a plaintext application password against a hashed password. + * + * @since x.y.z + * + * @param string $password Plaintext password. + * @param string $hash Hash of the password to check against. + * @return bool Whether the password matches the hashed password. + */ + public static function check_password( $password, $hash ) { + return wp_verify_hashed_value( $password, $hash ); + } + + /** + * Checks whether a password hash needs to be rehashed. + * + * A password hashed in a prior version of WordPress may still be hashed with phpass and will + * need to be rehashed. If the algorithm is changed in WordPress then a password hashed in a + * previous version 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. + */ + public static function password_needs_rehash( $hash ) { + return ! str_starts_with( $hash, '$generic$' ); + } } diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index d77f2e375a7b0..8a392ee7bb20a 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -437,17 +437,10 @@ 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 ) { - $valid = wp_check_password( $password, $item['password'], $user->ID ); - - if ( ! $valid ) { + if ( ! WP_Application_Passwords::check_password( $password, $item['password'] ) ) { continue; } - if ( wp_password_needs_rehash( $item['password'] ) ) { - $item['password'] = wp_hash_password( $password ); - WP_Application_Passwords::update_application_password( $user->ID, $item['uuid'], $item ); - } - $error = new WP_Error(); /** @@ -471,6 +464,11 @@ function wp_authenticate_application_password( $input_user, $username, $password return $error; } + if ( WP_Application_Passwords::password_needs_rehash( $item['password'] ) ) { + $item['password'] = WP_Application_Passwords::hash_password( $password ); + WP_Application_Passwords::update_application_password( $user->ID, $item['uuid'], $item ); + } + WP_Application_Passwords::record_application_password_usage( $user->ID, $item['uuid'] ); /** diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index bc77132e94c70..dfe94fcc728ba 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1039,7 +1039,7 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa // Verify that the application password needs rehashing. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertTrue( wp_password_needs_rehash( $hash ) ); + $this->assertTrue( WP_Application_Passwords::password_needs_rehash( $hash ) ); // Authenticate. $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); @@ -1051,7 +1051,7 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa // Verify that the application password no longer needs rehashing. $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertFalse( wp_password_needs_rehash( $hash ) ); + $this->assertFalse( WP_Application_Passwords::password_needs_rehash( $hash ) ); // Verify that the user's password has not been touched. $this->assertSame( $user_pass, get_userdata( self::$user_id )->user_pass ); From f222f64a67b23093e3053bf3195d61f8c58cd3a4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 8 Jan 2025 01:04:56 +0100 Subject: [PATCH 63/73] Remove opportunistic rehashing of application passwords. --- src/wp-includes/user.php | 5 ----- tests/phpunit/tests/auth.php | 24 ++---------------------- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 8a392ee7bb20a..c60c759a7bd82 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -464,11 +464,6 @@ function wp_authenticate_application_password( $input_user, $username, $password return $error; } - if ( WP_Application_Passwords::password_needs_rehash( $item['password'] ) ) { - $item['password'] = WP_Application_Passwords::hash_password( $password ); - WP_Application_Passwords::update_application_password( $user->ID, $item['uuid'], $item ); - } - WP_Application_Passwords::record_application_password_usage( $user->ID, $item['uuid'] ); /** diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index dfe94fcc728ba..23ee469d2b00a 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -1027,20 +1027,15 @@ public function test_user_activation_key_after_successful_login() { * @ticket 21022 * @ticket 50027 */ - public function test_phpass_password_is_rehashed_after_successful_application_password_authentication() { + public function test_phpass_application_password_is_accepted() { add_filter( 'application_password_is_api_request', '__return_true' ); add_filter( 'wp_is_application_passwords_available', '__return_true' ); - $password = 'password'; - $user_pass = get_userdata( self::$user_id )->user_pass; + $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 application password needs rehashing. - $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertTrue( WP_Application_Passwords::password_needs_rehash( $hash ) ); - // Authenticate. $user = wp_authenticate_application_password( null, self::USER_LOGIN, $password ); @@ -1048,21 +1043,6 @@ public function test_phpass_password_is_rehashed_after_successful_application_pa $this->assertNotWPError( $user ); $this->assertInstanceOf( 'WP_User', $user ); $this->assertSame( self::$user_id, $user->ID ); - - // Verify that the application password no longer needs rehashing. - $hash = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid )['password']; - $this->assertFalse( WP_Application_Passwords::password_needs_rehash( $hash ) ); - - // 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 hashed application password is valid. - $this->assertNotWPError( $user ); - $this->assertInstanceOf( 'WP_User', $user ); - $this->assertSame( self::$user_id, $user->ID ); } /** From c9661233fd1fb08c4eb2cb5237d903dc05a4449f Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 8 Jan 2025 13:51:28 +0100 Subject: [PATCH 64/73] The hash extension is now required due to the use of sha384. --- composer.json | 1 + src/wp-admin/includes/class-wp-site-health.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index eb78d144e590c..76b0b9b3991d6 100644 --- a/composer.json +++ b/composer.json @@ -10,6 +10,7 @@ "issues": "https://core.trac.wordpress.org/" }, "require": { + "ext-hash": "*", "ext-json": "*", "php": ">=7.2.24" }, diff --git a/src/wp-admin/includes/class-wp-site-health.php b/src/wp-admin/includes/class-wp-site-health.php index 3f89fc0a38ffc..19d109d447283 100644 --- a/src/wp-admin/includes/class-wp-site-health.php +++ b/src/wp-admin/includes/class-wp-site-health.php @@ -923,7 +923,7 @@ public function get_test_php_extensions() { ), 'hash' => array( 'function' => 'hash', - 'required' => false, + 'required' => true, ), 'imagick' => array( 'extension' => 'imagick', From 7b3eb9a7c6a77daa32c05bc70b2566ce6efba458 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Wed, 8 Jan 2025 23:18:57 +0100 Subject: [PATCH 65/73] Enforce an upper key length. --- src/wp-includes/functions.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index a60c817ffd055..a589c54bf5c8b 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9099,7 +9099,10 @@ function wp_is_heic_image_mime_type( $mime_type ) { * @return string The hash of the message. */ function wp_hash_value( $message ) { - return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message, wp_salt() ) ); + // @TODO make use of CRYPTO_GENERICHASH_KEYBYTES_MAX for the max key length. Needs to be compatible with libsdium and sodium_compat. + $key = substr( wp_salt(), 0, 64 ); + + return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message, $key ) ); } /** From e3fcdca0dd2f40934d5e0e7315ca225d8c9b5348 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 9 Jan 2025 12:37:35 +0100 Subject: [PATCH 66/73] Ensure the salt satisfies the constraints for key length in Sodium. --- src/wp-includes/functions.php | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index a589c54bf5c8b..677d0a800cac8 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9095,12 +9095,29 @@ function wp_is_heic_image_mime_type( $mime_type ) { * * @since x.y.z * + * @throws TypeError Thrown by Sodium + * @throws SodiumException Thrown by Sodium + * * @param string $message The message to hash. * @return string The hash of the message. */ function wp_hash_value( $message ) { - // @TODO make use of CRYPTO_GENERICHASH_KEYBYTES_MAX for the max key length. Needs to be compatible with libsdium and sodium_compat. - $key = substr( wp_salt(), 0, 64 ); + $min_key_length = defined( 'SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN' ) + ? SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN + : \ParagonIE_Sodium_Compat::CRYPTO_GENERICHASH_KEYBYTES_MIN; + $max_key_length = defined( 'SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MAX' ) + ? SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MAX + : \ParagonIE_Sodium_Compat::CRYPTO_GENERICHASH_KEYBYTES_MAX; + + $salt = wp_salt(); + + // Constrain the maximum key length. + $key = substr( $salt, 0, $max_key_length ); + + // Enforce the minimum key length. + if ( strlen( $key ) < $min_key_length ) { + $key .= str_repeat( '0', $min_key_length - strlen( $key ) ); + } return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message, $key ) ); } From 1d1ab91499ebce39506f5191341d4a40f8ce27b9 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 9 Jan 2025 12:40:01 +0100 Subject: [PATCH 67/73] Add some type safety. --- src/wp-includes/class-wp-application-passwords.php | 6 +++--- src/wp-includes/functions.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index dde7ce1e983aa..edd967ecd9c4a 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -483,7 +483,7 @@ public static function chunk_password( $raw_password ) { * @param string $password Plaintext password. * @return string Hashed password. */ - public static function hash_password( $password ) { + public static function hash_password( string $password ): string { return wp_hash_value( $password ); } @@ -496,7 +496,7 @@ public static function hash_password( $password ) { * @param string $hash Hash of the password to check against. * @return bool Whether the password matches the hashed password. */ - public static function check_password( $password, $hash ) { + public static function check_password( string $password, string $hash ): bool { return wp_verify_hashed_value( $password, $hash ); } @@ -512,7 +512,7 @@ public static function check_password( $password, $hash ) { * @param string $hash Hash of a password to check. * @return bool Whether the hash needs to be rehashed. */ - public static function password_needs_rehash( $hash ) { + public static function password_needs_rehash( string $hash ): bool { return ! str_starts_with( $hash, '$generic$' ); } } diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 677d0a800cac8..4248d1147848c 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9101,7 +9101,7 @@ function wp_is_heic_image_mime_type( $mime_type ) { * @param string $message The message to hash. * @return string The hash of the message. */ -function wp_hash_value( $message ) { +function wp_hash_value( string $message ): string { $min_key_length = defined( 'SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN' ) ? SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN : \ParagonIE_Sodium_Compat::CRYPTO_GENERICHASH_KEYBYTES_MIN; @@ -9134,7 +9134,7 @@ function wp_hash_value( $message ) { * @param string $hash Hash of the message to check against. * @return bool Whether the message matches the hashed message. */ -function wp_verify_hashed_value( $message, $hash ) { +function wp_verify_hashed_value( string $message, string $hash ): bool { if ( ! str_starts_with( $hash, '$generic$' ) ) { // Back-compat for old phpass hashes. require_once ABSPATH . WPINC . '/class-phpass.php'; From 20eb361d17f18095018a66ed0f42a116b147bbd4 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 10:12:18 +0100 Subject: [PATCH 68/73] Remove salting from the fast hashing function. --- src/wp-includes/functions.php | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 4248d1147848c..8f4638182f440 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9102,24 +9102,7 @@ function wp_is_heic_image_mime_type( $mime_type ) { * @return string The hash of the message. */ function wp_hash_value( string $message ): string { - $min_key_length = defined( 'SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN' ) - ? SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MIN - : \ParagonIE_Sodium_Compat::CRYPTO_GENERICHASH_KEYBYTES_MIN; - $max_key_length = defined( 'SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MAX' ) - ? SODIUM_CRYPTO_GENERICHASH_KEYBYTES_MAX - : \ParagonIE_Sodium_Compat::CRYPTO_GENERICHASH_KEYBYTES_MAX; - - $salt = wp_salt(); - - // Constrain the maximum key length. - $key = substr( $salt, 0, $max_key_length ); - - // Enforce the minimum key length. - if ( strlen( $key ) < $min_key_length ) { - $key .= str_repeat( '0', $min_key_length - strlen( $key ) ); - } - - return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message, $key ) ); + return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message ) ); } /** From 37e0cd718164cb426166a3e9df44947e3f3d6bdf Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 10:12:25 +0100 Subject: [PATCH 69/73] Docs. --- src/wp-includes/functions.php | 2 +- src/wp-includes/pluggable.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 8f4638182f440..fdfcf28ca33b0 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9095,7 +9095,7 @@ function wp_is_heic_image_mime_type( $mime_type ) { * * @since x.y.z * - * @throws TypeError Thrown by Sodium + * @throws TypeError Thrown by Sodium * @throws SodiumException Thrown by Sodium * * @param string $message The message to hash. diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 3218ffb1fd3b1..822edcf58913c 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2698,9 +2698,9 @@ function wp_hash_password( $password ) { /** * Checks a plaintext password against a hashed password. * - * Note that this function is used to check more than just user passwords, for - * example it's also used to check application passwords and post passwords. - * There is not always a user ID associated with the password. + * Note that this function is used to check more than just user passwords, for example + * it's also used to check post passwords and may be used by plugins to check other + * types of password. 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. From 9a58b9154361590c1f0fd15e36169bdbf8479331 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 10:12:31 +0100 Subject: [PATCH 70/73] Tidying up. --- src/wp-includes/functions.php | 5 +++-- src/wp-includes/pluggable.php | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index fdfcf28ca33b0..55ed645d17ef1 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9095,8 +9095,7 @@ function wp_is_heic_image_mime_type( $mime_type ) { * * @since x.y.z * - * @throws TypeError Thrown by Sodium - * @throws SodiumException Thrown by Sodium + * @throws TypeError Thrown by Sodium if the message is not a string. * * @param string $message The message to hash. * @return string The hash of the message. @@ -9113,6 +9112,8 @@ function wp_hash_value( string $message ): string { * * @since x.y.z * + * @throws TypeError Thrown by Sodium if the message is not a string. + * * @param string $message The plaintext message. * @param string $hash Hash of the message to check against. * @return bool Whether the message matches the hashed message. diff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php index 822edcf58913c..798daeb9ce151 100644 --- a/src/wp-includes/pluggable.php +++ b/src/wp-includes/pluggable.php @@ -2753,8 +2753,7 @@ function wp_check_password( $password, $hash, $user_id = '' ) { } elseif ( str_starts_with( $hash, '$P$' ) ) { // Check the password using phpass. require_once ABSPATH . WPINC . '/class-phpass.php'; - $hasher = new PasswordHash( 8, true ); - $check = $hasher->CheckPassword( $password, $hash ); + $check = ( new PasswordHash( 8, true ) )->CheckPassword( $password, $hash ); } else { // Check the password using compat support for any non-prefixed hash. $check = password_verify( $password, $hash ); From d62c5d3ee726d372c1d152e6824610e284da6b4b Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 10:42:07 +0100 Subject: [PATCH 71/73] Rename these functions. --- src/wp-includes/class-wp-application-passwords.php | 12 ++++++------ .../class-wp-recovery-mode-key-service.php | 10 +++++----- src/wp-includes/class-wp-user-request.php | 2 +- src/wp-includes/functions.php | 6 +++--- src/wp-includes/user.php | 8 ++++---- tests/phpunit/tests/auth.php | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/wp-includes/class-wp-application-passwords.php b/src/wp-includes/class-wp-application-passwords.php index edd967ecd9c4a..316fe268104d1 100644 --- a/src/wp-includes/class-wp-application-passwords.php +++ b/src/wp-includes/class-wp-application-passwords.php @@ -60,7 +60,7 @@ public static function is_in_use() { * * @since 5.6.0 * @since 5.7.0 Returns WP_Error if application name already exists. - * @since x.y.z The hashed password value now uses wp_hash_value() instead of phpass. + * @since x.y.z The hashed password value now uses wp_fast_hash() instead of phpass. * * @param int $user_id User ID. * @param array $args { @@ -125,7 +125,7 @@ public static function create_new_application_password( $user_id, $args = array( * Fires when an application password is created. * * @since 5.6.0 - * @since x.y.z @since x.y.z The hashed password value now uses wp_hash_value() instead of phpass. + * @since x.y.z @since x.y.z The hashed password value now uses wp_fast_hash() instead of phpass. * * @param int $user_id The user ID. * @param array $new_item { @@ -251,7 +251,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 wp_hash_value(). + * @since x.y.z The actual password should now be hashed using wp_fast_hash(). * * @param int $user_id User ID. * @param string $uuid The password's UUID. @@ -304,7 +304,7 @@ 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 wp_hash_value() instead of phpass. + * @since x.y.z The password is now hashed using wp_fast_hash() instead of phpass. * Existing passwords may still be hashed using phpass. * * @param int $user_id The user ID. @@ -484,7 +484,7 @@ public static function chunk_password( $raw_password ) { * @return string Hashed password. */ public static function hash_password( string $password ): string { - return wp_hash_value( $password ); + return wp_fast_hash( $password ); } /** @@ -497,7 +497,7 @@ public static function hash_password( string $password ): string { * @return bool Whether the password matches the hashed password. */ public static function check_password( string $password, string $hash ): bool { - return wp_verify_hashed_value( $password, $hash ); + return wp_verify_fast_hash( $password, $hash ); } /** 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 52dfcc876d267..bbb949ada4a45 100644 --- a/src/wp-includes/class-wp-recovery-mode-key-service.php +++ b/src/wp-includes/class-wp-recovery-mode-key-service.php @@ -37,7 +37,7 @@ public function generate_recovery_mode_token() { * Creates a recovery mode key. * * @since 5.2.0 - * @since x.y.z The stored key is now hashed using wp_hash_value() instead of phpass. + * @since x.y.z The stored key is now hashed using wp_fast_hash() instead of phpass. * * @param string $token A token generated by {@see generate_recovery_mode_token()}. * @return string Recovery mode key. @@ -48,7 +48,7 @@ public function generate_and_store_recovery_mode_key( $token ) { $records = $this->get_keys(); $records[ $token ] = array( - 'hashed_key' => wp_hash_value( $key ), + 'hashed_key' => wp_fast_hash( $key ), 'created_at' => time(), ); @@ -94,7 +94,7 @@ public function validate_recovery_mode_key( $token, $key, $ttl ) { return new WP_Error( 'invalid_recovery_key_format', __( 'Invalid recovery key format.' ) ); } - if ( ! wp_verify_hashed_value( $key, $record['hashed_key'] ) ) { + if ( ! wp_verify_fast_hash( $key, $record['hashed_key'] ) ) { return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) ); } @@ -149,7 +149,7 @@ private function remove_key( $token ) { * Gets the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key is now hashed using wp_hash_value() instead of phpass. + * @since x.y.z Each key is now hashed using wp_fast_hash() instead of phpass. * Existing keys may still be hashed using phpass. * * @return array { @@ -172,7 +172,7 @@ private function get_keys() { * Updates the recovery key records. * * @since 5.2.0 - * @since x.y.z Each key should now be hashed using wp_hash_value() instead of phpass. + * @since x.y.z Each key should now be hashed using wp_fast_hash() instead of phpass. * * @param array $keys { * Associative array of token => data pairs, where the data is an associative diff --git a/src/wp-includes/class-wp-user-request.php b/src/wp-includes/class-wp-user-request.php index cfeacd7bd553c..b52aefda9fb10 100644 --- a/src/wp-includes/class-wp-user-request.php +++ b/src/wp-includes/class-wp-user-request.php @@ -92,7 +92,7 @@ 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 wp_hash_value() instead of phpass. + * @since x.y.z The key is now hashed using wp_fast_hash() instead of phpass. * * @var string */ diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 55ed645d17ef1..1392f3f90ed35 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9100,7 +9100,7 @@ function wp_is_heic_image_mime_type( $mime_type ) { * @param string $message The message to hash. * @return string The hash of the message. */ -function wp_hash_value( string $message ): string { +function wp_fast_hash( string $message ): string { return '$generic$' . sodium_bin2hex( sodium_crypto_generichash( $message ) ); } @@ -9118,12 +9118,12 @@ function wp_hash_value( string $message ): string { * @param string $hash Hash of the message to check against. * @return bool Whether the message matches the hashed message. */ -function wp_verify_hashed_value( string $message, string $hash ): bool { +function wp_verify_fast_hash( string $message, string $hash ): bool { if ( ! str_starts_with( $hash, '$generic$' ) ) { // Back-compat for old phpass hashes. require_once ABSPATH . WPINC . '/class-phpass.php'; return ( new PasswordHash( 8, true ) )->CheckPassword( $message, $hash ); } - return hash_equals( substr( $hash, 9 ), wp_hash_value( $message ) ); + return hash_equals( substr( $hash, 9 ), wp_fast_hash( $message ) ); } diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index e09cb430d93dc..7d4f4c2d3f124 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2989,7 +2989,7 @@ function get_password_reset_key( $user ) { */ do_action( 'retrieve_password_key', $user->user_login, $key ); - $hashed = time() . ':' . wp_hash_value( $key ); + $hashed = time() . ':' . wp_fast_hash( $key ); $key_saved = wp_update_user( array( @@ -3057,7 +3057,7 @@ function check_password_reset_key( $key, $login ) { return new WP_Error( 'invalid_key', __( 'Invalid key.' ) ); } - $hash_is_correct = wp_verify_hashed_value( $key, $pass_key ); + $hash_is_correct = wp_verify_fast_hash( $key, $pass_key ); if ( $hash_is_correct && $expiration_time && time() < $expiration_time ) { return $user; @@ -4882,7 +4882,7 @@ function wp_generate_user_request_key( $request_id ) { array( 'ID' => $request_id, 'post_status' => 'request-pending', - 'post_password' => wp_hash_value( $key ), + 'post_password' => wp_fast_hash( $key ), ) ); @@ -4926,7 +4926,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_verify_hashed_value( $key, $saved_key ) ) { + if ( ! wp_verify_fast_hash( $key, $saved_key ) ) { return new WP_Error( 'invalid_key', __( 'The confirmation key is invalid for this personal data request.' ) ); } diff --git a/tests/phpunit/tests/auth.php b/tests/phpunit/tests/auth.php index c3fabf9b24307..af1a86321619e 100644 --- a/tests/phpunit/tests/auth.php +++ b/tests/phpunit/tests/auth.php @@ -718,7 +718,7 @@ public function test_user_activation_key_is_checked() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_hash_value( $key ), + 'user_activation_key' => strtotime( '-1 hour' ) . ':' . wp_fast_hash( $key ), ), array( 'ID' => $this->user->ID, @@ -756,7 +756,7 @@ public function test_expired_user_activation_key_is_rejected() { $wpdb->update( $wpdb->users, array( - 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_hash_value( $key ), + 'user_activation_key' => strtotime( '-48 hours' ) . ':' . wp_fast_hash( $key ), ), array( 'ID' => $this->user->ID, From 5f1a8ee11aa5a9f8a95aba359ce2e1c8bef35885 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 10:42:13 +0100 Subject: [PATCH 72/73] Docs. --- src/wp-includes/functions.php | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 1392f3f90ed35..68ce80402f23f 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9087,11 +9087,18 @@ function wp_is_heic_image_mime_type( $mime_type ) { } /** - * Returns a cryptographically secure hash of the message. + * Returns a cryptographically secure hash of a message using a fast generic hash function. * - * The BLAKE2b algorithm is used by Sodium to hash the message. + * This function does not salt the value prior to being hashed, therefore input to this function should be generated + * with sufficiently high entropy if the data is sensitive, preferably greater than 128 bits. This function is used + * internally in WordPress core to hash security keys and application passwords, all of which are generated with + * high entropy. + * + * This function is not intended to be used for hashing user-generated passwords. Use wp_hash_password() for that. * - * @TODO explain when to use this instead of wp_hash(). + * Use the wp_verify_fast_hash() function to verify the hash. + * + * The BLAKE2b algorithm is used by Sodium to hash the message. * * @since x.y.z * @@ -9105,10 +9112,11 @@ function wp_fast_hash( string $message ): string { } /** - * Checks whether a plaintext message matches the hashed value. + * Checks whether a plaintext message matches the hashed value. Used to verify values hashed via wp_fast_hash(). * - * The function uses Sodium to hash the message and compare it to the hashed value. If the hash - * is not a generic hash, the hash is treated as a phpass portable hash. + * The function uses Sodium to hash the message and compare it to the hashed value. If the hash is not a generic hash, + * the hash is treated as a phpass portable hash in order to provide backward compatibility for application passwords + * which were hashed using phpass prior to WordPress x.y.z. * * @since x.y.z * From 893103738ea70766adf829f0132ee877bcdcacfd Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 14 Jan 2025 11:11:33 +0100 Subject: [PATCH 73/73] More docs. --- src/wp-includes/functions.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 68ce80402f23f..4d79176e4e480 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -9091,10 +9091,9 @@ function wp_is_heic_image_mime_type( $mime_type ) { * * This function does not salt the value prior to being hashed, therefore input to this function should be generated * with sufficiently high entropy if the data is sensitive, preferably greater than 128 bits. This function is used - * internally in WordPress core to hash security keys and application passwords, all of which are generated with - * high entropy. + * internally in WordPress to hash security keys and application passwords which are generated with high entropy. * - * This function is not intended to be used for hashing user-generated passwords. Use wp_hash_password() for that. + * This function must not be used for hashing user-generated passwords. Use wp_hash_password() for that. * * Use the wp_verify_fast_hash() function to verify the hash. *