Commit 2a8db717 authored by Matt Caswell's avatar Matt Caswell
Browse files

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

ssl/ssl_lib.c

+19 −2
Original line number Diff line number Diff line
@@ -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 1;



    case SSL_EARLY_DATA_FINISHED_READING:

    case SSL_EARLY_DATA_READ_RETRY:

ssl/ssl_locl.h

+1 −0
Original line number Diff line number Diff line
@@ -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

+3 −0
Original line number Diff line number Diff line
@@ -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

+3 −5
Original line number Diff line number Diff line
@@ -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

+2 −6
Original line number Diff line number Diff line
@@ -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);