[openssl-commits] [openssl] OpenSSL source code branch master updated. 740580c2b2b86c2ffdc4a2d36850248c6091d6a0
Emilia Kasper
emilia at openssl.org
Fri Dec 5 12:30:48 EST 2014
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "OpenSSL source code".
The branch, master has been updated
via 740580c2b2b86c2ffdc4a2d36850248c6091d6a0 (commit)
from 33d5ba862939ff8db70a9e36fc9a326fab3e8d98 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 740580c2b2b86c2ffdc4a2d36850248c6091d6a0
Author: Emilia Kasper <emilia at openssl.org>
Date: Mon Dec 1 16:55:55 2014 +0100
Add extra checks for odd-length EC curve lists.
Odd-length lists should be rejected everywhere upon parsing. Nevertheless,
be extra careful and add guards against off-by-one reads.
Also, drive-by replace inexplicable double-negation with an explicit comparison.
Reviewed-by: Matt Caswell <matt at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/ssl.h | 1 +
ssl/ssl_err.c | 2 +
ssl/t1_lib.c | 182 ++++++++++++++++++++++++++++++++++-----------------------
3 files changed, 111 insertions(+), 74 deletions(-)
diff --git a/ssl/ssl.h b/ssl/ssl.h
index 388d400..61c9890 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2727,6 +2727,7 @@ void ERR_load_SSL_strings(void);
#define SSL_F_TLS1_CHECK_SERVERHELLO_TLSEXT 274
#define SSL_F_TLS1_ENC 210
#define SSL_F_TLS1_EXPORT_KEYING_MATERIAL 314
+#define SSL_F_TLS1_GET_CURVELIST 338
#define SSL_F_TLS1_HEARTBEAT 315
#define SSL_F_TLS1_PREPARE_CLIENTHELLO_TLSEXT 275
#define SSL_F_TLS1_PREPARE_SERVERHELLO_TLSEXT 276
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 220b6d7..00c4bc8 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -80,6 +80,7 @@ static ERR_STRING_DATA SSL_str_functs[]=
{ERR_FUNC(SSL_F_DTLS1_CHECK_TIMEOUT_NUM), "dtls1_check_timeout_num"},
{ERR_FUNC(SSL_F_DTLS1_CLIENT_HELLO), "dtls1_client_hello"},
{ERR_FUNC(SSL_F_DTLS1_CONNECT), "dtls1_connect"},
+{ERR_FUNC(SSL_F_DTLS1_ENC), "DTLS1_ENC"},
{ERR_FUNC(SSL_F_DTLS1_GET_HELLO_VERIFY), "DTLS1_GET_HELLO_VERIFY"},
{ERR_FUNC(SSL_F_DTLS1_GET_MESSAGE), "dtls1_get_message"},
{ERR_FUNC(SSL_F_DTLS1_GET_MESSAGE_FRAGMENT), "DTLS1_GET_MESSAGE_FRAGMENT"},
@@ -267,6 +268,7 @@ static ERR_STRING_DATA SSL_str_functs[]=
{ERR_FUNC(SSL_F_TLS1_CHECK_SERVERHELLO_TLSEXT), "TLS1_CHECK_SERVERHELLO_TLSEXT"},
{ERR_FUNC(SSL_F_TLS1_ENC), "tls1_enc"},
{ERR_FUNC(SSL_F_TLS1_EXPORT_KEYING_MATERIAL), "tls1_export_keying_material"},
+{ERR_FUNC(SSL_F_TLS1_GET_CURVELIST), "TLS1_GET_CURVELIST"},
{ERR_FUNC(SSL_F_TLS1_HEARTBEAT), "tls1_heartbeat"},
{ERR_FUNC(SSL_F_TLS1_PREPARE_CLIENTHELLO_TLSEXT), "TLS1_PREPARE_CLIENTHELLO_TLSEXT"},
{ERR_FUNC(SSL_F_TLS1_PREPARE_SERVERHELLO_TLSEXT), "TLS1_PREPARE_SERVERHELLO_TLSEXT"},
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index c5c8bb9..debad3b 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -384,44 +384,69 @@ int tls1_ec_nid2curve_id(int nid)
return 0;
}
}
-/* Get curves list, if "sess" is set return client curves otherwise
- * preferred list
+/*
+ * Get curves list, if "sess" is set return client curves otherwise
+ * preferred list.
+ * Sets |num_curves| to the number of curves in the list, i.e.,
+ * the length of |pcurves| is 2 * num_curves.
+ * Returns 1 on success and 0 if the client curves list has invalid format.
+ * The latter indicates an internal error: we should not be accepting such
+ * lists in the first place.
+ * TODO(emilia): we should really be storing the curves list in explicitly
+ * parsed form instead. (However, this would affect binary compatibility
+ * so cannot happen in the 1.0.x series.)
*/
-static void tls1_get_curvelist(SSL *s, int sess,
+static int tls1_get_curvelist(SSL *s, int sess,
const unsigned char **pcurves,
- size_t *pcurveslen)
+ size_t *num_curves)
{
+ size_t pcurveslen = 0;
if (sess)
{
*pcurves = s->session->tlsext_ellipticcurvelist;
- *pcurveslen = s->session->tlsext_ellipticcurvelist_length;
- return;
+ pcurveslen = s->session->tlsext_ellipticcurvelist_length;
}
- /* For Suite B mode only include P-256, P-384 */
- switch (tls1_suiteb(s))
+ else
{
- case SSL_CERT_FLAG_SUITEB_128_LOS:
- *pcurves = suiteb_curves;
- *pcurveslen = sizeof(suiteb_curves);
- break;
+ /* For Suite B mode only include P-256, P-384 */
+ switch (tls1_suiteb(s))
+ {
+ case SSL_CERT_FLAG_SUITEB_128_LOS:
+ *pcurves = suiteb_curves;
+ pcurveslen = sizeof(suiteb_curves);
+ break;
- case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY:
- *pcurves = suiteb_curves;
- *pcurveslen = 2;
- break;
+ case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY:
+ *pcurves = suiteb_curves;
+ pcurveslen = 2;
+ break;
- case SSL_CERT_FLAG_SUITEB_192_LOS:
- *pcurves = suiteb_curves + 2;
- *pcurveslen = 2;
- break;
- default:
- *pcurves = s->tlsext_ellipticcurvelist;
- *pcurveslen = s->tlsext_ellipticcurvelist_length;
+ case SSL_CERT_FLAG_SUITEB_192_LOS:
+ *pcurves = suiteb_curves + 2;
+ pcurveslen = 2;
+ break;
+ default:
+ *pcurves = s->tlsext_ellipticcurvelist;
+ pcurveslen = s->tlsext_ellipticcurvelist_length;
+ }
+ if (!*pcurves)
+ {
+ *pcurves = eccurves_default;
+ pcurveslen = sizeof(eccurves_default);
+ }
+ }
+
+ /* We do not allow odd length arrays to enter the system. */
+ if (pcurveslen & 1)
+ {
+ SSLerr(SSL_F_TLS1_GET_CURVELIST, ERR_R_INTERNAL_ERROR);
+ *num_curves = 0;
+ return 0;
}
- if (!*pcurves)
+ else
{
- *pcurves = eccurves_default;
- *pcurveslen = sizeof(eccurves_default);
+ *num_curves = pcurveslen / 2;
+ return 1;
}
}
@@ -446,7 +471,7 @@ static int tls_curve_allowed(SSL *s, const unsigned char *curve, int op)
int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
{
const unsigned char *curves;
- size_t curveslen, i;
+ size_t num_curves, i;
unsigned int suiteb_flags = tls1_suiteb(s);
if (len != 3 || p[0] != NAMED_CURVE_TYPE)
return 0;
@@ -469,8 +494,9 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
else /* Should never happen */
return 0;
}
- tls1_get_curvelist(s, 0, &curves, &curveslen);
- for (i = 0; i < curveslen; i += 2, curves += 2)
+ if (!tls1_get_curvelist(s, 0, &curves, &num_curves))
+ return 0;
+ for (i = 0; i < num_curves; i++, curves += 2)
{
if (p[1] == curves[0] && p[2] == curves[1])
return tls_curve_allowed(s, p + 1, SSL_SECOP_CURVE_CHECK);
@@ -486,7 +512,7 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
int tls1_shared_curve(SSL *s, int nmatch)
{
const unsigned char *pref, *supp;
- size_t preflen, supplen, i, j;
+ size_t num_pref, num_supp, i, j;
int k;
/* Can't do anything on client side */
if (s->server == 0)
@@ -510,17 +536,21 @@ int tls1_shared_curve(SSL *s, int nmatch)
/* If not Suite B just return first preference shared curve */
nmatch = 0;
}
- tls1_get_curvelist(s, !!(s->options & SSL_OP_CIPHER_SERVER_PREFERENCE),
- &supp, &supplen);
- tls1_get_curvelist(s, !(s->options & SSL_OP_CIPHER_SERVER_PREFERENCE),
- &pref, &preflen);
- preflen /= 2;
- supplen /= 2;
+ /*
+ * Avoid truncation. tls1_get_curvelist takes an int
+ * but s->options is a long...
+ */
+ if (!tls1_get_curvelist(s, (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) != 0,
+ &supp, &num_supp))
+ return 0;
+ if(!tls1_get_curvelist(s, !(s->options & SSL_OP_CIPHER_SERVER_PREFERENCE),
+ &pref, &num_pref))
+ return 0;
k = 0;
- for (i = 0; i < preflen; i++, pref+=2)
+ for (i = 0; i < num_pref; i++, pref+=2)
{
const unsigned char *tsupp = supp;
- for (j = 0; j < supplen; j++, tsupp+=2)
+ for (j = 0; j < num_supp; j++, tsupp+=2)
{
if (pref[0] == tsupp[0] && pref[1] == tsupp[1])
{
@@ -675,22 +705,22 @@ static int tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id,
static int tls1_check_ec_key(SSL *s,
unsigned char *curve_id, unsigned char *comp_id)
{
- const unsigned char *p;
- size_t plen, i;
+ const unsigned char *pformats, *pcurves;
+ size_t num_formats, num_curves, i;
int j;
/* If point formats extension present check it, otherwise everything
* is supported (see RFC4492).
*/
if (comp_id && s->session->tlsext_ecpointformatlist)
{
- p = s->session->tlsext_ecpointformatlist;
- plen = s->session->tlsext_ecpointformatlist_length;
- for (i = 0; i < plen; i++, p++)
+ pformats = s->session->tlsext_ecpointformatlist;
+ num_formats = s->session->tlsext_ecpointformatlist_length;
+ for (i = 0; i < num_formats; i++, pformats++)
{
- if (*comp_id == *p)
+ if (*comp_id == *pformats)
break;
}
- if (i == plen)
+ if (i == num_formats)
return 0;
}
if (!curve_id)
@@ -698,13 +728,15 @@ static int tls1_check_ec_key(SSL *s,
/* Check curve is consistent with client and server preferences */
for (j = 0; j <= 1; j++)
{
- tls1_get_curvelist(s, j, &p, &plen);
- for (i = 0; i < plen; i+=2, p+=2)
+ if (!tls1_get_curvelist(s, j, &pcurves, &num_curves))
+ return 0;
+ for (i = 0; i < num_curves; i++, pcurves += 2)
{
- if (p[0] == curve_id[0] && p[1] == curve_id[1])
+ if (pcurves[0] == curve_id[0] &&
+ pcurves[1] == curve_id[1])
break;
}
- if (i == plen)
+ if (i == num_curves)
return 0;
/* For clients can only check sent curve list */
if (!s->server)
@@ -714,23 +746,23 @@ static int tls1_check_ec_key(SSL *s,
}
static void tls1_get_formatlist(SSL *s, const unsigned char **pformats,
- size_t *pformatslen)
+ size_t *num_formats)
{
/* If we have a custom point format list use it otherwise
* use default */
if (s->tlsext_ecpointformatlist)
{
*pformats = s->tlsext_ecpointformatlist;
- *pformatslen = s->tlsext_ecpointformatlist_length;
+ *num_formats = s->tlsext_ecpointformatlist_length;
}
else
{
*pformats = ecformats_default;
/* For Suite B we don't support char2 fields */
if (tls1_suiteb(s))
- *pformatslen = sizeof(ecformats_default) - 1;
+ *num_formats = sizeof(ecformats_default) - 1;
else
- *pformatslen = sizeof(ecformats_default);
+ *num_formats = sizeof(ecformats_default);
}
}
@@ -1244,34 +1276,36 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c
{
/* Add TLS extension ECPointFormats to the ClientHello message */
long lenmax;
- const unsigned char *plist;
- size_t plistlen;
+ const unsigned char *pcurves, *pformats;
+ size_t num_curves, num_formats, curves_list_len;
size_t i;
unsigned char *etmp;
- tls1_get_formatlist(s, &plist, &plistlen);
+ tls1_get_formatlist(s, &pformats, &num_formats);
if ((lenmax = limit - ret - 5) < 0) return NULL;
- if (plistlen > (size_t)lenmax) return NULL;
- if (plistlen > 255)
+ 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;
}
s2n(TLSEXT_TYPE_ec_point_formats,ret);
- s2n(plistlen + 1,ret);
- *(ret++) = (unsigned char)plistlen ;
- memcpy(ret, plist, plistlen);
- ret+=plistlen;
+ /* The point format list has 1-byte length. */
+ s2n(num_formats + 1,ret);
+ *(ret++) = (unsigned char)num_formats ;
+ memcpy(ret, pformats, num_formats);
+ ret+=num_formats;
/* Add TLS extension EllipticCurves to the ClientHello message */
- plist = s->tlsext_ellipticcurvelist;
- tls1_get_curvelist(s, 0, &plist, &plistlen);
+ pcurves = s->tlsext_ellipticcurvelist;
+ if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves))
+ return NULL;
if ((lenmax = limit - ret - 6) < 0) return NULL;
- if (plistlen > (size_t)lenmax) return NULL;
- if (plistlen > 65532)
+ 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;
@@ -1281,20 +1315,20 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c
s2n(TLSEXT_TYPE_elliptic_curves,ret);
etmp = ret + 4;
/* Copy curve ID if supported */
- for (i = 0; i < plistlen; i += 2, plist += 2)
+ for (i = 0; i < num_curves; i++, pcurves += 2)
{
- if (tls_curve_allowed(s, plist, SSL_SECOP_CURVE_SUPPORTED))
+ if (tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED))
{
- *etmp++ = plist[0];
- *etmp++ = plist[1];
+ *etmp++ = pcurves[0];
+ *etmp++ = pcurves[1];
}
}
- plistlen = etmp - ret - 4;
+ curves_list_len = etmp - ret - 4;
- s2n(plistlen + 2, ret);
- s2n(plistlen, ret);
- ret+=plistlen;
+ s2n(curves_list_len + 2, ret);
+ s2n(curves_list_len, ret);
+ ret += curves_list_len;
}
#endif /* OPENSSL_NO_EC */
hooks/post-receive
--
OpenSSL source code
More information about the openssl-commits
mailing list