mirror of
				https://github.com/openresty/openresty.git
				synced 2024-10-13 00:29:41 +00:00 
			
		
		
		
	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
    }
		
	
		
			
				
	
	
		
			202 lines
		
	
	
		
			7.8 KiB
		
	
	
	
		
			C
		
	
	
	
	
	
			
		
		
	
	
			202 lines
		
	
	
		
			7.8 KiB
		
	
	
	
		
			C
		
	
	
	
	
	
| diff --git a/include/openssl/bio.h b/include/openssl/bio.h
 | |
| --- a/include/openssl/bio.h
 | |
| +++ b/include/openssl/bio.h
 | |
| @@ -219,6 +219,8 @@ void BIO_clear_flags(BIO *b, int flags);
 | |
|  /* Returned from the accept BIO when an accept would have blocked */
 | |
|  # define BIO_RR_ACCEPT                   0x03
 | |
|  
 | |
| +# define BIO_RR_SSL_SESSION_LOOKUP       0x09
 | |
| +
 | |
|  /* These are passed by the BIO callback */
 | |
|  # define BIO_CB_FREE     0x01
 | |
|  # define BIO_CB_READ     0x02
 | |
| diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
 | |
| --- a/include/openssl/ssl.h
 | |
| +++ b/include/openssl/ssl.h
 | |
| @@ -896,6 +896,7 @@ __owur int SSL_extension_supported(unsigned int ext_type);
 | |
|  # define SSL_ASYNC_PAUSED       5
 | |
|  # define SSL_ASYNC_NO_JOBS      6
 | |
|  # define SSL_CLIENT_HELLO_CB    7
 | |
| +# define SSL_SESS_LOOKUP        99
 | |
|  
 | |
|  /* These will only be used when doing non-blocking IO */
 | |
|  # define SSL_want_nothing(s)         (SSL_want(s) == SSL_NOTHING)
 | |
| @@ -905,6 +906,7 @@ __owur int SSL_extension_supported(unsigned int ext_type);
 | |
|  # define SSL_want_async(s)           (SSL_want(s) == SSL_ASYNC_PAUSED)
 | |
|  # define SSL_want_async_job(s)       (SSL_want(s) == SSL_ASYNC_NO_JOBS)
 | |
|  # define SSL_want_client_hello_cb(s) (SSL_want(s) == SSL_CLIENT_HELLO_CB)
 | |
| +# define SSL_want_sess_lookup(s)     (SSL_want(s) == SSL_SESS_LOOKUP)
 | |
|  
 | |
|  # define SSL_MAC_FLAG_READ_MAC_STREAM 1
 | |
|  # define SSL_MAC_FLAG_WRITE_MAC_STREAM 2
 | |
| @@ -1190,6 +1192,8 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION)
 | |
|  # define SSL_ERROR_WANT_ASYNC            9
 | |
|  # define SSL_ERROR_WANT_ASYNC_JOB       10
 | |
|  # define SSL_ERROR_WANT_CLIENT_HELLO_CB 11
 | |
| +# define SSL_ERROR_WANT_SESSION_LOOKUP  99
 | |
| +# define SSL_ERROR_PENDING_SESSION      99 /* BoringSSL compatibility */
 | |
|  # define SSL_CTRL_SET_TMP_DH                     3
 | |
|  # define SSL_CTRL_SET_TMP_ECDH                   4
 | |
|  # define SSL_CTRL_SET_TMP_DH_CB                  6
 | |
| @@ -1662,6 +1666,7 @@ int SSL_SESSION_print(BIO *fp, const SSL_SESSION *ses);
 | |
|  int SSL_SESSION_print_keylog(BIO *bp, const SSL_SESSION *x);
 | |
|  int SSL_SESSION_up_ref(SSL_SESSION *ses);
 | |
|  void SSL_SESSION_free(SSL_SESSION *ses);
 | |
| +SSL_SESSION *SSL_magic_pending_session_ptr(void);
 | |
|  __owur int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp);
 | |
|  __owur int SSL_set_session(SSL *to, SSL_SESSION *session);
 | |
