The branch master has been updated
       via  418a18a2deddc0b0d6181de0008219c899ca6ddf (commit)
       via  15e6be6c5c518f3b1594a2370638d5ae4e9742d1 (commit)
       via  c49e1912300e9d4346d568e99a4908d5678d5e53 (commit)
       via  6400f338184b7acc94b8c21febdc68ec6f7fe3de (commit)
      from  28aef3d9558dc2e11ba56576b3a4d3faaef8a9d3 (commit)


- Log -----------------------------------------------------------------
commit 418a18a2deddc0b0d6181de0008219c899ca6ddf
Author: Matt Caswell <m...@openssl.org>
Date:   Tue Sep 20 10:16:15 2016 +0100

    Style tweaks following review feedback
    
    Reviewed-by: Rich Salz <rs...@openssl.org>

commit 15e6be6c5c518f3b1594a2370638d5ae4e9742d1
Author: Matt Caswell <m...@openssl.org>
Date:   Wed Sep 14 12:10:33 2016 +0100

    Convert NextProto message construction to WPACKET
    
    Reviewed-by: Rich Salz <rs...@openssl.org>

commit c49e1912300e9d4346d568e99a4908d5678d5e53
Author: Matt Caswell <m...@openssl.org>
Date:   Wed Sep 14 11:41:27 2016 +0100

    Convert Certificate message construction to WPACKET
    
    Reviewed-by: Rich Salz <rs...@openssl.org>

commit 6400f338184b7acc94b8c21febdc68ec6f7fe3de
Author: Matt Caswell <m...@openssl.org>
Date:   Wed Sep 14 11:10:37 2016 +0100

    Convert ClientVerify Construction to WPACKET
    
    Reviewed-by: Rich Salz <rs...@openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 include/openssl/ssl.h    |  1 +
 ssl/ssl_cert.c           | 40 ++++++++-------------
 ssl/ssl_err.c            |  1 +
 ssl/ssl_locl.h           |  6 ++--
 ssl/statem/statem_clnt.c | 92 ++++++++++++++++++++++++++++++++++--------------
 ssl/statem/statem_lib.c  | 31 ++++++++++------
 ssl/statem/statem_srvr.c |  2 +-
 ssl/t1_lib.c             | 27 +++++++++++++-
 8 files changed, 132 insertions(+), 68 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 1fcdbd2..234a25e 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2232,6 +2232,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY                358
 # define SSL_F_TLS_CONSTRUCT_FINISHED                     359
 # define SSL_F_TLS_CONSTRUCT_HELLO_REQUEST                373
+# define SSL_F_TLS_CONSTRUCT_NEXT_PROTO                   426
 # define SSL_F_TLS_CONSTRUCT_SERVER_CERTIFICATE           374
 # define SSL_F_TLS_CONSTRUCT_SERVER_DONE                  375
 # define SSL_F_TLS_CONSTRUCT_SERVER_HELLO                 376
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 0c931db..9d35957 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -740,48 +740,36 @@ 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)
+    if (cpk == NULL || cpk->x509 == NULL)
         return 1;
 
     x = cpk->x509;
@@ -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_err.c b/ssl/ssl_err.c
index f776f61..7f94ca6 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -257,6 +257,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_FINISHED), "tls_construct_finished"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_REQUEST),
      "tls_construct_hello_request"},
+    {ERR_FUNC(SSL_F_TLS_CONSTRUCT_NEXT_PROTO), "tls_construct_next_proto"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_SERVER_CERTIFICATE),
      "tls_construct_server_certificate"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_SERVER_DONE), "tls_construct_server_done"},
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 26485f6..6ad2735 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);
@@ -2038,8 +2038,10 @@ __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);
 void ssl_set_sig_mask(uint32_t *pmask_a, SSL *s, int op);
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 703e6a6..95af064 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2616,22 +2616,31 @@ 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 +2648,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)) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
-            goto err;
-        }
-        p += 2;
-        n = 2;
+    if (SSL_USE_SIGALGS(s)&& !tls12_get_sigandhash(&pkt, pkey, md)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY, ERR_R_INTERNAL_ERROR);
+        goto err;
     }
 #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 +2676,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;
 }
 
@@ -2876,22 +2894,42 @@ int ssl3_check_cert_and_algorithm(SSL *s)
 #ifndef OPENSSL_NO_NEXTPROTONEG
 int tls_construct_next_proto(SSL *s)
 {
-    unsigned int len, padding_len;
-    unsigned char *d;
+    size_t len, padding_len;
+    unsigned char *padding = NULL;
+    WPACKET pkt;
+
+    if (!WPACKET_init(&pkt, s->init_buf)) {
+        /* Should not happen */
+        SSLerr(SSL_F_TLS_CONSTRUCT_NEXT_PROTO, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_NEXT_PROTO)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_NEXT_PROTO, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     len = s->next_proto_negotiated_len;
     padding_len = 32 - ((len + 2) % 32);
-    d = (unsigned char *)s->init_buf->data;
-    d[4] = len;
-    memcpy(d + 5, s->next_proto_negotiated, len);
-    d[5 + len] = padding_len;
-    memset(d + 6 + len, 0, padding_len);
-    *(d++) = SSL3_MT_NEXT_PROTO;
-    l2n3(2 + len + padding_len, d);
-    s->init_num = 4 + 2 + len + padding_len;
-    s->init_off = 0;
+
+    if (!WPACKET_sub_memcpy_u8(&pkt, s->next_proto_negotiated, len)
+            || !WPACKET_sub_allocate_bytes_u8(&pkt, padding_len, &padding)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_NEXT_PROTO, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    memset(padding, 0, padding_len);
+
+    if (!ssl_close_construct_packet(s, &pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_NEXT_PROTO, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     return 1;
+ err:
+    WPACKET_cleanup(&pkt);
+    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+    return 0;
 }
 #endif
 
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 4171594..882e150 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)
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 818f48d..7536e6e 100644
--- a/ssl/statem/statem_srvr.c
+++ b/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,
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 4f5ea42..eea7802 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3119,7 +3119,32 @@ 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 == NULL)
+        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)
_____
openssl-commits mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits

Reply via email to