Encryption with many algorithms using Crypto++

Question

For the last few days I have been working on this code to develop a class that allow me to crypt a block of byte with many algorithme.

One block will be divided into three equal blocks and every block will be encrypted and decrypted by one of the three algorithme.

I want to discover the main weak points of my code especially concerning cryptography within Crypto++.

It is my first prototype and I'm certain there are many point to discuss in this code.

#include "..\PanawaraReader\common.h" #include "cryptopp_wrapper.h" class rServer { public: public: /** rServer class has for roles : - Creation of a crypted files. - Creation of Client Applications. */ // MILESTONE 1 /** Returns a crypted block of 1024 bytes with ECB algorithm. @param _data input array of 1024 bytes. @return crypted array of 1024 bytes. */ DATA CryptBlockWithAESmodeCBC(char _data[1024]); /** Returns a crypted block of 1024 bytes with Blowfish algorithm. @param _data input array of 1024 bytes. @return crypted array. */ DATA CryptBlockWithBlowfish(char _data[1024]); /** Returns a crypted block of 1024 bytes with RSA algorithm. @param _data input array of 1024 bytes. @return crypted array of 1024 bytes. */ DATA CryptBlockWithRSA(char _data[1024]); }; 

rServer.cpp :

#include <iostream> #include "../cryptopp562/sha.h" #include "../cryptopp562/filters.h" #include "../cryptopp562/hex.h" #include <string> #include <sstream> #include "../cryptopp562/cryptlib.h" using CryptoPP::Exception; #include "../cryptopp562/hex.h" using CryptoPP::HexEncoder; using CryptoPP::HexDecoder; #include "../cryptopp562/filters.h" using CryptoPP::StringSink; using CryptoPP::StringSource; using CryptoPP::StreamTransformationFilter; #include "../cryptopp562/des.h" using CryptoPP::DES_EDE2; #include "rServer.h"; #include "../cryptopp562/modes.h" using CryptoPP::CBC_Mode; #include "../cryptopp562/secblock.h" using CryptoPP::SecByteBlock; #include <iostream> #include <string> #include "../cryptopp562/modes.h" #include "../cryptopp562/aes.h" #include "../cryptopp562/filters.h" #include "../cryptopp562/rsa.h" #include <cstdint> #include "../cryptopp562/integer.h" #include "../cryptopp562/osrng.h" using namespace std; /** Returns a crypted block of 1024 bytes with AES algorithm. @param _data input array of 1024 bytes. @return crypted array of 1024 bytes. */ DATA rServer::CryptBlockWithAESmodeCBC(char _data[1024]) { DATA d; char body[1024]; // ---------- il es conseillé d'utiliser allcoation dynamique: char * body =(char *) malloc(taille_de_la chaine);---------- char* entete; // entete = iv+ key char typeAlgo[1024]="AES"; // mode CBC char* texteChiffre; std::string key = "0123456789abcdef"; std::string iv = "aaaaaaaaaaaaaaaa"; //string plain = "CBC Mode Test"; string cipher, encoded, recovered; std::string plaintext = _data; std::string ciphertext; std::string decryptedtext; cout<<"****************** Algorithme AES *****************"<<endl<<endl; std::cout << " Plain Text (" << plaintext.size() << " bytes): " ; std::cout << " "+plaintext; std::cout << std::endl << std::endl; CryptoPP::AES::Encryption aesEncryption((byte *)key.c_str(), CryptoPP::AES::DEFAULT_KEYLENGTH); CryptoPP::CBC_Mode_ExternalCipher::Encryption cbcEncryption( aesEncryption, (byte *)iv.c_str() ); CryptoPP::StreamTransformationFilter stfEncryptor(cbcEncryption, new CryptoPP::StringSink( ciphertext ) ); stfEncryptor.Put( reinterpret_cast<const unsigned char*>( plaintext.c_str() ), plaintext.length() + 1 ); stfEncryptor.MessageEnd(); //cout << "cipher text plain: " << ciphertext << endl; //std::cout << "Cipher Text (" << ciphertext.size() << " bytes)" << std::endl; texteChiffre= (char*)ciphertext.c_str(); /*std::cout <<"cipher text In HEX FORM:: "; for( int i = 0; i < ciphertext.size(); i++ ) { std::cout << "0x" << std::hex << (0xFF & static_cast<byte>(ciphertext[i])) << " "; } cout << endl; cout << endl;*/ strcpy(d.body,texteChiffre); strcpy(d.header.typeAlgo,typeAlgo); char* vector=(char*)iv.c_str(); strcpy(d.header.vector,vector); char* keyChar=(char*)key.c_str(); strcpy(d.header.key1,keyChar); return d; } /** Returns a crypted block of 1024 bytes with Blowfish algorithm. @param _data input array of 1024 bytes. @return crypted array. */ DATA rServer::CryptBlockWithBlowfish(char _data[1024]) { DATA d; char body[1024]; // ---------- il est conseillé d'utiliser allcoation dynamique: char * body =(char *) malloc(taille_de_la chaine);---------- char* blKeyChar; // entete = key char typeAlgo[1024]="BLF"; char* texteChiffre; cout<<"****************** Algorithme BlowFish ******************"<<endl<<endl; SpaceCrypto::CryptBlowFish hello; hello.setPlainString(_data); hello.setKey("mySecUreKey!!"); std::string crypt; crypt = hello.Encrypt(); cout<<" Plain Text: "<< _data <<endl; cout << endl; texteChiffre= (char*)crypt.c_str(); strcpy(d.body,texteChiffre); std::string blKey=hello.getKey(); blKeyChar=(char*)blKey.c_str(); strcpy(d.header.typeAlgo,typeAlgo); strcpy(d.header.key1,blKeyChar); return d; } /** Returns a crypted block of 1024 bytes with RSA algorithm. @param _data input array of 1024 bytes. @return crypted array of 1024 bytes. */ DATA rServer::CryptBlockWithRSA(char _data[1024]) { DATA data; char body[1024]; // ---------- il es conseillé d'utiliser allcoation dynamique: char * body =(char *) malloc(taille_de_la chaine);---------- char* entete; // entete = key char typeAlgo[4]="RSA"; char* texteChiffre; cout<<"****************** Algorithme RSA ******************"<<endl<<endl; // Keys // La clé publique est la paire (e, n) et la clé secrète est d, donc aussi p et q. // p = 3, q = 11, n = 3 x 11, f = (11–1).(3–1) = 20. On choisit d=7 (7 et 20 sont bien premiers entre eux). // e = 3 car e.d= 20 * 1 + 1 CryptoPP::Integer n("0xbeaadb3d839f3b5f"), e("0x11"), d("0x21a5ae37b9959db9"); CryptoPP::RSA::PrivateKey privKey; privKey.Initialize(n, e, d); CryptoPP::RSA::PublicKey pubKey; pubKey.Initialize(n,e); //convert char _data[1024] to string string msg; msg= _data; // Encryption: In RSA, encryption is simply c = me. So our task is to encode the string as an Integer in preparation for encryption. CryptoPP::Integer m((const byte *)msg.data(), msg.size()); //m (the word secret) is encoded as an integer. We can use C++'s insertion operator to inspect m: /*std::cout << "m: " << m << std::endl;*/ //At this point, we have n, e, d and m. To encrypt m, we perform the following. //ApplyFunction is the 'easy to compute' transformation. CryptoPP::Integer c = pubKey.ApplyFunction(m); /*std::cout << "c: " << std::hex << c << std::endl;*/ // conversion CryptoPP::Integer to char* std::stringstream ss; ss << c; std::string s = ss.str(); texteChiffre= (char*)s.c_str(); strcpy(data.body,texteChiffre); strcpy(data.header.typeAlgo,typeAlgo); return data; } 

