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.

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

#21022 Switch to using bcrypt for hashing passwords and security keys #7333

Open
wants to merge 50 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
fe1c16e
Begin switching the password hashing mechanism from phpass to bcrypt.
johnbillion Sep 11, 2024
82d937f
Update the handling of user passwords, password reset keys, and user …
johnbillion Sep 11, 2024
f12e93d
Update the handling of the recovery mode key.
johnbillion Sep 11, 2024
5a46809
Update the handling of post passwords.
johnbillion Sep 11, 2024
d4e4e47
Automatically rehash user passwords and application passwords after t…
johnbillion Sep 11, 2024
638421d
Docs.
johnbillion Sep 11, 2024
316cb41
Juggle this a bit.
johnbillion Sep 11, 2024
76fa518
Trying to get these tests in order.
johnbillion Sep 11, 2024
d707939
More docs.
johnbillion Sep 11, 2024
4a796b7
Retain validity of phpass hashed password reset keys.
johnbillion Sep 11, 2024
64dfd57
Retain validity of a phpass hashed recovery mode key.
johnbillion Sep 11, 2024
e161994
Retain validity of phpass hashed user request keys.
johnbillion Sep 11, 2024
548f937
More docs.
johnbillion Sep 11, 2024
fe9b053
Add tests for password rehashing when signing in with a username or e…
johnbillion Sep 12, 2024
33cf000
Docs.
johnbillion Sep 12, 2024
d51cbc2
Ensure `wp_check_password()` remains compatible with changes to the d…
johnbillion Sep 12, 2024
6b5441c
Simplify this logic.
johnbillion Sep 12, 2024
89e0645
Add tests for Argon2 support.
johnbillion Sep 12, 2024
085b73c
Always pass the return value of `wp_check_password()` through the `ch…
johnbillion Sep 12, 2024
3e078a7
Reintroduce support for the `$wp_hasher` global if it's set.
johnbillion Sep 12, 2024
be636be
Test the tests.
johnbillion Sep 12, 2024
4265787
Docs.
johnbillion Sep 12, 2024
fd6bcdd
Start splitting up and fixing these tests.
johnbillion Sep 12, 2024
392e612
Coding standards.
johnbillion Sep 12, 2024
9040193
Add a todo.
johnbillion Sep 13, 2024
bc99ca1
Add some more assertions to the application password auth tests.
johnbillion Sep 17, 2024
58a89e2
Correct and add tests for the application password rehashing.
johnbillion Sep 17, 2024
49871d0
Done.
johnbillion Sep 17, 2024
b42e57b
Coding standards.
johnbillion Sep 17, 2024
2baeb19
Allow either the name or password to change in order to allow an appl…
johnbillion Sep 17, 2024
95f6d94
Docs.
johnbillion Sep 17, 2024
f8974eb
PHP 7.2 compatibility.
johnbillion Oct 8, 2024
ff85df2
Docs.
johnbillion Oct 8, 2024
ebcdd26
Add a test to ensure the user's account password isn't touched when r…
johnbillion Oct 8, 2024
140b9d4
Merge branch 'trunk' into 21022-bcrypt
johnbillion Oct 8, 2024
ea8c56c
One fewer regexes in the world.
johnbillion Oct 8, 2024
2ef2077
Merge branch 'trunk' into 21022-bcrypt
johnbillion Oct 9, 2024
3a951ef
Allow the password hashing options to be filtered.
johnbillion Oct 11, 2024
53acf70
Docs.
johnbillion Oct 11, 2024
5944d9a
Merge branch 'trunk' into 21022-bcrypt
johnbillion Oct 21, 2024
820f48f
Cease hashing passwords as md5 during the upgrade routine and/or the …
johnbillion Oct 21, 2024
c07e618
More tests for empty values.
johnbillion Oct 21, 2024
5d33621
More updates to the tests.
johnbillion Oct 21, 2024
0063154
Add a test to verify that a password gets rehashed when the default c…
johnbillion Oct 22, 2024
1aea250
This might as well go here.
johnbillion Oct 22, 2024
d6c7837
Add tests for post password handling.
johnbillion Oct 22, 2024
8b1dbd2
Merge branch 'trunk' into 21022-bcrypt
johnbillion Oct 22, 2024
5824f4c
Clear the cookie value before performing the assertions.
johnbillion Oct 22, 2024
d86ef6d
Remove assertions that are unnecessarily specific to bcrypt.
johnbillion Nov 25, 2024
09ddc5e
The default bcrypt cost was increased in PHP 8.4.
johnbillion Nov 25, 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
8 changes: 1 addition & 7 deletions src/wp-admin/includes/upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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();

Expand Down
8 changes: 8 additions & 0 deletions src/wp-includes/class-wp-application-passwords.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,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.
Expand Down Expand Up @@ -284,6 +285,11 @@ public static function update_application_password( $user_id, $uuid, $update = a
$save = true;
}

if ( ! empty( $update['password'] ) && $item['password'] !== $update['password'] ) {
$item['password'] = $update['password'];
$save = true;
}

if ( $save ) {
$saved = static::set_user_application_passwords( $user_id, $passwords );

Expand All @@ -296,6 +302,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.
* Existing passwords may still be hashed using phpass.
*
* @param int $user_id The user ID.
* @param array $item {
Expand Down
55 changes: 29 additions & 26 deletions src/wp-includes/class-wp-recovery-mode-key-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 ] ) ) {
Expand All @@ -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 ( ! wp_check_password( $key, $record['hashed_key'] ) ) {
return new WP_Error( 'hash_mismatch', __( 'Invalid recovery key.' ) );
}

Expand Down Expand Up @@ -169,9 +151,20 @@ 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.
* 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() );
Expand All @@ -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 ) {
Expand Down
2 changes: 2 additions & 0 deletions src/wp-includes/class-wp-user-request.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down
1 change: 1 addition & 0 deletions src/wp-includes/class-wp-user.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 75 additions & 35 deletions src/wp-includes/pluggable.php
Original file line number Diff line number Diff line change
Expand Up @@ -2603,90 +2603,129 @@ function wp_hash( $data, $scheme = 'auth' ) {
* instead use the other package password hashing algorithm.
*
* @since 2.5.0
* @since x.y.z The password is now hashed using bcrypt instead of phpass.
*
* @global PasswordHash $wp_hasher PHPass object.
* @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 ) ) {
require_once ABSPATH . WPINC . '/class-phpass.php';
// By default, use the portable hash from phpass.
$wp_hasher = new PasswordHash( 8, true );
if ( ! empty( $wp_hasher ) ) {
return $wp_hasher->HashPassword( trim( $password ) );
}

return $wp_hasher->HashPassword( trim( $password ) );
/**
* 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 );
Copy link

@soatok soatok Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be advisable to prevent the 72-byte truncation by design.

Consider this:

$prehashed = base64_encode( hash_hmac( 'sha512', trim( $password ), CONFIG_SALT_GOES_HERE, true ) );
return password_hash( $prehashed, PASSWORD_BCRYPT, $options );

You can verify that this is safer than a naked password_hash() with a simple unit test:

  • Hash the letter A repeated 72 times.
  • Attempt to verify a password consisting of the letter A repeated 73 times, using the hash from the previous step.
  • This should succeed for password_hash() with BCRYPT, but fail with my strategy.

Why hash_hmac()?

This binds the prehash to a server-side secret, that can exist alongside the usual WordPress secrets. In essence, without this secret, passwords cannot even begin to be cracked.

Why SHA-512 + Base64?

SHA-512 produces 64 bytes of output, which is just shy of the 72 byte limit for bcrypt. However, there can be a NUL character in the midst, so it could truncate early if not encoded.

By base64-encoding the SHA-512 hash (really, HMAC output), you prevent NUL characters. This is the optimal trade-off.

  • By itself, SHA-512 hashes have a collision resistance of 256 bits.
  • The base64-encoded hash will be about 86 bytes (88 with padding).
  • Although bcrypt silently truncates the last 14 (16 with padding) characters, we still have 72 base64-encoded characters (about 432 bits of security).
  • Each WordPress install will have a different HMAC key, which will frustrate crackers tremendously (especially if they can only leak the contents of the SQL database, not the filesystem.

I believe this small bit of complexity is worth the obvious security benefits.

Can we do it later?

I don't believe we can. We either adopt this strategy as part of the phpass migration, or we don't do it ever.

}
endif;

if ( ! function_exists( 'wp_check_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.
* 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.
*
* 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.
* Passwords hashed with md5 are no longer supported.
*
* @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.
* @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 = '' ) {
global $wp_hasher;

// If the hash is still md5...
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 );
}
$check = false;

// If the hash is still md5 or otherwise truncated then invalidate it.
if ( strlen( $hash ) <= 32 ) {
/**
* 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.
* 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.
* @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 the stored hash is longer than an MD5,
* presume the new style phpass portable hash.
*/
if ( empty( $wp_hasher ) ) {
if ( ! empty( $wp_hasher ) ) {
$check = $wp_hasher->CheckPassword( $password, $hash );
} elseif ( str_starts_with( $hash, '$P$' ) ) {
require_once ABSPATH . WPINC . '/class-phpass.php';
// By default, use the portable hash from phpass.
$wp_hasher = new PasswordHash( 8, true );
// Use the portable hash from phpass.
$hasher = new PasswordHash( 8, true );
$check = $hasher->CheckPassword( $password, $hash );
} else {
$check = password_verify( $password, $hash );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with above, the congruent change here would be:

$prehashed = base64_encode( hash_hmac( 'sha512', trim( $password ), CONFIG_SALT_GOES_HERE, true ) );
$check = password_verify( $prehashed, $hash );

}

$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
* 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
*
* @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;
}

/** 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;

if ( ! function_exists( 'wp_generate_password' ) ) :
/**
* Generates a random password drawn from the defined set of characters.
Expand Down Expand Up @@ -2835,6 +2874,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.
*
Expand Down
7 changes: 2 additions & 5 deletions src/wp-includes/post-template.php
Original file line number Diff line number Diff line change
Expand Up @@ -882,14 +882,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 );
}

/**
Expand Down
Loading
Loading