1

We have a stable codebase (we think) that we stream a file into, encrypt it, and write out to a resultant encrypted file. Then we decrypt the encrypted file to ensure that the process worked correctly full circle.

We have been using PBEWITHSHA1ANDDESEDE and it's been working fine for about a year, but it came to our attention that this algorithm is considered risky/broken. However, we tried to make a swap to a better algorithm, and now the content can not be decrypted and comes out unreadable. The algorithm we swapped to is PBEWithHmacSHA512AndAES_128 as per a Veracode blog we found here.

Why would switching to this alternate algorithm cause the file not to encrypt/decrypt full circle, if we made no other code changes? What can we do to make it work?

Below is included the important (I think) part of the code, with notes/comments.

// defined outside the below method; this algorithm works:
private static String ALGORITHM = "PBEWITHSHA1ANDDESEDE";

// the new version, which fails:
//private static String ALGORITHM = "PBEWithHmacSHA512AndAES_128";

private static Cipher getCipher(int mode, String password) throws NoSuchAlgorithmException, InvalidKeySpecException,
    NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {

    // Create secret key using password
    PBEKeySpec pbeKeySpec = new PBEKeySpec(password.toCharArray());
    SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(ALGORITHM);
    SecretKey secretKey = secretKeyFactory.generateSecret(pbeKeySpec);

    // Create the cipher
    byte[] salt = new byte[SALT_SIZE];
    salt = password.getBytes();

    PBEParameterSpec pbeParameterSpec = new PBEParameterSpec(salt, PBEPARAMETERSPEC_ITERATION_COUNT);
    Cipher cipher = Cipher.getInstance(ALGORITHM);

    // this original line causes crash with the new algorithm, reporting:
    // Exception in thread "main" java.security.InvalidAlgorithmParameterException: Missing parameter type: IV expected
    // solved as per this S.O. post:
    // https://stackoverflow.com/questions/29215274/how-do-i-properly-use-the-pbewithhmacsha512andaes-256-algorithm
    cipher.init(mode, secretKey, pbeParameterSpec);

    // this new line causes the encryption/decryption to apparently fail, giving results that look like this:
    // �0�.�����j�"��ۗP#o˾���IYc� �we����)�Tq(f�C���.��njDt�.pG��
    //cipher.init(mode, secretKey, cipher.getParameters());
    return cipher;
}

Edit:

We are currently using PBEWITHSHA1ANDDESEDE to encrypt and decrypt. What we want to do is switch so that we use PBEWithHmacSHA512AndAES_128 to encrypt and decrypt. We do not expect that ciphertext encrypted with PBEWITHSHA1ANDDESEDE could be decrypted with PBEWithHmacSHA512AndAES_128.

To be clear, this is all taking place in a single class with a single execution in a sandbox as a proof of concept. We just want to get the PBEWithHmacSHA512AndAES_128 algorithm to encrypt, and immediately decrypt a single piece of non-critical content. We just want to replace PBEWITHSHA1ANDDESEDE at the recommendation of Veracode and we can't figure out why this isn't working.

By request, here is the entire PoC:

import java.io.ByteArrayOutputStream;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;

import javax.crypto.Cipher;
import javax.crypto.CipherOutputStream;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.PBEParameterSpec;

public class FileEncryption {

    private static String PASSWORD = "fake-password-for-stack-overflow-post";
    private static String SOURCE_PATH = "";
    private static String SOURCE_FILE;
    private static String ENCRYPTED_FILE;
    private static String DECRYPTED_FILE;

    private static String ALGORITHM = "PBEWithHmacSHA512AndAES_128";
    private static int SALT_SIZE = 8;
    private static int PBEPARAMETERSPEC_ITERATION_COUNT = 100;

    public static void main(String[] args) throws Exception {
        if(args.length != 1) {
            System.out.println("Only accept a single variable, the path to input.txt.");
            System.out.println("(output will go to same dir)");
            return;
        }

        SOURCE_PATH = args[0];
        System.out.println("Set path: " + SOURCE_PATH);

        if( SOURCE_PATH.charAt(SOURCE_PATH.length()-1) != '/' ) {
            SOURCE_PATH += '/';
        }

        SOURCE_FILE = SOURCE_PATH + "plainfile.txt";
        ENCRYPTED_FILE = SOURCE_PATH + "plainfile.encrypted.txt";
        DECRYPTED_FILE = SOURCE_PATH + "plainfile.decrypted.txt";
        encryptContent(SOURCE_FILE, ENCRYPTED_FILE, PASSWORD);
        decryptContent(ENCRYPTED_FILE, DECRYPTED_FILE, PASSWORD);
    }

    private static void encryptContent(String inputFile, String outputFile, String password) throws Exception {
        Cipher cipher = getCipher(Cipher.ENCRYPT_MODE, password);
        performReadWrite(inputFile, outputFile, cipher);
    }

