From fa60f9e8b2f0d6f15d1cfe25a6ae8e3115e3e592 Mon Sep 17 00:00:00 2001 From: Andrew O'Neil Date: Wed, 2 May 2012 13:51:29 +1200 Subject: [PATCH] ENHANCEMENT: Implement blowfish encryption and use it by default. (#7111) --- _config/PasswordEncryptor.yml | 4 +- dev/Backtrace.php | 2 + security/PasswordEncryptor.php | 45 ++++++++++++++++++++- security/Security.php | 2 +- tests/security/PasswordEncryptorTest.php | 19 +++++++-- tests/tasks/EncryptAllPasswordsTaskTest.php | 2 +- 6 files changed, 66 insertions(+), 8 deletions(-) diff --git a/_config/PasswordEncryptor.yml b/_config/PasswordEncryptor.yml index 662d22732..6ffe30dad 100644 --- a/_config/PasswordEncryptor.yml +++ b/_config/PasswordEncryptor.yml @@ -11,4 +11,6 @@ PasswordEncryptor: md5_v2.4: PasswordEncryptor_PHPHash: md5 sha1_v2.4: - PasswordEncryptor_PHPHash: sha1 \ No newline at end of file + PasswordEncryptor_PHPHash: sha1 + blowfish: + PasswordEncryptor_Blowfish: \ No newline at end of file diff --git a/dev/Backtrace.php b/dev/Backtrace.php index 04b7d169e..de032e37c 100644 --- a/dev/Backtrace.php +++ b/dev/Backtrace.php @@ -33,6 +33,8 @@ class SS_Backtrace { array('PasswordEncryptor_MySQLPassword', 'salt'), array('PasswordEncryptor_MySQLOldPassword', 'encrypt'), array('PasswordEncryptor_MySQLOldPassword', 'salt'), + array('PasswordEncryptor_Blowfish', 'encrypt'), + array('PasswordEncryptor_Blowfish', 'salt'), ); /** diff --git a/security/PasswordEncryptor.php b/security/PasswordEncryptor.php index 90874aa2b..d8869e3b4 100644 --- a/security/PasswordEncryptor.php +++ b/security/PasswordEncryptor.php @@ -116,7 +116,44 @@ abstract class PasswordEncryptor { } /** - * This is the default class used for built-in hash types in PHP. + * Blowfish encryption - this is the default from SilverStripe 3. + * PHP 5.3+ will provide a php implementation if there is no system + * version available. + * + * @package framework + * @subpackage security + */ +class PasswordEncryptor_Blowfish extends PasswordEncryptor { + /** + * Cost of encryption. + * Higher costs will increase security, but also increase server load. + * If you are using basic auth, you may need to decrease this as encryption + * will be run on every request. + * Must be between 4 and 31. + */ + protected static $cost = 10; + + function encrypt($password, $salt = null, $member = null) { + $method_and_salt = '$2y$' . $salt; + $encrypted_password = crypt($password, $method_and_salt); + // We *never* want to generate blank passwords. If something + // goes wrong, throw an exception. + if(strpos($encrypted_password, $method_and_salt) === false) + throw new PasswordEncryptor_EncryptionFailed('Blowfish password encryption failed.'); + + // Remove the method and salt from the password, as the salt + // is stored in a separate column. + return substr($encrypted_password, strlen($method_and_salt)); + } + + function salt($password, $memeber = null) { + $generator = new RandomGenerator(); + return self::$cost . '$' . substr($generator->generateHash('sha1'), 0, 21); + } +} + +/** + * Encryption using built-in hash types in PHP. * Please note that the implemented algorithms depend on the PHP * distribution and architecture. * @@ -240,3 +277,9 @@ class PasswordEncryptor_None extends PasswordEncryptor { * @subpackage security */ class PasswordEncryptor_NotFoundException extends Exception {} + +/** + * @package framework + * @subpackage security + */ +class PasswordEncryptor_EncryptionFailed extends Exception {} diff --git a/security/Security.php b/security/Security.php index c28999126..29765ec96 100644 --- a/security/Security.php +++ b/security/Security.php @@ -50,7 +50,7 @@ class Security extends Controller { * * @var string */ - protected static $encryptionAlgorithm = 'sha1_v2.4'; + protected static $encryptionAlgorithm = 'blowfish'; /** * Showing "Remember me"-checkbox diff --git a/tests/security/PasswordEncryptorTest.php b/tests/security/PasswordEncryptorTest.php index 5cdff1c80..0a5096b80 100644 --- a/tests/security/PasswordEncryptorTest.php +++ b/tests/security/PasswordEncryptorTest.php @@ -44,13 +44,13 @@ class PasswordEncryptorTest extends SapphireTest { $this->assertNotContains('test', array_keys(PasswordEncryptor::get_encryptors())); } - function testEncrytorPHPHashWithArguments() { + function testEncryptorPHPHashWithArguments() { Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_md5'=>array('PasswordEncryptor_PHPHash'=>'md5'))); $e = PasswordEncryptor::create_for_algorithm('test_md5'); $this->assertEquals('md5', $e->getAlgorithm()); } - function testEncrytorPHPHash() { + function testEncryptorPHPHash() { Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1'=>array('PasswordEncryptor_PHPHash'=>'sha1'))); $e = PasswordEncryptor::create_for_algorithm('test_sha1'); $password = 'mypassword'; @@ -60,8 +60,19 @@ class PasswordEncryptorTest extends SapphireTest { $e->encrypt($password, $salt) ); } + + function testEncryptorBlowfish() { + Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_blowfish'=>array('PasswordEncryptor_Blowfish'=>''))); + $e = PasswordEncryptor::create_for_algorithm('test_blowfish'); + $password = 'mypassword'; + $salt = '10$mysaltmustbetwen2char'; + $this->assertEquals( + crypt($password, '$2y$' . $salt), + '$2y$' . $salt . $e->encrypt($password, $salt) + ); + } - function testEncrytorPHPHashCompare() { + function testEncryptorPHPHashCompare() { Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1'=>array('PasswordEncryptor_PHPHash'=>'sha1'))); $e = PasswordEncryptor::create_for_algorithm('test_sha1'); $this->assertTrue($e->compare(sha1('mypassword'), sha1('mypassword'))); @@ -74,7 +85,7 @@ class PasswordEncryptorTest extends SapphireTest { * Handy command for reproducing via CLI on different architectures: * php -r "echo(base_convert(sha1('mypassword'), 16, 36));" */ - function testEncrytorLegacyPHPHashCompare() { + function testEncryptorLegacyPHPHashCompare() { Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_sha1legacy'=>array('PasswordEncryptor_LegacyPHPHash'=>'sha1'))); $e = PasswordEncryptor::create_for_algorithm('test_sha1legacy'); // precomputed hashes for 'mypassword' from different architectures diff --git a/tests/tasks/EncryptAllPasswordsTaskTest.php b/tests/tasks/EncryptAllPasswordsTaskTest.php index 430efba0c..508845bc5 100644 --- a/tests/tasks/EncryptAllPasswordsTaskTest.php +++ b/tests/tasks/EncryptAllPasswordsTaskTest.php @@ -14,7 +14,7 @@ class EncryptAllPasswordsTaskTest extends SapphireTest { $t->run(null); $m = DataObject::get_by_id('Member', $m->ID); - $this->assertEquals($m->PasswordEncryption, 'sha1_v2.4'); + $this->assertEquals($m->PasswordEncryption, 'blowfish'); $this->assertNotEquals($m->Password, 'plain'); $result = $m->checkPassword('plain'); $this->assertTrue($result->valid());