Make WordPress Core

Opened 9 years ago

Closed 6 years ago

Last modified 6 years ago

#14803 closed enhancement (fixed)

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

Reported by: coffee2code Owned by: duck_
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.0.1
Component: Security Keywords: has-patch ux-feedback
Focuses: Cc:
PR Number:


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 (5)

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

Download all attachments as: .zip

Change History (13)

9 years ago

Patch mentioned in ticket.

9 years ago

Image mentioned in the ticket.

#1 @nacin
9 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.

#2 @Denis-de-Bernardy
9 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.



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.

9 years ago

merged messages

9 years ago

merged messages

#3 @duck_
9 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.

#4 @markjaquith
9 years ago

  • Milestone changed from Awaiting Review to Future Release

#5 @coffee2code
7 years ago

  • Version set to 3.0.1

6 years ago

#6 @duck_
6 years ago

Since [19771] for #19599, wp_salt() will automatically create secret keys/salts if any key is duplicated or has the default value. Therefore, I think we can just remove this warning now, see 14803.remove.diff.

#7 @duck_
6 years ago

  • Owner set to duck_
  • Resolution set to fixed
  • Status changed from new to closed

In 24813:

Remove unnecessary secret_salt_warning().

If salts/keys are not defined in wp-config.php then they will be generated
automatically and stored in the database. [19771] also deals with values
that are duplicated or set to default.

Fixes #14803.

#8 @duck_
6 years ago

  • Milestone changed from Future Release to 3.7
Note: See TracTickets for help on using tickets.