[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Thu Sep 29 09:09:21 UTC 2016


The branch master has been updated
       via  25849a8f8bb64956f35a8a2a160ae0de1d2990c6 (commit)
       via  7facdbd66f19f4a87cf2a5a335568c879772d92f (commit)
       via  7507e73d409b8f3046d6efcc3f4c0b6208b59b64 (commit)
       via  150e298551a6788baac56c0c89dc8b8342ac0079 (commit)
       via  8157d44b624da08142f3f9f6edc37fb5542c2573 (commit)
      from  2f2d6e3e3ccd1ae7bba9f1af62f97dfca986e083 (commit)


- Log -----------------------------------------------------------------
commit 25849a8f8bb64956f35a8a2a160ae0de1d2990c6
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Sep 29 10:06:11 2016 +0100

    Address style feedback comments
    
    Merge declarations of same type together.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 7facdbd66f19f4a87cf2a5a335568c879772d92f
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Sep 28 13:33:41 2016 +0100

    Fix a bug in the construction of the ClienHello SRTP extension
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 7507e73d409b8f3046d6efcc3f4c0b6208b59b64
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Sep 28 12:03:30 2016 +0100

    Fix heartbeat compilation error
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 150e298551a6788baac56c0c89dc8b8342ac0079
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Sep 28 11:15:36 2016 +0100

    Delete some unneeded code
    
    Some functions were being called from both code that used WPACKETs and code
    that did not. Now that more code has been converted to use WPACKETs some of
    that duplication can be removed.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 8157d44b624da08142f3f9f6edc37fb5542c2573
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Sep 28 11:13:48 2016 +0100

    Convert ServerHello construction to WPACKET
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

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

Summary of changes:
 ssl/d1_srtp.c            |  24 ------
 ssl/s3_lib.c             |  20 -----
 ssl/ssl_locl.h           |  11 +--
 ssl/statem/statem_srvr.c |  82 +++++++------------
 ssl/t1_ext.c             |  65 ---------------
 ssl/t1_lib.c             | 209 +++++++++++++++++++++--------------------------
 ssl/t1_reneg.c           |  36 +++-----
 7 files changed, 138 insertions(+), 309 deletions(-)

diff --git a/ssl/d1_srtp.c b/ssl/d1_srtp.c
index b5e5ef3..bcefb9e 100644
--- a/ssl/d1_srtp.c
+++ b/ssl/d1_srtp.c
@@ -203,30 +203,6 @@ int ssl_parse_clienthello_use_srtp_ext(SSL *s, PACKET *pkt, int *al)
     return 0;
 }
 
-int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len,
-                                     int maxlen)
-{
-    if (p) {
-        if (maxlen < 5) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_USE_SRTP_EXT,
-                   SSL_R_SRTP_PROTECTION_PROFILE_LIST_TOO_LONG);
-            return 1;
-        }
-
-        if (s->srtp_profile == 0) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_USE_SRTP_EXT,
-                   SSL_R_USE_SRTP_NOT_NEGOTIATED);
-            return 1;
-        }
-        s2n(2, p);
-        s2n(s->srtp_profile->id, p);
-        *p++ = 0;
-    }
-    *len = 5;
-
-    return 0;
-}
-
 int ssl_parse_serverhello_use_srtp_ext(SSL *s, PACKET *pkt, int *al)
 {
     unsigned int id, ct, mki;
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 2a4dc6d..2115a7e 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3571,26 +3571,6 @@ const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p)
     return cp;
 }
 
