[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Tue Jul 18 15:58:13 UTC 2017


The branch master has been updated
       via  6b84e6bf19f5afad338f22a1a6d71a75d2d95fbf (commit)
       via  d4504fe5792b2dcf8ae6ef35634f1494e72d109b (commit)
      from  1e3f62a3823f7e3db9d403f724fd9d66f5b04cf8 (commit)


- Log -----------------------------------------------------------------
commit 6b84e6bf19f5afad338f22a1a6d71a75d2d95fbf
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Jul 14 12:23:56 2017 +0100

    Add a test for early_data when an HRR occurs
    
    Reviewed-by: Ben Kaduk <kaduk at mit.edu>
    (Merged from https://github.com/openssl/openssl/pull/3933)

commit d4504fe5792b2dcf8ae6ef35634f1494e72d109b
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Jul 14 14:50:48 2017 +0100

    Fix early_data with an HRR
    
    early_data is not allowed after an HRR. We failed to handle that
    correctly.
    
    Reviewed-by: Ben Kaduk <kaduk at mit.edu>
    (Merged from https://github.com/openssl/openssl/pull/3933)

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

Summary of changes:
 ssl/statem/extensions_srvr.c |  5 ++++
 ssl/statem/statem.c          |  9 ++-----
 ssl/statem/statem_clnt.c     |  7 ++++++
 ssl/statem/statem_srvr.c     | 18 +++++++-------
 test/sslapitest.c            | 57 ++++++++++++++++++++++++++++++++++++--------
 5 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 7f30ac7..9fe58a7 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -678,6 +678,11 @@ int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
         return 0;
     }
 
+    if (s->hello_retry_request) {
+        *al = SSL_AD_ILLEGAL_PARAMETER;
+        return 0;
+    }
+
     return 1;
 }
 
diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c
index 9eab8ce..e5a50c4 100644
--- a/ssl/statem/statem.c
+++ b/ssl/statem/statem.c
@@ -157,13 +157,8 @@ int ossl_statem_skip_early_data(SSL *s)
     if (s->ext.early_data != SSL_EARLY_DATA_REJECTED)
         return 0;
 
-    if (s->hello_retry_request) {
-        if (s->statem.hand_state != TLS_ST_SW_HELLO_RETRY_REQUEST)
-            return 0;
-    } else {
-        if (!s->server || s->statem.hand_state != TLS_ST_EARLY_DATA)
-            return 0;
-    }
+    if (!s->server || s->statem.hand_state != TLS_ST_EARLY_DATA)
+        return 0;
 
     return 1;
 }
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 55ac4dd..ed9bd5c 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1571,6 +1571,13 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
 
     s->hello_retry_request = 1;
 
