Skip to content

Commit

Permalink
Fix mem leak in sslapi test
Browse files Browse the repository at this point in the history 
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 <viktor@openssl.org>
(Merged from #6198)
  • Loading branch information
@mattcaswell
mattcaswell committed May 11, 2018
1 parent d0191fe commit c20e3b2
Showing 1 changed file with 16 additions and 15 deletions.
31 changes: 16 additions & 15 deletions test/sslapitest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c20e3b2

Please sign in to comment.