Cipher text is not a multiple of block size

Question

I've written a simple AES encryption program for a piece of text and whenever I attempt to decrypt the text, I am thrown an error;

StreamTransformationFilter: ciphertext length is not a multiple of block size

This is my method of encryption;

c_crypto::cryptkey aes_key;

c_crypto::make_keys(aes_key);

std::string testmessage = "test";

c_crypto::encrypt_buffer(testmessage, aes_key.enc_key, aes_key.enc_iv);
c_crypto::decrypt_buffer(testmessage, aes_key.enc_key, aes_key.enc_iv);

std::cout << testmessage << std::endl;

cryptkey struct;

struct cryptkey {
    unsigned char enc_key[AES_KEY_SIZE];
    unsigned char enc_iv[AES_KEY_SIZE];
};

AES_KEY_SIZE = 16

The make_keys() function simply calls;

void c_crypto::random_bytes(const int &amount, unsigned char *result) {
    CryptoPP::AutoSeededRandomPool rng;
    rng.GenerateBlock(result, amount);
}

With the paramters of AES_KEY_SIZE of either cryptkey.enc_iv or cryptkey.enc_key

In the decrypt function;

void c_crypto::decrypt_buffer(std::string &input, unsigned char key[AES_KEY_SIZE], unsigned char iv[AES_KEY_SIZE]) {
    input = base64_decode(input);

    CryptoPP::AES::Decryption aesDecryption(key, AES_KEY_SIZE);
    CryptoPP::CBC_Mode_ExternalCipher::Decryption cbcDecryption(aesDecryption, iv);

    CryptoPP::StreamTransformationFilter stfDecryptor(cbcDecryption, new CryptoPP::StringSink(input));
    stfDecryptor.Put(reinterpret_cast<const unsigned char*>(input.c_str()), input.size());
    stfDecryptor.MessageEnd();
}

The error is thrown in stfDecryptor.MessageEnd();

Here is the encrypt function if it is of any use;

void c_crypto::encrypt_buffer(std::string &input, unsigned char key[AES_KEY_SIZE], unsigned char iv[AES_KEY_SIZE]) {
    CryptoPP::AES::Encryption aesEncryption(key, AES_KEY_SIZE);
    CryptoPP::CBC_Mode_ExternalCipher::Encryption cbcEncryption(aesEncryption, iv);

    CryptoPP::StreamTransformationFilter stfEncryptor(cbcEncryption, new CryptoPP::StringSink(input));
    stfEncryptor.Put(reinterpret_cast<const unsigned char*>(input.c_str()), input.length());
    stfEncryptor.MessageEnd();

    input = base64_encode(input);
}

EDIT:

Here's my decode and encode functions:

std::string c_crypto::base64_encode(std::string &input) {
    std::string result;
    CryptoPP::StringSource(input, true, new CryptoPP::Base64Encoder(new CryptoPP::StringSink(result)));
    return result;
}

std::string c_crypto::base64_decode(std::string &input) {
    std::string result;
    CryptoPP::StringSource(input, true, new CryptoPP::Base64Decoder(new CryptoPP::StringSink(result)));
    std::cout << (input.size() % 16 == 0) << std::endl;
    return result;
}

Answer 1

void c_crypto::encrypt_buffer(std::string &input, unsigned char key[AES_KEY_SIZE], unsigned char iv[AES_KEY_SIZE]) {
    CryptoPP::AES::Encryption aesEncryption(key, AES_KEY_SIZE);
    CryptoPP::CBC_Mode_ExternalCipher::Encryption cbcEncryption(aesEncryption, iv);

    CryptoPP::StreamTransformationFilter stfEncryptor(cbcEncryption, new CryptoPP::StringSink(input));
    stfEncryptor.Put(reinterpret_cast<const unsigned char*>(input.c_str()), input.length());
    stfEncryptor.MessageEnd();

    input = base64_encode(input);
}

Don't use the same string as a source and a sink like you are doing. Use a separate temporary instead.

Most (all?) block ciphers can take a 16-byte block and encrypt or decrypt in place. However, in the case of string, the as string changes the underlying allocation changes. When the allocation changes the underlying pointer changes.

Maybe use something like the following. The temporaries sidestep the problems of using a string as both a source and sink. It also provides better exception safety. If something goes sideways your input is left unchanged.

void c_crypto::encrypt_buffer(std::string &input, unsigned char key[AES_KEY_SIZE], unsigned char iv[AES_KEY_SIZE]) {
    AES::Encryption aesEncryption(key, AES_KEY_SIZE);
    CBC_Mode_ExternalCipher::Encryption cbcEncryption(aesEncryption, iv);

    std::string encrypted; encrypted.reserve(input.size()+16);
    StreamTransformationFilter stfEncryptor(cbcEncryption, new StringSink(encrypted));
    stfEncryptor.Put(reinterpret_cast<const unsigned char*>(input.c_str()), input.length());
    stfEncryptor.MessageEnd();

    std::string encoded = base64_encode(encrypted);
    std::swap(encoded, input);
}

void c_crypto::decrypt_buffer(std::string &input, unsigned char key[AES_KEY_SIZE], unsigned char iv[AES_KEY_SIZE]) {
    std::string decoded = base64_decode(input);
    std::string decrypted; decrypted.reserve(decoded.size());

    AES::Decryption aesDecryption(key, AES_KEY_SIZE);
    CBC_Mode_ExternalCipher::Decryption cbcDecryption(aesDecryption, iv);

    StreamTransformationFilter stfDecryptor(cbcDecryption, new StringSink(decrypted));
    stfDecryptor.Put(reinterpret_cast<const unsigned char*>(decoded.c_str()), decoded.size());
    stfDecryptor.MessageEnd();

    std::swap(decrypted, input);
}

---

Using a buffer as an in/out parameter works fine. In this example the pointer will not change part way through the process:

byte buff[16] = {0};
CBC_Mode<AES>::Encryption enc(key, 16, iv);
enc.ProcessBlock(buff);

However your use case is a little trickier because the cipher text is appended to the string input by way of StringSink(input), which inevitably makes input grow and invalidates the iterators being used to feed the data into the encryptor via StringSource(input, ...).

If interested, the ProcessBlock and ProcessString are the low-level member functions used to transform the data. Filters like StreamTransformationFilter simply calls it for you. There's no magic or mischief. It is just a high level way to process data. Also see BlockTransformation in the doxygen manual.

---

Regarding padding, you used:

StreamTransformationFilter stfEncryptor(cbcEncryption, new StringSink(input));

Which is really this StreamTransformationFilter constructor:

StreamTransformationFilter (StreamTransformation &c, BufferedTransformation *attachment=NULL, BlockPaddingScheme padding=DEFAULT_PADDING)

DEFAULT_PADDING is one of two things. For modes that need padding, like ECB and CBC, it is PKCS #7 padding (PKCS_PADDING). For modes that don't need padding, like CTR, it is no padding (NO_PADDING).

You should usually take the default when it comes to padding schemes.