WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#5367 closed defect (bug) (fixed)

Wordpress cookie authentication vulnerability

Reported by: sjmurdoch Owned by: westi
Milestone: 2.5 Priority: normal
Severity: normal Version: 2.3.1
Component: Security Keywords: security, password, md5
Focuses: Cc:

Description

Wordpress Cookie Authentication Vulnerability

Original release date: 2007-11-19
Last revised: 2007-11-19
Latest version: http://www.cl.cam.ac.uk/users/sjm217/advisories/wordpress-cookie-auth.txt
CVE ID: <pending>
Source: Steven J. Murdoch http://www.cl.cam.ac.uk/users/sjm217/

Systems Affected:

Wordpress 1.5 -- 2.3.1 (including current version, as of 2007-11-19)

Overview:

With read-only access to the Wordpress database, it is possible to
generate a valid login cookie for any account, without resorting to a
brute force attack. This allows a limited SQL injection vulnerability
to be escalated into administrator access.

This vulnerability is known to be actively exploited, hence the
expedited public release.

I. Description

For authentication, the Wordpress user database stores the MD5 hash
of login passwords. A client is permitted access if they can present a
password whose hash matches the stored one.

 $ mysql -u wordpress -p wordpress
   Enter password: ********

   mysql> SELECT ID, user_login, user_pass FROM wp_users;
   +----+-------------+----------------------------------+
   | ID | user_login  | user_pass                        |
   +----+-------------+----------------------------------+
   |  1 | admin       | 4cee2c84f6de6d89a4db4f2894d14e38 |
   ...

Of course, entering your password after each action that requires
authorization would be exceptionally tedious. So, after logging in,
Wordpress presents the client with two cookies:

wordpressuser_6092254072ca971c70b3ff302411aa5f=admin
wordpresspass_6092254072ca971c70b3ff302411aa5f=813cadd8658c4776afbe5de8f304a684

The cookie names contains the MD5 hash (6092...1a5f) of the blog URL.
The value of wordpressuser_... is the login name, and the value of
wordpresspass is the double-MD5 hash of the user password.

Wordpress will permit access to a given user account if the
wordpressuserpass_... cookie matches the hash of the specified user's
wp_users.user_pass database entry.

In other words, the database contains MD5(password) and the cookie
contains MD5(MD5(password)). It is thus trivial to convert a database
entry into an authentication cookie.

At this point the vulnerability should be clear. If an attacker can
gain read access to the wp_user table, for example due to a publicly
visible backup or SQL injection vulnerability, a valid cookie can be
generated for any account.

This applies even if the user's password is sufficiently complex to
resist brute force and rainbow table attacks. While it should be
computationally infeasible to go backwards from MD5(password) to
password, the attacker needs only to go forwards.

The exploitation steps are therefore:

1) Find the hash of the blog URL: Either just look at the URL, or

create an account to get a user cookie

2) Read the user_pass entry from wp_users table: Look for

backups, perform SQL injection, etc...

3) Set the following cookies:

wordpressuser_<MD5(url)>=admin
wordpresspass_<MD5(url)>=MD5(user_pass)

4) You have admin access to the blog

II. Impact

A remote attacker, with read access to the password database can gain
administrator rights. This may be used in conjunction with an SQL
injection attack, or after locating a database backup.

An attacker who has alternatively compromised the database of one
Wordpress blog can also gain access to any other whose users have the
same password on both.

III. Solution

No vendor patch is available.
No timeline for a vendor patch has been announced.

Workarounds:

  • Protect the Wordpress database, and do not allow backups to be released.
  • Keep your Wordpress installation up to date. This should reduce the risk that your database will be compromised.
  • Do not share passwords across different sites.
  • If you suspect a database to be compromised, change all passwords to different ones. It is not adequate to change the passwords to the same ones, since Wordpress does not "salt" [1] the password database.
  • Remove write permissions on the Wordpress files for the system account that the webserver runs as. This will disable the theme editor, but make it more difficult to escalate Wordpress administrator access into the capability to execute arbitrary code
  • Configure the webserver to not execute files in any directory writable by the webserver system account (e.g. the upload directory).

Potential fixes:

The problem occurs because it is easy to go from the password hash
in the database to a cookie (i.e the application of MD5 is the wrong
way around). The simplest fix is to store MD5(MD5(password)) in the
database, and make the cookie MD5(password). This still makes it
infeasible to retrieve the password from a cookie, but means that it
is also infeasible to generate a valid cookie from the database
entry.

