Fix memory leak of SSL_CTX structures in TLS core.
authorMark Michelson <mmichelson@digium.com>
Fri, 18 May 2012 17:24:57 +0000 (17:24 +0000)
committerMark Michelson <mmichelson@digium.com>
Fri, 18 May 2012 17:24:57 +0000 (17:24 +0000)
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

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@367010 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/chan_sip.c
include/asterisk/tcptls.h
main/tcptls.c

index e2d2daf..a84adeb 100644 (file)
@@ -31884,6 +31884,7 @@ static int unload_module(void)
        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);
index ba6ac12..6d8d149 100644 (file)
@@ -196,9 +196,26 @@ void ast_tcptls_server_start(struct ast_tcptls_session_args *desc);
  * \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.
  */
 int ast_tls_read_conf(struct ast_tls_config *tls_cfg, struct ast_tcptls_session_args *tls_desc, const char *varname, const char *value);
index 7b17722..a96fb55 100644 (file)
@@ -131,6 +131,14 @@ HOOK_T ast_tcptls_server_write(struct ast_tcptls_session_instance *tcptls_sessio
        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
 * creates a FILE * from the fd passed by the accept thread.
 * This operation is potentially expensive (certificate verification),
@@ -279,7 +287,7 @@ void *ast_tcptls_server_root(void *data)
                        }
                        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)) {
@@ -319,6 +327,14 @@ static int __ssl_setup(struct ast_tls_config *cfg, int client)
        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
                if (ast_test_flag(&cfg->flags, AST_SSL_SSLV2_CLIENT)) {
@@ -354,6 +370,8 @@ static int __ssl_setup(struct ast_tls_config *cfg, int client)
                                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 @@ static int __ssl_setup(struct ast_tls_config *cfg, int client)
                                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 @@ static int __ssl_setup(struct ast_tls_config *cfg, int client)
                                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;
                        }
                }
@@ -393,6 +415,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)
 {
        struct ast_tcptls_session_args *desc;
@@ -471,7 +501,7 @@ struct ast_tcptls_session_instance *ast_tcptls_client_create(struct ast_tcptls_s
                }
        }
 
-       if (!(tcptls_session = ao2_alloc(sizeof(*tcptls_session), NULL))) {
+       if (!(tcptls_session = ao2_alloc(sizeof(*tcptls_session), session_instance_destructor))) {
                goto error;
        }