3
\$\begingroup\$

I've written a simple Java class to encrypt and decrypt files using AES. The code works and I'm able to encrypt and decrypt files.

However, I understand this is an easy way of implementing AES and far from the most secure (my usage of ECB mode for example).

I'm looking to improve the strength of encryption this code provides.

import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.security.Key;

class Crypto {

    // Process a file (Encrypts or decrypts depending on cipherMode)
    private void processFile(boolean encrypt, File inputFile, String inputKey, File outputFile) throws Exception {
        // Convert key into bytes
        Key key = new SecretKeySpec(inputKey.getBytes(),"AES");

        // Get cipher instance
        Cipher cipher = Cipher.getInstance("AES");


        if(encrypt) {
            cipher.init(Cipher.ENCRYPT_MODE,key);
        }
        else {
            cipher.init(Cipher.DECRYPT_MODE,key);
        }

        // Read input file into byte array
        FileInputStream fileInputStream = new FileInputStream(inputFile);
        byte[] inputBytes = new byte[(int)inputFile.length()];
        fileInputStream.read(inputBytes);

        // Process the byte array from the input file
        byte[] outputBytes = cipher.doFinal(inputBytes);

        // Write the output byte array to the output file
        FileOutputStream fileOutputStream = new FileOutputStream(outputFile);
        fileOutputStream.write(outputBytes);

        // Close file streams
        fileInputStream.close();
        fileOutputStream.close();
    }

    // Encrypts a file
    void encrypt(File inputFile, String inputKey, File outputFile) throws Exception {
        processFile(true,inputFile,inputKey,outputFile);
    }

    // Decrypts a file
    void decrypt(File inputFile, String inputKey, File outputFile) throws Exception {
        processFile(false,inputFile,inputKey,outputFile);
    }

}

The encrypt() and decrypt() methods are there purely for easy of use and readability when using the code.

\$\endgroup\$
  • 1
    \$\begingroup\$ Even if you did not ask for it, one comment about file handling: java.nio.file.Files has been around since Java 7, so it may be time to use Path instead of File and Files.readAllBytes() and Files.write() instead of old handmade I/O code... \$\endgroup\$ – mtj Jun 15 '18 at 8:05
  • \$\begingroup\$ Using a String for the secret key is a very bad idea. The crypto API and e.g. JPasswordField don't use them. \$\endgroup\$ – Mark Jeronimus Jun 15 '18 at 15:39
2
\$\begingroup\$

As far as security goes, rather than just using "AES" for your algorithm, from the documentation:

Every implementation of the Java platform is required to support the following standard Cipher transformations with the keysizes in parentheses:

  • AES/CBC/NoPadding (128)
  • AES/CBC/PKCS5Padding (128)
  • AES/ECB/NoPadding (128)
  • AES/ECB/PKCS5Padding (128)
  • AES/GCM/NoPadding (128)
  • DES/CBC/NoPadding (56)
  • DES/CBC/PKCS5Padding (56)
  • DES/ECB/NoPadding (56)
  • DES/ECB/PKCS5Padding (56)
  • DESede/CBC/NoPadding (168)
  • DESede/CBC/PKCS5Padding (168)
  • DESede/ECB/NoPadding (168)
  • DESede/ECB/PKCS5Padding (168)
  • RSA/ECB/PKCS1Padding (1024, 2048)
  • RSA/ECB/OAEPWithSHA-1AndMGF1Padding (1024, 2048)
  • RSA/ECB/OAEPWithSHA-256AndMGF1Padding (1024, 2048)

Likewise, here's more documentation on the subject of what is supported.

If you're unsure which one to choose, this answer will shed some light. Here's the summary:

In general, stick with CBC or CTR, with PKCS#7 where necessary (you don't need padding on stream cipher modes) and use an authenticity check (HMAC-SHA256 for example) on the ciphertext. Both CBC and CTR come recommended by Niels Ferguson and Bruce Schneier, both of whom are respected cryptographers.

What's the difference between PKCS5 and PKCS7?

... fundamentally PKCS#5 padding is a subset of PKCS#7 padding for 8 byte block sizes. Hence, PKCS#5 padding can not be used for AES ...

Some cryptographic libraries such as the SUN provider in Java indicate PKCS#5 where PKCS#7 should be used - "PKCS5Padding" should have been "PKCS7Padding". This is a legacy from the time that only 8 byte block ciphers such as (triple) DES symmetric cipher were available.

However, that doesn't stop you from using PKCS#5 with AES.

Given the answers to the question linked above, I would suggest using the following:

"AES/CBC/PKCS5Padding"

After that, reading erikson's answer will tell you most of what you need to know about how to write this code in a relatively secure manner.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.