Skip to content
Browse files

ENHANCEMENT: Added the ability to set a cost (the property was protec…

…ted 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.
  • Loading branch information...
1 parent e23a758 commit 9139f737b8c12587e637452a2a388a6c84a90201 Cam Spiers committed
Showing with 68 additions and 4 deletions.
  1. +30 −2 security/PasswordEncryptor.php
  2. +38 −2 tests/security/PasswordEncryptorTest.php
View
32 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 @@ function checkAEncryptionLevel() {
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) {
View
40 tests/security/PasswordEncryptorTest.php
@@ -64,13 +64,49 @@ function testEncryptorPHPHash() {
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() {

0 comments on commit 9139f73

Please sign in to comment.
Something went wrong with that request. Please try again.