Make WordPress Core

Opened 14 years ago

Closed 11 years ago

Last modified 11 years ago

#14803 closed enhancement (fixed)

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

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


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 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 14 years ago.
Patch mentioned in ticket.
14803.png (58.9 KB) - added by coffee2code 14 years ago.
Image mentioned in the ticket.
14803.2.diff (2.0 KB) - added by duck_ 14 years ago.
merged messages
14803.2.png (18.2 KB) - added by duck_ 14 years ago.
merged messages
14803.remove.diff (1.3 KB) - added by duck_ 11 years ago.

Download all attachments as: .zip

Change History (13)

14 years ago

Patch mentioned in ticket.

14 years ago

Image mentioned in the ticket.

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

14 years ago

merged messages

14 years ago

merged messages

#3 @duck_
14 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
13 years ago

  • Milestone changed from Awaiting Review to Future Release

#5 @coffee2code
12 years ago

  • Version set to 3.0.1

11 years ago

#6 @duck_
11 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_
11 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_
11 years ago

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