A patch for Bug#17087862: THREAD POOL PLUGIN FAILS TO MAINTAIN

Alexander Nozdrin · Alexander Nozdrin · commit 01038fd603e0 · 2013-08-12T12:42:37.000+04:00
ENCRYPTED CONNECTION UNDER LOAD. The user visible problem was that an error in one connection might have affected other connections. When that happened, the other connection was closed by the server, thus the client received the "Lost connection" error. The server however didn't crash. Technically, the problem was that OpenSSL requires that the thread's error queue is empty before invoking any I/O operation (https://www.openssl.org/docs/ssl/SSL_get_error.html). That was not the case. The bug happened when the error queue is not empty and a non-blocking I/O-operation failed because it would block. YaSSL is not affected as it employs different scheme for handling errors. MySQL-5.5 is not affected as it uses blocking I/O calls. Test case is missing because the bug appeared under the highload and depended on thread scheduling.
diff --git a/mysys_ssl/my_rnd.cc b/mysys_ssl/my_rnd.cc
@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+   Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -26,6 +26,7 @@
 
 #elif defined(HAVE_OPENSSL)
 #include <openssl/rand.h>
+#include <openssl/err.h>
 #endif /* HAVE_YASSL */
 
 
@@ -65,22 +66,32 @@ double my_rnd(struct rand_struct *rand_st)
 
 double my_rnd_ssl(struct rand_struct *rand_st)
 {
-
 #if defined(HAVE_YASSL) || defined(HAVE_OPENSSL)
   int rc;
   unsigned int res;
 
-#if defined(HAVE_YASSL)
+#if defined(HAVE_YASSL) /* YaSSL */
   rc= yaSSL::RAND_bytes((unsigned char *) &res, sizeof (unsigned int));
-#else
+
+  if (!rc)
+    return my_rnd(rand_st);
+#else                   /* OpenSSL */
   rc= RAND_bytes((unsigned char *) &res, sizeof (unsigned int));
+
+  if (!rc)
+  {
+    ERR_clear_error();
+    return my_rnd(rand_st);
+  }
 #endif /* HAVE_YASSL */
 
-  if (rc)
-    return (double)res / (double)UINT_MAX;
-  else
+  return (double)res / (double)UINT_MAX;
+
+#else /* !defined(HAVE_YASSL) && !defined(HAVE_OPENSSL) */
+
+  return my_rnd(rand_st);
+
 #endif /* defined(HAVE_YASSL) || defined(HAVE_OPENSSL) */
-    return my_rnd(rand_st);
 }
 
 #ifdef __cplusplus
diff --git a/sql-common/client_authentication.cc b/sql-common/client_authentication.cc
@@ -111,6 +111,7 @@ RSA *rsa_init(MYSQL *mysql)
   fclose(pub_key_file);
   if (g_public_key == NULL)
   {
+    ERR_clear_error();
     fprintf(stderr, "Public key is not in PEM format: '%s'\n",
             mysql->options.extension->server_public_key_path);
     return 0;
@@ -205,7 +206,10 @@ int sha256_password_auth_client(MYSQL_PLUGIN_VIO *vio, MYSQL *mysql)
         public_key= PEM_read_bio_RSA_PUBKEY(bio, NULL, NULL, NULL);
         BIO_free(bio);
         if (public_key == 0)
+        {
+          ERR_clear_error();
           DBUG_RETURN(CR_ERROR);
+        }
         got_public_key_from_server= true;
       }
       
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
@@ -11567,6 +11567,13 @@ class Rsa_authentication_keys
         ERR_error_string_n(ERR_get_error(), error_buf, MYSQL_ERRMSG_SIZE);
         sql_print_error("Failure to parse RSA %s key (file exists): %s:"
                         " %s", key_type, key_file_path.c_ptr(), error_buf);
+
+        /*
+          Call ERR_clear_error() just in case there are more than 1 entry in the
+          OpenSSL thread's error queue.
+        */
+        ERR_clear_error();
+
         return true;
       }
 