However, there are other vulnerabilities in the Wordpress cookie and
password handling, which should be resolved too:

  • Passwords are unsalted [2], leaving them open to brute force, rainbow table and other attacks [3].
  • It is impossible to revoke a cookie without changing the user's password.
  • Cookies do not contain an expiry time, so are always valid (until the user's password changes)
  • There ought to be an option to limit cookies to a particular IP address or range.

References:

[1] http://en.wikipedia.org/wiki/Salt_(cryptography)
[2] http://trac.wordpress.org/ticket/2394
[3] http://www.lightbluetouchpaper.org/2007/11/16/google-as-a-password-cracker/

Timeline:

2007-10-29: security@… notified; no response
2007-11-02: security@… notified; Confirmation of active exploitation requested by Wordpress
2007-11-02: Confirmation sent; no response
2007-11-19: Advisory released to full-disclosure and BugTraq

Attachments (9)

auth.diff (7.1 KB) - added by westi 9 years ago.
First pass patch for switching to auth cookie
secret_key.diff (1.0 KB) - added by ryan 9 years ago.
Add SECRET_KEY to salt
secure_cookie.diff (10.6 KB) - added by ryan 9 years ago.
secure_cookie.2.diff (11.5 KB) - added by ryan 9 years ago.
secure_cookie.3.diff (11.6 KB) - added by ryan 9 years ago.
Allow POST requests a grace period
secure_cookie.4.diff (14.4 KB) - added by ryan 9 years ago.
secure_cookie.5.diff (14.7 KB) - added by ryan 9 years ago.
secret_key.2.diff (1.4 KB) - added by ryan 9 years ago.
5367.diff (441 bytes) - added by DD32 9 years ago.

Download all attachments as: .zip

Change History (84)

#1 @ryan
9 years ago

  • Milestone set to 2.4

Would phpass [1] for salting and hashing passwords plus a two-way salted encryption (such as [2]) on cookies be good enough? Are there more contemporary libraries to consider that are portable enough for WP's needs?

[1] http://www.openwall.com/phpass/
[2] http://www.tonymarston.net/php-mysql/encryption.html#2004-08-27

#2 @sjmurdoch
9 years ago

This vulnerability been assigned the CVE candidate ID CVE-2007-6013.

#3 @sjmurdoch
9 years ago

  • Cc sjmurdoch added

The phpass library looks like a reasonable basis for password hashing.

That PHP encryption algorithm looks like something from the 19th century, and is almost certainly broken. Ignore it.

Is there really nothing better than MD5 to use? For hashing SHA-1 is better and some proper symmetric block ciphers would also be useful.

The scheme I was thinking of is something along the lines of storing salt,H(crypted salted password) in the database and in the cookie storing E(user id,crypted salted password,start time) and its HMAC; where H is the hash and E, HMAC are encryption and authentication under site-specific keys.

This means that given the contents of the database, the attacker cannot create a valid cookie as you can't go from H(crypted salted password) to crypted salted password. Also, since the crypted salted password is encrypted it is infeasible to brute force the password given only a cookie. Setting a start time means that cookies can be expired and this timestamp can't be modified due to the HMAC.

This is just an initial idea and more refinement and analysis are needed, but that's a rough outline.

#4 @ryan
9 years ago

That scheme sounds good to me, but since we currently support PHP back to 4.2 we have to be aware of portability problems. I don't think we can count on sha1() or the hash module, for example.

#5 @santosj
9 years ago

If someone is still running PHP 4.2, they have even more security issues to worry about than this one. I would suggest not caring for them based on that fact, but since I'm not a security expert, I'll leave it at that.

#6 @santosj
9 years ago

Also, the fact that 4.3 totally eclipsed 4.2 since 2005, and 4.2 stands at less than 5%.

I'm just throwing this out there, but is it still worth it?

#7 @jhodgdon
9 years ago

  • Keywords security password md5 added

There are some interesting comments on secure hashing on this page: http://us.php.net/sha1 ; lots of links to alternatives for old versions of PHP, though none of them looks all that great.

#8 @sjmurdoch
9 years ago

In case I wasn't clear, I should point out that MD5 is not the problem here. Switching to SHA-1 will leave Wordpress vulnerable to precisely the same issues. Although MD5 is vulnerable to collisions from carefully crafted input, this is not an issue for Wordpress's use.

I think the optimum solution would be to use an existing scheme, and preferably a well-audited library for handling authentication and cookie management. Is there really nothing out these which does the job. I'm not a PHP programmer, but it seems that what Wordpress needs is a very common requirement.

#9 @santosj
9 years ago

Question: Would not having the username and password in the cookie fix this and not break anything else?

I'm not sure why sessions aren't used instead to store this information (always confused about this). Which you'll have another problem with session hijacking, but there are measures that can easily be used to correct that problem.

#10 @dougal
9 years ago

On the one hand, this is yet another case of "Let's take some generic, lame, bad security scenario, slap the name 'WordPress' on it (for no apparent reason other than the fact that WP is popular), and release a security announcement!" As has already been noted, if an attacker already has read access to your database, then you've probably lost the battle, regardless of anything else.

On the other hand, we could definitely stand to improve the security of the cookie authentication. Things are the way they are now to make it convenient for the user. But it's really hard to provide convenient persistent authentication in a secure fashion without some external method of security (e.g. SSL). We probably need to provide better support for SSL (when it's available), or lose the persistence in most cases. There might be situations where we can provide persistence more securely, but probably not in all server setups.

