Commit c7f47786 authored by Matt Caswell's avatar Matt Caswell
Browse files

Move state machine knowledge out of the record layer 



The record layer was making decisions that should really be left to the
state machine around unexpected handshake messages that are received after
the initial handshake (i.e. renegotiation related messages). This commit
removes that code from the record layer and updates the state machine
accordingly. This simplifies the state machine and paves the way for
handling other messages post-handshake such as the NewSessionTicket in
TLSv1.3.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
parent 0386aad1

include/openssl/ssl.h

+5 −2
Original line number Diff line number Diff line
@@ -878,7 +878,8 @@ typedef enum { 
    TLS_ST_SW_ENCRYPTED_EXTENSIONS,

    TLS_ST_CR_ENCRYPTED_EXTENSIONS,

    TLS_ST_CR_CERT_VRFY,

    TLS_ST_SW_CERT_VRFY

    TLS_ST_SW_CERT_VRFY,

    TLS_ST_CR_HELLO_REQ

} OSSL_HANDSHAKE_STATE;



/*
@@ -1647,7 +1648,7 @@ __owur STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s); 


__owur int SSL_do_handshake(SSL *s);

int SSL_renegotiate(SSL *s);

__owur int SSL_renegotiate_abbreviated(SSL *s);

int SSL_renegotiate_abbreviated(SSL *s);

__owur int SSL_renegotiate_pending(SSL *s);

int SSL_shutdown(SSL *s);
@@ -2344,6 +2345,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE            382

# define SSL_F_TLS_PROCESS_ENCRYPTED_EXTENSIONS           444

# define SSL_F_TLS_PROCESS_FINISHED                       364

# define SSL_F_TLS_PROCESS_HELLO_REQ                      507

# define SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT          442

# define SSL_F_TLS_PROCESS_KEY_EXCHANGE                   365

# define SSL_F_TLS_PROCESS_NEW_SESSION_TICKET             366
@@ -2356,6 +2358,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE               421

# define SSL_F_TLS_PROCESS_SKE_SRP                        422

# define SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT                450

# define SSL_F_TLS_SETUP_HANDSHAKE                        508

# define SSL_F_USE_CERTIFICATE_CHAIN_FILE                 220



/* Reason codes. */

ssl/record/rec_layer_d1.c

+16 −68
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 
#include <openssl/evp.h>

#include <openssl/buffer.h>

#include "record_locl.h"

#include <assert.h>



int DTLS_RECORD_LAYER_new(RECORD_LAYER *rl)

