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: NRichard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4802)

ssl/ssl_lib.c

@@ -1969,6 +1969,7 @@ 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;
 
     switch (s->early_data_state) {
     case SSL_EARLY_DATA_NONE:
@@ -1994,9 +1995,25 @@ 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);
+        ret = SSL_write_ex(s, buf, num, &writtmp);
+        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;
+        /*
+         * TODO(TLS1.3): Technically this may not be correct in the event of
+         * SSL_MODE_ENABLE_PARTIAL_WRITE. What should we do about this?
+         */
+        *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:

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,

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);

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

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);

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 */

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;