a pjproject change that caused a memory leak with TLS in chan_pjsip. Found by
Mark Patruck and diagnosed by him with upstream.
This commit is contained in:
sthen 2019-09-25 11:39:15 +00:00
parent 7b36b4ded8
commit 8242e664b6
5 changed files with 1492 additions and 1 deletions

View File

@ -1,4 +1,4 @@
# $OpenBSD: Makefile,v 1.305 2019/09/05 20:06:34 sthen Exp $
# $OpenBSD: Makefile,v 1.306 2019/09/25 11:39:15 sthen Exp $
COMMENT-main= open source multi-protocol PBX and telephony toolkit
@ -7,6 +7,7 @@ PJ_V= 2.9
DISTNAME= asterisk-${VER:S/beta/-beta/:S/rc/-rc/}
DISTFILES= ${DISTNAME}${EXTRACT_SUFX} pjproject-${PJ_V}.tar.bz2:0
PKGNAME-main= asterisk-${VER}
REVISION-main= 0
.if ${MACHINE_ARCH} == i386
# i386 issue linking libasteriskpj.so.0.0:

View File

@ -0,0 +1,107 @@
$OpenBSD: patch-third-party_pjproject_patches_0030-Revert-Misc-re-2147-Fixed-warnings-in-SSL-socket-red_patch,v 1.1 2019/09/25 11:39:15 sthen Exp $
From 2b75c4fc514c3a6e2fcb5d89a47ea552764d5992 Mon Sep 17 00:00:00 2001
From: George Joseph <gjoseph@digium.com>
Date: Thu, 19 Sep 2019 08:50:07 -0600
Subject: [PATCH] pjproject_bundled: Revert pjproject 2.9 commits causing leaks
We've found a connection re-use regression in pjproject 2.9
introduced by commit
"Close #1019: Support for multiple listeners."
https://trac.pjsip.org/repos/changeset/6002
https://trac.pjsip.org/repos/ticket/1019
Normally, multiple SSL requests should reuse the same connection
if one already exists to the remote server. When a transport
error occurs, the next request should establish a new connection
and any following requests should use that same one. With this
patch, when a transport error occurs, every new request creates
a new connection so you can wind up with thousands of open tcp
sockets, possibly exhausting file handles, and increasing memory
usage.
Reverting pjproject commit 6002 (and related 6021) restores the
expected behavior.
We also found a memory leak in SSL processing that was introduced by
commit
"Fixed #2204: Add OpenSSL remote certificate chain info"
https://trac.pjsip.org/repos/changeset/6014
https://trac.pjsip.org/repos/ticket/2204
Apparently the remote certificate chain is continually recreated
causing the leak.
Reverting pjproject commit 6014 (and related 6022) restores the
expected behavior.
Both of these issues have been acknowledged by Teluu.
ASTERISK-28521
Change-Id: I8ae7233c3ac4ec29a3b991f738e655dabcaba9f1
Index: third-party/pjproject/patches/0030-Revert-Misc-re-2147-Fixed-warnings-in-SSL-socket-red.patch
--- third-party/pjproject/patches/0030-Revert-Misc-re-2147-Fixed-warnings-in-SSL-socket-red.patch.orig
+++ third-party/pjproject/patches/0030-Revert-Misc-re-2147-Fixed-warnings-in-SSL-socket-red.patch
@@ -0,0 +1,60 @@
+From 8d0652d4a02c7b8da58b1b98421cfda57056184d Mon Sep 17 00:00:00 2001
+From: George Joseph <gjoseph@digium.com>
+Date: Tue, 24 Sep 2019 06:41:16 -0600
+Subject: [PATCH 30/33] Revert "Misc (re #2147): Fixed warnings in SSL socket:
+ redefinition of typedef 'pj_ssl_sock_t' and unused 'get_pem'."
+
+This reverts commit 688a9b0de685328f62b2df86304b44c21e4460ae.
+---
+ pjlib/src/pj/ssl_sock_imp_common.h | 4 ++--
+ pjlib/src/pj/ssl_sock_ossl.c | 5 +----
+ 2 files changed, 3 insertions(+), 6 deletions(-)
+
+diff --git a/pjlib/src/pj/ssl_sock_imp_common.h b/pjlib/src/pj/ssl_sock_imp_common.h
+index 09f259ef7..4edbb3b82 100644
+--- a/pjlib/src/pj/ssl_sock_imp_common.h
++++ b/pjlib/src/pj/ssl_sock_imp_common.h
+@@ -93,7 +93,7 @@ typedef struct circ_buf_t {
+ /*
+ * Secure socket structure definition.
+ */
+-struct pj_ssl_sock_t
++typedef struct pj_ssl_sock_t
+ {
+ pj_pool_t *pool;
+ pj_ssl_sock_t *parent;
+@@ -139,7 +139,7 @@ struct pj_ssl_sock_t
+
+ circ_buf_t circ_buf_output;
+ pj_lock_t *circ_buf_output_mutex;
+-};
++} pj_ssl_sock_t;
+
+
+ /*
+diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
+index b4ac5c15f..debb105b1 100644
+--- a/pjlib/src/pj/ssl_sock_ossl.c
++++ b/pjlib/src/pj/ssl_sock_ossl.c
+@@ -37,6 +37,7 @@
+ #if defined(PJ_HAS_SSL_SOCK) && PJ_HAS_SSL_SOCK != 0 && \
+ (PJ_SSL_SOCK_IMP == PJ_SSL_SOCK_IMP_OPENSSL)
+
++#include "ssl_sock_imp_common.h"
+ #include "ssl_sock_imp_common.c"
+
+ #define THIS_FILE "ssl_sock_ossl.c"
+@@ -1575,10 +1576,6 @@ static void ssl_update_remote_cert_chain_info(pj_pool_t *pool,
+ {
+ int i;
+
+- /* For now, get_pem has to be PJ_TRUE */
+- pj_assert(get_pem);
+- PJ_UNUSED_ARG(get_pem);
+-
+ ci->raw_chain.cert_raw = (pj_str_t *)pj_pool_calloc(pool,
+ sk_X509_num(chain),
+ sizeof(pj_str_t));
+--
+2.21.0
+

View File

@ -0,0 +1,131 @@
$OpenBSD: patch-third-party_pjproject_patches_0031-Revert-Fixed-2204-Add-OpenSSL-remote-certificate-cha_patch,v 1.1 2019/09/25 11:39:15 sthen Exp $
From 2b75c4fc514c3a6e2fcb5d89a47ea552764d5992 Mon Sep 17 00:00:00 2001
From: George Joseph <gjoseph@digium.com>
Date: Thu, 19 Sep 2019 08:50:07 -0600
Subject: [PATCH] pjproject_bundled: Revert pjproject 2.9 commits causing leaks
We've found a connection re-use regression in pjproject 2.9
introduced by commit
"Close #1019: Support for multiple listeners."
https://trac.pjsip.org/repos/changeset/6002
https://trac.pjsip.org/repos/ticket/1019
Normally, multiple SSL requests should reuse the same connection
if one already exists to the remote server. When a transport
error occurs, the next request should establish a new connection
and any following requests should use that same one. With this
patch, when a transport error occurs, every new request creates
a new connection so you can wind up with thousands of open tcp
sockets, possibly exhausting file handles, and increasing memory
usage.
Reverting pjproject commit 6002 (and related 6021) restores the
expected behavior.
We also found a memory leak in SSL processing that was introduced by
commit
"Fixed #2204: Add OpenSSL remote certificate chain info"
https://trac.pjsip.org/repos/changeset/6014
https://trac.pjsip.org/repos/ticket/2204
Apparently the remote certificate chain is continually recreated
causing the leak.
Reverting pjproject commit 6014 (and related 6022) restores the
expected behavior.
Both of these issues have been acknowledged by Teluu.
ASTERISK-28521
Change-Id: I8ae7233c3ac4ec29a3b991f738e655dabcaba9f1
Index: third-party/pjproject/patches/0031-Revert-Fixed-2204-Add-OpenSSL-remote-certificate-cha.patch
--- third-party/pjproject/patches/0031-Revert-Fixed-2204-Add-OpenSSL-remote-certificate-cha.patch.orig
+++ third-party/pjproject/patches/0031-Revert-Fixed-2204-Add-OpenSSL-remote-certificate-cha.patch
@@ -0,0 +1,84 @@
+From 616a13933f33a6d74f84d85b5bfb858279a09e2d Mon Sep 17 00:00:00 2001
+From: George Joseph <gjoseph@digium.com>
+Date: Tue, 24 Sep 2019 06:42:04 -0600
+Subject: [PATCH 31/33] Revert "Fixed #2204: Add OpenSSL remote certificate
+ chain info"
+
+This reverts commit f71d60c866c4572a7c8398fe982416771fc6a7f5.
+---
+ pjlib/src/pj/ssl_sock_ossl.c | 45 ------------------------------------
+ 1 file changed, 45 deletions(-)
+
+diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
+index debb105b1..109c5c1e2 100644
+--- a/pjlib/src/pj/ssl_sock_ossl.c
++++ b/pjlib/src/pj/ssl_sock_ossl.c
+@@ -1566,41 +1566,6 @@ static void get_cert_info(pj_pool_t *pool, pj_ssl_cert_info *ci, X509 *x,
+ }
+ }
+
+-/* Update remote certificates chain info. This function should be
+- * called after handshake or renegotiation successfully completed.
+- */
+-static void ssl_update_remote_cert_chain_info(pj_pool_t *pool,
+- pj_ssl_cert_info *ci,
+- STACK_OF(X509) *chain,
+- pj_bool_t get_pem)
+-{
+- int i;
+-
+- ci->raw_chain.cert_raw = (pj_str_t *)pj_pool_calloc(pool,
+- sk_X509_num(chain),
+- sizeof(pj_str_t));
+- ci->raw_chain.cnt = sk_X509_num(chain);
+-
+- for (i = 0; i < sk_X509_num(chain); i++) {
+- BIO *bio;
+- BUF_MEM *ptr;
+- X509 *x = sk_X509_value(chain, i);
+-
+- bio = BIO_new(BIO_s_mem());
+-
+- if (!PEM_write_bio_X509(bio, x)) {
+- PJ_LOG(3, (THIS_FILE, "Error retrieving raw certificate info"));
+- ci->raw_chain.cert_raw[i].ptr = NULL;
+- ci->raw_chain.cert_raw[i].slen = 0;
+- } else {
+- BIO_write(bio, "\0", 1);
+- BIO_get_mem_ptr(bio, &ptr);
+- pj_strdup2(pool, &ci->raw_chain.cert_raw[i], ptr->data );
+- }
+-
+- BIO_free(bio);
+- }
+-}
+
+ /* Update local & remote certificates info. This function should be
+ * called after handshake or renegotiation successfully completed.
+@@ -1609,7 +1574,6 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock)
+ {
+ ossl_sock_t *ossock = (ossl_sock_t *)ssock;
+ X509 *x;
+- STACK_OF(X509) *chain;
+
+ pj_assert(ssock->ssl_state == SSL_STATE_ESTABLISHED);
+
+@@ -1631,15 +1595,6 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock)
+ } else {
+ pj_bzero(&ssock->remote_cert_info, sizeof(pj_ssl_cert_info));
+ }
+-
+- chain = SSL_get_peer_cert_chain(ossock->ossl_ssl);
+- if (chain) {
+- ssl_update_remote_cert_chain_info(ssock->pool,
+- &ssock->remote_cert_info,
+- chain, PJ_TRUE);
+- } else {
+- ssock->remote_cert_info.raw_chain.cnt = 0;
+- }
+ }
+
+
+--
+2.21.0
+

View File

@ -0,0 +1,111 @@
$OpenBSD: patch-third-party_pjproject_patches_0032-Revert-Re-2147-misc-Fix-failed-pjsip-test-transport__patch,v 1.1 2019/09/25 11:39:15 sthen Exp $
From 2b75c4fc514c3a6e2fcb5d89a47ea552764d5992 Mon Sep 17 00:00:00 2001
From: George Joseph <gjoseph@digium.com>
Date: Thu, 19 Sep 2019 08:50:07 -0600
Subject: [PATCH] pjproject_bundled: Revert pjproject 2.9 commits causing leaks
We've found a connection re-use regression in pjproject 2.9
introduced by commit
"Close #1019: Support for multiple listeners."
https://trac.pjsip.org/repos/changeset/6002
https://trac.pjsip.org/repos/ticket/1019
Normally, multiple SSL requests should reuse the same connection
if one already exists to the remote server. When a transport
error occurs, the next request should establish a new connection
and any following requests should use that same one. With this
patch, when a transport error occurs, every new request creates
a new connection so you can wind up with thousands of open tcp
sockets, possibly exhausting file handles, and increasing memory
usage.
Reverting pjproject commit 6002 (and related 6021) restores the
expected behavior.
We also found a memory leak in SSL processing that was introduced by
commit
"Fixed #2204: Add OpenSSL remote certificate chain info"
https://trac.pjsip.org/repos/changeset/6014
https://trac.pjsip.org/repos/ticket/2204
Apparently the remote certificate chain is continually recreated
causing the leak.
Reverting pjproject commit 6014 (and related 6022) restores the
expected behavior.
Both of these issues have been acknowledged by Teluu.
ASTERISK-28521
Change-Id: I8ae7233c3ac4ec29a3b991f738e655dabcaba9f1
Index: third-party/pjproject/patches/0032-Revert-Re-2147-misc-Fix-failed-pjsip-test-transport_.patch
--- third-party/pjproject/patches/0032-Revert-Re-2147-misc-Fix-failed-pjsip-test-transport_.patch.orig
+++ third-party/pjproject/patches/0032-Revert-Re-2147-misc-Fix-failed-pjsip-test-transport_.patch
@@ -0,0 +1,64 @@
+From 17cd744e19cd332a219a512770fa6e18453044ba Mon Sep 17 00:00:00 2001
+From: George Joseph <gjoseph@digium.com>
+Date: Tue, 24 Sep 2019 06:45:25 -0600
+Subject: [PATCH 32/33] Revert "Re #2147 (misc): Fix failed pjsip-test
+ (transport_loop_test) caused by r6002."
+
+This reverts commit 342148f5bcf3a6b0029ce834b8567c2cd691b15b.
+---
+ pjsip/src/pjsip/sip_transport.c | 12 +++++-------
+ pjsip/src/pjsip/sip_transport_loop.c | 2 +-
+ pjsip/src/test/transport_loop_test.c | 1 -
+ 3 files changed, 6 insertions(+), 9 deletions(-)
+
+diff --git a/pjsip/src/pjsip/sip_transport.c b/pjsip/src/pjsip/sip_transport.c
+index 65ac823d4..d63823a98 100644
+--- a/pjsip/src/pjsip/sip_transport.c
++++ b/pjsip/src/pjsip/sip_transport.c
+@@ -1222,13 +1222,11 @@ PJ_DEF(pj_status_t) pjsip_transport_register( pjsip_tpmgr *mgr,
+
+ pj_lock_release(mgr->lock);
+
+- TRACE_((THIS_FILE, "Transport %s registered: type=%s, remote=%s:%d",
+- tp->obj_name,
+- pjsip_transport_get_type_name(tp->key.type),
+- pj_sockaddr_has_addr(&tp->key.rem_addr)?
+- addr_string(&tp->key.rem_addr):"",
+- pj_sockaddr_has_addr(&tp->key.rem_addr)?
+- pj_sockaddr_get_port(&tp->key.rem_addr):0));
++ TRACE_((THIS_FILE,"Transport %s registered: type=%s, remote=%s:%d",
++ tp->obj_name,
++ pjsip_transport_get_type_name(tp->key.type),
++ addr_string(&tp->key.rem_addr),
++ pj_sockaddr_get_port(&tp->key.rem_addr)));
+
+ return PJ_SUCCESS;
+ }
+diff --git a/pjsip/src/pjsip/sip_transport_loop.c b/pjsip/src/pjsip/sip_transport_loop.c
+index 37e20e69b..24e1a5f69 100644
+--- a/pjsip/src/pjsip/sip_transport_loop.c
++++ b/pjsip/src/pjsip/sip_transport_loop.c
+@@ -376,7 +376,7 @@ PJ_DEF(pj_status_t) pjsip_loop_start( pjsip_endpoint *endpt,
+ if (status != PJ_SUCCESS)
+ goto on_error;
+ loop->base.key.type = PJSIP_TRANSPORT_LOOP_DGRAM;
+- //loop->base.key.rem_addr.addr.sa_family = pj_AF_INET();
++ loop->base.key.rem_addr.addr.sa_family = pj_AF_INET();
+ loop->base.type_name = "LOOP-DGRAM";
+ loop->base.info = "LOOP-DGRAM";
+ loop->base.flag = PJSIP_TRANSPORT_DATAGRAM;
+diff --git a/pjsip/src/test/transport_loop_test.c b/pjsip/src/test/transport_loop_test.c
+index 5f2f03904..efa2ea116 100644
+--- a/pjsip/src/test/transport_loop_test.c
++++ b/pjsip/src/test/transport_loop_test.c
+@@ -36,7 +36,6 @@ static int datagram_loop_test()
+
+ PJ_LOG(3,(THIS_FILE, "testing datagram loop transport"));
+
+- pj_sockaddr_in_init(&addr, NULL, 0);
+ /* Test acquire transport. */
+ status = pjsip_endpt_acquire_transport( endpt, PJSIP_TRANSPORT_LOOP_DGRAM,
+ &addr, sizeof(addr), NULL, &loop);
+--
+2.21.0
+