-/*
- * Old version of the ssl3_put_cipher_by_char 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 ssl3_put_cipher_by_char_old(const SSL_CIPHER *c, unsigned char *p)
-{
-    long l;
-
-    if (p != NULL) {
-        l = c->id;
-        if ((l & 0xff000000) != 0x03000000)
-            return (0);
-        p[0] = ((unsigned char)(l >> 8L)) & 0xFF;
-        p[1] = ((unsigned char)(l)) & 0xFF;
-    }
-    return (2);
-}
-
 int ssl3_put_cipher_by_char(const SSL_CIPHER *c, WPACKET *pkt, size_t *len)
 {
     if ((c->id & 0xff000000) != 0x03000000) {
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 630fea8..7dbff76 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1863,7 +1863,6 @@ __owur int ssl_derive(SSL *s, EVP_PKEY *privkey, EVP_PKEY *pubkey);
 __owur EVP_PKEY *ssl_dh_to_pkey(DH *dh);
 
 __owur const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p);
-__owur int ssl3_put_cipher_by_char_old(const SSL_CIPHER *c, unsigned char *p);
 __owur int ssl3_put_cipher_by_char(const SSL_CIPHER *c, WPACKET *pkt,
                                    size_t *len);
 int ssl3_init_finished_mac(SSL *s);
@@ -2017,8 +2016,7 @@ __owur int tls1_shared_list(SSL *s,
                             const unsigned char *l1, size_t l1len,
                             const unsigned char *l2, size_t l2len, int nmatch);
 __owur int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al);
-__owur unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
-                                                 unsigned char *limit, int *al);
+__owur int ssl_add_serverhello_tlsext(SSL *s, WPACKET *pkt, int *al);
 __owur int ssl_parse_clienthello_tlsext(SSL *s, PACKET *pkt);
 void ssl_set_default_md(SSL *s);
 __owur int tls1_set_server_sigalgs(SSL *s);
@@ -2066,8 +2064,7 @@ __owur int ssl_security_cert_chain(SSL *s, STACK_OF(X509) *sk, X509 *ex,
 
 __owur EVP_MD_CTX *ssl_replace_hash(EVP_MD_CTX **hash, const EVP_MD *md);
 void ssl_clear_hash_ctx(EVP_MD_CTX **hash);
-__owur int ssl_add_serverhello_renegotiate_ext(SSL *s, unsigned char *p,
-                                               int *len, int maxlen);
+__owur int ssl_add_serverhello_renegotiate_ext(SSL *s, WPACKET *pkt);
 __owur int ssl_parse_serverhello_renegotiate_ext(SSL *s, PACKET *pkt, int *al);
 __owur int ssl_parse_clienthello_renegotiate_ext(SSL *s, PACKET *pkt, int *al);
 __owur long ssl_get_algorithm2(SSL *s);
@@ -2084,8 +2081,6 @@ void ssl_set_client_disabled(SSL *s);
 __owur int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op);
 
 __owur int ssl_parse_clienthello_use_srtp_ext(SSL *s, PACKET *pkt, int *al);
-__owur int ssl_add_serverhello_use_srtp_ext(SSL *s, unsigned char *p, int *len,
-                                            int maxlen);
 __owur int ssl_parse_serverhello_use_srtp_ext(SSL *s, PACKET *pkt, int *al);
 
 __owur int ssl_handshake_hash(SSL *s, unsigned char *out, int outlen);
@@ -2121,8 +2116,6 @@ __owur int custom_ext_parse(SSL *s, int server,
                             unsigned int ext_type,
                             const unsigned char *ext_data, size_t ext_size,
                             int *al);
-__owur int custom_ext_add_old(SSL *s, int server, unsigned char **pret,
-                              unsigned char *limit, int *al);
 __owur int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al);
 
 __owur int custom_exts_copy(custom_ext_methods *dst,
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 8a2791a..bf50e79 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1499,26 +1499,21 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
 
 int tls_construct_server_hello(SSL *s)
 {
-    unsigned char *buf;
-    unsigned char *p, *d;
-    int i, sl;
-    int al = 0;
-    unsigned long l;
-
-    buf = (unsigned char *)s->init_buf->data;
-
-    /* Do the message type and length last */
-    d = p = ssl_handshake_start(s);
-
-    *(p++) = s->version >> 8;
-    *(p++) = s->version & 0xff;
+    int sl, compm, al = SSL_AD_INTERNAL_ERROR;
+    size_t len;
+    WPACKET pkt;
 
