Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

APICHANGE: PasswordEncryptor::check() allows for more powerful passwo…

…rd checking, deprecating PasswordEncryptor::compare()
  • Loading branch information...
commit 89fc8e5fdd03c20922b8529549cfc072f3decaf6 1 parent 4e18cc5
@ajoneil ajoneil authored
View
11 security/Member.php
@@ -190,15 +190,8 @@ static function set_login_marker_cookie($cookieName) {
public function checkPassword($password) {
$result = $this->canLogIn();
- $spec = Security::encrypt_password(
- $password,
- $this->Salt,
- $this->PasswordEncryption,
- $this
- );
- $e = $spec['encryptor'];
-
- if(!$e->compare($this->Password, $spec['password'])) {
+ $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
+ if(!$e->check($this->Password, $password, $this->Salt, $this)) {
$result->error(_t (
'Member.ERRORWRONGCRED',
'That doesn\'t seem to be the right e-mail address or password. Please try again.'
View
9 security/MemberPassword.php
@@ -42,13 +42,8 @@ static function log($member) {
* @return Boolean
*/
function checkPassword($password) {
- $spec = Security::encrypt_password(
- $password,
- $this->Salt,
- $this->PasswordEncryption
- );
- $e = $spec['encryptor'];
- return $e->compare($this->Password, $spec['password']);
+ $e = PasswordEncryptor::create_for_algorithm($this->PasswordEncryption);
+ return $e->check($this->Password, $password, $this->Salt, $this->Member());
}
View
21 security/PasswordEncryptor.php
@@ -109,10 +109,23 @@ function salt($password, $member = null) {
* @param String $hash1
* @param String $hash2
* @return boolean
+ *
+ * @deprecated 3.0 - Use PasswordEncryptor::check() instead.
*/
function compare($hash1, $hash2) {
+ Deprecation::notice('3.0.0', 'PasswordEncryptor::compare() is deprecated, replaced by PasswordEncryptor::check().');
return ($hash1 === $hash2);
}
+
+ /**
+ * This usually just returns a strict string comparison,
+ * but is necessary for retain compatibility with password hashed
+ * with flawed algorithms - see {@link PasswordEncryptor_LegacyPHPHash} and
+ * {@link PasswordEncryptor_Blowfish}
+ */
+ function check($hash, $password, $salt = null, $member = null) {
+ return $hash === $this->encrypt($password, $salt, $member);
+ }
}
/**
@@ -217,10 +230,18 @@ function encrypt($password, $salt = null, $member = null) {
}
function compare($hash1, $hash2) {
+ Deprecation::notice('3.0.0', 'PasswordEncryptor::compare() is deprecated, replaced by PasswordEncryptor::check().');
+
// 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));
}
+
+ function check($hash, $password, $salt = null, $member = null) {
+ // Due to flawed base_convert() floating poing precision,
+ // only the first 10 characters are consistently useful for comparisons.
+ return (substr($hash, 0, 10) === substr($this->encrypt($password, $salt, $member), 0, 10));
+ }
}
/**
View
13 tests/security/PasswordEncryptorTest.php
@@ -72,11 +72,11 @@ function testEncryptorBlowfish() {
);
}
- function testEncryptorPHPHashCompare() {
+ function testEncryptorPHPHashCheck() {
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')));
- $this->assertFalse($e->compare(sha1('mypassword'), sha1('mywrongpassword')));
+ $this->assertTrue($e->check(sha1('mypassword'), 'mypassword'));
+ $this->assertFalse($e->check(sha1('mypassword'), 'mywrongpassword'));
}
/**
@@ -85,15 +85,16 @@ function testEncryptorPHPHashCompare() {
* Handy command for reproducing via CLI on different architectures:
* php -r "echo(base_convert(sha1('mypassword'), 16, 36));"
*/
- function testEncryptorLegacyPHPHashCompare() {
+ function testEncryptorLegacyPHPHashCheck() {
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
$amdHash = 'h1fj0a6m4o6k0sosks88oo08ko4gc4s';
$intelHash = 'h1fj0a6m4o0g04ocg00o4kwoc4wowws';
$wrongHash = 'h1fjxxxxxxxxxxxxxxxxxxxxxxxxxxx';
- $this->assertTrue($e->compare($amdHash, $intelHash));
- $this->assertFalse($e->compare($amdHash, $wrongHash));
+ $this->assertTrue($e->check($amdHash, "mypassword"));
+ $this->assertTrue($e->check($intelHash, "mypassword"));
+ $this->assertFalse($e->check($wrongHash, "mypassword"));
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.