Commit 981ff94d 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.
parent 8fb82e3f
...@@ -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