WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 4 years ago

#20276 closed task (blessed)

Tie nonces and cookies to expirable sessions — at Version 6

Reported by: ryan Owned by:
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Security Keywords:
Focuses: Cc:

Description (last modified by duck_)

Authentication cookies are re-usable even after a user decides to explicitly logout. Cookies should be tied to an expirable session that can also be deleted upon logout.

Also, nonce security can be improved by associating them with the same session information. Owasp specifies that "the synchronizer token pattern requires the generating of random challenge tokens that are associated with the user's current session." Our nonces have a timeout, but that timeout can span cookie sessions. Instead, nonces should be somehow tied to the current auth cookie and invalidate whenever the cookie invalidates.

https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet

Change History (7)

#1 @nacin
9 years ago

  • Type changed from defect (bug) to enhancement

I imagine we can take a piece of the auth cookie and include it in the hash. We'll need to include an identifier at a consistent location in the nonce in order to make note of which cookie was used, as we are going to want to leverage the SSL cookie if possible, other times we may need to use the logged_in cookie (say, the logout nonce).

If we generate a nonce in the backend with an admin cookie, but try to use the nonce on the frontend, the nonce will fail. So perhaps we need to stick to logged_in cookie for now.

#2 @scribu
9 years ago

  • Cc scribu added

#3 @johnbillion
9 years ago

  • Cc johnbillion added

#4 @juliobox
8 years ago

  • Cc juliobosk@… added

@duck_
7 years ago

#5 @duck_
7 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 3.7
  • Summary changed from Tie nonces to the current session to Tie nonces and cookies to expirable sessions

20276.diff is a first pass at implementing expirable sessions. This patch aims to make auth cookies unforgeable with read-only access to filesystem and/or database, and invalidate auth cookies upon explicit logout.

On login a long random string, r, is generated. r is included in the user's cookie and H(r) is stored in the database. On future requests, r is extracted from the cookie and H(r) is compared to the value in the database. Storing the hash of r means that read-only SQL injection does not allow an attacker to create cookies since they cannot reverse the value to find a valid r. Each "session token" is also associated with an expiry time, so they can only be used for a limited time.

This information is stored in the database as a piece of user meta named "session_tokens" that is an array mapping the random strings to expiry time. Expired tokens are cleared upon login and logout. The current token, as found in the cookie, is also removed from the array on logout.

Questions to answer:

  • Should there be a limit on the number of session tokens?
  • How should the new $token parameter be added to wp_generate_auth_cookie()? Currently it's just added in the middle. This is nicer, but not backwards compatible.
  • Could documentation about these "sessions" be confused with native PHP sessions?

Todo:

  • Use the session token as part of nonce generation.
  • Add an action if wp_verify_session_token() fails (?)
  • Better method for expiry. _session_not_expired() calls time() for every array element.

#6 @duck_
7 years ago

  • Description modified (diff)

How should the new $token parameter be added to wp_generate_auth_cookie()? Currently it's just added in the middle. This is nicer, but not backwards compatible.

The best solution to this is probably going to be to have $token added as an optional fourth parameter. If it is not present then a session token will be generated automatically.

Note: See TracTickets for help on using tickets.