#11 @sjmurdoch
9 years ago

@dougal

I didn't pick WordPress arbitrarily, or because it is popular, but rather because my WordPress blog (and probably a large number of others) was hacked using precisely this vulnerability.

And no, just because an attacker has had read access to your database does not mean you've lost the battle. Using security measures that have been standard since the 1970s (password hashing and salting), it is quite easy to recover from such compromises, by restoring the database. Then there is the well established principle of "defence in depth".

There are a variety of ways an attacker could get read access to the database, while not being able to do anything more. For example, certain SQL injection flaws can only read but not modify tables, or the attacker could simply find a backup. Before I went public with this vulnerability, I trawled Google for people who had left database backups online and recommended that they remove the files.

Regarding fixes, I think it is possible to improve the security without affecting user convenience at all. Salting passwords and hashing cookies in the right direction would be a good start. Protecting data on the wire (with SSL or otherwise) is nice, but a much less important issue than the one raised here and in #2394.

#12 @sjmurdoch
9 years ago

Eduardo Tongson posted a helpful message to full-disclosure pointing to two papers on cookie authentication. I strongly recommend reading these before implementing any changes.

#13 @nbachiyski
9 years ago

Another way is to leave md5(md5(pass)) in the cookie and store triple-md5-ed pass in the DB. Thus we cover the following cases:

  • if the attacker gets the cookie it's almost unbreakable, just as it is now
  • if the attacker gets the DB, she can't generate a cookie
  • if the attacker gets the DB, she can't use rainbow to get the password

And the logic is pretty simple and foolproof. Of course one day the rainbow databases may include many md5 strings hashes, or even triple hashes, but I think it would suffice for now.

The only disadvantage of all the techniques proposed above is that the cookie value cannot be generated using the information. Now this functionality is used to update the cookies, when the URL of the blog changes. The code is in wp-admin/includes/misc.php.

#14 @Otto42
9 years ago

Salting the passwords would be nice to prevent dictionary attacks, and using something other than MD5 would be nice as well, but these won't solve the problem.

Why are we storing the username and password in cookies at all? Is there any particular reason that we're not using PHP sessions? With a session, we could have the username and hashed password remain on the server as session variables, and the only cookie would be the randomly generated session ID. No real vulnerability there at all, eh?

#15 follow-up: @jammycakes
9 years ago

Otto42:

Salting the passwords would be nice to prevent dictionary attacks, and using something other than MD5 would be nice as well, but these won't solve the problem.

They won't solve the problem completely but they will help quite considerably, so this is not a valid reason for not salting. It's all part of a strategy of "defence in depth," which is an approach that security experts all strongly recommend.

You need to have an extremely good reason to reject an additional recommended security measure, especially when its impact on usability or other functionality is zero.

However, I'd agree that sessions are the way to go rather than throwing around hashes of your user name and password in cookies.

nbachiyski:

one day the rainbow databases may include many md5 strings hashes, or even triple hashes, but I think it would suffice for now.

I wouldn't put it past crackers to come up with one, especially given the popularity of WordPress. That's why I'm strongly in favour of salting in addition to everything else. Again, it boils down to "defence in depth."

In short, we need to do all the following:

  1. salt users' passwords
  2. use a more secure hash algorithm than MD5
  3. use PHP sessions rather than user name and password
  4. give sessions a finite (sliding) expiration time of no more than a few hours.

It would also be a good idea to consider tying sessions to a specific IP address or range of IP addresses, although this could cause usability problems for AOL users who go through random proxy servers.

Another thing -- why has this been given a milestone of 2.4? Since it is a security issue that is actively being exploited, shouldn't it be scheduled, at least partially, for the next security release?

#16 in reply to: ↑ 15 @DD32
9 years ago

Replying to jammycakes:

Another thing -- why has this been given a milestone of 2.4? Since it is a security issue that is actively being exploited, shouldn't it be scheduled, at least partially, for the next security release?

Things are fixed in Trunk(2.4) and then backported to 2.3
Major changes such as this would also go into the next stable release rather than a maintainence release i believe. Allthough extra checks might be added to a 2.3 maintainence release to at least remove some of the vulnerability(ie. patch it, just not completely replace the authentication functions) is need be.

#17 @westi
9 years ago

  • Owner changed from anonymous to westi
  • Status changed from new to assigned

@westi
9 years ago

First pass patch for switching to auth cookie

#18 follow-ups: @westi
9 years ago

I've uploaded a first pass patch for auth cookies

Major changes to the current mechanism:

  1. Cookie is not based on the password hash
  2. Cookie is not derived from information stored in the database

Known issues:

  1. Only supports a single authentication token for a user so you cannot be logged in from two places at once.

#19 in reply to: ↑ 18 ; follow-up: @darkdragon
9 years ago

Replying to westi:

Known issues:

  1. Only supports a single authentication token for a user so you cannot be logged in from two places at once.

