From 0217dd19c00657b8bfd2bce1090785eb32abb235 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 6 Sep 2016 15:09:51 +0100 Subject: [PATCH] Move from explicit sub-packets to implicit ones No need to declare an explicit sub-packet. Just start one. Reviewed-by: Rich Salz --- ssl/packet.c | 232 +++++++++++++++++++-------------------- ssl/packet_locl.h | 60 +++++----- ssl/s3_lib.c | 4 +- ssl/ssl_locl.h | 13 +-- ssl/statem/statem_clnt.c | 42 +++---- ssl/statem/statem_dtls.c | 6 +- ssl/statem/statem_lib.c | 2 +- ssl/t1_ext.c | 7 +- ssl/t1_lib.c | 133 ++++++++++------------ 9 files changed, 236 insertions(+), 263 deletions(-) diff --git a/ssl/packet.c b/ssl/packet.c index 6b6cc481a2..d4f964922d 100644 --- a/ssl/packet.c +++ b/ssl/packet.c @@ -10,33 +10,35 @@ #include "packet_locl.h" /* - * Allocate bytes in the WPACKET_BUF for the output. This reserves the bytes + * Allocate bytes in the WPACKET for the output. This reserves the bytes * and count them as "written", but doesn't actually do the writing. */ -static unsigned char *WPACKET_BUF_allocate(WPACKET_BUF *wbuf, size_t len) +int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes) { - unsigned char *ret = wbuf->curr; - - if (SIZE_MAX - wbuf->written < len) + if (pkt->subs == NULL || len == 0) return 0; - if (wbuf->maxsize > 0 && wbuf->written + len > wbuf->maxsize) + if (SIZE_MAX - pkt->written < len) return 0; - if (wbuf->buf->length - wbuf->written < len) { + if (pkt->maxsize > 0 && pkt->written + len > pkt->maxsize) + return 0; + + if (pkt->buf->length - pkt->written < len) { size_t newlen; - if (wbuf->buf->length > SIZE_MAX / 2) + if (pkt->buf->length > SIZE_MAX / 2) newlen = SIZE_MAX; else - newlen = wbuf->buf->length * 2; - if (BUF_MEM_grow(wbuf->buf, newlen) == 0) - return NULL; + newlen = pkt->buf->length * 2; + if (BUF_MEM_grow(pkt->buf, newlen) == 0) + return 0; } - wbuf->written += len; - wbuf->curr += len; + pkt->written += len; + *allocbytes = pkt->curr; + pkt->curr += len; - return ret; + return 1; } /* @@ -47,39 +49,28 @@ static unsigned char *WPACKET_BUF_allocate(WPACKET_BUF *wbuf, size_t len) */ int WPACKET_init_len(WPACKET *pkt, BUF_MEM *buf, size_t lenbytes) { - WPACKET_BUF *wbuf; /* Sanity check */ if (buf == NULL) return 0; - wbuf = OPENSSL_zalloc(sizeof(WPACKET_BUF)); - if (wbuf == NULL) { - pkt->isclosed = 1; + pkt->buf = buf; + pkt->curr = (unsigned char *)buf->data; + pkt->written = 0; + pkt->maxsize = 0; + + pkt->subs = OPENSSL_zalloc(sizeof(*pkt->subs)); + if (pkt->subs == NULL) return 0; - } - wbuf->buf = buf; - wbuf->curr = (unsigned char *)buf->data; - wbuf->written = 0; - wbuf->maxsize = 0; - - pkt->parent = NULL; - pkt->wbuf = wbuf; - pkt->pwritten = lenbytes; - pkt->lenbytes = lenbytes; - pkt->haschild = 0; - pkt->isclosed = 0; - - if (lenbytes == 0) { - pkt->packet_len = NULL; + if (lenbytes == 0) return 1; - } - pkt->packet_len = WPACKET_BUF_allocate(wbuf, lenbytes); - if (pkt->packet_len == NULL) { - OPENSSL_free(wbuf); - pkt->wbuf = NULL; - pkt->isclosed = 1; + pkt->subs->pwritten = lenbytes; + pkt->subs->lenbytes = lenbytes; + + if (!WPACKET_allocate_bytes(pkt, lenbytes, &(pkt->subs->packet_len))) { + OPENSSL_free(pkt->subs); + pkt->subs = NULL; return 0; } @@ -107,59 +98,61 @@ int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len, size_t lenbytes) { /* We only allow this to be set once */ - if (pkt->isclosed || pkt->packet_len != NULL) + if (pkt->subs == NULL) return 0; - pkt->lenbytes = lenbytes; - pkt->packet_len = packet_len; + pkt->subs->lenbytes = lenbytes; + pkt->subs->packet_len = packet_len; return 1; } int WPACKET_set_flags(WPACKET *pkt, unsigned int flags) { - pkt->flags = flags; + if (pkt->subs == NULL) + return 0; + + pkt->subs->flags = flags; return 1; } + /* - * Closes the WPACKET and marks it as invalid for future writes. It also writes - * out the length of the packet to the required location (normally the start - * of the WPACKET) if appropriate. A WPACKET cannot be closed if it has an - * active sub-packet. + * Internal helper function used by WPACKET_close() and WPACKET_finish() to + * close a sub-packet and write out its length if necessary. */ -int WPACKET_close(WPACKET *pkt) +static int wpacket_intern_close(WPACKET *pkt) { size_t packlen; + WPACKET_SUB *sub = pkt->subs; - if (pkt->isclosed || pkt->haschild) - return 0; - - packlen = pkt->wbuf->written - pkt->pwritten; - if (packlen == 0 && pkt->flags & OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) + packlen = pkt->written - sub->pwritten; + if (packlen == 0 + && sub->flags & OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) return 0; if (packlen == 0 - && pkt->flags & OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) { + && sub->flags & OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) { /* Deallocate any bytes allocated for the length of the WPACKET */ - if ((pkt->wbuf->curr - pkt->lenbytes) == pkt->packet_len) { - pkt->wbuf->written -= pkt->lenbytes; - pkt->wbuf->curr -= pkt->lenbytes; + if ((pkt->curr - sub->lenbytes) == sub->packet_len) { + pkt->written -= sub->lenbytes; + pkt->curr -= sub->lenbytes; } /* Don't write out the packet length */ - pkt->packet_len = NULL; + sub->packet_len = NULL; } /* Write out the WPACKET length if needed */ - if (pkt->packet_len != NULL) { + if (sub->packet_len != NULL) { size_t lenbytes; - lenbytes = pkt->lenbytes; + lenbytes = sub->lenbytes; for (; lenbytes > 0; lenbytes--) { - pkt->packet_len[lenbytes - 1] = (unsigned char)(packlen & 0xff); + sub->packet_len[lenbytes - 1] + = (unsigned char)(packlen & 0xff); packlen >>= 8; } if (packlen > 0) { @@ -170,51 +163,74 @@ int WPACKET_close(WPACKET *pkt) } } - if (pkt->parent != NULL) { - if (pkt->parent->haschild != 1) { - /* Should not happen! */ - return 0; - } - pkt->parent->haschild = 0; - pkt->parent = NULL; - } - - pkt->isclosed = 1; + pkt->subs = sub->parent; + OPENSSL_free(sub); return 1; } /* - * Initialise a new sub-packet (|subpkt|), based on a parent (|pkt|). - * Additionally |lenbytes| of data is preallocated at the start of the - * sub-packet to store its length once we know it. + * Closes the most recent sub-packet. It also writes out the length of the + * packet to the required location (normally the start of the WPACKET) if + * appropriate. The top level WPACKET should be closed using WPACKET_finish() + * instead of this function. */ -int WPACKET_get_sub_packet_len(WPACKET *pkt, WPACKET *subpkt, size_t lenbytes) +int WPACKET_close(WPACKET *pkt) { - if (pkt->isclosed || pkt->haschild || subpkt == NULL) + if (pkt->subs == NULL || pkt->subs->parent == NULL) return 0; - subpkt->parent = pkt; - subpkt->wbuf = pkt->wbuf; - subpkt->pwritten = pkt->wbuf->written + lenbytes; - subpkt->lenbytes = lenbytes; - subpkt->haschild = 0; - subpkt->isclosed = 0; + return wpacket_intern_close(pkt); +} + +/* + * The same as WPACKET_close() but only for the top most WPACKET. Additionally + * frees memory resources for this WPACKET. + */ +int WPACKET_finish(WPACKET *pkt) +{ + int ret; + + if (pkt->subs == NULL || pkt->subs->parent != NULL) + return 0; + + ret = wpacket_intern_close(pkt); + + /* We free up memory no matter whether |ret| is zero or not */ + OPENSSL_free(pkt->subs); + pkt->subs = NULL; + return ret; +} + +/* + * 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. + */ +int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes) +{ + WPACKET_SUB *sub; + + if (pkt->subs == NULL) + return 0; + + sub = OPENSSL_zalloc(sizeof(*sub)); + if (sub == NULL) + return 0; + + sub->parent = pkt->subs; + pkt->subs = sub; + sub->pwritten = pkt->written + lenbytes; + sub->lenbytes = lenbytes; if (lenbytes == 0) { - subpkt->packet_len = NULL; - pkt->haschild = 1; + sub->packet_len = NULL; return 1; } - subpkt->packet_len = WPACKET_BUF_allocate(pkt->wbuf, lenbytes); - if (subpkt->packet_len == NULL) { - subpkt->isclosed = 1; + if (!WPACKET_allocate_bytes(pkt, lenbytes, &sub->packet_len)) { return 0; } - pkt->haschild = 1; - return 1; } @@ -222,31 +238,9 @@ int WPACKET_get_sub_packet_len(WPACKET *pkt, WPACKET *subpkt, size_t lenbytes) * Same as WPACKET_get_sub_packet_len() except no bytes are pre-allocated for * the sub-packet length. */ -int WPACKET_get_sub_packet(WPACKET *pkt, WPACKET *subpkt) +int WPACKET_start_sub_packet(WPACKET *pkt) { - return WPACKET_get_sub_packet_len(pkt, subpkt, 0); -} - -/* - * Allocate some bytes in the WPACKET for writing. That number of bytes is - * marked as having been written, and a pointer to their location is stored in - * |*allocbytes|. - */ -int WPACKET_allocate_bytes(WPACKET *pkt, size_t bytes, - unsigned char **allocbytes) -{ - unsigned char *data; - - if (pkt->isclosed || pkt->haschild || bytes == 0) - return 0; - - data = WPACKET_BUF_allocate(pkt->wbuf, bytes); - if (data == NULL) - return 0; - - *allocbytes = data; - - return 1; + return WPACKET_start_sub_packet_len(pkt, 0); } /* @@ -283,7 +277,7 @@ int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t bytes) */ int WPACKET_set_max_size(WPACKET *pkt, size_t maxsize) { - pkt->wbuf->maxsize = maxsize; + pkt->maxsize = maxsize; return 1; } @@ -312,24 +306,24 @@ int WPACKET_memcpy(WPACKET *pkt, const void *src, size_t len) */ int WPACKET_get_total_written(WPACKET *pkt, size_t *written) { - if (pkt->isclosed || written == NULL) + if (pkt->subs == NULL || written == NULL) return 0; - *written = pkt->wbuf->curr - (unsigned char *)pkt->wbuf->buf->data; + *written = pkt->written; return 1; } /* - * Returns the length of this WPACKET so far. This excludes any bytes allocated + * Returns the length of the last sub-packet. This excludes any bytes allocated * for the length itself. */ int WPACKET_get_length(WPACKET *pkt, size_t *len) { - if (pkt->isclosed || len == NULL) + if (pkt->subs == NULL || len == NULL) return 0; - *len = pkt->wbuf->written - pkt->pwritten; + *len = pkt->written - pkt->subs->pwritten; return 1; } diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index 88bc7ddebc..5ec5b045f1 100644 --- a/ssl/packet_locl.h +++ b/ssl/packet_locl.h @@ -551,7 +551,29 @@ __owur static ossl_inline int PACKET_get_length_prefixed_3(PACKET *pkt, /* Writeable packets */ -typedef struct packetw_buf { +typedef struct wpacket_sub WPACKET_SUB; +struct wpacket_sub { + /* The parent WPACKET_SUB if we have one or NULL otherwise */ + WPACKET_SUB *parent; + + /* + * Pointer to where the length of this WPACKET goes (or NULL if we don't + * write the length) + */ + unsigned char *packet_len; + + /* Number of bytes in the packet_len */ + size_t lenbytes; + + /* Number of bytes written to the buf prior to this packet starting */ + size_t pwritten; + + /* Flags for this sub-packet */ + unsigned int flags; +}; + +typedef struct wpacket_st WPACKET; +struct wpacket_st { /* The buffer where we store the output data */ BUF_MEM *buf; @@ -566,36 +588,9 @@ typedef struct packetw_buf { * if no maximum */ size_t maxsize; -} WPACKET_BUF; -typedef struct packetw_st WPACKET; -struct packetw_st { - /* The parent WPACKET if we have one or NULL otherwise */ - WPACKET *parent; - - /* The actual buffer - shared with sub-packets */ - WPACKET_BUF *wbuf; - - /* Flags for this WPACKET */ - unsigned int flags; - - /* - * Pointer to where the length of this WPACKET goes (or NULL if we don't - * write the length) - */ - unsigned char *packet_len; - - /* Number of bytes in the packet_len */ - size_t lenbytes; - - /* Number of bytes written to the buf prior to this packet starting */ - size_t pwritten; - - /* True if we have an active sub-packet or false otherwise */ - int haschild; - - /* True if WPACKET_close() has been called on this WPACKET */ - int isclosed; + /* Our sub-packets (always at least one if not closed) */ + WPACKET_SUB *subs; }; /* Flags */ @@ -614,8 +609,9 @@ int WPACKET_set_flags(WPACKET *pkt, unsigned int flags); int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len, size_t lenbytes); int WPACKET_close(WPACKET *pkt); -int WPACKET_get_sub_packet_len(WPACKET *pkt, WPACKET *subpkt, size_t lenbytes); -int WPACKET_get_sub_packet(WPACKET *pkt, WPACKET *subpkt); +int WPACKET_finish(WPACKET *pkt); +int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes); +int WPACKET_start_sub_packet(WPACKET *pkt); int WPACKET_allocate_bytes(WPACKET *pkt, size_t bytes, unsigned char **allocbytes); int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t bytes); diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 7cec0116c0..1f9f6afff8 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -2795,11 +2795,11 @@ int ssl3_set_handshake_header(SSL *s, int htype, unsigned long len) * at that point. * TODO - RENAME ME */ -int ssl3_set_handshake_header2(SSL *s, WPACKET *pkt, WPACKET *body, int htype) +int ssl3_set_handshake_header2(SSL *s, WPACKET *pkt, int htype) { /* Set the content type and 3 bytes for the message len */ if (!WPACKET_put_bytes(pkt, htype, 1) - || !WPACKET_get_sub_packet_len(pkt, body, 3)) + || !WPACKET_start_sub_packet_len(pkt, 3)) return 0; return 1; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index b4aa091453..26485f6570 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1586,8 +1586,7 @@ typedef struct ssl3_enc_method { /* Set the handshake header */ int (*set_handshake_header) (SSL *s, int type, unsigned long len); /* Set the handshake header */ - int (*set_handshake_header2) (SSL *s, WPACKET *pkt, WPACKET *body, - int type); + int (*set_handshake_header2) (SSL *s, WPACKET *pkt, int type); /* Close construction of the handshake message */ int (*close_construct_packet) (SSL *s, WPACKET *pkt); /* Write out handshake message */ @@ -1599,8 +1598,8 @@ typedef struct ssl3_enc_method { (((unsigned char *)s->init_buf->data) + s->method->ssl3_enc->hhlen) # define ssl_set_handshake_header(s, htype, len) \ s->method->ssl3_enc->set_handshake_header(s, htype, len) -# define ssl_set_handshake_header2(s, pkt, body, htype) \ - s->method->ssl3_enc->set_handshake_header2((s), (pkt), (body), (htype)) +# define ssl_set_handshake_header2(s, pkt, htype) \ + s->method->ssl3_enc->set_handshake_header2((s), (pkt), (htype)) # define ssl_close_construct_packet(s, pkt) \ s->method->ssl3_enc->close_construct_packet((s), (pkt)) # define ssl_do_write(s) s->method->ssl3_enc->do_write(s) @@ -1906,11 +1905,9 @@ __owur int ssl3_do_change_cipher_spec(SSL *ssl); __owur long ssl3_default_timeout(void); __owur int ssl3_set_handshake_header(SSL *s, int htype, unsigned long len); -__owur int ssl3_set_handshake_header2(SSL *s, WPACKET *pkt, WPACKET *body, - int htype); +__owur int ssl3_set_handshake_header2(SSL *s, WPACKET *pkt, int htype); __owur int tls_close_construct_packet(SSL *s, WPACKET *pkt); -__owur int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, WPACKET *body, - int htype); +__owur int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, int htype); __owur int dtls1_close_construct_packet(SSL *s, WPACKET *pkt); __owur int ssl3_handshake_write(SSL *s); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index faa44b6c02..cba0ece62f 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -697,7 +697,7 @@ int tls_construct_client_hello(SSL *s) SSL_COMP *comp; #endif SSL_SESSION *sess = s->session; - WPACKET pkt, body, spkt; + WPACKET pkt; if (!WPACKET_init(&pkt, s->init_buf) || !WPACKET_set_max_size(&pkt, SSL3_RT_MAX_PLAIN_LENGTH)) { @@ -746,7 +746,7 @@ int tls_construct_client_hello(SSL *s) if (i && ssl_fill_hello_random(s, 0, p, sizeof(s->s3->client_random)) <= 0) goto err; - if (!ssl_set_handshake_header2(s, &pkt, &body, SSL3_MT_CLIENT_HELLO)) { + if (!ssl_set_handshake_header2(s, &pkt, SSL3_MT_CLIENT_HELLO)) { ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; @@ -782,8 +782,8 @@ int tls_construct_client_hello(SSL *s) * client_version in client hello and not resetting it to * the negotiated version. */ - if (!WPACKET_put_bytes(&body, s->client_version, 2) - || !WPACKET_memcpy(&body, s->s3->client_random, SSL3_RANDOM_SIZE)) { + if (!WPACKET_put_bytes(&pkt, s->client_version, 2) + || !WPACKET_memcpy(&pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -794,9 +794,9 @@ int tls_construct_client_hello(SSL *s) else i = s->session->session_id_length; if (i > (int)sizeof(s->session->session_id) - || !WPACKET_get_sub_packet_len(&body, &spkt, 1) - || (i != 0 && !WPACKET_memcpy(&spkt, s->session->session_id, i)) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(&pkt, 1) + || (i != 0 && !WPACKET_memcpy(&pkt, s->session->session_id, i)) + || !WPACKET_close(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -804,29 +804,29 @@ 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_get_sub_packet_len(&body, &spkt, 1) - || !WPACKET_memcpy(&spkt, s->d1->cookie, s->d1->cookie_len) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(&pkt, 1) + || !WPACKET_memcpy(&pkt, s->d1->cookie, s->d1->cookie_len) + || !WPACKET_close(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } } /* Ciphers supported */ - if (!WPACKET_get_sub_packet_len(&body, &spkt, 2)) { + if (!WPACKET_start_sub_packet_len(&pkt, 2)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } /* ssl_cipher_list_to_bytes() raises SSLerr if appropriate */ - if (!ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &spkt)) + if (!ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &pkt)) goto err; - if (!WPACKET_close(&spkt)) { + if (!WPACKET_close(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } /* COMPRESSION */ - if (!WPACKET_get_sub_packet_len(&body, &spkt, 1)) { + if (!WPACKET_start_sub_packet_len(&pkt, 1)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -835,7 +835,7 @@ int tls_construct_client_hello(SSL *s) int compnum = sk_SSL_COMP_num(s->ctx->comp_methods); for (i = 0; i < compnum; i++) { comp = sk_SSL_COMP_value(s->ctx->comp_methods, i); - if (!WPACKET_put_bytes(&spkt, comp->id, 1)) { + if (!WPACKET_put_bytes(&pkt, comp->id, 1)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -843,7 +843,7 @@ int tls_construct_client_hello(SSL *s) } #endif /* Add the NULL method */ - if (!WPACKET_put_bytes(&spkt, 0, 1) || !WPACKET_close(&spkt)) { + if (!WPACKET_put_bytes(&pkt, 0, 1) || !WPACKET_close(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -853,21 +853,21 @@ int tls_construct_client_hello(SSL *s) SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT); goto err; } - if (!WPACKET_get_sub_packet_len(&body, &spkt, 2) + if (!WPACKET_start_sub_packet_len(&pkt, 2) /* * If extensions are of zero length then we don't even add the * extensions length bytes */ - || !WPACKET_set_flags(&spkt, + || !WPACKET_set_flags(&pkt, OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) - || !ssl_add_clienthello_tlsext(s, &spkt, &al) - || !WPACKET_close(&spkt)) { + || !ssl_add_clienthello_tlsext(s, &pkt, &al) + || !WPACKET_close(&pkt)) { ssl3_send_alert(s, SSL3_AL_FATAL, al); SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } - if (!WPACKET_close(&body) || !ssl_close_construct_packet(s, &pkt)) { + if (!WPACKET_close(&pkt) || !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; diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 492d37736d..18ab7dc8f2 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -1197,7 +1197,7 @@ void dtls1_get_message_header(unsigned char *data, struct hm_header_st *msg_hdr) * at that point. * TODO - RENAME ME */ -int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, WPACKET *body, int htype) +int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, int htype) { unsigned char *header; dtls1_set_message_header(s, htype, 0, 0, 0); @@ -1207,7 +1207,7 @@ int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, WPACKET *body, int htype) * in later */ if (!WPACKET_allocate_bytes(pkt, DTLS1_HM_HEADER_LENGTH, &header) - || !WPACKET_get_sub_packet(pkt, body)) + || !WPACKET_start_sub_packet(pkt)) return 0; return 1; @@ -1219,7 +1219,7 @@ int dtls1_close_construct_packet(SSL *s, WPACKET *pkt) if (!WPACKET_get_length(pkt, &msglen) || msglen > INT_MAX - || !WPACKET_close(pkt)) + || !WPACKET_finish(pkt)) return 0; s->d1->w_msg_hdr.msg_len = msglen - DTLS1_HM_HEADER_LENGTH; s->d1->w_msg_hdr.frag_len = msglen - DTLS1_HM_HEADER_LENGTH; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 3f5628cbc4..7ad38998c8 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -63,7 +63,7 @@ int tls_close_construct_packet(SSL *s, WPACKET *pkt) if (!WPACKET_get_length(pkt, &msglen) || msglen > INT_MAX - || !WPACKET_close(pkt)) + || !WPACKET_finish(pkt)) return 0; s->init_num = (int)msglen; s->init_off = 0; diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c index 734522b110..cf4f5b0b25 100644 --- a/ssl/t1_ext.c +++ b/ssl/t1_ext.c @@ -148,7 +148,6 @@ int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al) for (i = 0; i < exts->meths_count; i++) { const unsigned char *out = NULL; size_t outlen = 0; - WPACKET spkt; meth = exts->meths + i; @@ -173,9 +172,9 @@ int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al) } if (!WPACKET_put_bytes(pkt, meth->ext_type, 2) - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || (outlen > 0 && !WPACKET_memcpy(&spkt, out, outlen)) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || (outlen > 0 && !WPACKET_memcpy(pkt, out, outlen)) + || !WPACKET_close(pkt)) { *al = SSL_AD_INTERNAL_ERROR; return 0; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 0ea80d9a10..41a5035094 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1015,7 +1015,6 @@ static int tls1_check_duplicate_extensions(const PACKET *packet) int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) { - WPACKET spkt; #ifndef OPENSSL_NO_EC /* See if we support any ECC ciphersuites */ int using_ecc = 0; @@ -1041,10 +1040,10 @@ 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_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_memcpy(&spkt, s->s3->previous_client_finished, + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_memcpy(pkt, s->s3->previous_client_finished, s->s3->previous_client_finished_len) - || !WPACKET_close(&spkt)) { + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1055,21 +1054,19 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (s->tlsext_hostname != NULL) { /* Add TLS extension servername to the Client Hello message */ - WPACKET slistpkt, hostpkt; - if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_server_name, 2) /* Sub-packet for server_name extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 2) /* Sub-packet for servername list (always 1 hostname)*/ - || !WPACKET_get_sub_packet_len(&spkt, &slistpkt, 2) - || !WPACKET_put_bytes(&slistpkt, TLSEXT_NAMETYPE_host_name, 1) + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_put_bytes(pkt, TLSEXT_NAMETYPE_host_name, 1) /* Sub-packet for a single hostname host name */ - || !WPACKET_get_sub_packet_len(&slistpkt, &hostpkt, 2) - || !WPACKET_memcpy(&hostpkt, s->tlsext_hostname, + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_memcpy(pkt, s->tlsext_hostname, strlen(s->tlsext_hostname)) - || !WPACKET_close(&hostpkt) - || !WPACKET_close(&slistpkt) - || !WPACKET_close(&spkt)) { + || !WPACKET_close(pkt) + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1077,19 +1074,17 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) #ifndef OPENSSL_NO_SRP /* Add SRP username if there is one */ if (s->srp_ctx.login != NULL) { - WPACKET loginpkt; - if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_srp, 2) /* Sub-packet for SRP extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_get_sub_packet_len(&spkt, &loginpkt, 1) + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 1) /* login must not be zero...internal error if so */ - || !WPACKET_set_flags(&loginpkt, + || !WPACKET_set_flags(pkt, OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) - || !WPACKET_memcpy(&loginpkt, s->srp_ctx.login, + || !WPACKET_memcpy(pkt, s->srp_ctx.login, strlen(s->srp_ctx.login)) - || !WPACKET_close(&loginpkt) - || !WPACKET_close(&spkt)) { + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1098,8 +1093,6 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) #ifndef OPENSSL_NO_EC if (using_ecc) { - WPACKET formatspkt, curveslistpkt; - /* * Add TLS extension ECPointFormats to the ClientHello message */ @@ -1111,11 +1104,11 @@ 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_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_get_sub_packet_len(&spkt, &formatspkt, 1) - || !WPACKET_memcpy(&formatspkt, pformats, num_formats) - || !WPACKET_close(&formatspkt) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 1) + || !WPACKET_memcpy(pkt, pformats, num_formats) + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1131,23 +1124,23 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_elliptic_curves, 2) /* Sub-packet for curves extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_get_sub_packet_len(&spkt, &curveslistpkt, 2)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 2)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } /* Copy curve ID if supported */ for (i = 0; i < num_curves; i++, pcurves += 2) { if (tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) { - if (!WPACKET_put_bytes(&curveslistpkt, pcurves[0], 1) - || !WPACKET_put_bytes(&curveslistpkt, pcurves[1], 1)) { + if (!WPACKET_put_bytes(pkt, pcurves[0], 1) + || !WPACKET_put_bytes(pkt, pcurves[1], 1)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } } } - if (!WPACKET_close(&curveslistpkt) || !WPACKET_close(&spkt)) { + if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1177,9 +1170,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_session_ticket, 2) /* Sub-packet for ticket extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_memcpy(&spkt, s->session->tlsext_tick, ticklen) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_memcpy(pkt, s->session->tlsext_tick, ticklen) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1189,33 +1182,31 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (SSL_CLIENT_USE_SIGALGS(s)) { size_t salglen; const unsigned char *salg; - WPACKET salgslistpkt; salglen = tls12_get_psigalgs(s, &salg); if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_signature_algorithms, 2) /* Sub-packet for sig-algs extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 2) /* Sub-packet for the actual list */ - || !WPACKET_get_sub_packet_len(&spkt, &salgslistpkt, 2) - || !tls12_copy_sigalgs(s, &salgslistpkt, salg, salglen) - || !WPACKET_close(&salgslistpkt) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !tls12_copy_sigalgs(s, pkt, salg, salglen) + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } } #ifndef OPENSSL_NO_OCSP if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp) { - WPACKET idspkt, extpkt; int i; if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_status_request, 2) /* Sub-packet for status request extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_put_bytes(&spkt, TLSEXT_STATUSTYPE_ocsp, 1) + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_put_bytes(pkt, TLSEXT_STATUSTYPE_ocsp, 1) /* Sub-packet for the ids */ - || !WPACKET_get_sub_packet_len(&spkt, &idspkt, 2)) { + || !WPACKET_start_sub_packet_len(pkt, 2)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1223,22 +1214,21 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) unsigned char *idbytes; int idlen; OCSP_RESPID *id; - WPACKET idpkt; id = sk_OCSP_RESPID_value(s->tlsext_ocsp_ids, i); idlen = i2d_OCSP_RESPID(id, NULL); if (idlen <= 0 /* Sub-packet for an individual id */ - || !WPACKET_get_sub_packet_len(&idspkt, &idpkt, 1) - || !WPACKET_allocate_bytes(&idpkt, idlen, &idbytes) + || !WPACKET_start_sub_packet_len(pkt, 1) + || !WPACKET_allocate_bytes(pkt, idlen, &idbytes) || i2d_OCSP_RESPID(id, &idbytes) != idlen - || !WPACKET_close(&idpkt)) { + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } } - if (!WPACKET_close(&idspkt) - || !WPACKET_get_sub_packet_len(&spkt, &extpkt, 2)) { + if (!WPACKET_close(pkt) + || !WPACKET_start_sub_packet_len(pkt, 2)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1250,14 +1240,14 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } - if (!WPACKET_allocate_bytes(&extpkt, extlen, &extbytes) + if (!WPACKET_allocate_bytes(pkt, extlen, &extbytes) || i2d_X509_EXTENSIONS(s->tlsext_ocsp_exts, &extbytes) != extlen) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } } - if (!WPACKET_close(&extpkt) || !WPACKET_close(&spkt)) { + if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1279,9 +1269,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_heartbeat, 2) /* Sub-packet for Hearbeat extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_put_bytes(&spkt, mode, 1) - || !WPACKET_close(&spkt)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_put_bytes(pkt, mode, 1) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1308,18 +1298,16 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) * (see longer comment below) */ if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) { - WPACKET plistpkt; - if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_application_layer_protocol_negotiation, 2) /* Sub-packet ALPN extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 2) /* Sub-packet for ALPN proto list */ - || !WPACKET_get_sub_packet_len(&spkt, &plistpkt, 2) - || !WPACKET_memcpy(&plistpkt, s->alpn_client_proto_list, + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_memcpy(pkt, s->alpn_client_proto_list, s->alpn_client_proto_list_len) - || !WPACKET_close(&plistpkt) - || !WPACKET_close(&spkt)) { + || !WPACKET_close(pkt) + || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1330,25 +1318,24 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) STACK_OF(SRTP_PROTECTION_PROFILE) *clnt = 0; SRTP_PROTECTION_PROFILE *prof; int i, ct; - WPACKET plistpkt; if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_use_srtp, 2) /* Sub-packet for SRTP extension */ - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) + || !WPACKET_start_sub_packet_len(pkt, 2) /* Sub-packet for the protection profile list */ - || !WPACKET_get_sub_packet_len(&spkt, &plistpkt, 2)) { + || !WPACKET_start_sub_packet_len(pkt, 2)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } ct = sk_SRTP_PROTECTION_PROFILE_num(clnt); for (i = 0; i < ct; i++) { prof = sk_SRTP_PROTECTION_PROFILE_value(clnt, i); - if (prof == NULL || !WPACKET_put_bytes(&plistpkt, prof->id, 2)) { + if (prof == NULL || !WPACKET_put_bytes(pkt, prof->id, 2)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } } - if (!WPACKET_close(&plistpkt) || !WPACKET_close(&spkt)) { + if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1406,13 +1393,13 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) hlen = 0; if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_padding, 2) - || !WPACKET_get_sub_packet_len(pkt, &spkt, 2) - || !WPACKET_allocate_bytes(&spkt, hlen, &padbytes)) { + || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_allocate_bytes(pkt, hlen, &padbytes)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } memset(padbytes, 0, hlen); - if (!WPACKET_close(&spkt)) { + if (!WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; }