[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Thu Dec 28 17:40:29 UTC 2017


The branch master has been updated
       via  c6a623adaa0ac4ea6b148172aaa466f287b1d8ae (commit)
       via  f7414b082714692bce592cf1645e6b1839269c1b (commit)
       via  2a8db717132ec8be7dc24ce7083972245b1173ae (commit)
      from  bfa470a4f64313651a35571883e235d3335054eb (commit)


- Log -----------------------------------------------------------------
commit c6a623adaa0ac4ea6b148172aaa466f287b1d8ae
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Dec 27 13:55:03 2017 +0000

    Update the documentation for SSL_write_early_data()
    
    Now that we attempt to send early data in the first TCP packet along with
    the ClientHello, the documentation for SSL_write_early_data() needed a
    tweak.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4802)

commit f7414b082714692bce592cf1645e6b1839269c1b
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Dec 27 13:36:45 2017 +0000

    Disable partial writes for early data
    
    We don't keep track of the number of bytes written between in the
    SSL_write_ex() call and the subsequent flush. If the flush needs to be
    retried then we will have forgotten how many bytes actually got written.
    The simplest solution is to just disable it for this scenario.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4802)

commit 2a8db717132ec8be7dc24ce7083972245b1173ae
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Nov 27 15:20:06 2017 +0000

    Don't flush the ClientHello if we're going to send early data
    
    We'd like the first bit of early_data and the ClientHello to go in the
    same TCP packet if at all possible to enable things like TCP Fast Open.
    Also, if you're only going to send one block of early data then you also
    don't need to worry about TCP_NODELAY.
    
    Fixes #4783
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4802)

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

Summary of changes:
 doc/man3/SSL_read_early_data.pod | 11 +++++++----
 ssl/ssl_lib.c                    | 26 ++++++++++++++++++++++++--
 ssl/ssl_locl.h                   |  1 +
 ssl/statem/statem.h              |  3 +++
 ssl/statem/statem_clnt.c         |  8 +++-----
 ssl/statem/statem_lib.c          |  8 ++------
 ssl/statem/statem_locl.h         |  6 ++----
 ssl/statem/statem_srvr.c         |  4 ++--
 8 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/doc/man3/SSL_read_early_data.pod b/doc/man3/SSL_read_early_data.pod
index da95a2a..d916756 100644
--- a/doc/man3/SSL_read_early_data.pod
+++ b/doc/man3/SSL_read_early_data.pod
@@ -188,10 +188,13 @@ early data solution as implemented in OpenSSL. In Nagle's algorithm the OS will
 buffer outgoing TCP data if a TCP packet has already been sent which we have not
 yet received an ACK for from the peer. The buffered data will only be
 transmitted if enough data to fill an entire TCP packet is accumulated, or if
