Commit 9e84a42d authored by Dr. Stephen Henson's avatar Dr. Stephen Henson
Browse files

Store groups as uint16_t 



Instead of storing supported groups in on-the-wire format store
them as parsed uint16_t values. This simplifies handling of groups
as the values can be directly used instead of being converted.

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

ssl/s3_lib.c

+4 −6
Original line number Diff line number Diff line
@@ -3602,25 +3602,23 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) 
#ifndef OPENSSL_NO_EC

    case SSL_CTRL_GET_GROUPS:

        {

            unsigned char *clist;

            uint16_t *clist;

            size_t clistlen;



            if (!s->session)

                return 0;

            clist = s->session->ext.supportedgroups;

            clistlen = s->session->ext.supportedgroups_len / 2;

            clistlen = s->session->ext.supportedgroups_len;

            if (parg) {

                size_t i;

                int *cptr = parg;

                unsigned int cid, nid;

                for (i = 0; i < clistlen; i++) {

                    n2s(clist, cid);

                    /* TODO(TLS1.3): Handle DH groups here */

                    nid = tls1_ec_curve_id2nid(cid, NULL);

                    int nid = tls1_ec_curve_id2nid(clist[i], NULL);

                    if (nid != 0)

                        cptr[i] = nid;

                    else

                        cptr[i] = TLSEXT_nid_unknown | cid;

                        cptr[i] = TLSEXT_nid_unknown | clist[i];

                }

            }

            return (int)clistlen;

ssl/ssl_lib.c

+2 −1
Original line number Diff line number Diff line
@@ -719,7 +719,8 @@ SSL *SSL_new(SSL_CTX *ctx) 
    if (ctx->ext.supportedgroups) {

        s->ext.supportedgroups =

            OPENSSL_memdup(ctx->ext.supportedgroups,

                           ctx->ext.supportedgroups_len);

                           ctx->ext.supportedgroups_len

                                * sizeof(ctx->ext.supportedgroups));

        if (!s->ext.supportedgroups)

            goto err;

        s->ext.supportedgroups_len = ctx->ext.supportedgroups_len;

ssl/ssl_locl.h

+11 −12
Original line number Diff line number Diff line
@@ -543,7 +543,7 @@ struct ssl_session_st { 
        size_t ecpointformats_len;

        unsigned char *ecpointformats; /* peer's list */

        size_t supportedgroups_len;

        unsigned char *supportedgroups; /* peer's list */

        uint16_t *supportedgroups; /* peer's list */

# endif                         /* OPENSSL_NO_EC */

    /* RFC4507 info */

        unsigned char *tick; /* Session ticket */
@@ -902,7 +902,7 @@ struct ssl_ctx_st { 
        size_t ecpointformats_len;

        unsigned char *ecpointformats;

        size_t supportedgroups_len;

        unsigned char *supportedgroups;

        uint16_t *supportedgroups;

# endif                         /* OPENSSL_NO_EC */



        /*
@@ -1205,7 +1205,7 @@ struct ssl_st { 
        unsigned char *ecpointformats;

        size_t supportedgroups_len;

        /* our list */

        unsigned char *supportedgroups;

        uint16_t *supportedgroups;

# endif                         /* OPENSSL_NO_EC */

        /* TLS Session Ticket extension override */

        TLS_SESSION_TICKET_EXT *session_ticket;
@@ -1523,7 +1523,7 @@ typedef struct ssl3_state_st { 
    /* For clients: peer temporary key */

# if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH)

    /* The group_id for the DH/ECDH key */

    unsigned int group_id;

    uint16_t group_id;

    EVP_PKEY *peer_tmp;

# endif
@@ -2333,15 +2333,13 @@ SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n); 
# define TLS_CURVE_CHAR2         0x1

# define TLS_CURVE_CUSTOM        0x2



#define bytestogroup(bytes) ((unsigned int)(bytes[0] << 8 | bytes[1]))



__owur int tls1_ec_curve_id2nid(int curve_id, unsigned int *pflags);

__owur int tls1_ec_nid2curve_id(int nid);

__owur int tls1_ec_curve_id2nid(uint16_t curve_id, unsigned int *pflags);

__owur uint16_t tls1_ec_nid2curve_id(int nid);

__owur int tls1_check_curve(SSL *s, const unsigned char *p, size_t len);

__owur int tls1_shared_group(SSL *s, int nmatch);

__owur int tls1_set_groups(unsigned char **pext, size_t *pextlen,

__owur int tls1_set_groups(uint16_t **pext, size_t *pextlen,

                           int *curves, size_t ncurves);

__owur int tls1_set_groups_list(unsigned char **pext, size_t *pextlen,

__owur int tls1_set_groups_list(uint16_t **pext, size_t *pextlen,

                                const char *str);

void tls1_get_formatlist(SSL *s, const unsigned char **pformats,

                         size_t *num_formats);
@@ -2349,8 +2347,8 @@ __owur int tls1_check_ec_tmp_key(SSL *s, unsigned long id); 
__owur EVP_PKEY *ssl_generate_pkey_curve(int id);

#  endif                        /* OPENSSL_NO_EC */



__owur int tls_curve_allowed(SSL *s, const unsigned char *curve, int op);

__owur  int tls1_get_curvelist(SSL *s, int sess, const unsigned char **pcurves,

__owur int tls_curve_allowed(SSL *s, uint16_t curve, int op);

__owur  int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves,

                               size_t *num_curves);



__owur int tls1_set_server_sigalgs(SSL *s);
@@ -2410,6 +2408,7 @@ void ssl_clear_hash_ctx(EVP_MD_CTX **hash); 
__owur long ssl_get_algorithm2(SSL *s);

__owur int tls12_copy_sigalgs(SSL *s, WPACKET *pkt,

                              const uint16_t *psig, size_t psiglen);

__owur int tls1_save_u16(PACKET *pkt, uint16_t **pdest, size_t *pdestlen);

__owur int tls1_save_sigalgs(SSL *s, PACKET *pkt);

__owur int tls1_process_sigalgs(SSL *s);

__owur int tls1_set_peer_legacy_sigalg(SSL *s, const EVP_PKEY *pkey);

ssl/statem/extensions.c

+3 −4
Original line number Diff line number Diff line
@@ -1137,7 +1137,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) 
                && (!s->hit

                    || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)

                       != 0)) {

            const unsigned char *pcurves, *pcurvestmp, *clntcurves;

            const uint16_t *pcurves, *clntcurves;

            size_t num_curves, clnt_num_curves, i;

            unsigned int group_id = 0;
@@ -1158,9 +1158,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) 
            }



            /* Find the first group we allow that is also in client's list */

            for (i = 0, pcurvestmp = pcurves; i < num_curves;

                 i++, pcurvestmp += 2) {

                group_id = bytestogroup(pcurvestmp);

            for (i = 0; i < num_curves; i++) {

                group_id = pcurves[i];



                if (check_in_list(s, group_id, clntcurves, clnt_num_curves, 1))

                    break;

ssl/statem/extensions_clnt.c

+15 −15
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, 
                                               unsigned int context, X509 *x,

                                               size_t chainidx, int *al)

{

    const unsigned char *pcurves = NULL, *pcurvestmp;

    const uint16_t *pcurves = NULL;

    size_t num_curves = 0, i;



    if (!use_ecc(s))
@@ -154,7 +154,6 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, 
               ERR_R_INTERNAL_ERROR);

        return EXT_RETURN_FAIL;

    }

    pcurvestmp = pcurves;



    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)

               /* Sub-packet for supported_groups extension */
