Convert CertificateRequest construction to WPACKET

Reviewed-by: Rich Salz <rsalz@openssl.org>

3 files changed, 61 insertions, 59 deletions

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 2115a7e0fa..ea607a51b9 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3708,15 +3708,13 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
     return (ret);
 }
 
-int ssl3_get_req_cert_type(SSL *s, unsigned char *p)
+int ssl3_get_req_cert_type(SSL *s, WPACKET *pkt)
 {
-    int ret = 0;
     uint32_t alg_k, alg_a = 0;
 
     /* If we have custom certificate types set, use them */
     if (s->cert->ctypes) {
-        memcpy(p, s->cert->ctypes, s->cert->ctype_num);
-        return (int)s->cert->ctype_num;
+        return WPACKET_memcpy(pkt, s->cert->ctypes, s->cert->ctype_num);
     }
     /* Get mask of algorithms disabled by signature list */
     ssl_set_sig_mask(&alg_a, s, SSL_SECOP_SIGALG_MASK);
@@ -3724,45 +3722,43 @@ int ssl3_get_req_cert_type(SSL *s, unsigned char *p)
     alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
 
 #ifndef OPENSSL_NO_GOST
-    if (s->version >= TLS1_VERSION) {
-        if (alg_k & SSL_kGOST) {
-            p[ret++] = TLS_CT_GOST01_SIGN;
-            p[ret++] = TLS_CT_GOST12_SIGN;
-            p[ret++] = TLS_CT_GOST12_512_SIGN;
-            return (ret);
-        }
-    }
+    if (s->version >= TLS1_VERSION && (alg_k & SSL_kGOST))
+            return WPACKET_put_bytes_u8(pkt, TLS_CT_GOST01_SIGN)
+                    && WPACKET_put_bytes_u8(pkt, TLS_CT_GOST12_SIGN)
+                    && WPACKET_put_bytes_u8(pkt, TLS_CT_GOST12_512_SIGN);
 #endif
 
     if ((s->version == SSL3_VERSION) && (alg_k & SSL_kDHE)) {
 #ifndef OPENSSL_NO_DH
 # ifndef OPENSSL_NO_RSA
-        p[ret++] = SSL3_CT_RSA_EPHEMERAL_DH;
+        if (!WPACKET_put_bytes_u8(pkt, SSL3_CT_RSA_EPHEMERAL_DH))
+            return 0;
 # endif
 # ifndef OPENSSL_NO_DSA
-        p[ret++] = SSL3_CT_DSS_EPHEMERAL_DH;
+        if (!WPACKET_put_bytes_u8(pkt, SSL3_CT_DSS_EPHEMERAL_DH))
+            return 0;
 # endif
 #endif                          /* !OPENSSL_NO_DH */
     }
 #ifndef OPENSSL_NO_RSA
-    if (!(alg_a & SSL_aRSA))
-        p[ret++] = SSL3_CT_RSA_SIGN;
+    if (!(alg_a & SSL_aRSA) && !WPACKET_put_bytes_u8(pkt, SSL3_CT_RSA_SIGN))
+        return 0;
 #endif
 #ifndef OPENSSL_NO_DSA
-    if (!(alg_a & SSL_aDSS))
-        p[ret++] = SSL3_CT_DSS_SIGN;
+    if (!(alg_a & SSL_aDSS) && !WPACKET_put_bytes_u8(pkt, SSL3_CT_DSS_SIGN))
+        return 0;
 #endif
 #ifndef OPENSSL_NO_EC
     /*
      * ECDSA certs can be used with RSA cipher suites too so we don't
      * need to check for SSL_kECDH or SSL_kECDHE
      */
-    if (s->version >= TLS1_VERSION) {
-        if (!(alg_a & SSL_aECDSA))
-            p[ret++] = TLS_CT_ECDSA_SIGN;
-    }
+    if (s->version >= TLS1_VERSION
+            && !(alg_a & SSL_aECDSA)
+            && !WPACKET_put_bytes_u8(pkt, TLS_CT_ECDSA_SIGN))
+        return 0;
 #endif
-    return (ret);
+    return 1;
 }
 
 static int ssl3_set_req_cert_type(CERT *c, const unsigned char *p, size_t len)
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 7dbff76eab..4fbea50a31 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1873,7 +1873,7 @@ __owur int ssl3_do_write(SSL *s, int type);
 int ssl3_send_alert(SSL *s, int level, int desc);
 __owur int ssl3_generate_master_secret(SSL *s, unsigned char *out,
                                        unsigned char *p, int len);
