AES with password based SecretKeySpec vs PBE

Question

After reading through several stackoverflow questions about implementing AES I think I'm starting to understand the basics:

• Every single time I should generate a new IV
• When using PBE the iteration count should be around 1000-4000+
• Since I can't predict the amount of data to be encrypted I shouldn't use ECB cipher mode

My environment is quite simple:

• the passphrase should be secure, it's a securerandom generated random 32 character at the moment (i.e. not set by a user).
• the generated encrypted content may end up being stored as cookies, so they are somewhat public

Based on these I came up with the following Java code:

public class SecureEncryption {

private static final String CONTENT = "thisneedstobestoredverysecurely";
private static final String PASSPHRASE = "mysuperstrongpassword";
private static final int IV_LENGTH = 16;

public static void main(String[] args) throws Exception {
    MessageDigest digest = MessageDigest.getInstance("SHA-1");
    byte[] passphrase = digest.digest(PASSPHRASE.getBytes("UTF-8"));

    Cipher instance = Cipher.getInstance("AES/CFB/NoPadding");
    passphrase = Arrays.copyOf(passphrase, 16);
    SecretKeySpec secretKey = new SecretKeySpec(passphrase, "AES");
    byte[] iv = new byte[16];
    SecureRandom sr = new SecureRandom();
    sr.nextBytes(iv);
    instance.init(Cipher.ENCRYPT_MODE, secretKey, new IvParameterSpec(iv));
    byte[] encrypted = instance.doFinal(CONTENT.getBytes("UTF-8"));
    byte[] result = addIVtoEncrypted(iv, encrypted);

    System.arraycopy(result, 0, iv, 0, IV_LENGTH);
    System.arraycopy(result, IV_LENGTH, encrypted, 0, result.length - IV_LENGTH);
    instance.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(iv));
    byte[] decrypted = instance.doFinal(encrypted);
    System.out.println(new String(decrypted, "UTF-8"));
}

private static byte[] addIVtoEncrypted(byte[] iv, byte[] encrypted) {
    byte[] ret = new byte[IV_LENGTH + encrypted.length];
    System.arraycopy(iv, 0, ret, 0, IV_LENGTH);
    System.arraycopy(encrypted, 0, ret, IV_LENGTH, encrypted.length);
    return ret;
}
}

While this works fine, I'm not sure if it's as secure as it can get.. I'm kind of lost at the moment regarding the following things:

• is PBE using AES+SHA more secure? Does the salt+iteration count adds considerably to security? Which exact combination of PBE should I use if so?
• Should I instead consider using salt for the encryptable content (rather than for the PBE key)?
• If using salt for the content, what is preferred: one static value, or different values, but appended/prepended to the encrypted result (just as it is done with IV)?

UPDATE: based on the recommendations received here I rewrote my implementation:

public class SecureEncryption {

private static final String CONTENT = "thisneedstobestoredverysecurely";
private static final String PASSPHRASE = "mysuperstrongpassword";
private static final int IV_LENGTH = 16;
private static final int AES_KEY_LENGTH = 16;
private static final int MAC_KEY_LENGTH = 16;
private static final int MAC_LENGTH = 20;
private static final int ITERATION_COUNT = 4096;
private static final String AES = "AES";
private static final String CIPHER_ALGORITHM = "AES/CFB/NoPadding";
private static final String SECRET_KEY_ALGORITHM = "PBKDF2WithHmacSHA1";
private static final String MAC_ALGORITHM = "HmacSHA1";

public static void main(String[] args) throws Exception {
    Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
    SecureRandom sr = new SecureRandom();
    byte[] salt = new byte[16];
    sr.nextBytes(salt);

    SecretKeyFactory factory = SecretKeyFactory.getInstance(SECRET_KEY_ALGORITHM);
    SecretKey secretKey = factory.generateSecret(new PBEKeySpec(PASSPHRASE.toCharArray(), salt, ITERATION_COUNT, 256));
    byte[] secretBytes = secretKey.getEncoded();

    byte[] iv = new byte[16];
    sr.nextBytes(iv);
    cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(secretBytes, 0, AES_KEY_LENGTH, AES), new IvParameterSpec(iv));
    byte[] encrypted = cipher.doFinal(CONTENT.getBytes("UTF-8"));
    byte[] result = concatArrays(iv, encrypted);

