From: Matt Caswell Date: Fri, 18 Jan 2019 15:24:57 +0000 (+0000) Subject: Make sure we trigger retransmits in DTLS testing X-Git-Tag: openssl-3.0.0-alpha1~2616 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=80c455d5ae405e855391e298a2bf8a24629dd95d;p=oweals%2Fopenssl.git 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) --- diff --git a/test/dtlstest.c b/test/dtlstest.c index 0b048869d6..d196fb55dc 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 1868eb31f9..69520d7404 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -5593,7 +5593,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 78c0e8eb79..2f662674e7 100644 --- a/test/ssltestlib.c +++ b/test/ssltestlib.c @@ -835,8 +835,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; @@ -874,11 +878,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"); @@ -907,7 +924,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 8dd626947c..6573e385ba 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_objects2(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, SSL **cssl, int sfd, int cfd); int create_test_sockets(int *cfd, int *sfd);