How to correctly encrypt and decrypt a file using a secret key derived from a password

I am trying to work out the correct process to encrypt and decrypt a file using the "PBEWithHmacSHA256AndAES_256" standard.

From what I understand from looking at this example code from Oracle.

I have gathered that a salt is needed, as well as an iteration count and the hash standard.

So I have my main method, passing into the encryption method:

1. a user-defined password new String(key).toCharArray() as a byte array (using this method for other encryption runs)
2. a secure random IV initVector as a byte array
3. the plain text file inputFile as a String
4. the name of, to be created, ciphertext file outputFile as a String

I have followed the code example to code up what I believe is correct for the encrypt method. And I am storing the salt and IV to be used for decryption by appending them both to the ciphertext.

private static void encrypt(byte[] key, byte[] initVector, String inputFile, String outputFile) //exceptions for throws... {
//Initalisation for encryption
Cipher cipher;
byte[] salt = new byte[16];
SecureRandom rand = new SecureRandom();
// Salt randomly generated with base64
rand.nextBytes(salt);
System.out.println("my salt should be" + Base64.getEncoder().encodeToString(salt));
salt = Base64.getEncoder().encode(salt);
// Iteration count
int count = 1000;
IvParameterSpec iv = new IvParameterSpec(initVector);
// Create PBE parameter set
PBEParameterSpec pbeParamSpec = new PBEParameterSpec(Base64.getDecoder().decode(salt), count, iv);                
// Convert pass into SecretKey object
PBEKeySpec pbeKeySpec = new PBEKeySpec(new String(key).toCharArray());
SecretKeyFactory keyFac = SecretKeyFactory.getInstance("PBEWithHmacSHA256AndAES_256");
SecretKey pbeKey;
try {
pbeKey = keyFac.generateSecret(pbeKeySpec);
} catch (InvalidKeySpecException e) {
System.out.println("Sorry, the password specified cannot be used as a secret key");
System.out.println("Please check that your password uses valid characters");
return;
}
// Create PBE Cipher
cipher = Cipher.getInstance("PBEWithHmacSHA256AndAES_256");
// Initialize PBE Cipher with key and parameters
cipher.init(Cipher.ENCRYPT_MODE, pbeKey, pbeParamSpec);
}
//File error checking and file handling (i.e. generating file paths)...
System.out.println("Secret key is " + Base64.getEncoder().encodeToString(key));
System.out.println("IV is " + Base64.getEncoder().encodeToString(initVector));
//Special file reading and writing with 'Cipher Stream'
try (InputStream fin = FileEncryptor.class.getResourceAsStream(loadFile.getName());
OutputStream fout = Files.newOutputStream(saveFile);
CipherOutputStream cipherOut = new CipherOutputStream(fout, cipher) {
}) {
final byte[] bytes = new byte[1024];
for(int length=fin.read(bytes); length!=-1; length = fin.read(bytes)){
fout.write(initVector);
fout.write(salt);
cipherOut.write(bytes, 0, length);
}
} catch (IOException e) {
System.out.println("Something went wrong with reading and writing these files!");
System.out.println("Please check you have the latest version of this program");
System.out.println("Contact your IT admin to make sure you have sufficient privileges");
}
System.out.println("SUCCESS! Encryption finished, saved at specified location");
}

Then I also have my main method, passing into the decryption method:

1. a user-defined password String inputKEY as a string (also using this method for other dencryption runs)

2. a String for inputIV , has been passed in as null, since not used for PBE.

3. the ciphertext file inputFile as a String

