bzr commit into mysql-5.1-bugteam branch (davi:2909) Bug#42158

[Davi Arnaut]

# At a local mysql-5.1-bugteam repository of davi

 2909 Davi Arnaut	2009-05-25
      Bug#42158: leak: SSL_get_peer_certificate() doesn't have matching X509_free()
      
      The problem is that the server failed to follow the rule that
      every X509 object retrieved using SSL_get_peer_certificate()
      must be explicitly freed by X509_free(). This caused a memory
      leak for builds linked against OpenSSL where the X509 object
      is reference counted -- improper counting will prevent the
      object from being destroyed once the session containing the
      peer certificate is freed.
      
      The solution is to explicitly free every X509 object used.
     @ mysql-test/r/openssl_1.result
        Add test case result for Bug#42158
     @ mysql-test/t/openssl_1.test
        Add test case for Bug#42158
     @ sql/sql_acl.cc
        Deallocate X509 objects.

    modified:
      mysql-test/r/openssl_1.result
      mysql-test/t/openssl_1.test
      sql/sql_acl.cc
=== modified file 'mysql-test/r/openssl_1.result'
--- a/mysql-test/r/openssl_1.result	2009-04-11 09:46:14 +0000
+++ b/mysql-test/r/openssl_1.result	2009-05-25 13:00:18 +0000
@@ -202,4 +202,10 @@ Ssl_cipher	RC4-SHA
 select 'is still running; no cipher request crashed the server' as result from dual;
 result
 is still running; no cipher request crashed the server
+GRANT SELECT ON test.* TO bug42158@localhost REQUIRE X509;
+FLUSH PRIVILEGES;
+SHOW STATUS LIKE 'Ssl_cipher';
+Variable_name	Value
+Ssl_cipher	DHE-RSA-AES256-SHA
+DROP USER bug42158@localhost;
 End of 5.1 tests

=== modified file 'mysql-test/t/openssl_1.test'
--- a/mysql-test/t/openssl_1.test	2009-04-11 09:46:14 +0000
+++ b/mysql-test/t/openssl_1.test	2009-05-25 13:00:18 +0000
@@ -238,7 +238,18 @@ DROP TABLE t1;
 --enable_query_log
 select 'is still running; no cipher request crashed the server' as result from dual;
 
-##
+#
+# Bug#42158: leak: SSL_get_peer_certificate() doesn't have matching X509_free()
+#
+
+GRANT SELECT ON test.* TO bug42158@localhost REQUIRE X509;
+FLUSH PRIVILEGES;
+connect(con1,localhost,bug42158,,,,,SSL);
+SHOW STATUS LIKE 'Ssl_cipher';
+disconnect con1;
+connection default;
+DROP USER bug42158@localhost;
+
 --echo End of 5.1 tests
 
 # Wait till we reached the initial number of concurrent sessions

=== modified file 'sql/sql_acl.cc'
--- a/sql/sql_acl.cc	2009-04-08 23:42:51 +0000
+++ b/sql/sql_acl.cc	2009-05-25 13:00:18 +0000
@@ -936,6 +936,7 @@ int acl_getroot(THD *thd, USER_RESOURCES
 #ifdef HAVE_OPENSSL
     Vio *vio=thd->net.vio;
     SSL *ssl= (SSL*) vio->ssl_arg;
+    X509 *cert;
 #endif
 
     /*
@@ -964,8 +965,11 @@ int acl_getroot(THD *thd, USER_RESOURCES
       */
       if (vio_type(vio) == VIO_TYPE_SSL &&
 	  SSL_get_verify_result(ssl) == X509_V_OK &&
-	  SSL_get_peer_certificate(ssl))
+	  (cert= SSL_get_peer_certificate(ssl)))
+      {
 	user_access= acl_user->access;
+        X509_free(cert);
+      }
       break;
     case SSL_TYPE_SPECIFIED: /* Client should have specified attrib */
       /*
@@ -974,7 +978,6 @@ int acl_getroot(THD *thd, USER_RESOURCES
 	If cipher name is specified, we compare it to actual cipher in
 	use.
       */
-      X509 *cert;
       if (vio_type(vio) != VIO_TYPE_SSL ||
 	  SSL_get_verify_result(ssl) != X509_V_OK)
 	break;
@@ -1014,6 +1017,7 @@ int acl_getroot(THD *thd, USER_RESOURCES
             sql_print_information("X509 issuer mismatch: should be '%s' "
 			      "but is '%s'", acl_user->x509_issuer, ptr);
           free(ptr);
+          X509_free(cert);
           user_access=NO_ACCESS;
           break;
         }
@@ -1033,12 +1037,15 @@ int acl_getroot(THD *thd, USER_RESOURCES
             sql_print_information("X509 subject mismatch: should be '%s' but is '%s'",
                             acl_user->x509_subject, ptr);
           free(ptr);
+          X509_free(cert);
           user_access=NO_ACCESS;
           break;
         }
         user_access= acl_user->access;
         free(ptr);
       }
+      /* Deallocate the X509 certificate. */
+      X509_free(cert);
       break;
 #else  /* HAVE_OPENSSL */
     default:


Attachment: [text/bzr-bundle] bzr/[e-mail]