Commit 75c13e78 authored by Dr. Stephen Henson's avatar Dr. Stephen Henson
Browse files

Tidy up certificate type handling. 



The certificate types used to be held in a fixed length array or (if
it was too long) a malloced buffer. This was done to retain binary
compatibility. The code can be simplified now SSL is opaque by always
using a malloced buffer.

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

ssl/s3_lib.c

+13 −17
Original line number Diff line number Diff line
@@ -2925,6 +2925,7 @@ void ssl3_free(SSL *s) 
    s->s3->tmp.pkey = NULL;

#endif



    OPENSSL_free(s->s3->tmp.ctype);

    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);

    OPENSSL_free(s->s3->tmp.ciphers_raw);

    OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
@@ -2943,6 +2944,7 @@ void ssl3_free(SSL *s) 
void ssl3_clear(SSL *s)

{

    ssl3_cleanup_key_block(s);

    OPENSSL_free(s->s3->tmp.ctype);

    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);

    OPENSSL_free(s->s3->tmp.ciphers_raw);

    OPENSSL_clear_free(s->s3->tmp.pms, s->s3->tmp.pmslen);
@@ -3234,14 +3236,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) 
            const unsigned char **pctype = parg;

            if (s->server || !s->s3->tmp.cert_req)

                return 0;

            if (s->cert->ctypes) {

            if (pctype)

                    *pctype = s->cert->ctypes;

                return (int)s->cert->ctype_num;

            }

            if (pctype)

                *pctype = (unsigned char *)s->s3->tmp.ctype;

            return s->s3->tmp.ctype_num;

                *pctype = s->s3->tmp.ctype;

            return s->s3->tmp.ctype_len;

        }



    case SSL_CTRL_SET_CLIENT_CERT_TYPES:
@@ -3787,9 +3784,8 @@ int ssl3_get_req_cert_type(SSL *s, WPACKET *pkt) 
    uint32_t alg_k, alg_a = 0;



    /* If we have custom certificate types set, use them */

    if (s->cert->ctypes) {

        return WPACKET_memcpy(pkt, s->cert->ctypes, s->cert->ctype_num);

    }

    if (s->cert->ctype)

        return WPACKET_memcpy(pkt, s->cert->ctype, s->cert->ctype_len);

    /* Get mask of algorithms disabled by signature list */

    ssl_set_sig_mask(&alg_a, s, SSL_SECOP_SIGALG_MASK);
@@ -3837,17 +3833,17 @@ int ssl3_get_req_cert_type(SSL *s, WPACKET *pkt) 


static int ssl3_set_req_cert_type(CERT *c, const unsigned char *p, size_t len)

{

    OPENSSL_free(c->ctypes);

    c->ctypes = NULL;

    if (!p || !len)

    OPENSSL_free(c->ctype);

    c->ctype = NULL;

    c->ctype_len = 0;

    if (p == NULL || len == 0)

        return 1;

    if (len > 0xff)

        return 0;

    c->ctypes = OPENSSL_malloc(len);

    if (c->ctypes == NULL)

    c->ctype = OPENSSL_memdup(p, len);

    if (c->ctype == NULL)

        return 0;

    memcpy(c->ctypes, p, len);

    c->ctype_num = len;

    c->ctype_len = len;

    return 1;

}

ssl/ssl_cert.c

+5 −6
Original line number Diff line number Diff line
@@ -164,12 +164,11 @@ CERT *ssl_cert_dup(CERT *cert) 
    /* Shared sigalgs also NULL */

    ret->shared_sigalgs = NULL;

    /* Copy any custom client certificate types */

    if (cert->ctypes) {

        ret->ctypes = OPENSSL_malloc(cert->ctype_num);

        if (ret->ctypes == NULL)

    if (cert->ctype) {

        ret->ctype = OPENSSL_memdup(cert->ctype, cert->ctype_len);

        if (ret->ctype == NULL)

            goto err;

        memcpy(ret->ctypes, cert->ctypes, cert->ctype_num);

        ret->ctype_num = cert->ctype_num;

        ret->ctype_len = cert->ctype_len;

    }



    ret->cert_flags = cert->cert_flags;
@@ -252,7 +251,7 @@ void ssl_cert_free(CERT *c) 
    OPENSSL_free(c->conf_sigalgs);

    OPENSSL_free(c->client_sigalgs);

    OPENSSL_free(c->shared_sigalgs);

    OPENSSL_free(c->ctypes);

    OPENSSL_free(c->ctype);

    X509_STORE_free(c->verify_store);

    X509_STORE_free(c->chain_store);

    custom_exts_free(&c->cli_ext);

ssl/ssl_locl.h

+6 −9
Original line number Diff line number Diff line
@@ -1316,8 +1316,9 @@ typedef struct ssl3_state_st { 
# endif

        /* used for certificate requests */

        int cert_req;

        int ctype_num;

        char ctype[SSL3_CT_NUMBER];

        /* Certificate types in certificate request message. */

        uint8_t *ctype;

        size_t ctype_len;

        STACK_OF(X509_NAME) *ca_names;

        size_t key_block_length;

        unsigned char *key_block;
@@ -1606,13 +1607,9 @@ typedef struct cert_st { 
    /* Flags related to certificates */

    uint32_t cert_flags;

    CERT_PKEY pkeys[SSL_PKEY_NUM];

    /*

     * Certificate types (received or sent) in certificate request message.

     * On receive this is only set if number of certificate types exceeds

     * SSL3_CT_NUMBER.

     */

    unsigned char *ctypes;

    size_t ctype_num;

    /* Custom certificate types sent in certificate request message. */

    uint8_t *ctype;

    size_t ctype_len;

    /*

     * supported signature algorithms. When set on a client this is sent in

     * the client hello as the supported signature algorithms extension. For

ssl/statem/statem_clnt.c

+6 −18
Original line number Diff line number Diff line
@@ -2196,11 +2196,11 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) 
MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)

{

    int ret = MSG_PROCESS_ERROR;

    unsigned int list_len, ctype_num, i, name_len;

    unsigned int list_len, i, name_len;

    X509_NAME *xn = NULL;

    const unsigned char *data;

    const unsigned char *namestart, *namebytes;

    STACK_OF(X509_NAME) *ca_sk = NULL;

    PACKET ctypes;



    if ((ca_sk = sk_X509_NAME_new(ca_dn_cmp)) == NULL) {

        SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);
@@ -2208,27 +2208,16 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) 
    }



    /* get the certificate types */

    if (!PACKET_get_1(pkt, &ctype_num)

        || !PACKET_get_bytes(pkt, &data, ctype_num)) {

    if (!PACKET_get_length_prefixed_1(pkt, &ctypes)) {

        ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);

        SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, SSL_R_LENGTH_MISMATCH);

        goto err;

    }

    OPENSSL_free(s->cert->ctypes);

    s->cert->ctypes = NULL;

    if (ctype_num > SSL3_CT_NUMBER) {

        /* If we exceed static buffer copy all to cert structure */

        s->cert->ctypes = OPENSSL_malloc(ctype_num);

        if (s->cert->ctypes == NULL) {

            SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE);



    if (!PACKET_memdup(&ctypes, &s->s3->tmp.ctype, &s->s3->tmp.ctype_len)) {

        SSLerr(SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);

            goto err;

    }

        memcpy(s->cert->ctypes, data, ctype_num);

        s->cert->ctype_num = ctype_num;

        ctype_num = SSL3_CT_NUMBER;

    }

    for (i = 0; i < ctype_num; i++)

        s->s3->tmp.ctype[i] = data[i];



    if (SSL_USE_SIGALGS(s)) {

        PACKET sigalgs;
@@ -2297,7 +2286,6 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) 


    /* we should setup a certificate to return.... */

    s->s3->tmp.cert_req = 1;

    s->s3->tmp.ctype_num = ctype_num;

    sk_X509_NAME_pop_free(s->s3->tmp.ca_names, X509_NAME_free);

    s->s3->tmp.ca_names = ca_sk;

    ca_sk = NULL;

ssl/t1_lib.c

+7 −12
Original line number Diff line number Diff line
@@ -2009,25 +2009,20 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain, 
            break;

        }

        if (check_type) {

            const unsigned char *ctypes;

            int ctypelen;

            if (c->ctypes) {

                ctypes = c->ctypes;

                ctypelen = (int)c->ctype_num;

            } else {

                ctypes = (unsigned char *)s->s3->tmp.ctype;

                ctypelen = s->s3->tmp.ctype_num;

            }

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

                if (ctypes[i] == check_type) {

            const uint8_t *ctypes = s->s3->tmp.ctype;

            size_t j;



            for (j = 0; j < s->s3->tmp.ctype_len; j++, ctypes++) {

                if (*ctypes == check_type) {

                    rv |= CERT_PKEY_CERT_TYPE;

                    break;

                }

            }

            if (!(rv & CERT_PKEY_CERT_TYPE) && !check_flags)

                goto end;

        } else

        } else {

            rv |= CERT_PKEY_CERT_TYPE;

        }



        ca_dn = s->s3->tmp.ca_names;