From f651443189f7332c44c73a6627e2f8880d8b5700 Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Tue, 11 Jul 2023 17:52:56 +0100 Subject: [PATCH 1/6] Use OpenSSL crypto instead of libsodium when libsodium isn't available --- common/Makefile.am | 2 +- common/sodcrypto.cc | 389 +++++++++++++++++++++-------------- common/sodcrypto.hh | 41 +++- wforce/wforce-replication.cc | 6 +- 4 files changed, 273 insertions(+), 165 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 1e810b2d..59f82cae 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -24,7 +24,7 @@ AM_LDFLAGS = \ noinst_LTLIBRARIES=libweakforce.la libweakforce_la_LIBADD=$(EXT_LIBS) $(JSON11_LIBS) $(BOOST_FILESYSTEM_LIBS) $(BOOST_SYSTEM_LIBS) \ $(GEOIP_LIBS) $(MMDB_LIBS) $(LIBPROMETHEUS_LIBS) $(LIBDROGON_LIBS) \ - $(LIBJSONCPP_LIBS) $(LIBZ_LIBS) $(LIBCURL) $(LIBSSL_LIBS) + $(LIBJSONCPP_LIBS) $(LIBZ_LIBS) $(LIBCURL) $(LIBSSL_LIBS) $(LIBCRYPTO_LIBS) libweakforce_la_SOURCES= \ base64.hh \ dns_lookup.cc dns_lookup.hh \ diff --git a/common/sodcrypto.cc b/common/sodcrypto.cc index 6b23a9e8..e84f00b7 100644 --- a/common/sodcrypto.cc +++ b/common/sodcrypto.cc @@ -21,13 +21,12 @@ */ #include -#include +#include #include "namespaces.hh" #include "misc.hh" #include "base64.hh" #include "sodcrypto.hh" - #ifdef HAVE_LIBSODIUM string newKey() @@ -53,7 +52,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium unsigned char decrypted[msg.length() - crypto_secretbox_MACBYTES]; if (crypto_secretbox_open_easy(decrypted, (const unsigned char*)msg.c_str(), - msg.length(), nonce.value, (const unsigned char*)key.c_str()) != 0) { + msg.length(), nonce.value, (const unsigned char*)key.c_str()) != 0) { throw std::runtime_error("Could not decrypt message"); } nonce.increment(); @@ -65,20 +64,113 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium } } #else -std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce) + +#define POLY1305_BLOCK_SIZE 16 + +string newKey() { - return msg; + unsigned char key[CHACHA20_POLY1305_KEY_SIZE]; + if (!RAND_priv_bytes(key, sizeof key)) { + throw std::runtime_error( + "Could not initialize random number generator for cryptographic functions - this is not recoverable"); + } + return "\"" + Base64Encode(string((char*) key, sizeof key)) + "\""; } -std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce) + + +std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce) { - return msg; + unsigned char ciphertext[msg.length() + POLY1305_BLOCK_SIZE]; + int len; + int ciphertext_len; + // Each thread gets its own cipher context + thread_local EVP_CIPHER_CTX *ctx = nullptr; + + if (key.length() != 32) { + ostringstream err; + err << "sodEncryptSym: key length is not 32 bytes (actual length=" << key.length() << ")"; + throw std::runtime_error(err.str().c_str()); + } + + if (ctx == nullptr) { + if (!(ctx = EVP_CIPHER_CTX_new())) + throw std::runtime_error("sodEncryptSym: EVP_CIPHER_CTX_new() could not initialize cipher context"); + + /* Initialise the encryption operation. */ + if (1 != EVP_EncryptInit_ex(ctx, EVP_chacha20_poly1305(), NULL, NULL, NULL)) + throw std::runtime_error("sodEncryptSym: EVP_EncryptInit_ex() could not initialize encryption operation"); + } + + if (1 != EVP_EncryptInit_ex(ctx, NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) + throw std::runtime_error("sodEncryptSym: EVP_EncryptInit_ex() could not initialize encryption key and IV"); + + if (1 != + EVP_EncryptUpdate(ctx, ciphertext + POLY1305_BLOCK_SIZE, &len, (unsigned char*) msg.c_str(), msg.length())) + throw std::runtime_error("sodEncryptSym: EVP_EncryptUpdate() could not encrypt message"); + ciphertext_len = len; + + if (1 != EVP_EncryptFinal_ex(ctx, ciphertext + len + POLY1305_BLOCK_SIZE, &len)) + throw std::runtime_error("sodEncryptSym: EVP_EncryptFinal_ex() could finalize message encryption");; + ciphertext_len += len; + + /* Get the tag */ + if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, POLY1305_BLOCK_SIZE, ciphertext)) + throw std::runtime_error("sodEncryptSym: EVP_CIPHER_CTX_ctrl() could not get tag"); + + nonce.increment(); + return string((char*) ciphertext, sizeof(ciphertext)); } -string newKey() +std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce) { - return "\"plaintext\""; -} + int len; + int plaintext_len; + // Each thread gets its own cipher context + thread_local EVP_CIPHER_CTX *ctx = nullptr; + + if (key.length() != 32) { + ostringstream err; + err << "sodDecryptSym: key length is not 32 bytes (actual length=" << key.length() << ")"; + throw std::runtime_error(err.str().c_str()); + } + + if ((msg.length() - POLY1305_BLOCK_SIZE) > 0) { + + string tag = msg.substr(0, POLY1305_BLOCK_SIZE); + char plaintext[msg.length() - POLY1305_BLOCK_SIZE]; + + if (ctx == nullptr) { + if (!(ctx = EVP_CIPHER_CTX_new())) + throw std::runtime_error("sodDecryptSym: EVP_CIPHER_CTX_new() could not initialize cipher context"); + + if (1 != EVP_DecryptInit_ex(ctx, EVP_chacha20_poly1305(), NULL, NULL, NULL)) + throw std::runtime_error("sodDecryptSym: EVP_DecryptInit_ex() could not initialize decryption operation"); + } + + if (1 != EVP_DecryptInit_ex(ctx, NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) + throw std::runtime_error("sodDecryptSym: EVP_DecryptInit_ex() could not initialize decryption key and IV"); + + if (!EVP_DecryptUpdate(ctx, (unsigned char*) plaintext, &len, + (unsigned char*) (msg.c_str() + POLY1305_BLOCK_SIZE), msg.length() - POLY1305_BLOCK_SIZE)) + throw std::runtime_error("sodDecryptSym: EVP_DecryptUpdate() could not decrypt message"); + plaintext_len = len; + + /* Set expected tag value. Works in OpenSSL 1.0.1d and later */ + if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, POLY1305_BLOCK_SIZE, (void*) tag.c_str())) + throw std::runtime_error("sodDecryptSym: EVP_CIPHER_CTX_ctrl() AEAD tag could not be validated"); + + if (!EVP_DecryptFinal_ex(ctx, (unsigned char*) (plaintext + len), &len)) + throw std::runtime_error("sodDecryptSym: EVP_DecryptFinal_ex() failed - plaintext cannot be trusted"); + + nonce.increment(); + return string(plaintext, plaintext_len); + } + else { + nonce.increment(); + return string(); + } +} #endif @@ -87,92 +179,87 @@ string newKey() #include namespace anonpdns { -char B64Decode1(char cInChar) -{ - // The incoming character will be A-Z, a-z, 0-9, +, /, or =. - // The idea is to quickly determine which grouping the - // letter belongs to and return the associated value - // without having to search the global encoding string - // (the value we're looking for would be the resulting - // index into that string). - // - // To do that, we'll play some tricks... - unsigned char iIndex = '\0'; - switch ( cInChar ) { - case '+': - iIndex = 62; - break; - - case '/': - iIndex = 63; - break; - - case '=': - iIndex = 0; - break; - - default: - // Must be 'A'-'Z', 'a'-'z', '0'-'9', or an error... + char B64Decode1(char cInChar) + { + // The incoming character will be A-Z, a-z, 0-9, +, /, or =. + // The idea is to quickly determine which grouping the + // letter belongs to and return the associated value + // without having to search the global encoding string + // (the value we're looking for would be the resulting + // index into that string). // - // Numerically, small letters are "greater" in value than - // capital letters and numerals (ASCII value), and capital - // letters are "greater" than numerals (again, ASCII value), - // so we check for numerals first, then capital letters, - // and finally small letters. - iIndex = '9' - cInChar; - if ( iIndex > 0x3F ) { - // Not from '0' to '9'... - iIndex = 'Z' - cInChar; - if ( iIndex > 0x3F ) { - // Not from 'A' to 'Z'... - iIndex = 'z' - cInChar; - if ( iIndex > 0x3F ) { - // Invalid character...cannot - // decode! - iIndex = 0x80; // set the high bit + // To do that, we'll play some tricks... + unsigned char iIndex = '\0'; + switch (cInChar) { + case '+': + iIndex = 62; + break; + + case '/': + iIndex = 63; + break; + + case '=': + iIndex = 0; + break; + + default: + // Must be 'A'-'Z', 'a'-'z', '0'-'9', or an error... + // + // Numerically, small letters are "greater" in value than + // capital letters and numerals (ASCII value), and capital + // letters are "greater" than numerals (again, ASCII value), + // so we check for numerals first, then capital letters, + // and finally small letters. + iIndex = '9' - cInChar; + if (iIndex > 0x3F) { + // Not from '0' to '9'... + iIndex = 'Z' - cInChar; + if (iIndex > 0x3F) { + // Not from 'A' to 'Z'... + iIndex = 'z' - cInChar; + if (iIndex > 0x3F) { + // Invalid character...cannot + // decode! + iIndex = 0x80; // set the high bit + } // if + else { + // From 'a' to 'z' + iIndex = (('z' - iIndex) - 'a') + 26; + } // else + } // if + else { + // From 'A' to 'Z' + iIndex = ('Z' - iIndex) - 'A'; + } // else } // if else { - // From 'a' to 'z' - iIndex = (('z' - iIndex) - 'a') + 26; + // Adjust the index... + iIndex = (('9' - iIndex) - '0') + 52; } // else - } // if - else { - // From 'A' to 'Z' - iIndex = ('Z' - iIndex) - 'A'; - } // else - } // if - else { - // Adjust the index... - iIndex = (('9' - iIndex) - '0') + 52; - } // else - break; + break; - } // switch + } // switch - return iIndex; -} + return iIndex; + } -inline char B64Encode1(unsigned char uc) -{ - if (uc < 26) - { - return 'A'+uc; + inline char B64Encode1(unsigned char uc) + { + if (uc < 26) { + return 'A' + uc; } - if (uc < 52) - { - return 'a'+(uc-26); + if (uc < 52) { + return 'a' + (uc - 26); } - if (uc < 62) - { - return '0'+(uc-52); + if (uc < 62) { + return '0' + (uc - 52); } - if (uc == 62) - { + if (uc == 62) { return '+'; } - return '/'; -}; - + return '/'; + }; } @@ -182,7 +269,7 @@ int B64Decode(const std::string& strInput, std::string& strOutput) { // Set up a decoding buffer long cBuf = 0; - char* pBuf = (char*)&cBuf; + char* pBuf = (char*) &cBuf; // Decoding management... int iBitGroup = 0, iInNum = 0; @@ -200,16 +287,16 @@ int B64Decode(const std::string& strInput, std::string& strOutput) int iInSize = strInput.size(); unsigned char cChar = '\0'; uint8_t pad = 0; - while ( iInNum < iInSize ) { + while (iInNum < iInSize) { // Fill the decode buffer with 4 groups of 6 bits cBuf = 0; // clear pad = 0; - for ( iBitGroup = 0; iBitGroup < 4; ++iBitGroup ) { - if ( iInNum < iInSize ) { + for (iBitGroup = 0; iBitGroup < 4; ++iBitGroup) { + if (iInNum < iInSize) { // Decode a character - if(strInput.at(iInNum)=='=') - pad++; - while(isspace(strInput.at(iInNum))) + if (strInput.at(iInNum) == '=') + pad++; + while (isspace(strInput.at(iInNum))) iInNum++; cChar = B64Decode1(strInput.at(iInNum++)); @@ -220,28 +307,28 @@ int B64Decode(const std::string& strInput, std::string& strOutput) } // else // Check for valid decode - if ( cChar > 0x7F ) + if (cChar > 0x7F) return -1; // Adjust the bits - switch ( iBitGroup ) { - case 0: - // The first group is copied into - // the least significant 6 bits of - // the decode buffer...these 6 bits - // will eventually shift over to be - // the most significant bits of the - // third byte. - cBuf = cBuf | cChar; - break; - - default: - // For groupings 1-3, simply shift - // the bits in the decode buffer over - // by 6 and insert the 6 from the - // current decode character. - cBuf = (cBuf << 6) | cChar; - break; + switch (iBitGroup) { + case 0: + // The first group is copied into + // the least significant 6 bits of + // the decode buffer...these 6 bits + // will eventually shift over to be + // the most significant bits of the + // third byte. + cBuf = cBuf | cChar; + break; + + default: + // For groupings 1-3, simply shift + // the bits in the decode buffer over + // by 6 and insert the 6 from the + // current decode character. + cBuf = (cBuf << 6) | cChar; + break; } // switch } // for @@ -253,8 +340,8 @@ int B64Decode(const std::string& strInput, std::string& strOutput) strOutput += pBuf[1]; strOutput += pBuf[0]; } // while - if(pad) - strOutput.resize(strOutput.length()-pad); + if (pad) + strOutput.resize(strOutput.length() - pad); return 1; } @@ -266,52 +353,44 @@ The Encode static method takes an array of 8-bit values and returns a base-64 st */ -std::string Base64Encode (const std::string& vby) +std::string Base64Encode(const std::string& vby) { std::string retval; - if (vby.size () == 0) - { - return retval; + if (vby.size() == 0) { + return retval; + }; + for (unsigned int i = 0; i < vby.size(); i += 3) { + unsigned char by1 = 0, by2 = 0, by3 = 0; + by1 = vby[i]; + if (i + 1 < vby.size()) { + by2 = vby[i + 1]; + }; + if (i + 2 < vby.size()) { + by3 = vby[i + 2]; + } + unsigned char by4 = 0, by5 = 0, by6 = 0, by7 = 0; + by4 = by1 >> 2; + by5 = ((by1 & 0x3) << 4) | (by2 >> 4); + by6 = ((by2 & 0xf) << 2) | (by3 >> 6); + by7 = by3 & 0x3f; + retval += B64Encode1(by4); + retval += B64Encode1(by5); + if (i + 1 < vby.size()) { + retval += B64Encode1(by6); + } + else { + retval += "="; }; - for (unsigned int i = 0; i < vby.size (); i += 3) - { - unsigned char by1 = 0, by2 = 0, by3 = 0; - by1 = vby[i]; - if (i + 1 < vby.size ()) - { - by2 = vby[i + 1]; - }; - if (i + 2 < vby.size ()) - { - by3 = vby[i + 2]; - } - unsigned char by4 = 0, by5 = 0, by6 = 0, by7 = 0; - by4 = by1 >> 2; - by5 = ((by1 & 0x3) << 4) | (by2 >> 4); - by6 = ((by2 & 0xf) << 2) | (by3 >> 6); - by7 = by3 & 0x3f; - retval += B64Encode1 (by4); - retval += B64Encode1 (by5); - if (i + 1 < vby.size ()) - { - retval += B64Encode1 (by6); - } - else - { - retval += "="; - }; - if (i + 2 < vby.size ()) - { - retval += B64Encode1 (by7); - } - else - { - retval += "="; - }; - /* if ((i % (76 / 4 * 3)) == 0) - { - retval += "\r\n"; - }*/ + if (i + 2 < vby.size()) { + retval += B64Encode1(by7); + } + else { + retval += "="; }; + /* if ((i % (76 / 4 * 3)) == 0) + { + retval += "\r\n"; + }*/ + }; return retval; }; diff --git a/common/sodcrypto.hh b/common/sodcrypto.hh index a0b75dc0..5a8cac2f 100644 --- a/common/sodcrypto.hh +++ b/common/sodcrypto.hh @@ -28,15 +28,44 @@ #include #ifndef HAVE_LIBSODIUM +#include +#include +#include + +#define CHACHA20_POLY1305_IV_SIZE 12 +#define CHACHA20_POLY1305_KEY_SIZE 32 + struct SodiumNonce { - void init(){}; - void merge(const SodiumNonce& lower, const SodiumNonce& higher){}; - void increment(){}; - std::string toString() { return string(""); } - unsigned char value[1]; + void init() + { + if (!RAND_priv_bytes(value, sizeof value)) { + throw std::runtime_error("Could not initialize random number generator for cryptographic functions - this is not recoverable"); + } + } + + void merge(const SodiumNonce& lower, const SodiumNonce& higher) + { + static const size_t halfSize = (sizeof value) / 2; + memcpy(value, lower.value, halfSize); + memcpy(value + halfSize, higher.value + halfSize, halfSize); + } + + void increment() + { + uint32_t* p = (uint32_t*)value; + uint32_t count=htonl(*p); + ++count; + *p=ntohl(count); + } + + string toString() const + { + return string((const char*)value, sizeof value); + } + + unsigned char value[CHACHA20_POLY1305_IV_SIZE]; }; -#define crypto_secretbox_NONCEBYTES 0 #else #include diff --git a/wforce/wforce-replication.cc b/wforce/wforce-replication.cc index 1d791b8f..db39ab9a 100644 --- a/wforce/wforce-replication.cc +++ b/wforce/wforce-replication.cc @@ -62,12 +62,12 @@ bool WforceReplication::decryptMsg(const char* buf, size_t len, std::string& msg { SodiumNonce nonce; - if (len < static_cast(crypto_secretbox_NONCEBYTES)) { + if (len < static_cast(sizeof(nonce.value))) { errlog("Could not decrypt replication operation: not enough bytes (%d) to hold nonce", len); return false; } - memcpy((char*)&nonce, buf, crypto_secretbox_NONCEBYTES); - string packet(buf + crypto_secretbox_NONCEBYTES, buf+len); + memcpy((char*)&nonce, buf, sizeof(nonce.value)); + string packet(buf + sizeof(nonce.value), buf+len); try { msg=sodDecryptSym(packet, d_key, nonce); } From b9ef332df279d0b3aeb5280531c9a244716db8ad Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Mon, 24 Jul 2023 13:29:21 +0100 Subject: [PATCH 2/6] Wrap openssl ctx in a smart ptr to avoid leaks --- common/sodcrypto.cc | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/common/sodcrypto.cc b/common/sodcrypto.cc index e84f00b7..8ab5554b 100644 --- a/common/sodcrypto.cc +++ b/common/sodcrypto.cc @@ -84,7 +84,7 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium int len; int ciphertext_len; // Each thread gets its own cipher context - thread_local EVP_CIPHER_CTX *ctx = nullptr; + thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; if (key.length() != 32) { ostringstream err; @@ -92,29 +92,29 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium throw std::runtime_error(err.str().c_str()); } - if (ctx == nullptr) { - if (!(ctx = EVP_CIPHER_CTX_new())) + if (ctx.get() == nullptr) { + if (!(ctx = std::unique_ptr(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free))) throw std::runtime_error("sodEncryptSym: EVP_CIPHER_CTX_new() could not initialize cipher context"); /* Initialise the encryption operation. */ - if (1 != EVP_EncryptInit_ex(ctx, EVP_chacha20_poly1305(), NULL, NULL, NULL)) + if (1 != EVP_EncryptInit_ex(ctx.get(), EVP_chacha20_poly1305(), NULL, NULL, NULL)) throw std::runtime_error("sodEncryptSym: EVP_EncryptInit_ex() could not initialize encryption operation"); } - if (1 != EVP_EncryptInit_ex(ctx, NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) + if (1 != EVP_EncryptInit_ex(ctx.get(), NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) throw std::runtime_error("sodEncryptSym: EVP_EncryptInit_ex() could not initialize encryption key and IV"); if (1 != - EVP_EncryptUpdate(ctx, ciphertext + POLY1305_BLOCK_SIZE, &len, (unsigned char*) msg.c_str(), msg.length())) + EVP_EncryptUpdate(ctx.get(), ciphertext + POLY1305_BLOCK_SIZE, &len, (unsigned char*) msg.c_str(), msg.length())) throw std::runtime_error("sodEncryptSym: EVP_EncryptUpdate() could not encrypt message"); ciphertext_len = len; - if (1 != EVP_EncryptFinal_ex(ctx, ciphertext + len + POLY1305_BLOCK_SIZE, &len)) + if (1 != EVP_EncryptFinal_ex(ctx.get(), ciphertext + len + POLY1305_BLOCK_SIZE, &len)) throw std::runtime_error("sodEncryptSym: EVP_EncryptFinal_ex() could finalize message encryption");; ciphertext_len += len; /* Get the tag */ - if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, POLY1305_BLOCK_SIZE, ciphertext)) + if (1 != EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, POLY1305_BLOCK_SIZE, ciphertext)) throw std::runtime_error("sodEncryptSym: EVP_CIPHER_CTX_ctrl() could not get tag"); nonce.increment(); @@ -126,7 +126,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium int len; int plaintext_len; // Each thread gets its own cipher context - thread_local EVP_CIPHER_CTX *ctx = nullptr; + thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; if (key.length() != 32) { ostringstream err; @@ -139,27 +139,27 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium string tag = msg.substr(0, POLY1305_BLOCK_SIZE); char plaintext[msg.length() - POLY1305_BLOCK_SIZE]; - if (ctx == nullptr) { - if (!(ctx = EVP_CIPHER_CTX_new())) + if (ctx.get() == nullptr) { + if (!(ctx = std::unique_ptr(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free))) throw std::runtime_error("sodDecryptSym: EVP_CIPHER_CTX_new() could not initialize cipher context"); - if (1 != EVP_DecryptInit_ex(ctx, EVP_chacha20_poly1305(), NULL, NULL, NULL)) + if (1 != EVP_DecryptInit_ex(ctx.get(), EVP_chacha20_poly1305(), NULL, NULL, NULL)) throw std::runtime_error("sodDecryptSym: EVP_DecryptInit_ex() could not initialize decryption operation"); } - if (1 != EVP_DecryptInit_ex(ctx, NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) + if (1 != EVP_DecryptInit_ex(ctx.get(), NULL, NULL, (unsigned char*) key.c_str(), nonce.value)) throw std::runtime_error("sodDecryptSym: EVP_DecryptInit_ex() could not initialize decryption key and IV"); - if (!EVP_DecryptUpdate(ctx, (unsigned char*) plaintext, &len, + if (!EVP_DecryptUpdate(ctx.get(), (unsigned char*) plaintext, &len, (unsigned char*) (msg.c_str() + POLY1305_BLOCK_SIZE), msg.length() - POLY1305_BLOCK_SIZE)) throw std::runtime_error("sodDecryptSym: EVP_DecryptUpdate() could not decrypt message"); plaintext_len = len; /* Set expected tag value. Works in OpenSSL 1.0.1d and later */ - if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, POLY1305_BLOCK_SIZE, (void*) tag.c_str())) + if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_TAG, POLY1305_BLOCK_SIZE, (void*) tag.c_str())) throw std::runtime_error("sodDecryptSym: EVP_CIPHER_CTX_ctrl() AEAD tag could not be validated"); - if (!EVP_DecryptFinal_ex(ctx, (unsigned char*) (plaintext + len), &len)) + if (!EVP_DecryptFinal_ex(ctx.get(), (unsigned char*) (plaintext + len), &len)) throw std::runtime_error("sodDecryptSym: EVP_DecryptFinal_ex() failed - plaintext cannot be trusted"); nonce.increment(); From 59163158ce82aa3219f8c27566e50f5a445c2c43 Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Mon, 24 Jul 2023 13:32:22 +0100 Subject: [PATCH 3/6] Compare RAND_priv_bytes to 1 explicitly as it can return -1 in some failure cases --- common/sodcrypto.cc | 2 +- common/sodcrypto.hh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/sodcrypto.cc b/common/sodcrypto.cc index 8ab5554b..0da6bc1c 100644 --- a/common/sodcrypto.cc +++ b/common/sodcrypto.cc @@ -70,7 +70,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium string newKey() { unsigned char key[CHACHA20_POLY1305_KEY_SIZE]; - if (!RAND_priv_bytes(key, sizeof key)) { + if (RAND_priv_bytes(key, sizeof key) != 1) { throw std::runtime_error( "Could not initialize random number generator for cryptographic functions - this is not recoverable"); } diff --git a/common/sodcrypto.hh b/common/sodcrypto.hh index 5a8cac2f..6e82851b 100644 --- a/common/sodcrypto.hh +++ b/common/sodcrypto.hh @@ -39,7 +39,7 @@ struct SodiumNonce { void init() { - if (!RAND_priv_bytes(value, sizeof value)) { + if (RAND_priv_bytes(value, sizeof value) != 1) { throw std::runtime_error("Could not initialize random number generator for cryptographic functions - this is not recoverable"); } } From 18aa5a55960a4cdf7e440931a3d80c585398a636 Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Mon, 24 Jul 2023 13:34:16 +0100 Subject: [PATCH 4/6] Use CHACHA20_POLY1305_KEY_SIZE instead of 32 --- common/sodcrypto.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/sodcrypto.cc b/common/sodcrypto.cc index 0da6bc1c..685be5b2 100644 --- a/common/sodcrypto.cc +++ b/common/sodcrypto.cc @@ -86,7 +86,7 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium // Each thread gets its own cipher context thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; - if (key.length() != 32) { + if (key.length() != CHACHA20_POLY1305_KEY_SIZE) { ostringstream err; err << "sodEncryptSym: key length is not 32 bytes (actual length=" << key.length() << ")"; throw std::runtime_error(err.str().c_str()); @@ -128,7 +128,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium // Each thread gets its own cipher context thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; - if (key.length() != 32) { + if (key.length() != CHACHA20_POLY1305_KEY_SIZE) { ostringstream err; err << "sodDecryptSym: key length is not 32 bytes (actual length=" << key.length() << ")"; throw std::runtime_error(err.str().c_str()); From 7dc2c2666631ccb8bb71a0420f3aca8e00aabb61 Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Mon, 24 Jul 2023 17:30:24 +0100 Subject: [PATCH 5/6] Test with and without sodium in regression --- docker/Makefile.am | 4 ++-- docker/regression/regression.sh | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docker/Makefile.am b/docker/Makefile.am index 85d4cae0..48b55e4c 100644 --- a/docker/Makefile.am +++ b/docker/Makefile.am @@ -32,10 +32,10 @@ clean_docker: clean: clean_docker regression-gcc: build_image start - $(DCMP) exec -T $(REGRESSION_SERVICE) docker/regression/regression.sh gcc g++ + $(DCMP) exec -T $(REGRESSION_SERVICE) docker/regression/regression.sh gcc g++ --enable-sodium regression-clang: build_image start - $(DCMP) exec -T $(REGRESSION_SERVICE) docker/regression/regression.sh clang clang++ + $(DCMP) exec -T $(REGRESSION_SERVICE) docker/regression/regression.sh clang clang++ --disable-sodium regression-none: echo "Regression tests skipped due to compiler 'none'" diff --git a/docker/regression/regression.sh b/docker/regression/regression.sh index 49330efc..a043fb26 100755 --- a/docker/regression/regression.sh +++ b/docker/regression/regression.sh @@ -2,20 +2,22 @@ set -e -if [ $# -ne 2 ] +if [ $# -ne 3 ] then export MYCC=clang export MYCXX=clang++ + export SODIUM= else export MYCC=$1 export MYCXX=$2 + export SODIUM=$3 fi echo "CC=$MYCC" echo "CXX=$MYCXX" autoreconf -v -i -f -./configure --enable-trackalert --enable-systemd --disable-docker --enable-unit-tests --enable-asan --enable-ubsan --disable-silent-rules CC=$MYCC CXX=$MYCXX +./configure --enable-trackalert --enable-systemd --disable-docker --enable-unit-tests --enable-asan --enable-ubsan $SODIUM --disable-silent-rules CC=$MYCC CXX=$MYCXX make clean make make check || (cat common/test-suite.log && false) From 0be795293cebf46af5b03ac007864921a2f1dc5e Mon Sep 17 00:00:00 2001 From: Neil Cook Date: Tue, 25 Jul 2023 08:38:27 +0100 Subject: [PATCH 6/6] Make context a static variable so that it doesn't get created everytime --- common/sodcrypto.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/sodcrypto.cc b/common/sodcrypto.cc index 685be5b2..573e499a 100644 --- a/common/sodcrypto.cc +++ b/common/sodcrypto.cc @@ -84,7 +84,7 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium int len; int ciphertext_len; // Each thread gets its own cipher context - thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; + static thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free); if (key.length() != CHACHA20_POLY1305_KEY_SIZE) { ostringstream err; @@ -126,7 +126,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium int len; int plaintext_len; // Each thread gets its own cipher context - thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free);; + static thread_local auto ctx = std::unique_ptr(nullptr, EVP_CIPHER_CTX_free); if (key.length() != CHACHA20_POLY1305_KEY_SIZE) { ostringstream err;