Commit 7c4eb8c0 authored by Davi Arnaut's avatar Davi Arnaut

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.
parent 73481404
...@@ -202,4 +202,10 @@ Ssl_cipher RC4-SHA ...@@ -202,4 +202,10 @@ Ssl_cipher RC4-SHA
select 'is still running; no cipher request crashed the server' as result from dual; select 'is still running; no cipher request crashed the server' as result from dual;
result result
is still running; no cipher request crashed the server 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 End of 5.1 tests
...@@ -238,7 +238,18 @@ DROP TABLE t1; ...@@ -238,7 +238,18 @@ DROP TABLE t1;
--enable_query_log --enable_query_log
select 'is still running; no cipher request crashed the server' as result from dual; 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 --echo End of 5.1 tests
# Wait till we reached the initial number of concurrent sessions # Wait till we reached the initial number of concurrent sessions
......
...@@ -936,6 +936,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, ...@@ -936,6 +936,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh,
#ifdef HAVE_OPENSSL #ifdef HAVE_OPENSSL
Vio *vio=thd->net.vio; Vio *vio=thd->net.vio;
SSL *ssl= (SSL*) vio->ssl_arg; SSL *ssl= (SSL*) vio->ssl_arg;
X509 *cert;
#endif #endif
/* /*
...@@ -964,8 +965,11 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, ...@@ -964,8 +965,11 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh,
*/ */
if (vio_type(vio) == VIO_TYPE_SSL && if (vio_type(vio) == VIO_TYPE_SSL &&
SSL_get_verify_result(ssl) == X509_V_OK && SSL_get_verify_result(ssl) == X509_V_OK &&
SSL_get_peer_certificate(ssl)) (cert= SSL_get_peer_certificate(ssl)))
{
user_access= acl_user->access; user_access= acl_user->access;
X509_free(cert);
}
break; break;
case SSL_TYPE_SPECIFIED: /* Client should have specified attrib */ case SSL_TYPE_SPECIFIED: /* Client should have specified attrib */
/* /*
...@@ -974,7 +978,6 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, ...@@ -974,7 +978,6 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh,
If cipher name is specified, we compare it to actual cipher in If cipher name is specified, we compare it to actual cipher in
use. use.
*/ */
X509 *cert;
if (vio_type(vio) != VIO_TYPE_SSL || if (vio_type(vio) != VIO_TYPE_SSL ||
SSL_get_verify_result(ssl) != X509_V_OK) SSL_get_verify_result(ssl) != X509_V_OK)
break; break;
...@@ -1014,6 +1017,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, ...@@ -1014,6 +1017,7 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh,
sql_print_information("X509 issuer mismatch: should be '%s' " sql_print_information("X509 issuer mismatch: should be '%s' "
"but is '%s'", acl_user->x509_issuer, ptr); "but is '%s'", acl_user->x509_issuer, ptr);
free(ptr); free(ptr);
X509_free(cert);
user_access=NO_ACCESS; user_access=NO_ACCESS;
break; break;
} }
...@@ -1033,12 +1037,15 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh, ...@@ -1033,12 +1037,15 @@ int acl_getroot(THD *thd, USER_RESOURCES *mqh,
sql_print_information("X509 subject mismatch: should be '%s' but is '%s'", sql_print_information("X509 subject mismatch: should be '%s' but is '%s'",
acl_user->x509_subject, ptr); acl_user->x509_subject, ptr);
free(ptr); free(ptr);
X509_free(cert);
user_access=NO_ACCESS; user_access=NO_ACCESS;
break; break;
} }
user_access= acl_user->access; user_access= acl_user->access;
free(ptr); free(ptr);
} }
/* Deallocate the X509 certificate. */
X509_free(cert);
break; break;
#else /* HAVE_OPENSSL */ #else /* HAVE_OPENSSL */
default: default:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment