From 9139f737b8c12587e637452a2a388a6c84a90201 Mon Sep 17 00:00:00 2001
From: Cam Spiers <cameron@heyday.co.nz>
Date: Thu, 14 Jun 2012 15:04:01 +1200
Subject: [PATCH] ENHANCEMENT: Added the ability to set a cost (the property
 was protected before and there were no setters and getters) and enforced the
 php requirements on the cost string used in the salt of crypt. Specifically,
 two digit from 04-31. Updated unit tests for blowfish algorithm to actually
 use the salt generation function and to test the newly implemented cost
 setting and getting functionality.

---
 security/PasswordEncryptor.php           | 32 +++++++++++++++++--
 tests/security/PasswordEncryptorTest.php | 40 ++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/security/PasswordEncryptor.php b/security/PasswordEncryptor.php
index 8a9dbfbae..d1cd2d1d1 100644
--- a/security/PasswordEncryptor.php
+++ b/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) {
diff --git a/tests/security/PasswordEncryptorTest.php b/tests/security/PasswordEncryptorTest.php
index 1c990bb5b..84ebd0fb1 100644
--- a/tests/security/PasswordEncryptorTest.php
+++ b/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() {