    private static void decryptContent(String inputFile, String outputFile, String password) throws Exception {
        Cipher cipher = getCipher(Cipher.DECRYPT_MODE, password);
        performReadWrite(inputFile, outputFile, cipher);
        performRead(inputFile, cipher);
    }

    private static void performReadWrite(String inputFile, String outputFile, Cipher cipher)
            throws FileNotFoundException, IOException {

            FileInputStream inFile = new FileInputStream(inputFile);
            FileOutputStream outFile = new FileOutputStream(outputFile);

        CipherOutputStream cos = new CipherOutputStream(outFile, cipher);

        int c;
        while ((c = inFile.read()) != -1)
        {
            cos.write(c);
        }

        cos.close();
        cos = null;
        inFile.close();
        inFile = null;
    }

    private static void performRead(String inputFile, Cipher cipher)
            throws FileNotFoundException, IOException {

            FileInputStream inFile = new FileInputStream(inputFile);

            ByteArrayOutputStream os = new ByteArrayOutputStream();
        CipherOutputStream cos = new CipherOutputStream(os, cipher);

        int c;
        while ((c = inFile.read()) != -1)
        {
            cos.write(c);
        }

        cos.close();
        cos = null;
        inFile.close();
        inFile = null;

//      aClass.outputStreamMethod(os);
            String aString = new String(os.toByteArray(),"UTF-8");
        System.out.println(aString);
    }

    private static Cipher getCipher(int mode, String password) throws NoSuchAlgorithmException, InvalidKeySpecException,
            NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {

            // Create secret key using password
            PBEKeySpec pbeKeySpec = new PBEKeySpec(password.toCharArray());
            SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(ALGORITHM);
            SecretKey secretKey = secretKeyFactory.generateSecret(pbeKeySpec);

            // Create the cipher
            byte[] salt = new byte[SALT_SIZE];
            salt = password.getBytes();

            PBEParameterSpec pbeParameterSpec = new PBEParameterSpec(salt, PBEPARAMETERSPEC_ITERATION_COUNT);
            Cipher cipher = Cipher.getInstance(ALGORITHM);
        cipher.init(mode, secretKey, cipher.getParameters());
            return cipher;
    }

}
Steverino
  • 2,099
  • 6
  • 26
  • 50

2 Answers2

2

OK, so you encrypt some data with algorithm A, then you change the code to use algorithm B and now you're trying to decrypt the ciphertexts that were encrypted with A, right? Why do you think this would work? The algorithms are different after all.

Instead, you should write some migration code which uses algorithm A to decrypt the existing ciphertexts and immediately encrypt them with algorithm B. Run the migration code over all ciphertext to get a clean set of encrypted files and then use algorithm B until algorithm C comes along.


Security considerations

Since you're changing your code, you can make it even better.

It is better to authenticate your ciphertexts so that attacks like a padding oracle attack are not possible. This can be done with authenticated modes like GCM or EAX, or with an encrypt-then-MAC scheme.

Artjom B.
  • 61,146
  • 24
  • 125
  • 222
  • Is the finding valid? As far as I can see, he's using SHA1 and 3DES as a PRF. 3DES still provides 112-bits of security, and birthday attacks are not a concern. (It would be different if this was a long term digital signature using SHA-1). – jww Jun 19 '17 at 23:34
  • I'm not sure what you mean, saying algorithm A and B. We are using PBEWITHSHA1ANDDESEDE to encrypt and decrypt. Later, we switched so that we used PBEWithHmacSHA512AndAES_128 to encrypt and decrypt. We do not use PBEWITHSHA1ANDDESEDE to encrypt and PBEWithHmacSHA512AndAES_128 to decrypt, for example. This is all taking place in a single file, you could think of it as a PoC just to get the PBEWithHmacSHA512AndAES_128 algorithm to encrypt, and immediately decrypt a single piece of non-critical content. This is what's failing. I'll also update the question to be clear on this. – Steverino Jun 20 '17 at 00:36
1

Your original code uses the PBEWITHSHA1ANDDESEDE algorithm. This algorithm uses CBC mode that relies on an IV. As the PBEWithSHA1AndDESede algorithm derives an IV from the input parameters (password, iteration count etc.), this works without you handling the IV in your code.

You can verify this by examining the ciphers IV after initialization - it stays the same in this case with each run. You can get the IV with byte[] iv = cipher.getIV();

However, the PBEWithHmacSHA512AndAES_128 algorithm don't derive an IV. Thus as you don't specify an IV in your code you end up with a random IV generated by the Cipher. This makes the decrypt break, as you don't do the decrypt with the same IV as used on the encrypt. If you examine the IV in this case it will be different each time you run it (and this is an improvement security wise).

So basically you need to introduce logic for handling the IV into your code. Often the IV is simply pre-prended the encrypted data, so it's readily available when you need to initialize the decryption cipher.

Community
  • 1
  • 1
Ebbe M. Pedersen
  • 7,250
  • 3
  • 27
  • 47