views:

116

answers:

2

In order that I might feed AES encrypted text as an std::istream to a parser component I am trying to create a std::streambuf implementation wrapping the vanilla crypto++ encryption/decryption.

The main() function calls the following functions to compare my wrapper with the vanilla implementation:

  • EncryptFile() - encrypt file using my streambuf implementation
  • DecryptFile() - decrypt file using my streambuf implementation
  • EncryptFileVanilla() - encrypt file using vanilla crypto++
  • DecryptFileVanilla() - decrypt file using vanilla crypto++

The problem is that whilst the encrypted files created by EncryptFile() and EncryptFileVanilla() are identical. The decrypted file created by DecryptFile() is incorrect being 16 bytes short of that created by DecryptFileVanilla(). Probably not coincidentally the block size is also 16.

I think the issue must be in CryptStreamBuffer::GetNextChar(), but I've been staring at it and the crypto++ documentation for hours.

Can anybody help/explain?

Any other comments about how crummy or naive my std::streambuf implementation are also welcome ;-)

Thanks,

Tom

// Runtime Includes
#include <iostream>

// Crypto++ Includes
#include "aes.h"
#include "modes.h"      // xxx_Mode< >
#include "filters.h"    // StringSource and
                        // StreamTransformation
#include "files.h"

using namespace std;

class CryptStreamBuffer: public std::streambuf {

public:

    CryptStreamBuffer(istream& encryptedInput, CryptoPP::StreamTransformation& c);

    CryptStreamBuffer(ostream& encryptedOutput, CryptoPP::StreamTransformation& c);

    ~CryptStreamBuffer();

protected:
    virtual int_type overflow(int_type ch = traits_type::eof());

    virtual int_type uflow();

    virtual int_type underflow();

    virtual int_type pbackfail(int_type ch);

    virtual int sync();

private:
    int GetNextChar();

    int m_NextChar; // Buffered character

    CryptoPP::StreamTransformationFilter* m_StreamTransformationFilter;

    CryptoPP::FileSource* m_Source;

    CryptoPP::FileSink* m_Sink;

}; // class CryptStreamBuffer

CryptStreamBuffer::CryptStreamBuffer(istream& encryptedInput, CryptoPP::StreamTransformation& c) :
    m_NextChar(traits_type::eof()),
    m_StreamTransformationFilter(0),
    m_Source(0),
    m_Sink(0) {

    m_StreamTransformationFilter = new CryptoPP::StreamTransformationFilter(c, 0, CryptoPP::BlockPaddingSchemeDef::PKCS_PADDING);
    m_Source = new CryptoPP::FileSource(encryptedInput, false, m_StreamTransformationFilter);
}

CryptStreamBuffer::CryptStreamBuffer(ostream& encryptedOutput, CryptoPP::StreamTransformation& c) :
    m_NextChar(traits_type::eof()),
    m_StreamTransformationFilter(0),
    m_Source(0),
    m_Sink(0) {

    m_Sink = new CryptoPP::FileSink(encryptedOutput);
    m_StreamTransformationFilter = new CryptoPP::StreamTransformationFilter(c, m_Sink, CryptoPP::BlockPaddingSchemeDef::PKCS_PADDING);
}

CryptStreamBuffer::~CryptStreamBuffer() {

    if (m_Sink) {
        delete m_StreamTransformationFilter;
        // m_StreamTransformationFilter owns and deletes m_Sink.
    }
    if (m_Source) {
        delete m_Source;
        // m_Source owns and deletes m_StreamTransformationFilter.
    }
}

CryptStreamBuffer::int_type CryptStreamBuffer::overflow(int_type ch) {

    return m_StreamTransformationFilter->Put((byte)ch);
}

CryptStreamBuffer::int_type CryptStreamBuffer::uflow() {

    int_type result = GetNextChar();

    // Reset the buffered character
    m_NextChar = traits_type::eof();

    return result;
}

CryptStreamBuffer::int_type CryptStreamBuffer::underflow() {

    return GetNextChar();
}

CryptStreamBuffer::int_type CryptStreamBuffer::pbackfail(int_type ch) {

    return traits_type::eof();
}

int CryptStreamBuffer::sync() {

    // TODO: Not sure sync is the correct place to be doing this.
    //       Should it be in the destructor?
    if (m_Sink) {
        m_StreamTransformationFilter->MessageEnd();
        // m_StreamTransformationFilter->Flush(true);
    }

    return 0;
}