    byte[] macResult = getMAC(secretBytes, result);
    result = concatArrays(macResult, result);

    System.arraycopy(result, 0, macResult, 0, MAC_LENGTH);
    System.arraycopy(result, MAC_LENGTH, iv, 0, IV_LENGTH);
    System.arraycopy(result, MAC_LENGTH + IV_LENGTH, encrypted, 0, result.length - IV_LENGTH - MAC_LENGTH);

    if (!Arrays.equals(getDigest(getMAC(secretBytes, concatArrays(iv, encrypted))), getDigest(macResult))) {
        System.out.println("Invalid MAC");
    }
    cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(secretBytes, 0, AES_KEY_LENGTH, AES), new IvParameterSpec(iv));
    byte[] decrypted = cipher.doFinal(encrypted);
    System.out.println(new String(decrypted, "UTF-8"));
}

private static byte[] getDigest(byte[] mac) throws Exception {
    MessageDigest digest = MessageDigest.getInstance("SHA1");
    return digest.digest(mac);
}

private static byte[] getMAC(byte[] secretBytes, byte[] data) throws Exception {
    Mac mac = Mac.getInstance(MAC_ALGORITHM);
    mac.init(new SecretKeySpec(secretBytes, AES_KEY_LENGTH, MAC_KEY_LENGTH, MAC_ALGORITHM));
    return mac.doFinal(data);
}

private static byte[] concatArrays(byte[] first, byte[] second) {
    byte[] ret = new byte[first.length + second.length];
    System.arraycopy(first, 0, ret, 0, first.length);
    System.arraycopy(second, 0, ret, first.length, second.length);
    return ret;
}
}

The plan will be to generate the salt installation time, and then it will remain the same for all encryption/decryption operations. I'm assuming that this should provide good enough protection against rainbow table attacks.

UPDATE 2: I had to realize that my MAC verification code wasn't quite optimal: the MAC already was SHA-1 hashed, so there was no point in creating a yet another SHA1 digest. I've also adjusted the MAC verification so it no longer uses Arrays.equals as that is vulnerable against timing attacks.

Answer 1

You should always use a key stretching algorithm when taking user input for a key such as a typed password. The key stretching does a few nice things. First it redistributes the entropy of your key (SHA1 does this as well) making the key appear more random (it isn't actually more random, the entropy remains the same), and second it makes brute forcing the key more computationally intensive (increasing with iterations obviously). The use of a random salt also precludes using precomputed lookup tables.

You should absolutely use a standard algorithm for this such as PBKDF2. In Java you can get a key factory for this via SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");

If your storing encrypted data in an environment you do not control you should also generate a MAC on your IV+Ciphertext and store it with your ciphertext. You can prepend it in the clear similar to the way you are storing the IV. Verify the MAC before decryption, you should verify indirectly by hashing the MAC first (a simple SHA1 works here) so as not to create a timing attack vector.

A MAC algorithm such as HMACSHA1 requires a secret key similar to a cipher. You should not use the same key for encrypting and generating the MAC. You can use the key stretching algorithm to generate a long enough key that you can use part for your cipher and part for your MAC.

ADDENDUM: If you are using Java 7 (or an external JCA provider that supports it) include a MAC with your AES cipher by using GCM mode. AES in GCM mode is a form of authenticated encryption that validates integrity as part of cipher. Implementing MAC generation and validation has various pitfalls that need to be avoided (such as the timing attack I mentioned or using separate keys) and rolling it in to the cipher is one less thing to screw up.

Creating secure crypto systems is not a trivial task, there are lots of ways to screw it up and make the entire process insecure. Instead of creating your own crypto system by putting together various crypto primitives it is generally a better idea to use a higher level library to handle things like cookie encryption and data storage or SSL/TLS for data in transit.