AES encryption, got extra trash characters in decrypted file

Question

Im making a debug loggin function in an android app. I have a simple class which is logging to .txt file using 128 bit AES encryption.

After the logging is done, i decrypt the logged file with a simple JAVA program.

The problem is when i decrypt the encrypted log i got some weird content in it, i also got the encrypted content, but there are some extra characters, see below.

Android app logging part:

public class FileLogger {

//file and folder name
public static String LOG_FILE_NAME = "my_log.txt";
public static String LOG_FOLDER_NAME = "my_log_folder";

static SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MM-dd_HH-mm-ss_SSS");

//My secret key, 16 bytes = 128 bit
static byte[] key = {1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6};

//Appends to a log file, using encryption
public static void appendToLog(Context context, Object msg) {

    String msgStr;
    String timestamp = "t:" + formatter.format(new java.util.Date());

    msgStr = msg + "|" + timestamp + "\n";

    File sdcard = Environment.getExternalStorageDirectory();
    File dir = new File(sdcard.getAbsolutePath() + "/" + LOG_FOLDER_NAME);
    if (!dir.exists()) {
        dir.mkdir();
    }

    File encryptedFile = new File(dir, LOG_FILE_NAME);

    try {
        
        //Encryption using my key above defined
        Key secretKey = new SecretKeySpec(key, "AES");
        Cipher cipher = Cipher.getInstance("AES");
        cipher.init(Cipher.ENCRYPT_MODE, secretKey);

        byte[] outputBytes = cipher.doFinal(msgStr.getBytes());

        //Writing to the file using append mode
        FileOutputStream outputStream = new FileOutputStream(encryptedFile, true);
        outputStream.write(outputBytes);
        outputStream.close();
        
        
    } catch (FileNotFoundException e) {
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    } catch (NoSuchAlgorithmException e) {
        e.printStackTrace();
    } catch (NoSuchPaddingException e) {
        e.printStackTrace();
    } catch (IllegalBlockSizeException e) {
        e.printStackTrace();
    } catch (BadPaddingException e) {
        e.printStackTrace();
    } catch (InvalidKeyException e) {
        e.printStackTrace();
    }

}

}

And this is the decrypter JAVA program:

public class Main {

    

//output file name after decryption
private static String decryptedFileName;
//input encrypted file
private static String fileSource;
//a prefix tag for output file name
private static String outputFilePrefix = "decrypted_";
//My key for decryption, its the same as in the encrypter program.
static byte[] key = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6 };

//Decrypting function
public static void decrypt(byte[] key, File inputFile, File outputFile) throws Exception {
    try {

        Key secretKey = new SecretKeySpec(key, "AES");
        Cipher cipher = Cipher.getInstance("AES");
        cipher.init(Cipher.DECRYPT_MODE, secretKey);

        FileInputStream inputStream = new FileInputStream(inputFile);
        byte[] inputBytes = new byte[(int) inputFile.length()];
        inputStream.read(inputBytes);

        byte[] outputBytes = cipher.doFinal(inputBytes);

        FileOutputStream outputStream = new FileOutputStream(outputFile, true);
        outputStream.write(outputBytes);

        inputStream.close();
        outputStream.close();

    } catch (Exception ex) {
        ex.printStackTrace();
    }
}

//first argument is the intput file source
public static void main(String[] args) {

    if (args.length != 1) {
        System.out.println("Add log file name as a parameter.");

    } else {
        fileSource = args[0];

        try {
            File sourceFile = new File(fileSource);
            if (sourceFile.exists()) {
                
                //Decrption
                decryptedFileName = outputFilePrefix + sourceFile.getName();
                File decryptedFile = new File(decryptedFileName);
                decrypt(key, sourceFile, decryptedFile);
            } else {
                System.out.println("Log file not found: " + fileSource);
            }

        } catch (Exception e) {
            e.printStackTrace();
        }

        System.out.println("Decryption done, output file: " + decryptedFileName);
    }

}

}

Output decrypted log (Opened with notepad++):

