Merge pull request #17 from ivarb/feat-salting

Fixed salting
AngryBytes · Jul 29, 2015 · f0f3af2 · f0f3af2
2 parents bfd9f54 + 3e518cc
commit f0f3af2
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 17 deletions.
diff --git a/src/AngryBytes/Hash/Hash.php b/src/AngryBytes/Hash/Hash.php
@@ -45,7 +45,6 @@ class Hash
      *
      * @param  HasherInterface $hasher
      * @param  string          $salt
-     * @return void
      **/
     public function __construct(HasherInterface $hasher, $salt)
     {
@@ -97,10 +96,10 @@ public function setSalt($salt)
     {
         // Make sure it's of sufficient length
         if (strlen($salt) < 20) {
-            throw new InvalidArgumentException('
-                Provided salt "' . $salt . '" is not long enough.
-                A minimum length of 20 characters is required
-            ');
+            throw new InvalidArgumentException(sprintf(
+                'Provided salt "%s" is not long enough. A minimum length of 20 characters is required',
+                $salt
+            ));
         }
 
         $this->salt = $salt;
@@ -168,7 +167,7 @@ public function shortHash()
      * @param  string $hash
      * @return bool
      **/
-    public function matchesShortHash($compareTo)
+    public function matchesShortHash()
     {
         // Full args to method
         $args = func_get_args();

diff --git a/src/AngryBytes/Hash/Hasher/Blowfish.php b/src/AngryBytes/Hash/Hasher/Blowfish.php
@@ -33,16 +33,16 @@ class Blowfish implements HasherInterface
     /**
      * Work factor for blowfish
      *
+     * Defaults to '15' (32768 iterations)
+     *
      * @var int
      **/
     private $workFactor = 15;
 
     /**
-     * Constructor
+     * Detect Blowfish support
      *
      * @throws RuntimeException
-     *
-     * @return void
      **/
     public function __construct()
     {
@@ -66,7 +66,7 @@ public function getWorkFactor()
     /**
      * Set the blowfish work factor
      *
-     * @param int $workFactor
+     * @param  int $workFactor
      * @return Blowfish
      */
     public function setWorkFactor($workFactor)
@@ -76,7 +76,7 @@ public function setWorkFactor($workFactor)
                 'Work factor needs to be greater than 3 and smaller than 32'
             );
         }
-        $this->workFactor = $workFactor;
+        $this->workFactor = (int) $workFactor;
 
         return $this;
     }
@@ -97,7 +97,7 @@ public function hash($data, $salt)
      * Generate a bcrypt salt from a string salt
      *
      * @param  string $salt
-     * @return string
+     * @return string       Format: "$2y$[workfactor]$[salt]$"
      **/
     private function bcryptSalt($salt)
     {
@@ -112,17 +112,22 @@ private function bcryptSalt($salt)
     }
 
     /**
-     * Get valid salt substr for blowfish
+     * Get valid salt string for Blowfish usage
      *
-     * Blowfish accepts 22 chars as a salt
-     *
-     * Will take a hash of $salt to take changes over 22 chars into account
+     * Blowfish accepts 22 chars (./0-9A-Za-z) as a salt if anything else is passed,
+     * this method will take a hash of $salt to transform it into 22 supported characters
      *
      * @param  string $salt
      * @return string
      **/
     private static function getSaltSubstr($salt)
     {
+        // Return salt when it is a valid Blowfish salt
+        if (preg_match('!^[\./0-9A-Za-z]{22}$!', $salt) === 1) {
+            return $salt;
+        }
+
+        // fallback to md5() to make the salt valid
         return substr(
             md5($salt),
             0, 22

diff --git a/src/AngryBytes/Hash/RandomString.php b/src/AngryBytes/Hash/RandomString.php
@@ -37,7 +37,7 @@ public static function generateBytes($bytes)
         }
 
         // Read the required number of bytes
-        $bytes =  fread($fp, $bytes);
+        $bytes = fread($fp, $bytes);
 
         // Close the file handle
         fclose($fp);

diff --git a/tests/AngryBytes/Hash/Test/BlowfishTest.php b/tests/AngryBytes/Hash/Test/BlowfishTest.php
@@ -126,6 +126,29 @@ public function testWorkFactor()
         );
     }
 
+    /**
+     * Test salting
+     **/
+    public function testSalt()
+    {
+        $hasher = $this->createHasher();
+        $hasher->getHasher()->setWorkFactor(5);
+
+        // Test salt with 22 valid characters
+        $this->assertEquals(
+            // Pre-generated hash outcome for password 'foo' and given salt
+            '$2y$05$./A1aaaaaaaaaaaaaaaaaOZW9OJaO6Alj4.ZDbOi6Jrbn.bGZfYRK',
+            $hasher->getHasher()->hash('foo', './A1aaaaaaaaaaaaaaaaaa')
+        );
+
+        // Test salt with less invalid characters
+        $this->assertEquals(
+            // Pre-generated hash outcome for password 'foo' and given salt (md5'ed)
+            '$2y$05$ceb20772e0c9d240c75ebugm2AOmnuR5.LsdpDZGAjkE1DupDTPFW',
+            $hasher->getHasher()->hash('foo', 'salt')
+        );
+    }
+
     /**
      * Create hasher
      *