What do you think of my simple Java class that encrypts text into SHA-1?

Question

Anyway I'm starting out to make Java programs (a programming newb) and I've decide to do a simple utility class that converts text into SHA-1 strings. I'm planning on using this to encrypt passwords for databases. Feel free to comment and I'd love to learn from you guys. Here's my code:

/*
 * Utility class that encrypts a string created for storing passwords
 * into databases
 */

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

/**
 *
 * @author mamamambo
 */
public final class Encryptor {
    // Salt for extra security
    private static String salt = "54L7";

    private static String encrptionAlgorithm = "SHA-1";

    private Encryptor() { }

    public static String encrypt(String str) {
        StringBuffer encryptedResult = new StringBuffer();
        String strWithSalt = salt + str;

        try {
            MessageDigest md = MessageDigest.getInstance(encrptionAlgorithm);
            md.update(strWithSalt.getBytes());
            byte[] digest = md.digest();

            for (byte b : digest) {
                int number = b;
                // Convert negative numbers
                number = (number < 0) ? (number + 256) : number;
                encryptedResult.append(Integer.toHexString(number));
            }

        } catch (NoSuchAlgorithmException ex) {
            System.out.println(ex.toString());
        }

        return encryptedResult.toString();
    }
}

Answer 1

First of all, having a constant salt is just security through obscurity. You really should have a unique random salt for each hash.

Also (I'm not a Java expert but), why would md.digest() include negative numbers?

Answer 2

If you're using this to create hashed passwords why not just store the bytes directly in the database. The advantage is that you know the size of the field in advance. I also agree that if you're going to the effort of using salts then it makes sense to create and store a unique salt for each as this makes rainbow tables useless even if you know the salts (for example if your database and code becomes public). To create a random salt use code like this:

//declaracations at top of class
static Random srnd;
//static constructor
static {
    srnd = new SecureRandom(); 
}

//just before you need to use the salt
byte[] salt = new byte[20];
srnd.nextBytes(salt);

Note that you should create the SecureRandom object outside of the method and cache it because it takes a while to initialise. Also there is no need to append the bytes of the salt to the string before calling update, simply call update twice with the salt in the first call and the password to be hashed in the second.

Note that if you really do need the output as a string then I recommend using a Formatter to ensure that each number is 3 digits. Use code like this:

StringBuilder sb = new StringBuilder();
Formatter formatter = new Formatter(sb);
formatter.format("%03d", integerToOutput);
String output = sb.toString();

Answer 3

I agree with the other respondents about not having a hardcoded salt. If you have that and somebody knows what it is (say a disgruntled employee), then that person can pretty easily recover passwords using standard techniques (perhaps a newly created rainbow table built from that known salt).

While you can create a random salt for each user and store it with the user record, another approach is just to use some field that won't change, like the user's numeric ID. That way an attacker who acquires the user table must attack each user individually instead of attacking all at once (i.e. a single rainbow table can't hit the whole user table). Don't use an e-mail address since presumably that could change, which would make it impossible to authenticate the user.

Note however that once your user table is compromised, if the attacker knows the salt and hash scheme (what you're hashing, and the hash alg itself), then he can get passwords by starting with popular passwords and comparing the hashed/salted version against each user record.

I wrote a couple of articles about this topic if you are interested:

• http://wheelersoftware.com/articles/storing-passwords-securely-hash-salt.html
• http://wheelersoftware.com/articles/spring-security-hash-salt-passwords.html

Answer 4

Rather than add 256, use bit operations:

number &= 0xFF;

This will work ok because by that time, it's already been widened to an int. Or just

encryptedResult.append(Integer.toHexString((int)b & 0xFF));

the cast is sugar - the language rules force a cast anyway.

I like bit operations because the code says "chopping off the extra bits" rather than "relying on the effect of addition on numbers expressed in twos-complement notation".

In any case, this is still a bad idea: the numbers 0-15 will get converted to a one-byte-long string, creating ambiguities. The byte sequences 0xBB 0x0B and 0x0B 0xBB will get converted to the same thing.

Instead, use a base64 encoding. It's a well-understood industry standard, and somewhat more efficient than what you are doing.

Answer 5

Rather than rolling your own why don't you use jBCrypt. BCrypt is a tested and proven solution for storing passwords.

The great thing about jBCrypt, is that stores the salt in the result itself. Secondly jBCrypt is just one single Java file.

In, my project, I have just copied the file into my util directory.