Crypto++ explicit destruction during encryption/decryption?

Question

I wrote some wrapper functions to encrypt/decrypt files using crypto++. I tried looking in the wiki but could find my answer. I am wondering if I need to explicitly destroy my objects that are created?

I found in the wiki that some objects when passed into functions are destroyed for you, but no examples of my exact use were there so I just wanted to be sure.

   CryptoPP::AutoSeededRandomPool prng;
   //Key generation
   byte key[AES::DEFAULT_KEYLENGTH];
   prng.GenerateBlock(key, sizeof(key));
   //IV generation
   byte iv[AES::BLOCKSIZE];
   prng.GenerateBlock(iv, sizeof(iv));



   //print key
   encoded.clear();
   StringSource(key, sizeof(key), true, new HexEncoder(new StringSink(encoded)));
   cout << "key: " << encoded << endl;
   cout << "Size of key: " << sizeof(key) << endl;

   //print iv
   encoded.clear();
   StringSource(iv, sizeof(iv), true, new HexEncoder(new StringSink(encoded)));
   cout << "iv: " << encoded << endl;
   cout << "Size of iv: " << sizeof(iv) << endl;

   //See function below
   encrypt_file(inFile, outFile, key, iv, err); 

   inFile.close();
   outFile.close();

Once in this function the bytes arrays are truncated for some reason

Encrypt_file

    bool encrypt_file(std::ifstream& inFile,
       std::ofstream& outFile,
       const byte* key, const byte* iv,
       std::string& errMsg)
    {
       std::string encoded;
       //print key
       encoded.clear();
       StringSource(key, sizeof(key), true, new HexEncoder(new StringSink(encoded)));
       cout << "key: " << encoded << endl;
       cout << "Size of key: " << sizeof(key) << endl;

       //print iv
       encoded.clear();
       StringSource(iv, sizeof(iv), true, new HexEncoder(new StringSink(encoded)));
       cout << "iv: " << encoded << endl;
       cout << "Size of iv: " << sizeof(iv) << endl;
       try {
          CryptoPP::CBC_Mode<CryptoPP::AES>::Encryption e;
          e.SetKeyWithIV(key, sizeof(key), iv);
          CryptoPP::FileSource(inFile, true, new CryptoPP::StreamTransformationFilter(e, new CryptoPP::FileSink(outFile)));
          inFile.close();
          outFile.close();
       }
       catch (CryptoPP::Exception& e) {
          errMsg = e.GetWhat();
          return false;
       }
       return true;
    }

Output:

key: 6574D7BDFD0DD3BC59CD3846D4A196A8
Size of key: 16
iv: 1B4ED692F91A32246B41F63F6B8C6EAA
Size of iv: 16
key: 6574D7BDFD0DD3BC
Size of key: 8
iv: 1B4ED692F91A3224
Size of iv: 8

Answer 1

No, you don't. The objects you create have automatic storage duration, which means their destructor will be automatically invoked at the end of their scope. Moreover, the arguments that you pass with new will be owned by the Crypto++ objects, and their corresponding destructor will release the memory for you. They fall into the category of a sink or a filter, and it turns out that you also pass the ownership. For more details see:

https://www.cryptopp.com/wiki/Pipelining#Ownership

Basically this is what happens (super simplified example):

#include <iostream>

struct Foo{};

class X
{
    Foo *p_;
public:
    X(Foo* p): p_(p) {}
    // we'd also need a copy ctor and copy assignment operator, ignored here
    ~X()
    {
        std::cout << "Releasing the memory...\n";
        delete p_;
    }
};

int main()
{
    X x(new Foo()); // sinking, no memory leak
}

Live on Coliru

I have to say that this is by far my least favourite style of software design. One can use templates and mixins to probably achieve similar things (read about policy based design), without pointers floating around with no clear ownership.

Answer 2

I wrote some wrapper functions to encrypt/decrypt files using crypto++. I tried looking in the wiki but could find my answer. I am wondering if I need to explicitly destroy my objects that are created?

It depends. From the README under Important Usage Notes (the two items are listed):

1. If a constructor for A takes a pointer to an object B (except primitive types such as int and char), then A owns B and will delete B at A's destruction. If a constructor for A takes a reference to an object B, then the caller retains ownership of B and should not destroy it until A no longer needs it.

2. Crypto++ is thread safe at the class level. This means you can use Crypto++ safely in a multithreaded application, but you must provide synchronization when multiple threads access a common Crypto++ object.

Here's your code. It looks good, and it will not need to be changed. But we can walk though it for completeness (the CryptoPP were removed for brevity):

FileSource(inFile, true, new StreamTransformationFilter(encryptor, new FileSink(outFile)));

1. you have the stack based FileSource. Its an automatic variable and it is deleted when it goes out of scope. Its boilerplate C++.
2. you have inFile. Its a reference, you are responsible for deleting it. Its stack based, and it is deleted when it goes out of scope in the caller. Its boilerplate C++.
3. you have the StreamTransformationFilter created with new. Its a pointer and the FileSource owns it. It will be deleted when the FileSource destructor runs. Pipelines are an acquired taste.
4. you have encryptor. Its a reference, you are responsible for deleting it. Its stack based, and it is deleted when it goes out of scope. Its boilerplate C++.
5. you have the FileSink created with new. Its a pointer and the StreamTransformationFilter owns it. It will be deleted when the StreamTransformationFilter destructor runs. Pipelines are an acquired taste.
6. you have outFile. Its a reference, you are responsible for deleting it. Its stack based, and it is deleted when it goes out of scope in the caller. Its boilerplate C++.

The information is on the wiki, but its kind of hard to find if you don't know what you are looking for. Also see Pipelining | Ownership on the wiki.

---

Related, this looks suspect:

e.SetKeyWithIV(key, sizeof(key), iv);

Because key is a function parameter declared as ... byte key[], byte iv[] ..., I believe it decays to a pointer with a size of 4 (i686) or 8 (x86_64). You should use something like the following, which allows you to specify the size of the array:

bool encrypt_file(std::ifstream& inFile,
    std::ofstream& outFile,
    const byte* key, size_t ksize,
    const byte* iv, size_t vsize,
    std::string& errMsg)
{
    ...
    e.SetKeyWithIV(key, ksize, iv);
    ...
}

So, given:

byte key[AES::DEFAULT_KEYLENGTH];
prng.GenerateBlock(key, sizeof(key));
byte iv[AES::BLOCKSIZE];
prng.GenerateBlock(iv, sizeof(iv));

Then call it like so:

encrypt_file(inFile, outFile, key, sizeof(key), iv, sizeof(iv), err);