-__owur int ssl3_get_req_cert_type(SSL *s, unsigned char *p);
+__owur int ssl3_get_req_cert_type(SSL *s, WPACKET *pkt);
 __owur int ssl3_num_ciphers(void);
 __owur const SSL_CIPHER *ssl3_get_cipher(unsigned int u);
 int ssl3_renegotiate(SSL *ssl);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index bf50e79e83..799700b051 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1960,62 +1960,66 @@ int tls_construct_server_key_exchange(SSL *s)
 
 int tls_construct_certificate_request(SSL *s)
 {
-    unsigned char *p, *d;
-    int i, j, nl, off, n;
+    int i, nl;
     STACK_OF(X509_NAME) *sk = NULL;
-    X509_NAME *name;
-    BUF_MEM *buf;
+    WPACKET pkt;
 
-    buf = s->init_buf;
+    if (!WPACKET_init(&pkt, s->init_buf)
+            || !ssl_set_handshake_header2(s, &pkt,
+                                          SSL3_MT_CERTIFICATE_REQUEST)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
-    d = p = ssl_handshake_start(s);
 
     /* get the list of acceptable cert types */
-    p++;
-    n = ssl3_get_req_cert_type(s, p);
-    d[0] = n;
-    p += n;
-    n++;
+    if (!WPACKET_start_sub_packet_u8(&pkt)
+            || !ssl3_get_req_cert_type(s, &pkt)
+            || !WPACKET_close(&pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     if (SSL_USE_SIGALGS(s)) {
         const unsigned char *psigs;
-        unsigned char *etmp = p;
         nl = tls12_get_psigalgs(s, &psigs);
-        /* Skip over length for now */
-        p += 2;
-        nl = tls12_copy_sigalgs_old(s, p, psigs, nl);
-        /* Now fill in length */
-        s2n(nl, etmp);
-        p += nl;
-        n += nl + 2;
+        if (!WPACKET_start_sub_packet_u16(&pkt)
+                || !tls12_copy_sigalgs(s, &pkt, psigs, nl)
+                || !WPACKET_close(&pkt)) {
+            SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST,
+                   ERR_R_INTERNAL_ERROR);
+            goto err;
+        }
     }
 
-    off = n;
-    p += 2;
-    n += 2;
+    /* Start sub-packet for client CA list */
+    if (!WPACKET_start_sub_packet_u16(&pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     sk = SSL_get_client_CA_list(s);
-    nl = 0;
     if (sk != NULL) {
         for (i = 0; i < sk_X509_NAME_num(sk); i++) {
-            name = sk_X509_NAME_value(sk, i);
-            j = i2d_X509_NAME(name, NULL);
-            if (!BUF_MEM_grow_clean(buf, SSL_HM_HEADER_LENGTH(s) + n + j + 2)) {
-                SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_BUF_LIB);
+            unsigned char *namebytes;
+            X509_NAME *name = sk_X509_NAME_value(sk, i);
+            int namelen;
+
+            if (name == NULL
+                    || (namelen = i2d_X509_NAME(name, NULL)) < 0
+                    || !WPACKET_sub_allocate_bytes_u16(&pkt, namelen,
+                                                       &namebytes)
+                    || i2d_X509_NAME(name, &namebytes) != namelen) {
+                SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST,
+                       ERR_R_INTERNAL_ERROR);
                 goto err;
             }
-            p = ssl_handshake_start(s) + n;
-            s2n(j, p);
-            i2d_X509_NAME(name, &p);
-            n += 2 + j;
-            nl += 2 + j;
         }
     }
     /* else no CA names */
-    p = ssl_handshake_start(s) + off;
-    s2n(nl, p);
 
-    if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_REQUEST, n)) {
+    if (!WPACKET_close(&pkt)
+            || !ssl_close_construct_packet(s, &pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_REQUEST, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -2024,6 +2028,8 @@ int tls_construct_certificate_request(SSL *s)
 
     return 1;
  err:
+    WPACKET_cleanup(&pkt);
+    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
     ossl_statem_set_error(s);
     return 0;
 }