diff --git a/vio/viossl.c b/vio/viossl.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -106,18 +106,24 @@ static void ssl_set_sys_error(int ssl_error)
 
 
 /**
-  Indicate whether a SSL I/O operation must be retried later.
+  This function does two things:
+    - it indicates whether a SSL I/O operation must be retried later;
+    - it clears the OpenSSL error queue, thus the next OpenSSL-operation can be
+      performed even after failed OpenSSL-call.
 
   @param vio  VIO object representing a SSL connection.
   @param ret  Value returned by a SSL I/O function.
-  @param event[out] The type of I/O event to wait/retry.
+  @param event[out]             The type of I/O event to wait/retry.
+  @param ssl_errno_holder[out]  The SSL error code.
 
   @return Whether a SSL I/O operation should be deferred.
   @retval TRUE    Temporary failure, retry operation.
   @retval FALSE   Indeterminate failure.
 */
 
-static my_bool ssl_should_retry(Vio *vio, int ret, enum enum_vio_io_event *event)
+static my_bool ssl_should_retry(Vio *vio, int ret,
+                                enum enum_vio_io_event *event,
+                                unsigned long *ssl_errno_holder)
 {
   int ssl_error;
   SSL *ssl= vio->ssl_arg;
@@ -136,14 +142,22 @@ static my_bool ssl_should_retry(Vio *vio, int ret, enum enum_vio_io_event *event
     *event= VIO_IO_EVENT_WRITE;
     break;
   default:
-#ifndef DBUG_OFF
+#ifndef DBUG_OFF  /* Debug build */
+    /* Note: the OpenSSL error queue gets cleared in report_errors(). */
     report_errors(ssl);
+#else             /* Release build */
+# ifndef HAVE_YASSL
+    /* OpenSSL: clear the error queue. */
+    ERR_clear_error();
+# endif
 #endif
     should_retry= FALSE;
     ssl_set_sys_error(ssl_error);
     break;
   }
 
+  *ssl_errno_holder= ssl_error;
+
   return should_retry;
 }
 