Answer 1

I have never used Crypto++, but can give you several suggestion on how to improve the overall style and quality of your code.

Missing an include guard:

Your header file doesn't have an include guard. You will need one to be able to include the header in other .cpp files. Note that #pragma once is also viable.

Type and variable naming:

It is unusual to start the name of a type in C++ with a lower-case letter (rServer). This form of camelCaseing is usually applied to variables and object instances. User defined C++ types are commonly named using PascalCase (first letter uppercase).

DATA CryptBlockWithAESmodeCBC(char _data[1024]); 

Regarding the _data param, names starting with _ may be reserved for compiler/library use. See this SO thread. It is best to avoid those.

Declaring variables at the top of a function:

Declaring all variables at the beginning of a function is considered bad practice in C++. Variables should be declared when they are first used or in a way to be visible to the needed scope(s) only. Variables declared at the top of a function are like mini-globals.

using namespace:

using namespace std (or any namespace for that matter) is usually not a good practice because it defeats the purpose of a namespace, which is to allow identical names to coexist without clashes. You can read more about this here.

Also, I would group all the #includes and all the using directives in the .cpp file. E.g.:

#include "../cryptopp562/hex.h" ... all the inslcudes ... using CryptoPP::StringSink; ... all the usings ... 

Using cout directly:

Your code is cluttered with log calls to std::cout. You shouldn't be using it directly inside functions like you did. Most users won't want the verbose output on every execution of the program. If you need them for debugging, a better approach is to wrap those calls in a macro that can be disabled at compile time, e.g.:

#define DEBUG_LOG(msg) do { std::cout << msg << std::end; } while(0) 

Magic constants:

What are these supposed to mean?

std::string key = "0123456789abcdef"; std::string iv = "aaaaaaaaaaaaaaaa"; 

And these?

CryptoPP::Integer n("0xbeaadb3d839f3b5f"), e("0x11"), d("0x21a5ae37b9959db9"); 

That's pretty unclear. Define named constants for those, using a const char[] or a const std::string, and of course, give the constants meaningful names.

Using old C-string functions:

Mixing std::string with strcpy & friends is totally unnecessary. std::string has all the methods you need. E.g.: operator +, +=, append(), etc.

texteChiffre= (char*)crypt.c_str(); strcpy(d.body,texteChiffre); std::string blKey=hello.getKey(); blKeyChar=(char*)blKey.c_str(); strcpy(d.header.typeAlgo,typeAlgo); strcpy(d.header.key1,blKeyChar); 

I suspect you did this because DATA is using a char array internally. If that is the case, can't you also use an std::string for it? It would also be more efficient since you could then avoid a string copy altogether by just using C++11's std::move: data.body = std::move(ciphertext);

Avoid C-style casts:

C-style casts are ugly, unsafe and provide no compiler diagnostics.

texteChiffre = (char*)ciphertext.c_str(); 

Always use the proper C++ cast operators. The cast above is wrong BTW. You should never cast away the constess of a pointer returned by std::string::c_str().

Clear commented out code:

You have a few commented out blocks. Please remove those once you are done experimenting. This is only visual pollution.

Be consistent with your comments and var names:

If you are writing code that will be read by programmers around the world, then all comments and variables/type-names should be in English.

Answer 2

Encryption with many algorithms using Crypto++...

Let me make a quick comment on algorithm agility, and block cipher agility in particular, because this seems to be a goal of your project.

Having an explicit exported function for each algorithm is OK. The C++ way to do it is with templates and function templates, but here's article that discusses why it may not be the best strategy: Why Not Specialize Function Templates?. In fact, it solves the problem of explicit instantiations in library code.

• EncryptWith3DES(...)
• EncryptWithAES(...)
• EncryptWithCamellia(...)
• ...

With you exported functions available, you would only need a function to return a pointer to a StreamTransformation:

void EncryptWith3DES(...) { string algorithm = "DES-EDE3-CBC"; auto_ptr<StreamTransformation> stream; GetBlockCipherEncryptor(algorithm, stream); ... } 

Then, something like:

void GetBlockCipherEncryptor(string algorithm, auto_ptr<StreamTransformation>& stream) { std::transform(algorithm.begin(), algorithm.end(), algorithm.begin(), (int(*)(int))std::toupper); if(algorithm == "AES-256-CBC") { stream = auto_ptr<StreamTransformation>(new CBC_Mode<AES>::Encryption); } else if(algorithm == "AES-192-CBC") { stream = auto_ptr<StreamTransformation>(new CBC_Mode<AES>::Encryption); } else if(algorithm == "AES-128-CBC") { stream = auto_ptr<StreamTransformation>(new CBC_Mode<AES>::Encryption); } else if(algorithm == "DES-EDE3-CBC") { stream = auto_ptr<StreamTransformation>(new CBC_Mode<DES_EDE3>::Encryption); } ... else { throw NotImplemented("GetBlockCipherEncryptor: '" + algorithm + "' is not implemented"); } } 

