[TASK] Compare password hashes in constant time 37/51737/11
authorChristian Futterlieb <christian@futterlieb.ch>
Sat, 18 Feb 2017 10:51:07 +0000 (11:51 +0100)
committerMarkus Klein <markus.klein@typo3.org>
Thu, 23 Feb 2017 17:21:42 +0000 (18:21 +0100)
In order to avoid time-based hash-based attacks, the native
PHP security functions are used instead of simple string
comparisons, when comparing passwords with hashes.

Change-Id: I0dbe2c12c5017f9d71ea7628ddd35d919510ac12
Releases: master
Resolves: #79888
Related: #79795
Reviewed-on: https://review.typo3.org/51737
Reviewed-by: Helmut Hummel <typo3@helhum.io>
Tested-by: Helmut Hummel <typo3@helhum.io>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mads L√łnne Jensen <mlj@systime.dk>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
typo3/sysext/saltedpasswords/Classes/Salt/Md5Salt.php
typo3/sysext/saltedpasswords/Classes/Salt/Pbkdf2Salt.php
typo3/sysext/saltedpasswords/Classes/Salt/PhpassSalt.php
typo3/sysext/saltedpasswords/Tests/Unit/Salt/BlowfishSaltTest.php
typo3/sysext/saltedpasswords/Tests/Unit/Salt/Md5SaltTest.php
typo3/sysext/saltedpasswords/Tests/Unit/Salt/Pbkdf2SaltTest.php
typo3/sysext/saltedpasswords/Tests/Unit/Salt/PhpassSaltTest.php

index fc7ba6a..0543b43 100644 (file)
@@ -82,7 +82,7 @@ class Md5Salt extends AbstractSalt implements SaltInterface
     {
         $isCorrect = false;
         if ($this->isValidSalt($saltedHashPW)) {
-            $isCorrect = crypt($plainPW, $saltedHashPW) == $saltedHashPW;
+            $isCorrect = \password_verify($plainPW, $saltedHashPW);
         }
         return $isCorrect;
     }
index 3f965c0..5578472 100644 (file)
@@ -108,7 +108,7 @@ class Pbkdf2Salt extends AbstractSalt implements SaltInterface
      */
     public function checkPassword($plainPW, $saltedHashPW)
     {
-        return $this->isValidSalt($saltedHashPW) && $this->getHashedPassword($plainPW, $saltedHashPW) === $saltedHashPW;
+        return $this->isValidSalt($saltedHashPW) && \hash_equals($this->getHashedPassword($plainPW, $saltedHashPW), $saltedHashPW);
     }
 
     /**
index 3b538bf..89d93cb 100644 (file)
@@ -125,7 +125,7 @@ class PhpassSalt extends AbstractSalt implements SaltInterface
     public function checkPassword($plainPW, $saltedHashPW)
     {
         $hash = $this->cryptPassword($plainPW, $saltedHashPW);
-        return $hash && $saltedHashPW === $hash;
+        return $hash && \hash_equals($hash, $saltedHashPW);
     }
 
     /**
index d9b4275..c1c98e5 100644 (file)
@@ -134,6 +134,33 @@ class BlowfishSaltTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     }
 
     /**
+     * Tests authentication procedure with fixed password and fixed (pre-generated) hash.
+     *
+     * Checks if a "plain-text password" is every time mapped to the
+     * same "salted password hash" when using the same fixed salt.
+     *
+     * @test
+     */
+    public function authenticationWithValidAlphaCharClassPasswordAndFixedHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$2a$07$Rvtl6CyMhR8GZGhHypjwOuydeN0nKFAlgo1LmmGrLowtIrtkov5Na';
+        $this->assertTrue($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
+     * Tests that authentication procedure fails with broken hash to compare to
+     *
+     * @test
+     */
+    public function authenticationFailsWithBrokenHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$2a$07$Rvtl6CyMhR8GZGhHypjwOuydeN0nKFAlgo1LmmGrLowtIrtkov5N';
+        $this->assertFalse($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
      * Tests authentication procedure with alphabet characters.
      *
      * Checks if a "plain-text password" is every time mapped to the
index 96959e5..0a09ed4 100644 (file)
@@ -118,6 +118,33 @@ class Md5SaltTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     }
 
     /**
+     * Tests authentication procedure with fixed password and fixed (pre-generated) hash.
+     *
+     * Checks if a "plain-text password" is every time mapped to the
+     * same "salted password hash" when using the same fixed salt.
+     *
+     * @test
+     */
+    public function authenticationWithValidAlphaCharClassPasswordAndFixedHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$1$GNu9HdMt$RwkPb28pce4nXZfnplVZY/';
+        $this->assertTrue($this->objectInstance->checkPassword($password, $saltedHashPassword), $this->getWarningWhenMethodUnavailable());
+    }
+
+    /**
+     * Tests that authentication procedure fails with broken hash to compare to
+     *
+     * @test
+     */
+    public function authenticationFailsWithBrokenHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$1$GNu9HdMt$RwkPb28pce4nXZfnplVZY';
+        $this->assertFalse($this->objectInstance->checkPassword($password, $saltedHashPassword), $this->getWarningWhenMethodUnavailable());
+    }
+
+    /**
      * Tests authentication procedure with alphabet characters.
      *
      * Checks if a "plain-text password" is every time mapped to the
index 59292a3..4b734f3 100644 (file)
@@ -121,6 +121,33 @@ class Pbkdf2SaltTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     }
 
     /**
+     * Tests authentication procedure with fixed password and fixed (pre-generated) hash.
+     *
+     * Checks if a "plain-text password" is every time mapped to the
+     * same "salted password hash" when using the same fixed salt.
+     *
+     * @test
+     */
+    public function authenticationWithValidAlphaCharClassPasswordAndFixedHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$pbkdf2-sha256$1000$woPhT0yoWm3AXJXSjuxJ3w$iZ6EvTulMqXlzr0NO8z5EyrklFcJk5Uw2Fqje68FfaQ';
+        $this->assertTrue($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
+     * Tests that authentication procedure fails with broken hash to compare to
+     *
+     * @test
+     */
+    public function authenticationFailsWithBrokenHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$pbkdf2-sha256$1000$woPhT0yoWm3AXJXSjuxJ3w$iZ6EvTulMqXlzr0NO8z5EyrklFcJk5Uw2Fqje68Ffa';
+        $this->assertFalse($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
      * Tests authentication procedure with alphabet characters.
      *
      * Checks if a "plain-text password" is every time mapped to the
index 0df190f..4772385 100644 (file)
@@ -118,6 +118,33 @@ class PhpassSaltTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     }
 
     /**
+     * Tests authentication procedure with fixed password and fixed (pre-generated) hash.
+     *
+     * Checks if a "plain-text password" is every time mapped to the
+     * same "salted password hash" when using the same fixed salt.
+     *
+     * @test
+     */
+    public function authenticationWithValidAlphaCharClassPasswordAndFixedHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$P$C7u7E10SBEie/Jbdz0jDtUcWhzgOPF.';
+        $this->assertTrue($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
+     * Tests that authentication procedure fails with broken hash to compare to
+     *
+     * @test
+     */
+    public function authenticationFailsWithBrokenHash()
+    {
+        $password = 'password';
+        $saltedHashPassword = '$P$C7u7E10SBEie/Jbdz0jDtUcWhzgOPF';
+        $this->assertFalse($this->objectInstance->checkPassword($password, $saltedHashPassword));
+    }
+
+    /**
      * Tests authentication procedure with alphabet characters.
      *
      * Checks if a "plain-text password" is every time mapped to the