summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGES6
-rw-r--r--include/openssl/ssl.h1
-rw-r--r--include/openssl/tls1.h1
-rw-r--r--ssl/packet_locl.h63
-rw-r--r--ssl/s3_enc.c2
-rw-r--r--ssl/t1_enc.c2
-rw-r--r--ssl/t1_lib.c513
-rw-r--r--test/packettest.c95
-rw-r--r--test/recipes/80-test_ssl.t11
9 files changed, 415 insertions, 279 deletions
diff --git a/CHANGES b/CHANGES
index 0b8c558b06..618655816f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,12 @@
Changes between 1.0.2g and 1.1.0 [xx XXX xxxx]
+ *) If the server has ALPN configured, but supports no protocols that the
+ client advertises, send a fatal "no_application_protocol" alert.
+ This behaviour is SHALL in RFC 7301, though it isn't universally
+ implemented by other servers.
+ [Emilia Käsper]
+
*) Add X25519 support.
Integrate support for X25519 into EC library. This includes support
for public and private key encoding using the format documented in
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index c9119e345e..6e223960a9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1109,6 +1109,7 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION)
# define SSL_AD_UNKNOWN_PSK_IDENTITY TLS1_AD_UNKNOWN_PSK_IDENTITY
/* fatal */
# define SSL_AD_INAPPROPRIATE_FALLBACK TLS1_AD_INAPPROPRIATE_FALLBACK
+# define SSL_AD_NO_APPLICATION_PROTOCOL TLS1_AD_NO_APPLICATION_PROTOCOL
# define SSL_ERROR_NONE 0
# define SSL_ERROR_SSL 1
# define SSL_ERROR_WANT_READ 2
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 0f0d4a3713..c2fe36430b 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -204,6 +204,7 @@ extern "C" {
# define TLS1_AD_BAD_CERTIFICATE_STATUS_RESPONSE 113
# define TLS1_AD_BAD_CERTIFICATE_HASH_VALUE 114
# define TLS1_AD_UNKNOWN_PSK_IDENTITY 115/* fatal */
+# define TLS1_AD_NO_APPLICATION_PROTOCOL 120 /* fatal */
/* ExtensionType values from RFC3546 / RFC4366 / RFC6066 */
# define TLSEXT_TYPE_server_name 0
diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index 48767b6728..fd1f9f4ecf 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -93,6 +93,16 @@ static ossl_inline size_t PACKET_remaining(const PACKET *pkt)
}
/*
+ * Returns a pointer to the first byte after the packet data.
+ * Useful for integrating with non-PACKET parsing code.
+ * Specifically, we use PACKET_end() to verify that a d2i_... call
+ * has consumed the entire packet contents.
+ */
+static ossl_inline const unsigned char *PACKET_end(const PACKET *pkt)
+{
+ return pkt->curr + pkt->remaining;
+}
+/*
* Returns a pointer to the PACKET's current position.
* For use in non-PACKETized APIs.
*/
@@ -452,6 +462,12 @@ __owur static ossl_inline int PACKET_strndup(const PACKET *pkt, char **data)
return (*data != NULL);
}
+/* Returns 1 if |pkt| contains at least one 0-byte, 0 otherwise. */
+static ossl_inline int PACKET_contains_zero_byte(const PACKET *pkt)
+{
+ return memchr(pkt->curr, 0, pkt->remaining) != NULL;
+}
+
/* Move the current reading position forward |len| bytes */
__owur static ossl_inline int PACKET_forward(PACKET *pkt, size_t len)
{
@@ -489,6 +505,28 @@ __owur static ossl_inline int PACKET_get_length_prefixed_1(PACKET *pkt,
}
/*
+ * Like PACKET_get_length_prefixed_1, but additionally, fails when there are
+ * leftover bytes in |pkt|.
+ */
+__owur static ossl_inline int PACKET_as_length_prefixed_1(PACKET *pkt, PACKET *subpkt)
+{
+ unsigned int length;
+ const unsigned char *data;
+ PACKET tmp = *pkt;
+ if (!PACKET_get_1(&tmp, &length) ||
+ !PACKET_get_bytes(&tmp, &data, (size_t)length) ||
+ PACKET_remaining(&tmp) != 0) {
+ return 0;
+ }
+
+ *pkt = tmp;
+ subpkt->curr = data;
+ subpkt->remaining = length;
+
+ return 1;
+}
+
+/*
* Reads a variable-length vector prefixed with a two-byte length, and stores
* the contents in |subpkt|. |pkt| can equal |subpkt|.
* Data is not copied: the |subpkt| packet will share its underlying buffer with
@@ -501,6 +539,7 @@ __owur static ossl_inline int PACKET_get_length_prefixed_2(PACKET *pkt,
unsigned int length;
const unsigned char *data;
PACKET tmp = *pkt;
+
if (!PACKET_get_net_2(&tmp, &length) ||
!PACKET_get_bytes(&tmp, &data, (size_t)length)) {
return 0;
@@ -514,6 +553,30 @@ __owur static ossl_inline int PACKET_get_length_prefixed_2(PACKET *pkt,
}
/*
+ * Like PACKET_get_length_prefixed_2, but additionally, fails when there are
+ * leftover bytes in |pkt|.
+ */
+__owur static ossl_inline int PACKET_as_length_prefixed_2(PACKET *pkt,
+ PACKET *subpkt)
+{
+ unsigned int length;
+ const unsigned char *data;
+ PACKET tmp = *pkt;
+
+ if (!PACKET_get_net_2(&tmp, &length) ||
+ !PACKET_get_bytes(&tmp, &data, (size_t)length) ||
+ PACKET_remaining(&tmp) != 0) {
+ return 0;
+ }
+
+ *pkt = tmp;
+ subpkt->curr = data;
+ subpkt->remaining = length;
+
+ return 1;
+}
+
+/*
* Reads a variable-length vector prefixed with a three-byte length, and stores
* the contents in |subpkt|. |pkt| can equal |subpkt|.
* Data is not copied: the |subpkt| packet will share its underlying buffer with
diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index d4d64d0e5e..1c493e2807 100644
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -667,6 +667,8 @@ int ssl3_alert_code(int code)
return (TLS1_AD_UNKNOWN_PSK_IDENTITY);
case SSL_AD_INAPPROPRIATE_FALLBACK:
return (TLS1_AD_INAPPROPRIATE_FALLBACK);
+ case SSL_AD_NO_APPLICATION_PROTOCOL:
+ return (TLS1_AD_NO_APPLICATION_PROTOCOL);
default:
return (-1);
}
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 1b2820bff9..21eb3283da 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -792,6 +792,8 @@ int tls1_alert_code(int code)
return (TLS1_AD_UNKNOWN_PSK_IDENTITY);
case SSL_AD_INAPPROPRIATE_FALLBACK:
return (TLS1_AD_INAPPROPRIATE_FALLBACK);
+ case SSL_AD_NO_APPLICATION_PROTOCOL:
+ return (TLS1_AD_NO_APPLICATION_PROTOCOL);
default:
return (-1);
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index f02317e09f..3aa01db7e5 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1732,63 +1732,62 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
}
/*
- * tls1_alpn_handle_client_hello is called to process the ALPN extension in a
- * ClientHello. data: the contents of the extension, not including the type
- * and length. data_len: the number of bytes in |data| al: a pointer to the
- * alert value to send in the event of a non-zero return. returns: 0 on
- * success.
+ * Process the ALPN extension in a ClientHello.
+ * pkt: the contents of the ALPN extension, not including type and length.
+ * al: a pointer to the alert value to send in the event of a failure.
+ * returns: 1 on success, 0 on error.
*/
static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
{
- unsigned int data_len;
- unsigned int proto_len;
const unsigned char *selected;
- const unsigned char *data;
unsigned char selected_len;
int r;
+ PACKET protocol_list, save_protocol_list, protocol;
- if (s->ctx->alpn_select_cb == NULL)
- return 0;
+ *al = SSL_AD_DECODE_ERROR;
- /*
- * data should contain a uint16 length followed by a series of 8-bit,
- * length-prefixed strings.
- */
- if (!PACKET_get_net_2(pkt, &data_len)
- || PACKET_remaining(pkt) != data_len
- || !PACKET_peek_bytes(pkt, &data, data_len))
- goto parse_error;
+ if (!PACKET_as_length_prefixed_2(pkt, &protocol_list)
+ || PACKET_remaining(&protocol_list) < 2) {
+ return 0;
+ }
+ save_protocol_list = protocol_list;
do {
- if (!PACKET_get_1(pkt, &proto_len)
- || proto_len == 0
- || !PACKET_forward(pkt, proto_len))
- goto parse_error;
- } while (PACKET_remaining(pkt));
+ /* Protocol names can't be empty. */
+ if (!PACKET_get_length_prefixed_1(&protocol_list, &protocol)
+ || PACKET_remaining(&protocol) == 0) {
+ return 0;
+ }
+ } while (PACKET_remaining(&protocol_list) != 0);
- r = s->ctx->alpn_select_cb(s, &selected, &selected_len, data, data_len,
+ if (s->ctx->alpn_select_cb == NULL)
+ return 1;
+
+ r = s->ctx->alpn_select_cb(s, &selected, &selected_len,
+ PACKET_data(&save_protocol_list),
+ PACKET_remaining(&save_protocol_list),
s->ctx->alpn_select_cb_arg);
if (r == SSL_TLSEXT_ERR_OK) {
OPENSSL_free(s->s3->alpn_selected);
s->s3->alpn_selected = OPENSSL_malloc(selected_len);
if (s->s3->alpn_selected == NULL) {
*al = SSL_AD_INTERNAL_ERROR;
- return -1;
+ return 0;
}
memcpy(s->s3->alpn_selected, selected, selected_len);
s->s3->alpn_selected_len = selected_len;
+ } else {
+ *al = SSL_AD_NO_APPLICATION_PROTOCOL;
+ return 0;
}
- return 0;
- parse_error:
- *al = SSL_AD_DECODE_ERROR;
- return -1;
+ return 1;
}
#ifndef OPENSSL_NO_EC
/*-
* ssl_check_for_safari attempts to fingerprint Safari using OS X
- * SecureTransport using the TLS extension block in |d|, of length |n|.
+ * SecureTransport using the TLS extension block in |pkt|.
* Safari, since 10.6, sends exactly these extensions, in this order:
* SNI,
* elliptic_curves
@@ -1801,9 +1800,9 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al)
*/
static void ssl_check_for_safari(SSL *s, const PACKET *pkt)
{
- unsigned int type, size;
- const unsigned char *eblock1, *eblock2;
- PACKET tmppkt;
+ unsigned int type;
+ PACKET sni, tmppkt;
+ size_t ext_len;
static const unsigned char kSafariExtensionsBlock[] = {
0x00, 0x0a, /* elliptic_curves extension */
@@ -1817,10 +1816,7 @@ static void ssl_check_for_safari(SSL *s, const PACKET *pkt)
0x00, 0x02, /* 2 bytes */
0x01, /* 1 point format */
0x00, /* uncompressed */
- };
-
- /* The following is only present in TLS 1.2 */
- static const unsigned char kSafariTLS12ExtensionsBlock[] = {
+ /* The following is only present in TLS 1.2 */
0x00, 0x0d, /* signature_algorithms */
0x00, 0x0c, /* 12 bytes */
0x00, 0x0a, /* 10 bytes */
@@ -1831,51 +1827,46 @@ static void ssl_check_for_safari(SSL *s, const PACKET *pkt)
0x02, 0x03, /* SHA-1/ECDSA */
};
+ /* Length of the common prefix (first two extensions). */
+ static const size_t kSafariCommonExtensionsLength = 18;
+
tmppkt = *pkt;
if (!PACKET_forward(&tmppkt, 2)
- || !PACKET_get_net_2(&tmppkt, &type)
- || !PACKET_get_net_2(&tmppkt, &size)
- || !PACKET_forward(&tmppkt, size))
+ || !PACKET_get_net_2(&tmppkt, &type)
+ || !PACKET_get_length_prefixed_2(&tmppkt, &sni)) {
return;
+ }
if (type != TLSEXT_TYPE_server_name)
return;
- if (TLS1_get_client_version(s) >= TLS1_2_VERSION) {
- const size_t len1 = sizeof(kSafariExtensionsBlock);
- const size_t len2 = sizeof(kSafariTLS12ExtensionsBlock);
-
- if (!PACKET_get_bytes(&tmppkt, &eblock1, len1)
- || !PACKET_get_bytes(&tmppkt, &eblock2, len2)
- || PACKET_remaining(&tmppkt))
- return;
- if (memcmp(eblock1, kSafariExtensionsBlock, len1) != 0)
- return;
- if (memcmp(eblock2, kSafariTLS12ExtensionsBlock, len2) != 0)
- return;
- } else {
- const size_t len = sizeof(kSafariExtensionsBlock);
-
- if (!PACKET_get_bytes(&tmppkt, &eblock1, len)
- || PACKET_remaining(&tmppkt))
- return;
- if (memcmp(eblock1, kSafariExtensionsBlock, len) != 0)
- return;
- }
+ ext_len = TLS1_get_client_version(s) >= TLS1_2_VERSION ?
+ sizeof(kSafariExtensionsBlock) : kSafariCommonExtensionsLength;
- s->s3->is_probably_safari = 1;
+ s->s3->is_probably_safari = PACKET_equal(&tmppkt, kSafariExtensionsBlock,
+ ext_len);
}
#endif /* !OPENSSL_NO_EC */
+/*
+ * Parse ClientHello extensions and stash extension info in various parts of
+ * the SSL object. Verify that there are no duplicate extensions.
+ *
+ * Behaviour upon resumption is extension-specific. If the extension has no
+ * effect during resumption, it is parsed (to verify its format) but otherwise
+ * ignored.
+ *
+ * Consumes the entire packet in |pkt|. Returns 1 on success and 0 on failure.
+ * Upon failure, sets |al| to the appropriate alert.
+ */
static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
{
unsigned int type;
- unsigned int size;
- unsigned int len;
- const unsigned char *data;
int renegotiate_seen = 0;
+ PACKET extensions;
+ *al = SSL_AD_DECODE_ERROR;
s->servername_done = 0;
s->tlsext_status_type = -1;
#ifndef OPENSSL_NO_NEXTPROTONEG
@@ -1911,29 +1902,29 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
if (PACKET_remaining(pkt) == 0)
goto ri_check;
- if (!PACKET_get_net_2(pkt, &len))
- goto err;
-
- if (PACKET_remaining(pkt) != len)
- goto err;
-
- if (!tls1_check_duplicate_extensions(pkt))
- goto err;
+ if (!PACKET_as_length_prefixed_2(pkt, &extensions))
+ return 0;
- while (PACKET_get_net_2(pkt, &type) && PACKET_get_net_2(pkt, &size)) {
- PACKET subpkt;
+ if (!tls1_check_duplicate_extensions(&extensions))
+ return 0;
- if (!PACKET_peek_bytes(pkt, &data, size))
- goto err;
+ /*
+ * We parse all extensions to ensure the ClientHello is well-formed but,
+ * unless an extension specifies otherwise, we ignore extensions upon
+ * resumption.
+ */
+ while (PACKET_get_net_2(&extensions, &type)) {
+ PACKET extension;
+ if (!PACKET_get_length_prefixed_2(&extensions, &extension))
+ return 0;
if (s->tlsext_debug_cb)
- s->tlsext_debug_cb(s, 0, type, data, size, s->tlsext_debug_arg);
-
- if (!PACKET_get_sub_packet(pkt, &subpkt, size))
- goto err;
+ s->tlsext_debug_cb(s, 0, type, PACKET_data(&extension),
+ PACKET_remaining(&extension),
+ s->tlsext_debug_arg);
if (type == TLSEXT_TYPE_renegotiate) {
- if (!ssl_parse_clienthello_renegotiate_ext(s, &subpkt, al))
+ if (!ssl_parse_clienthello_renegotiate_ext(s, &extension, al))
return 0;
renegotiate_seen = 1;
} else if (s->version == SSL3_VERSION) {
@@ -1964,219 +1955,185 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
*/
else if (type == TLSEXT_TYPE_server_name) {
- const unsigned char *sdata;
unsigned int servname_type;
- unsigned int dsize;
- PACKET ssubpkt;
-
- if (!PACKET_get_net_2(&subpkt, &dsize)
- || !PACKET_get_sub_packet(&subpkt, &ssubpkt, dsize))
- goto err;
-
- while (PACKET_remaining(&ssubpkt) > 3) {
- if (!PACKET_get_1(&ssubpkt, &servname_type)
- || !PACKET_get_net_2(&ssubpkt, &len)
- || PACKET_remaining(&ssubpkt) < len)
- goto err;
-
- if (s->servername_done == 0)
- switch (servname_type) {
- case TLSEXT_NAMETYPE_host_name:
- if (!s->hit) {
- if (s->session->tlsext_hostname)
- goto err;
-
- if (len > TLSEXT_MAXLEN_host_name) {
- *al = TLS1_AD_UNRECOGNIZED_NAME;
- return 0;
- }
- if ((s->session->tlsext_hostname =
- OPENSSL_malloc(len + 1)) == NULL) {
- *al = TLS1_AD_INTERNAL_ERROR;
- return 0;
- }
- if (!PACKET_copy_bytes(&ssubpkt,
- (unsigned char *)s->session
- ->tlsext_hostname,
- len)) {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- s->session->tlsext_hostname[len] = '\0';
- if (strlen(s->session->tlsext_hostname) != len) {
- OPENSSL_free(s->session->tlsext_hostname);
- s->session->tlsext_hostname = NULL;
- *al = TLS1_AD_UNRECOGNIZED_NAME;
- return 0;
- }
- s->servername_done = 1;
-
- } else {
- if (!PACKET_get_bytes(&ssubpkt, &sdata, len)) {
- *al = SSL_AD_DECODE_ERROR;
- return 0;
- }
- s->servername_done = s->session->tlsext_hostname
- && strlen(s->session->tlsext_hostname) == len
- && strncmp(s->session->tlsext_hostname,
- (char *)sdata, len) == 0;
- }
-
- break;
-
- default:
- break;
- }
+ PACKET sni, hostname;
+
+ if (!PACKET_as_length_prefixed_2(&extension, &sni)
+ /* ServerNameList must be at least 1 byte long. */
+ || PACKET_remaining(&sni) == 0) {
+ return 0;
}
- /* We shouldn't have any bytes left */
- if (PACKET_remaining(&ssubpkt) != 0)
- goto err;
+ /*
+ * Although the server_name extension was intended to be
+ * extensible to new name types, RFC 4366 defined the
+ * syntax inextensibly and OpenSSL 1.0.x parses it as
+ * such.
+ * RFC 6066 corrected the mistake but adding new name types
+ * is nevertheless no longer feasible, so act as if no other
+ * SNI types can exist, to simplify parsing.
+ *
+ * Also note that the RFC permits only one SNI value per type,
+ * i.e., we can only have a single hostname.
+ */
+ if (!PACKET_get_1(&sni, &servname_type)
+ || servname_type != TLSEXT_NAMETYPE_host_name
+ || !PACKET_as_length_prefixed_2(&sni, &hostname)) {
+ return 0;
+ }
+
+ if (!s->hit) {
+ if (PACKET_remaining(&hostname) > TLSEXT_MAXLEN_host_name) {
+ *al = TLS1_AD_UNRECOGNIZED_NAME;
+ return 0;
+ }
+
+ if (PACKET_contains_zero_byte(&hostname)) {
+ *al = TLS1_AD_UNRECOGNIZED_NAME;
+ return 0;
+ }
+
+ if (!PACKET_strndup(&hostname, &s->session->tlsext_hostname)) {
+ *al = TLS1_AD_INTERNAL_ERROR;
+ return 0;
+ }
+
+ s->servername_done = 1;
+ } else {
+ /*
+ * TODO(openssl-team): if the SNI doesn't match, we MUST
+ * fall back to a full handshake.
+ */
+ s->servername_done = s->session->tlsext_hostname
+ && PACKET_equal(&hostname, s->session->tlsext_hostname,
+ strlen(s->session->tlsext_hostname));
+ }
}
#ifndef OPENSSL_NO_SRP
else if (type == TLSEXT_TYPE_srp) {
- if (!PACKET_get_1(&subpkt, &len)
- || s->srp_ctx.login != NULL)
- goto err;
-
- if ((s->srp_ctx.login = OPENSSL_malloc(len + 1)) == NULL)
- return -1;
- if (!PACKET_copy_bytes(&subpkt, (unsigned char *)s->srp_ctx.login,
- len))
- goto err;
- s->srp_ctx.login[len] = '\0';
-
- if (strlen(s->srp_ctx.login) != len
- || PACKET_remaining(&subpkt))
- goto err;
+ PACKET srp_I;
+
+ if (!PACKET_as_length_prefixed_1(&extension, &srp_I))
+ return 0;
+
+ if (PACKET_contains_zero_byte(&srp_I))
+ return 0;
+
+ /*
+ * TODO(openssl-team): currently, we re-authenticate the user
+ * upon resumption. Instead, we MUST ignore the login.
+ */
+ if (!PACKET_strndup(&srp_I, &s->srp_ctx.login)) {
+ *al = TLS1_AD_INTERNAL_ERROR;
+ return 0;
+ }
}
#endif
#ifndef OPENSSL_NO_EC
else if (type == TLSEXT_TYPE_ec_point_formats) {
- unsigned int ecpointformatlist_length;
+ PACKET ec_point_format_list;
- if (!PACKET_get_1(&subpkt, &ecpointformatlist_length)
- || ecpointformatlist_length == 0)
- goto err;
+ if (!PACKET_as_length_prefixed_1(&extension,
+ &ec_point_format_list)
+ || PACKET_remaining(&ec_point_format_list) == 0) {
+ return 0;
+ }
if (!s->hit) {
- OPENSSL_free(s->session->tlsext_ecpointformatlist);
- s->session->tlsext_ecpointformatlist = NULL;
- s->session->tlsext_ecpointformatlist_length = 0;
- if ((s->session->tlsext_ecpointformatlist =
- OPENSSL_malloc(ecpointformatlist_length)) == NULL) {
+ if (!PACKET_memdup(&ec_point_format_list,
+ &s->session->tlsext_ecpointformatlist,
+ &s->session->tlsext_ecpointformatlist_length)) {
*al = TLS1_AD_INTERNAL_ERROR;
return 0;
}
- s->session->tlsext_ecpointformatlist_length =
- ecpointformatlist_length;
- if (!PACKET_copy_bytes(&subpkt,
- s->session->tlsext_ecpointformatlist,
- ecpointformatlist_length))
- goto err;
- } else if (!PACKET_forward(&subpkt, ecpointformatlist_length)) {
- goto err;
- }
- /* We should have consumed all the bytes by now */
- if (PACKET_remaining(&subpkt)) {
- *al = TLS1_AD_DECODE_ERROR;
- return 0;
}
} else if (type == TLSEXT_TYPE_elliptic_curves) {
- unsigned int ellipticcurvelist_length;
+ PACKET elliptic_curve_list;
- /* Each NamedCurve is 2 bytes and we must have at least 1 */
- if (!PACKET_get_net_2(&subpkt, &ellipticcurvelist_length)
- || ellipticcurvelist_length == 0
- || (ellipticcurvelist_length & 1) != 0)
- goto err;
+ /* Each NamedCurve is 2 bytes and we must have at least 1. */
+ if (!PACKET_as_length_prefixed_2(&extension,
+ &elliptic_curve_list)
+ || PACKET_remaining(&elliptic_curve_list) == 0
+ || (PACKET_remaining(&elliptic_curve_list) % 2) != 0) {
+ return 0;
+ }
if (!s->hit) {
- if (s->session->tlsext_ellipticcurvelist)
- goto err;
-
- s->session->tlsext_ellipticcurvelist_length = 0;
- if ((s->session->tlsext_ellipticcurvelist =
- OPENSSL_malloc(ellipticcurvelist_length)) == NULL) {
+ if (!PACKET_memdup(&elliptic_curve_list,
+ &s->session->tlsext_ellipticcurvelist,
+ &s->session->tlsext_ellipticcurvelist_length)) {
*al = TLS1_AD_INTERNAL_ERROR;
return 0;
}
- s->session->tlsext_ellipticcurvelist_length =
- ellipticcurvelist_length;
- if (!PACKET_copy_bytes(&subpkt,
- s->session->tlsext_ellipticcurvelist,
- ellipticcurvelist_length))
- goto err;
- } else if (!PACKET_forward(&subpkt, ellipticcurvelist_length)) {
- goto err;
- }
- /* We should have consumed all the bytes by now */
- if (PACKET_remaining(&subpkt)) {
- goto err;
}
}
#endif /* OPENSSL_NO_EC */
else if (type == TLSEXT_TYPE_session_ticket) {
- if (!PACKET_forward(&subpkt, size)
- || (s->tls_session_ticket_ext_cb &&
- !s->tls_session_ticket_ext_cb(s, data, size,
- s->tls_session_ticket_ext_cb_arg))) {
+ if (s->tls_session_ticket_ext_cb &&
+ !s->tls_session_ticket_ext_cb(s, PACKET_data(&extension),
+ PACKET_remaining(&extension),
+ s->tls_session_ticket_ext_cb_arg)) {
*al = TLS1_AD_INTERNAL_ERROR;
return 0;
}
} else if (type == TLSEXT_TYPE_signature_algorithms) {
- unsigned int dsize;
-
- if (s->s3->tmp.peer_sigalgs
- || !PACKET_get_net_2(&subpkt, &dsize)
- || (dsize & 1) != 0
- || (dsize == 0)
- || !PACKET_get_bytes(&subpkt, &data, dsize)
- || PACKET_remaining(&subpkt) != 0
- || !tls1_save_sigalgs(s, data, dsize)) {
- goto err;
+ PACKET supported_sig_algs;
+
+ if (!PACKET_as_length_prefixed_2(&extension, &supported_sig_algs)
+ || (PACKET_remaining(&supported_sig_algs) % 2) != 0
+ || PACKET_remaining(&supported_sig_algs) == 0) {
+ return 0;
+ }
+
+ if (!s->hit) {
+ if (!tls1_save_sigalgs(s, PACKET_data(&supported_sig_algs),
+ PACKET_remaining(&supported_sig_algs))) {
+ return 0;
+ }
}
} else if (type == TLSEXT_TYPE_status_request) {
- PACKET ssubpkt;
+ const unsigned char *ext_data;
- if (!PACKET_get_1(&subpkt,
- (unsigned int *)&s->tlsext_status_type))
- goto err;
+ if (!PACKET_get_1(&extension,
+ (unsigned int *)&s->tlsext_status_type)) {
+ return 0;
+ }
if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp) {
- const unsigned char *sdata;
- unsigned int dsize;
- /* Read in responder_id_list */
- if (!PACKET_get_net_2(&subpkt, &dsize)
- || !PACKET_get_sub_packet(&subpkt, &ssubpkt, dsize))
- goto err;
-
- while (PACKET_remaining(&ssubpkt)) {
+ PACKET responder_id_list, exts;
+ if (!PACKET_get_length_prefixed_2(&extension, &responder_id_list))
+ return 0;
+
+ while (PACKET_remaining(&responder_id_list) > 0) {
OCSP_RESPID *id;
- unsigned int idsize;
+ PACKET responder_id;
+ const unsigned char *id_data;
- if (PACKET_remaining(&ssubpkt) < 4
- || !PACKET_get_net_2(&ssubpkt, &idsize)
- || !PACKET_get_bytes(&ssubpkt, &data, idsize)) {
- goto err;
+ if (!PACKET_get_length_prefixed_2(&responder_id_list,
+ &responder_id)
+ || PACKET_remaining(&responder_id) == 0) {
+ return 0;
}
- sdata = data;
- data += idsize;
- id = d2i_OCSP_RESPID(NULL, &sdata, idsize);
- if (!id)
- goto err;
- if (data != sdata) {
- OCSP_RESPID_free(id);
- goto err;
+
+ if (s->tlsext_ocsp_ids == NULL
+ && (s->tlsext_ocsp_ids =
+ sk_OCSP_RESPID_new_null()) == NULL) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ return 0;
}
- if (!s->tlsext_ocsp_ids
- && !(s->tlsext_ocsp_ids =
- sk_OCSP_RESPID_new_null())) {
+
+ id_data = PACKET_data(&responder_id);
+ id = d2i_OCSP_RESPID(NULL, &id_data,
+ PACKET_remaining(&responder_id));
+ if (id == NULL)
+ return 0;
+
+ if (id_data != PACKET_end(&responder_id)) {
OCSP_RESPID_free(id);
- *al = SSL_AD_INTERNAL_ERROR;
return 0;
}
+
if (!sk_OCSP_RESPID_push(s->tlsext_ocsp_ids, id)) {
OCSP_RESPID_free(id);
*al = SSL_AD_INTERNAL_ERROR;
@@ -2185,33 +2142,34 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
}
/* Read in request_extensions */
- if (!PACKET_get_net_2(&subpkt, &dsize)
- || !PACKET_get_bytes(&subpkt, &data, dsize)
- || PACKET_remaining(&subpkt)) {
- goto err;
- }
- sdata = data;
- if (dsize > 0) {
+ if (!PACKET_as_length_prefixed_2(&extension, &exts))
+ return 0;
+
+ if (PACKET_remaining(&exts) > 0) {
+ ext_data = PACKET_data(&exts);
sk_X509_EXTENSION_pop_free(s->tlsext_ocsp_exts,
X509_EXTENSION_free);
s->tlsext_ocsp_exts =
- d2i_X509_EXTENSIONS(NULL, &sdata, dsize);
- if (!s->tlsext_ocsp_exts || (data + dsize != sdata))
- goto err;
+ d2i_X509_EXTENSIONS(NULL, &ext_data,
+ PACKET_remaining(&exts));
+ if (s->tlsext_ocsp_exts == NULL
+ || ext_data != PACKET_end(&exts)) {
+ return 0;
+ }
}
- }
/*
* We don't know what to do with any other type * so ignore it.
*/
- else
+ } else {
s->tlsext_status_type = -1;
+ }
}
#ifndef OPENSSL_NO_HEARTBEATS
else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_heartbeat) {
unsigned int hbtype;
- if (!PACKET_get_1(&subpkt, &hbtype)
- || PACKET_remaining(&subpkt)) {
+ if (!PACKET_get_1(&extension, &hbtype)
+ || PACKET_remaining(&extension)) {
*al = SSL_AD_DECODE_ERROR;
return 0;
}
@@ -2255,8 +2213,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
#endif
else if (type == TLSEXT_TYPE_application_layer_protocol_negotiation &&
- s->ctx->alpn_select_cb && s->s3->tmp.finish_md_len == 0) {
- if (tls1_alpn_handle_client_hello(s, &subpkt, al) != 0)
+ s->s3->tmp.finish_md_len == 0) {
+ if (!tls1_alpn_handle_client_hello(s, &extension, al))
return 0;
#ifndef OPENSSL_NO_NEXTPROTONEG
/* ALPN takes precedence over NPN. */
@@ -2268,7 +2226,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
#ifndef OPENSSL_NO_SRTP
else if (SSL_IS_DTLS(s) && SSL_get_srtp_profiles(s)
&& type == TLSEXT_TYPE_use_srtp) {
- if (ssl_parse_clienthello_use_srtp_ext(s, &subpkt, al))
+ if (ssl_parse_clienthello_use_srtp_ext(s, &extension, al))
return 0;
}
#endif
@@ -2289,14 +2247,17 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
* ServerHello may be later returned.
*/
else if (!s->hit) {
- if (custom_ext_parse(s, 1, type, data, size, al) <= 0)
+ if (custom_ext_parse(s, 1, type, PACKET_data(&extension),
+ PACKET_remaining(&extension), al) <= 0)
return 0;
}
}
- /* Spurious data on the end */
- if (PACKET_remaining(pkt) != 0)
- goto err;
+ if (PACKET_remaining(pkt) != 0) {
+ /* tls1_check_duplicate_extensions should ensure this never happens. */
+ *al = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
ri_check:
@@ -2310,10 +2271,13 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
return 0;
}
+ /*
+ * This function currently has no state to clean up, so it returns directly.
+ * If parsing fails at any point, the function returns early.
+ * The SSL object may be left with partial data from extensions, but it must
+ * then no longer be used, and clearing it up will free the leftovers.
+ */
return 1;
-err:
- *al = SSL_AD_DECODE_ERROR;
- return 0;
}
int ssl_parse_clienthello_tlsext(SSL *s, PACKET *pkt)
@@ -2324,7 +2288,6 @@ int ssl_parse_clienthello_tlsext(SSL *s, PACKET *pkt)
ssl3_send_alert(s, SSL3_AL_FATAL, al);
return 0;
}
-
if (ssl_check_clienthello_tlsext_early(s) <= 0) {
SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, SSL_R_CLIENTHELLO_TLSEXT);
return 0;
diff --git a/test/packettest.c b/test/packettest.c
index 0555c7be74..57ef51bcf2 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -77,6 +77,24 @@ static int test_PACKET_remaining(unsigned char buf[BUF_LEN])
return 1;
}
+static int test_PACKET_end(unsigned char buf[BUF_LEN])
+{
+ PACKET pkt;
+
+ if ( !PACKET_buf_init(&pkt, buf, BUF_LEN)
+ || PACKET_remaining(&pkt) != BUF_LEN
+ || PACKET_end(&pkt) != buf + BUF_LEN
+ || !PACKET_forward(&pkt, BUF_LEN - 1)
+ || PACKET_end(&pkt) != buf + BUF_LEN
+ || !PACKET_forward(&pkt, 1)
+ || PACKET_end(&pkt) != buf + BUF_LEN) {
+ fprintf(stderr, "test_PACKET_end() failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
static int test_PACKET_get_1(unsigned char buf[BUF_LEN])
{
unsigned int i;
@@ -308,6 +326,26 @@ static int test_PACKET_strndup()
return 1;
}
+static int test_PACKET_contains_zero_byte()
+{
+ char buf[10], buf2[10];
+ PACKET pkt;
+
+ memset(buf, 'x', 10);
+ memset(buf2, 'y', 10);
+ buf2[5] = '\0';
+
+ if ( !PACKET_buf_init(&pkt, (unsigned char*)buf, 10)
+ || PACKET_contains_zero_byte(&pkt)
+ || !PACKET_buf_init(&pkt, (unsigned char*)buf2, 10)
+ || !PACKET_contains_zero_byte(&pkt)) {
+ fprintf(stderr, "test_PACKET_contains_zero_byte failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
static int test_PACKET_forward(unsigned char buf[BUF_LEN])
{
const unsigned char *byte;
@@ -457,6 +495,57 @@ static int test_PACKET_get_length_prefixed_3()
return 1;
}
+static int test_PACKET_as_length_prefixed_1()
+{
+ unsigned char buf[BUF_LEN];
+ const size_t len = 16;
+ unsigned int i;
+ PACKET pkt, exact_pkt, subpkt;
+
+ buf[0] = len;
+ for (i = 1; i < BUF_LEN; i++) {
+ buf[i] = (i * 2) & 0xff;
+ }
+
+ if ( !PACKET_buf_init(&pkt, buf, BUF_LEN)
+ || !PACKET_buf_init(&exact_pkt, buf, len + 1)
+ || PACKET_as_length_prefixed_1(&pkt, &subpkt)
+ || PACKET_remaining(&pkt) != BUF_LEN
+ || !PACKET_as_length_prefixed_1(&exact_pkt, &subpkt)
+ || PACKET_remaining(&exact_pkt) != 0
+ || PACKET_remaining(&subpkt) != len) {
+ fprintf(stderr, "test_PACKET_as_length_prefixed_1() failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
+static int test_PACKET_as_length_prefixed_2()
+{
+ unsigned char buf[1024];
+ const size_t len = 516; /* 0x0204 */
+ unsigned int i;
+ PACKET pkt, exact_pkt, subpkt;
+
+ for (i = 1; i <= 1024; i++) {
+ buf[i-1] = (i * 2) & 0xff;
+ }
+
+ if ( !PACKET_buf_init(&pkt, buf, 1024)
+ || !PACKET_buf_init(&exact_pkt, buf, len + 2)
+ || PACKET_as_length_prefixed_2(&pkt, &subpkt)
+ || PACKET_remaining(&pkt) != 1024
+ || !PACKET_as_length_prefixed_2(&exact_pkt, &subpkt)
+ || PACKET_remaining(&exact_pkt) != 0
+ || PACKET_remaining(&subpkt) != len) {
+ fprintf(stderr, "test_PACKET_as_length_prefixed_2() failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
int main(int argc, char **argv)
{
unsigned char buf[BUF_LEN];
@@ -470,6 +559,7 @@ int main(int argc, char **argv)
if ( !test_PACKET_buf_init()
|| !test_PACKET_null_init()
|| !test_PACKET_remaining(buf)
+ || !test_PACKET_end(buf)
|| !test_PACKET_equal(buf)
|| !test_PACKET_get_1(buf)
|| !test_PACKET_get_4(buf)
@@ -482,10 +572,13 @@ int main(int argc, char **argv)
|| !test_PACKET_copy_all(buf)
|| !test_PACKET_memdup(buf)
|| !test_PACKET_strndup()
+ || !test_PACKET_contains_zero_byte()
|| !test_PACKET_forward(buf)
|| !test_PACKET_get_length_prefixed_1()
|| !test_PACKET_get_length_prefixed_2()
- || !test_PACKET_get_length_prefixed_3()) {
+ || !test_PACKET_get_length_prefixed_3()
+ || !test_PACKET_as_length_prefixed_1()
+ || !test_PACKET_as_length_prefixed_2()) {
return 1;
}
printf("PASS\n");
diff --git a/test/recipes/80-test_ssl.t b/test/recipes/80-test_ssl.t
index e0f2fc5513..bcab4b5f78 100644
--- a/test/recipes/80-test_ssl.t
+++ b/test/recipes/80-test_ssl.t
@@ -606,20 +606,25 @@ sub testssl {
subtest 'ALPN tests' => sub {
######################################################################
- plan tests => 12;
+ plan tests => 14;
SKIP: {
skip "TLSv1.0 is not supported by this OpenSSL build", 12
if $no_tls1;
- ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "bar"])));
+ ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo"])));
+ ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_server", "foo"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "foo", "-alpn_expected", "foo"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo,bar", "-alpn_server", "foo", "-alpn_expected", "foo"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "foo", "-alpn_expected", "foo"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "foo,bar", "-alpn_expected", "foo"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "bar,foo", "-alpn_server", "bar,foo", "-alpn_expected", "bar"])));
ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo,bar", "-alpn_server", "bar,foo", "-alpn_expected", "bar"])));
- ok(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "baz", "-alpn_server", "bar,foo"])));
+
+ is(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "foo", "-alpn_server", "bar"])), 0,
+ "Testing ALPN with protocol mismatch, expecting failure");
+ is(run(test([@ssltest, "-bio_pair", "-tls1", "-alpn_client", "baz", "-alpn_server", "bar,foo"])), 0,
+ "Testing ALPN with protocol mismatch, expecting failure");
SKIP: {
skip "skipping SRP tests", 4
View Lorry Controller Status