Here is an article on password hashing, along with an implementation.
Is this code secure with number of iterations 10000, key length 256 and salt bytes 32?
Is there a rule-of-thumb for key length vs. salt bytes?
Regarding
key.destroy()andpbeKeySpec.clearPassword(), is it necessary to "manually" destroy the key and clear the password? If so, does the order matter (e.g. should the key be destroyed before the password is cleared or vice-versa)?Is it considered good practice to "zero out" password char array in the below example?
Are there vulnerabilities or incorrectly implemented sections in this code?
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import javax.xml.bind.DatatypeConverter;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.spec.InvalidKeySpecException;
/**
* Request for code review on Stack Exchange:
* https://codereview.stackexchange.com/questions/56939/secure-password-hashing-in-java
*
* Some reference pages from around the web:
* https://crackstation.net/hashing-security.htm
* https://owasp.org/index.php/Hashing_Java
* http://stackoverflow.com/questions/2267036/work-sun-misc-base64encoder-decoder-for-getting-byte
* http://stackoverflow.com/questions/2860943/suggestions-for-library-to-hash-passwords-in-java
* https://stackoverflow.com/questions/6464662/web-application-storing-a-password
* https://stackoverflow.com/questions/6126061/pbekeyspec-what-do-the-iterationcount-and-keylength-parameters-influence
* https://stackoverflow.com/questions/992019/java-256-bit-aes-password-based-encryption
*/
public abstract class PasswordDigester
{
private static final int NUM_ITERATIONS = 10000;
private static final int KEY_LENGTH = 256;
private static final int SALT_BYTES = 32;
/**
* Hash a plain text password into the format iterations:salt:hashed-password.
* @param newPassword A plain text password that needs to be hashed.
* @return a hashed password in the format iterations:salt:hashed-password.
*/
public static String getSaltedHash(String newPassword)
{
final byte[] nextRandomSalt = new byte[SALT_BYTES];
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
return getSaltedHash(NUM_ITERATIONS, nextRandomSalt, newPassword.toCharArray(), KEY_LENGTH);
}
/**
* Obtain the number of iterations and the salt from a previously hashed password and use them to salt- and
* hash a password candidate. Useful to validate a password.
* @param unhashedPasswordCandidate A password submitted by a client presumably to login to an application.
* @param previouslyHashedPassword The password on record whose iterations and salt will be used to hash
* the unhashed password candidate.
* @return a hashed password that can be validated for correctness against the previously hashed password.
*/
public static String getSaltedHash(String unhashedPasswordCandidate, String previouslyHashedPassword)
{
String[] params = previouslyHashedPassword.split(":");
int iterations = Integer.parseInt(params[0]);
byte[] randomizedSalt = DatatypeConverter.parseBase64Binary(params[1]);
return getSaltedHash(iterations, randomizedSalt, unhashedPasswordCandidate.toCharArray(), KEY_LENGTH);
}
private static String getSaltedHash(int iterations, byte[] randomizedSalt, char[] password, int keyLength)
{
try
{
PBEKeySpec pbeKeySpec = new PBEKeySpec(password, randomizedSalt, iterations, keyLength);
SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
SecretKey key = secretKeyFactory.generateSecret(pbeKeySpec);
String result =
iterations +
":" +
DatatypeConverter.printBase64Binary(randomizedSalt) +
":" +
DatatypeConverter.printBase64Binary(key.getEncoded());
// Note: key.destroy() always throws DestroyFailedException, so I've commented it out.
//key.destroy();
pbeKeySpec.clearPassword();
// For security, "zero out" password char array.
for(int i = 0; i < password.length; i++) { password[i] = '0'; }
return result;
}
catch(InvalidKeySpecException | NoSuchAlgorithmException e)
{
throw new IllegalStateException(e);
}
}
}
With the above settings this is generated (new lines after each colon):
10000:
pwYOF5KPSB+hbLVmuBxuVG0H12GReS+8A+UgSiWrpbM=:
63M+swkV4binnRKXQYgZJbJJtzT9asR0w0bJgytJZpg=
Any advice or words of caution or ways to secure or correct this code would be welcome.