Make sure we trigger retransmits in DTLS testing
authorMatt Caswell <matt@openssl.org>
Fri, 18 Jan 2019 15:24:57 +0000 (15:24 +0000)
committerMatt Caswell <matt@openssl.org>
Thu, 24 Jan 2019 13:44:27 +0000 (13:44 +0000)
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)

(cherry picked from commit 80c455d5ae405e855391e298a2bf8a24629dd95d)

test/dtlstest.c
test/sslapitest.c
test/ssltestlib.c
test/ssltestlib.h

index 772528f..8517eae 100644 (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
      * 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) {
index a4bbb4f..3e84694 100644 (file)
@@ -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;
index 2a774f2..320a82d 100644 (file)
@@ -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;
 
     /*
index 27b040c..6b34941 100644 (file)
@@ -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);