-
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 #7333
base: trunk
Are you sure you want to change the base?
Changes from 48 commits
fe1c16e
82d937f
f12e93d
5a46809
d4e4e47
638421d
316cb41
76fa518
d707939
4a796b7
64dfd57
e161994
548f937
fe9b053
33cf000
d51cbc2
6b5441c
89e0645
085b73c
3e078a7
be636be
4265787
fd6bcdd
392e612
9040193
bc99ca1
58a89e2
49871d0
b42e57b
2baeb19
95f6d94
f8974eb
ff85df2
ebcdd26
140b9d4
ea8c56c
2ef2077
3a951ef
53acf70
5944d9a
820f48f
c07e618
5d33621
0063154
1aea250
d6c7837
8b1dbd2
5824f4c
2502179
2321412
d86ef6d
09ddc5e
a27411d
e5f8d5c
9bb86a8
1cebd80
537ee4a
8a99edd
68634f0
adf983f
754519b
35d1885
0c49b94
f85a6cb
413e81f
4880c6e
cb46077
a9a1c17
8b4d40e
9994922
d80491d
f222f64
c966123
7b3eb9a
e3fcdca
1d1ab91
a94d7ff
8e4652e
20eb361
37e0cd7
9a58b91
d62c5d3
5f1a8ee
8931037
141b031
a5c105b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 ) ) { | ||
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. I feel like this has the potential for a lot of breakage. IIRC, a ton of plugins that do custom user creation and/or profile updates do checks like "if empty, create new password hash instance". This opens up the possibility that password hashes are created with $wp_hasher, but not validated with it, since those same plugins would not re-instantiate on the fly during validation. 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. 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. Ok, so we can boil that down to: plugins that manually hash a password with The problem you describe would only take effect if a plugin specifically assigns a hasher instance to the Am I understanding correctly? Search results for instances of that here: https://wpdirectory.net/search/01JH19AV75Q6TJA6VRZ0A9EMG7 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. The reverse case is also true, there are plugins calling It's not immediately clear to me what the implications are. There are several scenarios that'd need to be taken into account. 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. I have this documented in the Fortress docs here There's nothing that can be done to support this particular edge case. For me, that was an acceptable trade-off. But the scale is obviously different. 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. I'll do some digging through plugins which are using |
||
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 ); | ||
johnbillion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
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 ); | ||
johnbillion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
$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 ) { | ||
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. Why is this a new pluggable function? As-is, the WP password functionality now exposes a new public API for plugins to use, which also means that any custom code that changes pluggable functions needs to implement this "interface" even if not needed. Is there a reason why this can't be done inline inside Usually, rehashing is only performed if you know that a valid password was provided. The function signature with just 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. The 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. How should an implementor even know what It could be any kind of 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. IMO, At a minimum, the new API should also receive a user id argument, and a "type" argument. Type being an enum of 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.
This for 100%, but different path towards end goal. Everything should remain how it is atm, from core functionality perspective. Next stages and iterations to allow - give conditions all of these concerns to be addressable via plugins - web industry will show the right path forward. That way, there is no need of backward fatal changes in the core like XMLRPC and mutation of crucial and well implemented functionalities like Application Passwords.
|
||
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. | ||
* | ||
|
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.
There's no need to use
wp_hash_password
here.wp_hash
is appropriate.