AES BadPaddingException when decrypting

Question

So I am trying to make a web socket which sends data that will be encoded to base64 and then encrypted with AES, the resulting byte array will be sent over a stream between sockets and the server. This works fine up until I am trying to make a certain command which gives me the BadPaddingException. All the pieces of code which are using the Cryptographer class use the exact same class with the same secret.

The websockets has threads for each connection to read and write data. All these threads use the same cyptographer.

The function which is giving me the exception when the result is being decrypted in the connection's thread is written like this within a class called MessageSender, The Exception will occur when decrypting the result of sendFile():

package com.company.client.workers;

import com.company.security.Cryptographer;

import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.nio.file.Files;
import java.util.Base64;

public class MessageSender {
    private PrintWriter writer;
    private OutputStream outputStream;
    private Cryptographer cryptographer;

    public MessageSender(OutputStream outputStream) {
        this.outputStream = outputStream;
        this.writer = new PrintWriter(this.outputStream);
        cryptographer = new Cryptographer();
    }

    /**
     * Sends a message to the PrintWriter.
     * @param message to send.
     */
    public void send(String message) {
        try {
            String b64 = Base64.getEncoder().encodeToString(message.getBytes());
            byte[] bytes = cryptographer.getData(b64.getBytes(), 0);
            outputStream.write(bytes, 0, bytes.length);
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    /**
     * Send a file over the server.
     * @param receiver to send to
     * @param path location of file
     */
    public void sendFile(String receiver, String path) {
        File file = new File(path);
        try {
            byte[] bytes = Files.readAllBytes(file.toPath());
            // The encrypted result of this will throw the exception once the server tries to decrypt it.
            String message = "SFC " + receiver + " " + bytes.length + " " + getFileName(path);
            send(message);
            outputStream.write(cryptographer.getData(bytes, 0), 0, bytes.length);
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    private String getFileName(String path) {
        String[] parts = path.split("/");
        return parts[parts.length-1];
    }
}

The following run function will read data from an inputstream(this is the socket). And it will also call the cryptographer class to encrypt and decrypt data, this is the same class which also will be used in the client side with a separate instance. In the case of this exception IsReceiving is still set to false. Here is the Cryptographer class:

package com.company.security;

import javax.crypto.*;
import javax.crypto.spec.SecretKeySpec;
import java.security.InvalidKeyException;
import java.security.Key;
import java.security.NoSuchAlgorithmException;
import java.util.Base64;

public class Cryptographer {
private Key secretKey;

public Cryptographer() {
    byte[] secret = new byte[16]; // 128 bit is 16 bytes, and AES accepts 16 bytes, and a few others.
    byte[] secretBytes = "secret".getBytes();
    System.arraycopy(secretBytes, 0, secret, 0, secretBytes.length);
    secretKey = new SecretKeySpec(secret, "AES");
}

/**
 * Get data from either encryption and decryption.
 * @param data to encrypt or decrypt
 * @param mode 0 to encrypt en other numbers to decrypt
 * @return result data as byte array format.
 */
public byte[] getData(byte[] data, int mode) {
    Cipher c;

    try {
        c = Cipher.getInstance("AES");
    } catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
        e.printStackTrace();
        return null;
    }

    try {
        if(mode == 0) { // 0 is encrypt mode.
            c.init(Cipher.ENCRYPT_MODE, secretKey);
        } else { // other numbers are decrypt mode.
            c.init(Cipher.DECRYPT_MODE, secretKey);
        }
    } catch (InvalidKeyException e) {
        e.printStackTrace();
        return null;
    }

    try {
        return c.doFinal(data);
    } catch (IllegalBlockSizeException | BadPaddingException e) {
        e.printStackTrace();
        return null;
    }
}

/**
 * Decode base64 byte array.
 * @param encoded encoded byte array
 * @return decoded string
 */
public String decodeBaseToString(byte[] encoded) {
    return new String(Base64.getDecoder().decode(encoded));
}

/**
 * Decode base64 byte array.
 * @param encoded encoded byte array
 * @return decoded byte array
 */
public byte[] decodeBaseToBytes(byte[] encoded) {
    return Base64.getDecoder().decode(encoded);
}

/**
 * Encode byte array to base64.
 * @param source array to encode
 * @return encoded base64 byte array
 */
public byte[] encodeBase(byte[] source) {
    return Base64.getEncoder().encode(source);
}

}

The exception stacktrace:

javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
    at java.base/com.sun.crypto.provider.CipherCore.unpad(CipherCore.java:975)
    at java.base/com.sun.crypto.provider.CipherCore.fillOutputBuffer(CipherCore.java:1056)
    at java.base/com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:853)
    at java.base/com.sun.crypto.provider.AESCipher.engineDoFinal(AESCipher.java:446)
    at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2208)
    at com.company.security.Cryptographer.getData(Cryptographer.java:48)
    at com.company.client.Reader.run(Reader.java:45)
    at java.base/java.lang.Thread.run(Thread.java:835)
Exception in thread "Thread-3" java.lang.NullPointerException
    at java.base/java.lang.String.<init>(String.java:623)
    at com.company.client.Reader.run(Reader.java:47)
    at java.base/java.lang.Thread.run(Thread.java:835)

The encryption works fine with every other command and it also works when I don't include the file name in sendFile(). What am I doing wrong here?

Answer 1

The obvious mistakes;

1. You encode the data base64 before encryption, there is no need for this.

2. You send the data in binary form, which can cause errors. Convert the bytes into base64 before sending and decode after receiving. In Encrypt

    byte[] encrypted = cipher.doFinal(data.getBytes());
    return Base64.encodeBase64String(encrypted);
   

In decrypt

    byte[] original = cipher.doFinal(Base64.decodeBase64(encrypted));

3. Naming/implementing encryption/decryption function with getData. Make them separate functions.

4. With the Cipher.getInstance("AES") you are using ECB mode of operation. This is insecure, leaks pattern, see Wikipedia's Penguin.

5. Always, explicitly define your mode of operation and padding like

    Cipher.getInstance("AES/CBC/PKCS5Padding");
   

   this is for CBC mode.

6. Prefer AES-GCM

    Cipher.getInstance("AES/GCM/NoPadding");
   

   This will provide Confidentiality (by CTR mode of operation) and Integrity and authentication (by GHash both together make GCM). The CTR mode doesn't need padding so one gets rid of padding errors and padding oracle attacks if applicable. And see What are the rules for using AES-GCM correctly? on Cryptography.SE

   Make sure that the IV newer repeats under the same key otherwise it can cause catastrophic results. Like, reveal of the plaintext and more than the forgeries. One can use counter/LFSR to make sure that the IV never repeats. In the case of system failures a nonce = random||(LFSR|Counter) will be a better solution, one can read more here.