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: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2733)

5 files changed, 37 insertions, 62 deletions

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 6449f8c4f1..1df1afa02e 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -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;
 }
 
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index a75fb6564a..70aa697564 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -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);
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 26464a584d..ebfd459c82 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -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
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index bc35a3ea25..6e162fb305 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -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;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index f13c0ada93..8b31e84b63 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -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;