I would term this a feature, but since I'm always signed in at home and usually sign in from various other places, it could become a hassle. What if I'm in a location temporary and leave without the ability to go back? Will there be a time limit of when that token will expire so that I can eventually sign in at another location even if I hadn't signed out at that other place?

#20 in reply to: ↑ 19 @westi
9 years ago

Replying to darkdragon:

Replying to westi:

Known issues:

  1. Only supports a single authentication token for a user so you cannot be logged in from two places at once.

I would term this a feature, but since I'm always signed in at home and usually sign in from various other places, it could become a hassle. What if I'm in a location temporary and leave without the ability to go back? Will there be a time limit of when that token will expire so that I can eventually sign in at another location even if I hadn't signed out at that other place?

If you sign in somewhere new then the old token is invalidated.

There is currently no timeout on tokens but for multiple logins to be supported we will need a way of timing tokens out.

#21 follow-up: @sjmurdoch
9 years ago

There are roughly two approaches to dealing with this problem.

Sessions involves a record being kept of each login session, which PHP already has existing infrastructure for this. The down side is that there is a database write for each login, and the size of the session table scales with the number of active sessions. Alternatively there can be a restriction of one session per user, as in westi's patch.

An alternative is to use something along the lines of the papers I mentioned earlier. This supports multiple sessions per user, with no need to store extra state. Database writes are only needed on changing the password.

A variant of this is to store the hash post-image of a cookie in the database, which means someone with read access to the database can't generate a valid cookie. The down side of this approach is that some fairly subtle cryptography is needed. I think it's possible to do with MD5, but will need some thought.

#22 in reply to: ↑ 18 ; follow-ups: @sjmurdoch
9 years ago

Replying to westi:

I've uploaded a first pass patch for auth cookies

