SessionManager: Add provision for encrypting session data
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 26 Apr 2016 17:56:35 +0000 (13:56 -0400)
committerGergő Tisza <gtisza@wikimedia.org>
Fri, 29 Apr 2016 15:34:55 +0000 (15:34 +0000)
This follows the model Chris Steipp implemented for OATHAuth.

At the moment, this avoids the need to require a crypto PHP extension by
adding a configuration variable to enable plaintext storage. Someday
when there's time for the necessary code review, we should probably
import a pure-PHP implementation of AES to fall back to when the crypto
extensions are unavailable.

Change-Id: Ie9cae1526d3b8bf3f517f3226ddd888893f65656

RELEASE-NOTES-1.27
includes/DefaultSettings.php
includes/session/Session.php
tests/phpunit/includes/session/SessionTest.php

index 9b77cd1..3a0326e 100644 (file)
@@ -13,6 +13,8 @@ HHVM 3.1. Additionally, the following PHP extensions are required:
 * json
 * mbstring
 * xml
+The following PHP extensions are strongly recommended:
+* openssl
 
 === Configuration changes in 1.27 ===
 * $wgAllowMicrodataAttributes and $wgAllowRdfaAttributes were removed,
@@ -114,6 +116,9 @@ HHVM 3.1. Additionally, the following PHP extensions are required:
   module should express a dependency on it.
 * Removed configuration option $wgCopyrightIcon (deprecated since 1.18). Use
   $wgFooterIcons['copyright']['copyright'] instead.
+* If the openssl and mcrypt PHP extensions are both unavailable, secure
+  session storage (soon to be used for login) will raise an exception. This
+  exception may be bypassed by setting $wgSessionInsecureSecrets = true.
 
 === New features in 1.27 ===
 * $wgDataCenterUpdateStickTTL was also added. This decides how long a user
index 5b3684b..ab9a28d 100644 (file)
@@ -7990,6 +7990,23 @@ $wgPagePropsHaveSortkey = true;
  */
 $wgHttpsPort = 443;
 