int CryptStreamBuffer::GetNextChar() {

    // If we have a buffered character do nothing
    if (m_NextChar != traits_type::eof()) {
        return m_NextChar;
    }

    // If there are no more bytes currently available then pump the source
    if (m_StreamTransformationFilter->MaxRetrievable() == 0) {
        m_Source->Pump(1024);
    }

    // Retrieve the next byte
    byte nextByte;
    size_t noBytes = m_StreamTransformationFilter->Get(nextByte);
    if (0 == noBytes) {
        return traits_type::eof();
    }

    // Buffer up the next character
    m_NextChar = nextByte;

    return m_NextChar;
}

void InitKey(byte key[]) {

    key[0] = -62;
    key[1] = 102;
    key[2] = 78;
    key[3] = 75;
    key[4] = -96;
    key[5] = 125;
    key[6] = 66;
    key[7] = 125;
    key[8] = -95;
    key[9] = -66;
    key[10] = 114;
    key[11] = 22;
    key[12] = 48;
    key[13] = 111;
    key[14] = -51;
    key[15] = 112;
}

/** Decrypt using my CryptStreamBuffer */
void DecryptFile(const char* sourceFileName, const char* destFileName) {

    ifstream ifs(sourceFileName, ios::in | ios::binary);
    ofstream ofs(destFileName, ios::out | ios::binary);

    byte key[CryptoPP::AES::DEFAULT_KEYLENGTH];
    InitKey(key);

    CryptoPP::ECB_Mode<CryptoPP::AES>::Decryption decryptor(key, sizeof(key));

    if (ifs) {
        if (ofs) {
            CryptStreamBuffer cryptBuf(ifs, decryptor);
            std::istream decrypt(&cryptBuf);

            int c;
            while (EOF != (c = decrypt.get())) {
                ofs << (char)c;
            }
            ofs.flush();
        }
        else {
            std::cerr << "Failed to open file '" << destFileName << "'." << endl;
        }
    }
    else {
        std::cerr << "Failed to open file '" << sourceFileName << "'." << endl;
    }  
}

/** Encrypt using my CryptStreamBuffer */
void EncryptFile(const char* sourceFileName, const char* destFileName) {

    ifstream ifs(sourceFileName, ios::in | ios::binary);
    ofstream ofs(destFileName, ios::out | ios::binary);

    byte key[CryptoPP::AES::DEFAULT_KEYLENGTH];
    InitKey(key);

    CryptoPP::ECB_Mode<CryptoPP::AES>::Encryption encryptor(key, sizeof(key));

    if (ifs) {
        if (ofs) {
            CryptStreamBuffer cryptBuf(ofs, encryptor);
            std::ostream encrypt(&cryptBuf);

            int c;
            while (EOF != (c = ifs.get())) {
                encrypt << (char)c;
            }
            encrypt.flush();
        }
        else {
            std::cerr << "Failed to open file '" << destFileName << "'." << endl;
        }
    }
    else {
        std::cerr << "Failed to open file '" << sourceFileName << "'." << endl;
    }  
}

/** Decrypt using vanilla crypto++ */
void DecryptFileVanilla(const char* sourceFileName, const char* destFileName) {

    byte key[CryptoPP::AES::DEFAULT_KEYLENGTH];
    InitKey(key);

    CryptoPP::ECB_Mode<CryptoPP::AES>::Decryption decryptor(key, sizeof(key));

    CryptoPP::FileSource(sourceFileName, true,
      new CryptoPP::StreamTransformationFilter(decryptor,
        new CryptoPP::FileSink(destFileName), CryptoPP::BlockPaddingSchemeDef::PKCS_PADDING
      ) // StreamTransformationFilter
    ); // FileSource
}

/** Encrypt using vanilla crypto++ */
void EncryptFileVanilla(const char* sourceFileName, const char* destFileName) {

    byte key[CryptoPP::AES::DEFAULT_KEYLENGTH];
    InitKey(key);

    CryptoPP::ECB_Mode<CryptoPP::AES>::Encryption encryptor(key, sizeof(key));

    CryptoPP::FileSource(sourceFileName, true,
      new CryptoPP::StreamTransformationFilter(encryptor,
        new CryptoPP::FileSink(destFileName), CryptoPP::BlockPaddingSchemeDef::PKCS_PADDING
      ) // StreamTransformationFilter
    ); // FileSource
}

