Hi, Vladislav! Again, that's quite ok. I don't like a new lock around sslaccept(), but I couldn't find any other solution for it :( See few comments below. On Dec 03, Vladislav Vaintroub wrote:
revision-id: af6a8ea0f79d628384a61986482dec7d3ff751dd (mariadb-10.4.0-55-gaf6a8ea0f79) parent(s): 757530b83ca1cc550127311dc6edb47152ba0c6a author: Vladislav Vaintroub <wlad@mariadb.com> committer: Vladislav Vaintroub <wlad@mariadb.com> timestamp: 2018-12-02 01:25:04 +0100 message:
MDEV-16266 : New command FLUSH SSL ro reload SSL acceptor context.
Reloads certificate, key, CA, CRL.
diff --git a/mysql-test/main/flush_ssl.test b/mysql-test/main/flush_ssl.test new file mode 100644 index 00000000000..24dbb4719f3 --- /dev/null +++ b/mysql-test/main/flush_ssl.test @@ -0,0 +1,24 @@ +source include/have_ssl_communication.inc; + +connect ssl_con,localhost,root,,,,,SSL; +SELECT VARIABLE_VALUE != '0' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS'; +FLUSH SSL; + +# Check if SSL_ACCEPTS was flushed (SSL_ACCEPTS = 0) +SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS';
How about the test that a certificate was actually reloaded? That's the main functionality, right? Nobody wanted a new command just to reset SSL_ACCEPTS Note, that if you want to generate a new certificate for mysql-test, don't forget to extend mysql-test/lib/generate-ssl-certs.sh
+ +CREATE USER u; +connect ssl_con2,localhost,u,,,,,SSL; + +# Check SSL_ACCEPTS increased to 1 +SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS'; + +# Check that FLUSH SSL does not work for unprivileged user +error ER_SPECIFIC_ACCESS_DENIED_ERROR; +FLUSH SSL; + +disconnect ssl_con2; +disconnect ssl_con; +connection default; +DROP USER u; + diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 9ff47dc1ff1..13aad0a0197 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4766,6 +4771,56 @@ static void openssl_lock(int mode, openssl_lock_t *lock, const char *file, } #endif /* HAVE_OPENSSL10 */
+ +struct SSL_ACCEPTOR_STATS +{ + long accept; + long accept_good; + long cache_size; + long verify_mode; + long verify_depth; + const char *session_cache_mode; + long zero; + SSL_ACCEPTOR_STATS() + { + session_cache_mode = "NONE";
are you relying on everything else being zero'ed by default because SSL_ACCEPTOR_STATS is instantiated statically? May be, still, add a bzero?
+ } + void init() + { + DBUG_ASSERT(ssl_acceptor_fd); + accept = 0; + accept_good = 0; + verify_mode = SSL_CTX_get_verify_mode(ssl_acceptor_fd->ssl_context); + verify_depth = SSL_CTX_get_verify_depth(ssl_acceptor_fd->ssl_context); + cache_size = SSL_CTX_sess_get_cache_size(ssl_acceptor_fd->ssl_context); + switch (SSL_CTX_get_session_cache_mode(ssl_acceptor_fd->ssl_context)) + { + case SSL_SESS_CACHE_OFF: + session_cache_mode = "OFF"; break; + case SSL_SESS_CACHE_CLIENT: + session_cache_mode = "CLIENT"; break; + case SSL_SESS_CACHE_SERVER: + session_cache_mode = "SERVER"; break; + case SSL_SESS_CACHE_BOTH: + session_cache_mode = "BOTH"; break; + case SSL_SESS_CACHE_NO_AUTO_CLEAR: + session_cache_mode = "NO_AUTO_CLEAR"; break; + case SSL_SESS_CACHE_NO_INTERNAL_LOOKUP: + session_cache_mode = "NO_INTERNAL_LOOKUP"; break; + default: + session_cache_mode = "Unknown"; break; + } + } +}; + +static SSL_ACCEPTOR_STATS ssl_acceptor_stats; +void ssl_acceptor_stats_update(int sslaccept_ret) +{ + ssl_acceptor_stats.accept++; + if (!sslaccept_ret) + ssl_acceptor_stats.accept_good++;
use statistic_increment here
+} + static void init_ssl() { #if defined(HAVE_OPENSSL) && !defined(EMBEDDED_LIBRARY) @@ -4804,6 +4862,38 @@ static void init_ssl() #endif /* HAVE_OPENSSL && ! EMBEDDED_LIBRARY */ }
+/* Reinitialize SSL (FLUSH SSL) */ +int reinit_ssl() +{ +#if defined(HAVE_OPENSSL) && !defined(EMBEDDED_LIBRARY) + if (!opt_use_ssl) + return 0; +#ifndef DBUG_OFF + char *old_ssl_cert;
it's generally prefered to use __attribute__((unused)) instead of #ifdef
+#endif + + DBUG_EXECUTE_IF("simulate_bad_ssl_cert", old_ssl_cert= opt_ssl_cert; opt_ssl_cert=const_cast<char *>("");); + + enum enum_ssl_init_error error = SSL_INITERR_NOERROR; + st_VioSSLFd *new_fd = new_VioSSLAcceptorFd(opt_ssl_key, opt_ssl_cert, + opt_ssl_ca, opt_ssl_capath, opt_ssl_cipher, &error, opt_ssl_crl, opt_ssl_crlpath); + + DBUG_EXECUTE_IF("simulate_bad_ssl_cert", opt_ssl_cert= old_ssl_cert;); + + if (!new_fd) + { + my_printf_error(ER_UNKNOWN_ERROR, "Failed to refresh SSL, error: %s", MYF(0), + sslGetErrString(error)); + return 1; + } + mysql_rwlock_wrlock(&LOCK_ssl_refresh); + free_vio_ssl_acceptor_fd(ssl_acceptor_fd); + ssl_acceptor_fd= new_fd; + ssl_acceptor_stats.init(); + mysql_rwlock_unlock(&LOCK_ssl_refresh); + return 0; +#endif +}
Regards, Sergei Chief Architect MariaDB and security@mariadb.org