[asterisk-commits] mmichelson: trunk r367010 - in /trunk: ./ channels/ include/asterisk/ main/

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Fri May 18 12:24:57 2012
New Revision: 367010

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=367010
Log:
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)
........

Merged revisions 367002 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 367003 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c
    trunk/include/asterisk/tcptls.h
    trunk/main/tcptls.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=367010&r1=367009&r2=367010
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Fri May 18 12:24:57 2012
@@ -31884,6 +31884,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: trunk/include/asterisk/tcptls.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/tcptls.h?view=diff&rev=367010&r1=367009&r2=367010
==============================================================================
--- trunk/include/asterisk/tcptls.h (original)
+++ trunk/include/asterisk/tcptls.h Fri May 18 12:24:57 2012
@@ -196,7 +196,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: trunk/main/tcptls.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/tcptls.c?view=diff&rev=367010&r1=367009&r2=367010
==============================================================================
--- trunk/main/tcptls.c (original)
+++ trunk/main/tcptls.c Fri May 18 12:24:57 2012
@@ -129,6 +129,14 @@
 	}
 #endif
 	return write(tcptls_session->fd, buf, count);
+}
+
+static void session_instance_destructor(void *obj)
+{
+	struct ast_tcptls_session_instance *i = obj;
+	if (i->parent && i->parent->tls_cfg) {
+		ast_ssl_teardown(i->parent->tls_cfg);
+	}
 }
 
 /*! \brief
@@ -279,7 +287,7 @@
 			}
 			continue;
 		}
-		tcptls_session = ao2_alloc(sizeof(*tcptls_session), NULL);
+		tcptls_session = ao2_alloc(sizeof(*tcptls_session), session_instance_destructor);
 		if (!tcptls_session) {
 			ast_log(LOG_WARNING, "No memory for new session: %s\n", strerror(errno));
 			if (close(fd)) {
@@ -318,6 +326,14 @@
 
 	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
@@ -354,6 +370,8 @@
 				ast_verb(0, "SSL error loading cert file. <%s>\n", cfg->certfile);
 				sleep(2);
 				cfg->enabled = 0;
+				SSL_CTX_free(cfg->ssl_ctx);
+				cfg->ssl_ctx = NULL;
 				return 0;
 			}
 		}
@@ -363,6 +381,8 @@
 				ast_verb(0, "SSL error loading private key file. <%s>\n", tmpprivate);
 				sleep(2);
 				cfg->enabled = 0;
+				SSL_CTX_free(cfg->ssl_ctx);
+				cfg->ssl_ctx = NULL;
 				return 0;
 			}
 		}
@@ -373,6 +393,8 @@
 				ast_verb(0, "SSL cipher error <%s>\n", cfg->cipher);
 				sleep(2);
 				cfg->enabled = 0;
+				SSL_CTX_free(cfg->ssl_ctx);
+				cfg->ssl_ctx = NULL;
 				return 0;
 			}
 		}
@@ -391,6 +413,14 @@
 int ast_ssl_setup(struct ast_tls_config *cfg)
 {
 	return __ssl_setup(cfg, 0);
+}
+
+void ast_ssl_teardown(struct ast_tls_config *cfg)
+{
+	if (cfg->ssl_ctx) {
+		SSL_CTX_free(cfg->ssl_ctx);
+		cfg->ssl_ctx = NULL;
+	}
 }
 
 struct ast_tcptls_session_instance *ast_tcptls_client_start(struct ast_tcptls_session_instance *tcptls_session)
@@ -471,7 +501,7 @@
 		}
 	}
 
-	if (!(tcptls_session = ao2_alloc(sizeof(*tcptls_session), NULL))) {
+	if (!(tcptls_session = ao2_alloc(sizeof(*tcptls_session), session_instance_destructor))) {
 		goto error;
 	}