@@ -152,14 +166,30 @@ size_t vio_ssl_read(Vio *vio, uchar *buf, size_t size)
 {
   int ret;
   SSL *ssl= vio->ssl_arg;
+  unsigned long ssl_errno_not_used;
+
   DBUG_ENTER("vio_ssl_read");
 
-  while ((ret= SSL_read(ssl, buf, size)) < 0)
+  while (1)
   {
     enum enum_vio_io_event event;
 
+#ifndef HAVE_YASSL
+    /*
+      OpenSSL: check that the SSL thread's error queue is cleared. Otherwise
+      SSL_read() returns an error from the error queue, when SSL_read() failed
+      because it would block.
+    */
+    DBUG_ASSERT(ERR_peek_error() == 0);
+#endif
+
+    ret= SSL_read(ssl, buf, size);
+
+    if (ret >= 0)
+      break;
+
     /* Process the SSL I/O error. */
-    if (!ssl_should_retry(vio, ret, &event))
+    if (!ssl_should_retry(vio, ret, &event, &ssl_errno_not_used))
       break;
 
     /* Attempt to wait for an I/O event. */
@@ -175,14 +205,30 @@ size_t vio_ssl_write(Vio *vio, const uchar *buf, size_t size)
 {
   int ret;
   SSL *ssl= vio->ssl_arg;
+  unsigned long ssl_errno_not_used;
+
   DBUG_ENTER("vio_ssl_write");
 
-  while ((ret= SSL_write(ssl, buf, size)) < 0)
+  while (1)
   {
     enum enum_vio_io_event event;
 
+#ifndef HAVE_YASSL
+    /*
+      OpenSSL: check that the SSL thread's error queue is cleared. Otherwise
+      SSL_write() returns an error from the error queue, when SSL_write() failed
+      because it would block.
+    */
+    DBUG_ASSERT(ERR_peek_error() == 0);
+#endif
+
+    ret= SSL_write(ssl, buf, size);
+
+    if (ret >= 0)
+      break;
+
     /* Process the SSL I/O error. */
-    if (!ssl_should_retry(vio, ret, &event))
+    if (!ssl_should_retry(vio, ret, &event, &ssl_errno_not_used))
       break;
 
     /* Attempt to wait for an I/O event. */
@@ -277,23 +323,40 @@ typedef int (*ssl_handshake_func_t)(SSL*);
   @param vio    VIO object representing a SSL connection.
   @param ssl    SSL structure for the connection.
   @param func   SSL handshake handler.
+  @param ssl_errno_holder[out]  The SSL error code.
 
   @return Return value is 1 on success.
 */
 
-static int ssl_handshake_loop(Vio *vio, SSL *ssl, ssl_handshake_func_t func)
+static int ssl_handshake_loop(Vio *vio, SSL *ssl,
+                              ssl_handshake_func_t func,
+                              unsigned long *ssl_errno_holder)
 {
   int ret;
 
   vio->ssl_arg= ssl;
 
   /* Initiate the SSL handshake. */
-  while ((ret= func(ssl)) < 1)
+  while (1)
   {
     enum enum_vio_io_event event;
 
+#ifndef HAVE_YASSL
+    /*
+      OpenSSL: check that the SSL thread's error queue is cleared. Otherwise
+      SSL-handshake-function returns an error from the error queue, when the
+      function failed because it would block.
+    */
+    DBUG_ASSERT(ERR_peek_error() == 0);
+#endif
+
+    ret= func(ssl);
+
+    if (ret >= 1)
+      break;
+
     /* Process the SSL I/O error. */
-    if (!ssl_should_retry(vio, ret, &event))
+    if (!ssl_should_retry(vio, ret, &event, ssl_errno_holder))
       break;
 
     /* Wait for I/O so that the handshake can proceed. */
@@ -308,7 +371,8 @@ static int ssl_handshake_loop(Vio *vio, SSL *ssl, ssl_handshake_func_t func)
 
 
 static int ssl_do(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
-                  ssl_handshake_func_t func, unsigned long *errptr)
+                  ssl_handshake_func_t func,
+                  unsigned long *ssl_errno_holder)
 {
   int r;
   SSL *ssl;
@@ -320,7 +384,7 @@ static int ssl_do(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
   if (!(ssl= SSL_new(ptr->ssl_context)))
   {
     DBUG_PRINT("error", ("SSL_new failure"));
-    *errptr= ERR_get_error();
+    *ssl_errno_holder= ERR_get_error();
     DBUG_RETURN(1);
   }
   DBUG_PRINT("info", ("ssl: 0x%lx timeout: %ld", (long) ssl, timeout));
@@ -345,10 +409,9 @@ static int ssl_do(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
   yaSSL_transport_set_send_function(ssl, yassl_send);
 #endif
 
-  if ((r= ssl_handshake_loop(vio, ssl, func)) < 1)
+  if ((r= ssl_handshake_loop(vio, ssl, func, ssl_errno_holder)) < 1)
   {
     DBUG_PRINT("error", ("SSL_connect/accept failure"));
-    *errptr= SSL_get_error(ssl, r);
     SSL_free(ssl);
     DBUG_RETURN(1);
   }
@@ -395,17 +458,19 @@ static int ssl_do(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
 }
 
 
-int sslaccept(struct st_VioSSLFd *ptr, Vio *vio, long timeout, unsigned long *errptr)
+int sslaccept(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
+              unsigned long *ssl_errno_holder)
 {
   DBUG_ENTER("sslaccept");
-  DBUG_RETURN(ssl_do(ptr, vio, timeout, SSL_accept, errptr));
+  DBUG_RETURN(ssl_do(ptr, vio, timeout, SSL_accept, ssl_errno_holder));
 }
 
 
-int sslconnect(struct st_VioSSLFd *ptr, Vio *vio, long timeout, unsigned long *errptr)
+int sslconnect(struct st_VioSSLFd *ptr, Vio *vio, long timeout,
+               unsigned long *ssl_errno_holder)
 {
   DBUG_ENTER("sslconnect");
-  DBUG_RETURN(ssl_do(ptr, vio, timeout, SSL_connect, errptr));
+  DBUG_RETURN(ssl_do(ptr, vio, timeout, SSL_connect, ssl_errno_holder));
 }
 
 
