Hi, Georg!
First, I've looked a bit at EVP_CIPHER_CTX_new/etc.
With the CRYPTO_set_mem_functions() one can overwrite malloc/realloc/free
functions that OpenSSL is using. We absolutely should do it.
At the very least, OpenSSL should use my_malloc/my_realloc/my_free.
But a more interesting approach would be to allocate a buffer on the
stack and let our openssl m/r/f helpers take the memory from there.
And print a warning with a fallback to malloc if that buffer isn't
enough. This way openssl won't need mallocs is most cases.
Still, I'd suggest to do this in a separate commit. In your main
"OpenSSL 1.1 support" commit, just use CRYPTO_set_mem_functions to
put OpenSSL through my_malloc/my_realloc/my_free.
On Nov 15, Georg Richter wrote:
> From 6103b311c4c5740998d342afbba9ac70f8ec5731 Mon Sep 17 00:00:00 2001
> From: Georg Richter <ge...@mariadb.com>
> Date: Tue, 15 Nov 2016 18:39:54 +0100
> Subject: [PATCH] MDEV-10332: Added support for OpenSSL 1.1
>
> diff --git a/mysql-test/r/openssl_6975,tlsv10.result
> b/mysql-test/r/openssl_6975,tlsv10.result
> index a65167f..e216ec2 100644
> --- a/mysql-test/r/openssl_6975,tlsv10.result
> +++ b/mysql-test/r/openssl_6975,tlsv10.result
> @@ -1,11 +1,11 @@
> create user ssl_sslv3@localhost;
> -grant select on test.* to ssl_sslv3@localhost require cipher "RC4-SHA";
> +grant select on test.* to ssl_sslv3@localhost require cipher "DES-CBC3-SHA";
did you verify that after your changes all tests also work with older
openssl versions and with yassl?
> create user ssl_tls12@localhost;
> grant select on test.* to ssl_tls12@localhost require cipher "AES128-SHA256";
> TLS1.2 ciphers: user is ok with any cipher
> ERROR 2026 (HY000): SSL connection error: sslv3 alert handshake failure
> diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc
> index 49bd9af..d72bc72 100644
> --- a/mysys_ssl/my_crypt.cc
> +++ b/mysys_ssl/my_crypt.cc
> @@ -36,9 +36,20 @@
> class MyCTX
> {
> public:
> - EVP_CIPHER_CTX ctx;
> - MyCTX() { EVP_CIPHER_CTX_init(&ctx); }
> - virtual ~MyCTX() { EVP_CIPHER_CTX_cleanup(&ctx); ERR_remove_state(0); }
> + EVP_CIPHER_CTX *ctx;
> +#if defined(HAVE_YASSL)
> + MyCTX() { ctx= new EVP_CIPHER_CTX; EVP_CIPHER_CTX_init(ctx); }
> + virtual ~MyCTX() { EVP_CIPHER_CTX_cleanup(ctx); delete ctx;
> ERR_remove_state(0); }
> +#else
> + MyCTX() {ctx= EVP_CIPHER_CTX_new(); EVP_CIPHER_CTX_init(ctx); }
> + virtual ~MyCTX() { EVP_CIPHER_CTX_free(ctx);
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> + ERR_remove_state(0);
> +#else
> + ERR_clear_error();
> +#endif
> + }
> +#endif
No #ifdef's here, please. YaSSL-specific code from mysys_ssl/yassl.cc
makes yassl to look sufficiently like openssl, so that
mysys_ssl/my_crypt.cc wouldn't need any ifdefs. Please, keep it that
way.
And, certainly, don't put any #if's that check openssl version in the
code, keep them in the headers or in the beginning of the file (there
is, already #define ERR_remove_state(X) there)
> virtual int init(const EVP_CIPHER *cipher, int encrypt, const uchar *key,
> uint klen, const uchar *iv, uint ivlen)
> @@ -72,7 +83,10 @@ class MyCTX_nopad : public MyCTX
> {
> public:
> const uchar *key;
> + const uchar *src;
> int klen;
> + uchar oiv[90];
90 bytes for an IV?
> + uint extra_bytes;
>
> MyCTX_nopad() : MyCTX() { }
> ~MyCTX_nopad() { }
> @@ -285,7 +310,11 @@ int my_random_bytes(uchar *buf, int num)
> instead of whatever random engine is currently set in OpenSSL. That way
> we are guaranteed to have a non-blocking random.
> */
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> RAND_METHOD *rand = RAND_SSLeay();
> +#else
> + RAND_METHOD *rand = RAND_OpenSSL();
> +#endif
no #if's in the code, please
> if (rand == NULL || rand->bytes(buf, num) != 1)
> return MY_AES_OPENSSL_ERROR;
> return MY_AES_OK;
> diff --git a/mysys_ssl/my_md5.cc b/mysys_ssl/my_md5.cc
> index 7139ea9..4885033 100644
> --- a/mysys_ssl/my_md5.cc
> +++ b/mysys_ssl/my_md5.cc
> @@ -58,10 +57,11 @@ static void md5_result(MD5_CONTEXT *context, uchar
> digest[MD5_HASH_SIZE])
>
> #elif defined(HAVE_OPENSSL)
> #include <openssl/evp.h>
> -typedef EVP_MD_CTX MD5_CONTEXT;
> +typedef void MD5_CONTEXT;
>
> -static void md5_init(MD5_CONTEXT *context)
> +static void md5_init(MD5_CONTEXT *ctx)
> {
> + EVP_MD_CTX *context= (EVP_MD_CTX *)ctx;
> EVP_MD_CTX_init(context);
> #ifdef EVP_MD_CTX_FLAG_NON_FIPS_ALLOW
> /* Ok to ignore FIPS: MD5 is not used for crypto here */
> @@ -123,21 +143,43 @@ void my_md5_multi(uchar *digest, ...)
> {
> va_list args;
> va_start(args, digest);
> -
> - MD5_CONTEXT md5_context;
> +#ifdef HAVE_YASSL
> + MD5_CONTEXT *md5_context= new MD5_CONTEXT;
> +#else
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> + EVP_MD_CTX *md5_context= (EVP_MD_CTX *)alloca(sizeof(EVP_MD_CTX));
> +#else
> + EVP_MD_CTX *md5_context= EVP_MD_CTX_new();
> +#endif
> +#endif
> const uchar *str;
>
> - md5_init_fast(&md5_context);
> + md5_init_fast(md5_context);
> for (str= va_arg(args, const uchar*); str; str= va_arg(args, const uchar*))
> - md5_input(&md5_context, str, va_arg(args, size_t));
> + md5_input(md5_context, str, va_arg(args, size_t));
>
> - md5_result(&md5_context, digest);
> + md5_result(md5_context, digest);
> va_end(args);
> +#if defined(HAVE_YASSL)
> + delete md5_context;
> +#else
> +#if OPENSSL_VERSION_NUMBER >= 0x10100000L
> + EVP_MD_CTX_free(md5_context);
> +#endif
> +#endif
> }
>
> size_t my_md5_context_size()
> {
> +#if defined(HAVE_YASSL)
> return sizeof(MD5_CONTEXT);
> +#elif OPENSSL_VERSION_NUMBER < 0x10100000L
> + return sizeof(EVP_MD_CTX);
> +#else
> + /* Since OpenSSL 1.1 we can't determine the length of EVP_MD_CTX anymore,
> + so we need to return a number (48 bytes + 16 bytes reserved) */
> + return 64;
How can you be sure that it's enough?
I'd just return sizeof(void*) and in
md5_init stored a return value of EVP_MD_CTX_new() there.
Until we do that optimization that I mentioned in the beginning of the
email.
And get rid of #if's please.
> +#endif
> }
Regards,
Sergei
Chief Architect MariaDB
and secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
