[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Tue Sep 13 23:05:06 UTC 2016
The branch master has been updated
via 869d0a37cfa7cfdbd42026d2b75d14cdc64e81e0 (commit)
via c9216d1485a350585a7363f46f3e69a840f2d385 (commit)
via b2b3024e0eef58589f7a49ebd48da98d4564a348 (commit)
via f1ec23c0bcc8ebb40331120b87a0e99f6823da67 (commit)
from 497f3bf9a75a2917e50b16b7985e87c89b86a39b (commit)
- Log -----------------------------------------------------------------
commit 869d0a37cfa7cfdbd42026d2b75d14cdc64e81e0
Author: Matt Caswell <matt at openssl.org>
Date: Tue Sep 13 15:42:12 2016 +0100
Encourage use of the macros for the various "sub" functions
Don't call WPACKET_sub_memcpy(), WPACKET_sub_allocation_bytes() and
WPACKET_start_sub_packet_len() directly.
Reviewed-by: Rich Salz <rsalz at openssl.org>
commit c9216d1485a350585a7363f46f3e69a840f2d385
Author: Matt Caswell <matt at openssl.org>
Date: Tue Sep 13 14:17:09 2016 +0100
Make wpackettest conform to style rules
Remove extra indentation at the start of an "if".
Reviewed-by: Rich Salz <rsalz at openssl.org>
commit b2b3024e0eef58589f7a49ebd48da98d4564a348
Author: Matt Caswell <matt at openssl.org>
Date: Tue Sep 13 11:32:52 2016 +0100
Add a WPACKET_sub_allocate_bytes() function
Updated the construction code to use the new function. Also added some
convenience macros for WPACKET_sub_memcpy().
Reviewed-by: Rich Salz <rsalz at openssl.org>
commit f1ec23c0bcc8ebb40331120b87a0e99f6823da67
Author: Matt Caswell <matt at openssl.org>
Date: Tue Sep 13 11:01:04 2016 +0100
Convert CKE construction to use the WPACKET API
Reviewed-by: Rich Salz <rsalz at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/packet.c | 20 ++++--
ssl/packet_locl.h | 54 ++++++++++++---
ssl/statem/statem_clnt.c | 170 +++++++++++++++++++++++------------------------
ssl/statem/statem_dtls.c | 3 +-
ssl/statem/statem_lib.c | 3 +-
ssl/t1_lib.c | 31 ++++-----
test/wpackettest.c | 68 +++++++++++--------
7 files changed, 201 insertions(+), 148 deletions(-)
diff --git a/ssl/packet.c b/ssl/packet.c
index b7084b0..7d80ebc 100644
--- a/ssl/packet.c
+++ b/ssl/packet.c
@@ -41,6 +41,17 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes)
return 1;
}
+int WPACKET_sub_allocate_bytes__(WPACKET *pkt, size_t len,
+ unsigned char **allocbytes, size_t lenbytes)
+{
+ if (!WPACKET_start_sub_packet_len__(pkt, lenbytes)
+ || !WPACKET_allocate_bytes(pkt, len, allocbytes)
+ || !WPACKET_close(pkt))
+ return 0;
+
+ return 1;
+}
+
static size_t maxmaxsize(size_t lenbytes)
{
if (lenbytes >= sizeof(size_t) || lenbytes == 0)
@@ -187,7 +198,7 @@ int WPACKET_finish(WPACKET *pkt)
return ret;
}
-int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes)
+int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)
{
WPACKET_SUB *sub;
unsigned char *lenchars;
@@ -220,7 +231,7 @@ int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes)
int WPACKET_start_sub_packet(WPACKET *pkt)
{
- return WPACKET_start_sub_packet_len(pkt, 0);
+ return WPACKET_start_sub_packet_len__(pkt, 0);
}
int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t size)
@@ -279,9 +290,10 @@ int WPACKET_memcpy(WPACKET *pkt, const void *src, size_t len)
return 1;
}
-int WPACKET_sub_memcpy(WPACKET *pkt, const void *src, size_t len, size_t lenbytes)
+int WPACKET_sub_memcpy__(WPACKET *pkt, const void *src, size_t len,
+ size_t lenbytes)
{
- if (!WPACKET_start_sub_packet_len(pkt, lenbytes)
+ if (!WPACKET_start_sub_packet_len__(pkt, lenbytes)
|| !WPACKET_memcpy(pkt, src, len)
|| !WPACKET_close(pkt))
return 0;
diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index daef69e..0ec5a38 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -643,26 +643,27 @@ int WPACKET_finish(WPACKET *pkt);
/*
* Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated
- * at the start of the sub-packet to store its length once we know it.
+ * at the start of the sub-packet to store its length once we know it. Don't
+ * call this directly. Use the convenience macros below instead.
*/
-int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes);
+int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes);
/*
* Convenience macros for calling WPACKET_start_sub_packet_len with different
* lengths
*/
#define WPACKET_start_sub_packet_u8(pkt) \
- WPACKET_start_sub_packet_len((pkt), 1)
+ WPACKET_start_sub_packet_len__((pkt), 1)
#define WPACKET_start_sub_packet_u16(pkt) \
- WPACKET_start_sub_packet_len((pkt), 2)
+ WPACKET_start_sub_packet_len__((pkt), 2)
#define WPACKET_start_sub_packet_u24(pkt) \
- WPACKET_start_sub_packet_len((pkt), 3)
+ WPACKET_start_sub_packet_len__((pkt), 3)
#define WPACKET_start_sub_packet_u32(pkt) \
- WPACKET_start_sub_packet_len((pkt), 4)
+ WPACKET_start_sub_packet_len__((pkt), 4)
/*
- * Same as WPACKET_start_sub_packet_len() except no bytes are pre-allocated for
- * the sub-packet length.
+ * Same as WPACKET_start_sub_packet_len__() except no bytes are pre-allocated
+ * for the sub-packet length.
*/
int WPACKET_start_sub_packet(WPACKET *pkt);
@@ -675,6 +676,28 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t bytes,
unsigned char **allocbytes);
/*
+ * The same as WPACKET_allocate_bytes() except additionally a new sub-packet is
+ * started for the allocated bytes, and then closed immediately afterwards. The
+ * number of length bytes for the sub-packet is in |lenbytes|. Don't call this
+ * directly. Use the convenience macros below instead.
+ */
+int WPACKET_sub_allocate_bytes__(WPACKET *pkt, size_t len,
+ unsigned char **allocbytes, size_t lenbytes);
+
+/*
+ * Convenience macros for calling WPACKET_sub_allocate_bytes with different
+ * lengths
+ */
+#define WPACKET_sub_allocate_bytes_u8(pkt, len, bytes) \
+ WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 1)
+#define WPACKET_sub_allocate_bytes_u16(pkt, len, bytes) \
+ WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 2)
+#define WPACKET_sub_allocate_bytes_u24(pkt, len, bytes) \
+ WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 3)
+#define WPACKET_sub_allocate_bytes_u32(pkt, len, bytes) \
+ WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 4)
+
+/*
* Write the value stored in |val| into the WPACKET. The value will consume
* |bytes| amount of storage. An error will occur if |val| cannot be
* accommodated in |bytes| storage, e.g. attempting to write the value 256 into
@@ -690,11 +713,22 @@ int WPACKET_memcpy(WPACKET *pkt, const void *src, size_t len);
/*
* Copy |len| bytes of data from |*src| into the WPACKET and prefix with its
- * length (consuming |lenbytes| of data for the length)
+ * length (consuming |lenbytes| of data for the length). Don't call this
+ * directly. Use the convenience macros below instead.
*/
-int WPACKET_sub_memcpy(WPACKET *pkt, const void *src, size_t len,
+int WPACKET_sub_memcpy__(WPACKET *pkt, const void *src, size_t len,
size_t lenbytes);
+/* Convenience macros for calling WPACKET_sub_memcpy with different lengths */
+#define WPACKET_sub_memcpy_u8(pkt, src, len) \
+ WPACKET_sub_memcpy__((pkt), (src), (len), 1)
+#define WPACKET_sub_memcpy_u16(pkt, src, len) \
+ WPACKET_sub_memcpy__((pkt), (src), (len), 2)
+#define WPACKET_sub_memcpy_bytes_u24(pkt, src, len) \
+ WPACKET_sub_memcpy__((pkt), (src), (len), 3)
+#define WPACKET_sub_memcpy_bytes_u32(pkt, src, len) \
+ WPACKET_sub_memcpy__((pkt), (src), (len), 4)
+
/*
* Return the total number of bytes written so far to the underlying buffer
* including any storage allocated for length bytes
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 59d21df..703e6a6 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -804,8 +804,8 @@ int tls_construct_client_hello(SSL *s)
/* cookie stuff for DTLS */
if (SSL_IS_DTLS(s)) {
if (s->d1->cookie_len > sizeof(s->d1->cookie)
- || !WPACKET_sub_memcpy(&pkt, s->d1->cookie, s->d1->cookie_len,
- 1)) {
+ || !WPACKET_sub_memcpy_u8(&pkt, s->d1->cookie,
+ s->d1->cookie_len)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
goto err;
}
@@ -865,7 +865,7 @@ int tls_construct_client_hello(SSL *s)
goto err;
}
- if (!WPACKET_close(&pkt) || !ssl_close_construct_packet(s, &pkt)) {
+ if (!ssl_close_construct_packet(s, &pkt)) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
goto err;
@@ -2071,8 +2071,7 @@ MSG_PROCESS_RETURN tls_process_server_done(SSL *s, PACKET *pkt)
return MSG_PROCESS_FINISHED_READING;
}
-static int tls_construct_cke_psk_preamble(SSL *s, unsigned char **p,
- size_t *pskhdrlen, int *al)
+static int tls_construct_cke_psk_preamble(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_PSK
int ret = 0;
@@ -2133,10 +2132,12 @@ static int tls_construct_cke_psk_preamble(SSL *s, unsigned char **p,
OPENSSL_free(s->session->psk_identity);
s->session->psk_identity = tmpidentity;
tmpidentity = NULL;
- s2n(identitylen, *p);
- memcpy(*p, identity, identitylen);
- *pskhdrlen = 2 + identitylen;
- *p += identitylen;
+
+ if (!WPACKET_sub_memcpy_u16(pkt, identity, identitylen)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_CKE_PSK_PREAMBLE, ERR_R_INTERNAL_ERROR);
+ *al = SSL_AD_INTERNAL_ERROR;
+ goto err;
+ }
ret = 1;
@@ -2154,10 +2155,10 @@ static int tls_construct_cke_psk_preamble(SSL *s, unsigned char **p,
#endif
}
-static int tls_construct_cke_rsa(SSL *s, unsigned char **p, int *len, int *al)
+static int tls_construct_cke_rsa(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_RSA
- unsigned char *q;
+ unsigned char *encdata = NULL;
EVP_PKEY *pkey = NULL;
EVP_PKEY_CTX *pctx = NULL;
size_t enclen;
@@ -2192,21 +2193,22 @@ static int tls_construct_cke_rsa(SSL *s, unsigned char **p, int *len, int *al)
goto err;
}
- q = *p;
/* Fix buf for TLS and beyond */
- if (s->version > SSL3_VERSION)
- *p += 2;
+ if (s->version > SSL3_VERSION && !WPACKET_start_sub_packet_u16(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_CKE_RSA, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
pctx = EVP_PKEY_CTX_new(pkey, NULL);
if (pctx == NULL || EVP_PKEY_encrypt_init(pctx) <= 0
|| EVP_PKEY_encrypt(pctx, NULL, &enclen, pms, pmslen) <= 0) {
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_RSA, ERR_R_EVP_LIB);
goto err;
}
- if (EVP_PKEY_encrypt(pctx, *p, &enclen, pms, pmslen) <= 0) {
+ if (!WPACKET_allocate_bytes(pkt, enclen, &encdata)
+ || EVP_PKEY_encrypt(pctx, encdata, &enclen, pms, pmslen) <= 0) {
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_RSA, SSL_R_BAD_RSA_ENCRYPT);
goto err;
}
- *len = enclen;
EVP_PKEY_CTX_free(pctx);
pctx = NULL;
# ifdef PKCS1_CHECK
@@ -2217,9 +2219,9 @@ static int tls_construct_cke_rsa(SSL *s, unsigned char **p, int *len, int *al)
# endif
/* Fix buf for TLS and beyond */
- if (s->version > SSL3_VERSION) {
- s2n(*len, q);
- *len += 2;
+ if (s->version > SSL3_VERSION && !WPACKET_close(pkt)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_CKE_RSA, ERR_R_INTERNAL_ERROR);
+ goto err;
}
s->s3->tmp.pms = pms;
@@ -2238,49 +2240,48 @@ static int tls_construct_cke_rsa(SSL *s, unsigned char **p, int *len, int *al)
#endif
}
-static int tls_construct_cke_dhe(SSL *s, unsigned char **p, int *len, int *al)
+static int tls_construct_cke_dhe(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_DH
DH *dh_clnt = NULL;
const BIGNUM *pub_key;
EVP_PKEY *ckey = NULL, *skey = NULL;
+ unsigned char *keybytes = NULL;
skey = s->s3->peer_tmp;
- if (skey == NULL) {
- SSLerr(SSL_F_TLS_CONSTRUCT_CKE_DHE, ERR_R_INTERNAL_ERROR);
- return 0;
- }
+ if (skey == NULL)
+ goto err;
+
ckey = ssl_generate_pkey(skey);
dh_clnt = EVP_PKEY_get0_DH(ckey);
- if (dh_clnt == NULL || ssl_derive(s, ckey, skey) == 0) {
- SSLerr(SSL_F_TLS_CONSTRUCT_CKE_DHE, ERR_R_INTERNAL_ERROR);
- EVP_PKEY_free(ckey);
- return 0;
- }
+ if (dh_clnt == NULL || ssl_derive(s, ckey, skey) == 0)
+ goto err;
/* send off the data */
DH_get0_key(dh_clnt, &pub_key, NULL);
- *len = BN_num_bytes(pub_key);
- s2n(*len, *p);
- BN_bn2bin(pub_key, *p);
- *len += 2;
+ if (!WPACKET_sub_allocate_bytes_u16(pkt, BN_num_bytes(pub_key), &keybytes))
+ goto err;
+
+ BN_bn2bin(pub_key, keybytes);
EVP_PKEY_free(ckey);
return 1;
-#else
+ err:
+ EVP_PKEY_free(ckey);
+#endif
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_DHE, ERR_R_INTERNAL_ERROR);
*al = SSL_AD_INTERNAL_ERROR;
return 0;
-#endif
}
-static int tls_construct_cke_ecdhe(SSL *s, unsigned char **p, int *len, int *al)
+static int tls_construct_cke_ecdhe(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_EC
unsigned char *encodedPoint = NULL;
int encoded_pt_len = 0;
EVP_PKEY *ckey = NULL, *skey = NULL;
+ int ret = 0;
skey = s->s3->peer_tmp;
if (skey == NULL) {
@@ -2303,25 +2304,16 @@ static int tls_construct_cke_ecdhe(SSL *s, unsigned char **p, int *len, int *al)
goto err;
}
- EVP_PKEY_free(ckey);
- ckey = NULL;
-
- *len = encoded_pt_len;
-
- /* length of encoded point */
- **p = *len;
- *p += 1;
- /* copy the point */
- memcpy(*p, encodedPoint, *len);
- /* increment len to account for length field */
- *len += 1;
-
- OPENSSL_free(encodedPoint);
+ if (!WPACKET_sub_memcpy_u8(pkt, encodedPoint, encoded_pt_len)) {
+ SSLerr(SSL_F_TLS_CONSTRUCT_CKE_ECDHE, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
- return 1;
+ ret = 1;
err:
+ OPENSSL_free(encodedPoint);
EVP_PKEY_free(ckey);
- return 0;
+ return ret;
#else
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_ECDHE, ERR_R_INTERNAL_ERROR);
*al = SSL_AD_INTERNAL_ERROR;
@@ -2329,7 +2321,7 @@ static int tls_construct_cke_ecdhe(SSL *s, unsigned char **p, int *len, int *al)
#endif
}
-static int tls_construct_cke_gost(SSL *s, unsigned char **p, int *len, int *al)
+static int tls_construct_cke_gost(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_GOST
/* GOST key exchange message creation */
@@ -2425,22 +2417,21 @@ static int tls_construct_cke_gost(SSL *s, unsigned char **p, int *len, int *al)
/*
* Encapsulate it into sequence
*/
- *((*p)++) = V_ASN1_SEQUENCE | V_ASN1_CONSTRUCTED;
msglen = 255;
if (EVP_PKEY_encrypt(pkey_ctx, tmp, &msglen, pms, pmslen) <= 0) {
*al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_GOST, SSL_R_LIBRARY_BUG);
goto err;
}
- if (msglen >= 0x80) {
- *((*p)++) = 0x81;
- *((*p)++) = msglen & 0xff;
- *len = msglen + 3;
- } else {
- *((*p)++) = msglen & 0xff;
- *len = msglen + 2;
+
+ if (!WPACKET_put_bytes(pkt, V_ASN1_SEQUENCE | V_ASN1_CONSTRUCTED, 1)
+ || (msglen >= 0x80 && !WPACKET_put_bytes(pkt, 0x81, 1))
+ || !WPACKET_sub_memcpy_u8(pkt, tmp, msglen)) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ SSLerr(SSL_F_TLS_CONSTRUCT_CKE_GOST, ERR_R_INTERNAL_ERROR);
+ goto err;
}
- memcpy(*p, tmp, msglen);
+
/* Check if pubkey from client certificate was used */
if (EVP_PKEY_CTX_ctrl(pkey_ctx, -1, -1, EVP_PKEY_CTRL_PEER_KEY, 2,
NULL) > 0) {
@@ -2464,19 +2455,19 @@ static int tls_construct_cke_gost(SSL *s, unsigned char **p, int *len, int *al)
#endif
}
-static int tls_construct_cke_srp(SSL *s, unsigned char **p, int *len, int *al)
+static int tls_construct_cke_srp(SSL *s, WPACKET *pkt, int *al)
{
#ifndef OPENSSL_NO_SRP
- if (s->srp_ctx.A != NULL) {
- /* send off the data */
- *len = BN_num_bytes(s->srp_ctx.A);
- s2n(*len, *p);
- BN_bn2bin(s->srp_ctx.A, *p);
- *len += 2;
- } else {
+ unsigned char *abytes = NULL;
+
+ if (s->srp_ctx.A == NULL
+ || !WPACKET_sub_allocate_bytes_u16(pkt, BN_num_bytes(s->srp_ctx.A),
+ &abytes)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CKE_SRP, ERR_R_INTERNAL_ERROR);
return 0;
}
+ BN_bn2bin(s->srp_ctx.A, abytes);
+
OPENSSL_free(s->session->srp_username);
s->session->srp_username = OPENSSL_strdup(s->srp_ctx.login);
if (s->session->srp_username == NULL) {
@@ -2494,36 +2485,42 @@ static int tls_construct_cke_srp(SSL *s, unsigned char **p, int *len, int *al)
int tls_construct_client_key_exchange(SSL *s)
{
- unsigned char *p;
- int len;
- size_t pskhdrlen = 0;
unsigned long alg_k;
int al = -1;
+ WPACKET pkt;
- alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
+ if (!WPACKET_init(&pkt, s->init_buf)) {
+ /* Should not happen */
+ SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
- p = ssl_handshake_start(s);
+ if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_CLIENT_KEY_EXCHANGE)) {
+ ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+ SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
+
+ alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
if ((alg_k & SSL_PSK)
- && !tls_construct_cke_psk_preamble(s, &p, &pskhdrlen, &al))
+ && !tls_construct_cke_psk_preamble(s, &pkt, &al))
goto err;
- if (alg_k & SSL_kPSK) {
- len = 0;
- } else if (alg_k & (SSL_kRSA | SSL_kRSAPSK)) {
- if (!tls_construct_cke_rsa(s, &p, &len, &al))
+ if (alg_k & (SSL_kRSA | SSL_kRSAPSK)) {
+ if (!tls_construct_cke_rsa(s, &pkt, &al))
goto err;
} else if (alg_k & (SSL_kDHE | SSL_kDHEPSK)) {
- if (!tls_construct_cke_dhe(s, &p, &len, &al))
+ if (!tls_construct_cke_dhe(s, &pkt, &al))
goto err;
} else if (alg_k & (SSL_kECDHE | SSL_kECDHEPSK)) {
- if (!tls_construct_cke_ecdhe(s, &p, &len, &al))
+ if (!tls_construct_cke_ecdhe(s, &pkt, &al))
goto err;
} else if (alg_k & SSL_kGOST) {
- if (!tls_construct_cke_gost(s, &p, &len, &al))
+ if (!tls_construct_cke_gost(s, &pkt, &al))
goto err;
} else if (alg_k & SSL_kSRP) {
- if (!tls_construct_cke_srp(s, &p, &len, &al))
+ if (!tls_construct_cke_srp(s, &pkt, &al))
goto err;
} else {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
@@ -2531,9 +2528,7 @@ int tls_construct_client_key_exchange(SSL *s)
goto err;
}
- len += pskhdrlen;
-
- if (!ssl_set_handshake_header(s, SSL3_MT_CLIENT_KEY_EXCHANGE, len)) {
+ if (!ssl_close_construct_packet(s, &pkt)) {
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);
goto err;
@@ -2549,6 +2544,7 @@ int tls_construct_client_key_exchange(SSL *s)
OPENSSL_clear_free(s->s3->tmp.psk, s->s3->tmp.psklen);
s->s3->tmp.psk = NULL;
#endif
+ WPACKET_cleanup(&pkt);
ossl_statem_set_error(s);
return 0;
}
diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 25c4575..b9a53b0 100644
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -1218,7 +1218,8 @@ int dtls1_close_construct_packet(SSL *s, WPACKET *pkt)
{
size_t msglen;
- if (!WPACKET_get_length(pkt, &msglen)
+ if (!WPACKET_close(pkt)
+ || !WPACKET_get_length(pkt, &msglen)
|| msglen > INT_MAX
|| !WPACKET_finish(pkt))
return 0;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 7ad3899..4171594 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -61,7 +61,8 @@ int tls_close_construct_packet(SSL *s, WPACKET *pkt)
{
size_t msglen;
- if (!WPACKET_get_length(pkt, &msglen)
+ if (!WPACKET_close(pkt)
+ || !WPACKET_get_length(pkt, &msglen)
|| msglen > INT_MAX
|| !WPACKET_finish(pkt))
return 0;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 50083a9..696998f 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1040,8 +1040,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
/* Add RI if renegotiating */
if (s->renegotiate) {
if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_renegotiate, 2)
- || !WPACKET_sub_memcpy(pkt, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len, 2)) {
+ || !WPACKET_sub_memcpy_u16(pkt, s->s3->previous_client_finished,
+ s->s3->previous_client_finished_len)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
}
@@ -1058,8 +1058,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
/* Sub-packet for servername list (always 1 hostname)*/
|| !WPACKET_start_sub_packet_u16(pkt)
|| !WPACKET_put_bytes(pkt, TLSEXT_NAMETYPE_host_name, 1)
- || !WPACKET_sub_memcpy(pkt, s->tlsext_hostname,
- strlen(s->tlsext_hostname), 2)
+ || !WPACKET_sub_memcpy_u16(pkt, s->tlsext_hostname,
+ strlen(s->tlsext_hostname))
|| !WPACKET_close(pkt)
|| !WPACKET_close(pkt)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
@@ -1099,7 +1099,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_ec_point_formats, 2)
/* Sub-packet for formats extension */
|| !WPACKET_start_sub_packet_u16(pkt)
- || !WPACKET_sub_memcpy(pkt, pformats, num_formats, 1)
+ || !WPACKET_sub_memcpy_u8(pkt, pformats, num_formats)
|| !WPACKET_close(pkt)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
@@ -1161,8 +1161,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
goto skip_ext;
if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_session_ticket, 2)
- || !WPACKET_sub_memcpy(pkt, s->session->tlsext_tick, ticklen,
- 2)) {
+ || !WPACKET_sub_memcpy_u16(pkt, s->session->tlsext_tick,
+ ticklen)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
}
@@ -1209,10 +1209,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
idlen = i2d_OCSP_RESPID(id, NULL);
if (idlen <= 0
/* Sub-packet for an individual id */
- || !WPACKET_start_sub_packet_u8(pkt)
- || !WPACKET_allocate_bytes(pkt, idlen, &idbytes)
- || i2d_OCSP_RESPID(id, &idbytes) != idlen
- || !WPACKET_close(pkt)) {
+ || !WPACKET_sub_allocate_bytes_u8(pkt, idlen, &idbytes)
+ || i2d_OCSP_RESPID(id, &idbytes) != idlen) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
}
@@ -1292,8 +1290,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
TLSEXT_TYPE_application_layer_protocol_negotiation, 2)
/* Sub-packet ALPN extension */
|| !WPACKET_start_sub_packet_u16(pkt)
- || !WPACKET_sub_memcpy(pkt, s->alpn_client_proto_list,
- s->alpn_client_proto_list_len, 2)
+ || !WPACKET_sub_memcpy_u16(pkt, s->alpn_client_proto_list,
+ s->alpn_client_proto_list_len)
|| !WPACKET_close(pkt)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
@@ -1380,16 +1378,11 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
hlen = 0;
if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_padding, 2)
- || !WPACKET_start_sub_packet_u16(pkt)
- || !WPACKET_allocate_bytes(pkt, hlen, &padbytes)) {
+ || !WPACKET_sub_allocate_bytes_u16(pkt, hlen, &padbytes)) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return 0;
}
memset(padbytes, 0, hlen);
- if (!WPACKET_close(pkt)) {
- SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
- return 0;
- }
}
}
diff --git a/test/wpackettest.c b/test/wpackettest.c
index ca2a1a7..ac712b4 100644
--- a/test/wpackettest.c
+++ b/test/wpackettest.c
@@ -35,7 +35,7 @@ static int test_WPACKET_init(void)
int i;
size_t written;
- if ( !WPACKET_init(&pkt, buf)
+ if (!WPACKET_init(&pkt, buf)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
/* Closing a top level WPACKET should fail */
|| WPACKET_close(&pkt)
@@ -55,7 +55,7 @@ static int test_WPACKET_init(void)
}
/* Now try with a one byte length prefix */
- if ( !WPACKET_init_len(&pkt, buf, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
@@ -66,7 +66,7 @@ static int test_WPACKET_init(void)
}
/* And a longer length prefix */
- if ( !WPACKET_init_len(&pkt, buf, 4)
+ if (!WPACKET_init_len(&pkt, buf, 4)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
@@ -103,7 +103,7 @@ static int test_WPACKET_set_max_size(void)
WPACKET pkt;
size_t written;
- if ( !WPACKET_init(&pkt, buf)
+ if (!WPACKET_init(&pkt, buf)
/*
* No previous lenbytes set so we should be ok to set the max
* possible max size
@@ -118,7 +118,7 @@ static int test_WPACKET_set_max_size(void)
return 0;
}
- if ( !WPACKET_init_len(&pkt, buf, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
/*
* Should fail because we already consumed 1 byte with the
* length
@@ -160,7 +160,7 @@ static int test_WPACKET_start_sub_packet(void)
size_t written;
size_t len;
- if ( !WPACKET_init(&pkt, buf)
+ if (!WPACKET_init(&pkt, buf)
|| !WPACKET_start_sub_packet(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
/* Can't finish because we have a sub packet */
@@ -178,8 +178,8 @@ static int test_WPACKET_start_sub_packet(void)
}
/* Single sub-packet with length prefix */
- if ( !WPACKET_init(&pkt, buf)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ if (!WPACKET_init(&pkt, buf)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_close(&pkt)
|| !WPACKET_finish(&pkt)
@@ -191,10 +191,10 @@ static int test_WPACKET_start_sub_packet(void)
}
/* Nested sub-packets with length prefixes */
- if ( !WPACKET_init(&pkt, buf)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ if (!WPACKET_init(&pkt, buf)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_get_length(&pkt, &len)
|| len != 1
@@ -211,11 +211,11 @@ static int test_WPACKET_start_sub_packet(void)
}
/* Sequential sub-packets with length prefixes */
- if ( !WPACKET_init(&pkt, buf)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ if (!WPACKET_init(&pkt, buf)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_close(&pkt)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_close(&pkt)
|| !WPACKET_finish(&pkt)
@@ -236,7 +236,7 @@ static int test_WPACKET_set_flags(void)
size_t written;
/* Set packet to be non-zero length */
- if ( !WPACKET_init(&pkt, buf)
+ if (!WPACKET_init(&pkt, buf)
|| !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)
/* Should fail because of zero length */
|| WPACKET_finish(&pkt)
@@ -250,7 +250,7 @@ static int test_WPACKET_set_flags(void)
}
/* Repeat above test in a sub-packet */
- if ( !WPACKET_init(&pkt, buf)
+ if (!WPACKET_init(&pkt, buf)
|| !WPACKET_start_sub_packet(&pkt)
|| !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)
/* Should fail because of zero length */
@@ -266,7 +266,7 @@ static int test_WPACKET_set_flags(void)
}
/* Set packet to abandon non-zero length */
- if ( !WPACKET_init_len(&pkt, buf, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
|| !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
|| !WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
@@ -276,8 +276,8 @@ static int test_WPACKET_set_flags(void)
}
/* Repeat above test but only abandon a sub-packet */
- if ( !WPACKET_init_len(&pkt, buf, 1)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
|| !WPACKET_close(&pkt)
|| !WPACKET_finish(&pkt)
@@ -289,8 +289,8 @@ static int test_WPACKET_set_flags(void)
}
/* And repeat with a non empty sub-packet */
- if ( !WPACKET_init(&pkt, buf)
- || !WPACKET_start_sub_packet_len(&pkt, 1)
+ if (!WPACKET_init(&pkt, buf)
+ || !WPACKET_start_sub_packet_u8(&pkt)
|| !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
|| !WPACKET_put_bytes(&pkt, 0xff, 1)
|| !WPACKET_close(&pkt)
@@ -310,14 +310,14 @@ static int test_WPACKET_allocate_bytes(void)
size_t written;
unsigned char *bytes;
- if ( !WPACKET_init_len(&pkt, buf, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
|| !WPACKET_allocate_bytes(&pkt, 2, &bytes)) {
testfail("test_WPACKET_allocate_bytes():1 failed\n", &pkt);
return 0;
}
bytes[0] = 0xfe;
bytes[1] = 0xff;
- if ( !WPACKET_finish(&pkt)
+ if (!WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
|| written != sizeof(alloc)
|| memcmp(buf->data, &alloc, written) != 0) {
@@ -325,6 +325,22 @@ static int test_WPACKET_allocate_bytes(void)
return 0;
}
+ /* Repeat with WPACKET_sub_allocate_bytes */
+ if (!WPACKET_init_len(&pkt, buf, 1)
+ || !WPACKET_sub_allocate_bytes_u8(&pkt, 2, &bytes)) {
+ testfail("test_WPACKET_allocate_bytes():3 failed\n", &pkt);
+ return 0;
+ }
+ bytes[0] = 0xfe;
+ bytes[1] = 0xff;
+ if (!WPACKET_finish(&pkt)
+ || !WPACKET_get_total_written(&pkt, &written)
+ || written != sizeof(submem)
+ || memcmp(buf->data, &submem, written) != 0) {
+ testfail("test_WPACKET_allocate_bytes():4 failed\n", &pkt);
+ return 0;
+ }
+
return 1;
}
@@ -334,7 +350,7 @@ static int test_WPACKET_memcpy(void)
size_t written;
const unsigned char bytes[] = { 0xfe, 0xff };
- if ( !WPACKET_init_len(&pkt, buf, 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
|| !WPACKET_memcpy(&pkt, bytes, sizeof(bytes))
|| !WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
@@ -345,8 +361,8 @@ static int test_WPACKET_memcpy(void)
}
/* Repeat with WPACKET_sub_memcpy() */
- if ( !WPACKET_init_len(&pkt, buf, 1)
- || !WPACKET_sub_memcpy(&pkt, bytes, sizeof(bytes), 1)
+ if (!WPACKET_init_len(&pkt, buf, 1)
+ || !WPACKET_sub_memcpy_u8(&pkt, bytes, sizeof(bytes))
|| !WPACKET_finish(&pkt)
|| !WPACKET_get_total_written(&pkt, &written)
|| written != sizeof(submem)
More information about the openssl-commits
mailing list