From e675381cd41a64dce7d2690ebcdee8ee61115e5d Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 6 Nov 2009 02:23:21 +0000 Subject: [PATCH] ENHANCEMENT Pluggable password encryption through PasswordEncryptor class (#3665) BUGFIX Fixed password hashing design flaw in Security::encrypt_password(). Removing base_convert() packing with unsafe precision, but retaining backwards compatibilty through pluggable encryptors: PasswordEncryptor_LegacyPHPHash (#3004) API CHANGE Deprecated Security::encrypt_passwords() API CHANGE Deprecated Security::$useSalt, use custom PasswordEncryptor implementation API CHANGE Removed Security::get_encryption_algorithms() API CHANGE MySQL-specific encyrption types 'password' and 'old_password' are no longer included by default. Use PasswordEncryptor_MySQLPassword and PasswordEncryptor_MySQLOldPassword API CHANGE Built-in number of hashing algorithms has been reduced to 'none', 'md5', 'sha1'. Use PasswordEncryptor::register() and PasswordEncryptor_PHPHash to re-add others. git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@90949 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- _config.php | 7 +- security/Member.php | 62 +++--- security/MemberPassword.php | 15 +- security/PasswordEncryptor.php | 239 +++++++++++++++++++++++ security/Security.php | 191 +++++------------- tests/security/MemberTest.php | 106 ++++++---- tests/security/PasswordEncryptorTest.php | 81 ++++++++ 7 files changed, 497 insertions(+), 204 deletions(-) create mode 100644 security/PasswordEncryptor.php create mode 100644 tests/security/PasswordEncryptorTest.php diff --git a/_config.php b/_config.php index 2df556ef3..546252f70 100755 --- a/_config.php +++ b/_config.php @@ -66,4 +66,9 @@ define('MCE_ROOT', 'jsparty/tiny_mce/'); */ define('EMAIL_BOUNCEHANDLER_KEY', '1aaaf8fb60ea253dbf6efa71baaacbb3'); -?> +PasswordEncryptor::register('none', 'PasswordEncryptor_None'); +PasswordEncryptor::register('md5', 'PasswordEncryptor_LegacyPHPHash("md5")'); +PasswordEncryptor::register('sha1','PasswordEncryptor_LegacyPHPHash("sha1")'); +PasswordEncryptor::register('md5_v2.4', 'PasswordEncryptor_PHPHash("md5")'); +PasswordEncryptor::register('sha1_v2.4','PasswordEncryptor_PHPHash("sha1")'); +?> \ No newline at end of file diff --git a/security/Member.php b/security/Member.php index 5a7ad9faf..ca3d9de06 100644 --- a/security/Member.php +++ b/security/Member.php @@ -10,14 +10,18 @@ class Member extends DataObject { 'FirstName' => 'Varchar', 'Surname' => 'Varchar', 'Email' => 'Varchar', - 'Password' => 'Varchar(64)', // support for up to SHA256! + 'Password' => 'Varchar(160)', 'RememberLoginToken' => 'Varchar(50)', 'NumVisit' => 'Int', 'LastVisited' => 'SS_Datetime', 'Bounced' => 'Boolean', // Note: This does not seem to be used anywhere. 'AutoLoginHash' => 'Varchar(30)', 'AutoLoginExpired' => 'SS_Datetime', - 'PasswordEncryption' => "Enum('none', 'none')", + // This is an arbitrary code pointing to a PasswordEncryptor instance, + // not an actual encryption algorithm. + // Warning: Never change this field after its the first password hashing without + // providing a new cleartext password as well. + 'PasswordEncryption' => "Varchar(50)", 'Salt' => 'Varchar(50)', 'PasswordExpiry' => 'Date', 'LockedOutUntil' => 'SS_Datetime', @@ -98,6 +102,9 @@ class Member extends DataObject { */ protected static $login_marker_cookie = null; + public static function init_db_fields() { + } + /** * If this is called, then a session cookie will be set to "1" whenever a user * logs in. This lets 3rd party tools, such as apache's mod_rewrite, detect @@ -120,24 +127,6 @@ class Member extends DataObject { static function set_login_marker_cookie($cookieName) { self::$login_marker_cookie = $cookieName; } - - /** - * This method is used to initialize the static database members - * - * Since PHP doesn't support any expressions for the initialization of - * static member variables we need a method that does that. - * - * This method adds all supported encryption algorithms to the - * PasswordEncryption Enum field. - * - * @todo Maybe it would be useful to define this in DataObject and call - * it automatically? - */ - public static function init_db_fields() { - self::$db['PasswordEncryption'] = "Enum(array('none', '" . - implode("', '", array_map("addslashes", Security::get_encryption_algorithms())) . - "'), 'none')"; - } /** * Check if the passed password matches the stored one @@ -147,10 +136,17 @@ class Member extends DataObject { */ public function checkPassword($password) { // Only confirm that the password matches if the user isn't locked out - if(!$this->isLockedOut()) { - $encryption_details = Security::encrypt_password($password, $this->Salt, $this->PasswordEncryption); - return ($this->Password === $encryption_details['password']); - } + if($this->isLockedOut()) return false; + + $spec = Security::encrypt_password( + $password, + $this->Salt, + $this->PasswordEncryption, + $this + ); + $e = $spec['encryptor']; + + return $e->compare($this->Password, $spec['password']); } /** @@ -538,10 +534,18 @@ class Member extends DataObject { $this->sendInfo('changePassword'); } - // The test on $this->ID is used for when records are initially created + // The test on $this->ID is used for when records are initially created. + // Note that this only works with cleartext passwords, as we can't rehash + // existing passwords. if(!$this->ID || $this->isChanged('Password')) { // Password was changed: encrypt the password according the settings - $encryption_details = Security::encrypt_password($this->Password); + $encryption_details = Security::encrypt_password( + $this->Password, // this is assumed to be cleartext + $this->Salt, + $this->PasswordEncryption, + $this + ); + // Overwrite the Password property with the hashed value $this->Password = $encryption_details['password']; $this->Salt = $encryption_details['salt']; $this->PasswordEncryption = $encryption_details['algorithm']; @@ -1033,6 +1037,12 @@ class Member extends DataObject { return $valid; } + /** + * Change password. This will cause rehashing according to + * the `PasswordEncryption` property. + * + * @param String $password Cleartext password + */ function changePassword($password) { $this->Password = $password; $valid = $this->validate(); diff --git a/security/MemberPassword.php b/security/MemberPassword.php index 1063922b2..d53f07693 100644 --- a/security/MemberPassword.php +++ b/security/MemberPassword.php @@ -35,11 +35,20 @@ class MemberPassword extends DataObject { } /** - * Check if the given password is the same as the one stored in this record + * Check if the given password is the same as the one stored in this record. + * See {@link Member->checkPassword()}. + * + * @param String $password Cleartext password + * @return Boolean */ function checkPassword($password) { - $encryption_details = Security::encrypt_password($password, $this->Salt, $this->PasswordEncryption); - return ($this->Password === $encryption_details['password']); + $spec = Security::encrypt_password( + $password, + $this->Salt, + $this->PasswordEncryption + ); + $e = $spec['encryptor']; + return $e->compare($this->Password, $spec['password']); } diff --git a/security/PasswordEncryptor.php b/security/PasswordEncryptor.php new file mode 100644 index 000000000..a720d445b --- /dev/null +++ b/security/PasswordEncryptor.php @@ -0,0 +1,239 @@ +PasswordEncryption} property. + * @param String $class Classname of a {@link PasswordEncryptor} subclass + */ + static function register($code, $class) { + self::$encryptors[$code] = $class; + } + + /** + * @param String $code Unique lookup. + */ + static function unregister($code) { + if(isset(self::$encryptors[$code])) unset(self::$encryptors[$code]); + } + + /** + * @param String $algorithm + * @return PasswordEncryptor|Boolean Returns FALSE if class was not found + */ + static function create_for_algorithm($algorithm) { + if(!isset(self::$encryptors[$algorithm])) { + throw new PasswordEncryptor_NotFoundException( + sprintf('No implementation found for "%s"', $algorithm) + ); + } + + $classWithArgs = self::$encryptors[$algorithm]; + $class = (($p = strpos($classWithArgs, '(')) !== false) ? substr($classWithArgs, 0, $p) : $classWithArgs; + if(!class_exists($class)) { + throw new PasswordEncryptor_NotFoundException( + sprintf('No class found for "%s"', $class) + ); + } + + return eval("return new $classWithArgs;"); + } + + /** + * Return a string value stored in the {@link Member->Password} property. + * The password should be hashed with {@link salt()} if applicable. + * + * @param String $password Cleartext password to be hashed + * @param String $salt (Optional) + * @param Member $member (Optional) + * @return String Maximum of 512 characters. + */ + abstract function encrypt($password, $salt = null, $member = null); + + /** + * Return a string value stored in the {@link Member->Salt} property. + * By default uses sha1() and mt_rand(); + * + * Note: Only used when {@link Security::$useSalt} is TRUE. + * + * @param String $password Cleartext password + * @param Member $member (Optional) + * @return String Maximum of 50 characters + */ + function salt($password, $member = null) { + return substr(sha1(mt_rand()) . time(), 0, 50); + } + + /** + * This usually just returns a strict string comparison, + * but is necessary for {@link PasswordEncryptor_LegacyPHPHash}. + * + * @param String $hash1 + * @param String $hash2 + * @return boolean + */ + function compare($hash1, $hash2) { + return ($hash1 === $hash2); + } +} + +/** + * This is the default class used for built-in hash types in PHP. + * Please note that the implemented algorithms depend on the PHP + * distribution and architecture. + * + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_PHPHash extends PasswordEncryptor { + + protected $algorithm = 'sha1'; + + /** + * @param String $algorithm A PHP built-in hashing algorithm as defined by hash_algos() + */ + function __construct($algorithm) { + if(!in_array($algorithm, hash_algos())) { + throw new Exception( + sprintf('Hash algorithm "%s" not found in hash_algos()', $algorithm) + ); + } + + $this->algorithm = $algorithm; + } + + /** + * @return string + */ + function getAlgorithm() { + return $this->algorithm; + } + + function encrypt($password, $salt = null, $member = null) { + if(function_exists('hash')) { + // Available in PHP 5.1+ only + return hash($this->algorithm, $password . $salt); + } else { + // Fallback to global built-in methods + return call_user_func($this->algorithm, $password . $salt); + } + } +} + +/** + * Legacy implementation for SilverStripe 2.1 - 2.3, + * which had a design flaw in password hashing that caused + * the hashes to differ between architectures due to + * floating point precision problems in base_convert(). + * See http://open.silverstripe.org/ticket/3004 + * + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_LegacyPHPHash extends PasswordEncryptor_PHPHash { + function encrypt($password, $salt = null, $member = null) { + $password = parent::encrypt($password, $member, $salt); + + // Legacy fix: This shortening logic is producing unpredictable results. + // + // Convert the base of the hexadecimal password to 36 to make it shorter + // In that way we can store also a SHA256 encrypted password in just 64 + // letters. + return substr(base_convert($password, 16, 36), 0, 64); + } + + function compare($hash1, $hash2) { + // Due to flawed base_convert() floating poing precision, + // only the first 10 characters are consistently useful for comparisons. + return (substr($hash1, 0, 10) === substr($hash2, 0, 10)); + } +} + +/** + * Uses MySQL's PASSWORD encryption. Requires an active DB connection. + * + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_MySQLPassword extends PasswordEncryptor { + function encrypt($password, $salt = null, $member = null) { + return DB::query( + sprintf("SELECT PASSWORD('%s')", Convert::raw2sql($password)) + )->value(); + } + + function salt($password, $member = null) { + return false; + } +} + +/** + * Uses MySQL's OLD_PASSWORD encyrption. Requires an active DB connection. + * + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_MySQLOldPassword extends PasswordEncryptor { + function encrypt($password, $salt = null, $member = null) { + return DB::query( + sprintf("SELECT OLD_PASSWORD('%s')", Convert::raw2sql($password)) + )->value(); + } + + function salt($password, $member = null) { + return false; + } +} + +/** + * Cleartext passwords (used in SilverStripe 2.1). + * Also used when Security::$encryptPasswords is set to FALSE. + * Not recommended. + * + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_None extends PasswordEncryptor { + function encrypt($password, $salt = null, $member = null) { + return $password; + } + + function salt($password, $member = null) { + return false; + } +} + +/** + * @package sapphire + * @subpackage security + */ +class PasswordEncryptor_NotFoundException extends Exception {} \ No newline at end of file diff --git a/security/Security.php b/security/Security.php index b59e5627a..9627fcb8a 100644 --- a/security/Security.php +++ b/security/Security.php @@ -32,21 +32,23 @@ class Security extends Controller { /** * Should passwords be stored encrypted? + * @deprecated 2.4 Please use 'none' as the default $encryptionAlgorithm instead * * @var bool */ protected static $encryptPasswords = true; /** - * The password encryption algorithm to use if {@link $encryptPasswords} - * is set to TRUE. + * The password encryption algorithm to use by default. + * This is an arbitrary code registered through {@link PasswordEncryptor}. * * @var string */ - protected static $encryptionAlgorithm = 'sha1'; + protected static $encryptionAlgorithm = 'sha1_v2.4'; /** * Should a salt be used for the password encryption? + * @deprecated 2.4 Please use a custom {@link PasswordEncryptor} instead * * @var bool */ @@ -703,6 +705,8 @@ class Security extends Controller { /** * Set if passwords should be encrypted or not * + * @deprecated 2.4 Use PasswordEncryptor_None instead. + * * @param bool $encrypt Set to TRUE if you want that all (new) passwords * will be stored encrypted, FALSE if you want to * store the passwords in clear text. @@ -713,36 +717,15 @@ class Security extends Controller { /** - * Get a list of all available encryption algorithms + * Get a list of all available encryption algorithms. + * Note: These are arbitrary codes, and not callable methods. + * + * @deprecated 2.4 Use PasswordEncryptor::get_encryptors() * - * @return array Returns an array of strings containing all supported - * encryption algorithms. + * @return array Returns an array of strings containing all supported encryption algorithms. */ public static function get_encryption_algorithms() { - $result = function_exists('hash_algos') ? hash_algos() : array(); - - if(count($result) == 0) { - if(function_exists('md5')) $result[] = 'md5'; - - if(function_exists('sha1')) $result[] = 'sha1'; - } else { - foreach ($result as $i => $algorithm) { - if (preg_match('/,/',$algorithm)) { - unset($result[$i]); - } - } - } - - // Support for MySQL password() and old_password() functions. These aren't recommended unless you need them, - // but can be helpful for migrating legacy user-sets into a SilverStripe application. - // Since DB::getConn() doesn't exist yet, we need to look at $databaseConfig. Gack! - global $databaseConfig; - if($databaseConfig['type'] == 'MySQLDatabase') { - $result[] = 'password'; - $result[] = 'old_password'; - } - - return $result; + return array_keys(PasswordEncryptor::get_encryptors()); } @@ -750,138 +733,65 @@ class Security extends Controller { * Set the password encryption algorithm * * @param string $algorithm One of the available password encryption - * algorithms determined by - * {@link Security::get_encryption_algorithms()} - * @param bool $use_salt Set to TRUE if a random salt should be used to - * encrypt the passwords, otherwise FALSE - * @return bool Returns TRUE if the passed algorithm was valid, otherwise - * FALSE. + * algorithms determined by {@link Security::get_encryption_algorithms()} + * @return bool Returns TRUE if the passed algorithm was valid, otherwise FALSE. */ - public static function set_password_encryption_algorithm($algorithm, - $use_salt) { - if(in_array($algorithm, self::get_encryption_algorithms()) == false) - return false; - + public static function set_password_encryption_algorithm($algorithm) { + if(!array_key_exists($algorithm, PasswordEncryptor::get_encryptors())) return false; + self::$encryptionAlgorithm = $algorithm; - self::$useSalt = (bool)$use_salt; - return true; } - - + /** - * Get the the password encryption details - * - * The return value is an array of the following form: - * - * array('encrypt_passwords' => bool, - * 'algorithm' => string, - * 'use_salt' => bool) - * - * - * @return array Returns an associative array containing all the - * password encryption relevant information. + * @return String */ - public static function get_password_encryption_details() { - return array('encrypt_passwords' => self::$encryptPasswords, - 'algorithm' => self::$encryptionAlgorithm, - 'use_salt' => self::$useSalt); + public static function get_password_encryption_algorithm() { + return self::$encryptionAlgorithm; } - /** - * Encrypt a password - * - * Encrypt a password according to the current password encryption - * settings. - * Use {@link Security::get_password_encryption_details()} to retrieve the - * current settings. + * Encrypt a password according to the current password encryption settings. * If the settings are so that passwords shouldn't be encrypted, the * result is simple the clear text password with an empty salt except when * a custom algorithm ($algorithm parameter) was passed. * * @param string $password The password to encrypt * @param string $salt Optional: The salt to use. If it is not passed, but - * needed, the method will automatically create a - * random salt that will then be returned as return - * value. + * needed, the method will automatically create a + * random salt that will then be returned as return value. * @param string $algorithm Optional: Use another algorithm to encrypt the - * password (so that the encryption algorithm can - * be changed over the time). + * password (so that the encryption algorithm can be changed over the time). + * @param Member $member Optional * @return mixed Returns an associative array containing the encrypted - * password and the used salt in the form - * array('encrypted_password' => string, 'salt' => - * string, 'algorithm' => string). - * If the passed algorithm is invalid, FALSE will be - * returned. + * password and the used salt in the form: + * + * array( + * 'password' => string, + * 'salt' => string, + * 'algorithm' => string, + * 'encryptor' => PasswordEncryptor instance + * ) + * + * If the passed algorithm is invalid, FALSE will be returned. * * @see encrypt_passwords() * @see set_password_encryption_algorithm() - * @see get_password_encryption_details() */ - public static function encrypt_password($password, $salt = null, - $algorithm = null) { - if(strlen(trim($password)) == 0) { - // An empty password was passed, return an empty password an salt! - return array('password' => null, - 'salt' => null, - 'algorithm' => 'none'); - - } elseif((!$algorithm && self::$encryptPasswords == false) || ($algorithm == 'none')) { - // The password should not be encrypted - return array('password' => substr($password, 0, 64), - 'salt' => null, - 'algorithm' => 'none'); - - } elseif(strlen(trim($algorithm)) != 0) { - // A custom encryption algorithm was passed, check if we can use it - if(in_array($algorithm, self::get_encryption_algorithms()) == false) - return false; - + static function encrypt_password($password, $salt = null, $algorithm = null, $member = null) { + if( + // if the password is empty, don't encrypt + strlen(trim($password)) == 0 + // if no algorithm is provided and no default is set, don't encrypt + || (!$algorithm && self::$encryptPasswords == false) + ) { + $algorithm = 'none'; } else { - // Just use the default encryption algorithm - $algorithm = self::$encryptionAlgorithm; - } + // Fall back to the default encryption algorithm + if(!$algorithm) $algorithm = self::$encryptionAlgorithm; + } - // Support for MySQL password() and old_password() authentication - if(strtolower($algorithm) == 'password' || strtolower($algorithm) == 'old_password') { - $SQL_password = Convert::raw2sql($password); - $enc = DB::query("SELECT $algorithm('$SQL_password')")->value(); - return array( - 'password' => $enc, - 'salt' => null, - 'algorithm' => $algorithm, - ); - } - - - // If no salt was provided but we need one we just generate a random one - if(strlen(trim($salt)) == 0) - $salt = null; - - if((self::$useSalt == true) && is_null($salt)) { - $salt = sha1(mt_rand()) . time(); - $salt = substr(base_convert($salt, 16, 36), 0, 50); - } - - - // Encrypt the password - if(function_exists('hash')) { - $password = hash($algorithm, $password . $salt); - } else { - $password = call_user_func($algorithm, $password . $salt); - } - - // Convert the base of the hexadecimal password to 36 to make it shorter - // In that way we can store also a SHA256 encrypted password in just 64 - // letters. - $password = substr(base_convert($password, 16, 36), 0, 64); - - - return array('password' => $password, - 'salt' => $salt, - 'algorithm' => $algorithm); - } + $e = PasswordEncryptor::create_for_algorithm($algorithm); // New salts will only need to be generated if the password is hashed for the first time $salt = ($salt) ? $salt : $e->salt($password); @@ -947,5 +857,4 @@ class Security extends Controller { } } - -?> +?> \ No newline at end of file diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php index f66630995..f5e0de5b7 100644 --- a/tests/security/MemberTest.php +++ b/tests/security/MemberTest.php @@ -6,34 +6,74 @@ class MemberTest extends SapphireTest { static $fixture_file = 'sapphire/tests/security/MemberTest.yml'; + function setUp() { + parent::setUp(); + + Member::set_password_validator(null); + } + + function testDefaultPasswordEncryptionOnMember() { + $member = new Member(); + $member->Password = 'mypassword'; + $member->write(); + $this->assertEquals( + $member->PasswordEncryption, + Security::get_password_encryption_algorithm(), + 'Password encryption is set for new member records on first write' + ); + } + + function testDefaultPasswordEncryptionDoesntChangeExistingMembers() { + $member = new Member(); + $member->Password = 'mypassword'; + $member->PasswordEncryption = 'sha1_v2.4'; + $member->write(); + + Security::set_password_encryption_algorithm('none'); + + $member->Password = 'mynewpassword'; + $member->write(); + + $this->assertEquals( + $member->PasswordEncryption, + 'sha1_v2.4' + ); + $this->assertTrue($member->checkPassword("mynewpassword")); + } + + function testSetPassword() { + $member = $this->objFromFixture('Member', 'test'); + $member->Password = "test1"; + $member->write(); + $this->assertTrue($member->checkPassword("test1")); + } + /** * Test that password changes are logged properly */ function testPasswordChangeLogging() { - Member::set_password_validator(null); - $member = $this->objFromFixture('Member', 'test'); $this->assertNotNull($member); $member->Password = "test1"; $member->write(); - + $member->Password = "test2"; $member->write(); - + $member->Password = "test3"; $member->write(); - + $passwords = DataObject::get("MemberPassword", "\"MemberID\" = $member->ID", "\"Created\" DESC, \"ID\" DESC")->getIterator(); $this->assertNotNull($passwords); $record = $passwords->rewind(); $this->assertTrue($record->checkPassword('test3'), "Password test3 not found in MemberRecord"); - + $record = $passwords->next(); $this->assertTrue($record->checkPassword('test2'), "Password test2 not found in MemberRecord"); - + $record = $passwords->next(); $this->assertTrue($record->checkPassword('test1'), "Password test1 not found in MemberRecord"); - + $record = $passwords->next(); $this->assertType('DataObject', $record); $this->assertTrue($record->checkPassword('1nitialPassword'), "Password 1nitialPassword not found in MemberRecord"); @@ -44,7 +84,7 @@ class MemberTest extends SapphireTest { */ function testChangedPasswordEmaling() { $this->clearEmails(); - + $member = $this->objFromFixture('Member', 'test'); $this->assertNotNull($member); $valid = $member->changePassword('32asDF##$$%%'); @@ -65,59 +105,59 @@ class MemberTest extends SapphireTest { $this->assertNotNull($member); Member::set_password_validator(new NZGovtPasswordValidator()); - + // BAD PASSWORDS $valid = $member->changePassword('shorty'); $this->assertFalse($valid->valid()); $this->assertContains("TOO_SHORT", $valid->codeList()); - + $valid = $member->changePassword('longone'); $this->assertNotContains("TOO_SHORT", $valid->codeList()); $this->assertContains("LOW_CHARACTER_STRENGTH", $valid->codeList()); $this->assertFalse($valid->valid()); - + $valid = $member->changePassword('w1thNumb3rs'); $this->assertNotContains("LOW_CHARACTER_STRENGTH", $valid->codeList()); $this->assertTrue($valid->valid()); // Clear out the MemberPassword table to ensure that the system functions properly in that situation DB::query("DELETE FROM \"MemberPassword\""); - + // GOOD PASSWORDS $valid = $member->changePassword('withSym###Ls'); $this->assertNotContains("LOW_CHARACTER_STRENGTH", $valid->codeList()); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls2'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls3'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls4'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls5'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls6'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls7'); $this->assertTrue($valid->valid()); - + // CAN'T USE PASSWORDS 2-7, but I can use pasword 1 - + $valid = $member->changePassword('withSym###Ls2'); $this->assertFalse($valid->valid()); $this->assertContains("PREVIOUS_PASSWORD", $valid->codeList()); - + $valid = $member->changePassword('withSym###Ls5'); $this->assertFalse($valid->valid()); $this->assertContains("PREVIOUS_PASSWORD", $valid->codeList()); - + $valid = $member->changePassword('withSym###Ls7'); $this->assertFalse($valid->valid()); $this->assertContains("PREVIOUS_PASSWORD", $valid->codeList()); @@ -126,19 +166,19 @@ class MemberTest extends SapphireTest { $this->assertTrue($valid->valid()); // HAVING DONE THAT, PASSWORD 2 is now available from the list - + $valid = $member->changePassword('withSym###Ls2'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls3'); $this->assertTrue($valid->valid()); - + $valid = $member->changePassword('withSym###Ls4'); $this->assertTrue($valid->valid()); - + Member::set_password_validator(null); } - + /** * Test that the PasswordExpiry date is set when passwords are changed */ @@ -152,11 +192,11 @@ class MemberTest extends SapphireTest { $expiryDate = date('Y-m-d', time() + 90*86400); $this->assertEquals($expiryDate, $member->PasswordExpiry); - + Member::set_password_expiry(null); $valid = $member->changePassword("Xx?1234235"); $this->assertTrue($valid->valid()); - + $this->assertNull($member->PasswordExpiry); } @@ -164,11 +204,11 @@ class MemberTest extends SapphireTest { $member = $this->objFromFixture('Member', 'test'); $this->assertNotNull($member); $this->assertFalse($member->isPasswordExpired()); - + $member = $this->objFromFixture('Member', 'noexpiry'); $member->PasswordExpiry = null; $this->assertFalse($member->isPasswordExpired()); - + $member = $this->objFromFixture('Member', 'expiredpassword'); $this->assertTrue($member->isPasswordExpired()); @@ -176,7 +216,7 @@ class MemberTest extends SapphireTest { // If PasswordExpiry == today, then it's expired $member->PasswordExpiry = date('Y-m-d'); $this->assertTrue($member->isPasswordExpired()); - + // If PasswordExpiry == tomorrow, then it's not $member->PasswordExpiry = date('Y-m-d', time() + 86400); $this->assertFalse($member->isPasswordExpired()); diff --git a/tests/security/PasswordEncryptorTest.php b/tests/security/PasswordEncryptorTest.php new file mode 100644 index 000000000..c514b3ab2 --- /dev/null +++ b/tests/security/PasswordEncryptorTest.php @@ -0,0 +1,81 @@ +assertType( + 'PasswordEncryptorTest_TestEncryptor', + $e + ); + } + + /** + * @expectedException PasswordEncryptor_NotFoundException + */ + function testCreateForCodeNotFound() { + PasswordEncryptor::create_for_algorithm('unknown'); + } + + function testRegister() { + PasswordEncryptor::register('test', 'PasswordEncryptorTest_TestEncryptor'); + $this->assertContains('test', array_keys(PasswordEncryptor::get_encryptors())); + $this->assertContains('PasswordEncryptorTest_TestEncryptor', array_values(PasswordEncryptor::get_encryptors())); + } + + function testUnregister() { + PasswordEncryptor::register('test', 'PasswordEncryptorTest_TestEncryptor'); + PasswordEncryptor::unregister('test'); + $this->assertNotContains('test', array_keys(PasswordEncryptor::get_encryptors())); + } + + function testEncrytorPHPHashWithArguments() { + PasswordEncryptor::register('test_md5', 'PasswordEncryptor_PHPHash("md5")'); + $e = PasswordEncryptor::create_for_algorithm('test_md5'); + $this->assertEquals('md5', $e->getAlgorithm()); + } + + function testEncrytorPHPHash() { + PasswordEncryptor::register('test_sha1', 'PasswordEncryptor_PHPHash("sha1")'); + $e = PasswordEncryptor::create_for_algorithm('test_sha1'); + $password = 'mypassword'; + $salt = 'mysalt'; + $this->assertEquals( + hash('sha1', $password . $salt), + $e->encrypt($password, $salt) + ); + } + + function testEncrytorPHPHashCompare() { + PasswordEncryptor::register('test_sha1', 'PasswordEncryptor_PHPHash("sha1")'); + $e = PasswordEncryptor::create_for_algorithm('test_sha1'); + $this->assertTrue($e->compare(sha1('mypassword'), sha1('mypassword'))); + $this->assertFalse($e->compare(sha1('mypassword'), sha1('mywrongpassword'))); + } + + /** + * See http://open.silverstripe.org/ticket/3004 + * + * Handy command for reproducing via CLI on different architectures: + * php -r "echo(base_convert(sha1('mypassword'), 16, 36));" + */ + function testEncrytorLegacyPHPHashCompare() { + PasswordEncryptor::register('test_sha1legacy', 'PasswordEncryptor_LegacyPHPHash("sha1")'); + $e = PasswordEncryptor::create_for_algorithm('test_sha1legacy'); + // precomputed hashes for 'mypassword' from different architectures + $amdHash = 'h1fj0a6m4o6k0sosks88oo08ko4gc4s'; + $intelHash = 'h1fj0a6m4o0g04ocg00o4kwoc4wowws'; + $wrongHash = 'h1fjxxxxxxxxxxxxxxxxxxxxxxxxxxx'; + $this->assertTrue($e->compare($amdHash, $intelHash)); + $this->assertFalse($e->compare($amdHash, $wrongHash)); + } +} + +class PasswordEncryptorTest_TestEncryptor extends PasswordEncryptor implements TestOnly { + function encrypt($password, $salt = null, $member = null) { + return 'password'; + } + + function salt($password, $member = null) { + return 'salt'; + } +} \ No newline at end of file