@@ -165,10 +164,11 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, 
        return EXT_RETURN_FAIL;

    }

    /* Copy curve ID if supported */

    for (i = 0; i < num_curves; i++, pcurvestmp += 2) {

        if (tls_curve_allowed(s, pcurvestmp, SSL_SECOP_CURVE_SUPPORTED)) {

            if (!WPACKET_put_bytes_u8(pkt, pcurvestmp[0])

                || !WPACKET_put_bytes_u8(pkt, pcurvestmp[1])) {

    for (i = 0; i < num_curves; i++) {

        uint16_t ctmp = pcurves[i];



        if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {

            if (!WPACKET_put_bytes_u16(pkt, ctmp)) {

                    SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,

                           ERR_R_INTERNAL_ERROR);

                    return EXT_RETURN_FAIL;
@@ -595,8 +595,8 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, 
{

#ifndef OPENSSL_NO_TLS1_3

    size_t i, num_curves = 0;

    const unsigned char *pcurves = NULL;

    unsigned int curve_id = 0;

    const uint16_t *pcurves = NULL;

    uint16_t curve_id = 0;



    /* key_share extension */

    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
@@ -620,12 +620,12 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, 
    if (s->s3->group_id != 0) {

        curve_id = s->s3->group_id;

    } else {

        for (i = 0; i < num_curves; i++, pcurves += 2) {

        for (i = 0; i < num_curves; i++) {



            if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED))

            if (!tls_curve_allowed(s, pcurves[i], SSL_SECOP_CURVE_SUPPORTED))

                continue;



            curve_id = bytestogroup(pcurves);

            curve_id = pcurves[i];

            break;

        }

    }
@@ -1521,7 +1521,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, 
    }



    if ((context & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) {

        unsigned const char *pcurves = NULL;

        const uint16_t *pcurves = NULL;

        size_t i, num_curves;



        if (PACKET_remaining(pkt) != 0) {
@@ -1545,12 +1545,12 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, 
            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);

            return 0;

        }

        for (i = 0; i < num_curves; i++, pcurves += 2) {

            if (group_id == bytestogroup(pcurves))

        for (i = 0; i < num_curves; i++) {

            if (group_id == pcurves[i])

                break;

        }

        if (i >= num_curves

                || !tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) {

                || !tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_SUPPORTED)) {

            *al = SSL_AD_ILLEGAL_PARAMETER;

            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);

            return 0;