WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 11 months ago

#14803 new enhancement

Admins should be warned if authentication keys and salts have the default phrase

Reported by: coffee2code Owned by:
Priority: normal Milestone: Future Release
Component: Security Version: 3.0.1
Severity: normal Keywords: has-patch ux-feedback
Cc:

Description

WordPress already warns admin users if any of the eight authentication keys/salts are not present in the wp-config.php. (See secret_salt_warning() in wp-admin/includes/ms.php) While performing that check, WP should also see if any of the keys/salts are using the default string of "put your unique phrase here".

The default string is pre-set for all eight keys/salts that ship in wp-config-sample.php. While the inline documentation indicates that those values should be changed, there is no notice or prompting to alert/remind the admin if the default string is left intact.

Bear in mind that wp_salt() (in wp-includes/pluggable.php) ignores the keys/salts that use the default phrase or are blank strings, so security isn't compromised. However, if we want the admins to define secure phrases in wp-config.php, we should make them aware when that's not the case.

The attached patch modifies secret_salt_warning() to also check that the keys/salts aren't using the default phrase and aren't blank strings (same check as done in wp_salt()). If any are, the warning message provides a link to the wordpress.org secret key service.

See the attached image to see an example where I've removed LOGGED_IN_SALT from wp-config.php (the error message for that is already what WP generates) and where I've left the default phrase in place for AUTH_KEY and AUTH_SALT and set NONCE_SALT to (triggering the error message added by the attached patch).

Attachments (4)

14803.diff (1.8 KB) - added by coffee2code 3 years ago.
Patch mentioned in ticket.
14803.png (58.9 KB) - added by coffee2code 3 years ago.
Image mentioned in the ticket.
14803.2.diff (2.0 KB) - added by duck_ 3 years ago.
merged messages
14803.2.png (18.2 KB) - added by duck_ 3 years ago.
merged messages

Download all attachments as: .zip

Change History (9)

coffee2code3 years ago

Patch mentioned in ticket.

coffee2code3 years ago

Image mentioned in the ticket.

comment:1 nacin3 years ago

I think we can probably merge these into a cleaner, single message nag.

Also, this only shows for multisite super admins. Maybe we should show it for single-site admins too? Originally brought up in #11764.

comment:2 Denis-de-Bernardy3 years ago

We could also use a unique salt per user and per session. And bcrypt (i.e. blowfish) to hash passwords. And hmac to generate nonces. Instead of trying to reinvent the wheel.

http://php.net/manual/en/function.crypt.php

http://php.net/manual/en/function.hash-hmac.php

PHP pass, which is included in WP, has the needed code for bcrypt. hash_hmac becomes available with WP 3.2 assuming we target PHP 5.2.

duck_3 years ago

merged messages

duck_3 years ago

merged messages

comment:3 duck_3 years ago

  • Keywords ux-feedback added

14803.2.diff provides a patch for merged messages, ux-feedback for double checking the strings.

It does seem strange this function is provided for multisite users and not singlesite. I have also written a patch for the Health Check plugin if this check sticks in multisite only.

comment:4 markjaquith2 years ago

  • Milestone changed from Awaiting Review to Future Release

comment:5 coffee2code11 months ago

  • Version set to 3.0.1
Note: See TracTickets for help on using tickets.