It looks like the cookie is of the form:
md5(DB_PASSWORD.DB_USER.DB_NAME.DB_HOST.ABSPATH.$username.uniqid(microtime())
and the hash of this is stored in the database.

If uniqid() isn't unpredictable, it would be possible to brute force the database password (the rest of the fields are pretty easy to guess in most situations). How secure is uniqid() in this usage?

Is there a better way to get unpredictable pseudorandom numbers?

#23 in reply to: ↑ 21 @westi
9 years ago

Replying to sjmurdoch:

There are roughly two approaches to dealing with this problem.

Sessions involves a record being kept of each login session, which PHP already has existing infrastructure for this. The down side is that there is a database write for each login, and the size of the session table scales with the number of active sessions. Alternatively there can be a restriction of one session per user, as in westi's patch.

An alternative is to use something along the lines of the papers I mentioned earlier. This supports multiple sessions per user, with no need to store extra state. Database writes are only needed on changing the password.

A variant of this is to store the hash post-image of a cookie in the database, which means someone with read access to the database can't generate a valid cookie. The down side of this approach is that some fairly subtle cryptography is needed. I think it's possible to do with MD5, but will need some thought.

I will have a read through those and try to understand how we can implement them.

#24 in reply to: ↑ 22 @westi
9 years ago

Replying to sjmurdoch:

Replying to westi:

I've uploaded a first pass patch for auth cookies

It looks like the cookie is of the form:
md5(DB_PASSWORD.DB_USER.DB_NAME.DB_HOST.ABSPATH.$username.uniqid(microtime())
and the hash of this is stored in the database.

If uniqid() isn't unpredictable, it would be possible to brute force the database password (the rest of the fields are pretty easy to guess in most situations). How secure is uniqid() in this usage?

Is there a better way to get unpredictable pseudorandom numbers?

That is the current cookie format in this patch.

It is in no way intended to be the final format - I just wanted something which would generate a hopefully unique cookie per-user for testing the infrastructure changes with WordPress - splitting out the login handling from the cookie verification

We do need to analyse the predictability of the cookies and ideally find a good available source of entropy for our random numbers

#25 in reply to: ↑ 22 @darkdragon
9 years ago

Replying to sjmurdoch:

If uniqid() isn't unpredictable, it would be possible to brute force the database password (the rest of the fields are pretty easy to guess in most situations). How secure is uniqid() in this usage?

Is there a better way to get unpredictable pseudorandom numbers?

From my reading of the security, it should be uniqid(microtime(), true) for better random-ness and security.

#26 follow-up: @ryan
9 years ago

The recipes using HMAC look good, however hash_hmac() is available only in recent PHP versions. We'll have to fall back to something else when it isn't available.

#27 in reply to: ↑ 26 @sjmurdoch
9 years ago

Replying to ryan:

The recipes using HMAC look good, however hash_hmac() is available only in recent PHP versions. We'll have to fall back to something else when it isn't available.

It is quite straightforward to build HMAC out of a hash function, and even using MD5 is OK for this because HMAC is not vulnerable to collision attacks. The Wikipedia article on HMAC shows how it is constructed. So I wouldn't let this be an impediment.

This is still more complicated than building sessions, and importantly if the attacker gets access to the database and key, he can still log in as any user. So while it fixes the expiry issues it will not completely stop the problem that this ticket is about.

#28 @darkdragon
9 years ago

Well, there is such a thing as Session Hijacking. Any thing that features Sessions will need to reset the Session Key after a jump in permissions. Such as accessing the administration panel and writing posts.

So even if an attacker gains access to the session key by XSS, cookie hijacking, or whatever, it won't matter since the token will be brief enough.

You can take it further and implement something that SMF has, which asks for authentication every 15 minutes to 2 hours. While this could be annoying (and it is really annoying in my humble opinion) it would negate most attacks such as this one and session hijacking.

#29 follow-up: @ryan
9 years ago

http://www.cse.msu.edu/~alexliu/publications/Cookie/cookie.pdf

That defines the following cookie structure.

user name|expiration time|(data)k|HMAC( user name|expiration time|data|session key, k)
where k=HMAC(user name|expiration time, sk)
and where sk is a secret key

To fulfill HMAC, we need the hash_hmac() function. We can fall back to the Crypt_HMAC code from PEAR if that is not available.

(data)k requires two-way encryption. We have that only if the mcrypt module is available. I don't know how widespread that is among hosts. I think that the data field is optional, however. We don't have any additional data we need to put in the cookie. Losing the data field loses the mcrypt requirement.

Since most WP installs don't run over SSL, we have to drop session key. The cookie now looks like this:

user name|expiration time|HMAC( user name|expiration time, k)
where k=HMAC(user name|expiration time, sk)
and where sk is a secret key

That looks pretty good, but leaves us with an attacker being able to create a cookie if he has access to the secret key in the DB. I think we can get around this by having part of the key defined in wp-config.php. We currently save a randomly generated secret in the 'secret' field in the options table. wp_salt() uses this for creating nonces. We can supplement this secret by concatenating it with a SECRET_KEY defined in wp-config.php. We can provide a default SECRET_KEY, but users will have to change this to something unique for it to be effective in preventing an intruder who knows the DB secret from creating a cookie. We don't allow viewing wp-config.php from anywhere in WP, so SECRET_KEY should be safe from intruders who can see the DB or who have gained user permissions. We could drop the secret from the DB and use just SECRET_KEY, but this would rely on users changing SECRET_KEY. We can't rely on that, so augmenting SECRET_KEY with a randomly generated DB secret is necessary. Not everyone will change the SECRET_KEY, but the option is there for those wanting extra protection.

@ryan
9 years ago

Add SECRET_KEY to salt

#30 in reply to: ↑ 29 @sjmurdoch
9 years ago

Replying to ryan:

(data)k requires two-way encryption. We have that only if the mcrypt module is available.

It would be possible to build two-way encryption out of MD5, if needed. It won't be as efficient as AES but should not be an issue in practice.

@ryan
9 years ago

#31 @ryan
9 years ago

First draft patch attached. Still to do is adding a PHP implemenation of hash_hmac() to compat.php. wp-login.php and the pluggable auth functions also need some serious cleanup.

@ryan
9 years ago

#32 @ryan
9 years ago

Patch adds hash_hmac() for compat with PHP versions lacking native hash_hmac(). Completely untested.

#33 follow-up: @ryan
9 years ago

Publishing a post to find that your cookie expired while you were composing will be an annoyance. Any way around that with this cookie protocol?

#34 in reply to: ↑ 33 @sjmurdoch
9 years ago

Replying to ryan:

Publishing a post to find that your cookie expired while you were composing will be an annoyance. Any way around that with this cookie protocol?

How about having two expiry times, a soft one and a hard one. If a user is doing most actions, they will be asked to re-login after the soft expiry. However, if they are composing, they will not be kicked out until the hard expiry. The difference between the soft and hard expiry times is the window where they can safely compose a post.

The two times could be explicitly included in a cookie. Alternatively the hard one could be included and the user asked to renew if they're close.

#35 @ryan
9 years ago

So, check to see if POST is not empty. If it is not empty and the time is within an hour of expiration, allow the cookie. Non-POST requests will honor the expiration with no grace period.

wp-login.php could be improved to redirect back to referrer when forcing a login due to cookie expiration.

@ryan
9 years ago

Allow POST requests a grace period

#36 @ryan
9 years ago

That patch adds a grace period for POST requests to wp_validate_auth_cookie().

@ryan
9 years ago

#37 follow-up: @ryan
9 years ago

Any objections? I'll commit this soon so testers can have a look at it. We can tweak it from there.

#38 in reply to: ↑ 37 ; follow-up: @westi
9 years ago

Replying to ryan:

Any objections? I'll commit this soon so testers can have a look at it. We can tweak it from there.

I am not completely happy that the new cookie scheme doesn't handle the issue whereby someone with read access to the database can generate a valid cookie - in general users are not going to update the SECRET define and so won't benefit from it.

What are the timeouts on the cookies it looks like 2 days or 14 days if I do my maths right - would we not do better with a shorter expiry time and resetting the cookie on every admin page access with a new expiry.

I think it would be nice to combine the two methods - an authentication cookie which authenticates the user has provided valid login credentials and the timeout functionality of this session cookie.

#39 in reply to: ↑ 38 ; follow-up: @ryan
9 years ago

Replying to westi:

Replying to ryan:

Any objections? I'll commit this soon so testers can have a look at it. We can tweak it from there.

I am not completely happy that the new cookie scheme doesn't handle the issue whereby someone with read access to the database can generate a valid cookie - in general users are not going to update the SECRET define and so won't benefit from it.

We could incorporate your session key stuff into the currently unused data field of the cookie. Store a random key in the cookie and store the hash of that key in the DB. But, that brings us back to allowing only one session at at time and requiring a DB write for every successful login attempt.

What are the timeouts on the cookies it looks like 2 days or 14 days if I do my maths right - would we not do better with a shorter expiry time and resetting the cookie on every admin page access with a new expiry.

Wouldn't that allow replaying an old cookie to get a new cookie with a fresh expiry?

#40 in reply to: ↑ 39 ; follow-up: @sjmurdoch
9 years ago

Replying to ryan:

We could incorporate your session key stuff into the currently unused data field of the cookie. Store a random key in the cookie and store the hash of that key in the DB. But, that brings us back to allowing only one session at at time and requiring a DB write for every successful login attempt.

I think it is possible to do better, but it could involve a bit of tricky cryptography. With only MD5 it will take more work, but it is possible to turn any hash function into a block cipher. I'm not sure if I'll have enough time this week to write it up properly though.

What are the timeouts on the cookies it looks like 2 days or 14 days if I do my maths right - would we not do better with a shorter expiry time and resetting the cookie on every admin page access with a new expiry.

Wouldn't that allow replaying an old cookie to get a new cookie with a fresh expiry?

A variant is to only allow auto-extension of valid cookies, so it means that really old ones won't work. That still means that if someone steals a cookie and uses it promptly, it will be possible to keep renewing it. I still think it is better to have a hard limit on cookie expiration, so someone with a cookie and no password will eventually get locked out.

#41 in reply to: ↑ 40 ; follow-up: @ryan
9 years ago

Replying to sjmurdoch:

Replying to ryan:

We could incorporate your session key stuff into the currently unused data field of the cookie. Store a random key in the cookie and store the hash of that key in the DB. But, that brings us back to allowing only one session at at time and requiring a DB write for every successful login attempt.

I think it is possible to do better, but it could involve a bit of tricky cryptography. With only MD5 it will take more work, but it is possible to turn any hash function into a block cipher. I'm not sure if I'll have enough time this week to write it up properly though.

Even with a block cipher we still have to worry about someone getting the key if it is stored in the DB, yes?

If a block cipher would indeed help prevent cookies being created from DB information, we could always make it an optional bit of extra security we add if mcrypt is available. If mcrypt is not available, the cookie doesn't get that extra bit of security. If we do want to provide a fall back for when mcrypt is not available, I found this. Don't know if it's any good.

http://www.jonasjohn.de/snippets/php/md5-based-block-cipher.htm

What are the timeouts on the cookies it looks like 2 days or 14 days if I do my maths right - would we not do better with a shorter expiry time and resetting the cookie on every admin page access with a new expiry.

Wouldn't that allow replaying an old cookie to get a new cookie with a fresh expiry?

A variant is to only allow auto-extension of valid cookies, so it means that really old ones won't work. That still means that if someone steals a cookie and uses it promptly, it will be possible to keep renewing it. I still think it is better to have a hard limit on cookie expiration, so someone with a cookie and no password will eventually get locked out.

I'd rather have a hard limit too. A reasonable timeout coupled with a grace period to allow POSTs to complete seems user friendly enough.

#42 in reply to: ↑ 41 ; follow-up: @sjmurdoch
9 years ago

Replying to ryan:

Even with a block cipher we still have to worry about someone getting the key if it is stored in the DB, yes?

The rough idea I was thinking of is storing the encrypted hash of the password in the cookie, and the double hash in the database. Then if an attacker can read the key and double-hash, they can still not generate a valid cookie.

Don't know if it's any good.

http://www.jonasjohn.de/snippets/php/md5-based-block-cipher.htm

It looks a bit weird (it's not CFB, like it says and it's not quite OFB either). Maybe it works though, but it needs more thought.

#43 in reply to: ↑ 42 @ryan
9 years ago

Replying to sjmurdoch:

Replying to ryan:

Even with a block cipher we still have to worry about someone getting the key if it is stored in the DB, yes?

The rough idea I was thinking of is storing the encrypted hash of the password in the cookie, and the double hash in the database. Then if an attacker can read the key and double-hash, they can still not generate a valid cookie.

Double hashing the password stored in the DB makes it hard to interop with things like mod_auth_mysql, yes? I'm not sure if portable phpass hashes are supported as is. wp_hash_password() and wp_check_password() could be replaced with something that mod_auth_mysql can handle, but requiring a double hash in the DB could foil that.

#44 follow-up: @ryan
9 years ago

If SECRET_KEY is not set to something unique, use the DB connect info in the secret key? Good enough? Does that balance paranoia and practicality well enough?

#45 in reply to: ↑ 44 ; follow-up: @sjmurdoch
9 years ago

Replying to ryan:

If SECRET_KEY is not set to something unique, use the DB connect info in the secret key? Good enough? Does that balance paranoia and practicality well enough?

That sounds like a bad idea to me. The database password is too short to resist an offline brute force/dictionary attack. If we use it in the cookie, anyone could discover the password with a bit of work.

#46 in reply to: ↑ 45 @DD32
9 years ago

Replying to sjmurdoch:

That sounds like a bad idea to me. The database password is too short to resist an offline brute force/dictionary attack. If we use it in the cookie, anyone could discover the password with a bit of work.

It'd more than just the password, And its allready in use, If you use multiple pieces and do an md5 or so of them, then its unlikely an attacker will back-engineer it.

wp-includes/cache.php:
$this->secret = DB_PASSWORD . DB_USER .
DB_NAME . DB_HOST . ABSPATH;

#47 follow-up: @ryan
9 years ago

To clarify, I meant using the DB connect information as part of sk, the secret key used when creating k. DB connect info plus the salt from the DB would be used in sk, not just the DB connect info alone. Someone without read access to the DB would have to contend with the random salt from the DB in addition to the DB connect info. Someone with read access to the DB would have the secret key from the DB, leaving just the DB connect info. So an attacker would have to get DB read info to reduce the sk down to just the DB connect info and then break the sk. Since the sk is used in the cookie only as a salt for hash_hmac(), how easy is it to brute force?

@ryan
9 years ago

#48 @ryan
9 years ago

Refreshed patch.

#49 @ryan
9 years ago

(In [6387]) New secure cookie protocol. see #5367

#50 @ryan
9 years ago

I committed what I had so we could get some testing of the overall mechanism and make sure our hash_hmac compat function works. Leaving the ticket open since discussion is ongoing.

#51 @ryan
9 years ago

(In [6389]) Don't show both log out and session expired messages when logging out. see #5367

#52 follow-up: @sambauers
9 years ago

These changes break bbPress integration. It would be easier from our end if you forced people to have a SECRET_KEY, then we could just ask for a matching secret key in their bbPress config and the cookie hashes would match (after these functions are implemented in bbPress).

So what do you think about making SECRET_KEY a mandatory config setting?

#53 in reply to: ↑ 52 @DD32
9 years ago

Replying to sambauers:

So what do you think about making SECRET_KEY a mandatory config setting?

Bad idea IMO, Mainly for upgrading purposes, However, I would suggest that it be included in the sample-config.php so that new installations set the secret straight up.

I think auth plugins should instruct users to set the secret key instead, As not many users actually use the integration plugins.

#54 @ryan
9 years ago

(In [6400]) Fix AJAX cookie validation. see #5367

#55 @sambauers
9 years ago

As a trade-off, is it possible to create a place in the WP admin area where the secret key (either manual or auto-generated) could be displayed so that I can direct people to it when they need to match it in their bbPress installs? That's a little easier than getting them to edit their WP config file for integration with a "third-party".

#56 @matt
9 years ago

For pure amusement purposes, here's the changeset from 4 years ago that originally switched our passwords to be MD5:

http://trac.wordpress.org/changeset/850

(Before that they were plain text.)

#57 @ryan
9 years ago

Exposing the secret key in the admin would compromise part of its purpose. And the key cannot be auto-generated because that would require putting it in the DB, also compromising its purpose. We already have an auto-generated secret key in the DB.

#58 @sambauers
9 years ago

Sorry, by "auto-generated" I meant the secret that is created by concatenating the other config settings when there is no secret specified in the config file.

If there is no chance of displaying the key in the admin area then I guess I am back to a fairly lengthy instruction for integrators to follow. That's OK, I suppose it's a small price for this level of security.

#59 @sambauers
9 years ago

So if I understand correctly, for bbPress (or any other program that wants to integrate) to be able to read the cookies which are now produced it needs to know the SECRET_KEY set in wp-config.php *and* the "secret" in the options table as well.

Just because I have to ask... :)

How critical is it that we have the "secret" option as well as the SECRET_KEY? I would have thought the stronger phpass hashing would make that second secret unnecessary?

I don't mean to harp on this, it's just that a lot of support issues for bbPress centre around integration and needing to retrieve the "secret" option from the raw database makes it even more onerous to implement cookie sharing.

#60 in reply to: ↑ 47 @sjmurdoch
9 years ago

Replying to ryan:

Since the sk is used in the cookie only as a salt for hash_hmac(), how easy is it to brute force?

It is exactly as easy as the entropy of the password. The vulnerability is that by including it into the hash you're turning an online attack against the database into an easily parallelizable offline attack. If the database password is large, e.g. 128 bits of random data, then the attack won't work, it is small, it will.

Users might not realize this additional requirement that is now being placed on the database password. It would be reasonable to expect that a 8 character password would be secure (41 bits of entropy) and against an online attack it probably is. Users who make this assumption, not realizing where else it is used, are at risk of an offline attack, for which 41 bits is trivial.

#61 @ryan
9 years ago

sjmurdoch, we can remove DB info from the secret key then. People will just have change their SECRET_KEY to get extra protection.

sambauers, we could have a SECRET_SALT define that overrides the secret in the DB to aid integrators. We create a random "secret" in the DB to make up for the fact that most people won't change SECRET_KEY. Integrators will have to change keys, so we can do without the secret in the DB. phpass, BTW, plays no role in the cookie protocol. phpass and password hashing are a separate consideration.

@ryan
9 years ago

#62 follow-up: @ryan
9 years ago

New patch uses SECRET_KEY instead of the DB secret if defined. Should we do this, or concat SECRET_KEY with the db secret and provide a separate SECRET_SALT define that can be used to replace the db secret?

#63 @ryan
9 years ago

(In [6471]) Don't fallback to DB info for secret key. Allow expiration grace period for AJAX requests. see #5367

#64 @Viper007Bond
9 years ago

The comment for SECRET_KEY in [6471] can easily be confusing for someone who doesn't know PHP. I'm betting many people would replace the text "SECRET_KEY" with their key rather correctly defining the constant.

The comment should be changed, or even better, perhaps using some example text like "uniquephrasehere" and then have WP ignore/unset the key constant if it's left at that.

#65 @ryan
9 years ago

Agreed. Better comment plus 'unique phrase here' that wp_salt() knows to ignore.

#66 in reply to: ↑ 62 @ryan
9 years ago

Replying to ryan:

New patch uses SECRET_KEY instead of the DB secret if defined. Should we do this, or concat SECRET_KEY with the db secret and provide a separate SECRET_SALT define that can be used to replace the db secret?

I'm think I'm leaning toward the second option. I'm not comfortable betting the farm on SECRET_KEY staying secret. We've gone from any who can get read access to the DB being able to generate a cookie to any who can read wp-config.php being able to generate a cookie. HTTP server exploits and directory traversals can expose wp-config.php. Having part of the key live in the DB provides a belt to go with our suspenders. Granted, someone who can get at wp-config.php will also get DB login information, but DB servers shouldn't be accepting connections from anywhere.

So, for the convenience of integrators, SECRET_SALT would be provided to override the DB salt. SECRET_SALT could be defined as a constant somewhere or populated with a value from a table shared by the applications being integrated.

#67 @ryan
9 years ago

(In [6478]) Allow DB salt to be overridden by SECRET_SALT. Add a filter to wp_salt(). see #5367

#68 @ryan
9 years ago

(In [6486]) set_auth_cookie action. see #5367

#69 follow-up: @ryan
9 years ago

(In [6529]) Separate cookie generation from cookie set. Introduce wp_generate_auth_cookie(). see #5367

#70 in reply to: ↑ 69 @DD32
9 years ago

Replying to ryan:

(In [6529]) Separate cookie generation from cookie set. Introduce wp_generate_auth_cookie(). see #5367

has rendered the folowing line useless:

   $user = get_userdata($user_id);

http://trac.wordpress.org/browser/trunk/wp-includes/pluggable.php?rev=6529#L375

see patch.

@DD32
9 years ago

#71 @ryan
9 years ago

(In [6531]) Remove unneeded get_userdata call. Props DD32. see #5367

#72 follow-up: @sambauers
9 years ago

Am I missing something or is the SECRET_KEY now not doing anything at all?

wp_salt() defines $secret_key from SECRET_KEY on lines 713 - 715 of pluggable.php, but then doesn't concatenate it with $salt

Also, should some value be auto-generated for $secret_key if there is no SECRET_KEY defined or do we just rely on the DB based secret in that case?

#73 @ryan
9 years ago

(In [6583]) Concat secret key with salt. see #5367

#74 in reply to: ↑ 72 @ryan
9 years ago

Replying to sambauers:

Am I missing something or is the SECRET_KEY now not doing anything at all?

wp_salt() defines $secret_key from SECRET_KEY on lines 713 - 715 of pluggable.php, but then doesn't concatenate it with $salt

Fixed.

Also, should some value be auto-generated for $secret_key if there is no SECRET_KEY defined or do we just rely on the DB based secret in that case?

Anything auto-generated would need to be DB based since we can't assume file write privs. We don't need two values stored in the DB, so if there is no secret key just using the salt is fine.

#75 @ryan
8 years ago

  • Milestone changed from 2.6 to 2.5
  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.