+/**
+ * Secret for session storage.
+ * This should be set in LocalSettings.php, otherwise wgSecretKey will
+ * be used.
+ * @since 1.27
+ */
+$wgSessionSecret = false;
+
+/**
+ * If for some reason you can't install the PHP OpenSSL or mcrypt extensions,
+ * you can set this to true to make MediaWiki work again at the cost of storing
+ * sensitive session data insecurely. But it would be much more secure to just
+ * install the OpenSSL extension.
+ * @since 1.27
+ */
+$wgSessionInsecureSecrets = false;
+
 /**
  * Secret for hmac-based key derivation function (fast,
  * cryptographically secure random numbers).
index 4188f4f..29878d4 100644 (file)
@@ -379,6 +379,156 @@ final class Session implements \Countable, \Iterator, \ArrayAccess {
                $this->remove( 'wsTokenSecrets' );
        }
 
+       /**
+        * Fetch the secret keys for self::setSecret() and self::getSecret().
+        * @return string[] Encryption key, HMAC key
+        */
+       private function getSecretKeys() {
+               global $wgSessionSecret, $wgSecretKey;
+
+               $wikiSecret = $wgSessionSecret ?: $wgSecretKey;
+               $userSecret = $this->get( 'wsSessionSecret', null );
+               if ( $userSecret === null ) {
+                       $userSecret = \MWCryptRand::generateHex( 32 );
+                       $this->set( 'wsSessionSecret', $userSecret );
+               }
+
+               $keymats = hash_pbkdf2( 'sha256', $wikiSecret, $userSecret, 10001, 64, true );
+               return [
+                       substr( $keymats, 0, 32 ),
+                       substr( $keymats, 32, 32 ),
+               ];
+       }
+
+       /**
+        * Set a value in the session, encrypted
+        *
+        * This relies on the secrecy of $wgSecretKey (by default), or $wgSessionSecret.
+        *
+        * @param string|int $key
+        * @param mixed $value
+        */
+       public function setSecret( $key, $value ) {
+               global $wgSessionInsecureSecrets;
+
+               list( $encKey, $hmacKey ) = $this->getSecretKeys();
+               $serialized = serialize( $value );
+
+               // The code for encryption (with OpenSSL) and sealing is taken from
+               // Chris Steipp's OATHAuthUtils class in Extension::OATHAuth.
+
+               // Encrypt
+               // @todo: import a pure-PHP library for AES instead of doing $wgSessionInsecureSecrets
+               $iv = \MWCryptRand::generate( 16, true );
+               if ( function_exists( 'openssl_encrypt' ) ) {
+                       $ciphertext = openssl_encrypt( $serialized, 'aes-256-ctr', $encKey, OPENSSL_RAW_DATA, $iv );
+                       if ( $ciphertext === false ) {
+                               throw new UnexpectedValueException( 'Encryption failed: ' . openssl_error_string() );
+                       }
+               } elseif ( function_exists( 'mcrypt_encrypt' ) ) {
+                       $ciphertext = mcrypt_encrypt( 'rijndael-128', $encKey, $serialized, 'ctr', $iv );
+                       if ( $ciphertext === false ) {
+                               throw new UnexpectedValueException( 'Encryption failed' );
+                       }
+               } elseif ( $wgSessionInsecureSecrets ) {
+                       $ex = new \Exception( 'No encryption is available, storing data as plain text' );
+                       $this->logger->warning( $ex->getMessage(), [ 'exception' => $ex ] );
+                       $ciphertext = $serialized;
+               } else {
+                       throw new \BadMethodCallException(
+                               'Encryption is not available. You really should install the PHP OpenSSL extension, ' .
+                               'or failing that the mcrypt extension. But if you really can\'t and you\'re willing ' .
+                               'to accept insecure storage of sensitive session data, set ' .
+                               '$wgSessionInsecureSecrets = true in LocalSettings.php to make this exception go away.'
+                       );
+               }
+
+               // Seal
+               $sealed = base64_encode( $iv ) . '.' . base64_encode( $ciphertext );
+               $hmac = hash_hmac( 'sha256', $sealed, $hmacKey, true );
+               $encrypted = base64_encode( $hmac ) . '.' . $sealed;
+
+               // Store
+               $this->set( $key, $encrypted );
+       }
+
+       /**
+        * Fetch a value from the session that was set with self::setSecret()
+        * @param string|int $key
+        * @param mixed $default Returned if $this->exists( $key ) would be false or decryption fails
+        * @return mixed
+        */
+       public function getSecret( $key, $default = null ) {
+               global $wgSessionInsecureSecrets;
+
+               // Fetch
+               $encrypted = $this->get( $key, null );
+               if ( $encrypted === null ) {
+                       return $default;
+               }
+
+               // The code for unsealing, checking, and decrypting (with OpenSSL) is
+               // taken from Chris Steipp's OATHAuthUtils class in
+               // Extension::OATHAuth.
+
+               // Unseal and check
+               $pieces = explode( '.', $encrypted );
+               if ( count( $pieces ) !== 3 ) {
+                       $ex = new \Exception( 'Invalid sealed-secret format' );
+                       $this->logger->warning( $ex->getMessage(), [ 'exception' => $ex ] );
+                       return $default;
+               }
+               list( $hmac, $iv, $ciphertext ) = $pieces;
+               list( $encKey, $hmacKey ) = $this->getSecretKeys();
+               $integCalc = hash_hmac( 'sha256', $iv . '.' . $ciphertext, $hmacKey, true );
+               if ( !hash_equals( $integCalc, base64_decode( $hmac ) ) ) {
+                       $ex = new \Exception( 'Sealed secret has been tampered with, aborting.' );
+                       $this->logger->warning( $ex->getMessage(), [ 'exception' => $ex ] );
+                       return $default;
+               }
+
+               // Decrypt
+               // @todo: import a pure-PHP library for AES instead of doing $wgSessionInsecureSecrets
+               if ( function_exists( 'openssl_decrypt' ) ) {
+                       $serialized = openssl_decrypt(
+                               base64_decode( $ciphertext ), 'aes-256-ctr', $encKey, OPENSSL_RAW_DATA, base64_decode( $iv )
+                       );
+                       if ( $serialized === false ) {
+                               $ex = new \Exception( 'Decyption failed: ' . openssl_error_string() );
+                               $this->logger->debug( $ex->getMessage(), [ 'exception' => $ex ] );
+                               return $default;
+                       }
+               } elseif ( function_exists( 'mcrypt_decrypt' ) ) {
+                       $serialized = mcrypt_decrypt(
+                               'rijndael-128', $encKey, base64_decode( $ciphertext ), 'ctr', base64_decode( $iv )
+                       );
+                       if ( $serialized === false ) {
+                               $ex = new \Exception( 'Decyption failed' );
+                               $this->logger->debug( $ex->getMessage(), [ 'exception' => $ex ] );
+                               return $default;
+                       }
+               } elseif ( $wgSessionInsecureSecrets ) {
+                       $ex = new \Exception(
+                               'No encryption is available, retrieving data that was stored as plain text'
+                       );
+                       $this->logger->warning( $ex->getMessage(), [ 'exception' => $ex ] );
+                       $serialized = base64_decode( $ciphertext );
+               } else {
+                       throw new \BadMethodCallException(
+                               'Encryption is not available. You really should install the PHP OpenSSL extension, ' .
+                               'or failing that the mcrypt extension. But if you really can\'t and you\'re willing ' .
+                               'to accept insecure storage of sensitive session data, set ' .
+                               '$wgSessionInsecureSecrets = true in LocalSettings.php to make this exception go away.'
+                       );
+               }
+
+               $value = unserialize( $serialized );
+               if ( $value === false && $serialized !== serialize( false ) ) {
+                       $value = $default;
+               }
+               return $value;
+       }
+
        /**
         * Delay automatic saving while multiple updates are being made
         *
index e87f41d..3815416 100644 (file)
@@ -267,21 +267,9 @@ class SessionTest extends MediaWikiTestCase {
        }
 
        public function testTokens() {
-               $rc = new \ReflectionClass( Session::class );
-               if ( !method_exists( $rc, 'newInstanceWithoutConstructor' ) ) {
-                       $this->markTestSkipped(
-                               'ReflectionClass::newInstanceWithoutConstructor isn\'t available'
-                       );
-               }
-
-               // Instead of actually constructing the Session, we use reflection to
-               // bypass the constructor and plug a mock SessionBackend into the
-               // private fields to avoid having to actually create a SessionBackend.
-               $backend = new DummySessionBackend;
-               $session = $rc->newInstanceWithoutConstructor();
+               $session = TestUtils::getDummySession();
                $priv = \TestingAccessWrapper::newFromObject( $session );
-               $priv->backend = $backend;
-               $priv->index = 42;
+               $backend = $priv->backend;
 
                $token = \TestingAccessWrapper::newFromObject( $session->getToken() );
                $this->assertArrayHasKey( 'wsTokenSecrets', $backend->data );
@@ -313,4 +301,72 @@ class SessionTest extends MediaWikiTestCase {
                $this->assertArrayNotHasKey( 'wsTokenSecrets', $backend->data );
 
        }
+
+       /**
+        * @dataProvider provideSecretsRoundTripping
+        * @param mixed $data
+        */
+       public function testSecretsRoundTripping( $data ) {
+               $session = TestUtils::getDummySession();
+
+               // Simple round-trip
+               $session->setSecret( 'secret', $data );
+               $this->assertNotEquals( $data, $session->get( 'secret' ) );
+               $this->assertEquals( $data, $session->getSecret( 'secret', 'defaulted' ) );
+       }
+
+       public static function provideSecretsRoundTripping() {
+               return [
+                       [ 'Foobar' ],
+                       [ 42 ],
+                       [ [ 'foo', 'bar' => 'baz', 'subarray' => [ 1, 2, 3 ] ] ],
+                       [ (object)[ 'foo', 'bar' => 'baz', 'subarray' => [ 1, 2, 3 ] ] ],
+                       [ true ],
+                       [ false ],
+                       [ null ],
+               ];
+       }
+
+       public function testSecrets() {
+               $logger = new \TestLogger;
+               $session = TestUtils::getDummySession( null, -1, $logger );
+
+               // Simple defaulting
+               $this->assertEquals( 'defaulted', $session->getSecret( 'test', 'defaulted' ) );
+
+               // Bad encrypted data
+               $session->set( 'test', 'foobar' );
+               $logger->setCollect( true );
+               $this->assertEquals( 'defaulted', $session->getSecret( 'test', 'defaulted' ) );
+               $logger->setCollect( false );
+               $this->assertSame( [
+                       [ LogLevel::WARNING, 'Invalid sealed-secret format' ]
+               ], $logger->getBuffer() );
+               $logger->clearBuffer();
+
+               // Tampered data
+               $session->setSecret( 'test', 'foobar' );
+               $encrypted = $session->get( 'test' );
+               $session->set( 'test', $encrypted . 'x' );
+               $logger->setCollect( true );
+               $this->assertEquals( 'defaulted', $session->getSecret( 'test', 'defaulted' ) );
+               $logger->setCollect( false );
+               $this->assertSame( [
+                       [ LogLevel::WARNING, 'Sealed secret has been tampered with, aborting.' ]
+               ], $logger->getBuffer() );
+               $logger->clearBuffer();
+
+               // Unserializable data
+               $iv = \MWCryptRand::generate( 16, true );
+               list( $encKey, $hmacKey ) = \TestingAccessWrapper::newFromObject( $session )->getSecretKeys();
+               $ciphertext = openssl_encrypt( 'foobar', 'aes-256-ctr', $encKey, OPENSSL_RAW_DATA, $iv );
+               $sealed = base64_encode( $iv ) . '.' . base64_encode( $ciphertext );
+               $hmac = hash_hmac( 'sha256', $sealed, $hmacKey, true );
+               $encrypted = base64_encode( $hmac ) . '.' . $sealed;
+               $session->set( 'test', $encrypted );
+               \MediaWiki\suppressWarnings();
+               $this->assertEquals( 'defaulted', $session->getSecret( 'test', 'defaulted' ) );
+               \MediaWiki\restoreWarnings();
+       }
+
 }