|  int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *session);
 | |
| diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c
 | |
| --- a/ssl/bio_ssl.c
 | |
| +++ b/ssl/bio_ssl.c
 | |
| @@ -139,6 +139,10 @@ static int ssl_read(BIO *b, char *buf, size_t size, size_t *readbytes)
 | |
|          BIO_set_retry_special(b);
 | |
|          retry_reason = BIO_RR_SSL_X509_LOOKUP;
 | |
|          break;
 | |
| +    case SSL_ERROR_WANT_SESSION_LOOKUP:
 | |
| +        BIO_set_retry_special(b);
 | |
| +        retry_reason = BIO_RR_SSL_SESSION_LOOKUP;
 | |
| +        break;
 | |
|      case SSL_ERROR_WANT_ACCEPT:
 | |
|          BIO_set_retry_special(b);
 | |
|          retry_reason = BIO_RR_ACCEPT;
 | |
| @@ -207,6 +211,10 @@ static int ssl_write(BIO *b, const char *buf, size_t size, size_t *written)
 | |
|          BIO_set_retry_special(b);
 | |
|          retry_reason = BIO_RR_SSL_X509_LOOKUP;
 | |
|          break;
 | |
| +    case SSL_ERROR_WANT_SESSION_LOOKUP:
 | |
| +        BIO_set_retry_special(b);
 | |
| +        retry_reason = BIO_RR_SSL_SESSION_LOOKUP;
 | |
| +        break;
 | |
|      case SSL_ERROR_WANT_CONNECT:
 | |
|          BIO_set_retry_special(b);
 | |
|          retry_reason = BIO_RR_CONNECT;
 | |
| @@ -361,6 +369,10 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
 | |
|              BIO_set_retry_special(b);
 | |
|              BIO_set_retry_reason(b, BIO_RR_SSL_X509_LOOKUP);
 | |
|              break;
 | |
| +        case SSL_ERROR_WANT_SESSION_LOOKUP:
 | |
| +            BIO_set_retry_special(b);
 | |
| +            BIO_set_retry_reason(b, BIO_RR_SSL_SESSION_LOOKUP);
 | |
| +            break;
 | |
|          default:
 | |
|              break;
 | |
|          }
 | |
| diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
 | |
| --- a/ssl/ssl_lib.c
 | |
| +++ b/ssl/ssl_lib.c
 | |
| @@ -3556,6 +3556,8 @@ int SSL_get_error(const SSL *s, int i)
 | |
|          return SSL_ERROR_WANT_ASYNC_JOB;
 | |
|      if (SSL_want_client_hello_cb(s))
 | |
|          return SSL_ERROR_WANT_CLIENT_HELLO_CB;
 | |
| +    if (SSL_want_sess_lookup(s))
 | |
| +        return SSL_ERROR_WANT_SESSION_LOOKUP;
 | |
|  
 | |
|      if ((s->shutdown & SSL_RECEIVED_SHUTDOWN) &&
 | |
|          (s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY))
 | |
| diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
 | |
| --- a/ssl/ssl_sess.c
 | |
| +++ b/ssl/ssl_sess.c
 | |
| @@ -16,6 +16,8 @@
 | |
|  #include "ssl_locl.h"
 | |
|  #include "statem/statem_locl.h"
 | |
|  
 | |
| +static const char g_pending_session_magic = 0;
 | |
| +
 | |
|  static void SSL_SESSION_list_remove(SSL_CTX *ctx, SSL_SESSION *s);
 | |
|  static void SSL_SESSION_list_add(SSL_CTX *ctx, SSL_SESSION *s);
 | |
|  static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck);
 | |
| @@ -476,6 +478,10 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id,
 | |
|  
 | |
|          ret = s->session_ctx->get_session_cb(s, sess_id, sess_id_len, ©);
 | |
|  
 | |
| +        if (ret == SSL_magic_pending_session_ptr()) {
 | |
| +            return ret; /* Retry later */
 | |
| +        }
 | |
| +
 | |