There is the valid content, but you also can see the extra thrash characters. If I open with the default windows text editor i also got thrash charaters, but different ones.

This is my first try with encrypt -decrypt, what m i doing wrong? Any ideas?

Answer 1

AES is a block cipher which only works on blocks. The plaintext that you want to encrypt can be of any length, so the cipher must always pad the plaintext to fill it up to a multiple of the block size (or add a complete block when it already is a multiple of the block size). In this PKCS#5/PKCS#7 padding each padding byte denotes the number of padded bytes.

The easy fix would be to iterate over outputBytes during decryption and remove those padding bytes which are always on the next line. This will break as soon as you use multiline log messages or use a semantically secure mode (more on that later).

The better fix would be to write the number of bytes for each log message before the message, read that and decrypt only that many bytes. This also probably easier to implement with file streams.

You currently use Cipher.getInstance("AES"); which is a non-fully qualified version of Cipher.getInstance("AES/ECB/PKCS5Padding");. ECB mode is not semantically secure. It simply encrypts each block (16 bytes) with AES and the key. So blocks that are the same will be the same in ciphertext. This is particularly bad, because some log messages start the same and an attacker might be able to distinguish them. This is also the reason why the decryption of the whole file worked despite being encrypted in chunks. You should use CBC mode with a random IV.

Here is some sample code for proper use of AES in CBC mode with a random IV using streams:

private static SecretKey key = generateAESkey();
private static String cipherString = "AES/CBC/PKCS5Padding";

public static void main(String[] args) throws Exception {
    ByteArrayOutputStream log = new ByteArrayOutputStream();
    appendToLog("Test1", log);
    appendToLog("Test2 is longer", log);
    appendToLog("Test3 is multiple of block size!", log);
    appendToLog("Test4 is shorter.", log);

    byte[] encLog = log.toByteArray();

    List<String> logs = decryptLog(new ByteArrayInputStream(encLog));

    for(String logLine : logs) {
        System.out.println(logLine);
    }
}

private static SecretKey generateAESkey() {
    try {
        return KeyGenerator.getInstance("AES").generateKey();
    } catch (NoSuchAlgorithmException e) {
        e.printStackTrace();
    }
    return null;
}

private static byte[] generateIV() {
    SecureRandom random = new SecureRandom();
    byte[] iv = new byte[16];
    random.nextBytes(iv);
    return iv;
}

public static void appendToLog(String s, OutputStream os) throws Exception {
    Cipher cipher = Cipher.getInstance(cipherString);
    byte[] iv = generateIV();
    cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(iv));
    byte[] data = cipher.doFinal(s.getBytes("UTF-8"));
    os.write(data.length);
    os.write(iv);
    os.write(data);
}

public static List<String> decryptLog(InputStream is) throws Exception{
    ArrayList<String> logs = new ArrayList<String>();
    while(is.available() > 0) {
        int len = is.read();
        byte[] encLogLine = new byte[len];
        byte[] iv = new byte[16];
        is.read(iv);
        is.read(encLogLine);

        Cipher cipher = Cipher.getInstance(cipherString);
        cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv));
        byte[] data = cipher.doFinal(encLogLine);
        logs.add(new String(data, "UTF-8"));
    }
    return logs;
}

Answer 2

You've encrypted each log message with a distinct encryption context. When you call the doFinal method on the cipher object the plaintext is padded out to a multiple of 16. Effectively, your log file is sequence of many small encrypted messages. However on decryption you are ignoring these message boundaries and treating the file as a single encrypted message. The result is that the padding characters are not being properly stripped. What you are seeing as 'trash' characters are likely these padding bytes. You will need to redesign your logfile format, either to preserve the message boundaries so the decryptor can discover them or to eliminate them altogether.

Also, don't use defaults in Java cryptography: they're not portable. For example, Cipher.getInstance() takes a string of the form alg/mode/padding. Always specify all three. I notice you also use the default no-args String.getBytes() method. Always specify a Charset, and almost always "UTF8" is the best choice.