-    /*
-     * Random stuff. Filling of the server_random takes place in
-     * tls_process_client_hello()
-     */
-    memcpy(p, s->s3->server_random, SSL3_RANDOM_SIZE);
-    p += SSL3_RANDOM_SIZE;
+    if (!WPACKET_init(&pkt, s->init_buf)
+            || !ssl_set_handshake_header2(s, &pkt, SSL3_MT_SERVER_HELLO)
+            || !WPACKET_put_bytes_u16(&pkt, s->version)
+               /*
+                * Random stuff. Filling of the server_random takes place in
+                * tls_process_client_hello()
+                */
+            || !WPACKET_memcpy(&pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
 
     /*-
      * There are several cases for the session ID to send
@@ -1544,50 +1539,35 @@ int tls_construct_server_hello(SSL *s)
     sl = s->session->session_id_length;
     if (sl > (int)sizeof(s->session->session_id)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
-        ossl_statem_set_error(s);
-        return 0;
+        goto err;
     }
-    *(p++) = sl;
-    memcpy(p, s->session->session_id, sl);
-    p += sl;
 
-    /* put the cipher */
-    i = ssl3_put_cipher_by_char_old(s->s3->tmp.new_cipher, p);
-    p += i;
-
-    /* put the compression method */
+    /* set up the compression method */
 #ifdef OPENSSL_NO_COMP
-    *(p++) = 0;
+    compm = 0;
 #else
     if (s->s3->tmp.new_compression == NULL)
-        *(p++) = 0;
+        compm = 0;
     else
-        *(p++) = s->s3->tmp.new_compression->id;
+        compm = s->s3->tmp.new_compression->id;
 #endif
 
-    if (ssl_prepare_serverhello_tlsext(s) <= 0) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, SSL_R_SERVERHELLO_TLSEXT);
-        ossl_statem_set_error(s);
-        return 0;
-    }
-    if ((p =
-         ssl_add_serverhello_tlsext(s, p, buf + SSL3_RT_MAX_PLAIN_LENGTH,
-                                    &al)) == NULL) {
-        ssl3_send_alert(s, SSL3_AL_FATAL, al);
-        SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
-        ossl_statem_set_error(s);
-        return 0;
-    }
-
-    /* do the header */
-    l = (p - d);
-    if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l)) {
+    if (!WPACKET_sub_memcpy_u8(&pkt, s->session->session_id, sl)
+            || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, &pkt, &len)
+            || !WPACKET_put_bytes_u8(&pkt, compm)
+            || !ssl_prepare_serverhello_tlsext(s)
+            || !ssl_add_serverhello_tlsext(s, &pkt, &al)
+            || !ssl_close_construct_packet(s, &pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
-        ossl_statem_set_error(s);
-        return 0;
+        goto err;
     }
 
     return 1;
+ err:
+    WPACKET_cleanup(&pkt);
+    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+    ossl_statem_set_error(s);
+    return 0;
 }
 
 int tls_construct_server_done(SSL *s)
diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c
index 099a0ae..30b3046 100644
--- a/ssl/t1_ext.c
+++ b/ssl/t1_ext.c
@@ -72,71 +72,6 @@ int custom_ext_parse(SSL *s, int server,
 
 /*
  * Request custom extension data from the application and add to the return
- * buffer. This is the old style function signature prior to WPACKET. This is
- * here temporarily until the conversion to WPACKET is completed, i.e. it is
- * used by code that hasn't been converted yet.
- * TODO - REMOVE THIS FUNCTION
- */
-int custom_ext_add_old(SSL *s, int server,
-                       unsigned char **pret, unsigned char *limit, int *al)
-{
-    custom_ext_methods *exts = server ? &s->cert->srv_ext : &s->cert->cli_ext;
-    custom_ext_method *meth;
-    unsigned char *ret = *pret;
-    size_t i;
-
-    for (i = 0; i < exts->meths_count; i++) {
-        const unsigned char *out = NULL;
-        size_t outlen = 0;
-        meth = exts->meths + i;
-
-        if (server) {
-            /*
-             * For ServerHello only send extensions present in ClientHello.
-             */
-            if (!(meth->ext_flags & SSL_EXT_FLAG_RECEIVED))
-                continue;
-            /* If callback absent for server skip it */
-            if (!meth->add_cb)
-                continue;
-        }
-        if (meth->add_cb) {
-            int cb_retval = 0;
-            cb_retval = meth->add_cb(s, meth->ext_type,
-                                     &out, &outlen, al, meth->add_arg);
-            if (cb_retval < 0)
-                return 0;       /* error */
-            if (cb_retval == 0)
-                continue;       /* skip this extension */
-        }
-        if (4 > limit - ret || outlen > (size_t)(limit - ret - 4))
-            return 0;
-        s2n(meth->ext_type, ret);
-        s2n(outlen, ret);
-        if (outlen) {
-            memcpy(ret, out, outlen);
-            ret += outlen;
-        }
-        /*
-         * We can't send duplicates: code logic should prevent this.
-         */
-        OPENSSL_assert(!(meth->ext_flags & SSL_EXT_FLAG_SENT));
-        /*
-         * Indicate extension has been sent: this is both a sanity check to
-         * ensure we don't send duplicate extensions and indicates that it is
-         * not an error if the extension is present in ServerHello.
-         */
-        meth->ext_flags |= SSL_EXT_FLAG_SENT;
-        if (meth->free_cb)
-            meth->free_cb(s, meth->ext_type, out, meth->add_arg);
-    }
-    *pret = ret;
-    return 1;
-}
-
-
-/*
- * Request custom extension data from the application and add to the return
  * buffer.
  */
 int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al)
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 40932fa..230fe66 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1302,7 +1302,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
     }
 #ifndef OPENSSL_NO_SRTP
     if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)) {
-        STACK_OF(SRTP_PROTECTION_PROFILE) *clnt = 0;
+        STACK_OF(SRTP_PROTECTION_PROFILE) *clnt = SSL_get_srtp_profiles(s);
         SRTP_PROTECTION_PROFILE *prof;
         int i, ct;
 
@@ -1322,7 +1322,10 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
                 return 0;
             }
         }
