[openssl-commits] [openssl] OpenSSL_1_1_0-stable update
Matt Caswell
matt at openssl.org
Fri Oct 28 08:46:13 UTC 2016
The branch OpenSSL_1_1_0-stable has been updated
via 5af2ad682e809c04bdc79357ac8cb6571139e098 (commit)
via 3ab5f981ed17adf0b804909d9aeac7419a432f01 (commit)
via 8c9365a690e2d5f0c49f3d9a3d41973ed9dcedcc (commit)
from 3bceb47a272cc930c48b88743c4734a891b1c09a (commit)
- Log -----------------------------------------------------------------
commit 5af2ad682e809c04bdc79357ac8cb6571139e098
Author: Matt Caswell <matt at openssl.org>
Date: Tue Oct 25 11:10:56 2016 +0100
Implement length checks as a macro
Replace the various length checks in the extension code with a macro to
simplify the logic.
Reviewed-by: Rich Salz <rsalz at openssl.org>
commit 3ab5f981ed17adf0b804909d9aeac7419a432f01
Author: Matt Caswell <matt at openssl.org>
Date: Fri Oct 14 13:07:00 2016 +0100
Ensure we have length checks for all extensions
The previous commit inspired a review of all the length checks for the
extension adding code. This adds more robust checks and adds checks where
some were missing previously. The real solution for this is to use WPACKET
which is currently in master - but that cannot be applied to release
branches.
Reviewed-by: Rich Salz <rsalz at openssl.org>
commit 8c9365a690e2d5f0c49f3d9a3d41973ed9dcedcc
Author: Matt Caswell <matt at openssl.org>
Date: Fri Oct 14 11:49:06 2016 +0100
Fix length check writing status request extension
The status request extension did not correctly check its length, meaning
that writing the extension could go 2 bytes beyond the buffer size. In
practice this makes little difference because, due to logic in buffer.c the
buffer is actually over allocated by approximately 5k!
Issue reported by Guido Vranken.
Reviewed-by: Rich Salz <rsalz at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/t1_lib.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 192 insertions(+), 53 deletions(-)
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index a3fb28e..a9fe445 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -20,6 +20,10 @@
#include "ssl_locl.h"
#include <openssl/ct.h>
+
+#define CHECKLEN(curr, val, limit) \
+ (((curr) >= (limit)) || (size_t)((limit) - (curr)) < (size_t)(val))
+
static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen,
const unsigned char *sess_id, int sesslen,
SSL_SESSION **psess);
@@ -1049,7 +1053,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
return NULL;
}
- if ((limit - ret - 4 - el) < 0)
+ if (CHECKLEN(ret, 4 + el, limit))
return NULL;
s2n(TLSEXT_TYPE_renegotiate, ret);
@@ -1068,8 +1072,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
if (s->tlsext_hostname != NULL) {
/* Add TLS extension servername to the Client Hello message */
- unsigned long size_str;
- long lenmax;
+ size_t size_str;
/*-
* check for enough space.
@@ -1079,9 +1082,8 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* 2 for hostname length
* + hostname length
*/
-
- if ((lenmax = limit - ret - 9) < 0
- || (size_str = strlen(s->tlsext_hostname)) > (unsigned long)lenmax)
+ size_str = strlen(s->tlsext_hostname);
+ if (CHECKLEN(ret, 9 + size_str, limit))
return NULL;
/* extension type and length */
@@ -1102,7 +1104,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
if (s->srp_ctx.login != NULL) { /* Add TLS extension SRP username to the
* Client Hello message */
- int login_len = strlen(s->srp_ctx.login);
+ size_t login_len = strlen(s->srp_ctx.login);
if (login_len > 255 || login_len == 0) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
@@ -1114,7 +1116,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* 1 for the srp user identity
* + srp user identity length
*/
- if ((limit - ret - 5 - login_len) < 0)
+ if (CHECKLEN(ret, 5 + login_len, limit))
return NULL;
/* fill in the extension */
@@ -1131,7 +1133,6 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
/*
* Add TLS extension ECPointFormats to the ClientHello message
*/
- long lenmax;
const unsigned char *pcurves, *pformats;
size_t num_curves, num_formats, curves_list_len;
size_t i;
@@ -1139,14 +1140,18 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
tls1_get_formatlist(s, &pformats, &num_formats);
- if ((lenmax = limit - ret - 5) < 0)
- return NULL;
- if (num_formats > (size_t)lenmax)
- return NULL;
if (num_formats > 255) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
}
+ /*-
+ * check for enough space.
+ * 4 bytes for the ec point formats type and extension length
+ * 1 byte for the length of the formats
+ * + formats length
+ */
+ if (CHECKLEN(ret, 5 + num_formats, limit))
+ return NULL;
s2n(TLSEXT_TYPE_ec_point_formats, ret);
/* The point format list has 1-byte length. */
@@ -1162,14 +1167,18 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves))
return NULL;
- if ((lenmax = limit - ret - 6) < 0)
- return NULL;
- if (num_curves > (size_t)lenmax / 2)
- return NULL;
if (num_curves > 65532 / 2) {
SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
}
+ /*-
+ * check for enough space.
+ * 4 bytes for the ec curves type and extension length
+ * 2 bytes for the curve list length
+ * + curve list length
+ */
+ if (CHECKLEN(ret, 6 + (num_curves * 2), limit))
+ return NULL;
s2n(TLSEXT_TYPE_elliptic_curves, ret);
etmp = ret + 4;
@@ -1190,7 +1199,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
#endif /* OPENSSL_NO_EC */
if (tls_use_ticket(s)) {
- int ticklen;
+ size_t ticklen;
if (!s->new_session && s->session && s->session->tlsext_tick)
ticklen = s->session->tlsext_ticklen;
else if (s->session && s->tlsext_session_ticket &&
@@ -1211,11 +1220,11 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* Check for enough room 2 for extension type, 2 for len rest for
* ticket
*/
- if ((long)(limit - ret - 4 - ticklen) < 0)
+ if (CHECKLEN(ret, 4 + ticklen, limit))
return NULL;
s2n(TLSEXT_TYPE_session_ticket, ret);
s2n(ticklen, ret);
- if (ticklen) {
+ if (ticklen > 0) {
memcpy(ret, s->session->tlsext_tick, ticklen);
ret += ticklen;
}
@@ -1227,7 +1236,14 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
const unsigned char *salg;
unsigned char *etmp;
salglen = tls12_get_psigalgs(s, &salg);
- if ((size_t)(limit - ret) < salglen + 6)
+
+ /*-
+ * check for enough space.
+ * 4 bytes for the sigalgs type and extension length
+ * 2 bytes for the sigalg list length
+ * + sigalg list length
+ */
+ if (CHECKLEN(ret, salglen + 6, limit))
return NULL;
s2n(TLSEXT_TYPE_signature_algorithms, ret);
etmp = ret;
@@ -1242,30 +1258,42 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
#ifndef OPENSSL_NO_OCSP
if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp) {
int i;
- long extlen, idlen, itmp;
+ size_t extlen, idlen;
+ int lentmp;
OCSP_RESPID *id;
idlen = 0;
for (i = 0; i < sk_OCSP_RESPID_num(s->tlsext_ocsp_ids); i++) {
id = sk_OCSP_RESPID_value(s->tlsext_ocsp_ids, i);
- itmp = i2d_OCSP_RESPID(id, NULL);
- if (itmp <= 0)
+ lentmp = i2d_OCSP_RESPID(id, NULL);
+ if (lentmp <= 0)
return NULL;
- idlen += itmp + 2;
+ idlen += (size_t)lentmp + 2;
}
if (s->tlsext_ocsp_exts) {
- extlen = i2d_X509_EXTENSIONS(s->tlsext_ocsp_exts, NULL);
- if (extlen < 0)
+ lentmp = i2d_X509_EXTENSIONS(s->tlsext_ocsp_exts, NULL);
+ if (lentmp < 0)
return NULL;
+ extlen = (size_t)lentmp;
} else
extlen = 0;
- if ((long)(limit - ret - 7 - extlen - idlen) < 0)
- return NULL;
- s2n(TLSEXT_TYPE_status_request, ret);
if (extlen + idlen > 0xFFF0)
return NULL;
+ /*
+ * 2 bytes for status request type
+ * 2 bytes for status request len
+ * 1 byte for OCSP request type
+ * 2 bytes for length of ids
+ * 2 bytes for length of extensions
+ * + length of ids
+ * + length of extensions
+ */
+ if (CHECKLEN(ret, 9 + idlen + extlen, limit))
+ return NULL;
+
+ s2n(TLSEXT_TYPE_status_request, ret);
s2n(extlen + idlen + 5, ret);
*(ret++) = TLSEXT_STATUSTYPE_ocsp;
s2n(idlen, ret);
@@ -1275,9 +1303,9 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
id = sk_OCSP_RESPID_value(s->tlsext_ocsp_ids, i);
/* skip over id len */
ret += 2;
- itmp = i2d_OCSP_RESPID(id, &ret);
+ lentmp = i2d_OCSP_RESPID(id, &ret);
/* write id len */
- s2n(itmp, q);
+ s2n(lentmp, q);
}
s2n(extlen, ret);
if (extlen > 0)
@@ -1287,8 +1315,15 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
#ifndef OPENSSL_NO_HEARTBEATS
if (SSL_IS_DTLS(s)) {
/* Add Heartbeat extension */
- if ((limit - ret - 4 - 1) < 0)
+
+ /*-
+ * check for enough space.
+ * 4 bytes for the heartbeat ext type and extension length
+ * 1 byte for the mode
+ */
+ if (CHECKLEN(ret, 5, limit))
return NULL;
+
s2n(TLSEXT_TYPE_heartbeat, ret);
s2n(1, ret);
/*-
@@ -1309,7 +1344,12 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* The client advertises an empty extension to indicate its support
* for Next Protocol Negotiation
*/
- if (limit - ret - 4 < 0)
+
+ /*-
+ * check for enough space.
+ * 4 bytes for the NPN ext type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
return NULL;
s2n(TLSEXT_TYPE_next_proto_neg, ret);
s2n(0, ret);
@@ -1322,7 +1362,13 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* (see longer comment below)
*/
if (s->alpn_client_proto_list && !s->s3->tmp.finish_md_len) {
- if ((size_t)(limit - ret) < 6 + s->alpn_client_proto_list_len)
+ /*-
+ * check for enough space.
+ * 4 bytes for the ALPN type and extension length
+ * 2 bytes for the ALPN protocol list length
+ * + ALPN protocol list length
+ */
+ if (CHECKLEN(ret, 6 + s->alpn_client_proto_list_len, limit))
return NULL;
s2n(TLSEXT_TYPE_application_layer_protocol_negotiation, ret);
s2n(2 + s->alpn_client_proto_list_len, ret);
@@ -1341,7 +1387,12 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
return NULL;
}
- if ((limit - ret - 4 - el) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the SRTP type and extension length
+ * + SRTP profiles length
+ */
+ if (CHECKLEN(ret, 4 + el, limit))
return NULL;
s2n(TLSEXT_TYPE_use_srtp, ret);
@@ -1365,16 +1416,36 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
* disable it in 1.1.0.
*/
if (!SSL_IS_DTLS(s)) {
+ /*-
+ * check for enough space.
+ * 4 bytes for the ETM type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
+ return NULL;
s2n(TLSEXT_TYPE_encrypt_then_mac, ret);
s2n(0, ret);
}
#ifndef OPENSSL_NO_CT
if (s->ct_validation_callback != NULL) {
+ /*-
+ * check for enough space.
+ * 4 bytes for the SCT type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
+ return NULL;
+
s2n(TLSEXT_TYPE_signed_certificate_timestamp, ret);
s2n(0, ret);
}
#endif
+
+ /*-
+ * check for enough space.
+ * 4 bytes for the EMS type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
+ return NULL;
s2n(TLSEXT_TYPE_extended_master_secret, ret);
s2n(0, ret);
@@ -1394,6 +1465,17 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
else
hlen = 0;
+ /*-
+ * check for enough space. Strictly speaking we know we've already
+ * got enough space because to get here the message size is < 0x200,
+ * but we know that we've allocated far more than that in the buffer
+ * - but for consistency and robustness we're going to check anyway.
+ *
+ * 4 bytes for the padding type and extension length
+ * + padding length
+ */
+ if (CHECKLEN(ret, 4 + hlen, limit))
+ return NULL;
s2n(TLSEXT_TYPE_padding, ret);
s2n(hlen, ret);
memset(ret, 0, hlen);
@@ -1438,7 +1520,12 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
return NULL;
}
- if ((limit - ret - 4 - el) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the reneg type and extension length
+ * + reneg data length
+ */
+ if (CHECKLEN(ret, 4 + el, limit))
return NULL;
s2n(TLSEXT_TYPE_renegotiate, ret);
@@ -1458,7 +1545,11 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
if (!s->hit && s->servername_done == 1
&& s->session->tlsext_hostname != NULL) {
- if ((long)(limit - ret - 4) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the server name type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
return NULL;
s2n(TLSEXT_TYPE_server_name, ret);
@@ -1471,19 +1562,23 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
/*
* Add TLS extension ECPointFormats to the ServerHello message
*/
- long lenmax;
tls1_get_formatlist(s, &plist, &plistlen);
- if ((lenmax = limit - ret - 5) < 0)
- return NULL;
- if (plistlen > (size_t)lenmax)
- return NULL;
if (plistlen > 255) {
SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
}
+ /*-
+ * check for enough space.
+ * 4 bytes for the ec points format type and extension length
+ * 1 byte for the points format list length
+ * + length of points format list
+ */
+ if (CHECKLEN(ret, 5 + plistlen, limit))
+ return NULL;
+
s2n(TLSEXT_TYPE_ec_point_formats, ret);
s2n(plistlen + 1, ret);
*(ret++) = (unsigned char)plistlen;
@@ -1498,7 +1593,11 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
#endif /* OPENSSL_NO_EC */
if (s->tlsext_ticket_expected && tls_use_ticket(s)) {
- if ((long)(limit - ret - 4) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the Ticket type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
return NULL;
s2n(TLSEXT_TYPE_session_ticket, ret);
s2n(0, ret);
@@ -1511,7 +1610,11 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
}
if (s->tlsext_status_expected) {
- if ((long)(limit - ret - 4) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the Status request type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
return NULL;
s2n(TLSEXT_TYPE_status_request, ret);
s2n(0, ret);
@@ -1525,7 +1628,12 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
SSLerr(SSL_F_SSL_ADD_SERVERHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
return NULL;
}
- if ((limit - ret - 4 - el) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the SRTP profiles type and extension length
+ * + length of the SRTP profiles list
+ */
+ if (CHECKLEN(ret, 4 + el, limit))
return NULL;
s2n(TLSEXT_TYPE_use_srtp, ret);
@@ -1550,16 +1658,23 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
0x2a, 0x85, 0x03, 0x02, 0x02, 0x16, 0x30, 0x08,
0x06, 0x06, 0x2a, 0x85, 0x03, 0x02, 0x02, 0x17
};
- if (limit - ret < 36)
+
+ /* check for enough space. */
+ if (CHECKLEN(ret, sizeof(cryptopro_ext), limit))
return NULL;
- memcpy(ret, cryptopro_ext, 36);
- ret += 36;
+ memcpy(ret, cryptopro_ext, sizeof(cryptopro_ext));
+ ret += sizeof(cryptopro_ext);
}
#ifndef OPENSSL_NO_HEARTBEATS
/* Add Heartbeat extension if we've received one */
if (SSL_IS_DTLS(s) && (s->tlsext_heartbeat & SSL_DTLSEXT_HB_ENABLED)) {
- if ((limit - ret - 4 - 1) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the Heartbeat type and extension length
+ * 1 byte for the mode
+ */
+ if (CHECKLEN(ret, 5, limit))
return NULL;
s2n(TLSEXT_TYPE_heartbeat, ret);
s2n(1, ret);
@@ -1588,7 +1703,12 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
s->
ctx->next_protos_advertised_cb_arg);
if (r == SSL_TLSEXT_ERR_OK) {
- if ((long)(limit - ret - 4 - npalen) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the NPN type and extension length
+ * + length of protocols list
+ */
+ if (CHECKLEN(ret, 4 + npalen, limit))
return NULL;
s2n(TLSEXT_TYPE_next_proto_neg, ret);
s2n(npalen, ret);
@@ -1611,20 +1731,39 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf,
|| s->s3->tmp.new_cipher->algorithm_enc == SSL_eGOST2814789CNT12)
s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC;
else {
+ /*-
+ * check for enough space.
+ * 4 bytes for the ETM type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
+ return NULL;
s2n(TLSEXT_TYPE_encrypt_then_mac, ret);
s2n(0, ret);
}
}
if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) {
+ /*-
+ * check for enough space.
+ * 4 bytes for the EMS type and extension length
+ */
+ if (CHECKLEN(ret, 4, limit))
+ return NULL;
s2n(TLSEXT_TYPE_extended_master_secret, ret);
s2n(0, ret);
}
if (s->s3->alpn_selected != NULL) {
const unsigned char *selected = s->s3->alpn_selected;
- unsigned int len = s->s3->alpn_selected_len;
+ size_t len = s->s3->alpn_selected_len;
- if ((long)(limit - ret - 4 - 2 - 1 - len) < 0)
+ /*-
+ * check for enough space.
+ * 4 bytes for the ALPN type and extension length
+ * 2 bytes for ALPN data length
+ * 1 byte for selected protocol length
+ * + length of the selected protocol
+ */
+ if (CHECKLEN(ret, 7 + len, limit))
return NULL;
s2n(TLSEXT_TYPE_application_layer_protocol_negotiation, ret);
s2n(3 + len, ret);
More information about the openssl-commits
mailing list