From f9ad0abb29aca7e765b041c3a13457a58ce66314 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 18 Jan 2019 15:24:57 +0000 Subject: [PATCH] Make sure we trigger retransmits in DTLS testing During a DTLS handshake we may need to periodically handle timeouts in the DTLS timer to ensure retransmits due to lost packets are performed. However, one peer will always complete a handshake before the other. The DTLS timer stops once the handshake has finished so any handshake messages lost after that point will not automatically get retransmitted simply by calling DTLSv1_handle_timeout(). However attempting an SSL_read implies a DTLSv1_handle_timeout() and additionally will process records received from the peer. If those records are themselves retransmits then we know that the peer has not completed its handshake yet and a retransmit of our final flight automatically occurs. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8047) (cherry picked from commit 80c455d5ae405e855391e298a2bf8a24629dd95d) --- test/dtlstest.c | 14 +++++++++----- test/sslapitest.c | 2 +- test/ssltestlib.c | 31 ++++++++++++++++++++++++------- test/ssltestlib.h | 3 ++- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/test/dtlstest.c b/test/dtlstest.c index 772528febf..8517eae54e 100644 --- a/test/dtlstest.c +++ b/test/dtlstest.c @@ -87,17 +87,21 @@ static int test_dtls_unprocessed(int testidx) /* * Inject a dummy record from the next epoch. In test 0, this should never * get used because the message sequence number is too big. In test 1 we set - * the record sequence number to be way off in the future. This should not - * have an impact on the record replay protection because the record should - * be dropped before it is marked as arrived + * the record sequence number to be way off in the future. */ c_to_s_mempacket = SSL_get_wbio(clientssl1); c_to_s_mempacket = BIO_next(c_to_s_mempacket); mempacket_test_inject(c_to_s_mempacket, (char *)certstatus, sizeof(certstatus), 1, INJECT_PACKET_IGNORE_REC_SEQ); - if (!TEST_true(create_ssl_connection(serverssl1, clientssl1, - SSL_ERROR_NONE))) + /* + * Create the connection. We use "create_bare_ssl_connection" here so that + * we can force the connection to not do "SSL_read" once partly conencted. + * We don't want to accidentally read the dummy records we injected because + * they will fail to decrypt. + */ + if (!TEST_true(create_bare_ssl_connection(serverssl1, clientssl1, + SSL_ERROR_NONE, 0))) goto end; if (timer_cb_count == 0) { diff --git a/test/sslapitest.c b/test/sslapitest.c index a4bbb4fead..3e8469419f 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -5408,7 +5408,7 @@ static int test_shutdown(int tst) if (tst == 3) { if (!TEST_true(create_bare_ssl_connection(serverssl, clientssl, - SSL_ERROR_NONE)) + SSL_ERROR_NONE, 1)) || !TEST_ptr_ne(sess = SSL_get_session(clientssl), NULL) || !TEST_false(SSL_SESSION_is_resumable(sess))) goto end; diff --git a/test/ssltestlib.c b/test/ssltestlib.c index 2a774f23c4..320a82d82b 100644 --- a/test/ssltestlib.c +++ b/test/ssltestlib.c @@ -719,8 +719,12 @@ int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, /* * Create an SSL connection, but does not ready any post-handshake * NewSessionTicket messages. + * If |read| is set and we're using DTLS then we will attempt to SSL_read on + * the connection once we've completed one half of it, to ensure any retransmits + * get triggered. */ -int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want) +int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want, + int read) { int retc = -1, rets = -1, err, abortctr = 0; int clienterr = 0, servererr = 0; @@ -758,11 +762,24 @@ int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want) return 0; if (clienterr && servererr) return 0; - if (isdtls) { - if (rets > 0 && retc <= 0) - DTLSv1_handle_timeout(serverssl); - if (retc > 0 && rets <= 0) - DTLSv1_handle_timeout(clientssl); + if (isdtls && read) { + unsigned char buf[20]; + + /* Trigger any retransmits that may be appropriate */ + if (rets > 0 && retc <= 0) { + if (SSL_read(serverssl, buf, sizeof(buf)) > 0) { + /* We don't expect this to succeed! */ + TEST_info("Unexpected SSL_read() success!"); + return 0; + } + } + if (retc > 0 && rets <= 0) { + if (SSL_read(clientssl, buf, sizeof(buf)) > 0) { + /* We don't expect this to succeed! */ + TEST_info("Unexpected SSL_read() success!"); + return 0; + } + } } if (++abortctr == MAXLOOPS) { TEST_info("No progress made"); @@ -791,7 +808,7 @@ int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want) unsigned char buf; size_t readbytes; - if (!create_bare_ssl_connection(serverssl, clientssl, want)) + if (!create_bare_ssl_connection(serverssl, clientssl, want, 1)) return 0; /* diff --git a/test/ssltestlib.h b/test/ssltestlib.h index 27b040c3cf..6b34941e9c 100644 --- a/test/ssltestlib.h +++ b/test/ssltestlib.h @@ -18,7 +18,8 @@ int create_ssl_ctx_pair(const SSL_METHOD *sm, const SSL_METHOD *cm, char *privkeyfile); int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, SSL **cssl, BIO *s_to_c_fbio, BIO *c_to_s_fbio); -int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want); +int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want, + int read); int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want); void shutdown_ssl_connection(SSL *serverssl, SSL *clientssl); -- 2.25.1