Convert ClientVerify Construction to WPACKET

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

ssl/ssl_locl.h

@@ -2038,7 +2038,9 @@ __owur int tls_check_serverhello_tlsext_early(SSL *s, const PACKET *ext,
                                               const PACKET *session_id,
                                               SSL_SESSION **ret);
 
-__owur int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk,
+__owur int tls12_get_sigandhash(WPACKET *pkt, const EVP_PKEY *pk,
+                                const EVP_MD *md);
+__owur int tls12_get_sigandhash_old(unsigned char *p, const EVP_PKEY *pk,
                                     const EVP_MD *md);
 __owur int tls12_get_sigid(const EVP_PKEY *pk);
 __owur const EVP_MD *tls12_get_hash(unsigned char hash_alg);

ssl/statem/statem_clnt.c

@@ -2616,22 +2616,32 @@ int tls_client_key_exchange_post_work(SSL *s)
 
 int tls_construct_client_verify(SSL *s)
 {
-    unsigned char *p;
     EVP_PKEY *pkey;
     const EVP_MD *md = s->s3->tmp.md[s->cert->key - s->cert->pkeys];
     EVP_MD_CTX *mctx;
     unsigned u = 0;
-    unsigned long n = 0;
     long hdatalen = 0;
     void *hdata;
+    unsigned char *sig = NULL;
+    WPACKET pkt;
+
+
+    if (!WPACKET_init(&pkt, s->init_buf)) {
+        /* Should not happen */
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_CERTIFICATE_VERIFY)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     mctx = EVP_MD_CTX_new();
     if (mctx == NULL) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE);
         goto err;
     }
-
-    p = ssl_handshake_start(s);
     pkey = s->cert->key->privatekey;
 
     hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
@@ -2639,24 +2649,25 @@ int tls_construct_client_verify(SSL *s)
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
         goto err;
     }
-    if (SSL_USE_SIGALGS(s)) {
-        if (!tls12_get_sigandhash(p, pkey, md)) {
+    if (SSL_USE_SIGALGS(s)&& !tls12_get_sigandhash(&pkt, pkey, md)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
         goto err;
     }
-        p += 2;
-        n = 2;
-    }
 #ifdef SSL_DEBUG
     fprintf(stderr, "Using client alg %s\n", EVP_MD_name(md));
 #endif
+    sig = OPENSSL_malloc(EVP_PKEY_size(pkey));
+    if (sig == NULL) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_MALLOC_FAILURE);
+        goto err;
+    }
     if (!EVP_SignInit_ex(mctx, md, NULL)
         || !EVP_SignUpdate(mctx, hdata, hdatalen)
         || (s->version == SSL3_VERSION
             && !EVP_MD_CTX_ctrl(mctx, EVP_CTRL_SSL3_MASTER_SECRET,
                                 s->session->master_key_length,
                                 s->session->master_key))
-        || !EVP_SignFinal(mctx, p + 2, &u, pkey)) {
+        || !EVP_SignFinal(mctx, sig, &u, pkey)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_EVP_LIB);
         goto err;
     }
@@ -2666,24 +2677,32 @@ int tls_construct_client_verify(SSL *s)
         if (pktype == NID_id_GostR3410_2001
             || pktype == NID_id_GostR3410_2012_256
             || pktype == NID_id_GostR3410_2012_512)
-            BUF_reverse(p + 2, NULL, u);
+            BUF_reverse(sig, NULL, u);
     }
 #endif
 
-    s2n(u, p);
-    n += u + 2;
+    if (!WPACKET_sub_memcpy_u16(&pkt, sig, u)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
     /* Digest cached records and discard handshake buffer */
     if (!ssl3_digest_cached_records(s, 0))
         goto err;
-    if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_VERIFY, n)) {
+
+    if (!ssl_close_construct_packet(s, &pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
         goto err;
     }
 
+    OPENSSL_free(sig);
     EVP_MD_CTX_free(mctx);
     return 1;
  err:
+    WPACKET_cleanup(&pkt);
+    OPENSSL_free(sig);
     EVP_MD_CTX_free(mctx);
+    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
     return 0;
 }
 

ssl/statem/statem_srvr.c

@@ -1918,7 +1918,7 @@ int tls_construct_server_key_exchange(SSL *s)
         if (md) {
             /* send signature algorithm */
             if (SSL_USE_SIGALGS(s)) {
-                if (!tls12_get_sigandhash(p, pkey, md)) {
+                if (!tls12_get_sigandhash_old(p, pkey, md)) {
                     /* Should never happen */
                     al = SSL_AD_INTERNAL_ERROR;
                     SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,

ssl/t1_lib.c

@@ -3119,7 +3119,31 @@ static int tls12_find_nid(int id, const tls12_lookup *table, size_t tlen)
     return NID_undef;
 }
 
-int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk, const EVP_MD *md)
+int tls12_get_sigandhash(WPACKET *pkt, const EVP_PKEY *pk, const EVP_MD *md)
+{
+    int sig_id, md_id;
+    if (!md)
+        return 0;
+    md_id = tls12_find_id(EVP_MD_type(md), tls12_md, OSSL_NELEM(tls12_md));
+    if (md_id == -1)
+        return 0;
+    sig_id = tls12_get_sigid(pk);
+    if (sig_id == -1)
+        return 0;
+    if (!WPACKET_put_bytes(pkt, md_id, 1) || !WPACKET_put_bytes(pkt, sig_id, 1))
+        return 0;
+
+    return 1;
+}
+
+/*
+ * Old version of the tls12_get_sigandhash function used by code that has not
+ * yet been converted to WPACKET yet. It will be deleted once WPACKET conversion
+ * is complete.
+ * TODO - DELETE ME
+ */
+int tls12_get_sigandhash_old(unsigned char *p, const EVP_PKEY *pk,
+                             const EVP_MD *md)
 {
     int sig_id, md_id;
     if (!md)