WordPress.org

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:

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

@coffee2code
9 years ago

Patch mentioned in ticket.

@coffee2code
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.

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_
9 years ago

merged messages

@duck_
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

@duck_
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.