mirrored from git://develop.git.wordpress.org/
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
johnbillion
wants to merge
50
commits into
WordPress:trunk
Choose a base branch
from
johnbillion:21022-bcrypt
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,021
−143
Open
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 82d937f
Update the handling of user passwords, password reset keys, and user …
johnbillion f12e93d
Update the handling of the recovery mode key.
johnbillion 5a46809
Update the handling of post passwords.
johnbillion d4e4e47
Automatically rehash user passwords and application passwords after t…
johnbillion 638421d
Docs.
johnbillion 316cb41
Juggle this a bit.
johnbillion 76fa518
Trying to get these tests in order.
johnbillion d707939
More docs.
johnbillion 4a796b7
Retain validity of phpass hashed password reset keys.
johnbillion 64dfd57
Retain validity of a phpass hashed recovery mode key.
johnbillion e161994
Retain validity of phpass hashed user request keys.
johnbillion 548f937
More docs.
johnbillion fe9b053
Add tests for password rehashing when signing in with a username or e…
johnbillion 33cf000
Docs.
johnbillion d51cbc2
Ensure `wp_check_password()` remains compatible with changes to the d…
johnbillion 6b5441c
Simplify this logic.
johnbillion 89e0645
Add tests for Argon2 support.
johnbillion 085b73c
Always pass the return value of `wp_check_password()` through the `ch…
johnbillion 3e078a7
Reintroduce support for the `$wp_hasher` global if it's set.
johnbillion be636be
Test the tests.
johnbillion 4265787
Docs.
johnbillion fd6bcdd
Start splitting up and fixing these tests.
johnbillion 392e612
Coding standards.
johnbillion 9040193
Add a todo.
johnbillion bc99ca1
Add some more assertions to the application password auth tests.
johnbillion 58a89e2
Correct and add tests for the application password rehashing.
johnbillion 49871d0
Done.
johnbillion b42e57b
Coding standards.
johnbillion 2baeb19
Allow either the name or password to change in order to allow an appl…
johnbillion 95f6d94
Docs.
johnbillion f8974eb
PHP 7.2 compatibility.
johnbillion ff85df2
Docs.
johnbillion ebcdd26
Add a test to ensure the user's account password isn't touched when r…
johnbillion 140b9d4
Merge branch 'trunk' into 21022-bcrypt
johnbillion ea8c56c
One fewer regexes in the world.
johnbillion 2ef2077
Merge branch 'trunk' into 21022-bcrypt
johnbillion 3a951ef
Allow the password hashing options to be filtered.
johnbillion 53acf70
Docs.
johnbillion 5944d9a
Merge branch 'trunk' into 21022-bcrypt
johnbillion 820f48f
Cease hashing passwords as md5 during the upgrade routine and/or the …
johnbillion c07e618
More tests for empty values.
johnbillion 5d33621
More updates to the tests.
johnbillion 0063154
Add a test to verify that a password gets rehashed when the default c…
johnbillion 1aea250
This might as well go here.
johnbillion d6c7837
Add tests for post password handling.
johnbillion 8b1dbd2
Merge branch 'trunk' into 21022-bcrypt
johnbillion 5824f4c
Clear the cookie value before performing the assertions.
johnbillion d86ef6d
Remove assertions that are unnecessarily specific to bcrypt.
johnbillion 09ddc5e
The default bcrypt cost was increased in PHP 8.4.
johnbillion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 ); | ||
} | ||
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 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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. | ||
* | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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:
You can verify that this is safer than a naked
password_hash()
with a simple unit test:A
repeated 72 times.A
repeated 73 times, using the hash from the previous step.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.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.