From 8f381bac6627a23c4082003a1b41c1b0657344ef Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Wed, 8 Nov 2023 16:23:09 +0000 Subject: [PATCH] HPCC-30214 Use openssl aes encrypt/decrypt functions Signed-off-by: Richard Chapman --- roxie/udplib/udpmsgpk.cpp | 4 +- system/jlib/jencrypt.cpp | 195 +++++++++++++++++++++++++++++++- system/jlib/jencrypt.hpp | 16 ++- testing/unittests/jlibtests.cpp | 70 ++++++++++++ 4 files changed, 274 insertions(+), 11 deletions(-) diff --git a/roxie/udplib/udpmsgpk.cpp b/roxie/udplib/udpmsgpk.cpp index c601ac8cfe6..c2cfff81d18 100644 --- a/roxie/udplib/udpmsgpk.cpp +++ b/roxie/udplib/udpmsgpk.cpp @@ -196,10 +196,8 @@ class PackageSequencer : public CInterface, implements IInterface // MORE - could argue that we would prefer to wait even longer - until we know consumer wants it - but that might be complex if (encrypted) { - // MORE - This is decrypting in-place. Is that ok?? Seems to be with the code we currently use, but if that changed - // might need to rethink this const MemoryAttr &udpkey = getSecretUdpKey(true); - size_t decryptedSize = aesDecrypt(udpkey.get(), udpkey.length(), pktHdr+1, pktHdr->length-sizeof(UdpPacketHeader), pktHdr+1, DATA_PAYLOAD-sizeof(UdpPacketHeader)); + size_t decryptedSize = aesDecryptInPlace(udpkey.get(), udpkey.length(), pktHdr+1, pktHdr->length-sizeof(UdpPacketHeader)); pktHdr->length = decryptedSize + sizeof(UdpPacketHeader); } diff --git a/system/jlib/jencrypt.cpp b/system/jlib/jencrypt.cpp index a0020f5f008..c79d8fd7343 100644 --- a/system/jlib/jencrypt.cpp +++ b/system/jlib/jencrypt.cpp @@ -21,6 +21,10 @@ #ifdef _USE_OPENSSL #include "ske.hpp" +#include +#include +#include +#include #endif @@ -1814,14 +1818,14 @@ MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *input, size return output; } -size_t aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, void *output, size_t outlen) +size_t aesDecryptInPlace(const void *key, size_t keylen, void *data, size_t inlen) { Rijndael rin; Rijndael::KeyLength keyType = getAesKeyType(keylen); rin.init(Rijndael::CBC, Rijndael::Decrypt, (const UINT8 *)key, keyType); size32_t truncInLen = (size32_t)inlen; - int len = rin.padDecrypt((const UINT8 *)input, truncInLen, (UINT8 *) output, outlen); + int len = rin.padDecrypt((const UINT8 *)data, truncInLen, (UINT8 *) data, inlen); if(len < 0) throw MakeStringException(-1,"AES Decryption error: %d, %s", len, getAesErrorText(len)); return len; @@ -1829,19 +1833,200 @@ size_t aesDecrypt(const void *key, size_t keylen, const void *input, size_t inle } // end of namespace jlib +#ifdef _USE_OPENSSL +namespace openssl { + +static void encryptError(const char *what) +{ + VStringBuffer s("openssl::aesEncrypt: %s", what); + throw makeStringException(0, s.str()); +} + +MemoryBuffer &aesEncrypt(const void *key, size_t keylen, const void *plaintext, size_t plaintext_len, MemoryBuffer &output) +{ + if (!plaintext || !plaintext_len) + return output; + EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); + if (!ctx) + encryptError("Failed to create context"); + try + { + unsigned char iv[16] = { 0 }; + switch (keylen) + { + case 32: + if(1 != EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, (const unsigned char *) key, iv)) + encryptError("Failed to initialize context"); + break; + case 24: + if(1 != EVP_EncryptInit_ex(ctx, EVP_aes_192_cbc(), NULL, (const unsigned char *) key, iv)) + encryptError("Failed to initialize context"); + break; + case 16: + if(1 != EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, (const unsigned char *) key, iv)) + encryptError("Failed to initialize context"); + break; + default: + encryptError("Unsupported key length"); + break; + } + byte *ciphertext = (byte *) output.reserve(plaintext_len + 100); + int ciphertext_len = 0; + int thislen = 0; + if(1 != EVP_EncryptUpdate(ctx, ciphertext, &thislen, (const unsigned char *) plaintext, plaintext_len)) + encryptError("Error in EVP_EncryptUpdate"); + ciphertext_len += thislen; + if(1 != EVP_EncryptFinal_ex(ctx, ciphertext + ciphertext_len, &thislen)) + encryptError("Error in EVP_EncryptFinal_ex"); + ciphertext_len += thislen; + EVP_CIPHER_CTX_free(ctx); + output.setLength(ciphertext_len); + return output; + } + catch (...) + { + EVP_CIPHER_CTX_free(ctx); + throw; + } +} + +static void decryptError(const char *what) +{ + VStringBuffer s("openssl::aesDecrypt: unexpected failure in %s", what); + throw makeStringException(0, s.str()); +} + +MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *ciphertext, size_t ciphertext_len, MemoryBuffer &output) +{ + if (!ciphertext || !ciphertext_len) + return output; + EVP_CIPHER_CTX *ctx; + + int thislen = 0; + int plaintext_len = 0; + + if(!(ctx = EVP_CIPHER_CTX_new())) + decryptError("Failed to create context"); + try + { + unsigned char iv[16] = { 0 }; + switch (keylen) + { + case 32: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + case 24: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_192_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + case 16: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + default: + decryptError("Unsupported key length"); + break; + } + byte *plaintext = (byte *) output.reserve(ciphertext_len); + if(1 != EVP_DecryptUpdate(ctx, plaintext, &thislen, (const unsigned char *) ciphertext, ciphertext_len)) + decryptError("Error in EVP_DecryptUpdate"); + plaintext_len += thislen; + + if(1 != EVP_DecryptFinal_ex(ctx, plaintext + plaintext_len, &thislen)) + decryptError("Error in EVP_DecryptFinal_ex"); + plaintext_len += thislen; + output.setLength(plaintext_len); + EVP_CIPHER_CTX_free(ctx); + return output; + } + catch (...) + { + EVP_CIPHER_CTX_free(ctx); + throw; + } +} + +size_t aesDecryptInPlace(const void *key, size_t keylen, void *ciphertext, size_t ciphertext_len) +{ + if (!ciphertext || !ciphertext_len) + return 0; + EVP_CIPHER_CTX *ctx; + + int thislen = 0; + size_t plaintext_len = 0; + + if(!(ctx = EVP_CIPHER_CTX_new())) + decryptError("Failed to create context"); + + try + { + unsigned char iv[16] = { 0 }; + switch (keylen) + { + case 32: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + case 24: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_192_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + case 16: + if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, (const unsigned char *) key, iv)) + decryptError("Failed to initialize context"); + break; + default: + decryptError("Unsupported key length"); + break; + } + byte *plaintext = (byte *) ciphertext; + if(1 != EVP_DecryptUpdate(ctx, plaintext, &thislen, (const unsigned char *) ciphertext, ciphertext_len)) + decryptError("Error in EVP_DecryptUpdate"); + plaintext_len += thislen; + + if(1 != EVP_DecryptFinal_ex(ctx, plaintext + plaintext_len, &thislen)) + decryptError("Error in EVP_DecryptFinal_ex"); + plaintext_len += thislen; + assertex(plaintext_len <= ciphertext_len); + EVP_CIPHER_CTX_free(ctx); + return plaintext_len; + } + catch (...) + { + EVP_CIPHER_CTX_free(ctx); + throw; + } +} + +} // namespace +#endif + MemoryBuffer &aesEncrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output) { +#ifdef _USE_OPENSSL + return openssl::aesEncrypt(key, keylen, input, inlen, output); +#else return jlib::aesEncrypt(key, keylen, input, inlen, output); +#endif } MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output) { +#ifdef _USE_OPENSSL + return openssl::aesDecrypt(key, keylen, input, inlen, output); +#else return jlib::aesDecrypt(key, keylen, input, inlen, output); +#endif } -size_t aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, void *output, size_t outlen) +size_t aesDecryptInPlace(const void *key, size_t keylen, void *data, size_t inlen) { - return jlib::aesDecrypt(key, keylen, input, inlen, output, outlen); +#ifdef _USE_OPENSSL + return openssl::aesDecryptInPlace(key, keylen, data, inlen); +#else + return jlib::aesDecryptInPlace(key, keylen, data, inlen); +#endif } #define CRYPTSIZE 32 @@ -1874,3 +2059,5 @@ void decrypt(StringBuffer &ret, const char *in) } } + + diff --git a/system/jlib/jencrypt.hpp b/system/jlib/jencrypt.hpp index 320796a52d8..b9711ca394a 100644 --- a/system/jlib/jencrypt.hpp +++ b/system/jlib/jencrypt.hpp @@ -30,14 +30,23 @@ namespace jlib { extern jlib_decl MemoryBuffer &aesEncrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); extern jlib_decl MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); - extern jlib_decl size_t aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, void *output, size_t outlen); + extern jlib_decl size_t aesDecryptInPlace(const void *key, size_t keylen, void *data, size_t inlen); } // end of namespace jlib; +#ifdef _USE_OPENSSL +namespace openssl +{ + extern jlib_decl MemoryBuffer &aesEncrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); + extern jlib_decl MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); + extern jlib_decl size_t aesDecryptInPlace(const void *key, size_t keylen, void *data, size_t inlen); +} // end of namespace openssl; +#endif + // NB: these are wrappers to either the openssl versions (if USE_OPENSSL) or the jlib version. -// MORE - are they?? I see no evidence of that! + extern jlib_decl MemoryBuffer &aesEncrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); extern jlib_decl MemoryBuffer &aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, MemoryBuffer &output); -extern jlib_decl size_t aesDecrypt(const void *key, size_t keylen, const void *input, size_t inlen, void *output, size_t outlen); +extern jlib_decl size_t aesDecryptInPlace(const void *key, size_t keylen, void *data, size_t inlen); #define encrypt _LogProcessError12 @@ -46,7 +55,6 @@ extern jlib_decl size_t aesDecrypt(const void *key, size_t keylen, const void *i extern jlib_decl void encrypt(StringBuffer &ret, const char *in); extern jlib_decl void decrypt(StringBuffer &ret, const char *in); - // simple inline block scrambler (shouldn't need jlib_decl) class Csimplecrypt { diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp index 862eb0d75e5..ce4c591cf55 100644 --- a/testing/unittests/jlibtests.cpp +++ b/testing/unittests/jlibtests.cpp @@ -3758,6 +3758,76 @@ class JLibUnicodeTest : public CppUnit::TestFixture CPPUNIT_TEST_SUITE_REGISTRATION( JLibUnicodeTest ); CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JLibUnicodeTest, "JLibUnicodeTest" ); +#ifdef _USE_OPENSSL +#include + +class JLibOpensslAESTest : public CppUnit::TestFixture +{ +public: + CPPUNIT_TEST_SUITE(JLibOpensslAESTest); + CPPUNIT_TEST(test); + CPPUNIT_TEST_SUITE_END(); + +protected: + + void testOne(unsigned len, const char *intext) + { + /* A 256 bit key */ + unsigned char key[] = { 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, + 0x36, 0x37, 0x38, 0x39, 0x30, 0x31, 0x32, 0x33, + 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x31 + }; + + MemoryBuffer ciphertext1, ciphertext2, decrypted1, decrypted2; + + openssl::aesEncrypt(key, 32, intext, len, ciphertext1); + jlib::aesEncrypt(key, 32, intext, len, ciphertext2); + + CPPUNIT_ASSERT(ciphertext1.length()==ciphertext2.length()); + CPPUNIT_ASSERT(memcmp(ciphertext1.bytes(), ciphertext2.bytes(), ciphertext1.length()) == 0); + + /* Decrypt the ciphertext */ + openssl::aesDecrypt(key, 32, ciphertext1.bytes(), ciphertext1.length(), decrypted1); + assert(decrypted1.length() == len); + CPPUNIT_ASSERT(decrypted1.length() == len); + CPPUNIT_ASSERT(memcmp(decrypted1.bytes(), intext, len) == 0); + CPPUNIT_ASSERT(memcmp(ciphertext1.bytes(), ciphertext2.bytes(), ciphertext1.length()) == 0); // check input unchanged + + jlib::aesDecrypt(key, 32, ciphertext2.bytes(), ciphertext2.length(), decrypted2); + CPPUNIT_ASSERT(decrypted2.length() == len); + CPPUNIT_ASSERT(memcmp(decrypted2.bytes(), intext, len) == 0); + CPPUNIT_ASSERT(memcmp(ciphertext1.bytes(), ciphertext2.bytes(), ciphertext1.length()) == 0); // check input unchanged + + // Now test in-place decrypt + unsigned cipherlen = ciphertext1.length(); + ciphertext1.append(4, "XXXX"); // Marker + unsigned decryptedlen = openssl::aesDecryptInPlace(key, 32, (void *) ciphertext1.bytes(), cipherlen); + CPPUNIT_ASSERT(decryptedlen == len); + CPPUNIT_ASSERT(memcmp(ciphertext1.bytes(), intext, len) == 0); + CPPUNIT_ASSERT(memcmp(ciphertext1.bytes()+cipherlen, "XXXX", 4) == 0); + + cipherlen = ciphertext2.length(); + ciphertext2.append(4, "XXXX"); // Marker + decryptedlen = jlib::aesDecryptInPlace(key, 32, (void *) ciphertext2.bytes(), cipherlen); + CPPUNIT_ASSERT(decryptedlen == len); + CPPUNIT_ASSERT(memcmp(ciphertext2.bytes(), intext, len) == 0); + CPPUNIT_ASSERT(memcmp(ciphertext2.bytes()+cipherlen, "XXXX", 4) == 0); + } + + void test() + { + /* Message to be encrypted */ + const char *plaintext = "The quick brown fox jumps over the lazy dog"; + for (unsigned l = 0; l < strlen(plaintext); l++) + testOne(l, plaintext); + } + +}; + +CPPUNIT_TEST_SUITE_REGISTRATION( JLibOpensslAESTest ); +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( JLibOpensslAESTest, "JLibOpensslAESTest" ); +#endif class JLibSecretsTest : public CppUnit::TestFixture {