[svn-commits] mjordan: branch certified-1.8.11 r373088 - in /certified/branches/1.8.11: ./ ...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri Sep 14 15:25:52 CDT 2012
Author: mjordan
Date: Fri Sep 14 15:25:44 2012
New Revision: 373088
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=373088
Log:
Multiple revisions 366052,367002,367416,369731,373061
........
r366052 | mmichelson | 2012-05-10 11:10:18 -0500 (Thu, 10 May 2012) | 7 lines
Close the proper tcptls_session when session creation fails.
(issue AST-998)
Reported by: Thomas Arimont
Tested by: Thomas Arimont
........
r367002 | mmichelson | 2012-05-18 11:53:47 -0500 (Fri, 18 May 2012) | 17 lines
Fix memory leak of SSL_CTX structures in TLS core.
SSL_CTX structures were allocated but never freed. This was a bigger
issue for clients than servers since new SSL_CTX structures could be
allocated for each connection. Servers, on the other hand, typically
set up a single SSL_CTX for their lifetime.
This is solved in two ways:
1. In __ssl_setup(), if a tcptls_cfg has an ssl_ctx on it, it is
freed so that a new one can take its place.
2. A companion to ast_ssl_setup() called ast_ssl_teardown() has
been added so that servers can properly free their SSL_CTXs.
(issue ASTERISK-19278)
........
r367416 | mmichelson | 2012-05-23 15:27:47 -0500 (Wed, 23 May 2012) | 5 lines
Only call SSL_CTX_free if DO_SSL is defined.
Thanks to Paul Belanger for pointing out this error.
........
r369731 | mmichelson | 2012-07-06 13:40:06 -0500 (Fri, 06 Jul 2012) | 19 lines
Remove a superfluous and dangerous freeing of an SSL_CTX.
The problem here is that multiple server sessions share
a SSL_CTX. When one session ended, the SSL_CTX would be
freed and set NULL, leaving the other sessions unable to
function.
The code being removed is superfluous because the SSL_CTX
structures for servers will be properly freed when ast_ssl_teardown
is called.
(closes issue ASTERISK-20074)
Reported by Trevor Helmsley
Patches:
ASTERISK-20074.diff uploaded by Mark Michelson (license #5049)
Testers:
Trevor Helmsley
........
r373061 | mjordan | 2012-09-14 14:07:20 -0500 (Fri, 14 Sep 2012) | 28 lines
Resolve memory leaks in TLS initialization and TLS client connections
This patch resolves two sources of memory leaks when using TLS in Asterisk:
1) It removes improper initialization (and multiple re-initializations) of
portions of the SSL library. Asterisk calls SSL_library_init and
SSL_load_error_strings during SSL initialization; collectively this
obviates the need for calling any of the following during initialization
or client connection handling:
* ERR_load_crypto_strings (handled by SSL_load_error_strings)
* OpenSSL_add_all_algorithms (synonym for SSL_library_init)
* SSLeay_add_ssl_algorithms (synonym for SSL_library_init)
2) Failure to completely clean up all memory allocated by Asterisk and by
the SSL library for TLS clients. This included not freeing the SSL_CTX
object in the SIP channel driver, as well as not clearing the error
stack when the TLS client exited.
Note that these memory leaks were found by Thomas Arimont, and this patch
was essentially written by him with some minor tweaks.
(closes issue AST-889)
Reported by: Thomas Arimont
Tested by: Thomas Arimont
patches:
(bugAST-889.patch) by Thomas Arimont (license 5525)
Review: https://reviewboard.asterisk.org/r/2105
........
Merged revisions 366052,367002,367416,369731,373061 from http://svn.asterisk.org/svn/asterisk/branches/1.8
Modified:
certified/branches/1.8.11/ (props changed)
certified/branches/1.8.11/channels/chan_sip.c
certified/branches/1.8.11/include/asterisk/tcptls.h
certified/branches/1.8.11/main/ssl.c
certified/branches/1.8.11/main/tcptls.c
Propchange: certified/branches/1.8.11/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: certified/branches/1.8.11/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.11/channels/chan_sip.c?view=diff&rev=373088&r1=373087&r2=373088
==============================================================================
--- certified/branches/1.8.11/channels/chan_sip.c (original)
+++ certified/branches/1.8.11/channels/chan_sip.c Fri Sep 14 15:25:44 2012
@@ -2381,6 +2381,8 @@
ast_free(args->tls_cfg->cipher);
ast_free(args->tls_cfg->cafile);
ast_free(args->tls_cfg->capath);
+
+ ast_ssl_teardown(args->tls_cfg);
}
ast_free(args->tls_cfg);
ast_free((char *) args->name);
@@ -26112,7 +26114,7 @@
ao2_t_ref(ca, -1, "failed to create client, getting rid of client tcptls_session arguments");
}
if (s->tcptls_session) {
- ast_tcptls_close_session_file(tcptls_session);
+ ast_tcptls_close_session_file(s->tcptls_session);
s->fd = -1;
ao2_ref(s->tcptls_session, -1);
s->tcptls_session = NULL;
@@ -31020,6 +31022,7 @@
if (sip_tls_desc.master) {
ast_tcptls_server_stop(&sip_tls_desc);
}
+ ast_ssl_teardown(sip_tls_desc.tls_cfg);
/* Kill all existing TCP/TLS threads */
i = ao2_iterator_init(threadt, 0);
Modified: certified/branches/1.8.11/include/asterisk/tcptls.h
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.11/include/asterisk/tcptls.h?view=diff&rev=373088&r1=373087&r2=373088
==============================================================================
--- certified/branches/1.8.11/include/asterisk/tcptls.h (original)
+++ certified/branches/1.8.11/include/asterisk/tcptls.h Fri Sep 14 15:25:44 2012
@@ -190,7 +190,24 @@
* \version 1.6.1 changed desc parameter to be of ast_tcptls_session_args type
*/
void ast_tcptls_server_stop(struct ast_tcptls_session_args *desc);
+
+/*!
+ * \brief Set up an SSL server
+ *
+ * \param cfg Configuration for the SSL server
+ * \retval 1 Success
+ * \retval 0 Failure
+ */
int ast_ssl_setup(struct ast_tls_config *cfg);
+
+/*!
+ * \brief free resources used by an SSL server
+ *
+ * \note This only needs to be called if ast_ssl_setup() was
+ * directly called first.
+ * \param cfg Configuration for the SSL server
+ */
+void ast_ssl_teardown(struct ast_tls_config *cfg);
/*!
* \brief Used to parse conf files containing tls/ssl options.
Modified: certified/branches/1.8.11/main/ssl.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.11/main/ssl.c?view=diff&rev=373088&r1=373087&r2=373088
==============================================================================
--- certified/branches/1.8.11/main/ssl.c (original)
+++ certified/branches/1.8.11/main/ssl.c Fri Sep 14 15:25:44 2012
@@ -77,9 +77,7 @@
SSL_library_init();
SSL_load_error_strings();
- ERR_load_crypto_strings();
ERR_load_BIO_strings();
- OpenSSL_add_all_algorithms();
/* Make OpenSSL thread-safe. */
Modified: certified/branches/1.8.11/main/tcptls.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/1.8.11/main/tcptls.c?view=diff&rev=373088&r1=373087&r2=373088
==============================================================================
--- certified/branches/1.8.11/main/tcptls.c (original)
+++ certified/branches/1.8.11/main/tcptls.c Fri Sep 14 15:25:44 2012
@@ -78,6 +78,7 @@
{
int cookie_fd = SSL_get_fd(cookie);
int ret;
+
if (cookie_fd > -1) {
/*
* According to the TLS standard, it is acceptable for an application to only send its shutdown
@@ -87,6 +88,12 @@
if ((ret = SSL_shutdown(cookie)) < 0) {
ast_log(LOG_ERROR, "SSL_shutdown() failed: %d\n", SSL_get_error(cookie, ret));
}
+
+ if (!((SSL*)cookie)->server) {
+ /* For client threads, ensure that the error stack is cleared */
+ ERR_remove_state(0);
+ }
+
SSL_free(cookie);
/* adding shutdown(2) here has no added benefit */
if (close(cookie_fd)) {
@@ -308,8 +315,13 @@
if (!cfg->enabled)
return 0;
- SSL_load_error_strings();
- SSLeay_add_ssl_algorithms();
+ /* Get rid of an old SSL_CTX since we're about to
+ * allocate a new one
+ */
+ if (cfg->ssl_ctx) {
+ SSL_CTX_free(cfg->ssl_ctx);
+ cfg->ssl_ctx = NULL;
+ }
if (client) {
#ifndef OPENSSL_NO_SSL2
@@ -344,8 +356,9 @@
if (!client) {
/* Clients don't need a certificate, but if its setup we can use it */
ast_verb(0, "SSL error loading cert file. <%s>", cfg->certfile);
- sleep(2);
cfg->enabled = 0;
+ SSL_CTX_free(cfg->ssl_ctx);
+ cfg->ssl_ctx = NULL;
return 0;
}
}
@@ -353,8 +366,9 @@
if (!client) {
/* Clients don't need a private key, but if its setup we can use it */
ast_verb(0, "SSL error loading private key file. <%s>", tmpprivate);
- sleep(2);
cfg->enabled = 0;
+ SSL_CTX_free(cfg->ssl_ctx);
+ cfg->ssl_ctx = NULL;
return 0;
}
}
@@ -363,8 +377,9 @@
if (SSL_CTX_set_cipher_list(cfg->ssl_ctx, cfg->cipher) == 0 ) {
if (!client) {
ast_verb(0, "SSL cipher error <%s>", cfg->cipher);
- sleep(2);
cfg->enabled = 0;
+ SSL_CTX_free(cfg->ssl_ctx);
+ cfg->ssl_ctx = NULL;
return 0;
}
}
@@ -382,6 +397,16 @@
int ast_ssl_setup(struct ast_tls_config *cfg)
{
return __ssl_setup(cfg, 0);
+}
+
+void ast_ssl_teardown(struct ast_tls_config *cfg)
+{
+#ifdef DO_SSL
+ if (cfg->ssl_ctx) {
+ SSL_CTX_free(cfg->ssl_ctx);
+ cfg->ssl_ctx = NULL;
+ }
+#endif
}
struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session)
More information about the svn-commits
mailing list