Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#21022 Add sha2 pre-hashing for passwords #5

Open
wants to merge 2 commits into
base: 21022-bcrypt
Choose a base branch
from
+116 −26
Open

#21022 Add sha2 pre-hashing for passwords #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2502179
Implement pre-hashing with sha384 to retain entropy of passwords over…
johnbillion Nov 5, 2024
2321412
Implement domain separation for the password to protect against passw…
johnbillion Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/wp-includes/pluggable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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, 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 );
}
endif;

Expand Down Expand Up @@ -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', 'wp-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 );
}

Expand Down Expand Up @@ -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;

Expand Down
116 changes: 93 additions & 23 deletions tests/phpunit/tests/auth.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class Tests_Auth extends WP_UnitTestCase {

protected static $phpass_length_limit = 4096;

protected static $password_length_limit = 4096;

/**
* Action hook.
*/
Expand Down Expand Up @@ -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 ) );
}

/**
Expand All @@ -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 ) );
}

/**
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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() );

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
Expand All @@ -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(
Expand Down Expand Up @@ -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 );
Expand Down