4

these are some functions I am using for password encryption and password verification. Was wondering if this is a good way to handle it. I am using the codeigniter framework.

This is the function to 'encrypt' :

function crypt_pass( $input ){ $salt = substr(sha1(date('r')), rand(0, 17), 22); $cost = 10; $hash = '$2y$' . $cost . '$' . $salt; $pw_and_salt['pw'] = crypt($input, "$hash"); $pw_and_salt['salt'] = $salt; return $pw_and_salt; }

I store both the password and the salt in my DB. Here is the login function:

function login(){ $this->db->select('salt'); $salt = $this->db->get_where('users', array('username' => $this->input->post('username') ) )->row(); $where = array( 'username' => $this->input->post('username'), 'password' => crypt( $this->input->post('password'), '$2y$10$' . $salt->salt), ); $user = $this->db->get_where('users', $where)->first_row(); if (!$user) { return FALSE; }else{ if(!empty($user->activation)){ return 2; }else if($user && empty($user->activation)){ $this->session->set_userdata('id',$user->id); $this->session->set_userdata('username',$user->username); $this->session->set_userdata('first_name',$user->first_name); return 1; } } }

Am I implementing this the right way? Is this enough security?

VERSION 2 : NOT STORING SALT, EXTRACTING FROM PASSWORD IN DB INSTEAD :

function login(){ $this->db->select('password'); $pw = $this->db->get_where('users', array('username' => $this->input->post('username') ) )->row(); $where = array( 'username' => $this->input->post('username'), 'password' => crypt( $this->input->post('password'), $pw->password), ); $user = $this->db->get_where('users', $where)->first_row(); if (!$user) { return FALSE; }else{ if(!empty($user->activation)){ return 2; }else if($user && empty($user->activation)){ $this->session->set_userdata('id',$user->id); $this->session->set_userdata('username',$user->username); $this->session->set_userdata('first_name',$user->first_name); return 1; } } }
Improve this question
4
  • 1
    I think this question would be better on the code review stack. Commented Dec 4, 2013 at 6:04
  • Also see Openwall's PHP password hashing framework (PHPass). Its portable and hardened against a number of common attacks on user passwords. The guy who wrote the framework (SolarDesigner) is the same guy who wrote John The Ripper and sits as a judge in the Password Hashing Competition. So he knows a thing or two about attacks on passwords. Commented Oct 12, 2014 at 1:51
  • This question appears to belong on another site in the Stack Exchange network. Perhaps you should try Code Review Stack Exchange. Commented Oct 12, 2014 at 1:54
  • $pw_and_salt['pw'] = crypt($input, "$hash"); why $hash is in quotes? It should not be, right? Commented Sep 27, 2020 at 17:35

1 Answer 1

Reset to default
6

There are some points that can be improved, but first i would recommend to use PHP's new function password_hash(). This function will generate a safe salt and includes it in the resulting hash-value, so you can store it in a single database field. There exists also a compatibility pack for earlier versions.

// Hash a new password for storing in the database. // The function automatically generates a cryptographically safe salt. $hashToStoreInDb = password_hash($password, PASSWORD_BCRYPT); // Check if the hash of the entered login password, matches the stored hash. // The salt and the cost factor will be extracted from $existingHashFromDb. $isPasswordCorrect = password_verify($password, $existingHashFromDb);

Some thoughts about your code:

  1. You generate a BCrypt hash with crypt(), so the salt will be part of the resulting hash. There is no need to store it separately.
  2. The generation of the salt can be improved, use the random source of the operating system MCRYPT_DEV_URANDOM.
  3. If you would change the cost factor to 9, the format would become invalid, because crypt expects two digits.
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Hi Martin, thanks for your input. I will certainly be using your suggestions. One question on the current setup, if I am not mistaken I can only verify an entered password by hashing it in the same way I hashed the password stored in the DB, how can I do this without actaully storing and retrieving the salt?
I added my approach for validation without a salt from DB to the original post.
@Jursels - The salt is already included in the resulting hash-value, and the password_verify() function will extract it from there, together with the cost factor. Example: the hash-value $2y$10$nOUIs5kJ7naTuTFkBy1veuK0kSxUFXfuaOKdOKf9xYT0KKIGSJwFa contains the salt after the third $, the salt is nOUIs5kJ7naTuTFkBy1veu.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.