-the ACK is received from the peer. The initial ClientHello will be sent as the
-first TCP packet, causing the early application data from calls to
-SSL_write_early_data() to be buffered by the OS and not sent until an ACK is
-received for the ClientHello packet. This means the early data is not actually
+the ACK is received from the peer. The initial ClientHello will be sent in the
+first TCP packet along with any data from the first call to
+SSL_write_early_data(). If the amount of data written will exceed the size of a
+single TCP packet, or if there are more calls to SSL_write_early_data() then
+that additional data will be sent in subsequent TCP packets which will be
+buffered by the OS and not sent until an ACK is received for the first packet
+containing the ClientHello. This means the early data is not actually
 sent until a complete round trip with the server has occurred which defeats the
 objective of early data.
 
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index fb113ba..e7867cd 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1969,6 +1969,8 @@ int SSL_write_ex(SSL *s, const void *buf, size_t num, size_t *written)
 int SSL_write_early_data(SSL *s, const void *buf, size_t num, size_t *written)
 {
     int ret, early_data_state;
+    size_t writtmp;
+    uint32_t partialwrite;
 
     switch (s->early_data_state) {
     case SSL_EARLY_DATA_NONE:
@@ -1994,9 +1996,29 @@ int SSL_write_early_data(SSL *s, const void *buf, size_t num, size_t *written)
 
     case SSL_EARLY_DATA_WRITE_RETRY:
         s->early_data_state = SSL_EARLY_DATA_WRITING;
-        ret = SSL_write_ex(s, buf, num, written);
+        /*
+         * We disable partial write for early data because we don't keep track
+         * of how many bytes we've written between the SSL_write_ex() call and
+         * the flush if the flush needs to be retried)
+         */
+        partialwrite = s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE;
+        s->mode &= ~SSL_MODE_ENABLE_PARTIAL_WRITE;
+        ret = SSL_write_ex(s, buf, num, &writtmp);
+        s->mode |= partialwrite;
+        if (!ret) {
+            s->early_data_state = SSL_EARLY_DATA_WRITE_RETRY;
+            return ret;
+        }
+        s->early_data_state = SSL_EARLY_DATA_WRITE_FLUSH;
+        /* fall through */
+
+    case SSL_EARLY_DATA_WRITE_FLUSH:
+        /* The buffering BIO is still in place so we need to flush it */
+        if (statem_flush(s) != 1)
+            return 0;
+        *written = num;
         s->early_data_state = SSL_EARLY_DATA_WRITE_RETRY;
-        return ret;
+        return 1;
 
     case SSL_EARLY_DATA_FINISHED_READING:
     case SSL_EARLY_DATA_READ_RETRY:
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index eec5be3..b5705ed 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -615,6 +615,7 @@ typedef enum {
     SSL_EARLY_DATA_CONNECTING,
     SSL_EARLY_DATA_WRITE_RETRY,
     SSL_EARLY_DATA_WRITING,
+    SSL_EARLY_DATA_WRITE_FLUSH,
     SSL_EARLY_DATA_UNAUTH_WRITING,
     SSL_EARLY_DATA_FINISHED_WRITING,
     SSL_EARLY_DATA_ACCEPT_RETRY,
diff --git a/ssl/statem/statem.h b/ssl/statem/statem.h
index 83bebe7..e8d9174 100644
--- a/ssl/statem/statem.h
+++ b/ssl/statem/statem.h
@@ -132,3 +132,6 @@ __owur int ossl_statem_skip_early_data(SSL *s);
 void ossl_statem_check_finish_init(SSL *s, int send);
 void ossl_statem_set_hello_verify_done(SSL *s);
 __owur int ossl_statem_app_data_allowed(SSL *s);
+
+/* Flush the write BIO */
+int statem_flush(SSL *s);
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 51cdd58..b47ae1e 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -665,9 +665,11 @@ WORK_STATE ossl_statem_client_pre_work(SSL *s, WORK_STATE wst)
         /* Fall through */
 
     case TLS_ST_EARLY_DATA:
+        return tls_finish_handshake(s, wst, 0, 1);
+
     case TLS_ST_OK:
         /* Calls SSLfatal() as required */
-        return tls_finish_handshake(s, wst, 1);
+        return tls_finish_handshake(s, wst, 1, 1);
     }
 
     return WORK_FINISHED_CONTINUE;
@@ -697,8 +699,6 @@ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst)
              * we call tls13_change_cipher_state() directly.
              */
             if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) == 0) {
-                if (!statem_flush(s))
-                    return WORK_MORE_A;
                 if (!tls13_change_cipher_state(s,
                             SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
                     /* SSLfatal() already called */
@@ -737,8 +737,6 @@ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst)
             break;
         if (s->early_data_state == SSL_EARLY_DATA_CONNECTING
                     && s->max_early_data > 0) {
-            if (statem_flush(s) != 1)
-                return WORK_MORE_A;
             /*
              * We haven't selected TLSv1.3 yet so we don't call the change
              * cipher state function associated with the SSL_METHOD. Instead
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index b65dfa1..02d75e7 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1004,7 +1004,7 @@ unsigned long ssl3_output_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk)
  * in NBIO events. If |clearbufs| is set then init_buf and the wbio buffer is
  * freed up as well.
  */
-WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs)
+WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
 {
     int discard;
     void (*cb) (const SSL *ssl, int type, int val) = NULL;
@@ -1083,11 +1083,7 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs)
         }
     }
 
-    /*
-     * If we've not cleared the buffers its because we've got more work to do,
-     * so continue.
-     */
-    if (!clearbufs)
+    if (!stop)
         return WORK_FINISHED_CONTINUE;
 
     ossl_statem_set_in_init(s, 0);
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 5e0ce7e..eae9a36 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -52,9 +52,6 @@ typedef enum {
     MSG_PROCESS_CONTINUE_READING
 } MSG_PROCESS_RETURN;
 
-/* Flush the write BIO */
-int statem_flush(SSL *s);
-
 typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
 
 int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups,
@@ -106,7 +103,8 @@ __owur int dtls_construct_change_cipher_spec(SSL *s, WPACKET *pkt);
 __owur int tls_construct_finished(SSL *s, WPACKET *pkt);
 __owur int tls_construct_key_update(SSL *s, WPACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt);
-__owur WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs);
+__owur WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs,
+                                       int stop);
 __owur WORK_STATE dtls_wait_for_dry(SSL *s);
 
 /* some client-only functions */
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index c6a9a66..70e026d 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -657,7 +657,7 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst)
              *
              * Calls SSLfatal as required.
              */
-            return tls_finish_handshake(s, wst, 0);
+            return tls_finish_handshake(s, wst, 0, 0);
         } if (SSL_IS_DTLS(s)) {
             /*
              * We're into the last flight. We don't retransmit the last flight
@@ -693,7 +693,7 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst)
 
     case TLS_ST_OK:
         /* Calls SSLfatal() as required */
-        return tls_finish_handshake(s, wst, 1);
+        return tls_finish_handshake(s, wst, 1, 1);
     }
 
     return WORK_FINISHED_CONTINUE;


More information about the openssl-commits mailing list