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 <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8047)
This commit is contained in:
Matt Caswell 2019-01-18 15:24:57 +00:00
parent 5cae2d349b
commit 80c455d5ae
4 changed files with 36 additions and 14 deletions

View file

@ -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 * 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 * 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 * the record sequence number to be way off in the future.
* have an impact on the record replay protection because the record should
* be dropped before it is marked as arrived
*/ */
c_to_s_mempacket = SSL_get_wbio(clientssl1); c_to_s_mempacket = SSL_get_wbio(clientssl1);
c_to_s_mempacket = BIO_next(c_to_s_mempacket); c_to_s_mempacket = BIO_next(c_to_s_mempacket);
mempacket_test_inject(c_to_s_mempacket, (char *)certstatus, mempacket_test_inject(c_to_s_mempacket, (char *)certstatus,
sizeof(certstatus), 1, INJECT_PACKET_IGNORE_REC_SEQ); 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; goto end;
if (timer_cb_count == 0) { if (timer_cb_count == 0) {

View file

@ -5593,7 +5593,7 @@ static int test_shutdown(int tst)
if (tst == 3) { if (tst == 3) {
if (!TEST_true(create_bare_ssl_connection(serverssl, clientssl, 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_ptr_ne(sess = SSL_get_session(clientssl), NULL)
|| !TEST_false(SSL_SESSION_is_resumable(sess))) || !TEST_false(SSL_SESSION_is_resumable(sess)))
goto end; goto end;

View file

@ -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 * Create an SSL connection, but does not ready any post-handshake
* NewSessionTicket messages. * 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 retc = -1, rets = -1, err, abortctr = 0;
int clienterr = 0, servererr = 0; int clienterr = 0, servererr = 0;
@ -874,11 +878,24 @@ int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want)
return 0; return 0;
if (clienterr && servererr) if (clienterr && servererr)
return 0; return 0;
if (isdtls) { if (isdtls && read) {
if (rets > 0 && retc <= 0) unsigned char buf[20];
DTLSv1_handle_timeout(serverssl);
if (retc > 0 && rets <= 0) /* Trigger any retransmits that may be appropriate */
DTLSv1_handle_timeout(clientssl); 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) { if (++abortctr == MAXLOOPS) {
TEST_info("No progress made"); TEST_info("No progress made");
@ -907,7 +924,7 @@ int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want)
unsigned char buf; unsigned char buf;
size_t readbytes; size_t readbytes;
if (!create_bare_ssl_connection(serverssl, clientssl, want)) if (!create_bare_ssl_connection(serverssl, clientssl, want, 1))
return 0; return 0;
/* /*

View file

@ -18,7 +18,8 @@ int create_ssl_ctx_pair(const SSL_METHOD *sm, const SSL_METHOD *cm,
char *privkeyfile); char *privkeyfile);
int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl,
SSL **cssl, BIO *s_to_c_fbio, BIO *c_to_s_fbio); 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, int create_ssl_objects2(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl,
SSL **cssl, int sfd, int cfd); SSL **cssl, int sfd, int cfd);
int create_test_sockets(int *cfd, int *sfd); int create_test_sockets(int *cfd, int *sfd);