From 250217989e828b8846c745fba8ff7376e466c604 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Tue, 5 Nov 2024 00:52:25 +0000 Subject: [PATCH 1/3] 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 2/3] 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 1cebd80c28cf5b537b4aabad392cbf5f879aa020 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 12 Dec 2024 12:53:03 +0100 Subject: [PATCH 3/3] 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.