Allow for time-limited tokens

Tokens returned from User::getEditToken will now include an encoded
timestamp (and therefore will be up to 16 bytes longer).

User::matchEditToken and User::matchEditTokenNoSuffix are gaining a
"maxage" parameter: when specified, only tokens with an encoded timestamp
within $maxage seconds will be accepted.

Change-Id: Ic4cf148cc1fa9d0561aac0bd2d68a09c7851896d

RELEASE-NOTES-1.25

@@ -12,6 +12,10 @@ production.
 * $wgPageShowWatchingUsers was removed.
 * $wgLocalVirtualHosts has been added to replace $wgConf->localVHosts.
 * $wgAntiLockFlags was removed.
+* Edit tokens returned from User::getEditToken may change on every call. Token
+  validity must be checked by passing the user-supplied token to
+  User::matchEditToken rather than by testing for equality with a
+  newly-generated token.
 
 === New features in 1.25 ===
 * (bug 58139) ResourceLoaderFileModule now supports language fallback
@@ -33,6 +37,8 @@ production.
   - Usage instructions: https://www.mediawiki.org/wiki/Help:Page_status_indicators
   - Adjusting custom skins to support indicators:
     https://www.mediawiki.org/wiki/Manual:Skinning#Page_status_indicators
+* Edit tokens may now be time-limited: passing a maximum age to
+  User::matchEditToken will reject any older tokens.
 
 === Bug fixes in 1.25 ===
 * (bug 71003) No additional code will be generated to try to load CSS-embedded

includes/User.php

@@ -3876,6 +3876,33 @@ class User implements IDBAccessObject {
 		return $this->getEditToken( $salt, $request );
 	}
 
+	/**
+	 * Internal implementation for self::getEditToken() and
+	 * self::matchEditToken().
+	 *
+	 * @param string|array $salt
+	 * @param WebRequest $request
+	 * @param string|int $timestamp
+	 * @return string
+	 */
+	private function getEditTokenAtTimestamp( $salt, $request, $timestamp ) {
+		if ( $this->isAnon() ) {
+			return self::EDIT_TOKEN_SUFFIX;
+		} else {
+			$token = $request->getSessionData( 'wsEditToken' );
+			if ( $token === null ) {
+				$token = MWCryptRand::generateHex( 32 );
+				$request->setSessionData( 'wsEditToken', $token );
+			}
+			if ( is_array( $salt ) ) {
+				$salt = implode( '|', $salt );
+			}
+			return hash_hmac( 'md5', $timestamp . $salt, $token, false ) .
+				dechex( $timestamp ) .
+				self::EDIT_TOKEN_SUFFIX;
+		}
+	}
+
 	/**
 	 * Initialize (if necessary) and return a session token value
 	 * which can be used in edit forms to show that the user's
@@ -3889,23 +3916,9 @@ class User implements IDBAccessObject {
 	 * @return string The new edit token
 	 */
 	public function getEditToken( $salt = '', $request = null ) {
-		if ( $request == null ) {
-			$request = $this->getRequest();
-		}
-
-		if ( $this->isAnon() ) {
-			return self::EDIT_TOKEN_SUFFIX;
-		} else {
-			$token = $request->getSessionData( 'wsEditToken' );
-			if ( $token === null ) {
-				$token = MWCryptRand::generateHex( 32 );
-				$request->setSessionData( 'wsEditToken', $token );
-			}
-			if ( is_array( $salt ) ) {
-				$salt = implode( '|', $salt );
-			}
-			return md5( $token . $salt ) . self::EDIT_TOKEN_SUFFIX;
-		}
+		return $this->getEditTokenAtTimestamp(
+			$salt, $request ?: $this->getRequest(), wfTimestamp()
+		);
 	}
 
 	/**
@@ -3928,15 +3941,34 @@ class User implements IDBAccessObject {
 	 * @param string $val Input value to compare
 	 * @param string $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request Object to use or null to use $wgRequest
+	 * @param int $maxage Fail tokens older than this, in seconds
 	 * @return bool Whether the token matches
 	 */
-	public function matchEditToken( $val, $salt = '', $request = null ) {
-		$sessionToken = $this->getEditToken( $salt, $request );
+	public function matchEditToken( $val, $salt = '', $request = null, $maxage = null ) {
+		if ( $this->isAnon() ) {
+			return $val === self::EDIT_TOKEN_SUFFIX;
+		}
+
+		$suffixLen = strlen( self::EDIT_TOKEN_SUFFIX );
+		if ( strlen( $val ) <= 32 + $suffixLen ) {
+			return false;
+		}
+
+		$timestamp = hexdec( substr( $val, 32, -$suffixLen ) );
+		if ( $maxage !== null && $timestamp < wfTimestamp() - $maxage ) {
+			// Expired token
+			return false;
+		}
+
+		$sessionToken = $this->getEditTokenAtTimestamp(
+			$salt, $request ?: $this->getRequest(), $timestamp
+		);
+
 		if ( $val != $sessionToken ) {
 			wfDebug( "User::matchEditToken: broken session data\n" );
 		}
 
-		return $val == $sessionToken;
+		return hash_equals( $sessionToken, $val );
 	}
 
 	/**
@@ -3946,11 +3978,12 @@ class User implements IDBAccessObject {
 	 * @param string $val Input value to compare
 	 * @param string $salt Optional function-specific data for hashing
 	 * @param WebRequest|null $request Object to use or null to use $wgRequest
+	 * @param int $maxage Fail tokens older than this, in seconds
 	 * @return bool Whether the token matches
 	 */
-	public function matchEditTokenNoSuffix( $val, $salt = '', $request = null ) {
-		$sessionToken = $this->getEditToken( $salt, $request );
-		return substr( $sessionToken, 0, 32 ) == substr( $val, 0, 32 );
+	public function matchEditTokenNoSuffix( $val, $salt = '', $request = null, $maxage = null ) {
+		$val = substr( $val, 0, strspn( $val, '0123456789abcdef' ) ) . self::EDIT_TOKEN_SUFFIX;
+		return $this->matchEditToken( $val, $salt, $request, $maxage );
 	}
 
 	/**

tests/phpunit/includes/api/ApiTestCase.php

@@ -134,10 +134,14 @@ abstract class ApiTestCase extends MediaWikiLangTestCase {
 		}
 
 		if ( isset( $session['wsToken'] ) && $session['wsToken'] ) {
+			// @todo Why does this directly mess with the session? Fix that.
 			// add edit token to fake session
 			$session['wsEditToken'] = $session['wsToken'];
 			// add token to request parameters
-			$params['token'] = md5( $session['wsToken'] ) . User::EDIT_TOKEN_SUFFIX;
+			$timestamp = wfTimestamp();
+			$params['token'] = hash_hmac( 'md5', $timestamp, $session['wsToken'] ) .
+				dechex( $timestamp ) .
+				User::EDIT_TOKEN_SUFFIX;
 
 			return $this->doApiRequest( $params, $session, false, $user );
 		} else {