Take the 2-minute tour ×
Information Security Stack Exchange is a question and answer site for Information security professionals. It's 100% free, no registration required.

I'm trying to write a secure code that can keep my site users passwords as safe as possible. Now I'm fairly new to encryption and would like some professional opinions and suggestions. As well as a raw estimate of how long it would take to brute force my current encryption method.

So my encryption begins with creating a random SALT value as such

$salt = dechex(mt_rand(0, 2147483647)) . dechex(mt_rand(0, 2147483647)); 

After the SALT value is create it is hashed using SHA-256 as such

$password = hash('sha256', $_POST['bindPass'] . $salt);

After the $password hash is create I re-hash the values to secure them farther

 for($round = 0; $round < 656757; $round++) 
 { 
     $password = hash('sha256', $password . $salt); 
 } 

Now after rehashing is done the Encryption is bound to a function and is added directly into PDO Insert query. Here is the Encryption function.

function encrypt_cpass($in)
{

    $iv_size = mcrypt_get_iv_size(MCRYPT_TWOFISH, MCRYPT_MODE_ECB);
    $iv = mcrypt_create_iv($iv_size, MCRYPT_RAND);
    $enc = mcrypt_encrypt(MCRYPT_TWOFISH,ENCRYPT_KEY, $in, MCRYPT_MODE_ECB, $iv);
    $enc = base64_encode($enc);
    return $enc;
 } 

And finally the PDO part

$query_params = array( 
    ':password' => encrypt_cpass($password),
            ..........
} 

I can't say I get bad performance with the following code and the script, but I am more worried that I may be doing the password securing not the right way and it may eventually be broken. So again if you have any suggestions or could point me to some good resources please post them.

Cheers.

share|improve this question
    
mt_rand is a very bad source of randomness. Don't invent your own mechanism; use bcrypt, stackoverflow.com/questions/4795385/… –  copy Aug 17 '13 at 16:29
    
Badly implemented, fairly quickly. Paying attention to the details, way too long. In your implementation, don't invoke Schneier's law. –  Fiasco Labs Aug 17 '13 at 17:11
1  
There are quite a lot of problems with this code. You should have a look at PHP's new password api password_hash(), there exists also a compatibility pack for earlier PHP versions. If you want more in-depth information, you may be interested in my tutorial about storing passwords. –  martinstoeckli Aug 17 '13 at 18:10

1 Answer 1

up vote 3 down vote accepted

There are a few things here. First and foremost, you don't need to encrypt a hash. The point of a hash is to protect the original password. Symmetric encryption on top of that doesn't add much since it is fairly likely that the key will be compromised if your servers are compromised anyway (which should be the only way that someone walks off with your user DB).

Second, if you are using a random IV but I don't see it being stored. I'm not that familiar with mcrypt specifically, so forgive me if it takes care of storing the IV in some format with the encrypted output, but if you don't store the IV in some way, decryption will be impossible.

Third, use a secure, slow cryptographic hash like bcrypt rather than SHA256 and you will get much further security from the hash itself. Speed in hashing is bad and encryption is unnecessary and ineffective. Use a slow enough hash that generating rainbow tables is cost prohibitive but verification is not and you should be fine.

share|improve this answer
    
So what would suggest to do as a secure alternative to my current code? The decryption part works fine for the password. But the passwords can never be decrypted to there hashed or plain-text value since as of moment its only one-way. –  29x29 Aug 17 '13 at 16:14
1  
I think AJs point is that there should never be a need to decrypt the password. –  BrianAdkins Aug 17 '13 at 17:08

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.