From c20e3b282c26205f39a89a23664245475d4d7cbc Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 9 May 2018 10:45:46 +0100 Subject: [PATCH] Fix mem leak in sslapi test The recent change in behaviour where you do not get a NewSessionTicket message sent if you established the connection using a PSK caused a mem leak to be triggered in sslapitest. It was actually a latent bug and we were just lucky we never hit it before. The problem is due to complexity with the way PSK sessions were set up in the early_data tests. PSK session reference counting was handled differently to normal session reference counting. This meant there were lots of special cases in the code where we don't free a session if it is a PSK. It makes things easier if we just handle PSK reference counts in the same way as other session reference counts, and then we can remove all of the special case code. Reviewed-by: Viktor Dukhovni (Merged from https://github.com/openssl/openssl/pull/6198) --- test/sslapitest.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/sslapitest.c b/test/sslapitest.c index 1db3998e51..efce84d2d9 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -1769,8 +1769,15 @@ static int setupearly_data_test(SSL_CTX **cctx, SSL_CTX **sctx, SSL **clientssl, } serverpsk = clientpsk; - if (sess != NULL) + if (sess != NULL) { + if (!TEST_true(SSL_SESSION_up_ref(clientpsk))) { + SSL_SESSION_free(clientpsk); + SSL_SESSION_free(serverpsk); + clientpsk = serverpsk = NULL; + return 0; + } *sess = clientpsk; + } return 1; } @@ -1939,9 +1946,7 @@ static int test_early_data_read_write(int idx) || !TEST_mem_eq(buf, readbytes, MSG7, strlen(MSG7))) goto end; - /* We keep the PSK session around if using PSK */ - if (idx != 2) - SSL_SESSION_free(sess); + SSL_SESSION_free(sess); sess = SSL_get1_session(clientssl); use_session_cb_cnt = 0; find_session_cb_cnt = 0; @@ -1991,8 +1996,7 @@ static int test_early_data_read_write(int idx) testresult = 1; end: - if (sess != clientpsk) - SSL_SESSION_free(sess); + SSL_SESSION_free(sess); SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; @@ -2043,8 +2047,7 @@ static int test_early_data_replay(int idx) testresult = 1; end: - if (sess != clientpsk) - SSL_SESSION_free(sess); + SSL_SESSION_free(sess); SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; @@ -2131,8 +2134,7 @@ static int early_data_skip_helper(int hrr, int idx) testresult = 1; end: - if (sess != clientpsk) - SSL_SESSION_free(clientpsk); + SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; SSL_SESSION_free(sess); @@ -2219,8 +2221,8 @@ static int test_early_data_not_sent(int idx) testresult = 1; end: - /* If using PSK then clientpsk and sess are the same */ SSL_SESSION_free(sess); + SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; SSL_free(serverssl); @@ -2428,6 +2430,7 @@ static int test_early_data_psk(int idx) testresult = 1; end: + SSL_SESSION_free(sess); SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; @@ -2485,8 +2488,8 @@ static int test_early_data_not_expected(int idx) testresult = 1; end: - /* If using PSK then clientpsk and sess are the same */ SSL_SESSION_free(sess); + SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; SSL_free(serverssl); @@ -2559,7 +2562,6 @@ static int test_early_data_tls1_2(int idx) testresult = 1; end: - /* If using PSK then clientpsk and sess are the same */ SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; @@ -3705,8 +3707,7 @@ static int test_export_key_mat_early(int idx) testresult = 1; end: - if (sess != clientpsk) - SSL_SESSION_free(sess); + SSL_SESSION_free(sess); SSL_SESSION_free(clientpsk); SSL_SESSION_free(serverpsk); clientpsk = serverpsk = NULL; -- 2.34.1