From 14400c8fb04e406a042695f28762b40cc7b2aa7b Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Sat, 2 Jan 2016 00:08:13 +0000 Subject: [PATCH] Merge memory leak fix into branch 'mbedtls-1.3' Merge of fix for memory leak in RSA-SSA signing - #372 --- ChangeLog | 10 ++++++++++ include/polarssl/config.h | 13 +++++++++++++ library/asn1write.c | 23 +++++++++++------------ library/bignum.c | 28 +++++++++++++++------------- library/ssl_cli.c | 16 ++++++++++++---- library/ssl_srv.c | 6 ++++++ 6 files changed, 67 insertions(+), 29 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2a8189436..20af9bc42 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,12 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 1.3.16 released 2015-12-xx +Security + * Fix potential double free when mbedtls_asn1_store_named_data() fails to + allocate memory. Only used for certificate generation, not triggerable + remotely in SSL/TLS. Found by Rafał Przywara. #367 + * Disable MD5 handshake signatures in TLS 1.2 by default + Bugfix * Fix over-restricive length limit in GCM. Found by Andreas-N. #362 * Fix bug in certificate validation that caused valid chains to be rejected @@ -10,6 +16,10 @@ Bugfix * Removed potential leak in mbedtls_rsa_rsassa_pkcs1_v15_sign(), found by JayaraghavendranK. #372 +Changes + * Add config.h option POLARSSL_SSL_ENABLE_MD5_SIGNATURES controlling + use of MD5-based signatures for TLS 1.2 handshake (disabled by default). + = mbed TLS 1.3.15 released 2015-11-04 Security diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 21aa9fa6a..4929aa1a9 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -1092,6 +1092,19 @@ */ #define POLARSSL_SSL_TRUNCATED_HMAC +/** + * \def POLARSSL_SSL_ENABLE_MD5_SIGNATURES + * + * Offer, accept and do MD5-based signatures in the TLS 1.2 handshake. + * Has no effect on which algorithms are accepted for certificates. + * Has no effect on other SSL/TLS versions. + * + * \warning Enabling this could be a security risk! + * + * Uncomment to enable MD5 signatures in TLS 1.2 + */ +//#define POLARSSL_SSL_ENABLE_MD5_SIGNATURES + /** * \def POLARSSL_SSL_SET_CURVES * diff --git a/library/asn1write.c b/library/asn1write.c index 6a7c9d35e..bb60830e5 100644 --- a/library/asn1write.c +++ b/library/asn1write.c @@ -342,19 +342,18 @@ asn1_named_data *asn1_store_named_data( asn1_named_data **head, } else if( cur->val.len < val_len ) { - // Enlarge existing value buffer if needed - // - polarssl_free( cur->val.p ); - cur->val.p = NULL; - - cur->val.len = val_len; - cur->val.p = polarssl_malloc( val_len ); - if( cur->val.p == NULL ) - { - polarssl_free( cur->oid.p ); - polarssl_free( cur ); + /* + * Enlarge existing value buffer if needed + * Preserve old data until the allocation succeeded, to leave list in + * a consistent state in case allocation fails. + */ + void *p = polarssl_malloc( val_len ); + if( p == NULL ) return( NULL ); - } + + polarssl_free( cur->val.p ); + cur->val.p = p; + cur->val.len = val_len; } if( val != NULL ) diff --git a/library/bignum.c b/library/bignum.c index 584ab85a3..b606238c3 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -34,7 +34,7 @@ * [3] GNU Multi-Precision Arithmetic Library * https://gmplib.org/manual/index.html * -*/ + */ #if !defined(POLARSSL_CONFIG_FILE) #include "polarssl/config.h" @@ -1218,22 +1218,28 @@ int mpi_mul_int( mpi *X, const mpi *A, t_sint b ) } /* - * Unsigned integer divide - 64bit dividend and 32bit divisor + * Unsigned integer divide - double t_uint, dividend, u1/u0, and t_uint + * divisor, d */ -static t_uint int_div_int(t_uint u1, t_uint u0, t_uint d, t_uint *r) +static t_uint int_div_int( t_uint u1, t_uint u0, t_uint d, t_uint *r ) { #if defined(POLARSSL_HAVE_UDBL) t_udbl dividend, quotient; +#else + const t_uint radix = 1 << biH; + t_uint d0, d1, q0, q1, rAX, r0, quotient; + t_uint u0_msw, u0_lsw; + int s; #endif /* * Check for overflow */ - if(( 0 == d ) || ( u1 >= d )) + if( 0 == d || u1 >= d ) { - if (r != NULL) *r = (~0); + if ( r != NULL ) *r = ~0; - return (~0); + return ( ~0 ); } #if defined(POLARSSL_HAVE_UDBL) @@ -1248,10 +1254,6 @@ static t_uint int_div_int(t_uint u1, t_uint u0, t_uint d, t_uint *r) return (t_uint) quotient; #else - const t_uint radix = 1 << biH; - t_uint d0, d1, q0, q1, rAX, r0, quotient; - t_uint u0_msw, u0_lsw; - int s; /* * Algorithm D, Section 4.3.1 - The Art of Computer Programming @@ -1265,7 +1267,7 @@ static t_uint int_div_int(t_uint u1, t_uint u0, t_uint d, t_uint *r) d = d << s; u1 = u1 << s; - u1 |= (u0 >> (32 - s)) & ( (-s) >> 31); + u1 |= ( u0 >> ( 32 - s ) ) & ( -s >> 31 ); u0 = u0 << s; d1 = d >> biH; @@ -1288,7 +1290,7 @@ static t_uint int_div_int(t_uint u1, t_uint u0, t_uint d, t_uint *r) if ( r0 >= radix ) break; } - rAX = (u1 * radix) + (u0_msw - q1 * d); + rAX = ( u1 * radix ) + ( u0_msw - q1 * d ); q0 = rAX / d1; r0 = rAX - q0 * d1; @@ -1301,7 +1303,7 @@ static t_uint int_div_int(t_uint u1, t_uint u0, t_uint d, t_uint *r) } if (r != NULL) - *r = (rAX * radix + u0_lsw - q0 * d) >> s; + *r = ( rAX * radix + u0_lsw - q0 * d ) >> s; quotient = q1 * radix + q0; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 39dc02e7d..b1cd7cbce 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -191,7 +191,7 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl, /* SHA1 + RSA signature */ sig_alg_len += 2; #endif -#if defined(POLARSSL_MD5_C) +#if defined(POLARSSL_MD5_C) && defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) /* MD5 + RSA signature */ sig_alg_len += 2; #endif @@ -209,7 +209,7 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl, /* SHA1 + ECDSA signature */ sig_alg_len += 2; #endif -#if defined(POLARSSL_MD5_C) +#if defined(POLARSSL_MD5_C) && defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) /* MD5 + ECDSA signature */ sig_alg_len += 2; #endif @@ -243,7 +243,7 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl, sig_alg_list[sig_alg_len++] = SSL_HASH_SHA1; sig_alg_list[sig_alg_len++] = SSL_SIG_RSA; #endif -#if defined(POLARSSL_MD5_C) +#if defined(POLARSSL_MD5_C) && defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) sig_alg_list[sig_alg_len++] = SSL_HASH_MD5; sig_alg_list[sig_alg_len++] = SSL_SIG_RSA; #endif @@ -265,7 +265,7 @@ static void ssl_write_signature_algorithms_ext( ssl_context *ssl, sig_alg_list[sig_alg_len++] = SSL_HASH_SHA1; sig_alg_list[sig_alg_len++] = SSL_SIG_ECDSA; #endif -#if defined(POLARSSL_MD5_C) +#if defined(POLARSSL_MD5_C) && defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) sig_alg_list[sig_alg_len++] = SSL_HASH_MD5; sig_alg_list[sig_alg_len++] = SSL_SIG_ECDSA; #endif @@ -2035,6 +2035,14 @@ static int ssl_parse_server_key_exchange( ssl_context *ssl ) SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); return( POLARSSL_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } + +#if !defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) + if( md_alg == POLARSSL_MD_MD5 ) + { + SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + return( POLARSSL_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } +#endif } else #endif /* POLARSSL_SSL_PROTO_TLS1_2 */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 3dc39f416..473f87d08 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -492,6 +492,12 @@ static int ssl_parse_signature_algorithms_ext( ssl_context *ssl, * So, just look at the HashAlgorithm part. */ for( md_cur = md_list(); *md_cur != POLARSSL_MD_NONE; md_cur++ ) { +#if !defined(POLARSSL_SSL_ENABLE_MD5_SIGNATURES) + /* Skip MD5 */ + if( *md_cur == POLARSSL_MD_MD5 ) + continue; +#endif + for( p = buf + 2; p < end; p += 2 ) { if( *md_cur == (int) ssl_md_alg_from_hash( p[0] ) ) { ssl->handshake->sig_alg = p[0];