+    /*
+     * If we were sending early_data then the enc_write_ctx is now invalid and
+     * should not be used.
+     */
+    EVP_CIPHER_CTX_free(s->enc_write_ctx);
+    s->enc_write_ctx = NULL;
+
     /* This will fail if it doesn't choose TLSv1.3+ */
     errorcode = ssl_choose_client_version(s, sversion, 0, &al);
     if (errorcode != 0) {
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index f3f54d4..9d3c387 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -48,15 +48,14 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
     default:
         break;
 
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        if (mt == SSL3_MT_CLIENT_HELLO) {
-            st->hand_state = TLS_ST_SR_CLNT_HELLO;
-            return 1;
-        }
-        break;
-
     case TLS_ST_EARLY_DATA:
-        if (s->ext.early_data == SSL_EARLY_DATA_ACCEPTED) {
+        if (s->hello_retry_request) {
+            if (mt == SSL3_MT_CLIENT_HELLO) {
+                st->hand_state = TLS_ST_SR_CLNT_HELLO;
+                return 1;
+            }
+            break;
+        } else if (s->ext.early_data == SSL_EARLY_DATA_ACCEPTED) {
             if (mt == SSL3_MT_END_OF_EARLY_DATA) {
                 st->hand_state = TLS_ST_SR_END_OF_EARLY_DATA;
                 return 1;
@@ -397,7 +396,8 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        return WRITE_TRAN_FINISHED;
+        st->hand_state = TLS_ST_EARLY_DATA;
+        return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_SRVR_HELLO:
         st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index b77a229..cd869e2 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -1536,10 +1536,10 @@ static int test_early_data_read_write(int idx)
 }
 
 /*
- * Test that a server attempting to read early data can handle a connection
- * from a client where the early data is not acceptable.
+ * Helper function to test that a server attempting to read early data can
+ * handle a connection from a client where the early data should be skipped.
  */
-static int test_early_data_skip(int idx)
+static int early_data_skip_helper(int hrr, int idx)
 {
     SSL_CTX *cctx = NULL, *sctx = NULL;
     SSL *clientssl = NULL, *serverssl = NULL;
@@ -1552,13 +1552,19 @@ static int test_early_data_skip(int idx)
                                         &serverssl, &sess, idx)))
         goto end;
 
-    /*
-     * Deliberately corrupt the creation time. We take 20 seconds off the time.
-     * It could be any value as long as it is not within tolerance. This should
-     * mean the ticket is rejected.
-     */
-    if (!TEST_true(SSL_SESSION_set_time(sess, time(NULL) - 20)))
-        goto end;
+    if (hrr) {
+        /* Force an HRR to occur */
+        if (!TEST_true(SSL_set1_groups_list(serverssl, "P-256")))
+            goto end;
+    } else {
+        /*
+         * Deliberately corrupt the creation time. We take 20 seconds off the
+         * time. It could be any value as long as it is not within tolerance.
+         * This should mean the ticket is rejected.
+         */
+        if (!TEST_true(SSL_SESSION_set_time(sess, time(NULL) - 20)))
+            goto end;
+    }
 
     /* Write some early data */
     if (!TEST_true(SSL_write_early_data(clientssl, MSG1, strlen(MSG1),
@@ -1575,6 +1581,18 @@ static int test_early_data_skip(int idx)
                             SSL_EARLY_DATA_REJECTED))
         goto end;
 
+    if (hrr) {
+        /*
+         * Finish off the handshake. We perform the same writes and reads as
+         * further down but we expect them to fail due to the incomplete
+         * handshake.
+         */
+        if (!TEST_false(SSL_write_ex(clientssl, MSG2, strlen(MSG2), &written))
+                || !TEST_false(SSL_read_ex(serverssl, buf, sizeof(buf),
+                               &readbytes)))
+            goto end;
+    }
+
     /* Should be able to send normal data despite rejection of early data */
     if (!TEST_true(SSL_write_ex(clientssl, MSG2, strlen(MSG2), &written))
             || !TEST_size_t_eq(written, strlen(MSG2))
@@ -1597,6 +1615,24 @@ static int test_early_data_skip(int idx)
 
 /*
  * Test that a server attempting to read early data can handle a connection
+ * from a client where the early data is not acceptable.
+ */
+static int test_early_data_skip(int idx)
+{
+    return early_data_skip_helper(0, idx);
+}
+
+/*
+ * Test that a server attempting to read early data can handle a connection
+ * from a client where an HRR occurs.
+ */
+static int test_early_data_skip_hrr(int idx)
+{
+    return early_data_skip_helper(1, idx);
+}
+
+/*
+ * Test that a server attempting to read early data can handle a connection
  * from a client that doesn't send any.
  */
 static int test_early_data_not_sent(int idx)
@@ -2652,6 +2688,7 @@ int test_main(int argc, char *argv[])
 #ifndef OPENSSL_NO_TLS1_3
     ADD_ALL_TESTS(test_early_data_read_write, 2);
     ADD_ALL_TESTS(test_early_data_skip, 2);
+    ADD_ALL_TESTS(test_early_data_skip_hrr, 2);
     ADD_ALL_TESTS(test_early_data_not_sent, 2);
     ADD_ALL_TESTS(test_early_data_not_expected, 2);
 # ifndef OPENSSL_NO_TLS1_2


More information about the openssl-commits mailing list