Commit 94ed2c67 authored by Matt Caswell's avatar Matt Caswell
Browse files

Fixed various style issues in the key_share code 



Numerous style issues as well as references to TLS1_3_VERSION instead of
SSL_IS_TLS13(s)

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent 5a8e54d9

include/openssl/ssl.h

+2 −0
Original line number Diff line number Diff line
@@ -2074,6 +2074,7 @@ int ERR_load_SSL_strings(void); 
/* Error codes for the SSL functions. */



/* Function codes. */

# define SSL_F_ADD_CLIENT_KEY_SHARE_EXT                   438

# define SSL_F_CHECK_SUITEB_CIPHER_LIST                   331

# define SSL_F_CT_MOVE_SCTS                               345

# define SSL_F_CT_STRICT                                  349
@@ -2105,6 +2106,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_F_OSSL_STATEM_SERVER13_READ_TRANSITION       437

# define SSL_F_OSSL_STATEM_SERVER_CONSTRUCT_MESSAGE       431

# define SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION         418

# define SSL_F_PROCESS_KEY_SHARE_EXT                      439

# define SSL_F_READ_STATE_MACHINE                         352

# define SSL_F_SSL3_CHANGE_CIPHER_STATE                   129

# define SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM              130

ssl/ssl_err.c

+2 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
# define ERR_REASON(reason) ERR_PACK(ERR_LIB_SSL,0,reason)



static ERR_STRING_DATA SSL_str_functs[] = {

    {ERR_FUNC(SSL_F_ADD_CLIENT_KEY_SHARE_EXT), "add_client_key_share_ext"},

    {ERR_FUNC(SSL_F_CHECK_SUITEB_CIPHER_LIST), "check_suiteb_cipher_list"},

    {ERR_FUNC(SSL_F_CT_MOVE_SCTS), "ct_move_scts"},

    {ERR_FUNC(SSL_F_CT_STRICT), "ct_strict"},
@@ -61,6 +62,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { 
     "ossl_statem_server_construct_message"},

    {ERR_FUNC(SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION),

     "ossl_statem_server_read_transition"},

    {ERR_FUNC(SSL_F_PROCESS_KEY_SHARE_EXT), "process_key_share_ext"},

    {ERR_FUNC(SSL_F_READ_STATE_MACHINE), "read_state_machine"},

    {ERR_FUNC(SSL_F_SSL3_CHANGE_CIPHER_STATE), "ssl3_change_cipher_state"},

    {ERR_FUNC(SSL_F_SSL3_CHECK_CERT_AND_ALGORITHM),

ssl/statem/statem_clnt.c

+22 −30
Original line number Diff line number Diff line
@@ -112,14 +112,18 @@ static int key_exchange_expected(SSL *s) 
 * server. The message type that the server has sent is provided in |mt|. The

 * current state is in |s->statem.hand_state|.

 *

 *  Return values are:

 *  1: Success (transition allowed)

 *  0: Error (transition not allowed)

 * Return values are 1 for success (transition allowed) and  0 on error

 * (transition not allowed)

 */

static int ossl_statem_client13_read_transition(SSL *s, int mt)

{

    OSSL_STATEM *st = &s->statem;



    /*

     * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time

     * we will update this to look more like real TLSv1.3

     */



    /*

     * Note: There is no case for TLS_ST_CW_CLNT_HELLO, because we haven't

     * yet negotiated TLSv1.3 at that point so that is handled by
@@ -218,9 +222,8 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt) 
 * server. The message type that the server has sent is provided in |mt|. The

 * current state is in |s->statem.hand_state|.

 *

 *  Return values are:

 *  1: Success (transition allowed)

 *  0: Error (transition not allowed)

 * Return values are 1 for success (transition allowed) and  0 on error

 * (transition not allowed)

 */

int ossl_statem_client_read_transition(SSL *s, int mt)

{
@@ -387,16 +390,16 @@ int ossl_statem_client_read_transition(SSL *s, int mt) 
 * ossl_statem_client13_write_transition() works out what handshake state to

 * move to next when the TLSv1.3 client is writing messages to be sent to the

 * server.

 *

 * Return values:

 * WRITE_TRAN_ERROR - an error occurred

 * WRITE_TRAN_CONTINUE - Successful transition, more writing to be done

 * WRITE_TRAN_FINISHED - Successful transition, no more writing to be done

 */

static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)

{

    OSSL_STATEM *st = &s->statem;



    /*

     * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time

     * we will update this to look more like real TLSv1.3

     */



    /*

     * Note: There are no cases for TLS_ST_BEFORE or TLS_ST_CW_CLNT_HELLO,

     * because we haven't negotiated TLSv1.3 yet at that point. They are
@@ -408,18 +411,14 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s) 
        return WRITE_TRAN_ERROR;



    case TLS_ST_CR_SRVR_DONE:

        if (s->s3->tmp.cert_req)

            st->hand_state = TLS_ST_CW_CERT;

        else

            st->hand_state = TLS_ST_CW_CHANGE;

        st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT

                                                    : TLS_ST_CW_CHANGE;

        return WRITE_TRAN_CONTINUE;



    case TLS_ST_CW_CERT:

        /* If a non-empty Certificate we also send CertificateVerify */

        if (s->s3->tmp.cert_req == 1)

            st->hand_state = TLS_ST_CW_CERT_VRFY;

        else

            st->hand_state = TLS_ST_CW_CHANGE;

        st->hand_state = (s->s3->tmp.cert_req == 1) ? TLS_ST_CW_CERT_VRFY

                                                    : TLS_ST_CW_CHANGE;

        return WRITE_TRAN_CONTINUE;



    case TLS_ST_CW_CERT_VRFY:
@@ -435,30 +434,23 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s) 
            st->hand_state = TLS_ST_OK;

            ossl_statem_set_in_init(s, 0);

            return WRITE_TRAN_CONTINUE;

        } else {

            return WRITE_TRAN_FINISHED;

        }

        return WRITE_TRAN_FINISHED;



    case TLS_ST_CR_FINISHED:

        if (s->hit) {

            st->hand_state = TLS_ST_CW_CHANGE;

            return WRITE_TRAN_CONTINUE;

        } else {

        }

        st->hand_state = TLS_ST_OK;

        ossl_statem_set_in_init(s, 0);

        return WRITE_TRAN_CONTINUE;

    }

}

}



