From 0b8b539a00226fb381a5c760bfc377a43fed558f Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 26 Apr 2016 13:56:35 -0400 Subject: [PATCH] SessionManager: Add provision for encrypting session data 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 | 5 + includes/DefaultSettings.php | 17 ++ includes/session/Session.php | 150 ++++++++++++++++++ .../phpunit/includes/session/SessionTest.php | 84 ++++++++-- 4 files changed, 242 insertions(+), 14 deletions(-) diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 9b77cd1bf4..3a0326e2ff 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -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 diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 5b3684b336..ab9a28db21 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -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). diff --git a/includes/session/Session.php b/includes/session/Session.php index 4188f4f308..29878d49f6 100644 --- a/includes/session/Session.php +++ b/includes/session/Session.php @@ -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 * diff --git a/tests/phpunit/includes/session/SessionTest.php b/tests/phpunit/includes/session/SessionTest.php index e87f41d977..3815416c31 100644 --- a/tests/phpunit/includes/session/SessionTest.php +++ b/tests/phpunit/includes/session/SessionTest.php @@ -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(); + } + } -- 2.20.1