Convert Certificate message construction to WPACKET

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

3 files changed, 34 insertions, 37 deletions

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 0c931db97d..e5fa66df53 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -740,47 +740,35 @@ int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
     return ret;
 }
 
-/* Add a certificate to a BUF_MEM structure */
-
-static int ssl_add_cert_to_buf(BUF_MEM *buf, unsigned long *l, X509 *x)
+/* Add a certificate to the WPACKET */
+static int ssl_add_cert_to_buf(WPACKET *pkt, X509 *x)
 {
-    int n;
-    unsigned char *p;
+    int len;
+    unsigned char *outbytes;
 
-    n = i2d_X509(x, NULL);
-    if (n < 0 || !BUF_MEM_grow_clean(buf, (int)(n + (*l) + 3))) {
+    len = i2d_X509(x, NULL);
+    if (len < 0) {
         SSLerr(SSL_F_SSL_ADD_CERT_TO_BUF, ERR_R_BUF_LIB);
         return 0;
     }
-    p = (unsigned char *)&(buf->data[*l]);
-    l2n3(n, p);
-    n = i2d_X509(x, &p);
-    if (n < 0) {
-        /* Shouldn't happen */
-        SSLerr(SSL_F_SSL_ADD_CERT_TO_BUF, ERR_R_BUF_LIB);
+    if (!WPACKET_sub_allocate_bytes_u24(pkt, len, &outbytes)
+            || i2d_X509(x, &outbytes) != len) {
+        SSLerr(SSL_F_SSL_ADD_CERT_TO_BUF, ERR_R_INTERNAL_ERROR);
         return 0;
     }
-    *l += n + 3;
 
     return 1;
 }
 
 /* Add certificate chain to internal SSL BUF_MEM structure */
-int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l)
+int ssl_add_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk)
 {
-    BUF_MEM *buf = s->init_buf;
     int i, chain_count;
     X509 *x;
     STACK_OF(X509) *extra_certs;
     STACK_OF(X509) *chain = NULL;
     X509_STORE *chain_store;
 
-    /* TLSv1 sends a chain with nothing in it, instead of an alert */
-    if (!BUF_MEM_grow_clean(buf, 10)) {
-        SSLerr(SSL_F_SSL_ADD_CERT_CHAIN, ERR_R_BUF_LIB);
-        return 0;
-    }
-
     if (!cpk || !cpk->x509)
         return 1;
 
@@ -839,7 +827,7 @@ int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l)
         for (i = 0; i < chain_count; i++) {
             x = sk_X509_value(chain, i);
 
-            if (!ssl_add_cert_to_buf(buf, l, x)) {
+            if (!ssl_add_cert_to_buf(pkt, x)) {
                 X509_STORE_CTX_free(xs_ctx);
                 return 0;
             }
@@ -851,11 +839,11 @@ int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l)
             SSLerr(SSL_F_SSL_ADD_CERT_CHAIN, i);
             return 0;
         }
-        if (!ssl_add_cert_to_buf(buf, l, x))
+        if (!ssl_add_cert_to_buf(pkt, x))
             return 0;
         for (i = 0; i < sk_X509_num(extra_certs); i++) {
             x = sk_X509_value(extra_certs, i);
-            if (!ssl_add_cert_to_buf(buf, l, x))
+            if (!ssl_add_cert_to_buf(pkt, x))
                 return 0;
         }
     }
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 841cb5ab42..6ad27354d1 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1830,7 +1830,7 @@ __owur X509 *ssl_cert_get0_next_certificate(CERT *c, int first);
 void ssl_cert_set_cert_cb(CERT *c, int (*cb) (SSL *ssl, void *arg), void *arg);
 
 __owur int ssl_verify_cert_chain(SSL *s, STACK_OF(X509) *sk);
-__owur int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l);
+__owur int ssl_add_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk);
 __owur int ssl_build_cert_chain(SSL *s, SSL_CTX *ctx, int flags);
 __owur int ssl_cert_set_cert_store(CERT *c, X509_STORE *store, int chain,
                                    int ref);
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 4171594220..882e150b3b 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -267,22 +267,31 @@ int tls_construct_change_cipher_spec(SSL *s)
 
 unsigned long ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk)
 {
-    unsigned char *p;
-    unsigned long l = 3 + SSL_HM_HEADER_LENGTH(s);
+    WPACKET pkt;
 
-    if (!ssl_add_cert_chain(s, cpk, &l))
-        return 0;
+    if (!WPACKET_init(&pkt, s->init_buf)) {
+        /* Should not happen */
+        SSLerr(SSL_F_SSL3_OUTPUT_CERT_CHAIN, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
-    l -= 3 + SSL_HM_HEADER_LENGTH(s);
-    p = ssl_handshake_start(s);
-    l2n3(l, p);
-    l += 3;
+    if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_CERTIFICATE)
+            || !WPACKET_start_sub_packet_u24(&pkt)) {
+        SSLerr(SSL_F_SSL3_OUTPUT_CERT_CHAIN, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    if (!ssl_add_cert_chain(s, &pkt, cpk))
+        goto err;
 
-    if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE, l)) {
+    if (!WPACKET_close(&pkt) || !ssl_close_construct_packet(s, &pkt)) {
         SSLerr(SSL_F_SSL3_OUTPUT_CERT_CHAIN, ERR_R_INTERNAL_ERROR);
-        return 0;
+        goto err;
     }
-    return l + SSL_HM_HEADER_LENGTH(s);
+    return 1;
+ err:
+    WPACKET_cleanup(&pkt);
+    return 0;
 }
 
 WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst)