186526/openresty
1
0
Fork 0
mirror of https://github.com/openresty/openresty.git synced 2024-10-13 00:29:41 +00:00
Code Issues Projects Releases Wiki Activity

bugfix: fixed a memory leak in the OpenSSL 1.1.1 sess_set_get_cb_yield patch.

Browse Source 
This memory leak was found by running the Valgrind testing mode against
lua-resty-core's `ssl-session-fetch.t` test suite:

    TEST 5: yield during doing handshake with client which uses low version OpenSSL

    ==16956== 64 (32 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 15
    ==16956== at 0x4C2B002: malloc (vg_replace_malloc.c:298)
    ==16956== by 0x5FFC868: CRYPTO_malloc (mem.c:222)
    ==16956== by 0x5FFC96F: CRYPTO_zalloc (mem.c:230)
    ==16956== by 0x603C54A: OPENSSL_sk_new_reserve (stack.c:209)
    ==16956== by 0x603C597: OPENSSL_sk_new_null (stack.c:118)
    ==16956== by 0x5C94A86: sk_SSL_CIPHER_new_null (ssl.h:960)
    ==16956== by 0x5C94A86: bytes_to_cipher_list (ssl_lib.c:5361)
    ==16956== by 0x5CB52E9: tls_early_post_process_client_hello (statem_srvr.c:1713)
    ==16956== by 0x5CB52E9: tls_post_process_client_hello (statem_srvr.c:2231)
    ==16956== by 0x5CB6F39: ossl_statem_server_post_process_message (statem_srvr.c:1218)
    ==16956== by 0x5CA4C11: read_state_machine (statem.c:664)
    ==16956== by 0x5CA4C11: state_machine (statem.c:434)
    ==16956== by 0x5CA538A: ossl_statem_accept (statem.c:255)
    ==16956== by 0x5C91759: SSL_do_handshake (ssl_lib.c:3609)
    ==16956== by 0x45456B: ngx_ssl_handshake (ngx_event_openssl.c:1606)
    ==16956== by 0x4698D3: ngx_http_ssl_handshake (ngx_http_request.c:751)
    ==16956== by 0x44ECA8: ngx_epoll_process_events (ngx_epoll_module.c:901)
    ==16956== by 0x443E94: ngx_process_events_and_timers (ngx_event.c:257)
    ==16956== by 0x44DC25: ngx_single_process_cycle (ngx_process_cycle.c:333)
    ==16956== by 0x4236AB: main (nginx.c:382)
    ==16956==
    {
    <insert_a_suppression_name_here>
    Memcheck:Leak
    match-leak-kinds: definite
    fun:malloc
    fun:CRYPTO_malloc
    fun:CRYPTO_zalloc
    fun:OPENSSL_sk_new_reserve
    fun:OPENSSL_sk_new_null
    fun:sk_SSL_CIPHER_new_null
    fun:bytes_to_cipher_list
    fun:tls_early_post_process_client_hello
    fun:tls_post_process_client_hello
    fun:ossl_statem_server_post_process_message
    fun:read_state_machine
    fun:state_machine
    fun:ossl_statem_accept
    fun:SSL_do_handshake
    fun:ngx_ssl_handshake
    fun:ngx_http_ssl_handshake
    fun:ngx_epoll_process_events
    fun:ngx_process_events_and_timers
    fun:ngx_single_process_cycle
    fun:main
    }

    ==16956== 368 (32 direct, 336 indirect) bytes in 1 blocks are definitely lost in loss record 8 of 15
    ==16956== at 0x4C2B002: malloc (vg_replace_malloc.c:298)
    ==16956== by 0x5FFC868: CRYPTO_malloc (mem.c:222)
    ==16956== by 0x5FFC96F: CRYPTO_zalloc (mem.c:230)
    ==16956== by 0x603C54A: OPENSSL_sk_new_reserve (stack.c:209)
    ==16956== by 0x603C597: OPENSSL_sk_new_null (stack.c:118)
    ==16956== by 0x5C94A79: sk_SSL_CIPHER_new_null (ssl.h:960)
    ==16956== by 0x5C94A79: bytes_to_cipher_list (ssl_lib.c:5360)
    ==16956== by 0x5CB52E9: tls_early_post_process_client_hello (statem_srvr.c:1713)
    ==16956== by 0x5CB52E9: tls_post_process_client_hello (statem_srvr.c:2231)
    ==16956== by 0x5CB6F39: ossl_statem_server_post_process_message (statem_srvr.c:1218)
    ==16956== by 0x5CA4C11: read_state_machine (statem.c:664)
    ==16956== by 0x5CA4C11: state_machine (statem.c:434)
    ==16956== by 0x5CA538A: ossl_statem_accept (statem.c:255)
    ==16956== by 0x5C91759: SSL_do_handshake (ssl_lib.c:3609)
    ==16956== by 0x45456B: ngx_ssl_handshake (ngx_event_openssl.c:1606)
    ==16956== by 0x4698D3: ngx_http_ssl_handshake (ngx_http_request.c:751)
    ==16956== by 0x44ECA8: ngx_epoll_process_events (ngx_epoll_module.c:901)
    ==16956== by 0x443E94: ngx_process_events_and_timers (ngx_event.c:257)
    ==16956== by 0x44DC25: ngx_single_process_cycle (ngx_process_cycle.c:333)
    ==16956== by 0x4236AB: main (nginx.c:382)
    ==16956==
    {
    <insert_a_suppression_name_here>
    Memcheck:Leak
    match-leak-kinds: definite
    fun:malloc
    fun:CRYPTO_malloc
    fun:CRYPTO_zalloc
    fun:OPENSSL_sk_new_reserve
    fun:OPENSSL_sk_new_null
    fun:sk_SSL_CIPHER_new_null
    fun:bytes_to_cipher_list
    fun:tls_early_post_process_client_hello
    fun:tls_post_process_client_hello
    fun:ossl_statem_server_post_process_message
    fun:read_state_machine
    fun:state_machine
    fun:ossl_statem_accept
    fun:SSL_do_handshake
    fun:ngx_ssl_handshake
    fun:ngx_http_ssl_handshake
    fun:ngx_epoll_process_events
    fun:ngx_process_events_and_timers
    fun:ngx_single_process_cycle
    fun:main
    }
This commit is contained in:
Thibault Charbonnier
2020-02-04 12:39:29 -08:00
committed by Thibault Charbonnier
parent d856e2bf4b
commit c1a0a9ad8f
1 changed files with 16 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
patches/openssl-1.1.1c-sess_set_get_cb_yield.patch
View File

@ -142,7 +142,7 @@ diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
--- a/ssl/statem/statem_srvr.c --- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c
@@ -1600,6 +1600,7 @@ static int tls_early_post_process_client_hello(SSL *s) @@ -1604,6 +1604,7 @@ static int tls_early_post_process_client_hello(SSL *s)
STACK_OF(SSL_CIPHER) *scsvs = NULL; STACK_OF(SSL_CIPHER) *scsvs = NULL;
CLIENTHELLO_MSG *clienthello = s->clienthello; CLIENTHELLO_MSG *clienthello = s->clienthello;
DOWNGRADE dgrd = DOWNGRADE_NONE; DOWNGRADE dgrd = DOWNGRADE_NONE;
@ -150,7 +150,7 @@ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
/* Finished parsing the ClientHello, now we can start processing it */ /* Finished parsing the ClientHello, now we can start processing it */
/* Give the ClientHello callback a crack at things */ /* Give the ClientHello callback a crack at things */
@@ -1707,6 +1708,7 @@ static int tls_early_post_process_client_hello(SSL *s) @@ -1711,6 +1712,7 @@ static int tls_early_post_process_client_hello(SSL *s)
} }
s->hit = 0; s->hit = 0;
@ -158,18 +158,18 @@ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites, if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites,
clienthello->isv2) || clienthello->isv2) ||
@@ -1812,6 +1814,10 @@ static int tls_early_post_process_client_hello(SSL *s) @@ -1816,6 +1818,10 @@ static int tls_early_post_process_client_hello(SSL *s)
} else if (i == -1) { } else if (i == -1) {
/* SSLfatal() already called */ /* SSLfatal() already called */
goto err; goto err;
+ } else if (i == -2) { + } else if (i == -2) {
+ clienthello->ciphersuites = saved_ciphers; + clienthello->ciphersuites = saved_ciphers;
+ s->rwstate = SSL_SESS_LOOKUP; + s->rwstate = SSL_SESS_LOOKUP;
+ return -1; + goto retry;
} else { } else {
/* i == 0 */ /* i == 0 */
if (!ssl_get_new_session(s, 1)) { if (!ssl_get_new_session(s, 1)) {
@@ -1819,6 +1825,7 @@ static int tls_early_post_process_client_hello(SSL *s) @@ -1823,6 +1829,7 @@ static int tls_early_post_process_client_hello(SSL *s)
goto err; goto err;
} }
} }
@ -177,6 +177,17 @@ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
} }
if (SSL_IS_TLS13(s)) { if (SSL_IS_TLS13(s)) {
@@ -2084,6 +2091,10 @@ static int tls_early_post_process_client_hello(SSL *s)
s->clienthello = NULL;
return 0;
+retry:
+ sk_SSL_CIPHER_free(ciphers);
+ sk_SSL_CIPHER_free(scsvs);
+ return -1;
}
/*
diff --git a/util/libssl.num b/util/libssl.num diff --git a/util/libssl.num b/util/libssl.num
--- a/util/libssl.num --- a/util/libssl.num
+++ b/util/libssl.num +++ b/util/libssl.num