Merge pull request #527 from camspiers/9139f737b8c12587e637452a2a388a6c84a90201

Added ability to set the cost of the blowfish crypt algorithm.

security/PasswordEncryptor.php

@@ -142,10 +142,35 @@ class PasswordEncryptor_Blowfish extends PasswordEncryptor {
 	 * 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.
+	 * The two digit cost parameter is the base-2 logarithm of the iteration 
+	 * count for the underlying Blowfish-based hashing algorithmeter and must 
+	 * be in range 04-31, values outside this range will cause crypt() to fail.
 	 */
 	protected static $cost = 10;
 
+	/**
+	 * Sets the cost of the blowfish algorithm.
+	 * See {@link PasswordEncryptor_Blowfish::$cost}
+	 * Cost is set as an integer but 
+	 * Ensure that set values are from 4-31
+	 * 
+	 * @param int $cost range 4-31
+	 * @return null
+	 */
+	public static function set_cost($cost) {
+		self::$cost = max(min(31, $cost), 4);
+	}
+
+	/**
+	 * Gets the cost that is set for the blowfish algorithm
+	 * 
+	 * @param int $cost
+	 * @return null
+	 */
+	public function get_cost() {
+		return self::$cost;
+	}
+
 	function encrypt($password, $salt = null, $member = null) {
 		// See: http://nz.php.net/security/crypt_blowfish.php
 		// There are three version of the algorithm - y, a and x, in order
@@ -246,9 +271,12 @@ class PasswordEncryptor_Blowfish extends PasswordEncryptor {
 		return 'unknown';
 	}
 
+	/**
+	 * self::$cost param is forced to be two digits with leading zeroes for ints 4-9
+	 */
 	function salt($password, $member = null) {
 		$generator = new RandomGenerator();
-		return self::$cost . '$' . substr($generator->generateHash('sha1'), 0, 22);
+		return sprintf('%02d', self::$cost) . '$' . substr($generator->generateHash('sha1'), 0, 22);
 	}
 
 	function check($hash, $password, $salt = null, $member = null) {

tests/security/PasswordEncryptorTest.php

@@ -64,13 +64,49 @@ class PasswordEncryptorTest extends SapphireTest {
 	function testEncryptorBlowfish() {
 		Config::inst()->update('PasswordEncryptor', 'encryptors', array('test_blowfish'=>array('PasswordEncryptor_Blowfish'=>'')));
 		$e = PasswordEncryptor::create_for_algorithm('test_blowfish');
+		
 		$password = 'mypassword';
-		$salt = '10$mysaltmustbetwen2chars';
+
+		$salt = $e->salt($password);
+		$modSalt = substr($salt, 0, 3) . str_shuffle(substr($salt, 3, strlen($salt)));
 
 		$this->assertTrue($e->checkAEncryptionLevel() == 'y' || $e->checkAEncryptionLevel() == 'x' || $e->checkAEncryptionLevel() == 'a');
 		$this->assertTrue($e->check($e->encrypt($password, $salt), "mypassword", $salt));
 		$this->assertFalse($e->check($e->encrypt($password, $salt), "anotherpw", $salt));
-		$this->assertFalse($e->check($e->encrypt($password, $salt), "mypassword", '10$anothersaltetwen2chars'));
+		$this->assertFalse($e->check($e->encrypt($password, $salt), "mypassword", $modSalt));
+
+		PasswordEncryptor_Blowfish::set_cost(1);
+		$salt = $e->salt($password);
+		$modSalt = substr($salt, 0, 3) . str_shuffle(substr($salt, 3, strlen($salt)));
+
+		$this->assertNotEquals(1, PasswordEncryptor_Blowfish::get_cost());
+		$this->assertEquals(4, PasswordEncryptor_Blowfish::get_cost());
+
+		$this->assertTrue($e->check($e->encrypt($password, $salt), "mypassword", $salt));
+		$this->assertFalse($e->check($e->encrypt($password, $salt), "anotherpw", $salt));
+		$this->assertFalse($e->check($e->encrypt($password, $salt), "mypassword", $modSalt));
+
+		PasswordEncryptor_Blowfish::set_cost(15);
+		$salt = $e->salt($password);
+		$modSalt = substr($salt, 0, 3) . str_shuffle(substr($salt, 3, strlen($salt)));
+
+		$this->assertEquals(15, PasswordEncryptor_Blowfish::get_cost());
+
+		$this->assertTrue($e->check($e->encrypt($password, $salt), "mypassword", $salt));
+		$this->assertFalse($e->check($e->encrypt($password, $salt), "anotherpw", $salt));
+		$this->assertFalse($e->check($e->encrypt($password, $salt), "mypassword", $modSalt));
+
+
+		PasswordEncryptor_Blowfish::set_cost(35);
+
+		$this->assertNotEquals(35, PasswordEncryptor_Blowfish::get_cost());
+		$this->assertEquals(31, PasswordEncryptor_Blowfish::get_cost());
+
+		//Don't actually test this one. It takes too long. 31 takes too long to process
+		// $salt = $e->salt($password);
+		// $this->assertTrue($e->check($e->encrypt($password, $salt), "mypassword", $salt));
+		// $this->assertFalse($e->check($e->encrypt($password, $salt), "anotherpw", $salt));
+
 	}
 	
 	function testEncryptorPHPHashCheck() {