int main(int argc, char* argv[])
{
    EncryptFile(argv[1], "encrypted.out");
    DecryptFile("encrypted.out", "decrypted.out");
    EncryptFileVanilla(argv[1], "encrypted_vanilla.out");
    DecryptFileVanilla("encrypted_vanilla.out", "decrypted_vanilla.out");
    return 0;
}
+3  A: 

If your input buffer is not a multiplicity of a 16-byte block, you need to stuff the last block with dummy bytes. If the last block is less than 16 bytes it is dropped by crypto++ and not encrypted. When decrypting, you need to truncate the dummy bytes. That 'another way' you are referring to, already does the addition and truncation for you. So what should be the dummy bytes, to know how many of them there is, thus should be truncated? I use the following pattern: fill each byte with the value of dummies count.

Examples: You need to add 8 bytes? set them to 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08. You need to add 3 bytes? set them to 0x03, 0x03, 0x03 etc.

When decrypting, get the value of last byte of the output buffer. Assume it is N. Check, if the values last N bytes are equal to N. Truncate, if true.

UPDATE:

CryptStreamBuffer::CryptStreamBuffer(istream& encryptedInput, CryptoPP::StreamTransformation& c) :
    m_NextChar(traits_type::eof()),
    m_StreamTransformationFilter(0),
    m_Source(0),
    m_Sink(0) {

    m_StreamTransformationFilter = new CryptoPP::StreamTransformationFilter(c, 0, CryptoPP::BlockPaddingSchemeDef::ZEROS_PADDING);
    m_Source = new CryptoPP::FileSource(encryptedInput, false, m_StreamTransformationFilter);
}

CryptStreamBuffer::CryptStreamBuffer(ostream& encryptedOutput, CryptoPP::StreamTransformation& c) :
    m_NextChar(traits_type::eof()),
    m_StreamTransformationFilter(0),
    m_Source(0),
    m_Sink(0) {

    m_Sink = new CryptoPP::FileSink(encryptedOutput);
    m_StreamTransformationFilter = new CryptoPP::StreamTransformationFilter(c, m_Sink, CryptoPP::BlockPaddingSchemeDef::ZEROS_PADDING);
}

Setting the ZEROS_PADDING made your code working (tested on text files). However why it does not work with DEFAULT_PADDING - I did not find the cause yet.

AOI Karasu
I do not believe this is the problem. With crypto++ the StreamTransformationFilter automatically stuffs the last block. That is what the call to MessageEnd() is for in sync(). Though I don't think sync() is ultimately the correct place for it.
Tom Williams
`int CryptStreamBuffer::sync() { if (m_Sink) { m_StreamTransformationFilter->MessageEnd(); } }`Why there is no return value here? Does your code compile? I cannot compile it?
AOI Karasu
Um. What if my file just so happens to end in the byte `0x01`? Prepend the data size to the data before padding if you need this kind of thing: for strong cryptography doing so won't provide a useful crib.
Steve Jessop
@Steve: good point, I use this only for text files, however.
AOI Karasu
@Steve: I believe the standard solution in this case (file ends on block boundary) is to add a whole block of padding characters (e.g. 16 bytes of 0x0f).
Amnon
@Amnon @Steve: No, 16 bytes of `0x10`. `0x0f` = 15.
Brian
Like I said I do not believe I need to worry about the message padding, it is handled automatically by the CryptoPP::StreamTransformationFilter. I have also updated my example to show that the encrypted file generated via the crypto++ vanilla methods is identical to that created by my streambuf wrapper. So the issue has to be on the decryption side.
Tom Williams
+3  A: 

After working with a debug build of crypto++ it turns out that what was missing was a call to the StreamTransformationFilter advising it that there would be nothing more coming from the Source and that it should wrap up the processing of the final few bytes, including the padding.

In CryptStreamBuffer::GetNextChar():

Replace:

// If there are no more bytes currently available then pump the source
if (m_StreamTransformationFilter->MaxRetrievable() == 0) {
    m_Source->Pump(1024);
}

With:

// If there are no more bytes currently available from the filter then
// pump the source.
if (m_StreamTransformationFilter->MaxRetrievable() == 0) {
    if (0 == m_Source->Pump(1024)) {
        // This seems to be required to ensure the final bytes are readable
        // from the filter.
        m_StreamTransformationFilter->ChannelMessageEnd(CryptoPP::DEFAULT_CHANNEL);
    }
}

I make no claims that this is the best solution, just one I discovered by trial and error that appears to work.

Tom Williams