4. the name of, to be created, revealplaintext file outputFile as a String

   private static void decrypt(String inputKEY, String inputIV, String inputFile, String outputFile) {
   Cipher cipher = null;

    //File error checking and file handling (i.e. generating file paths)...
   InputStream encryptedData = Files.newInputStream(loadFilePath);
   PBEKeySpec pbeKeySpec = new PBEKeySpec(inputKEY.toCharArray());
   SecretKeyFactory keyFac = SecretKeyFactory.getInstance("PBEWithHmacSHA256AndAES_256");
   SecretKey pbeKey = null;
   try {
   pbeKey = keyFac.generateSecret(pbeKeySpec);
   } catch (InvalidKeySpecException e) {
   // TODO Auto-generated catch block
   e.printStackTrace();
   }
   byte[] initVect = new byte[16];
   encryptedData.read(initVect);
   IvParameterSpec iv = new IvParameterSpec(Base64.getDecoder().decode(initVect);
   byte[] salt = new byte[16];
   encryptedData.read(salt);
   PBEParameterSpec pbeParamSpec = new PBEParameterSpec(Base64.getDecoder().decode(salt), 1000, iv);  
   cipher = Cipher.getInstance("PBEWithHmacSHA256AndAES_256");
   System.out.println("my salt should be" + Base64.getEncoder().encodeToString(Base64.getDecoder().decode(salt)));
   cipher.init(Cipher.DECRYPT_MODE, pbeKey, pbeParamSpec); 
   try (CipherInputStream decryptStream = new CipherInputStream(encryptedData, cipher);	
   OutputStream decryptedOut = Files.newOutputStream(saveFile)){
   final byte[] bytes = new byte[1024];
   for(int length=decryptStream.read(bytes); length!=-1; length = decryptStream.read(bytes)){
   decryptedOut.write(bytes, 0, length);
   }
   } catch (IOException e) { //This is caught when decryption is run
   System.out.println("Something went wrong with reading and writing these files!");
   System.out.println("Please check you have the latest version of this program");
   System.out.println("Contact your IT admin to make sure you have sufficient privileges");
   }
   System.out.println("SUCESS! Decryption finished, saved at specified location");
   

   }

I believe something is not right with my understanding of PBE, and thus the way I implemented it is probably wrong. Could anyone point out what seems to be wrong?

The main issues are:

• IV and Salt must not be written inside the for loop.
• The IV is stored in encrypt not Base64 encoded, but is Base64 decoded in decrypt.
• The 16 bytes salt is stored in encrypt (unnecessarily) Base64 encoded, i.e. 24 bytes are stored. In decrypt however only 16 bytes are loaded.

Also:

• Upon encoding/decoding, sometimes no encoding is specified, so the default encoding is used.
• encrypt and decrypt use different parameter types for key and IV.
• There are many copy/paste errors in the code.

Note: In contrast to your code, the linked code determines besides the key also the IV from password and salt.
In your code the IV is passed. Thus you have to ensure that a key/IV pair may only be used once. Usually a random IV is generated for each encryption.

In the following code (which is based on your code, but for simplicity without exception handling) these issues are fixed/optimized. Furthermore, the code applies FileInputStream and FileOutputStream instead of your classes (but this is not required):

<!-- language: none -->
private static void encrypt(String key, byte[] initVector, String inputFile, String outputFile) throws Exception {

    // Key
PBEKeySpec pbeKeySpec = new PBEKeySpec(key.toCharArray());
SecretKeyFactory keyFac = SecretKeyFactory.getInstance("PBEWithHmacSHA256AndAES_256");
SecretKey pbeKey = keyFac.generateSecret(pbeKeySpec);
// IV
IvParameterSpec iv = new IvParameterSpec(initVector);
// Salt
SecureRandom rand = new SecureRandom();
byte[] salt = new byte[16];
rand.nextBytes(salt);
// ParameterSpec
int count = 1000; // should be larger, see Michael Fehr's comment
PBEParameterSpec pbeParamSpec = new PBEParameterSpec(salt, count, iv);
// Cipher
Cipher cipher = Cipher.getInstance("PBEWithHmacSHA256AndAES_256");
cipher.init(Cipher.ENCRYPT_MODE, pbeKey, pbeParamSpec);
try (FileInputStream fin = new FileInputStream(inputFile);
FileOutputStream fout = new FileOutputStream(outputFile);
CipherOutputStream cipherOut = new CipherOutputStream(fout, cipher)) {
// Write IV, Salt
fout.write(initVector);
fout.write(salt);
// Encrypt
final byte[] bytes = new byte[1024];
for (int length = fin.read(bytes); length != -1; length = fin.read(bytes)) {
cipherOut.write(bytes, 0, length);
}
} 
}

<!-- language: none -->

<!-- language: none -->
private static void decrypt(String key, byte[] initVect, String inputFile, String outputFile) throws Exception {

    try (FileInputStream encryptedData = new FileInputStream(inputFile);
FileOutputStream decryptedOut = new FileOutputStream(outputFile)) {
// Key
PBEKeySpec pbeKeySpec = new PBEKeySpec(key.toCharArray());
SecretKeyFactory keyFac = SecretKeyFactory.getInstance("PBEWithHmacSHA256AndAES_256");
SecretKey pbeKey = keyFac.generateSecret(pbeKeySpec);
// Read IV
if (initVect == null) {
initVect = encryptedData.readNBytes(16);
}
IvParameterSpec iv = new IvParameterSpec(initVect);
// Read salt
byte[] salt = encryptedData.readNBytes(16);
// ParameterSpec
int count = 1000;
PBEParameterSpec pbeParamSpec = new PBEParameterSpec(salt, count, iv);
// Cipher
Cipher cipher = Cipher.getInstance("PBEWithHmacSHA256AndAES_256");
cipher.init(Cipher.DECRYPT_MODE, pbeKey, pbeParamSpec);
try (CipherInputStream decryptStream = new CipherInputStream(encryptedData, cipher)) {
// Decrypt
final byte[] bytes = new byte[1024];
for (int length = decryptStream.read(bytes); length != -1; length = decryptStream.read(bytes)) {
decryptedOut.write(bytes, 0, length);
}
} 
}
}

<!-- language: none -->

EDIT - Concerning the reading of salt and IV in decrypt:<br>As GPI pointed out in their comment, FileInputStream.read(byte[] b) generally reads b.length bytes, but this is not guaranteed. More robust is to determine the length of the read data and call the method in a loop until the data is complete. Another alternative is the use of InputStream.readNBytes​(int len), which is guaranteed to read len bytes (unless end of stream is encountered or an exception is thrown), as Zabuzard has suggested. In the code, the latter is now used, i.e. read was replaced by readNBytes​.