{
@@ -632,70 +633,6 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
     * (Possibly rr is 'empty' now, i.e. rr->length may be 0.)

     */



    /* If we are a client, check for an incoming 'Hello Request': */

    if ((!s->server) &&

        (s->rlayer.d->handshake_fragment_len >= DTLS1_HM_HEADER_LENGTH) &&

        (s->rlayer.d->handshake_fragment[0] == SSL3_MT_HELLO_REQUEST) &&

        (s->session != NULL) && (s->session->cipher != NULL)) {

        s->rlayer.d->handshake_fragment_len = 0;



        if ((s->rlayer.d->handshake_fragment[1] != 0) ||

            (s->rlayer.d->handshake_fragment[2] != 0) ||

            (s->rlayer.d->handshake_fragment[3] != 0)) {

            al = SSL_AD_DECODE_ERROR;

            SSLerr(SSL_F_DTLS1_READ_BYTES, SSL_R_BAD_HELLO_REQUEST);

            goto f_err;

        }



        /*

         * no need to check sequence number on HELLO REQUEST messages

         */



        if (s->msg_callback)

            s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE,

                            s->rlayer.d->handshake_fragment, 4, s,

                            s->msg_callback_arg);



        if (SSL_is_init_finished(s) &&

            !s->s3->renegotiate) {

            s->d1->handshake_read_seq++;

            s->new_session = 1;

            ssl3_renegotiate(s);

            if (ssl3_renegotiate_check(s)) {

                i = s->handshake_func(s);

                if (i < 0)

                    return i;

                if (i == 0) {

                    SSLerr(SSL_F_DTLS1_READ_BYTES, SSL_R_SSL_HANDSHAKE_FAILURE);

                    return -1;

                }



                if (!(s->mode & SSL_MODE_AUTO_RETRY)) {

                    if (SSL3_BUFFER_get_left(&s->rlayer.rbuf) == 0) {

                        /* no read-ahead left? */

                        BIO *bio;

                        /*

                         * In the case where we try to read application data,

                         * but we trigger an SSL handshake, we return -1 with

                         * the retry option set.  Otherwise renegotiation may

                         * cause nasty problems in the blocking world

                         */

                        s->rwstate = SSL_READING;

                        bio = SSL_get_rbio(s);

                        BIO_clear_retry_flags(bio);

                        BIO_set_retry_read(bio);

                        return -1;

                    }

                }

            }

        }

        /*

         * we either finished a handshake or ignored the request, now try

         * again to obtain the (application) data we were asked for

         */

        goto start;

    }



    if (s->rlayer.d->alert_fragment_len >= DTLS1_AL_HEADER_LENGTH) {

        int alert_level = s->rlayer.d->alert_fragment[0];

        int alert_descr = s->rlayer.d->alert_fragment[1];
@@ -837,11 +774,22 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
            goto start;

        }



        if (SSL_is_init_finished(s)) {

            ossl_statem_set_in_init(s, 1);

            s->renegotiate = 1;

            s->new_session = 1;

        /*

         * To get here we must be trying to read app data but found handshake

         * data. But if we're trying to read app data, and we're not in init

         * (which is tested for at the top of this function) then init must be

         * finished

         */

        assert(SSL_is_init_finished(s));

        if (!SSL_is_init_finished(s)) {

            al = SSL_AD_INTERNAL_ERROR;

            SSLerr(SSL_F_DTLS1_READ_BYTES, ERR_R_INTERNAL_ERROR);

            goto f_err;

        }



        /* We found handshake data, so we're going back into init */

        ossl_statem_set_in_init(s, 1);



        i = s->handshake_func(s);

        if (i < 0)

            return i;

ssl/record/rec_layer_s3.c

+18 −71
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ 
 */



#include <stdio.h>

#include <assert.h>

#include <limits.h>

#include <errno.h>

#define USE_SOCKETS
@@ -1387,69 +1388,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
     * (Possibly rr is 'empty' now, i.e. rr->length may be 0.)

     */



    /* If we are a client, check for an incoming 'Hello Request': */

    if ((!s->server) &&

        (s->rlayer.handshake_fragment_len >= 4) &&

        !SSL_IS_TLS13(s) &&

        (s->rlayer.handshake_fragment[0] == SSL3_MT_HELLO_REQUEST) &&

        (s->session != NULL) && (s->session->cipher != NULL)) {

        s->rlayer.handshake_fragment_len = 0;



        if ((s->rlayer.handshake_fragment[1] != 0) ||

            (s->rlayer.handshake_fragment[2] != 0) ||

            (s->rlayer.handshake_fragment[3] != 0)) {

            al = SSL_AD_DECODE_ERROR;

            SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_BAD_HELLO_REQUEST);

            goto f_err;

        }



        if (s->msg_callback)

            s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE,

                            s->rlayer.handshake_fragment, 4, s,

                            s->msg_callback_arg);



        if (SSL_is_init_finished(s) &&

            !s->s3->renegotiate) {

            ssl3_renegotiate(s);

            if (ssl3_renegotiate_check(s)) {

                i = s->handshake_func(s);

                if (i < 0)

                    return i;

                if (i == 0) {

                    SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_SSL_HANDSHAKE_FAILURE);

                    return -1;

                }



                if (!(s->mode & SSL_MODE_AUTO_RETRY)) {

                    if (SSL3_BUFFER_get_left(rbuf) == 0) {

                        /* no read-ahead left? */

                        BIO *bio;

                        /*

                         * In the case where we try to read application data,

                         * but we trigger an SSL handshake, we return -1 with

                         * the retry option set.  Otherwise renegotiation may

                         * cause nasty problems in the blocking world

                         */

                        s->rwstate = SSL_READING;

                        bio = SSL_get_rbio(s);

                        BIO_clear_retry_flags(bio);

                        BIO_set_retry_read(bio);

                        return -1;

                    }

                }

            } else {

                SSL3_RECORD_set_read(rr);

            }

        } else {

            /* Does this ever happen? */

            SSL3_RECORD_set_read(rr);

        }

        /*

         * we either finished a handshake or ignored the request, now try

         * again to obtain the (application) data we were asked for

         */

        goto start;

    }

    /*

     * If we are a server and get a client hello when renegotiation isn't

     * allowed send back a no renegotiation alert and carry on. WARNING:
@@ -1563,13 +1501,22 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
     */

    if ((s->rlayer.handshake_fragment_len >= 4)

            && !ossl_statem_get_in_handshake(s)) {

        if (SSL_is_init_finished(s)) {

            ossl_statem_set_in_init(s, 1);

            if (!SSL_IS_TLS13(s)) {

                s->renegotiate = 1;

                s->new_session = 1;

            }

        /*

         * To get here we must be trying to read app data but found handshake

         * data. But if we're trying to read app data, and we're not in init

         * (which is tested for at the top of this function) then init must be

         * finished

         */

        assert(SSL_is_init_finished(s));

        if (!SSL_is_init_finished(s)) {

            al = SSL_AD_INTERNAL_ERROR;

            SSLerr(SSL_F_SSL3_READ_BYTES, ERR_R_INTERNAL_ERROR);

            goto f_err;

        }



        /* We found handshake data, so we're going back into init */

        ossl_statem_set_in_init(s, 1);



        i = s->handshake_func(s);

        if (i < 0)

            return i;

ssl/s3_lib.c

+13 −5
Original line number Diff line number Diff line
@@ -3822,7 +3822,7 @@ int ssl3_write(SSL *s, const void *buf, size_t len, size_t *written) 
{

    clear_sys_error();

    if (s->s3->renegotiate)

        ssl3_renegotiate_check(s);

        ssl3_renegotiate_check(s, 0);



    return s->method->ssl_write_bytes(s, SSL3_RT_APPLICATION_DATA, buf, len,

                                      written);
@@ -3835,7 +3835,7 @@ static int ssl3_read_internal(SSL *s, void *buf, size_t len, int peek, 


    clear_sys_error();

    if (s->s3->renegotiate)

        ssl3_renegotiate_check(s);

        ssl3_renegotiate_check(s, 0);

    s->s3->in_read_app_data = 1;

    ret =

        s->method->ssl_read_bytes(s, SSL3_RT_APPLICATION_DATA, NULL, buf, len,
@@ -3878,14 +3878,22 @@ int ssl3_renegotiate(SSL *s) 
    return (1);

}



int ssl3_renegotiate_check(SSL *s)

/*

 * Check if we are waiting to do a renegotiation and if so whether now is a

 * good time to do it. If |initok| is true then we are being called from inside

 * the state machine so ignore the result of SSL_in_init(s). Otherwise we

 * should not do a renegotiation if SSL_in_init(s) is true. Returns 1 if we

 * should do a renegotiation now and sets up the state machine for it. Otherwise

 * returns 0.

 */

int ssl3_renegotiate_check(SSL *s, int initok)

{

    int ret = 0;



    if (s->s3->renegotiate) {

        if (!RECORD_LAYER_read_pending(&s->rlayer)

            && !RECORD_LAYER_write_pending(&s->rlayer)

            && !SSL_in_init(s)) {

            && (initok || !SSL_in_init(s))) {

            /*

             * if we are the server, and we have sent a 'RENEGOTIATE'

             * message, we need to set the state machine into the renegotiate
@@ -3898,7 +3906,7 @@ int ssl3_renegotiate_check(SSL *s) 
            ret = 1;

        }

    }

    return (ret);

    return ret;

}



/*

ssl/ssl_err.c

+2 −0
Original line number Diff line number Diff line
@@ -402,6 +402,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { 
    {ERR_FUNC(SSL_F_TLS_PROCESS_ENCRYPTED_EXTENSIONS),

     "tls_process_encrypted_extensions"},

    {ERR_FUNC(SSL_F_TLS_PROCESS_FINISHED), "tls_process_finished"},

    {ERR_FUNC(SSL_F_TLS_PROCESS_HELLO_REQ), "tls_process_hello_req"},

    {ERR_FUNC(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT),

     "tls_process_initial_server_flight"},

    {ERR_FUNC(SSL_F_TLS_PROCESS_KEY_EXCHANGE), "tls_process_key_exchange"},
@@ -419,6 +420,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { 
    {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_SRP), "tls_process_ske_srp"},

    {ERR_FUNC(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT),

     "tls_scan_clienthello_tlsext"},

    {ERR_FUNC(SSL_F_TLS_SETUP_HANDSHAKE), "tls_setup_handshake"},

    {ERR_FUNC(SSL_F_USE_CERTIFICATE_CHAIN_FILE),

     "use_certificate_chain_file"},

    {0, NULL}