-        if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
+        if (!WPACKET_close(pkt)
+                   /* Add an empty use_mki value */
+                || !WPACKET_put_bytes_u8(pkt, 0)
+                || !WPACKET_close(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
             return 0;
         }
@@ -1392,12 +1395,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
     return 1;
 }
 
-unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
-                                          unsigned char *limit, int *al)
+int ssl_add_serverhello_tlsext(SSL *s, WPACKET *pkt, int *al)
 {
-    int extdatalen = 0;
-    unsigned char *orig = buf;
-    unsigned char *ret = buf;
 #ifndef OPENSSL_NO_NEXTPROTONEG
     int next_proto_neg_seen;
 #endif
@@ -1408,30 +1407,16 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
     using_ecc = using_ecc && (s->session->tlsext_ecpointformatlist != NULL);
 #endif
 
-    ret += 2;
-    if (ret >= limit)
-        return NULL;            /* this really never occurs, but ... */
-
-    if (s->s3->send_connection_binding) {
-        int el;
-
-        if (!ssl_add_serverhello_renegotiate_ext(s, 0, &el, 0)) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-            return NULL;
-        }
-
-        if ((limit - ret - 4 - el) < 0)
-            return NULL;
-
-        s2n(TLSEXT_TYPE_renegotiate, ret);
-        s2n(el, ret);
-
-        if (!ssl_add_serverhello_renegotiate_ext(s, ret, &el, el)) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-            return NULL;
-        }
+    if (!WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_set_flags(pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)) {
+        SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
 
-        ret += el;
+    if (s->s3->send_connection_binding &&
+            !ssl_add_serverhello_renegotiate_ext(s, pkt)) {
+        SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+        return 0;
     }
 
     /* Only add RI for SSLv3 */
@@ -1439,12 +1424,12 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
         goto done;
 
     if (!s->hit && s->servername_done == 1
-        && s->session->tlsext_hostname != NULL) {
-        if ((long)(limit - ret - 4) < 0)
-            return NULL;
-
-        s2n(TLSEXT_TYPE_server_name, ret);
-        s2n(0, ret);
+            && s->session->tlsext_hostname != NULL) {
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_server_name)
+                || !WPACKET_put_bytes_u16(pkt, 0)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 #ifndef OPENSSL_NO_EC
     if (using_ecc) {
@@ -1453,25 +1438,15 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
         /*
          * Add TLS extension ECPointFormats to the ServerHello message
          */
-        long lenmax;
-
         tls1_get_formatlist(s, &plist, &plistlen);
 
-        if ((lenmax = limit - ret - 5) < 0)
-            return NULL;
-        if (plistlen > (size_t)lenmax)
-            return NULL;
-        if (plistlen > 255) {
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_ec_point_formats)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_sub_memcpy_u8(pkt, plist, plistlen)
+                || !WPACKET_close(pkt)) {
             SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-            return NULL;
+            return 0;
         }
-
-        s2n(TLSEXT_TYPE_ec_point_formats, ret);
-        s2n(plistlen + 1, ret);
-        *(ret++) = (unsigned char)plistlen;
-        memcpy(ret, plist, plistlen);
-        ret += plistlen;
-
     }
     /*
      * Currently the server should not respond with a SupportedCurves
@@ -1480,10 +1455,11 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
 #endif                          /* OPENSSL_NO_EC */
 
     if (s->tlsext_ticket_expected && tls_use_ticket(s)) {
-        if ((long)(limit - ret - 4) < 0)
-            return NULL;
-        s2n(TLSEXT_TYPE_session_ticket, ret);
-        s2n(0, ret);
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_session_ticket)
+                || !WPACKET_put_bytes_u16(pkt, 0)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     } else {
         /*
          * if we don't add the above TLSEXT, we can't add a session ticket
@@ -1493,31 +1469,23 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
     }
 
     if (s->tlsext_status_expected) {
-        if ((long)(limit - ret - 4) < 0)
-            return NULL;
-        s2n(TLSEXT_TYPE_status_request, ret);
-        s2n(0, ret);
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_status_request)
+                || !WPACKET_put_bytes_u16(pkt, 0)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 #ifndef OPENSSL_NO_SRTP
     if (SSL_IS_DTLS(s) && s->srtp_profile) {
-        int el;
-
-        /* Returns 0 on success!! */
-        if (ssl_add_serverhello_use_srtp_ext(s, 0, &el, 0)) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-            return NULL;
-        }
-        if ((limit - ret - 4 - el) < 0)
-            return NULL;
-
-        s2n(TLSEXT_TYPE_use_srtp, ret);
-        s2n(el, ret);
-
-        if (ssl_add_serverhello_use_srtp_ext(s, ret, &el, el)) {
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_use_srtp)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_put_bytes_u16(pkt, 2)
+                || !WPACKET_put_bytes_u16(pkt, s->srtp_profile->id)
+                || !WPACKET_put_bytes_u8(pkt, 0)
+                || !WPACKET_close(pkt)) {
             SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-            return NULL;
+            return 0;
         }