/*

 * ossl_statem_client_write_transition() works out what handshake state to

 * move to next when the client is writing messages to be sent to the server.

 *

 * Return values:

 * WRITE_TRAN_ERROR - an error occurred

 * WRITE_TRAN_CONTINUE - Successful transition, more writing to be done

 * WRITE_TRAN_FINISHED - Successful transition, no more writing to be done

 */

WRITE_TRAN ossl_statem_client_write_transition(SSL *s)

{

ssl/statem/statem_srvr.c

+26 −31
Original line number Diff line number Diff line
@@ -73,14 +73,18 @@ static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, 
 * the client. The message type that the client has sent is provided in |mt|.

 * The current state is in |s->statem.hand_state|.

 *

 *  Valid return values are:

 *  1: Success (transition allowed)

 *  0: Error (transition not allowed)

 * Return values are 1 for success (transition allowed) and  0 on error

 * (transition not allowed)

 */

static int ossl_statem_server13_read_transition(SSL *s, int mt)

{

    OSSL_STATEM *st = &s->statem;



    /*

     * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time

     * we will update this to look more like real TLSv1.3

     */



    /*

     * Note: There is no case for TLS_ST_BEFORE because at that stage we have

     * not negotiated TLSv1.3 yet, so that case is handled by
@@ -153,9 +157,8 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) 
 * client. The message type that the client has sent is provided in |mt|. The

 * current state is in |s->statem.hand_state|.

 *

 *  Valid return values are:

 *  1: Success (transition allowed)

 *  0: Error (transition not allowed)

 * Return values are 1 for success (transition allowed) and  0 on error

 * (transition not allowed)

 */

int ossl_statem_server_read_transition(SSL *s, int mt)

{
@@ -390,16 +393,16 @@ static int send_certificate_request(SSL *s) 
 * ossl_statem_server13_write_transition() works out what handshake state to

 * move to next when a TLSv1.3 server is writing messages to be sent to the

 * client.

 *

 * Return values:

 * WRITE_TRAN_ERROR - an error occurred

 * WRITE_TRAN_CONTINUE - Successful transition, more writing to be done

 * WRITE_TRAN_FINISHED - Successful transition, no more writing to be done

 */

static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)

{

    OSSL_STATEM *st = &s->statem;



    /*

     * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time

     * we will update this to look more like real TLSv1.3

     */



    /*

     * No case for TLS_ST_BEFORE, because at that stage we have not negotiated

     * TLSv1.3 yet, so that is handled by ossl_statem_server_write_transition()
@@ -415,14 +418,12 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) 
        return WRITE_TRAN_CONTINUE;



    case TLS_ST_SW_SRVR_HELLO:

        if (s->hit) {

            if (s->tlsext_ticket_expected)

                st->hand_state = TLS_ST_SW_SESSION_TICKET;

        if (s->hit)

            st->hand_state = s->tlsext_ticket_expected

                                ? TLS_ST_SW_SESSION_TICKET : TLS_ST_SW_CHANGE;

        else

                st->hand_state = TLS_ST_SW_CHANGE;

        } else {

            st->hand_state = TLS_ST_SW_CERT;

        }



        return WRITE_TRAN_CONTINUE;



    case TLS_ST_SW_CERT:
@@ -451,11 +452,10 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) 
            st->hand_state = TLS_ST_OK;

            ossl_statem_set_in_init(s, 0);

            return WRITE_TRAN_CONTINUE;

        } else if (s->tlsext_ticket_expected) {

            st->hand_state = TLS_ST_SW_SESSION_TICKET;

        } else {

            st->hand_state = TLS_ST_SW_CHANGE;

        }



        st->hand_state = s->tlsext_ticket_expected ? TLS_ST_SW_SESSION_TICKET

                                                   : TLS_ST_SW_CHANGE;

        return WRITE_TRAN_CONTINUE;



    case TLS_ST_SW_SESSION_TICKET:
@@ -467,9 +467,9 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) 
        return WRITE_TRAN_CONTINUE;



    case TLS_ST_SW_FINISHED:

        if (s->hit) {

        if (s->hit)

            return WRITE_TRAN_FINISHED;

        }



        st->hand_state = TLS_ST_OK;

        ossl_statem_set_in_init(s, 0);

        return WRITE_TRAN_CONTINUE;
@@ -479,11 +479,6 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) 
/*

 * ossl_statem_server_write_transition() works out what handshake state to move

 * to next when the server is writing messages to be sent to the client.

 *

 * Return values:

 * WRITE_TRAN_ERROR - an error occurred

 * WRITE_TRAN_CONTINUE - Successful transition, more writing to be done

 * WRITE_TRAN_FINISHED - Successful transition, no more writing to be done

 */

WRITE_TRAN ossl_statem_server_write_transition(SSL *s)

{
@@ -1452,7 +1447,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) 
    /* Check we've got a key_share for TLSv1.3 */

    if (s->version == TLS1_3_VERSION && s->s3->peer_tmp == NULL && !s->hit) {

        /* No suitable share */

        /* TODO(1.3): Send a HelloRetryRequest */

        /* TODO(TLS1.3): Send a HelloRetryRequest */

        al = SSL_AD_HANDSHAKE_FAILURE;

        SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_NO_SUITABLE_KEY_SHARE);

        goto f_err;
@@ -3123,7 +3118,7 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt) 
     * Freeze the handshake buffer. For <TLS1.3 we do this after the CKE

     * message

     */

    if (s->version == TLS1_3_VERSION && !ssl3_digest_cached_records(s, 1)) {

    if (SSL_IS_TLS13(s) && !ssl3_digest_cached_records(s, 1)) {

        al = SSL_AD_INTERNAL_ERROR;

        SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_INTERNAL_ERROR);

        goto f_err;

ssl/t1_enc.c

+1 −1
Original line number Diff line number Diff line
@@ -480,7 +480,7 @@ int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, 
     * handshake has). This will need to be removed later

     */

    if ((s->session->flags & SSL_SESS_FLAG_EXTMS)

            && s->version != TLS1_3_VERSION) {

            && SSL_IS_TLS13(s)) {

        unsigned char hash[EVP_MAX_MD_SIZE * 2];

        size_t hashlen;

        /*