From f73a68e15cab42ad2e2772b0eea51d75b44ba321 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Tue, 4 Oct 2016 10:48:02 -0700 Subject: [PATCH] Move most of MWCryptHKDF into libs Dependency-inject the MediaWiki-specific parts into a CryptHKDF instance, which MWCryptHKDF wraps around. Change-Id: Idff18635cfd8a3d93ea2ca8d56cdbd11eb4d3b2b --- autoload.php | 1 + includes/MediaWikiServices.php | 9 + includes/ServiceWiring.php | 23 ++ includes/libs/CryptHKDF.php | 282 ++++++++++++++++++ includes/utils/MWCryptHKDF.php | 243 +-------------- .../includes/MediaWikiServicesTest.php | 1 + 6 files changed, 322 insertions(+), 237 deletions(-) create mode 100644 includes/libs/CryptHKDF.php diff --git a/autoload.php b/autoload.php index 936b26183d..1beb00c5f2 100644 --- a/autoload.php +++ b/autoload.php @@ -296,6 +296,7 @@ $wgAutoloadLocalClasses = [ 'CreateAndPromote' => __DIR__ . '/maintenance/createAndPromote.php', 'CreateFileOp' => __DIR__ . '/includes/libs/filebackend/fileop/CreateFileOp.php', 'CreditsAction' => __DIR__ . '/includes/actions/CreditsAction.php', + 'CryptHKDF' => __DIR__ . '/includes/libs/CryptHKDF.php', 'CryptRand' => __DIR__ . '/includes/libs/CryptRand.php', 'CssContent' => __DIR__ . '/includes/content/CssContent.php', 'CssContentHandler' => __DIR__ . '/includes/content/CssContentHandler.php', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 7f94ced2c9..53be334197 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -3,6 +3,7 @@ namespace MediaWiki; use Config; use ConfigFactory; +use CryptHKDF; use CryptRand; use EventRelayerGroup; use GenderCache; @@ -532,6 +533,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'CryptRand' ); } + /** + * @since 1.28 + * @return CryptHKDF + */ + public function getCryptHKDF() { + return $this->getService( 'CryptHKDF' ); + } + /** * @since 1.28 * @return MediaHandlerFactory diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 49183e5705..a071ff719c 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -184,6 +184,29 @@ return [ ); }, + 'CryptHKDF' => function( MediaWikiServices $services ) { + $config = $services->getMainConfig(); + + $secret = $config->get( 'HKDFSecret' ) ?: $config->get( 'SecretKey' ); + if ( !$secret ) { + throw new RuntimeException( "Cannot use MWCryptHKDF without a secret." ); + } + + // In HKDF, the context can be known to the attacker, but this will + // keep simultaneous runs from producing the same output. + $context = [ microtime(), getmypid(), gethostname() ]; + + // Setup salt cache. Use APC, or fallback to the main cache if it isn't setup + $cache = $services->getLocalServerObjectCache(); + if ( $cache instanceof EmptyBagOStuff ) { + $cache = ObjectCache::getLocalClusterInstance(); + } + + return new CryptHKDF( $secret, $config->get( 'HKDFAlgorithm' ), + $cache, $context, $services->getCryptRand() + ); + }, + 'MediaHandlerFactory' => function( MediaWikiServices $services ) { return new MediaHandlerFactory( $services->getMainConfig()->get( 'MediaHandlers' ) diff --git a/includes/libs/CryptHKDF.php b/includes/libs/CryptHKDF.php new file mode 100644 index 0000000000..4c86757418 --- /dev/null +++ b/includes/libs/CryptHKDF.php @@ -0,0 +1,282 @@ + 16, + 'sha1' => 20, + 'sha224' => 28, + 'sha256' => 32, + 'sha384' => 48, + 'sha512' => 64, + 'ripemd128' => 16, + 'ripemd160' => 20, + 'ripemd256' => 32, + 'ripemd320' => 40, + 'whirlpool' => 64, + ]; + + /** + * @var CryptRand + */ + private $cryptRand; + + /** + * @param string $secretKeyMaterial + * @param string $algorithm Name of hashing algorithm + * @param BagOStuff $cache + * @param string|array $context Context to mix into HKDF context + * @param CryptRand $cryptRand + * @throws InvalidArgumentException if secret key material is too short + */ + public function __construct( $secretKeyMaterial, $algorithm, BagOStuff $cache, $context, + CryptRand $cryptRand + ) { + if ( strlen( $secretKeyMaterial ) < 16 ) { + throw new InvalidArgumentException( "secret was too short." ); + } + $this->skm = $secretKeyMaterial; + $this->algorithm = $algorithm; + $this->cache = $cache; + $this->context = is_array( $context ) ? $context : [ $context ]; + $this->cryptRand = $cryptRand; + + // To prevent every call from hitting the same memcache server, pick + // from a set of keys to use. mt_rand is only use to pick a random + // server, and does not affect the security of the process. + $this->cacheKey = $cache->makeKey( 'HKDF', mt_rand( 0, 16 ) ); + } + + /** + * Save the last block generated, so the next user will compute a different PRK + * from the same SKM. This should keep things unpredictable even if an attacker + * is able to influence CTXinfo. + */ + function __destruct() { + if ( $this->lastK ) { + $this->cache->set( $this->cacheKey, $this->lastK ); + } + } + + /** + * MW specific salt, cached from last run + * @return string Binary string + */ + protected function getSaltUsingCache() { + if ( $this->salt == '' ) { + $lastSalt = $this->cache->get( $this->cacheKey ); + if ( $lastSalt === false ) { + // If we don't have a previous value to use as our salt, we use + // 16 bytes from CryptRand, which will use a small amount of + // entropy from our pool. Note, "XTR may be deterministic or keyed + // via an optional “salt value” (i.e., a non-secret random + // value)..." - http://eprint.iacr.org/2010/264.pdf. However, we + // use a strongly random value since we can. + $lastSalt = $this->cryptRand->generate( 16 ); + } + // Get a binary string that is hashLen long + $this->salt = hash( $this->algorithm, $lastSalt, true ); + } + return $this->salt; + } + + /** + * Produce $bytes of secure random data. As a side-effect, + * $this->lastK is set to the last hashLen block of key material. + * + * @param int $bytes Number of bytes of data + * @param string $context Context to mix into CTXinfo + * @return string Binary string of length $bytes + */ + public function generate( $bytes, $context = '' ) { + if ( $this->prk === '' ) { + $salt = $this->getSaltUsingCache(); + $this->prk = self::HKDFExtract( + $this->algorithm, + $salt, + $this->skm + ); + } + + $CTXinfo = implode( ':', array_merge( $this->context, [ $context ] ) ); + + return self::HKDFExpand( + $this->algorithm, + $this->prk, + $CTXinfo, + $bytes, + $this->lastK + ); + } + + /** + * RFC5869 defines HKDF in 2 steps, extraction and expansion. + * From http://eprint.iacr.org/2010/264.pdf: + * + * The scheme HKDF is specifed as: + * HKDF(XTS, SKM, CTXinfo, L) = K(1) || K(2) || ... || K(t) + * where the values K(i) are defined as follows: + * PRK = HMAC(XTS, SKM) + * K(1) = HMAC(PRK, CTXinfo || 0); + * K(i+1) = HMAC(PRK, K(i) || CTXinfo || i), 1 <= i < t; + * where t = [L/k] and the value K(t) is truncated to its first d = L mod k bits; + * the counter i is non-wrapping and of a given fixed size, e.g., a single byte. + * Note that the length of the HMAC output is the same as its key length and therefore + * the scheme is well defined. + * + * XTS is the "extractor salt" + * SKM is the "secret keying material" + * + * N.B. http://eprint.iacr.org/2010/264.pdf seems to differ from RFC 5869 in that the test + * vectors from RFC 5869 only work if K(0) = '' and K(1) = HMAC(PRK, K(0) || CTXinfo || 1) + * + * @param string $hash The hashing function to use (e.g., sha256) + * @param string $ikm The input keying material + * @param string $salt The salt to add to the ikm, to get the prk + * @param string $info Optional context (change the output without affecting + * the randomness properties of the output) + * @param int $L Number of bytes to return + * @return string Cryptographically secure pseudorandom binary string + */ + public static function HKDF( $hash, $ikm, $salt, $info, $L ) { + $prk = self::HKDFExtract( $hash, $salt, $ikm ); + $okm = self::HKDFExpand( $hash, $prk, $info, $L ); + return $okm; + } + + /** + * Extract the PRK, PRK = HMAC(XTS, SKM) + * Note that the hmac is keyed with XTS (the salt), + * and the SKM (source key material) is the "data". + * + * @param string $hash The hashing function to use (e.g., sha256) + * @param string $salt The salt to add to the ikm, to get the prk + * @param string $ikm The input keying material + * @return string Binary string (pseudorandm key) used as input to HKDFExpand + */ + private static function HKDFExtract( $hash, $salt, $ikm ) { + return hash_hmac( $hash, $ikm, $salt, true ); + } + + /** + * Expand the key with the given context + * + * @param string $hash Hashing Algorithm + * @param string $prk A pseudorandom key of at least HashLen octets + * (usually, the output from the extract step) + * @param string $info Optional context and application specific information + * (can be a zero-length string) + * @param int $bytes Length of output keying material in bytes + * (<= 255*HashLen) + * @param string &$lastK Set by this function to the last block of the expansion. + * In MediaWiki, this is used to seed future Extractions. + * @return string Cryptographically secure random string $bytes long + * @throws InvalidArgumentException + */ + private static function HKDFExpand( $hash, $prk, $info, $bytes, &$lastK = '' ) { + $hashLen = self::$hashLength[$hash]; + $rounds = ceil( $bytes / $hashLen ); + $output = ''; + + if ( $bytes > 255 * $hashLen ) { + throw new InvalidArgumentException( 'Too many bytes requested from HDKFExpand' ); + } + + // K(1) = HMAC(PRK, CTXinfo || 1); + // K(i) = HMAC(PRK, K(i-1) || CTXinfo || i); 1 < i <= t; + for ( $counter = 1; $counter <= $rounds; ++$counter ) { + $lastK = hash_hmac( + $hash, + $lastK . $info . chr( $counter ), + $prk, + true + ); + $output .= $lastK; + } + + return substr( $output, 0, $bytes ); + } +} diff --git a/includes/utils/MWCryptHKDF.php b/includes/utils/MWCryptHKDF.php index 2756861e81..3bddd7794a 100644 --- a/includes/utils/MWCryptHKDF.php +++ b/includes/utils/MWCryptHKDF.php @@ -29,193 +29,17 @@ * @author Chris Steipp * @file */ + use MediaWiki\MediaWikiServices; class MWCryptHKDF { - /** - * Singleton instance for public use - */ - protected static $singleton = null; - - /** - * The persistant cache - */ - protected $cache = null; - - /** - * Cache key we'll use for our salt - */ - protected $cacheKey = null; - - /** - * The hash algorithm being used - */ - protected $algorithm = null; - - /** - * binary string, the salt for the HKDF - */ - protected $salt; - - /** - * The pseudorandom key - */ - private $prk; - - /** - * The secret key material. This must be kept secret to preserve - * the security properties of this RNG. - */ - private $skm; - - /** - * The last block (K(i)) of the most recent expanded key - */ - protected $lastK; - - /** - * a "context information" string CTXinfo (which may be null) - * See http://eprint.iacr.org/2010/264.pdf Section 4.1 - */ - protected $context = []; - - /** - * Round count is computed based on the hash'es output length, - * which neither php nor openssl seem to provide easily. - */ - public static $hashLength = [ - 'md5' => 16, - 'sha1' => 20, - 'sha224' => 28, - 'sha256' => 32, - 'sha384' => 48, - 'sha512' => 64, - 'ripemd128' => 16, - 'ripemd160' => 20, - 'ripemd256' => 32, - 'ripemd320' => 40, - 'whirlpool' => 64, - ]; - - /** - * @param string $secretKeyMaterial - * @param string $algorithm Name of hashing algorithm - * @param BagOStuff $cache - * @param string|array $context Context to mix into HKDF context - * @throws MWException - */ - public function __construct( $secretKeyMaterial, $algorithm, $cache, $context ) { - if ( strlen( $secretKeyMaterial ) < 16 ) { - throw new MWException( "MWCryptHKDF secret was too short." ); - } - $this->skm = $secretKeyMaterial; - $this->algorithm = $algorithm; - $this->cache = $cache; - $this->salt = ''; // Initialize a blank salt, see getSaltUsingCache() - $this->prk = ''; - $this->context = is_array( $context ) ? $context : [ $context ]; - - // To prevent every call from hitting the same memcache server, pick - // from a set of keys to use. mt_rand is only use to pick a random - // server, and does not affect the security of the process. - $this->cacheKey = wfMemcKey( 'HKDF', mt_rand( 0, 16 ) ); - } - - /** - * Save the last block generated, so the next user will compute a different PRK - * from the same SKM. This should keep things unpredictable even if an attacker - * is able to influence CTXinfo. - */ - function __destruct() { - if ( $this->lastK ) { - $this->cache->set( $this->cacheKey, $this->lastK ); - } - } - - /** - * MW specific salt, cached from last run - * @return string Binary string - */ - protected function getSaltUsingCache() { - if ( $this->salt == '' ) { - $lastSalt = $this->cache->get( $this->cacheKey ); - if ( $lastSalt === false ) { - // If we don't have a previous value to use as our salt, we use - // 16 bytes from MWCryptRand, which will use a small amount of - // entropy from our pool. Note, "XTR may be deterministic or keyed - // via an optional “salt value” (i.e., a non-secret random - // value)..." - http://eprint.iacr.org/2010/264.pdf. However, we - // use a strongly random value since we can. - $lastSalt = MWCryptRand::generate( 16 ); - } - // Get a binary string that is hashLen long - $this->salt = hash( $this->algorithm, $lastSalt, true ); - } - return $this->salt; - } - /** * Return a singleton instance, based on the global configs. - * @return self - * @throws MWException + * @return CryptHKDF */ protected static function singleton() { - global $wgHKDFAlgorithm, $wgHKDFSecret, $wgSecretKey; - - $secret = $wgHKDFSecret ?: $wgSecretKey; - if ( !$secret ) { - throw new MWException( "Cannot use MWCryptHKDF without a secret." ); - } - - // In HKDF, the context can be known to the attacker, but this will - // keep simultaneous runs from producing the same output. - $context = []; - $context[] = microtime(); - $context[] = getmypid(); - $context[] = gethostname(); - - // Setup salt cache - $cache = MediaWikiServices::getInstance()->getLocalServerObjectCache(); - if ( $cache instanceof EmptyBagOStuff ) { - // Use APC, or fallback to the main cache if it isn't setup - $cache = ObjectCache::getLocalClusterInstance(); - } - - if ( is_null( self::$singleton ) ) { - self::$singleton = new self( $secret, $wgHKDFAlgorithm, $cache, $context ); - } - - return self::$singleton; - } - - /** - * Produce $bytes of secure random data. As a side-effect, - * $this->lastK is set to the last hashLen block of key material. - * @param int $bytes Number of bytes of data - * @param string $context Context to mix into CTXinfo - * @return string Binary string of length $bytes - */ - protected function realGenerate( $bytes, $context = '' ) { - - if ( $this->prk === '' ) { - $salt = $this->getSaltUsingCache(); - $this->prk = self::HKDFExtract( - $this->algorithm, - $salt, - $this->skm - ); - } - - $CTXinfo = implode( ':', array_merge( $this->context, [ $context ] ) ); - - return self::HKDFExpand( - $this->algorithm, - $this->prk, - $CTXinfo, - $bytes, - $this->lastK - ); + return MediaWikiServices::getInstance()->getCryptHKDF(); } /** @@ -248,62 +72,7 @@ class MWCryptHKDF { * @return string Cryptographically secure pseudorandom binary string */ public static function HKDF( $hash, $ikm, $salt, $info, $L ) { - $prk = self::HKDFExtract( $hash, $salt, $ikm ); - $okm = self::HKDFExpand( $hash, $prk, $info, $L ); - return $okm; - } - - /** - * Extract the PRK, PRK = HMAC(XTS, SKM) - * Note that the hmac is keyed with XTS (the salt), - * and the SKM (source key material) is the "data". - * - * @param string $hash The hashing function to use (e.g., sha256) - * @param string $salt The salt to add to the ikm, to get the prk - * @param string $ikm The input keying material - * @return string Binary string (pseudorandm key) used as input to HKDFExpand - */ - private static function HKDFExtract( $hash, $salt, $ikm ) { - return hash_hmac( $hash, $ikm, $salt, true ); - } - - /** - * Expand the key with the given context - * - * @param string $hash Hashing Algorithm - * @param string $prk A pseudorandom key of at least HashLen octets - * (usually, the output from the extract step) - * @param string $info Optional context and application specific information - * (can be a zero-length string) - * @param int $bytes Length of output keying material in bytes - * (<= 255*HashLen) - * @param string &$lastK Set by this function to the last block of the expansion. - * In MediaWiki, this is used to seed future Extractions. - * @return string Cryptographically secure random string $bytes long - * @throws MWException - */ - private static function HKDFExpand( $hash, $prk, $info, $bytes, &$lastK = '' ) { - $hashLen = MWCryptHKDF::$hashLength[$hash]; - $rounds = ceil( $bytes / $hashLen ); - $output = ''; - - if ( $bytes > 255 * $hashLen ) { - throw new MWException( "Too many bytes requested from HDKFExpand" ); - } - - // K(1) = HMAC(PRK, CTXinfo || 1); - // K(i) = HMAC(PRK, K(i-1) || CTXinfo || i); 1 < i <= t; - for ( $counter = 1; $counter <= $rounds; ++$counter ) { - $lastK = hash_hmac( - $hash, - $lastK . $info . chr( $counter ), - $prk, - true - ); - $output .= $lastK; - } - - return substr( $output, 0, $bytes ); + return CryptHKDF::HKDF( $hash, $ikm, $salt, $info, $L ); } /** @@ -314,7 +83,7 @@ class MWCryptHKDF { * @return string Binary string of length $bytes */ public static function generate( $bytes, $context ) { - return self::singleton()->realGenerate( $bytes, $context ); + return self::singleton()->generate( $bytes, $context ); } /** @@ -327,7 +96,7 @@ class MWCryptHKDF { */ public static function generateHex( $chars, $context = '' ) { $bytes = ceil( $chars / 2 ); - $hex = bin2hex( self::singleton()->realGenerate( $bytes, $context ) ); + $hex = bin2hex( self::singleton()->generate( $bytes, $context ) ); return substr( $hex, 0, $chars ); } diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index 0ff903f0ec..a5e2ac0d8b 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -313,6 +313,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase { 'WatchedItemStore' => [ 'WatchedItemStore', WatchedItemStore::class ], 'WatchedItemQueryService' => [ 'WatchedItemQueryService', WatchedItemQueryService::class ], 'CryptRand' => [ 'CryptRand', CryptRand::class ], + 'CryptHKDF' => [ 'CryptHKDF', CryptHKDF::class ], 'MediaHandlerFactory' => [ 'MediaHandlerFactory', MediaHandlerFactory::class ], 'GenderCache' => [ 'GenderCache', GenderCache::class ], 'LinkCache' => [ 'LinkCache', LinkCache::class ], -- 2.20.1