CODEWITHSUNDEEP
javasecuritycryptographysalt-cryptographysha512
Aurum
Aurum

Reputation: 35

Is my implementation of Salting and SHA-512 hashing passwords correct/secure?

Is my implementation of Salting and SHA-512 hashing passwords correct/secure?

I know that I should iterate the algorithm a couple of times on the salted password. Other than that what more should i implement to make it secure or is this enough?

        public static String[] SHA512(String password)
{
    //Generates the salt
    SecureRandom saltRandomizer = new SecureRandom();
    byte[] salt = new byte[64]; //The same size as the output of SHA-512 (512 bits = 64 bytes)
    saltRandomizer.nextBytes(salt);
    String encodedSalt = Base64.encodeToString(salt, Base64.DEFAULT);


    //Prepends the salt to the password
    String saltedPassword = encodedSalt + password;

    //Hashed the salted password using SHA-512
    MessageDigest digester;
    byte[] digest = null;
    try {
        digester = MessageDigest.getInstance("SHA-512");
        digester.reset();
        digester.update(saltedPassword.getBytes());
        digest = digester.digest();
    } catch (NoSuchAlgorithmException e) {
        System.out.println("No such algorithm");
        e.printStackTrace();
    }

    String[] passwordPlusSalt = new String[2];
    passwordPlusSalt[0] = Base64.encodeToString(digest, Base64.DEFAULT);
    passwordPlusSalt[1] = encodedSalt;
    return passwordPlusSalt;
}

Thanks in advance

Upvotes: 2

Views: 6370

Answers (2)

Guy Bouallet
Guy Bouallet

Reputation: 2125

I think that taking action to hash passwords with a salt is a good step for securing your application and accessing higher security levels. However, there are problems with your implementation:

  • not using a seed for random bytes generation
  • not keeping track of the generated random bytes to use them for password verification
  • using string concatenation does not add algorithmic complexity but looks like security by obscurity, which has been proven to be inefficient in security critical environments.

Instead, use a standard HMAC algorithm specifically designed for your need. A list of algorithms supported by java 7 is available here. PBKDF2WithHmacSHA1 is a good choice. It can simply be used like this:

PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, bytes * 8);
SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
byte[] secretPassword = skf.generateSecret(spec).getEncoded();

A complete working code can be found here. It should limit the impact of a brute force/dictionnary attack, if your password database falls in wrong hands.

Note that the salt is a critical ring. Depending on your security requirements, you can store it on a separate filesystem, type or load it at server startup. There can even be specialized (and generally expensive) Hardware Security Modules which can keep it in safety.

Upvotes: 2

hoipolloi
hoipolloi

Reputation: 8044

No. It is not secure. Take a look at this line:

SecureRandom saltRandomizer = new SecureRandom();

I notice you don't specify a PRNG or provider. I'm no expert but I understand the SUN CSP default is to use sun.security.provider.NativePRNG on Solaris and Linux, which simply provides the output of /dev/urandom which may (or may not) be suitable for your needs (it is not, for example, recommended for the generation of cryptographic keys).

It is therefore recommended to always specify a PRNG and provider, as follows:

SecureRandom.getInstance("SHA1PRNG", "SUN");

Furthermore, it is advisable to:

  • Periodically throw away the existing java.security.SecureRandom instance and create a new one. This will generate a new instance with a new seed.
  • Periodically add new random material to the PRNG seed by making a call to java.security.SecureRandom.setSeed(java.security.SecureRandom.generateSeed(int)).

The point I'm making is that security is a complex topic that is so easy to get wrong. Don't try and homebrew a solution. Use an existing library. To quote Thomas Pornin's excellent answer:

Complexity is bad. Homemade is bad. New is bad.

Sources:

Upvotes: 6

Related Questions