|          if (ret != NULL) {
 | |
|              tsan_counter(&s->session_ctx->stats.sess_cb_hit);
 | |
|  
 | |
| @@ -564,6 +570,9 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
 | |
|                  try_session_cache = 1;
 | |
|                  ret = lookup_sess_in_cache(s, hello->session_id,
 | |
|                                             hello->session_id_len);
 | |
| +                if (ret == SSL_magic_pending_session_ptr()) {
 | |
| +                    return -2; /* Retry later */
 | |
| +                }
 | |
|              }
 | |
|              break;
 | |
|          case SSL_TICKET_NO_DECRYPT:
 | |
| @@ -989,6 +998,11 @@ X509 *SSL_SESSION_get0_peer(SSL_SESSION *s)
 | |
|      return s->peer;
 | |
|  }
 | |
|  
 | |
| +SSL_SESSION *SSL_magic_pending_session_ptr(void)
 | |
| +{
 | |
| +    return (SSL_SESSION *) &g_pending_session_magic;
 | |
| +}
 | |
| +
 | |
|  int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned char *sid_ctx,
 | |
|                                  unsigned int sid_ctx_len)
 | |
|  {
 | |
| diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
 | |
| --- a/ssl/statem/statem_srvr.c
 | |
| +++ b/ssl/statem/statem_srvr.c
 | |
| @@ -1604,6 +1604,7 @@ static int tls_early_post_process_client_hello(SSL *s)
 | |
|      STACK_OF(SSL_CIPHER) *scsvs = NULL;
 | |
|      CLIENTHELLO_MSG *clienthello = s->clienthello;
 | |
|      DOWNGRADE dgrd = DOWNGRADE_NONE;
 | |
| +    PACKET saved_ciphers;
 | |
|  
 | |
|      /* Finished parsing the ClientHello, now we can start processing it */
 | |
|      /* Give the ClientHello callback a crack at things */
 | |
| @@ -1711,6 +1712,7 @@ static int tls_early_post_process_client_hello(SSL *s)
 | |
|      }
 | |
|  
 | |
|      s->hit = 0;
 | |
| +    saved_ciphers = clienthello->ciphersuites;
 | |
|  
 | |
|      if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites,
 | |
|                                clienthello->isv2) ||
 | |
| @@ -1816,6 +1818,10 @@ static int tls_early_post_process_client_hello(SSL *s)
 | |
|          } else if (i == -1) {
 | |
|              /* SSLfatal() already called */
 | |
|              goto err;
 | |
| +        } else if (i == -2) {
 | |
| +            clienthello->ciphersuites = saved_ciphers;
 | |
| +            s->rwstate = SSL_SESS_LOOKUP;
 | |
| +            goto retry;
 | |
|          } else {
 | |
|              /* i == 0 */
 | |
|              if (!ssl_get_new_session(s, 1)) {
 | |
| @@ -1823,6 +1829,7 @@ static int tls_early_post_process_client_hello(SSL *s)
 | |
|                  goto err;
 | |
|              }
 | |
|          }
 | |
| +        s->rwstate = SSL_NOTHING;
 | |
|      }
 | |
|  
 | |
|      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
 | |
| --- a/util/libssl.num
 | |
| +++ b/util/libssl.num
 | |
| @@ -7,6 +7,7 @@ SSL_copy_session_id                     6	1_1_0	EXIST::FUNCTION:
 | |
|  SSL_CTX_set_srp_password                7	1_1_0	EXIST::FUNCTION:SRP
 | |
|  SSL_shutdown                            8	1_1_0	EXIST::FUNCTION:
 | |
|  SSL_CTX_set_msg_callback                9	1_1_0	EXIST::FUNCTION:
 | |
| +SSL_magic_pending_session_ptr           10	1_1_0   EXIST::FUNCTION:
 | |
|  SSL_SESSION_get0_ticket                 11	1_1_0	EXIST::FUNCTION:
 | |
|  SSL_get1_supported_ciphers              12	1_1_0	EXIST::FUNCTION:
 | |
|  SSL_state_string_long                   13	1_1_0	EXIST::FUNCTION:
 |