Re: wheezy: update for polarssl's CVE-2015-5291
Hi,
On Mon, Feb 01, 2016 at 09:51:54AM +0100, Sébastien Delafond wrote:
> On Jan/31, Guido Günther wrote:
> > Uploaded now. Thanks!
>
> Hi Guido,
>
> have you looked into fixing the jessie version (1.3.9-2.1) as well ? If
> not, I'll need to look into it later this week, so that a DSA for
> CVE-2015-5291 fixes both wheezy and jessie.
Debdiff attached. It's far more intrusive since we also have to deal
with CVE-2015-8036.
James you alread discussed the best way forward at
https://tls.mbed.org/discussions/bug-report-issues/question-about-cve-2015-5291
with upstream so I'm very interesed in your opinion on this as well.
Cheers,
-- Guido
diff --git a/debian/changelog b/debian/changelog
index a0b698f..2952b8f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+polarssl (1.3.9-2.2) jessie-security; urgency=high
+
+ * Non-maintainer upload by the LTS Team.
+ * Backport pathes for CVE-2015-5291 and CVE-2015-8036
+ (Closes: #801413)
+ * Add simple smoke test
+
+ -- Guido Günther <agx@sigxcpu.org> Fri, 05 Feb 2016 13:41:23 +0100
+
polarssl (1.3.9-2.1) unstable; urgency=high
* Non-maintainer upload.
diff --git a/debian/patches/CVE-2015-5291-Added-max-length-checking-of-hostname.patch b/debian/patches/CVE-2015-5291-Added-max-length-checking-of-hostname.patch
new file mode 100644
index 0000000..dd17324
--- /dev/null
+++ b/debian/patches/CVE-2015-5291-Added-max-length-checking-of-hostname.patch
@@ -0,0 +1,37 @@
+From: Simon Butcher <simon.butcher@arm.com>
+Date: Tue, 29 Sep 2015 23:27:20 +0100
+Subject: CVE-2015-5291: Added max length checking of hostname
+
+(cherry picked from commit c988f32adde62a169ba340fee0da15aecd40e76e)
+---
+ include/polarssl/ssl.h | 2 ++
+ library/ssl_tls.c | 3 +++
+ 2 files changed, 5 insertions(+)
+
+diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
+index 194e944..2bd66ca 100644
+--- a/include/polarssl/ssl.h
++++ b/include/polarssl/ssl.h
+@@ -194,6 +194,8 @@
+ #endif /* POLARSSL_SSL_PROTO_TLS1_1 */
+ #endif /* POLARSSL_SSL_PROTO_TLS1_2 */
+
++#define SSL_MAX_HOST_NAME_LEN 255 /*!< Maximum host name defined in RFC 1035 */
++
+ /* RFC 6066 section 4, see also mfl_code_to_length in ssl_tls.c
+ * NONE must be zero so that memset()ing structure to zero works */
+ #define SSL_MAX_FRAG_LEN_NONE 0 /*!< don't use this extension */
+diff --git a/library/ssl_tls.c b/library/ssl_tls.c
+index 5f080de..196f0c9 100644
+--- a/library/ssl_tls.c
++++ b/library/ssl_tls.c
+@@ -3903,6 +3903,9 @@ int ssl_set_hostname( ssl_context *ssl, const char *hostname )
+ if( ssl->hostname_len + 1 == 0 )
+ return( POLARSSL_ERR_SSL_BAD_INPUT_DATA );
+
++ if( ssl->hostname_len > SSL_MAX_HOST_NAME_LEN )
++ return( POLARSSL_ERR_SSL_BAD_INPUT_DATA );
++
+ ssl->hostname = (unsigned char *) polarssl_malloc( ssl->hostname_len + 1 );
+
+ if( ssl->hostname == NULL )
diff --git a/debian/patches/CVE-2015-8036-Add-extra-check-before-integer-conversion.patch b/debian/patches/CVE-2015-8036-Add-extra-check-before-integer-conversion.patch
new file mode 100644
index 0000000..2b11b31
--- /dev/null
+++ b/debian/patches/CVE-2015-8036-Add-extra-check-before-integer-conversion.patch
@@ -0,0 +1,24 @@
+From: =?utf-8?q?Manuel_P=C3=A9gouri=C3=A9-Gonnard?= <mpg2@elzevir.fr>
+Date: Fri, 2 Oct 2015 09:53:52 +0200
+Subject: CVE-2015-8036: Add extra check before integer conversion
+
+end < p should never happen, but just be extra sure
+
+(cherry picked from commit f3e6e4badb35760c9a543ee69b7449cb0cd9784b)
+---
+ library/ssl_cli.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/library/ssl_cli.c b/library/ssl_cli.c
+index 7696311..9b4b8a9 100644
+--- a/library/ssl_cli.c
++++ b/library/ssl_cli.c
+@@ -135,7 +135,7 @@ static void ssl_write_renegotiation_ext( ssl_context *ssl,
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) );
+
+- if( (size_t)(end - p) < 5 + ssl->verify_data_len )
++ if( end < p || (size_t)(end - p) < 5 + ssl->verify_data_len )
+ {
+ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
diff --git a/debian/patches/CVE-2015-8036-Added-bounds-checking-for-TLS-extensions.patch b/debian/patches/CVE-2015-8036-Added-bounds-checking-for-TLS-extensions.patch
new file mode 100644
index 0000000..e674cd9
--- /dev/null
+++ b/debian/patches/CVE-2015-8036-Added-bounds-checking-for-TLS-extensions.patch
@@ -0,0 +1,280 @@
+From: Simon Butcher <simon.butcher@arm.com>
+Date: Thu, 1 Oct 2015 00:24:36 +0100
+Subject: CVE-2015-8036: Added bounds checking for TLS extensions
+
+IOTSSL-478 - Added checks to prevent buffer overflows.
+
+(cherry picked from commit b1e325d6b2bd9c504536fbbd45dce348f0a6c40c)
+---
+ library/ssl_cli.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++-----
+ 1 file changed, 128 insertions(+), 13 deletions(-)
+
+diff --git a/library/ssl_cli.c b/library/ssl_cli.c
+index 27abb3e..71e1beb 100644
+--- a/library/ssl_cli.c
++++ b/library/ssl_cli.c
+@@ -68,6 +68,7 @@ static void ssl_write_hostname_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
+
+@@ -77,6 +78,12 @@ static void ssl_write_hostname_ext( ssl_context *ssl,
+ SSL_DEBUG_MSG( 3, ( "client hello, adding server name extension: %s",
+ ssl->hostname ) );
+
++ if( (size_t)(end - p) < ssl->hostname_len + 9 )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ /*
+ * struct {
+ * NameType name_type;
+@@ -119,6 +126,7 @@ static void ssl_write_renegotiation_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
+
+@@ -127,6 +135,12 @@ static void ssl_write_renegotiation_ext( ssl_context *ssl,
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) );
+
++ if( (size_t)(end - p) < 5 + ssl->verify_data_len )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ /*
+ * Secure renegotiation
+ */
+@@ -148,6 +162,7 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+ size_t sig_alg_len = 0;
+ #if defined(POLARSSL_RSA_C) || defined(POLARSSL_ECDSA_C)
+ unsigned char *sig_alg_list = buf + 6;
+@@ -160,9 +175,54 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl,
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) );
+
++#if defined(POLARSSL_RSA_C)
++#if defined(POLARSSL_SHA512_C)
++ /* SHA512 + RSA signature, SHA384 + RSA signature */
++ sig_alg_len += 4;
++#endif
++#if defined(POLARSSL_SHA256_C)
++ /* SHA256 + RSA signature, SHA224 + RSA signature */
++ sig_alg_len += 4;
++#endif
++#if defined(POLARSSL_SHA1_C)
++ /* SHA1 + RSA signature */
++ sig_alg_len += 2;
++#endif
++#if defined(POLARSSL_MD5_C)
++ /* MD5 + RSA signature */
++ sig_alg_len += 2;
++#endif
++#endif /* POLARSSL_RSA_C */
++#if defined(POLARSSL_ECDSA_C)
++#if defined(POLARSSL_SHA512_C)
++ /* SHA512 + ECDSA signature, SHA384 + ECDSA signature */
++ sig_alg_len += 4;
++#endif
++#if defined(POLARSSL_SHA256_C)
++ /* SHA256 + ECDSA signature, SHA224 + ECDSA signature */
++ sig_alg_len += 4;
++#endif
++#if defined(POLARSSL_SHA1_C)
++ /* SHA1 + ECDSA signature */
++ sig_alg_len += 2;
++#endif
++#if defined(POLARSSL_MD5_C)
++ /* MD5 + ECDSA signature */
++ sig_alg_len += 2;
++#endif
++#endif /* POLARSSL_ECDSA_C */
++
++ if( end < p || (size_t)( end - p ) < sig_alg_len + 6 )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ /*
+ * Prepare signature_algorithms extension (TLS 1.2)
+ */
++ sig_alg_len = 0;
++
+ #if defined(POLARSSL_RSA_C)
+ #if defined(POLARSSL_SHA512_C)
+ sig_alg_list[sig_alg_len++] = SSL_HASH_SHA512;
+@@ -244,6 +304,7 @@ static void ssl_write_supported_elliptic_curves_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+ unsigned char *elliptic_curve_list = p + 6;
+ size_t elliptic_curve_len = 0;
+ const ecp_curve_info *info;
+@@ -265,6 +326,25 @@ static void ssl_write_supported_elliptic_curves_ext( ssl_context *ssl,
+ for( info = ecp_curve_list(); info->grp_id != POLARSSL_ECP_DP_NONE; info++ )
+ {
+ #endif
++ elliptic_curve_len += 2;
++ }
++
++ if( end < p || (size_t)( end - p ) < 6 + elliptic_curve_len )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
++ elliptic_curve_len = 0;
++
++#if defined(POLARSSL_SSL_SET_CURVES)
++ for( grp_id = ssl->curve_list; *grp_id != POLARSSL_ECP_DP_NONE; grp_id++ )
++ {
++ info = ecp_curve_info_from_grp_id( *grp_id );
++#else
++ for( info = ecp_curve_list(); info->grp_id != POLARSSL_ECP_DP_NONE; info++ )
++ {
++#endif
+
+ elliptic_curve_list[elliptic_curve_len++] = info->tls_id >> 8;
+ elliptic_curve_list[elliptic_curve_len++] = info->tls_id & 0xFF;
+@@ -290,12 +370,18 @@ static void ssl_write_supported_point_formats_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
+- ((void) ssl);
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+
+ *olen = 0;
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) );
+
++ if( end < p || (size_t)( end - p ) < 6 )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ *p++ = (unsigned char)( ( TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF );
+ *p++ = (unsigned char)( ( TLS_EXT_SUPPORTED_POINT_FORMATS ) & 0xFF );
+
+@@ -315,14 +401,21 @@ static void ssl_write_max_fragment_length_ext( ssl_context *ssl,
+ size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
++
++ *olen = 0;
+
+- if( ssl->mfl_code == SSL_MAX_FRAG_LEN_NONE ) {
+- *olen = 0;
++ if( ssl->mfl_code == SSL_MAX_FRAG_LEN_NONE )
+ return;
+- }
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) );
+
++ if( end < p || (size_t)( end - p ) < 5 )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ *p++ = (unsigned char)( ( TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) & 0xFF );
+ *p++ = (unsigned char)( ( TLS_EXT_MAX_FRAGMENT_LENGTH ) & 0xFF );
+
+@@ -340,15 +433,21 @@ static void ssl_write_truncated_hmac_ext( ssl_context *ssl,
+ unsigned char *buf, size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
++
++ *olen = 0;
+
+ if( ssl->trunc_hmac == SSL_TRUNC_HMAC_DISABLED )
+- {
+- *olen = 0;
+ return;
+- }
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) );
+
++ if( end < p || (size_t)( end - p ) < 4 )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ *p++ = (unsigned char)( ( TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF );
+ *p++ = (unsigned char)( ( TLS_EXT_TRUNCATED_HMAC ) & 0xFF );
+
+@@ -364,16 +463,22 @@ static void ssl_write_session_ticket_ext( ssl_context *ssl,
+ unsigned char *buf, size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
+ size_t tlen = ssl->session_negotiate->ticket_len;
+
++ *olen = 0;
++
+ if( ssl->session_tickets == SSL_SESSION_TICKETS_DISABLED )
+- {
+- *olen = 0;
+ return;
+- }
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) );
+
++ if( end < p || (size_t)( end - p ) < 4 + tlen )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ *p++ = (unsigned char)( ( TLS_EXT_SESSION_TICKET >> 8 ) & 0xFF );
+ *p++ = (unsigned char)( ( TLS_EXT_SESSION_TICKET ) & 0xFF );
+
+@@ -401,16 +506,26 @@ static void ssl_write_alpn_ext( ssl_context *ssl,
+ unsigned char *buf, size_t *olen )
+ {
+ unsigned char *p = buf;
++ const unsigned char *end = ssl->out_msg + SSL_MAX_CONTENT_LEN;
++ size_t alpnlen = 0;
+ const char **cur;
+
++ *olen = 0;
++
+ if( ssl->alpn_list == NULL )
+- {
+- *olen = 0;
+ return;
+- }
+
+ SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) );
+
++ for( cur = ssl->alpn_list; *cur != NULL; cur++ )
++ alpnlen += (unsigned char)( strlen( *cur ) & 0xFF ) + 1;
++
++ if( end < p || (size_t)( end - p ) < 6 + alpnlen )
++ {
++ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
++ return;
++ }
++
+ *p++ = (unsigned char)( ( TLS_EXT_ALPN >> 8 ) & 0xFF );
+ *p++ = (unsigned char)( ( TLS_EXT_ALPN ) & 0xFF );
+
diff --git a/debian/patches/CVE-2015-8036-Reordered-extension-fields-and-added-to-Cha.patch b/debian/patches/CVE-2015-8036-Reordered-extension-fields-and-added-to-Cha.patch
new file mode 100644
index 0000000..ce5d5ce
--- /dev/null
+++ b/debian/patches/CVE-2015-8036-Reordered-extension-fields-and-added-to-Cha.patch
@@ -0,0 +1,43 @@
+From: Simon Butcher <simon.butcher@arm.com>
+Date: Thu, 1 Oct 2015 01:17:10 +0100
+Subject: CVE-2015-8036: Reordered extension fields and added to ChangeLog
+
+Reordered the transmission sequence of TLS extension fields in client hello
+and added to ChangeLog.
+
+(cherry picked from commit 643a922c56b77235e88f106fb1b41c1a764cea5f)
+---
+ library/ssl_cli.c | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/library/ssl_cli.c b/library/ssl_cli.c
+index 71e1beb..7696311 100644
+--- a/library/ssl_cli.c
++++ b/library/ssl_cli.c
+@@ -78,7 +78,7 @@ static void ssl_write_hostname_ext( ssl_context *ssl,
+ SSL_DEBUG_MSG( 3, ( "client hello, adding server name extension: %s",
+ ssl->hostname ) );
+
+- if( (size_t)(end - p) < ssl->hostname_len + 9 )
++ if( end < p || (size_t)( end - p ) < ssl->hostname_len + 9 )
+ {
+ SSL_DEBUG_MSG( 1, ( "buffer too small" ) );
+ return;
+@@ -766,13 +766,13 @@ static int ssl_write_client_hello( ssl_context *ssl )
+ ext_len += olen;
+ #endif
+
+-#if defined(POLARSSL_SSL_SESSION_TICKETS)
+- ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, &olen );
++#if defined(POLARSSL_SSL_ALPN)
++ ssl_write_alpn_ext( ssl, p + 2 + ext_len, &olen );
+ ext_len += olen;
+ #endif
+
+-#if defined(POLARSSL_SSL_ALPN)
+- ssl_write_alpn_ext( ssl, p + 2 + ext_len, &olen );
++#if defined(POLARSSL_SSL_SESSION_TICKETS)
++ ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, &olen );
+ ext_len += olen;
+ #endif
+
diff --git a/debian/patches/series b/debian/patches/series
index b0838f5..a17b089 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,7 @@
01-config.patch
02-makefile-destdir-fix.patch
CVE-2015-1182.patch
+CVE-2015-5291-Added-max-length-checking-of-hostname.patch
+CVE-2015-8036-Added-bounds-checking-for-TLS-extensions.patch
+CVE-2015-8036-Reordered-extension-fields-and-added-to-Cha.patch
+CVE-2015-8036-Add-extra-check-before-integer-conversion.patch
diff --git a/debian/tests/build-test b/debian/tests/build-test
new file mode 100755
index 0000000..42b7127
--- /dev/null
+++ b/debian/tests/build-test
@@ -0,0 +1,10 @@
+#!/usr/bin/make -f
+
+CFLAGS = -O2 -D_FILE_OFFSET_BITS=64 -Wall
+LDFLAGS += -lpolarssl
+
+a.out: programs/hash/hello.c
+ $(CC) $(CFLAGS) $(OFLAGS) $< $(LDFLAGS)
+ @echo "Build test of $< succeeded"
+ ./a.out
+ @rm -f a.out
diff --git a/debian/tests/control b/debian/tests/control
new file mode 100644
index 0000000..1a640bc
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,10 @@
+Tests: smoke
+Depends: libpolarssl-runtime
+
+Tests: ssl-server-test
+Depends: libpolarssl-runtime
+Restrictions: allow-stderr
+
+Tests: build-test
+Depends: libpolarssl-dev
+
diff --git a/debian/tests/smoke b/debian/tests/smoke
new file mode 100755
index 0000000..03df087
--- /dev/null
+++ b/debian/tests/smoke
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+set -e
+
+# Excercise some of the demos
+polarssl_hello
+polarssl_mpi_demo
+
+# Make sure output is identical to coreutil versions
+[ "$(polarssl_sha1sum /etc/passwd)" = "$(sha1sum /etc/passwd)" ]
+[ "$(polarssl_md5sum /etc/passwd)" = "$(md5sum /etc/passwd)" ]
+
+# Run the selftest
+polarssl_selftest
+
+echo 'Smoke test of polarssl succesful'
+exit 0
diff --git a/debian/tests/ssl-server-test b/debian/tests/ssl-server-test
new file mode 100755
index 0000000..3078dc6
--- /dev/null
+++ b/debian/tests/ssl-server-test
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+set -e
+set -x
+
+cleanup() {
+ [ -z "$PID" ] || kill -TERM $PID
+}
+
+trap cleanup ERR
+
+polarssl_ssl_server2 &
+PID=$!
+
+# Wait for server to spin up
+sleep 2
+
+polarssl_ssl_client2
+
+cleanup
+exit 0
+
Reply to: