awesomized / libmemcached

Resurrection of libmemcached
https://awesomized.github.io/libmemcached/
BSD 3-Clause "New" or "Revised" License
45 stars 26 forks source link

Remove internal AES implementation and replace it with use of libcrypto #114

Closed TomasKorbar closed 3 years ago

TomasKorbar commented 3 years ago

Hi, As we are planning to replace original libmemcached in Fedora with awesomized libmemcached i want to propose replacement of internal AES implementation with use of openssl. In my opinion it is safer and more maintainable to use cryptographic library. All tests are passing. Feel free to propose any change.

remicollet commented 3 years ago

@TomasKorbar this break build with old openssl (such as 1.02k in RHEL-7)

Proposal

diff -up ./src/libhashkit/aes.cc.old ./src/libhashkit/aes.cc
--- ./src/libhashkit/aes.cc.old 2021-06-25 14:53:38.891064258 +0200
+++ ./src/libhashkit/aes.cc 2021-06-25 14:53:45.840043166 +0200
@@ -37,10 +37,10 @@ bool aes_initialize(const unsigned char
     return false;
   }

-  if (EVP_CIPHER_CTX_init(encryption_context) != 1 ||
-      EVP_EncryptInit_ex(encryption_context, EVP_aes_256_cbc(), NULL, key,
+  EVP_CIPHER_CTX_init(encryption_context);
+  EVP_CIPHER_CTX_init(decryption_context);
+  if (EVP_EncryptInit_ex(encryption_context, EVP_aes_256_cbc(), NULL, key,
                          aes_iv) != 1 ||
-      EVP_CIPHER_CTX_init(decryption_context) != 1 ||
       EVP_DecryptInit_ex(decryption_context, EVP_aes_256_cbc(), NULL, key,
                          aes_iv) != 1) {
     return false;

in Openssl 1.0

void EVP_CIPHER_CTX_init(EVP_CIPHER_CTX *a);

In Openssl 1.1

#  define EVP_CIPHER_CTX_init(c)      EVP_CIPHER_CTX_reset(c)
int EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *c);
TomasKorbar commented 3 years ago

Hi @m6w6 , I updated pull request according to your review. What do you think about it now?

TomasKorbar commented 3 years ago

@m6w6 Any update on this?

m6w6 commented 3 years ago

Yes, the update is, that I'm currently camping and there have been thunderstorms tonight. 😉

TomasKorbar commented 3 years ago

@m6w6 No pressure. Just wanted to keep this alive :) Enjoy your camping and i hope that the storms did not cause you any trouble.

m6w6 commented 3 years ago

Merged, with a couple of fixes and simplifications, thank you!