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 <w...@mariadb.com>
> committer: Vladislav Vaintroub <w...@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 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
