Re: [Maria-developers] 9e176e81b72: MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library
Hi, Vladislav! It's good to see how small the patch is, and that wolfssl is more OpenSSL compatible than YaSSL. Few comments below. On May 19, Vladislav Vaintroub wrote:
revision-id: 9e176e81b72 (mariadb-10.4.4-61-g9e176e81b72) parent(s): ea679c88c32 author: Vladislav Vaintroub <wlad@mariadb.com> committer: Vladislav Vaintroub <wlad@mariadb.com> timestamp: 2019-05-01 11:28:56 +0100 message:
MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library
- Add new submodule for WolfSSL - Build and use wolfssl and wolfcrypt instead of yassl/taocrypt - Use HAVE_WOLFSSL instead of HAVE_YASSL - Increase MY_AES_CTX_SIZE, to avoid compile time asserts in my_crypt.cc (sizeof(EVP_CIPHER_CTX) is larger on WolfSSL)
diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake index aa42333a5c8..79a531ec323 100644 --- a/cmake/ssl.cmake +++ b/cmake/ssl.cmake @@ -48,29 +48,19 @@ MACRO (CHANGE_SSL_SETTINGS string) ENDMACRO()
MACRO (MYSQL_USE_BUNDLED_SSL) - SET(INC_DIRS - ${CMAKE_SOURCE_DIR}/extra/yassl/include - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/include + SET(INC_DIRS + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl/wolfssl + #${CMAKE_SOURCE_DIR}/extra/wolfssl/taocrypt/include
don't you want to remove these (and many below) commented lines?
) - SET(SSL_LIBRARIES yassl taocrypt) + SET(SSL_LIBRARIES wolfssl wolfcrypt) SET(SSL_INCLUDE_DIRS ${INC_DIRS}) - SET(SSL_INTERNAL_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/mySTL) + SET(SSL_DEFINES "-DHAVE_OPENSSL -DHAVE_WOLFSSL -DOPENSSL_ALL -DWOLFSSL_MYSQL_COMPATIBLE -DWC_NO_HARDEN") - SET(SSL_DEFINES "-DHAVE_YASSL -DYASSL_PREFIX -DHAVE_OPENSSL -DMULTI_THREADED") - SET(HAVE_ERR_remove_thread_state OFF CACHE INTERNAL "yassl doesn't have ERR_remove_thread_state") - SET(HAVE_EncryptAes128Ctr OFF CACHE INTERNAL "yassl doesn't support AES-CTR") - SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "yassl doesn't support AES-GCM") + SET(HAVE_ERR_remove_thread_state ON CACHE INTERNAL "wolfssl doesn't have ERR_remove_thread_state") + SET(HAVE_EncryptAes128Ctr ON CACHE INTERNAL "wolfssl does support AES-CTR") + SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "wolfssl does not support AES-GCM") CHANGE_SSL_SETTINGS("bundled") - ADD_SUBDIRECTORY(extra/yassl) - ADD_SUBDIRECTORY(extra/yassl/taocrypt) - GET_TARGET_PROPERTY(src yassl SOURCES) - FOREACH(file ${src}) - SET(SSL_SOURCES ${SSL_SOURCES} ${CMAKE_SOURCE_DIR}/extra/yassl/${file}) - ENDFOREACH() - GET_TARGET_PROPERTY(src taocrypt SOURCES) - FOREACH(file ${src}) - SET(SSL_SOURCES ${SSL_SOURCES} - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/${file}) - ENDFOREACH() + ADD_SUBDIRECTORY(extra/wolfssl) MESSAGE_ONCE(SSL_LIBRARIES "SSL_LIBRARIES = ${SSL_LIBRARIES}") ENDMACRO()
diff --git a/extra/wolfssl/CMakeLists.txt b/extra/wolfssl/CMakeLists.txt new file mode 100644 index 00000000000..c1165ad6c72 --- /dev/null +++ b/extra/wolfssl/CMakeLists.txt @@ -0,0 +1,119 @@ +# CMakeLists.txt +# +# Copyright (C) 2006-2015 wolfSSL Inc. +# +# This file is part of wolfSSL. (formerly known as CyaSSL) +# +# wolfSSL is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# wolfSSL is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + +SET(WOLFSSL_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/src) +ADD_DEFINITIONS(${SSL_DEFINES}) +ADD_DEFINITIONS( + -DWOLFSSL_MYSQL_COMPATIBLE + -DHAVE_ECC + -DECC_TIMING_RESISTANT + -DBUILDING_WOLFSSL + -DHAVE_HASHDRBG + -DWOLFSSL_AES_DIRECT + -DWOLFSSL_SHA384 + -DWOLFSSL_SHA512 + -DWOLFSSL_SHA224 + -DSESSION_CERT + -DKEEP_OUR_CERT + -DWOLFSSL_STATIC_RSA + -DWC_RSA_BLINDING + -DHAVE_TLS_EXTENSIONS + -DHAVE_AES_ECB + -DWOLFSSL_AES_COUNTER + -DNO_WOLFSSL_STUB) + +SET(WOLFSSL_SOURCES + ${WOLFSSL_SRCDIR}/crl.c + ${WOLFSSL_SRCDIR}/internal.c + ${WOLFSSL_SRCDIR}/keys.c + ${WOLFSSL_SRCDIR}/tls.c + ${WOLFSSL_SRCDIR}/wolfio.c + ${WOLFSSL_SRCDIR}/ocsp.c + ${WOLFSSL_SRCDIR}/ssl.c) +ADD_DEFINITIONS(-DWOLFSSL_LIB) +INCLUDE_DIRECTORIES(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl) +IF(MSVC) + # size_t to long truncation warning + ADD_DEFINITIONS(-wd4267) +ENDIF() + + +ADD_CONVENIENCE_LIBRARY(wolfssl ${WOLFSSL_SOURCES}) + +# Workaround linker crash with older Ubuntu binutils +# e.g aborting at ../../bfd/merge.c line 873 in _bfd_merged_section_offset +IF((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (CMAKE_LINKER STREQUAL "/usr/bin/ld")) + EXECUTE_PROCESS(COMMAND ${CMAKE_LINKER} -v OUTPUT_VARIABLE LINKER_VERSION_STR) + STRING(REGEX MATCH "[0-9]+\\.[0-9]+" LINKER_VERSION + "${LINKER_VERSION_STR}") + IF(LINKER_VERSION AND (LINKER_VERSION VERSION_LESS 2.25)) + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_RELWITHDEBINFO + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_DEBUG + ${CMAKE_C_FLAGS_DEBUG}) + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_RELWITHDEBINFO + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_DEBUG + ${CMAKE_C_FLAGS_DEBUG}) + ENDIF() +ENDIF()
I would've just disabled -g\w+ unconditionally, for a simpler CMakeLists.txt We aren't going to debug wolfssl anyway (and if I'd need to debug it, for some weird reason, I know how to enable debugging temporarily)
+ + +SET(WOLFCRYPT_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/wolfcrypt/src) +SET(WOLFCRYPT_SOURCES +${WOLFCRYPT_SRCDIR}/aes.c +${WOLFCRYPT_SRCDIR}/arc4.c +${WOLFCRYPT_SRCDIR}/asn.c +#${WOLFCRYPT_SRCDIR}/blake2b.c +#${WOLFCRYPT_SRCDIR}/camellia.c +#${WOLFCRYPT_SRCDIR}/chacha.c +${WOLFCRYPT_SRCDIR}/coding.c +#${WOLFCRYPT_SRCDIR}/compress.c +${WOLFCRYPT_SRCDIR}/des3.c +${WOLFCRYPT_SRCDIR}/dh.c +${WOLFCRYPT_SRCDIR}/dsa.c +${WOLFCRYPT_SRCDIR}/ecc.c +${WOLFCRYPT_SRCDIR}/error.c +#${WOLFCRYPT_SRCDIR}/hc128.c +${WOLFCRYPT_SRCDIR}/hmac.c +${WOLFCRYPT_SRCDIR}/integer.c +${WOLFCRYPT_SRCDIR}/logging.c +#${WOLFCRYPT_SRCDIR}/md2.c +${WOLFCRYPT_SRCDIR}/md4.c +${WOLFCRYPT_SRCDIR}/md5.c +${WOLFCRYPT_SRCDIR}/memory.c +#${WOLFCRYPT_SRCDIR}/pkcs7.c +${WOLFCRYPT_SRCDIR}/pkcs12.c +#${WOLFCRYPT_SRCDIR}/poly1305.c +${WOLFCRYPT_SRCDIR}/pwdbased.c +${WOLFCRYPT_SRCDIR}/rabbit.c +${WOLFCRYPT_SRCDIR}/random.c +#${WOLFCRYPT_SRCDIR}/ripemd.c +${WOLFCRYPT_SRCDIR}/rsa.c +${WOLFCRYPT_SRCDIR}/sha.c +${WOLFCRYPT_SRCDIR}/sha256.c +${WOLFCRYPT_SRCDIR}/sha512.c +#${WOLFCRYPT_SRCDIR}/tfm.c +${WOLFCRYPT_SRCDIR}/wc_port.c +${WOLFCRYPT_SRCDIR}/wc_encrypt.c +${WOLFCRYPT_SRCDIR}/hash.c +${WOLFCRYPT_SRCDIR}/wolfmath.c) +ADD_CONVENIENCE_LIBRARY(wolfcrypt ${WOLFCRYPT_SOURCES}) + diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl new file mode 160000 index 00000000000..dc313ccf6ee --- /dev/null +++ b/extra/wolfssl/wolfssl @@ -0,0 +1 @@ +Subproject commit dc313ccf6ee0e99fb5a13793b82e412f396e11c6
I think we should only use wolfssl (any external project, actually) only at release points, not at intermediate commits. 4.0.0-stable should be ok, I guess. And, generaly, pefer to use system globally installed library, if available. Although in case of wolfssl (and libmarias3) I would not bother.
diff --git a/include/ssl_compat.h b/include/ssl_compat.h index c94b9671d5f..105405efff2 100644 --- a/include/ssl_compat.h +++ b/include/ssl_compat.h @@ -17,9 +17,9 @@ #include <openssl/opensslv.h>
/* OpenSSL version specific definitions */ -#if !defined(HAVE_YASSL) && defined(OPENSSL_VERSION_NUMBER) +#if defined(OPENSSL_VERSION_NUMBER)
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)) || defined (HAVE_WOLFSSL)
What's OPENSSL_VERSION_NUMBER in wolfssl?
#define HAVE_X509_check_host 1
May be move it to ssl.cmake? Where other such defines are.
#endif
diff --git a/include/sslopt-case.h b/include/sslopt-case.h index 4a8c65948cb..b129a97cc76 100644 --- a/include/sslopt-case.h +++ b/include/sslopt-case.h @@ -29,8 +29,8 @@ One can disable SSL later by using --skip-ssl or --ssl=0 */ opt_use_ssl= 1; - /* crl has no effect in yaSSL */ -#ifdef HAVE_YASSL +#ifdef HAVE_WOLFSSL + /* CRL does not work with WolfSSL */
Still?
opt_ssl_crl= NULL; opt_ssl_crlpath= NULL; #endif diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc index 2d6f5188034..1ce0c6bbc03 100644 --- a/mysys_ssl/my_crypt.cc +++ b/mysys_ssl/my_crypt.cc @@ -18,14 +18,10 @@ #include <my_global.h> #include <string.h>
-#ifdef HAVE_YASSL -#include "yassl.cc"
delete yassl.cc file, perhaps?
-#else #include <openssl/evp.h> #include <openssl/aes.h> #include <openssl/err.h> #include <openssl/rand.h> -#endif
#include <my_crypt.h> #include <ssl_compat.h> @@ -70,8 +66,24 @@ class MyCTX } virtual int finish(uchar *dst, uint *dlen) { +#ifdef HAVE_WOLFSSL + /* + Bug in WolfSSL - sometimes EVP_CipherFinal_ex + returns success without setting destination length + when it should return error. + We catch it by presetting invalid value for length, + and checking if it has changed after the call. + + See https://github.com/wolfSSL/wolfssl/issues/2092 + */ + *dlen= UINT_MAX; +#endif
I suppose you can remove it. This is fixed as of 4.0.0-stable tag.
if (!EVP_CipherFinal_ex(ctx, dst, (int*)dlen)) return MY_AES_BAD_DATA; +#ifdef HAVE_WOLFSSL + if (*dlen == UINT_MAX) + return MY_AES_BAD_DATA; +#endif return MY_AES_OK; } }; diff --git a/plugin/file_key_management/parser.cc b/plugin/file_key_management/parser.cc index 13a9dfa0cb6..27277f62994 100644 --- a/plugin/file_key_management/parser.cc +++ b/plugin/file_key_management/parser.cc @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, secret, strlen(secret), 1, key, iv);
- but alas! we want to support yassl too
does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds.
*/
void Parser::bytes_to_key(const unsigned char *salt, const char *input, diff --git a/sql/mysqld.cc b/sql/mysqld.cc index bfa03aa57c1..8c3a59c55d1 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7317,13 +7315,13 @@ static int show_ssl_get_verify_mode(THD *thd, SHOW_VAR *var, char *buff, { var->type= SHOW_LONG; var->value= buff; -#ifndef HAVE_YASSL +#ifndef HAVE_WOLFSSL if( thd->net.vio && thd->net.vio->ssl_arg ) *((long *)buff)= (long)SSL_get_verify_mode((SSL*)thd->net.vio->ssl_arg);
no SSL_get_verify_mode or an analog in wolfssl? I'm asking, as I see it has gone a long way towards OpenSSL compatibility
else *((long *)buff)= 0; #else - *((long *)buff) = 0; + *((long *)buff)= 0; #endif return 0; }
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Serg, Thanks a lot for looking.
diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake ... + #${CMAKE_SOURCE_DIR}/extra/wolfssl/taocrypt/include
don't you want to remove these (and many below) commented lines?
Sure, will do
+# Workaround linker crash with older Ubuntu binutils +# e.g aborting at ../../bfd/merge.c line 873 in _bfd_merged_section_offset ... I would've just disabled -g\w+ unconditionally, for a simpler CMakeLists.txt We aren't going to debug wolfssl anyway (and if I'd need to debug it, for some weird reason, I know how to enable debugging temporarily)
I would simplify by removing the check for the linker version, but I still like to have at least minimum debug symbols, for profiling, if nothing else. I think -g1 in this case would be appropriate for all Linuxes.
I think we should only use wolfssl (any external project, actually) only at release points, not at intermediate commits. 4.0.0-stable should be ok, I guess.
Agree, will change to point to 4.0.0-stable.
And, generaly, pefer to use system globally installed library, if available. Although in case of wolfssl (and libmarias3) I would not bother.
For -DWITH_SSL=bundled, I would like to have a bundled SSL library, because this is what we meant. This meaning got a little lost for the client library, but for server it always held.
--- a/include/ssl_compat.h +++ b/include/ssl_compat.h ... -#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)) || defined (HAVE_WOLFSSL) What's OPENSSL_VERSION_NUMBER in wolfssl?
It is 0x10001000L
#define HAVE_X509_check_host 1
May be move it to ssl.cmake? Where other such defines are.
Yep, makes sense, will do.
--- a/include/sslopt-case.h +++ b/include/sslopt-case.h @@ -29,8 +29,8 @@ One can disable SSL later by using --skip-ssl or --ssl=0 */ opt_use_ssl= 1; - /* crl has no effect in yaSSL */ -#ifdef HAVE_YASSL +#ifdef HAVE_WOLFSSL + /* CRL does not work with WolfSSL */
Still?
It does not work the way we need it to. There seems to be some CRL functionality, however X509_STORE_load_locations() is implemented as stub function that does nothing . About “stubs” : To get a clear picture what is really inside WolfSSL, and what not, I disabled all stub functions via -DNO_WOLFSSL_STUB when compiling WolfSSL. With that, we’ll get an unresolved symbol for WolfSSL_X509_STORE_load_locations. I’m not sure whether and how we can live without X509_STORE_load_locations().
--- a/mysys_ssl/my_crypt.cc +++ b/mysys_ssl/my_crypt.cc ... -#include "yassl.cc"
delete yassl.cc file, perhaps?
Sure, will do.
virtual int finish(uchar *dst, uint *dlen) { +#ifdef HAVE_WOLFSSL + /* + Bug in WolfSSL - sometimes EVP_CipherFinal_ex + returns success without setting destination length + when it should return error. + We catch it by presetting invalid value for length, + and checking if it has changed after the call. + + See https://github.com/wolfSSL/wolfssl/issues/2092 + */ + *dlen= UINT_MAX; +#endif
I suppose you can remove it. This is fixed as of 4.0.0-stable tag.
Can’t remove it just yet. They fixed a test case that I provided, but there was still failure in another corner case, for which I filed https://github.com/wolfSSL/wolfssl/issues/2224 , which is not fixed in 4.0.0-stable.
--- a/plugin/file_key_management/parser.cc +++ b/plugin/file_key_management/parser.cc @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, secret, strlen(secret), 1, key, iv);
- but alas! we want to support yassl too
does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds.
Yes, they have EVP_BytesToKey now.
--- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7317,13 +7315,13 @@ static int show_ssl_get_verify_mode(THD *thd, SHOW_VAR *var, char *buff, { var->type= SHOW_LONG; var->value= buff; -#ifndef HAVE_YASSL +#ifndef HAVE_WOLFSSL if( thd->net.vio && thd->net.vio->ssl_arg ) *((long *)buff)= (long)SSL_get_verify_mode((SSL*)thd->net.vio->ssl_arg);
no SSL_get_verify_mode or an analog in wolfssl? I'm asking, as I see it has gone a long way towards OpenSSL compatibility
They have SSL_get_verify_mode, but implemented as a stub that always returns 0. As mentioned above, I disabled stub function, thus there is no SSL_get_verify_mode we could use. From: Sergei Golubchik<mailto:serg@mariadb.org> Sent: Monday, 20 May 2019 12:55 To: Vladislav Vaintroub<mailto:wlad@mariadb.com> Cc: maria-developers@lists.launchpad.net<mailto:maria-developers@lists.launchpad.net> Subject: Re: 9e176e81b72: MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library Hi, Vladislav! It's good to see how small the patch is, and that wolfssl is more OpenSSL compatible than YaSSL. Few comments below. On May 19, Vladislav Vaintroub wrote:
revision-id: 9e176e81b72 (mariadb-10.4.4-61-g9e176e81b72) parent(s): ea679c88c32 author: Vladislav Vaintroub <wlad@mariadb.com> committer: Vladislav Vaintroub <wlad@mariadb.com> timestamp: 2019-05-01 11:28:56 +0100 message:
MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library
- Add new submodule for WolfSSL - Build and use wolfssl and wolfcrypt instead of yassl/taocrypt - Use HAVE_WOLFSSL instead of HAVE_YASSL - Increase MY_AES_CTX_SIZE, to avoid compile time asserts in my_crypt.cc (sizeof(EVP_CIPHER_CTX) is larger on WolfSSL)
diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake index aa42333a5c8..79a531ec323 100644 --- a/cmake/ssl.cmake +++ b/cmake/ssl.cmake @@ -48,29 +48,19 @@ MACRO (CHANGE_SSL_SETTINGS string) ENDMACRO()
MACRO (MYSQL_USE_BUNDLED_SSL) - SET(INC_DIRS - ${CMAKE_SOURCE_DIR}/extra/yassl/include - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/include + SET(INC_DIRS + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl/wolfssl + #${CMAKE_SOURCE_DIR}/extra/wolfssl/taocrypt/include
don't you want to remove these (and many below) commented lines?
) - SET(SSL_LIBRARIES yassl taocrypt) + SET(SSL_LIBRARIES wolfssl wolfcrypt) SET(SSL_INCLUDE_DIRS ${INC_DIRS}) - SET(SSL_INTERNAL_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/mySTL) + SET(SSL_DEFINES "-DHAVE_OPENSSL -DHAVE_WOLFSSL -DOPENSSL_ALL -DWOLFSSL_MYSQL_COMPATIBLE -DWC_NO_HARDEN") - SET(SSL_DEFINES "-DHAVE_YASSL -DYASSL_PREFIX -DHAVE_OPENSSL -DMULTI_THREADED") - SET(HAVE_ERR_remove_thread_state OFF CACHE INTERNAL "yassl doesn't have ERR_remove_thread_state") - SET(HAVE_EncryptAes128Ctr OFF CACHE INTERNAL "yassl doesn't support AES-CTR") - SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "yassl doesn't support AES-GCM") + SET(HAVE_ERR_remove_thread_state ON CACHE INTERNAL "wolfssl doesn't have ERR_remove_thread_state") + SET(HAVE_EncryptAes128Ctr ON CACHE INTERNAL "wolfssl does support AES-CTR") + SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "wolfssl does not support AES-GCM") CHANGE_SSL_SETTINGS("bundled") - ADD_SUBDIRECTORY(extra/yassl) - ADD_SUBDIRECTORY(extra/yassl/taocrypt) - GET_TARGET_PROPERTY(src yassl SOURCES) - FOREACH(file ${src}) - SET(SSL_SOURCES ${SSL_SOURCES} ${CMAKE_SOURCE_DIR}/extra/yassl/${file}) - ENDFOREACH() - GET_TARGET_PROPERTY(src taocrypt SOURCES) - FOREACH(file ${src}) - SET(SSL_SOURCES ${SSL_SOURCES} - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/${file}) - ENDFOREACH() + ADD_SUBDIRECTORY(extra/wolfssl) MESSAGE_ONCE(SSL_LIBRARIES "SSL_LIBRARIES = ${SSL_LIBRARIES}") ENDMACRO()
diff --git a/extra/wolfssl/CMakeLists.txt b/extra/wolfssl/CMakeLists.txt new file mode 100644 index 00000000000..c1165ad6c72 --- /dev/null +++ b/extra/wolfssl/CMakeLists.txt @@ -0,0 +1,119 @@ +# CMakeLists.txt +# +# Copyright (C) 2006-2015 wolfSSL Inc. +# +# This file is part of wolfSSL. (formerly known as CyaSSL) +# +# wolfSSL is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# wolfSSL is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + +SET(WOLFSSL_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/src) +ADD_DEFINITIONS(${SSL_DEFINES}) +ADD_DEFINITIONS( + -DWOLFSSL_MYSQL_COMPATIBLE + -DHAVE_ECC + -DECC_TIMING_RESISTANT + -DBUILDING_WOLFSSL + -DHAVE_HASHDRBG + -DWOLFSSL_AES_DIRECT + -DWOLFSSL_SHA384 + -DWOLFSSL_SHA512 + -DWOLFSSL_SHA224 + -DSESSION_CERT + -DKEEP_OUR_CERT + -DWOLFSSL_STATIC_RSA + -DWC_RSA_BLINDING + -DHAVE_TLS_EXTENSIONS + -DHAVE_AES_ECB + -DWOLFSSL_AES_COUNTER + -DNO_WOLFSSL_STUB) + +SET(WOLFSSL_SOURCES + ${WOLFSSL_SRCDIR}/crl.c + ${WOLFSSL_SRCDIR}/internal.c + ${WOLFSSL_SRCDIR}/keys.c + ${WOLFSSL_SRCDIR}/tls.c + ${WOLFSSL_SRCDIR}/wolfio.c + ${WOLFSSL_SRCDIR}/ocsp.c + ${WOLFSSL_SRCDIR}/ssl.c) +ADD_DEFINITIONS(-DWOLFSSL_LIB) +INCLUDE_DIRECTORIES(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl) +IF(MSVC) + # size_t to long truncation warning + ADD_DEFINITIONS(-wd4267) +ENDIF() + + +ADD_CONVENIENCE_LIBRARY(wolfssl ${WOLFSSL_SOURCES}) + +# Workaround linker crash with older Ubuntu binutils +# e.g aborting at ../../bfd/merge.c line 873 in _bfd_merged_section_offset +IF((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (CMAKE_LINKER STREQUAL "/usr/bin/ld")) + EXECUTE_PROCESS(COMMAND ${CMAKE_LINKER} -v OUTPUT_VARIABLE LINKER_VERSION_STR) + STRING(REGEX MATCH "[0-9]+\\.[0-9]+" LINKER_VERSION + "${LINKER_VERSION_STR}") + IF(LINKER_VERSION AND (LINKER_VERSION VERSION_LESS 2.25)) + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_RELWITHDEBINFO + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_DEBUG + ${CMAKE_C_FLAGS_DEBUG}) + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_RELWITHDEBINFO + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_DEBUG + ${CMAKE_C_FLAGS_DEBUG}) + ENDIF() +ENDIF()
I would've just disabled -g\w+ unconditionally, for a simpler CMakeLists.txt We aren't going to debug wolfssl anyway (and if I'd need to debug it, for some weird reason, I know how to enable debugging temporarily)
+ + +SET(WOLFCRYPT_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/wolfcrypt/src) +SET(WOLFCRYPT_SOURCES +${WOLFCRYPT_SRCDIR}/aes.c +${WOLFCRYPT_SRCDIR}/arc4.c +${WOLFCRYPT_SRCDIR}/asn.c +#${WOLFCRYPT_SRCDIR}/blake2b.c +#${WOLFCRYPT_SRCDIR}/camellia.c +#${WOLFCRYPT_SRCDIR}/chacha.c +${WOLFCRYPT_SRCDIR}/coding.c +#${WOLFCRYPT_SRCDIR}/compress.c +${WOLFCRYPT_SRCDIR}/des3.c +${WOLFCRYPT_SRCDIR}/dh.c +${WOLFCRYPT_SRCDIR}/dsa.c +${WOLFCRYPT_SRCDIR}/ecc.c +${WOLFCRYPT_SRCDIR}/error.c +#${WOLFCRYPT_SRCDIR}/hc128.c +${WOLFCRYPT_SRCDIR}/hmac.c +${WOLFCRYPT_SRCDIR}/integer.c +${WOLFCRYPT_SRCDIR}/logging.c +#${WOLFCRYPT_SRCDIR}/md2.c +${WOLFCRYPT_SRCDIR}/md4.c +${WOLFCRYPT_SRCDIR}/md5.c +${WOLFCRYPT_SRCDIR}/memory.c +#${WOLFCRYPT_SRCDIR}/pkcs7.c +${WOLFCRYPT_SRCDIR}/pkcs12.c +#${WOLFCRYPT_SRCDIR}/poly1305.c +${WOLFCRYPT_SRCDIR}/pwdbased.c +${WOLFCRYPT_SRCDIR}/rabbit.c +${WOLFCRYPT_SRCDIR}/random.c +#${WOLFCRYPT_SRCDIR}/ripemd.c +${WOLFCRYPT_SRCDIR}/rsa.c +${WOLFCRYPT_SRCDIR}/sha.c +${WOLFCRYPT_SRCDIR}/sha256.c +${WOLFCRYPT_SRCDIR}/sha512.c +#${WOLFCRYPT_SRCDIR}/tfm.c +${WOLFCRYPT_SRCDIR}/wc_port.c +${WOLFCRYPT_SRCDIR}/wc_encrypt.c +${WOLFCRYPT_SRCDIR}/hash.c +${WOLFCRYPT_SRCDIR}/wolfmath.c) +ADD_CONVENIENCE_LIBRARY(wolfcrypt ${WOLFCRYPT_SOURCES}) + diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl new file mode 160000 index 00000000000..dc313ccf6ee --- /dev/null +++ b/extra/wolfssl/wolfssl @@ -0,0 +1 @@ +Subproject commit dc313ccf6ee0e99fb5a13793b82e412f396e11c6
I think we should only use wolfssl (any external project, actually) only at release points, not at intermediate commits. 4.0.0-stable should be ok, I guess. And, generaly, pefer to use system globally installed library, if available. Although in case of wolfssl (and libmarias3) I would not bother.
diff --git a/include/ssl_compat.h b/include/ssl_compat.h index c94b9671d5f..105405efff2 100644 --- a/include/ssl_compat.h +++ b/include/ssl_compat.h @@ -17,9 +17,9 @@ #include <openssl/opensslv.h>
/* OpenSSL version specific definitions */ -#if !defined(HAVE_YASSL) && defined(OPENSSL_VERSION_NUMBER) +#if defined(OPENSSL_VERSION_NUMBER)
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)) || defined (HAVE_WOLFSSL)
What's OPENSSL_VERSION_NUMBER in wolfssl?
#define HAVE_X509_check_host 1
May be move it to ssl.cmake? Where other such defines are.
#endif
diff --git a/include/sslopt-case.h b/include/sslopt-case.h index 4a8c65948cb..b129a97cc76 100644 --- a/include/sslopt-case.h +++ b/include/sslopt-case.h @@ -29,8 +29,8 @@ One can disable SSL later by using --skip-ssl or --ssl=0 */ opt_use_ssl= 1; - /* crl has no effect in yaSSL */ -#ifdef HAVE_YASSL +#ifdef HAVE_WOLFSSL + /* CRL does not work with WolfSSL */
Still?
opt_ssl_crl= NULL; opt_ssl_crlpath= NULL; #endif diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc index 2d6f5188034..1ce0c6bbc03 100644 --- a/mysys_ssl/my_crypt.cc +++ b/mysys_ssl/my_crypt.cc @@ -18,14 +18,10 @@ #include <my_global.h> #include <string.h>
-#ifdef HAVE_YASSL -#include "yassl.cc"
delete yassl.cc file, perhaps?
-#else #include <openssl/evp.h> #include <openssl/aes.h> #include <openssl/err.h> #include <openssl/rand.h> -#endif
#include <my_crypt.h> #include <ssl_compat.h> @@ -70,8 +66,24 @@ class MyCTX } virtual int finish(uchar *dst, uint *dlen) { +#ifdef HAVE_WOLFSSL + /* + Bug in WolfSSL - sometimes EVP_CipherFinal_ex + returns success without setting destination length + when it should return error. + We catch it by presetting invalid value for length, + and checking if it has changed after the call. + + See https://github.com/wolfSSL/wolfssl/issues/2092 + */ + *dlen= UINT_MAX; +#endif
I suppose you can remove it. This is fixed as of 4.0.0-stable tag.
if (!EVP_CipherFinal_ex(ctx, dst, (int*)dlen)) return MY_AES_BAD_DATA; +#ifdef HAVE_WOLFSSL + if (*dlen == UINT_MAX) + return MY_AES_BAD_DATA; +#endif return MY_AES_OK; } }; diff --git a/plugin/file_key_management/parser.cc b/plugin/file_key_management/parser.cc index 13a9dfa0cb6..27277f62994 100644 --- a/plugin/file_key_management/parser.cc +++ b/plugin/file_key_management/parser.cc @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, secret, strlen(secret), 1, key, iv);
- but alas! we want to support yassl too
does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds.
*/
void Parser::bytes_to_key(const unsigned char *salt, const char *input, diff --git a/sql/mysqld.cc b/sql/mysqld.cc index bfa03aa57c1..8c3a59c55d1 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7317,13 +7315,13 @@ static int show_ssl_get_verify_mode(THD *thd, SHOW_VAR *var, char *buff, { var->type= SHOW_LONG; var->value= buff; -#ifndef HAVE_YASSL +#ifndef HAVE_WOLFSSL if( thd->net.vio && thd->net.vio->ssl_arg ) *((long *)buff)= (long)SSL_get_verify_mode((SSL*)thd->net.vio->ssl_arg);
no SSL_get_verify_mode or an analog in wolfssl? I'm asking, as I see it has gone a long way towards OpenSSL compatibility
else *((long *)buff)= 0; #else - *((long *)buff) = 0; + *((long *)buff)= 0; #endif return 0; }
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi, Vladislav! On May 20, Vladislav Vaintroub wrote:
--- a/include/sslopt-case.h +++ b/include/sslopt-case.h @@ -29,8 +29,8 @@ One can disable SSL later by using --skip-ssl or --ssl=0 */ opt_use_ssl= 1; - /* crl has no effect in yaSSL */ -#ifdef HAVE_YASSL +#ifdef HAVE_WOLFSSL + /* CRL does not work with WolfSSL */
Still?
It does not work the way we need it to. There seems to be some CRL functionality, however X509_STORE_load_locations() is implemented as stub function that does nothing .
There's wolfSSL_CertManagerLoadCRL(), which, supposedly loads a certificate from a file. There's no functionality, as far as I can see, to load a whole directory of crls.
With that, we’ll get an unresolved symbol for WolfSSL_X509_STORE_load_locations. I’m not sure whether and how we can live without X509_STORE_load_locations().
Okay. If needed we could implement X509_STORE_load_locations for WolfSSL (like I had yassl.cc with missing functionality). I guess, just iterating all files in a dir and calling wolfSSL_CertManagerLoadCRL() for every file should do it. Probably not now, because "replacing yassl" only means "provide at least the functionality that yassl had".
virtual int finish(uchar *dst, uint *dlen) { +#ifdef HAVE_WOLFSSL + /* + Bug in WolfSSL - sometimes EVP_CipherFinal_ex + returns success without setting destination length + when it should return error. + We catch it by presetting invalid value for length, + and checking if it has changed after the call. + + See https://github.com/wolfSSL/wolfssl/issues/2092 + */ + *dlen= UINT_MAX; +#endif
I suppose you can remove it. This is fixed as of 4.0.0-stable tag.
Can’t remove it just yet. They fixed a test case that I provided, but there was still failure in another corner case, for which I filed https://github.com/wolfSSL/wolfssl/issues/2224 , which is not fixed in 4.0.0-stable.
Update the comment to mention the new issue, please
--- a/plugin/file_key_management/parser.cc +++ b/plugin/file_key_management/parser.cc @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, secret, strlen(secret), 1, key, iv);
- but alas! we want to support yassl too
does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds.
Yes, they have EVP_BytesToKey now.
That means you can replace the whole Parser::bytes_to_key() body with one EVP_BytesToKey(), right? Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Sergei Golubchik -
Vladislav Vaintroub