-        ret += el;
     }
 #endif
 
@@ -1532,28 +1500,32 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
             0x2a, 0x85, 0x03, 0x02, 0x02, 0x16, 0x30, 0x08,
             0x06, 0x06, 0x2a, 0x85, 0x03, 0x02, 0x02, 0x17
         };
-        if (limit - ret < 36)
-            return NULL;
-        memcpy(ret, cryptopro_ext, 36);
-        ret += 36;
-
+        if (!WPACKET_memcpy(pkt, cryptopro_ext, sizeof(cryptopro_ext))) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 #ifndef OPENSSL_NO_HEARTBEATS
     /* Add Heartbeat extension if we've received one */
     if (SSL_IS_DTLS(s) && (s->tlsext_heartbeat & SSL_DTLSEXT_HB_ENABLED)) {
-        if ((limit - ret - 4 - 1) < 0)
-            return NULL;
-        s2n(TLSEXT_TYPE_heartbeat, ret);
-        s2n(1, ret);
+        unsigned int mode;
         /*-
          * Set mode:
          * 1: peer may send requests
          * 2: peer not allowed to send requests
          */
         if (s->tlsext_heartbeat & SSL_DTLSEXT_HB_DONT_RECV_REQUESTS)
-            *(ret++) = SSL_DTLSEXT_HB_DONT_SEND_REQUESTS;
+            mode = SSL_DTLSEXT_HB_DONT_SEND_REQUESTS;
         else
-            *(ret++) = SSL_DTLSEXT_HB_ENABLED;
+            mode = SSL_DTLSEXT_HB_ENABLED;
+
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_heartbeat)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_put_bytes_u8(pkt, mode)
+                || !WPACKET_close(pkt)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
 
     }
 #endif
@@ -1570,18 +1542,20 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
                                               s->
                                               ctx->next_protos_advertised_cb_arg);
         if (r == SSL_TLSEXT_ERR_OK) {
-            if ((long)(limit - ret - 4 - npalen) < 0)
-                return NULL;
-            s2n(TLSEXT_TYPE_next_proto_neg, ret);
-            s2n(npalen, ret);
-            memcpy(ret, npa, npalen);
-            ret += npalen;
+            if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_next_proto_neg)
+                    || !WPACKET_sub_memcpy_u16(pkt, npa, npalen)) {
+                SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
             s->s3->next_proto_neg_seen = 1;
         }
     }
 #endif