In fact, I wrote the PEM Pack for Crypto++, and I use the exact same code in it (that's where this was copied/pasted from). See pem-rd.cpp and pem-wr.cpp.

Now, continuing with the example:

void EncryptWith3DES(...) { string algorithm = "DES-EDE3-CBC"; auto_ptr<StreamTransformation> stream; GetBlockCipherEncryptor(algorithm, stream); SecByteBlock master; GetMasterSecret(master); } 

The code above gets the "master" secret. Its can be a user password or something stuffed in a KeyChain or KeyStore.

With a master secret, you want to derive a key and an IV:

SecByteBlock master; GetMasterSecret(master); SecByteBlock key, iv; DeriveKeyAndIV(master, algorithm, key, 24, iv, 8); 

DeriveKeyAndIV needs to know the key size and the IV size. It also uses the algorithm so that derived keys and IVs are different for each algorithm.

With a key and IV, you want to key your cipher object:

SecByteBlock key, iv; DeriveKeyAndIV(master, algorithm, key, 24, iv, 8); stream->SetKeyWithIV(key.data(), key.size(), iv.data(), iv.size()); 

Now, you are ready to encrypt data:

void EncryptWith3DES(...) { string algorithm = "DES-EDE3-CBC"; auto_ptr<StreamTransformation> stream; GetBlockCipherEncryptor(algorithm, stream); SecByteBlock master; GetMasterSecret(master); SecByteBlock key, iv; DeriveKeyAndIV(master, algorithm, key, 24, iv, 8); stream->SetKeyWithIV(key.data(), key.size(), iv.data(), iv.size()); return EncryptData(stream, plain, cipher); } 

Your EncryptData is generic, and it takes the encryptor, the plain text data, and the cipher text data object. I used string, but you can use a SecByteBlock, a vector, etc.

EncryptData(auto_ptr<StreamTransformation>& stream, const string& plain, string& cipher) { StreamTransformationFilter filter(stream, new StringSink(cipher)); filter.Put(plain.data(), plain.size()); filter.MessageEnd(); } 

Below is what DeriveKeyAndIV might look like. It will produce an independent key and IV based on the master secret and algorithm you provide. The iteration count is low because independent derivation is the goal. That is, the key and IV will be independently derived, and they will be different for each algorithm.

If you want hardening like unique salts and high iteration counts, then you should do it on the master secret before its passed into this routine.

void DeriveKeyAndIV(const SecByteBlock& master, const string& algorithm, SecByteBlock& key, unsigned int ksize, SecByteBlock& iv, unsigned int vsize) { SecByteBlock temp(32); PKCS5_PBKDF2_HMAC< SHA256 > kdf; byte usage_1[] = { 'm','a','s','t','e','r',' ','s','e','c','r','e','t' }; byte usage_2[] = { 'k','e','y',' ','d','e','r','i','v','a','t','i','o','n' }; byte usage_3[] = { 'i','v',' ','d','e','r','i','v','a','t','i','o','n' }; // Derive on the master key kdf.DeriveKey(temp.data(), temp.size(), 0, master.data(), master.size(), usage_1, sizeof(usage_1), 100); // Derive on algorithm usage kdf.DeriveKey(temp.data(), temp.size(), 0, temp.data(), temp.size(), reinterpret_cast<const byte*>(algorithm.data()), algorithm.size(), 100); // Derive key from the temp key.resize(ksize); kdf.DeriveKey(key.data(), key.size(), 0, temp.data(), temp.size(), usage_2, sizeof(usage_2), 100); // Derive iv from the temp iv.resize(vsize); kdf.DeriveKey(iv.data(), iv.size(), 0, temp.data(), temp.size(), usage_3, sizeof(usage_3), 100); }