diff --git a/UserModule.php b/UserModule.php index 58d9783..47f74cd 100644 --- a/UserModule.php +++ b/UserModule.php @@ -160,16 +160,28 @@ public static function t($str='',$params=array(),$dic='user') { /** * @return hash string. */ - public static function encrypting($string="") { + public static function encrypting($string="",$salt=null) { $hash = Yii::app()->getModule('user')->hash; - if ($hash=="md5") - return md5($string); - if ($hash=="sha1") - return sha1($string); - else - return hash($hash,$string); + + switch($hash) + { + case "md5": + return md5($string); + case "sha1": + return sha1($string); + case "blowfish": + if(!$salt) { + Yii::import('user.extensions.Randomness.Randomness'); + $salt = Randomness::blowfishSalt(); + } + return crypt($string,$salt); + default: + return hash($hash,$string); + + } } + /** * @param $place * @return boolean diff --git a/components/UserIdentity.php b/components/UserIdentity.php index 6f8986c..5750ba4 100644 --- a/components/UserIdentity.php +++ b/components/UserIdentity.php @@ -26,13 +26,15 @@ public function authenticate() } else { $user=User::model()->notsafe()->findByAttributes(array('username'=>$this->username)); } + if($user===null) if (strpos($this->username,"@")) { $this->errorCode=self::ERROR_EMAIL_INVALID; } else { $this->errorCode=self::ERROR_USERNAME_INVALID; } - else if(Yii::app()->getModule('user')->encrypting($this->password)!==$user->password) + //Extra password parameter is for blowfish crypt + else if(Yii::app()->getModule('user')->encrypting($this->password,$user->password)!==$user->password) $this->errorCode=self::ERROR_PASSWORD_INVALID; else if($user->status==0&&Yii::app()->getModule('user')->loginNotActiv==false) $this->errorCode=self::ERROR_STATUS_NOTACTIV; @@ -43,6 +45,7 @@ public function authenticate() $this->username=$user->username; $this->errorCode=self::ERROR_NONE; } + return !$this->errorCode; } diff --git a/controllers/LoginController.php b/controllers/LoginController.php index 4c19e22..50e78c8 100644 --- a/controllers/LoginController.php +++ b/controllers/LoginController.php @@ -22,6 +22,7 @@ public function actionLogin() $this->redirect(Yii::app()->controller->module->returnUrl); else $this->redirect(Yii::app()->user->returnUrl); + } } // display the login form diff --git a/controllers/RegistrationController.php b/controllers/RegistrationController.php index 45bc5f9..6042c8b 100644 --- a/controllers/RegistrationController.php +++ b/controllers/RegistrationController.php @@ -41,10 +41,10 @@ public function actionRegistration() { { $soucePassword = $model->password; $model->activkey=UserModule::encrypting(microtime().$model->password); - $model->password=UserModule::encrypting($model->password); - $model->verifyPassword=UserModule::encrypting($model->verifyPassword); $model->superuser=0; $model->status=((Yii::app()->controller->module->activeAfterRegister)?User::STATUS_ACTIVE:User::STATUS_NOACTIVE); + $model->password=UserModule::encrypting($model->password); + $model->verifyPassword =UserModule::encrypting($model->verifyPassword,$model->password); if ($model->save()) { $profile->user_id=$model->id; diff --git a/extensions/Randomness/.gitignore b/extensions/Randomness/.gitignore new file mode 100644 index 0000000..723ef36 --- /dev/null +++ b/extensions/Randomness/.gitignore @@ -0,0 +1 @@ +.idea \ No newline at end of file diff --git a/extensions/Randomness/README.mkd b/extensions/Randomness/README.mkd new file mode 100644 index 0000000..af0f987 --- /dev/null +++ b/extensions/Randomness/README.mkd @@ -0,0 +1,162 @@ +Randomness +===== + +The static class Randomness is a collection of helper methods for web apps that need random data for security purposes. + +Cryptographically-secure random data +------------------------------------ + +Applications may require Cryptographically Secure (CS) random data +[Wikipedia CSPRNG](http://en.wikipedia.org/wiki/Cryptographically_secure_pseudorandom_number_generator) +to be used in forming, for example, encryption keys, random passwords, session keys, +stream initialization vectors, nonces, secure unique IDs, and some kinds of salts. + +PHP's `mt_rand()` is a simple pseudo-random number gnerator designed +for use in Monte Carlo simulations, not in security systems. It is not +cryptographically secure. You can determine the next random +number from previous ones or from knowing the internal state of the generator. + +Most operating systems on which PHP typically runs provide a CSPRNG as a service to +applications. On Windows it is called +[CryptGenRandom](http://msdn.microsoft.com/en-us/library/aa379942.aspx). +On Linux, OS X, FreeBSD etc. +applications may read the /dev/random pseudo-device. Each of these OSs also offers a +way for the user to query the status of the CSPRNG. But in PHP, accessing the CSPRNG +can be problematic. + +`Randomness::randomBytes` uses several different approaces to read from +the operating system's CSPRNG. It is possible that all of them may fail. In this +case it has an option to get data from the http://www.random.org service and another +option to fall back on its own non-crypto-secure generator. + +Storing passwords in web apps +----------------------------- + +There are many tutorials and examples that show storage of passwords in a table. +Often the methods used are substandard and very easy to crack. For example, the +["Agile Web Application Development with Yii1.1 and PHP5"](http://www.yiiframework.com/doc/) book's example stores `md5($password)` in the DB and calls it +"encryption". It is not.[ "The Yii Blog Tutorial"](http://www.yiiframework.com/doc/blog/1.1/en/prototype.auth) is a little better in +that it uses a salt but it still uses md5 and is easy to crack. Examples of the same errors abound. The [yii-user](http://www.yiiframework.com/extension/yii-user) and [yii-user-management](http://www.yiiframework.com/extension/yii-user-management) are similarly insecure. + +You cannot rely on a user to use a (practically) unguessable password or to not +use that password in systems other than yours. And you should not assume that +your server is so secure that an attacker cannot get hold of the password file or a backup of it. + +A very common error I see in what I read and other people's code is fast hashes. MD5, for example, is very fast. As of Nov +2011 you can check 350 million keys per second on a commodity nVidia processor. +So no matter what you do with salts, the combination of short passwords and fast +brute force checking means your system is open to intruders if you rely on a +non-iterated message digest such as MD5, any of the SHA algos and most of the rest. + +The Blowfish hash function is currently considered pretty good. It is designed to be slow. The +implementation in PHP's `crypt()` is easy to use. Set a cost parameter high enough +to make a brute force attack really slow. I set it so that it takes about 250 ms +on the production server (a completely arbitrary choice:-). + +Each password should have its own random salt. The salt's purpose is to make the +dictionary size in a [rainbow table](http://en.wikipedia.org/wiki/Rainbow_table) or [dictionary attack](http://en.wikipedia.org/wiki/Dictionary_attack) so large that the attack is not +feasible. Salts used with the Blowfish hash do not need to be +cryptographically secure random strings so Randomness's salt generator by default +uses the cass's own pseudo-random generator. + +Some people advocate resalting every time a user logs in. I think this is only +useful if you also limit the time interval between user logins, e.g. block an +account if the user hasn't logged in in more than *N* weeks. + + +Using PHP's crypt() to store passwords +-------------------------------------- + +People often get confused about how to use implement a password store using `crypt()`. +It is actually very simple but it helps to know that: + +* It is safe to store the salt together with the password hash. An attacker cannot use +it to make a dictionary attack easier. + +* The string `crypt()` returns is the concatenation of the salt you give it and the +hash value. + +* `crypt()` ignores excess characters in the input salt string. + +`crypt()` has function signature `string crypt (string $str, string $salt)` and the +salt string format determines the hash method. For Blowfish hashing, the format is: +`"$2a$"`, a two digit cost parameter, `"$"`, and 22 digits from the alphabet +`"./0-9A-Za-z"`. The cost must be between `04` and `31`. + + crypt('EgzamplPassword', '$2a$10$1qAz2wSx3eDc4rFv5tGb5t') + >> '$2a$10$1qAz2wSx3eDc4rFv5tGb5e4jVuld5/KF2Kpy.B8D2XoC031sReFGi' + +The first 29 characters are the same as the salt string. Anthing appended to the salt +string argument has no effect on the result: + + crypt('EgzamplPassword', '$2a$10$1qAz2wSx3eDc4rFv5tGb5t12345678901234567890') + >> '$2a$10$1qAz2wSx3eDc4rFv5tGb5e4jVuld5/KF2Kpy.B8D2XoC031sReFGi' + + crypt('EgzamplPassword', '$2a$10$1qAz2wSx3eDc4rFv5tGb5t$2a$10$1qAz2wSx3eDc4rFv5tGb5t') + >> '$2a$10$1qAz2wSx3eDc4rFv5tGb5e4jVuld5/KF2Kpy.B8D2XoC031sReFGi' + +And in particular, pass the value returned from `crypt()` back in as the salt argument: + + crypt('EgzamplPassword', '$2a$10$1qAz2wSx3eDc4rFv5tGb5e4jVuld5/KF2Kpy.B8D2XoC031sReFGi') + >> '$2a$10$1qAz2wSx3eDc4rFv5tGb5e4jVuld5/KF2Kpy.B8D2XoC031sReFGi' + + + +So we can use `crypt()` to authenticate a user by passing the hash value it +gave us previously back in as a salt when checking a password input. + +Example +------- + +Say we have a `user` table like this + + create table user ( + id int not null auto_increment primary key, + email varchar(255) not null, + password_hash char(64) not null, + unique key (email) + ) + +From a user account generation form assume that we have (already sanitized) user input in +`$form->email` and `$form->password`. We generate the hash: + + $password_hash = crypt($form->password, Randomness::blowfishSalt()); + +And insert a row into `user` containing `$form->email` and `$password_hash`. + +At user logon assume we again have sanitized user input in `$form->email` and `$form->password`. +To authenticate these against the accounts in `user` we select the `password_hash` field from table `user` where `email` = `$form->email` and, with that value in `$password_hash` + + if ($password_hash === crypt($form->password, $password_hash)) + // password is correct + else + // password is wrong + +So there is no need to store the salt in a separate column from the hash value because +`crypt()` conveniently keeps it in the same string as the hash. + +In Yii +------ + +`Randomness::blowfishSalt()` generates a salt to use with `crypt()`, for example somewhere in the controller action where you register new users: + + $user = new User; + $user->email = $form->email; + $user->bf_hash = crypt($form->password, Randomness::blowfishSalt()); + if ($user->save()) + ... + +To authenticate (refer to the authenticate method in protected/components/UserIdentity.php of a fresh yiic webapp): + + public function authenticate() { + $user = User::model()->findByAttributes(array( + 'email' => $this->username, + )); + if ($user === null + || crypt($this->password, $user->bf_hash) !== $user->bf_hash + ) { + $this->errorCode = self::ERROR_UNKNOWN_IDENTITY; + ... + } + ... + } diff --git a/extensions/Randomness/Randomness.php b/extensions/Randomness/Randomness.php new file mode 100644 index 0000000..4606694 --- /dev/null +++ b/extensions/Randomness/Randomness.php @@ -0,0 +1,227 @@ += $length + ) + return self::substr($s, 0, $length); + + // mcrypt_create_iv() with MCRYPT_RAND is not crypto-strong. With MCRYPT_DEV_URANDOM + // it can (on Linux) return non-crypto-strong result without warning, so don't use that. + if (function_exists('mcrypt_create_iv') + && false !== ($s = mcrypt_create_iv($length, MCRYPT_DEV_RANDOM)) + && self::strlen($s) >= $length + ) + return self::substr($s, 0, $length); + + // Try /dev/random directly. On Linux it may block so deal with that. + if (false !== ($f = @fopen('/dev/random', 'r')) + && stream_set_blocking($f, 0) + && false !== ($s = @fread($f, $length)) + && (fclose($f) || true) + && self::strlen($s) >= $length + ) + return self::substr($s, 0, $length); + + // Try (three times max) stealing entropy from the session manager. + $i = 0; + while ( + self::strlen($s) < $length + && false !== ($r = self::sessionBlock()) + && ++$i < 3 + ) + $s .= $r; + if (self::strlen($s) >= $length) + return self::substr($s, 0, $length); + + // Try http://random.org + if (self::strlen($s) < $length + && $http + && false !== ($r = @file_get_contents( + 'http://www.random.org/cgi-bin/randbyte?format=f&nbytes=' . $length + )) + && self::strlen($s .= $r) >= $length + ) + return self::substr($s, 0, $length); + + // No more sources for crypto-strong data available so + return false; + } + + // Use the wierd pseudo-random generator above + while (self::strlen($s) < $length) + $s .= self::pseudoRanBlock($cryptoStrong); + + return self::substr($s, 0, $length); + } + + /** + * Generate a random Blowfish salt for use in PHP's crypt(). + * + * @param $cost int cost parameter between 4 and 31 + * @param bool $cryptoStrong set to require crytoStrong randomness + * @return string salt starting $2a$ + */ + public static function blowfishSalt($cost = 10, $cryptoStrong = false) { + return + '$2a$' . str_pad($cost, 2, '0', STR_PAD_RIGHT) . '$' + . strtr( + substr(base64_encode(self::randomBytes(18, $cryptoStrong)), 0, 24), + array('+' => '.') + ); + } + + /** + * Generate a random ASCII string. + * + * Use only [0-9a-zA-z~.] which are all transparent in raw urlencoding. + * + * @param int $length length of the string in characters + * @param bool $cryptoStrong set to require crytoStrong randomness + * @return string the random string + */ + public static function randomString($length = 8, $cryptoStrong = true) { + return strtr( + self::substr( + base64_encode(self::randomBytes($length + 2, $cryptoStrong)), 0, $length + ), + array('+' => '_', '/' => '~') + ); + } + +} \ No newline at end of file diff --git a/messages/gb/user.php b/messages/gb/user.php new file mode 100644 index 0000000..ae0a35f --- /dev/null +++ b/messages/gb/user.php @@ -0,0 +1,13 @@ + 'Username', + 'password' => 'Password', + 'E-mail' => 'Email', + 'username or email' => 'Username or email', + 'Verification Code' => 'Verification code', + 'Retype Password' => 'Re-type password', + 'Retype Password is incorrect.' => 'Passwords do not match', + 'Lost Password?' => 'Lost password?', + 'Minimal password length 4 symbols.' => 'Minimum password length 4 characters', +); diff --git a/models/User.php b/models/User.php index 388c008..017c8d6 100644 --- a/models/User.php +++ b/models/User.php @@ -196,4 +196,5 @@ public function getLastvisit() { public function setLastvisit($value) { $this->lastvisit_at=date('Y-m-d H:i:s',$value); } + } \ No newline at end of file diff --git a/models/UserChangePassword.php b/models/UserChangePassword.php index e1256f5..6fa139a 100644 --- a/models/UserChangePassword.php +++ b/models/UserChangePassword.php @@ -39,7 +39,8 @@ public function attributeLabels() */ public function verifyOldPassword($attribute, $params) { - if (User::model()->notsafe()->findByPk(Yii::app()->user->id)->password != Yii::app()->getModule('user')->encrypting($this->$attribute)) + $user = User::model()->notsafe()->findByPk(Yii::app()->user->id); + if ($user->password != Yii::app()->getModule('user')->encrypting($this->$attribute,$user->password)) $this->addError($attribute, UserModule::t("Old Password is incorrect.")); } } \ No newline at end of file