Below is the list of changes that have just been committed into a local
5.0 repository of dkatz. When dkatz does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-07-05 15:01:29-04:00, dkatz@stripped +7 -0
Bug #29579 Clients using SSL can hang the server
Added an option to yassl to allow "quiet shutdown" like openssl does. This option causes the SSL libs to NOT perform the close_notify handshake during shutdown. This fixes a hang we experience because we hold a lock during socket shutdown.
client/mysqltest.c@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +67 -13
Added new command to mysqltest to send a quit command to the server, but to not close the actual socket on our end.
Also changed code to reuse connection slots, so that the tests can open and close sockets in a loop.
extra/yassl/include/openssl/ssl.h@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +2 -0
Added C accessors to the quietShutdown option.
extra/yassl/include/yassl_int.hpp@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +3 -0
Added quietShutdown_ member and accessor methods to the SSL class.
extra/yassl/src/ssl.cpp@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +16 -2
Added accessors to get/set the quietShutdown option and to not perform the shutdown handshake if quietShutdown is set.
extra/yassl/src/yassl_int.cpp@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +13 -1
Added quietShutdown_ member and accessor methods to the SSL class.
mysql-test/t/ssl.test@stripped, 2007-07-05 15:01:25-04:00, dkatz@stripped +41 -0
Added a test that causes an unpatched server to hang during SSL socket shutdown.
vio/viossl.c@stripped, 2007-07-05 15:01:26-04:00, dkatz@stripped +10 -0
Added line to set the quiet_shutdown option before shutting down the socket.
diff -Nrup a/client/mysqltest.c b/client/mysqltest.c
--- a/client/mysqltest.c 2007-06-14 07:37:34 -04:00
+++ b/client/mysqltest.c 2007-07-05 15:01:25 -04:00
@@ -280,6 +280,7 @@ enum enum_commands {
Q_REPLACE_REGEX, Q_REMOVE_FILE, Q_FILE_EXIST,
Q_WRITE_FILE, Q_COPY_FILE, Q_PERL, Q_DIE, Q_EXIT, Q_SKIP,
Q_CHMOD_FILE, Q_APPEND_FILE, Q_CAT_FILE, Q_DIFF_FILES,
+ Q_SEND_QUIT,
Q_UNKNOWN, /* Unknown command. */
Q_COMMENT, /* Comments, ignored. */
@@ -367,6 +368,7 @@ const char *command_names[]=
"append_file",
"cat_file",
"diff_files",
+ "send_quit",
0
};
@@ -2555,6 +2557,48 @@ void do_diff_files(struct st_command *co
DBUG_VOID_RETURN;
}
+ /*
+ SYNOPSIS
+ do_send_quit
+ command called command
+
+ DESCRIPTION
+ Sends a simple quite command to the server for the named connectioned.
+
+ */
+
+void do_send_quit(struct st_command *command)
+{
+ char *p= command->first_argument, *name;
+ struct st_connection *con;
+
+ DBUG_ENTER("do_send_quit");
+ DBUG_PRINT("enter",("name: '%s'",p));
+
+ if (!*p)
+ die("Missing connection name in do_send_quit");
+ name= p;
+ while (*p && !my_isspace(charset_info,*p))
+ p++;
+
+ if (*p)
+ *p++= 0;
+ command->last_argument= p;
+
+ /* Loop through connection pool for connection to close */
+ for (con= connections; con < next_con; con++)
+ {
+ DBUG_PRINT("info", ("con->name: %s", con->name));
+ if (!strcmp(con->name, name))
+ {
+ simple_command(&con->mysql,COM_QUIT,NullS,0,1);
+ DBUG_VOID_RETURN;
+ }
+ }
+ die("connection '%s' not found in connection pool", name);
+}
+
+
/*
SYNOPSIS
do_perl
@@ -3646,6 +3690,7 @@ void do_connect(struct st_command *comma
int con_port= opt_port;
char *con_options;
bool con_ssl= 0, con_compress= 0;
+ struct st_connection* con_slot;
static DYNAMIC_STRING ds_connection_name;
static DYNAMIC_STRING ds_host;
@@ -3731,19 +3776,24 @@ void do_connect(struct st_command *comma
con_options= end;
}
- if (next_con == connections_end)
- die("Connection limit exhausted, you can have max %d connections",
- (int) (sizeof(connections)/sizeof(struct st_connection)));
-
if (find_connection_by_name(ds_connection_name.str))
die("Connection %s already exists", ds_connection_name.str);
+
+ if (!(con_slot= find_connection_by_name("closed_connection")))
+ {
+ if (next_con == connections_end)
+ die("Connection limit exhausted, you can have max %d connections",
+ (int) (sizeof(connections)/sizeof(struct st_connection)));
+
+ con_slot= next_con;
+ }
- if (!mysql_init(&next_con->mysql))
+ if (!mysql_init(&con_slot->mysql))
die("Failed on mysql_init()");
if (opt_compress || con_compress)
- mysql_options(&next_con->mysql, MYSQL_OPT_COMPRESS, NullS);
- mysql_options(&next_con->mysql, MYSQL_OPT_LOCAL_INFILE, 0);
- mysql_options(&next_con->mysql, MYSQL_SET_CHARSET_NAME,
+ mysql_options(&con_slot->mysql, MYSQL_OPT_COMPRESS, NullS);
+ mysql_options(&con_slot->mysql, MYSQL_OPT_LOCAL_INFILE, 0);
+ mysql_options(&con_slot->mysql, MYSQL_SET_CHARSET_NAME,
charset_info->csname);
if (opt_charsets_dir)
mysql_options(&cur_con->mysql, MYSQL_SET_CHARSET_DIR,
@@ -3752,12 +3802,12 @@ void do_connect(struct st_command *comma
#ifdef HAVE_OPENSSL
if (opt_use_ssl || con_ssl)
{
- mysql_ssl_set(&next_con->mysql, opt_ssl_key, opt_ssl_cert, opt_ssl_ca,
+ mysql_ssl_set(&con_slot->mysql, opt_ssl_key, opt_ssl_cert, opt_ssl_ca,
opt_ssl_capath, opt_ssl_cipher);
#if MYSQL_VERSION_ID >= 50000
/* Turn on ssl_verify_server_cert only if host is "localhost" */
opt_ssl_verify_server_cert= !strcmp(ds_host.str, "localhost");
- mysql_options(&next_con->mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
+ mysql_options(&con_slot->mysql, MYSQL_OPT_SSL_VERIFY_SERVER_CERT,
&opt_ssl_verify_server_cert);
#endif
}
@@ -3771,16 +3821,19 @@ void do_connect(struct st_command *comma
if (ds_database.length && !strcmp(ds_database.str,"*NO-ONE*"))
dynstr_set(&ds_database, "");
- if (connect_n_handle_errors(command, &next_con->mysql,
+ if (connect_n_handle_errors(command, &con_slot->mysql,
ds_host.str,ds_user.str,
ds_password.str, ds_database.str,
con_port, ds_sock.str))
{
DBUG_PRINT("info", ("Inserting connection %s in connection pool",
ds_connection_name.str));
- if (!(next_con->name= my_strdup(ds_connection_name.str, MYF(MY_WME))))
+ if (!(con_slot->name= my_strdup(ds_connection_name.str, MYF(MY_WME))))
die("Out of memory");
- cur_con= next_con++;
+ cur_con= con_slot;
+
+ if (con_slot == next_con)
+ next_con++; /* if we used the next_con slot, advance the pointer */
}
dynstr_free(&ds_connection_name);
@@ -6349,6 +6402,7 @@ int main(int argc, char **argv)
case Q_WRITE_FILE: do_write_file(command); break;
case Q_APPEND_FILE: do_append_file(command); break;
case Q_DIFF_FILES: do_diff_files(command); break;
+ case Q_SEND_QUIT: do_send_quit(command); break;
case Q_CAT_FILE: do_cat_file(command); break;
case Q_COPY_FILE: do_copy_file(command); break;
case Q_CHMOD_FILE: do_chmod_file(command); break;
diff -Nrup a/extra/yassl/include/openssl/ssl.h b/extra/yassl/include/openssl/ssl.h
--- a/extra/yassl/include/openssl/ssl.h 2007-03-28 09:04:27 -04:00
+++ b/extra/yassl/include/openssl/ssl.h 2007-07-05 15:01:25 -04:00
@@ -277,6 +277,8 @@ int SSL_session_reused(SSL*);
int SSL_set_rfd(SSL*, int);
int SSL_set_wfd(SSL*, int);
void SSL_set_shutdown(SSL*, int);
+void SSL_set_quiet_shutdown(SSL *ssl,int mode);
+int SSL_get_quiet_shutdown(SSL *ssl);
int SSL_want_read(SSL*);
int SSL_want_write(SSL*);
diff -Nrup a/extra/yassl/include/yassl_int.hpp b/extra/yassl/include/yassl_int.hpp
--- a/extra/yassl/include/yassl_int.hpp 2007-03-14 22:15:08 -04:00
+++ b/extra/yassl/include/yassl_int.hpp 2007-07-05 15:01:25 -04:00
@@ -584,6 +584,7 @@ class SSL {
Socket socket_; // socket wrapper
Buffers buffers_; // buffered handshakes and data
Log log_; // logger
+ bool quietShutdown_;
// optimization variables
bool has_data_; // buffered data ready?
@@ -610,6 +611,7 @@ public:
Buffers& useBuffers();
bool HasData() const;
+ bool GetQuietShutdown() const;
// sets
void set_pending(Cipher suite);
@@ -621,6 +623,7 @@ public:
void SetError(YasslError);
int SetCompression();
void UnSetCompression();
+ void SetQuietShutdown(bool mode);
// helpers
bool isTLS() const;
diff -Nrup a/extra/yassl/src/ssl.cpp b/extra/yassl/src/ssl.cpp
--- a/extra/yassl/src/ssl.cpp 2007-03-28 12:28:29 -04:00
+++ b/extra/yassl/src/ssl.cpp 2007-07-05 15:01:25 -04:00
@@ -411,13 +411,27 @@ int SSL_clear(SSL* ssl)
int SSL_shutdown(SSL* ssl)
{
- Alert alert(warning, close_notify);
- sendAlert(*ssl, alert);
+ if (!ssl->GetQuietShutdown()) {
+ Alert alert(warning, close_notify);
+ sendAlert(*ssl, alert);
+ }
ssl->useLog().ShowTCP(ssl->getSocket().get_fd(), true);
GetErrors().Remove();
return SSL_SUCCESS;
+}
+
+
+void SSL_set_quiet_shutdown(SSL *ssl,int mode)
+{
+ ssl->SetQuietShutdown(mode != 0);
+}
+
+
+int SSL_get_quiet_shutdown(SSL *ssl)
+{
+ return ssl->GetQuietShutdown();
}
diff -Nrup a/extra/yassl/src/yassl_int.cpp b/extra/yassl/src/yassl_int.cpp
--- a/extra/yassl/src/yassl_int.cpp 2007-01-25 13:34:38 -05:00
+++ b/extra/yassl/src/yassl_int.cpp 2007-07-05 15:01:25 -04:00
@@ -291,7 +291,7 @@ const ClientKeyFactory& sslFactory::getC
SSL::SSL(SSL_CTX* ctx)
: secure_(ctx->getMethod()->getVersion(), crypto_.use_random(),
ctx->getMethod()->getSide(), ctx->GetCiphers(), ctx,
- ctx->GetDH_Parms().set_), has_data_(false)
+ ctx->GetDH_Parms().set_), has_data_(false), quietShutdown_(false)
{
if (int err = crypto_.get_random().GetError()) {
SetError(YasslError(err));
@@ -773,6 +773,12 @@ void SSL::SetError(YasslError ye)
// TODO: add string here
}
+// set the quiet shutdown mode (close_nofiy not sent or received on shutdown)
+void SSL::SetQuietShutdown(bool mode)
+{
+ quietShutdown_ = mode;
+}
+
Buffers& SSL::useBuffers()
{
@@ -1327,6 +1333,12 @@ const Socket& SSL::getSocket() const
YasslError SSL::GetError() const
{
return states_.What();
+}
+
+
+bool SSL::GetQuietShutdown() const
+{
+ return quietShutdown_;
}
diff -Nrup a/mysql-test/t/ssl.test b/mysql-test/t/ssl.test
--- a/mysql-test/t/ssl.test 2007-03-05 04:03:40 -05:00
+++ b/mysql-test/t/ssl.test 2007-07-05 15:01:25 -04:00
@@ -14,4 +14,45 @@ SHOW STATUS LIKE 'Ssl_cipher';
# Check ssl turned on
SHOW STATUS LIKE 'Ssl_cipher';
+disconnect ssl_con;
+
+
+#
+# Bug #29579 Clients using SSL can hang the server
+#
+
+connect (ssl_con,localhost,root,,,,,SSL);
+
+create table t1 (a int);
+
+disconnect ssl_con;
+
+let $count= 2000;
+while ($count)
+{
+
+ connect (ssl_con,localhost,root,,,,,SSL);
+
+ eval insert into t1 values ($count);
+
+ dec $count;
+
+ # This select causes the net buffer to fill as the server sends the results
+ # but the client doesn't reap the results. The results are larger each time
+ # through the loop, so that eventually the buffer is completely full
+ # at the exact moment the server attempts to the close the connection with
+ # the lock held.
+ send select * from t1;
+
+ # now send the quit the command so the server will initiate the shutdown.
+ send_quit ssl_con;
+
+ # if the server is hung, this will hang too:
+ connect (ssl_con2,localhost,root,,,,,SSL);
+
+ # no hang if we get here, close and retry
+ disconnect ssl_con2;
+
+ disconnect ssl_con;
+}
diff -Nrup a/vio/viossl.c b/vio/viossl.c
--- a/vio/viossl.c 2007-02-06 15:55:36 -05:00
+++ b/vio/viossl.c 2007-07-05 15:01:26 -04:00
@@ -123,6 +123,16 @@ int vio_ssl_close(Vio *vio)
if (ssl)
{
+ /*
+ THE SSL standard says that SSL sockets must send and receive a close_notify
+ alert on socket shutdown to avoid truncation attacks. However, this can
+ cause problems since we often hold a lock during shutdown and this IO can
+ take an unbounded amount of time to complete. Since our packets are self
+ describing with length, we aren't vunerable to these attacks. Therefore,
+ we just shutdown by closing the socket (quiet shutdown).
+ */
+ SSL_set_quiet_shutdown(ssl, 1);
+
switch ((r= SSL_shutdown(ssl)))
{
case 1:
