[openssl-commits] [openssl] OpenSSL_1_1_1-stable update

Matt Caswell matt at openssl.org
Thu Jan 24 13:48:39 UTC 2019


The branch OpenSSL_1_1_1-stable has been updated
       via  8e3df4012a8177b89707ebec249be417508c8c7f (commit)
       via  f9ad0abb29aca7e765b041c3a13457a58ce66314 (commit)
      from  d0a4e858bb658c098cd267252e1e7cfffe554aff (commit)


- Log -----------------------------------------------------------------
commit 8e3df4012a8177b89707ebec249be417508c8c7f
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Jan 18 12:10:07 2019 +0000

    Revert "Keep the DTLS timer running after the end of the handshake if appropriate"
    
    This commit erroneously kept the DTLS timer running after the end of the
    handshake. This is not correct behaviour and shold be reverted.
    
    This reverts commit f7506416b1311e65d5c440defdbcfe176f633c50.
    
    Fixes #7998
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8047)
    
    (cherry picked from commit bcc1f3e2baa9caa83a0a94bd19fb37488ef3ee57)

commit f9ad0abb29aca7e765b041c3a13457a58ce66314
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Jan 18 15:24:57 2019 +0000

    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 at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8047)
    
    (cherry picked from commit 80c455d5ae405e855391e298a2bf8a24629dd95d)

-----------------------------------------------------------------------

Summary of changes:
 ssl/record/rec_layer_d1.c | 13 -------------
 ssl/statem/statem_lib.c   | 18 ------------------
 test/dtlstest.c           | 14 +++++++++-----
 test/sslapitest.c         |  2 +-
 test/ssltestlib.c         | 31 ++++++++++++++++++++++++-------
 test/ssltestlib.h         |  3 ++-
 6 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index 1f9b319..1e129b7 100644
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -440,19 +440,6 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
             && SSL3_RECORD_get_length(rr) != 0)
         s->rlayer.alert_count = 0;
 
-    if (SSL3_RECORD_get_type(rr) != SSL3_RT_HANDSHAKE
-            && SSL3_RECORD_get_type(rr) != SSL3_RT_CHANGE_CIPHER_SPEC
-            && !SSL_in_init(s)
-            && (s->d1->next_timeout.tv_sec != 0
-                || s->d1->next_timeout.tv_usec != 0)) {
-        /*
-         * The timer is still running but we've received something that isn't
-         * handshake data - so the peer must have finished processing our
-         * last handshake flight. Stop the timer.
-         */
-        dtls1_stop_timer(s);
-    }
-
     /* we now have a packet which can be read and processed */
 
     if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index be270e2..cf62c8f 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1076,15 +1076,6 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
             /* N.B. s->ctx may not equal s->session_ctx */
             tsan_counter(&s->ctx->stats.sess_accept_good);
             s->handshake_func = ossl_statem_accept;
-
-            if (SSL_IS_DTLS(s) && !s->hit) {
-                /*
-                 * We are finishing after the client. We start the timer going
-                 * in case there are any retransmits of our final flight
-                 * required.
-                 */
-                dtls1_start_timer(s);
-            }
         } else {
             if (SSL_IS_TLS13(s)) {
                 /*
@@ -1106,15 +1097,6 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
 
             s->handshake_func = ossl_statem_connect;
             tsan_counter(&s->session_ctx->stats.sess_connect_good);
-
-            if (SSL_IS_DTLS(s) && s->hit) {
-                /*
-                 * We are finishing after the server. We start the timer going
-                 * in case there are any retransmits of our final flight
-                 * required.
-                 */
-                dtls1_start_timer(s);
-            }
         }
 
         if (SSL_IS_DTLS(s)) {
diff --git a/test/dtlstest.c b/test/dtlstest.c
index 772528f..8517eae 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 a4bbb4f..3e84694 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 2a774f2..320a82d 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 27b040c..6b34941 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);
 


More information about the openssl-commits mailing list