-    if (!custom_ext_add_old(s, 1, &ret, limit, al))
-        return NULL;
+    if (!custom_ext_add(s, 1, pkt, al)) {
+        SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
     if (s->s3->flags & TLS1_FLAGS_ENCRYPT_THEN_MAC) {
         /*
          * Don't use encrypt_then_mac if AEAD or RC4 might want to disable
@@ -1593,36 +1567,41 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
             || s->s3->tmp.new_cipher->algorithm_enc == SSL_eGOST2814789CNT12)
             s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC;
         else {
-            s2n(TLSEXT_TYPE_encrypt_then_mac, ret);
-            s2n(0, ret);
+            if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_encrypt_then_mac)
+                    || !WPACKET_put_bytes_u16(pkt, 0)) {
+                SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
         }
     }
     if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) {
-        s2n(TLSEXT_TYPE_extended_master_secret, ret);
-        s2n(0, ret);
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_extended_master_secret)
+                || !WPACKET_put_bytes_u16(pkt, 0)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 
     if (s->s3->alpn_selected != NULL) {
-        const unsigned char *selected = s->s3->alpn_selected;
-        unsigned int len = s->s3->alpn_selected_len;
-
-        if ((long)(limit - ret - 4 - 2 - 1 - len) < 0)
-            return NULL;
-        s2n(TLSEXT_TYPE_application_layer_protocol_negotiation, ret);
-        s2n(3 + len, ret);
-        s2n(1 + len, ret);
-        *ret++ = len;
-        memcpy(ret, selected, len);
-        ret += len;
+        if (!WPACKET_put_bytes_u16(pkt,
+                    TLSEXT_TYPE_application_layer_protocol_negotiation)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_sub_memcpy_u8(pkt, s->s3->alpn_selected,
+                                          s->s3->alpn_selected_len)
+                || !WPACKET_close(pkt)
+                || !WPACKET_close(pkt)) {
+            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 
  done:
-
-    if ((extdatalen = ret - orig - 2) == 0)
-        return orig;
-
-    s2n(extdatalen, orig);
-    return ret;
+    if (!WPACKET_close(pkt)) {
+        SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+    return 1;
 }
 
 /*
diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c
index f5136e2..f3e01bb 100644
--- a/ssl/t1_reneg.c
+++ b/ssl/t1_reneg.c
@@ -50,32 +50,18 @@ int ssl_parse_clienthello_renegotiate_ext(SSL *s, PACKET *pkt, int *al)
 }
 
 /* Add the server's renegotiation binding */
-int ssl_add_serverhello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
-                                        int maxlen)
+int ssl_add_serverhello_renegotiate_ext(SSL *s, WPACKET *pkt)
 {
-    if (p) {
-        if ((s->s3->previous_client_finished_len +
-             s->s3->previous_server_finished_len + 1) > maxlen) {
-            SSLerr(SSL_F_SSL_ADD_SERVERHELLO_RENEGOTIATE_EXT,
-                   SSL_R_RENEGOTIATE_EXT_TOO_LONG);
-            return 0;
-        }
-
-        /* Length byte */
-        *p = s->s3->previous_client_finished_len +
-            s->s3->previous_server_finished_len;
-        p++;
-
-        memcpy(p, s->s3->previous_client_finished,
-               s->s3->previous_client_finished_len);
-        p += s->s3->previous_client_finished_len;
-
-        memcpy(p, s->s3->previous_server_finished,
-               s->s3->previous_server_finished_len);
-    }
-
-    *len = s->s3->previous_client_finished_len
-        + s->s3->previous_server_finished_len + 1;
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
+            || !WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_start_sub_packet_u8(pkt)
+            || !WPACKET_memcpy(pkt, s->s3->previous_client_finished,
+                               s->s3->previous_client_finished_len)
+            || !WPACKET_memcpy(pkt, s->s3->previous_server_finished,
+                               s->s3->previous_server_finished_len)
+            || !WPACKET_close(pkt)
+            || !WPACKET_close(pkt))
+        return 0;
 
     return 1;
 }


More information about the openssl-commits mailing list