C++ program to encrypt hardcoded text

Question

This code takes in a hardcoded input buffer, maximum 64 chars, encrypts, decrypts and outputs decrypted buffer using AES CBC mechanism. Entered password is converted to a SHA256 digest and copied into a key Buffer.

Enter password: 7 er87 323nc37z3223 IV: 3EEBA5E2CDB7CBA525849CE812C2648 SHA256 password: 7902699be42c8a8e46fbbb4501726517e86b22c56a189f7625a6da49081b2451 Key buffer: 7902699BE42C8A8E46FBBB4501726517E86B22C56A189F7625A6DA49081B2451 Inp data: HELLO EQ 123566 ABCDEF 1211 34567 Encrypting... Decrypting... Decrypted output... HELLO EQ 123566 ABCDEF 1211 34567 

Entry point in main.cpp file

I know about the missing error handling in Crypto operations like new, free, etc with EVP APIs. Was lazy. :) Other than that, could you point out if there are some cpp specific problems in the code? The program works as expected. But I would like to improve my Cpp and programming skills in general. I also want to expand this to handle files, where I can input files to encrypt and outputs an encrypted file. I think it should be doable with the current design? What do you think?

main.cpp

#include "aescrypto.h" int main() { std::string file_password; std::cout << "Enter password: "; std::cin >> file_password; FileCryptoAES objAESCrypto(file_password); unsigned char inBuffer[] = "HELLO EQ 123566 ABCDEF 1211 34567"; // Initialize a vector from the array std::vector<std::uint8_t> input_vec(std::begin(inBuffer), std::end(inBuffer)); std::cout << "Inp data: " << input_vec.data() << std::endl; std::cout << "Encrypting..." << std::endl; objAESCrypto.encrypt(input_vec); std::cout << "Decrypting..." << std::endl; objAESCrypto.decrypt(); std::cout << "Decrypted output..." << std::endl; objAESCrypto.print_decrypted_buff(); return 0; } 

aescrypto.cpp

#include "aescrypto.h" #include <iomanip> #include <openssl/evp.h> #include <openssl/rand.h> void FileCryptoAES::hex_to_bytes(const std::string &hex) { unsigned char bytes[kLenKey]; for (size_t i = 0; i < kLenKey; i++) { sscanf(hex.c_str() + (i * 2), "%2hhx", &bytes[i]); } // copy from unsigned char buffer to vector mKeyBuffer.assign(bytes, bytes + kLenKey); std::cout << "Key buffer: "; for (int i = 0; i < kLenKey; i++) printf("%02X", mKeyBuffer[i]); printf("\n"); } bool FileCryptoAES::calc_sha256() { std::string hashedpass; EVP_MD_CTX *mdCtx = EVP_MD_CTX_new(); unsigned char mdVal[EVP_MAX_MD_SIZE]; unsigned int mdLen; EVP_DigestInit_ex(mdCtx, EVP_sha256(), NULL); EVP_DigestUpdate(mdCtx, get_file_password().c_str(), get_file_password().length()); EVP_DigestFinal_ex(mdCtx, mdVal, &mdLen); EVP_MD_CTX_free(mdCtx); std::stringstream ss; for (unsigned int i = 0; i < mdLen; ++i) { ss << std::hex << std::setw(2) << std::setfill('0') << (int)mdVal[i]; } hashedpass = ss.str(); set_hashed_pass(hashedpass); return true; } void FileCryptoAES::generate_key_from_pass() { calc_sha256(); std::cout << "SHA256 password: " << get_hashed_pass() << std::endl; hex_to_bytes(get_hashed_pass()); } void FileCryptoAES::encrypt(const std::vector<std::uint8_t> &inp_vec) { unsigned char *pKeyBuff = &mKeyBuffer[0]; unsigned char *pIVBuff = &mInitialisationVector[0]; unsigned char *pOutBuff = &mOutputBuffer[0]; const unsigned char *pInpBuff = reinterpret_cast<const unsigned char *>(inp_vec.data()); EVP_CIPHER_CTX *enc_Ctx = EVP_CIPHER_CTX_new(); EVP_EncryptInit_ex(enc_Ctx, EVP_aes_256_cbc(), NULL, pKeyBuff, pIVBuff); EVP_EncryptUpdate(enc_Ctx, pOutBuff, &mOutLen, pInpBuff, inp_vec.size()); EVP_EncryptFinal_ex(enc_Ctx, pOutBuff + mOutLen, &mFinalOutLen); mOutLen += mFinalOutLen; // Ensure final output length is correct EVP_CIPHER_CTX_free(enc_Ctx); } void FileCryptoAES::decrypt() { unsigned char *pKeyBuff = &mKeyBuffer[0]; unsigned char *pIVBuff = &mInitialisationVector[0]; unsigned char *pOutBuff = &mOutputBuffer[0]; unsigned char *pDecBuff = &mDecryptedBuffer[0]; EVP_CIPHER_CTX *dec_ctx = EVP_CIPHER_CTX_new(); EVP_DecryptInit_ex(dec_ctx, EVP_aes_256_cbc(), NULL, pKeyBuff, pIVBuff); EVP_DecryptUpdate(dec_ctx, pDecBuff, &mDecLen, pOutBuff, mOutLen); EVP_DecryptFinal_ex(dec_ctx, pDecBuff + mDecLen, &mFinalDecLen); mDecLen += mFinalDecLen; EVP_CIPHER_CTX_free(dec_ctx); } void FileCryptoAES::print_decrypted_buff() { for (size_t i = 0; i < mDecryptedBuffer.size(); i++) printf("%c", mDecryptedBuffer[i]); printf("\n"); } void FileCryptoAES::fill_iv_buffer() { unsigned char *pIVBuffer = &mInitialisationVector[0]; RAND_bytes(pIVBuffer, kIVLen); std::cout << "IV: "; for (int i = 0; i < kIVLen; i++) printf("%X", mInitialisationVector[i]); printf("\n"); } 

