0
\$\begingroup\$

I have written a C++ program that uses the Crypto++ library to encrypt files using AES-256Bit (in GCM mode). I am planning to eventually have the program store the keys for the files in a file e.g. keys.txt and encrypt that file again, maybe with a different algorithm which apparently adds security instead of using the same key for each file (correct me if I'm wrong).

OK, well, I have mostly "stolen" and modified the code but I wanted to know if my code is actually secure or not, as well as if I should stick with GCM or if I should use a different mode instead. Lastly, I have read that a random IV generation in the GCM mode is bad because of IV collision, but others say it's not a problem. What is the case in my program?

This is my first time using such an advanced tool (or whatever you like to call it) so please don't be to hard on me when you find mistakes I've made, but do still call them out.

Thanks in advance for checking.

#include <cryptopp/files.h>   // to encryt files (for FileSink and stuff like that)
#include <cryptopp/modes.h>   // different AES modes (GCM is used here)
#include <cryptopp/osrng.h>   // random key and iv gneration

#include <fstream>   // library to convert file to binary
#include <iostream>  // self explanitory

using aes_key_t = std::array<CryptoPP::byte, 32>;  // keysize definition (AES-256bit here)
using aes_iv_t = std::array<CryptoPP::byte, CryptoPP::AES::BLOCKSIZE>;  // block size definition (16 bytes)

void encrypt(const aes_key_t &key, const aes_iv_t &iv, const std::string &filename_in, const std::string &filename_out) {
  CryptoPP::GCM_Mode<CryptoPP::AES>::Encryption cipher{};
  cipher.SetKeyWithIV(key.data(), key.size(), iv.data());

  std::ifstream in{filename_in, std::ios::binary};    // open the inputfile in binary mode
  std::ofstream out{filename_out, std::ios::binary};  // open the outputfile in binary mode

  CryptoPP::FileSource{in, /*pumpAll=*/true,
                       new CryptoPP::StreamTransformationFilter{
                           cipher, new CryptoPP::FileSink{out}}};
}

void decrypt(const aes_key_t &key, const aes_iv_t &iv,const std::string &filename_in, const std::string &filename_out) {
  CryptoPP::GCM_MODE<CryptoPP::AES>::Decryption cipher{};
  cipher.SetKeyWithIV(key.data(), key.size(), iv.data());

  std::ifstream in{filename_in, std::ios::binary};
  std::ofstream out{filename_out, std::ios::binary};

  CryptoPP::FileSource{in, /*pumpAll=*/true,
                       new CryptoPP::StreamTransformationFilter{
                           cipher, new CryptoPP::FileSink{out}}};
}

int main(int argc, char **argv) {
  // prints block size
  std::cout <<  CryptoPP::AES::BLOCKSIZE << std::endl;

  CryptoPP::AutoSeededRandomPool rng{};

  // Generate a random key
  aes_key_t key{};
  rng.GenerateBlock(key.data(), key.size());

  // Generate a random IV
  aes_iv_t iv{};
  rng.GenerateBlock(iv.data(), iv.size());

  // encrypt
  encrypt(key, iv, "notes.txt", "notes_encrypted");

  // decrypt
  decrypt(key, iv, "notes_encrypted", "notes_decrypted.txt");

  return 0;
}

\$\endgroup\$
4
  • \$\begingroup\$ "secure or not, as well as if I should stick with GCM or if I should use a different mode instead" What's your usage going to be like? What and who are you protecting against? \$\endgroup\$
    – Mast
    May 24, 2022 at 21:14
  • \$\begingroup\$ @Mast the usage is encrypting files and the encryption protects against people who want your files without your passphrase or passkey etc. \$\endgroup\$
    – zero
    May 25, 2022 at 12:10
  • \$\begingroup\$ Well, yes, but there's a major difference between protecting against nosy family/colleagues or a government agency. There's a whole world in between. \$\endgroup\$
    – Mast
    May 25, 2022 at 15:33
  • \$\begingroup\$ @Mast Lets go with government agency \$\endgroup\$
    – zero
    May 28, 2022 at 17:44

2 Answers 2

4
\$\begingroup\$

Source code attribution

I have mostly "stolen" and modified the code

If you have stolen it from an open source project with a license that allows you to distribute copies of the modified source code, then this is fine, but you should add proper attribution to the original author of the code you "stole", and you should make sure you distribute it under a license compatible with that from the original source. This might be required by the original license, but even if not, this is the ethical thing to do.

Security is more than an encryption algorithm

It's hard to say if your code is secure. In a way, the notes_encrypted file you write is very secure; it is created using a key and IV that are thrown away when the program ends, so there is no way to decrypt it. On the other hand, your program stores an unencrypted copy of the input, so everyone can read the input after the program ends. This is very insecure.

Of course, you will probably not immediately write out an unencrypted copy in a real program. But you do have to think about key and IV management. Where are they going to be stored? Or if they are not randomly generated for every file, where do the key and IV come from? There are many possible answers to these questions, with different trade-offs. It is impossible to say whether your program is secure, because it is not clear what the goal is of encrypting the data, and what you are protecting the data against.

It might be instructive to look at existing open source software that encrypts and decrypts files, like GnuPG, and look at how they do this.

\$\endgroup\$
1
  • \$\begingroup\$ First thanks for the reply. Keys and IV's are going to be randomly generated for each files as I indirectly mentioned and then stored in a text file e.g. keys.txt which then gets encrypted too with maybe a different algorithm. But it probably wont be an issue if I would use AES again. The goal of encrypting the files is to make them secure against people who dont have your passphrase or passkey or whatever you choose then for the keys.txt file. Also don't worry the code was free to use. \$\endgroup\$
    – zero
    May 25, 2022 at 12:06
4
\$\begingroup\$

You are using authenticated encryption. This provides some protection against change, be it deliberate or due to bit rot. But what it doesn't protect against is replacing one file with another. As you haven't protected any meta data of the file (filename etc.) an adversary can change any of that without detection. In principle you can partly avoid this by adding the meta data to the Additional Authenticated Data (AAD, or just AD) of the AEAD cipher that is GCM.

There is also an issue when it comes to time. An adversary could store a previous version of a file and replace the current version with it. This is much harder to protect against, but it should be taken into consideration.


For GCM the most efficient nonce (or, if you must, IV) has a size of 96 bits / 12 bytes. Any other IV size will first be converted into a 96 bit IV. So it means additional processing without any security benefit.

The IV or nonce is generally stored with the ciphertext, otherwise it is impossible to get the value back. Your code works only because you encrypt and then immediately decrypt. The key can remain the same for many plaintext / ciphertext pairs, but please remember that the key / IV combination must be unique, or GCM gets broken completely.


Due to the size of the nonce and the way that the counter mode encryption inside GCM works, files can be at most 64 GiB in size (minus a few bytes). That sounds like a lot, but for instance some games easily pass this kind of size.

What is good is that you use streaming to encrypt / decrypt the files. You don't want to load large files into memory.


I don't have much problems with how the library itself is written. The variable names are clear, and the code is relatively well formatted. I'm not a fan of pre-formatted code, a double space indentation, a lack of white space and especially end of line comments, but that's mainly because of personal taste I guess.

I personally don't mind if code like this is used as an example or even partly copied; there is little to no code that can be done differently. What is a problem is that copying cryptographic code generally results in insecure code, with incorrect parameters or use cases. The IV issues and the lack of AAD is a clear indication that this is no exception.

I would introduce a constant such as AES_256_KEY_SIZE_BYTES 256 with a value of 32. When auditing it should be as clear from the code as possible that the specific key size is being used.

The main issue with cryptography is that getting it to work is not the big problem. If that would be the case then we would not need it.

\$\endgroup\$
2
  • \$\begingroup\$ thank you for the answer so what i have to do is generate unique key/IV combinations, add file metadata to the AAD and change the iv size to 96bit/12byte (just to sum it up) \$\endgroup\$
    – zero
    May 25, 2022 at 11:48
  • \$\begingroup\$ Right! So the key and IV generation should definitely not be in the same sequence. You'd normally have a (more) static key and a random IV for each time you are using it. Beware that it still depends on your use case if the entire thing is secure or not; we can find out if things are not secure without use case, but in the end security is part of a system, not of an encryption scheme. \$\endgroup\$ May 25, 2022 at 12:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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