From de451856f08364ad6c6659b6eacbe820edc2aab9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 9 Sep 2016 00:13:41 +0100 Subject: [PATCH] Address WPACKET review comments A few style tweaks here and there. The main change is that curr and packet_len are now offsets into the buffer to account for the fact that the pointers can change if the buffer grows. Also dropped support for the WPACKET_set_packet_len() function. I thought that was going to be needed but so far it hasn't been. It doesn't really work any more due to the offsets change. Reviewed-by: Rich Salz --- ssl/packet.c | 80 ++++++++++++----------------------- ssl/packet_locl.h | 45 +++++++++++--------- ssl/s3_lib.c | 2 +- ssl/statem/statem_clnt.c | 11 +++-- ssl/statem/statem_dtls.c | 1 + ssl/t1_ext.c | 2 +- ssl/t1_lib.c | 39 +++++++++-------- test/wpackettest.c | 90 +++++++++------------------------------- 8 files changed, 99 insertions(+), 171 deletions(-) diff --git a/ssl/packet.c b/ssl/packet.c index e75193ddc3..d984938b6d 100644 --- a/ssl/packet.c +++ b/ssl/packet.c @@ -25,23 +25,14 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes) if (pkt->buf->length > SIZE_MAX / 2) { newlen = SIZE_MAX; } else { - if (pkt->buf->length == 0) - newlen = DEFAULT_BUF_SIZE; - else - newlen = pkt->buf->length * 2; + newlen = (pkt->buf->length == 0) ? DEFAULT_BUF_SIZE + : pkt->buf->length * 2; } if (BUF_MEM_grow(pkt->buf, newlen) == 0) return 0; - if (pkt->curr == NULL) { - /* - * Can happen if initialised with a BUF_MEM that hasn't been - * pre-allocated. - */ - pkt->curr = (unsigned char *)pkt->buf->data; - } } pkt->written += len; - *allocbytes = pkt->curr; + *allocbytes = (unsigned char *)pkt->buf->data + pkt->curr; pkt->curr += len; return 1; @@ -57,12 +48,14 @@ static size_t maxmaxsize(size_t lenbytes) int WPACKET_init_len(WPACKET *pkt, BUF_MEM *buf, size_t lenbytes) { + unsigned char *lenchars; + /* Sanity check */ if (buf == NULL) return 0; pkt->buf = buf; - pkt->curr = (unsigned char *)buf->data; + pkt->curr = 0; pkt->written = 0; pkt->maxsize = maxmaxsize(lenbytes); @@ -76,11 +69,12 @@ int WPACKET_init_len(WPACKET *pkt, BUF_MEM *buf, size_t lenbytes) pkt->subs->pwritten = lenbytes; pkt->subs->lenbytes = lenbytes; - if (!WPACKET_allocate_bytes(pkt, lenbytes, &(pkt->subs->packet_len))) { + if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)) { OPENSSL_free(pkt->subs); pkt->subs = NULL; return 0; } + pkt->subs->packet_len = lenchars - (unsigned char *)pkt->buf->data; return 1; } @@ -90,25 +84,6 @@ int WPACKET_init(WPACKET *pkt, BUF_MEM *buf) return WPACKET_init_len(pkt, buf, 0); } -int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len, - size_t lenbytes) -{ - size_t maxmax; - - /* We only allow this to be set once */ - if (pkt->subs == NULL || pkt->subs->lenbytes != 0) - return 0; - - pkt->subs->lenbytes = lenbytes; - pkt->subs->packet_len = packet_len; - - maxmax = maxmaxsize(lenbytes); - if (pkt->maxsize > maxmax) - pkt->maxsize = maxmax; - - return 1; -} - int WPACKET_set_flags(WPACKET *pkt, unsigned int flags) { if (pkt->subs == NULL) @@ -126,16 +101,15 @@ int WPACKET_set_flags(WPACKET *pkt, unsigned int flags) */ static int wpacket_intern_close(WPACKET *pkt) { - size_t packlen; WPACKET_SUB *sub = pkt->subs; + size_t packlen = pkt->written - sub->pwritten; - packlen = pkt->written - sub->pwritten; if (packlen == 0 - && sub->flags & OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) + && sub->flags & WPACKET_FLAGS_NON_ZERO_LENGTH) return 0; if (packlen == 0 - && sub->flags & OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) { + && sub->flags & WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) { /* Deallocate any bytes allocated for the length of the WPACKET */ if ((pkt->curr - sub->lenbytes) == sub->packet_len) { pkt->written -= sub->lenbytes; @@ -143,17 +117,16 @@ static int wpacket_intern_close(WPACKET *pkt) } /* Don't write out the packet length */ - sub->packet_len = NULL; + sub->packet_len = 0; + sub->lenbytes = 0; } /* Write out the WPACKET length if needed */ - if (sub->packet_len != NULL) { + if (sub->lenbytes > 0) { size_t lenbytes; - lenbytes = sub->lenbytes; - - for (; lenbytes > 0; lenbytes--) { - sub->packet_len[lenbytes - 1] + for (lenbytes = sub->lenbytes; lenbytes > 0; lenbytes--) { + pkt->buf->data[sub->packet_len + lenbytes - 1] = (unsigned char)(packlen & 0xff); packlen >>= 8; } @@ -187,17 +160,18 @@ int WPACKET_finish(WPACKET *pkt) return 0; ret = wpacket_intern_close(pkt); - if (ret) { OPENSSL_free(pkt->subs); pkt->subs = NULL; } + return ret; } int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes) { WPACKET_SUB *sub; + unsigned char *lenchars; if (pkt->subs == NULL) return 0; @@ -212,13 +186,13 @@ int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes) sub->lenbytes = lenbytes; if (lenbytes == 0) { - sub->packet_len = NULL; + sub->packet_len = 0; return 1; } - if (!WPACKET_allocate_bytes(pkt, lenbytes, &sub->packet_len)) { + if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)) return 0; - } + sub->packet_len = lenchars - (unsigned char *)pkt->buf->data; return 1; } @@ -228,16 +202,15 @@ int WPACKET_start_sub_packet(WPACKET *pkt) return WPACKET_start_sub_packet_len(pkt, 0); } -int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t bytes) +int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t size) { unsigned char *data; - if (bytes > sizeof(unsigned int) - || !WPACKET_allocate_bytes(pkt, bytes, &data)) + if (size > sizeof(unsigned int) + || !WPACKET_allocate_bytes(pkt, size, &data)) return 0; - data += bytes - 1; - for (; bytes > 0; bytes--) { + for (data += size - 1; size > 0; size--) { *data = (unsigned char)(val & 0xff); data--; val >>= 8; @@ -259,7 +232,8 @@ int WPACKET_set_max_size(WPACKET *pkt, size_t maxsize) return 0; /* Find the WPACKET_SUB for the top level */ - for (sub = pkt->subs; sub->parent != NULL; sub = sub->parent); + for (sub = pkt->subs; sub->parent != NULL; sub = sub->parent) + continue; lenbytes = sub->lenbytes; if (lenbytes == 0) diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index 255a8a508a..daef69e30e 100644 --- a/ssl/packet_locl.h +++ b/ssl/packet_locl.h @@ -557,12 +557,12 @@ struct wpacket_sub { WPACKET_SUB *parent; /* - * Pointer to where the length of this WPACKET goes (or NULL if we don't - * write the length) + * Offset into the buffer where the length of this WPACKET goes. We use an + * offset in case the buffer grows and gets reallocated. */ - unsigned char *packet_len; + size_t packet_len; - /* Number of bytes in the packet_len */ + /* Number of bytes in the packet_len or 0 if we don't write the length */ size_t lenbytes; /* Number of bytes written to the buf prior to this packet starting */ @@ -577,8 +577,11 @@ struct wpacket_st { /* The buffer where we store the output data */ BUF_MEM *buf; - /* Pointer to where we are currently writing */ - unsigned char *curr; + /* + * Offset into the buffer where we are currently writing. We use an offset + * in case the buffer grows and gets reallocated. + */ + size_t curr; /* Number of bytes written so far */ size_t written; @@ -593,16 +596,16 @@ struct wpacket_st { /* Flags */ /* Default */ -#define OPENSSL_WPACKET_FLAGS_NONE 0 +#define WPACKET_FLAGS_NONE 0 /* Error on WPACKET_close() if no data written to the WPACKET */ -#define OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH 1 +#define WPACKET_FLAGS_NON_ZERO_LENGTH 1 /* * Abandon all changes on WPACKET_close() if no data written to the WPACKET, * i.e. this does not write out a zero packet length */ -#define OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH 2 +#define WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH 2 /* @@ -624,17 +627,6 @@ int WPACKET_init(WPACKET *pkt, BUF_MEM *buf); */ int WPACKET_set_flags(WPACKET *pkt, unsigned int flags); -/* - * Set the WPACKET length, and the location for where we should write that - * length. Normally this will be at the start of the WPACKET, and therefore - * the WPACKET would have been initialised via WPACKET_init_len(). However there - * is the possibility that the length needs to be written to some other location - * other than the start of the WPACKET. In that case init via WPACKET_init() and - * then set the location for the length using this function. - */ -int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len, - size_t lenbytes); - /* * 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 @@ -655,6 +647,19 @@ int WPACKET_finish(WPACKET *pkt); */ 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) +#define WPACKET_start_sub_packet_u16(pkt) \ + WPACKET_start_sub_packet_len((pkt), 2) +#define WPACKET_start_sub_packet_u24(pkt) \ + WPACKET_start_sub_packet_len((pkt), 3) +#define WPACKET_start_sub_packet_u32(pkt) \ + 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. diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 1f9f6afff8..3749b2c8f7 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -2799,7 +2799,7 @@ 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_start_sub_packet_len(pkt, 3)) + || !WPACKET_start_sub_packet_u24(pkt)) return 0; return 1; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 4f123ddc98..59d21df68a 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -794,7 +794,7 @@ int tls_construct_client_hello(SSL *s) else i = s->session->session_id_length; if (i > (int)sizeof(s->session->session_id) - || !WPACKET_start_sub_packet_len(&pkt, 1) + || !WPACKET_start_sub_packet_u8(&pkt) || (i != 0 && !WPACKET_memcpy(&pkt, s->session->session_id, i)) || !WPACKET_close(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); @@ -812,7 +812,7 @@ int tls_construct_client_hello(SSL *s) } /* Ciphers supported */ - if (!WPACKET_start_sub_packet_len(&pkt, 2)) { + if (!WPACKET_start_sub_packet_u16(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -825,7 +825,7 @@ int tls_construct_client_hello(SSL *s) } /* COMPRESSION */ - if (!WPACKET_start_sub_packet_len(&pkt, 1)) { + if (!WPACKET_start_sub_packet_u8(&pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto err; } @@ -852,13 +852,12 @@ int tls_construct_client_hello(SSL *s) SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT); goto err; } - if (!WPACKET_start_sub_packet_len(&pkt, 2) + if (!WPACKET_start_sub_packet_u16(&pkt) /* * If extensions are of zero length then we don't even add the * extensions length bytes */ - || !WPACKET_set_flags(&pkt, - OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) || !ssl_add_clienthello_tlsext(s, &pkt, &al) || !WPACKET_close(&pkt)) { ssl3_send_alert(s, SSL3_AL_FATAL, al); diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index 18ab7dc8f2..35f25f1f1e 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -1200,6 +1200,7 @@ void dtls1_get_message_header(unsigned char *data, struct hm_header_st *msg_hdr) int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, int htype) { unsigned char *header; + dtls1_set_message_header(s, htype, 0, 0, 0); /* diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c index cf4f5b0b25..664906c1c2 100644 --- a/ssl/t1_ext.c +++ b/ssl/t1_ext.c @@ -172,7 +172,7 @@ int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al) } if (!WPACKET_put_bytes(pkt, meth->ext_type, 2) - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || (outlen > 0 && !WPACKET_memcpy(pkt, out, outlen)) || !WPACKET_close(pkt)) { *al = SSL_AD_INTERNAL_ERROR; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index d6bc63ae48..024556e72c 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1054,9 +1054,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) /* Add TLS extension servername to the Client Hello message */ if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_server_name, 2) /* Sub-packet for server_name extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) /* Sub-packet for servername list (always 1 hostname)*/ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !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) @@ -1071,11 +1071,10 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (s->srp_ctx.login != NULL) { if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_srp, 2) /* Sub-packet for SRP extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) - || !WPACKET_start_sub_packet_len(pkt, 1) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_start_sub_packet_u8(pkt) /* login must not be zero...internal error if so */ - || !WPACKET_set_flags(pkt, - OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) + || !WPACKET_set_flags(pkt, WPACKET_FLAGS_NON_ZERO_LENGTH) || !WPACKET_memcpy(pkt, s->srp_ctx.login, strlen(s->srp_ctx.login)) || !WPACKET_close(pkt) @@ -1099,7 +1098,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_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_sub_memcpy(pkt, pformats, num_formats, 1) || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); @@ -1117,8 +1116,8 @@ 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_start_sub_packet_len(pkt, 2) - || !WPACKET_start_sub_packet_len(pkt, 2)) { + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_start_sub_packet_u16(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1178,9 +1177,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_signature_algorithms, 2) /* Sub-packet for sig-algs extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) /* Sub-packet for the actual list */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !tls12_copy_sigalgs(s, pkt, salg, salglen) || !WPACKET_close(pkt) || !WPACKET_close(pkt)) { @@ -1194,10 +1193,10 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_status_request, 2) /* Sub-packet for status request extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_put_bytes(pkt, TLSEXT_STATUSTYPE_ocsp, 1) /* Sub-packet for the ids */ - || !WPACKET_start_sub_packet_len(pkt, 2)) { + || !WPACKET_start_sub_packet_u16(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1210,7 +1209,7 @@ 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_len(pkt, 1) + || !WPACKET_start_sub_packet_u8(pkt) || !WPACKET_allocate_bytes(pkt, idlen, &idbytes) || i2d_OCSP_RESPID(id, &idbytes) != idlen || !WPACKET_close(pkt)) { @@ -1219,7 +1218,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) } } if (!WPACKET_close(pkt) - || !WPACKET_start_sub_packet_len(pkt, 2)) { + || !WPACKET_start_sub_packet_u16(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1260,7 +1259,7 @@ 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_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_put_bytes(pkt, mode, 1) || !WPACKET_close(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); @@ -1292,7 +1291,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_application_layer_protocol_negotiation, 2) /* Sub-packet ALPN extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_sub_memcpy(pkt, s->alpn_client_proto_list, s->alpn_client_proto_list_len, 2) || !WPACKET_close(pkt)) { @@ -1309,9 +1308,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_use_srtp, 2) /* Sub-packet for SRTP extension */ - || !WPACKET_start_sub_packet_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) /* Sub-packet for the protection profile list */ - || !WPACKET_start_sub_packet_len(pkt, 2)) { + || !WPACKET_start_sub_packet_u16(pkt)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; } @@ -1381,7 +1380,7 @@ 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_len(pkt, 2) + || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_allocate_bytes(pkt, hlen, &padbytes)) { SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); return 0; diff --git a/test/wpackettest.c b/test/wpackettest.c index 79248e3ba8..ca2a1a79c9 100644 --- a/test/wpackettest.c +++ b/test/wpackettest.c @@ -98,45 +98,10 @@ static int test_WPACKET_init(void) return 1; } -static int test_WPACKET_set_packet_len(void) -{ - WPACKET pkt; - size_t written; - unsigned char len; - - /* - * Calling set_packet_len when the packet len is already set - * should fail - */ - if ( !WPACKET_init_len(&pkt, buf, 1) - || WPACKET_set_packet_len(&pkt, &len, sizeof(len)) - || !WPACKET_finish(&pkt)) { - testfail("test_WPACKET_set_packet_len():1 failed\n", &pkt); - return 0; - } - - if ( !WPACKET_init(&pkt, buf) - || !WPACKET_set_packet_len(&pkt, &len, sizeof(len)) - /* Can't set it again */ - || WPACKET_set_packet_len(&pkt, &len, sizeof(len)) - || !WPACKET_put_bytes(&pkt, 0xff, 1) - || !WPACKET_finish(&pkt) - || !WPACKET_get_total_written(&pkt, &written) - || written != sizeof(simple1) - || memcmp(buf->data, &simple1, written) != 0 - || len != 1) { - testfail("test_WPACKET_set_packet_len():2 failed\n", &pkt); - return 0; - } - - return 1; -} - static int test_WPACKET_set_max_size(void) { WPACKET pkt; size_t written; - unsigned char len; if ( !WPACKET_init(&pkt, buf) /* @@ -148,16 +113,26 @@ static int test_WPACKET_set_max_size(void) || !WPACKET_set_max_size(&pkt, SIZE_MAX -1) /* And setting it bigger again should be ok */ || !WPACKET_set_max_size(&pkt, SIZE_MAX) - || !WPACKET_set_packet_len(&pkt, &len, 1) + || !WPACKET_finish(&pkt)) { + testfail("test_WPACKET_set_max_size():1 failed\n", &pkt); + return 0; + } + + if ( !WPACKET_init_len(&pkt, buf, 1) + /* + * Should fail because we already consumed 1 byte with the + * length + */ + || WPACKET_set_max_size(&pkt, 0) /* * Max size can't be bigger than biggest that will fit in * lenbytes */ || WPACKET_set_max_size(&pkt, 0x0101) /* It can be the same as the maximum possible size */ - || !WPACKET_set_max_size(&pkt, 0xff) + || !WPACKET_set_max_size(&pkt, 0x0100) /* Or it can be less */ - || !WPACKET_set_max_size(&pkt, 0x00) + || !WPACKET_set_max_size(&pkt, 0x01) /* * Should fail because packet is already filled */ @@ -165,34 +140,13 @@ static int test_WPACKET_set_max_size(void) /* * You can't put in more bytes than max size */ - || !WPACKET_set_max_size(&pkt, 0x01) - || !WPACKET_put_bytes(&pkt, 0xff, 1) - || WPACKET_put_bytes(&pkt, 0xff, 1) - || !WPACKET_finish(&pkt) - || !WPACKET_get_total_written(&pkt, &written) - || written != sizeof(simple1) - || memcmp(buf->data, &simple1, written) != 0 - || len != 1) { - testfail("test_WPACKET_set_max_size():1 failed\n", &pkt); - return 0; - } - - if ( !WPACKET_init_len(&pkt, buf, 1) - /* - * Should fail because we already consumed 1 byte with the - * length - */ - || WPACKET_set_max_size(&pkt, 0) - || !WPACKET_set_max_size(&pkt, 1) - || WPACKET_put_bytes(&pkt, 0xff, 1) - || !WPACKET_set_max_size(&pkt, 2) + || !WPACKET_set_max_size(&pkt, 0x02) || !WPACKET_put_bytes(&pkt, 0xff, 1) || WPACKET_put_bytes(&pkt, 0xff, 1) || !WPACKET_finish(&pkt) || !WPACKET_get_total_written(&pkt, &written) || written != sizeof(simple2) - || memcmp(buf->data, &simple2, written) != 0 - || len != 1) { + || memcmp(buf->data, &simple2, written) != 0) { testfail("test_WPACKET_set_max_size():2 failed\n", &pkt); return 0; } @@ -283,7 +237,7 @@ static int test_WPACKET_set_flags(void) /* Set packet to be non-zero length */ if ( !WPACKET_init(&pkt, buf) - || !WPACKET_set_flags(&pkt, OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH) /* Should fail because of zero length */ || WPACKET_finish(&pkt) || !WPACKET_put_bytes(&pkt, 0xff, 1) @@ -298,7 +252,7 @@ static int test_WPACKET_set_flags(void) /* Repeat above test in a sub-packet */ if ( !WPACKET_init(&pkt, buf) || !WPACKET_start_sub_packet(&pkt) - || !WPACKET_set_flags(&pkt, OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH) /* Should fail because of zero length */ || WPACKET_close(&pkt) || !WPACKET_put_bytes(&pkt, 0xff, 1) @@ -313,8 +267,7 @@ static int test_WPACKET_set_flags(void) /* Set packet to abandon non-zero length */ if ( !WPACKET_init_len(&pkt, buf, 1) - || !WPACKET_set_flags(&pkt, - OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) || !WPACKET_finish(&pkt) || !WPACKET_get_total_written(&pkt, &written) || written != 0) { @@ -325,8 +278,7 @@ 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) - || !WPACKET_set_flags(&pkt, - OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) || !WPACKET_close(&pkt) || !WPACKET_finish(&pkt) || !WPACKET_get_total_written(&pkt, &written) @@ -339,8 +291,7 @@ 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) - || !WPACKET_set_flags(&pkt, - OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) + || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) || !WPACKET_put_bytes(&pkt, 0xff, 1) || !WPACKET_close(&pkt) || !WPACKET_finish(&pkt) @@ -420,7 +371,6 @@ int main(int argc, char *argv[]) buf = BUF_MEM_new(); if (buf != NULL) { ADD_TEST(test_WPACKET_init); - ADD_TEST(test_WPACKET_set_packet_len); ADD_TEST(test_WPACKET_set_max_size); ADD_TEST(test_WPACKET_start_sub_packet); ADD_TEST(test_WPACKET_set_flags); -- GitLab