aescrypto.h

#include <iostream> #include <vector> class FileCryptoAES { private: static constexpr uint8_t kBuffSize = 64; static constexpr uint8_t kLenKey = 32; static constexpr uint8_t kIVLen = 16; std::string mfilePassword; std::string mhashedPassword; std::vector<std::uint8_t> mKeyBuffer; std::vector<std::uint8_t> mInitialisationVector; std::vector<std::uint8_t> mOutputBuffer; std::vector<std::uint8_t> mDecryptedBuffer; std::vector<std::uint8_t> mInputBuffer; int mOutLen, mFinalOutLen = 0; int mDecLen, mFinalDecLen; // Convert the first 32 hex characters (64 chars) into 32 raw bytes for AES // key void hex_to_bytes(const std::string &hex); const std::string get_file_password() { return mfilePassword; } bool calc_sha256(); void set_hashed_pass(std::string &hashed_pass) { mhashedPassword = hashed_pass; } std::string get_hashed_pass() { return mhashedPassword; } void fill_iv_buffer(); void generate_key_from_pass(); public: FileCryptoAES(std::string password) : mKeyBuffer(kLenKey), mInitialisationVector(kIVLen), mOutputBuffer(kBuffSize), mDecryptedBuffer(kBuffSize), mInputBuffer(kBuffSize) { mfilePassword = password; // Initialisation vector fill_iv_buffer(); // Generate Key from entered password generate_key_from_pass(); } void encrypt(const std::vector<std::uint8_t> &inp_vec); void decrypt(); void print_decrypted_buff(); }; 

Answer 1

Include guards

Header files should have include guards.

---

Error checking

Don't be lazy with error checking. If something bad can happen, it will, eventually. Probably at the least convenient time!

In C++, this is easier than C (mostly just check your file reads and writes if you don't enable exceptions on the streams).

For the C functions from OpenSSL, consider creating a C++ interface which can throw std::bad_alloc from failed allocation, and owns its resources (so that it cleans uup properly regardless of code path).

As a bare minimum, it's a good idea to use a smart pointer to ensure that "new" and "free" operations are paired, like this:

#include <memory> bool FileCryptoAES::calc_sha256() { auto const mdCtx = std::unique_ptr<EVP_MD_CTX, void(*)(EVP_MD_CTX*)>(EVP_MD_CTX_new(), &EVP_MD_CTX_free); 

---

Includes

aescrypto.h needs <cstdint> and <string>; doesn't need <iostream>.

aescrypto.cpp needs <cstdio> and <iostream>.

---

std namespace

We're using std::uint8_t, std::sscanf and std::printf without their namespace qualifiers. That's not something the standard allows us to assume.

---

Avoid fixed size buffers

When we construct a FileCryptoAES, we use fixed-size vectors for input and output buffers, despite knowing that input and output can be of arbitrary length.

This constructor should be explicit, and it should initialise mfilePassword (using std::move(password)) rather than assigning it after default-initialisation.

---

Unnecessary members

mFinalDecLen and mFinalOutLen don't store any useful state, and can be replaced by automatic variables in their respective function scopes.

We wouldn't need to store mfilePassword if we hashed it immediately, and only stored the hash. That also has a small security benefit if the password is re-used anywhere.

---

More const

get_file_password(), get_hashed_pass(), print_decrypted_buff() have no business modifying the file crypto object, so should be declared const.

get_file_password() current returns a const std::string, which is pointless - just return plain string.

---

Careful with formatting

fill_iv_buffer prints numbers back-to-back:

 for (int i = 0; i < kIVLen; i++) printf("%X", mInitialisationVector[i]); 

That makes it impossible to distinguish between (e.g.) 12, 3 and 1, 23.

print_decrypted_buff() dumps the whole of mDecryptedBuffer, including lots of NUL characters following the text - even though we dutifully updated mOutLen when decrypting.