Commit 7b1ec1cf authored by Matt Caswell's avatar Matt Caswell
Browse files

Fix HRR bug 



If an HRR gets sent without a key_share (e.g. cookie only) then the code
fails when it should not.

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

ssl/statem/extensions_clnt.c

+25 −17
Original line number Original line Diff line number Diff line
@@ -530,32 +530,41 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, unsigned int context, 
#ifndef OPENSSL_NO_TLS1_3

#ifndef OPENSSL_NO_TLS1_3

static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)

static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)

{

{

    unsigned char *encoded_point;

    unsigned char *encoded_point = NULL;

    EVP_PKEY *key_share_key;

    EVP_PKEY *key_share_key = NULL;

    size_t encodedlen;

    size_t encodedlen;





    if (s->s3->tmp.pkey != NULL) {

        assert(s->hello_retry_request);

        if (!s->hello_retry_request) {

            SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);

            return 0;

        }

        /*

         * Could happen if we got an HRR that wasn't requesting a new key_share

         */

        key_share_key = s->s3->tmp.pkey;

    } else {

        key_share_key = ssl_generate_pkey_curve(curve_id);

        key_share_key = ssl_generate_pkey_curve(curve_id);

        if (key_share_key == NULL) {

        if (key_share_key == NULL) {

            SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);

            SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);

            return 0;

            return 0;

        }

        }

    }





    /* Encode the public key. */

    /* Encode the public key. */

    encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key,

    encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key,

                                                &encoded_point);

                                                &encoded_point);

    if (encodedlen == 0) {

    if (encodedlen == 0) {

        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB);

        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB);

        EVP_PKEY_free(key_share_key);

        goto err;

        return 0;

    }

    }





    /* Create KeyShareEntry */

    /* Create KeyShareEntry */

    if (!WPACKET_put_bytes_u16(pkt, curve_id)

    if (!WPACKET_put_bytes_u16(pkt, curve_id)

            || !WPACKET_sub_memcpy_u16(pkt, encoded_point, encodedlen)) {

            || !WPACKET_sub_memcpy_u16(pkt, encoded_point, encodedlen)) {

        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);

        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);

        EVP_PKEY_free(key_share_key);

        goto err;

        OPENSSL_free(encoded_point);

        return 0;

    }

    }





    /*

    /*
@@ -568,6 +577,11 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id) 
    OPENSSL_free(encoded_point);

    OPENSSL_free(encoded_point);





    return 1;

    return 1;

 err:

    if (s->s3->tmp.pkey == NULL)

        EVP_PKEY_free(key_share_key);

    OPENSSL_free(encoded_point);

    return 0;

}

}

#endif

#endif
@@ -594,12 +608,6 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context, 
        return 0;

        return 0;

    }

    }





    if (s->s3->tmp.pkey != NULL) {

        /* Shouldn't happen! */

        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);

        return 0;

    }



    /*

    /*

     * TODO(TLS1.3): Make the number of key_shares sent configurable. For

     * TODO(TLS1.3): Make the number of key_shares sent configurable. For

     * now, just send one

     * now, just send one