From b2e553ae1dee7e86b20781126c365a2cbb68434c Mon Sep 17 00:00:00 2001 From: Elnur Abdurrakhimov Date: Sun, 3 Mar 2013 15:51:00 +0400 Subject: [PATCH] Outsource all the BCrypt heavy lifting to a library --- composer.json | 3 +- .../Core/Encoder/BCryptPasswordEncoder.php | 101 ++---------------- .../Encoder/BCryptPasswordEncoderTest.php | 60 +---------- src/Symfony/Component/Security/composer.json | 3 +- 4 files changed, 16 insertions(+), 151 deletions(-) diff --git a/composer.json b/composer.json index 6c04c49de510..bef084776eb4 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,8 @@ "symfony/icu": "~1.0", "doctrine/common": "~2.2", "twig/twig": "~1.11", - "psr/log": "~1.0" + "psr/log": "~1.0", + "ircmaxell/password-compat": "1.0.*" }, "replace": { "symfony/browser-kit": "self.version", diff --git a/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php b/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php index 1b7572d49ce7..3a9d03ee70bf 100644 --- a/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php +++ b/src/Symfony/Component/Security/Core/Encoder/BCryptPasswordEncoder.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Security\Core\Encoder; use Symfony\Component\Security\Core\Encoder\BasePasswordEncoder; -use Symfony\Component\Security\Core\Util\SecureRandomInterface; /** * @author Elnur Abdurrakhimov @@ -20,39 +19,26 @@ */ class BCryptPasswordEncoder extends BasePasswordEncoder { - /** - * @var SecureRandomInterface - */ - private $secureRandom; - /** * @var string */ private $cost; - private static $prefix = null; - /** * Constructor. * - * @param SecureRandomInterface $secureRandom A SecureRandomInterface instance - * @param integer $cost The algorithmic cost that should be used + * @param integer $cost The algorithmic cost that should be used * * @throws \InvalidArgumentException if cost is out of range */ - public function __construct(SecureRandomInterface $secureRandom, $cost) + public function __construct($cost) { - $this->secureRandom = $secureRandom; - $cost = (int) $cost; if ($cost < 4 || $cost > 31) { throw new \InvalidArgumentException('Cost must be in the range of 4-31.'); } - $this->cost = sprintf('%02d', $cost); - if (!self::$prefix) { - self::$prefix = '$'.(version_compare(phpversion(), '5.3.7', '>=') ? '2y' : '2a').'$'; - } + $this->cost = sprintf('%02d', $cost); } /** @@ -60,17 +46,9 @@ public function __construct(SecureRandomInterface $secureRandom, $cost) */ public function encodePassword($raw, $salt) { - if (function_exists('password_hash')) { - return password_hash($raw, PASSWORD_BCRYPT, array('cost' => $this->cost)); - } - - $salt = self::$prefix.$this->cost.'$'.$this->encodeSalt($this->getRawSalt()); - $encoded = crypt($raw, $salt); - if (!is_string($encoded) || strlen($encoded) <= 13) { - return false; - } - - return $encoded; + return password_hash($raw, PASSWORD_BCRYPT, array( + 'cost' => $this->cost, + )); } /** @@ -78,71 +56,6 @@ public function encodePassword($raw, $salt) */ public function isPasswordValid($encoded, $raw, $salt) { - if (function_exists('password_verify')) { - return password_verify($raw, $encoded); - } - - $crypted = crypt($raw, $encoded); - if (strlen($crypted) <= 13) { - return false; - } - - return $this->comparePasswords($encoded, $crypted); - } - - /** - * Encodes the salt to be used by Bcrypt. - * - * The blowfish/bcrypt algorithm used by PHP crypt expects a different - * set and order of characters than the usual base64_encode function. - * Regular b64: ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/ - * Bcrypt b64: ./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789 - * We care because the last character in our encoded string will - * only represent 2 bits. While two known implementations of - * bcrypt will happily accept and correct a salt string which - * has the 4 unused bits set to non-zero, we do not want to take - * chances and we also do not want to waste an additional byte - * of entropy. - * - * @param bytes $random a string of 16 random bytes - * - * @return string Properly encoded salt to use with php crypt function - * - * @throws \InvalidArgumentException if string of random bytes is too short - */ - protected function encodeSalt($random) - { - $len = strlen($random); - if ($len < 16) { - throw new \InvalidArgumentException('The bcrypt salt needs 16 random bytes.'); - } - if ($len > 16) { - $random = substr($random, 0, 16); - } - - $base64raw = str_replace('+', '.', base64_encode($random)); - $salt128bit = substr($base64raw, 0, 21); - $lastchar = substr($base64raw, 21, 1); - $lastchar = strtr($lastchar, 'AQgw', '.Oeu'); - $salt128bit .= $lastchar; - - return $salt128bit; - } - - /** - * @return bytes 16 random bytes to be used in the salt - */ - protected function getRawSalt() - { - $rawSalt = false; - $numBytes = 16; - if (function_exists('mcrypt_create_iv')) { - $rawSalt = mcrypt_create_iv($numBytes, MCRYPT_DEV_URANDOM); - } - if (!$rawSalt) { - $rawSalt = $this->secureRandom->nextBytes($numBytes); - } - - return $rawSalt; + return password_verify($raw, $encoded); } } diff --git a/src/Symfony/Component/Security/Tests/Core/Encoder/BCryptPasswordEncoderTest.php b/src/Symfony/Component/Security/Tests/Core/Encoder/BCryptPasswordEncoderTest.php index 45c8f7430f98..6378433aa1c7 100644 --- a/src/Symfony/Component/Security/Tests/Core/Encoder/BCryptPasswordEncoderTest.php +++ b/src/Symfony/Component/Security/Tests/Core/Encoder/BCryptPasswordEncoderTest.php @@ -22,30 +22,12 @@ class BCryptPasswordEncoderTest extends \PHPUnit_Framework_TestCase const BYTES = '0123456789abcdef'; const VALID_COST = '04'; - const SECURE_RANDOM_INTERFACE = 'Symfony\Component\Security\Core\Util\SecureRandomInterface'; - - /** - * @var \PHPUnit_Framework_MockObject_MockObject - */ - private $secureRandom; - - protected function setUp() - { - $this->secureRandom = $this->getMock(self::SECURE_RANDOM_INTERFACE); - - $this->secureRandom - ->expects($this->any()) - ->method('nextBytes') - ->will($this->returnValue(self::BYTES)) - ; - } - /** * @expectedException \InvalidArgumentException */ public function testCostBelowRange() { - new BCryptPasswordEncoder($this->secureRandom, 3); + new BCryptPasswordEncoder(3); } /** @@ -53,60 +35,28 @@ public function testCostBelowRange() */ public function testCostAboveRange() { - new BCryptPasswordEncoder($this->secureRandom, 32); + new BCryptPasswordEncoder(32); } public function testCostInRange() { for ($cost = 4; $cost <= 31; $cost++) { - new BCryptPasswordEncoder($this->secureRandom, $cost); + new BCryptPasswordEncoder($cost); } } public function testResultLength() { - $encoder = new BCryptPasswordEncoder($this->secureRandom, self::VALID_COST); + $encoder = new BCryptPasswordEncoder(self::VALID_COST); $result = $encoder->encodePassword(self::PASSWORD, null); $this->assertEquals(60, strlen($result)); } public function testValidation() { - $encoder = new BCryptPasswordEncoder($this->secureRandom, self::VALID_COST); + $encoder = new BCryptPasswordEncoder(self::VALID_COST); $result = $encoder->encodePassword(self::PASSWORD, null); $this->assertTrue($encoder->isPasswordValid($result, self::PASSWORD, null)); $this->assertFalse($encoder->isPasswordValid($result, 'anotherPassword', null)); } - - public function testValidationKnownPassword() - { - $encoder = new BCryptPasswordEncoder($this->secureRandom, self::VALID_COST); - $prefix = '$'.(version_compare(phpversion(), '5.3.7', '>=') - ? '2y' : '2a').'$'; - - $encrypted = $prefix.'04$ABCDEFGHIJKLMNOPQRSTU.uTmwd4KMSHxbUsG7bng8x7YdA0PM1iq'; - $this->assertTrue($encoder->isPasswordValid($encrypted, self::PASSWORD, null)); - } - - public function testSecureRandomIsUsed() - { - if (function_exists('mcrypt_create_iv')) { - return; - } - - $this->secureRandom - ->expects($this->atLeastOnce()) - ->method('nextBytes') - ; - - $encoder = new BCryptPasswordEncoder($this->secureRandom, self::VALID_COST); - $result = $encoder->encodePassword(self::PASSWORD, null); - - $prefix = '$'.(version_compare(phpversion(), '5.3.7', '>=') - ? '2y' : '2a').'$'; - $salt = 'MDEyMzQ1Njc4OWFiY2RlZe'; - $expected = crypt(self::PASSWORD, $prefix.self::VALID_COST.'$'.$salt); - - $this->assertEquals($expected, $result); - } } diff --git a/src/Symfony/Component/Security/composer.json b/src/Symfony/Component/Security/composer.json index dd4eecfb91aa..c66949c68795 100644 --- a/src/Symfony/Component/Security/composer.json +++ b/src/Symfony/Component/Security/composer.json @@ -19,7 +19,8 @@ "php": ">=5.3.3", "symfony/event-dispatcher": "~2.1", "symfony/http-foundation": ">=2.1,<2.4-dev", - "symfony/http-kernel": ">=2.1,<=2.3-dev" + "symfony/http-kernel": ">=2.1,<=2.3-dev", + "ircmaxell/password-compat": "1.0.*" }, "require-dev": { "symfony/form": "~2.0",