diff --git a/ChangeLog b/ChangeLog index 8f7843dc6..1b01eb682 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,10 +1,20 @@ mbed TLS ChangeLog (Sorted per branch, date) -= mbed TLS 2.1.x released xxxx-xx-xx + += mbed TLS 2.1.10 branch released 2017-xx-xx Bugfix * Fix ssl_parse_record_header() to silently discard invalid DTLS records as recommended in RFC 6347 Section 4.1.2.7. + * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. + Found by projectgus and jethrogb, #836. + * Fix usage help in ssl_server2 example. Found and fixed by Bei Lin. + * Parse signature algorithm extension when renegotiating. Previously, + renegotiated handshakes would only accept signatures using SHA-1 + regardless of the peer's preferences, or fail if SHA-1 was disabled. + * Fix leap year calculation in x509_date_is_valid() to ensure that invalid + dates on leap years with 100 and 400 intervals are handled correctly. Found + by Nicholas Wilson. #694 = mbed TLS 2.1.9 branch released 2017-08-10 diff --git a/configs/README.txt b/configs/README.txt index e9867bc15..933fa7f21 100644 --- a/configs/README.txt +++ b/configs/README.txt @@ -8,7 +8,7 @@ These files are complete replacements for the default config.h. To use one of them, you can pick one of the following methods: 1. Replace the default file include/mbedtls/config.h with the chosen one. - (Depending on your compiler, you may need to ajust the line with + (Depending on your compiler, you may need to adjust the line with #include "mbedtls/check_config.h" then.) 2. Define MBEDTLS_CONFIG_FILE and adjust the include path accordingly. diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 082832c87..e159e57ea 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -59,7 +59,7 @@ /** * \name DER constants - * These constants comply with DER encoded the ANS1 type tags. + * These constants comply with the DER encoded ASN.1 type tags. * DER encoding uses hexadecimal representation. * An example DER sequence is:\n * - 0x02 -- tag indicating INTEGER diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 60f59a9d5..12a98eb6d 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1579,14 +1579,23 @@ void mbedtls_ssl_conf_sig_hashes( mbedtls_ssl_config *conf, #if defined(MBEDTLS_X509_CRT_PARSE_C) /** - * \brief Set hostname for ServerName TLS extension - * (client-side only) + * \brief Set or reset the hostname to check against the received + * server certificate. It sets the ServerName TLS extension, + * too, if that extension is enabled. (client-side only) * * * \param ssl SSL context - * \param hostname the server hostname + * \param hostname the server hostname, may be NULL to clear hostname + + * \note Maximum hostname length MBEDTLS_SSL_MAX_HOST_NAME_LEN. * - * \return 0 if successful or MBEDTLS_ERR_SSL_ALLOC_FAILED + * \return 0 if successful, MBEDTLS_ERR_SSL_ALLOC_FAILED on + * allocation failure, MBEDTLS_ERR_SSL_BAD_INPUT_DATA on + * too long input hostname. + * + * Hostname set to the one provided on success (cleared + * when NULL). On allocation failure hostname is cleared. + * On too long input failure, old hostname is unchanged. */ int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 31eb203d8..94c521dd9 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -80,6 +80,13 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, } /* + * Sect. 3, RFC 6066 (TLS Extensions Definitions) + * + * In order to provide any of the server names, clients MAY include an + * extension of type "server_name" in the (extended) client hello. The + * "extension_data" field of this extension SHALL contain + * "ServerNameList" where: + * * struct { * NameType name_type; * select (name_type) { @@ -96,6 +103,7 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, * struct { * ServerName server_name_list<1..2^16-1> * } ServerNameList; + * */ *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SERVERNAME >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SERVERNAME ) & 0xFF ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 01b4ada30..1002adfd5 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1603,11 +1603,8 @@ read_record_header: #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) case MBEDTLS_TLS_EXT_SIG_ALG: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); -#if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS ) - break; -#endif + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); + ret = ssl_parse_signature_algorithms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ba586a05e..8088e881c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5980,7 +5980,7 @@ void mbedtls_ssl_conf_sig_hashes( mbedtls_ssl_config *conf, { conf->sig_hashes = hashes; } -#endif +#endif /* MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ #if defined(MBEDTLS_ECP_C) /* @@ -5991,36 +5991,55 @@ void mbedtls_ssl_conf_curves( mbedtls_ssl_config *conf, { conf->curve_list = curve_list; } -#endif +#endif /* MBEDTLS_ECP_C */ #if defined(MBEDTLS_X509_CRT_PARSE_C) int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ) { - size_t hostname_len; + /* Initialize to suppress unnecessary compiler warning */ + size_t hostname_len = 0; + + /* Check if new hostname is valid before + * making any change to current one */ + + if( hostname != NULL ) + { + hostname_len = strlen( hostname ); + + if( hostname_len > MBEDTLS_SSL_MAX_HOST_NAME_LEN ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + + /* Now it's clear that we will overwrite the old hostname, + * so we can free it safely */ + + if( ssl->hostname != NULL ) + { + mbedtls_zeroize( ssl->hostname, strlen( ssl->hostname ) ); + mbedtls_free( ssl->hostname ); + } + + /* Passing NULL as hostname shall clear the old one */ if( hostname == NULL ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + { + ssl->hostname = NULL; + } + else + { + ssl->hostname = mbedtls_calloc( 1, hostname_len + 1 ); - hostname_len = strlen( hostname ); + if( ssl->hostname == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - if( hostname_len + 1 == 0 ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + memcpy( ssl->hostname, hostname, hostname_len ); - if( hostname_len > MBEDTLS_SSL_MAX_HOST_NAME_LEN ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - - ssl->hostname = mbedtls_calloc( 1, hostname_len + 1 ); - - if( ssl->hostname == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - - memcpy( ssl->hostname, hostname, hostname_len ); - - ssl->hostname[hostname_len] = '\0'; + ssl->hostname[hostname_len] = '\0'; + } return( 0 ); } -#endif +#endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) void mbedtls_ssl_conf_sni( mbedtls_ssl_config *conf, diff --git a/library/x509.c b/library/x509.c index e87ba69da..3cfa1d1e9 100644 --- a/library/x509.c +++ b/library/x509.c @@ -491,9 +491,10 @@ static int x509_parse_int( unsigned char **p, size_t n, int *res ) return( 0 ); } -static int x509_date_is_valid(const mbedtls_x509_time *t) +static int x509_date_is_valid(const mbedtls_x509_time *t ) { int ret = MBEDTLS_ERR_X509_INVALID_DATE; + int month_len; CHECK_RANGE( 0, 9999, t->year ); CHECK_RANGE( 0, 23, t->hour ); @@ -503,17 +504,22 @@ static int x509_date_is_valid(const mbedtls_x509_time *t) switch( t->mon ) { case 1: case 3: case 5: case 7: case 8: case 10: case 12: - CHECK_RANGE( 1, 31, t->day ); + month_len = 31; break; case 4: case 6: case 9: case 11: - CHECK_RANGE( 1, 30, t->day ); + month_len = 30; break; case 2: - CHECK_RANGE( 1, 28 + (t->year % 4 == 0), t->day ); + if( ( !( t->year % 4 ) && t->year % 100 ) || + !( t->year % 400 ) ) + month_len = 29; + else + month_len = 28; break; default: return( ret ); } + CHECK_RANGE( 1, month_len, t->day ); return( 0 ); } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 728b9bae6..0271a8f4d 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -314,7 +314,7 @@ int main( void ) #define USAGE \ "\n usage: ssl_server2 param=<>...\n" \ "\n acceptable parameters:\n" \ - " server_addr=%%d default: (all interfaces)\n" \ + " server_addr=%%s default: (all interfaces)\n" \ " server_port=%%d default: 4433\n" \ " debug_level=%%d default: 0 (disabled)\n" \ " nbio=%%d default: 0 (blocking I/O)\n" \ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 9c9cf4651..d8f0fd720 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1340,6 +1340,40 @@ run_test "Renegotiation: server-initiated" \ -s "=> renegotiate" \ -s "write hello request" +# Checks that no Signature Algorithm with SHA-1 gets negotiated. Negotiating SHA-1 would mean that +# the server did not parse the Signature Algorithm extension. This test is valid only if an MD +# algorithm stronger than SHA-1 is enabled in config.h +run_test "Renegotiation: Signature Algorithms parsing, client-initiated" \ + "$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional" \ + "$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ + 0 \ + -c "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -s "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -c "=> renegotiate" \ + -s "=> renegotiate" \ + -S "write hello request" \ + -S "client hello v3, signature_algorithm ext: 2" # Is SHA-1 negotiated? + +# Checks that no Signature Algorithm with SHA-1 gets negotiated. Negotiating SHA-1 would mean that +# the server did not parse the Signature Algorithm extension. This test is valid only if an MD +# algorithm stronger than SHA-1 is enabled in config.h +run_test "Renegotiation: Signature Algorithms parsing, server-initiated" \ + "$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional renegotiate=1" \ + "$P_CLI debug_level=3 exchanges=2 renegotiation=1" \ + 0 \ + -c "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -s "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -c "found renegotiation extension" \ + -c "=> renegotiate" \ + -s "=> renegotiate" \ + -s "write hello request" \ + -S "client hello v3, signature_algorithm ext: 2" # Is SHA-1 negotiated? + run_test "Renegotiation: double" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional renegotiate=1" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index a39f6f09f..b92c1fe8a 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -54,3 +54,6 @@ ssl_dtls_replay:"abcd12340000,abcd12340100":"abcd12340101":0 SSL DTLS replay: big jump then just delayed ssl_dtls_replay:"abcd12340000,abcd12340100":"abcd123400ff":0 + +SSL SET_HOSTNAME memory leak: call ssl_set_hostname twice +ssl_set_hostname_twice:"server0":"server1" diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 8d3448cbc..60683afee 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -40,3 +40,16 @@ void ssl_dtls_replay( char *prevs, char *new, int ret ) mbedtls_ssl_config_free( &conf ); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C */ +void ssl_set_hostname_twice( char *hostname0, char *hostname1 ) +{ + mbedtls_ssl_context ssl; + mbedtls_ssl_init( &ssl ); + + TEST_ASSERT( mbedtls_ssl_set_hostname( &ssl, hostname0 ) == 0 ); + TEST_ASSERT( mbedtls_ssl_set_hostname( &ssl, hostname1 ) == 0 ); + + mbedtls_ssl_free( &ssl ); +} +/* END_CASE */ \ No newline at end of file diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index e337ed7cf..5731677fe 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1620,3 +1620,18 @@ X509 Get time (UTC invalid character in sec) depends_on:MBEDTLS_X509_USE_C x509_get_time:MBEDTLS_ASN1_UTC_TIME:"0011302359n0Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 +X509 Get time (Generalized Time, year multiple of 100 but not 400 is not a leap year) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_GENERALIZED_TIME:"19000229000000Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0 + +X509 Get time (Generalized Time, year multiple of 4 but not 100 is a leap year) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_GENERALIZED_TIME:"19920229000000Z":0:1992:2:29:0:0:0 + +X509 Get time (Generalized Time, year multiple of 400 is a leap year) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_GENERALIZED_TIME:"20000229000000Z":0:2000:2:29:0:0:0 + +X509 Get time (Generalized Time invalid leap year not multiple of 4, 100 or 400) +depends_on:MBEDTLS_X509_USE_C +x509_get_time:MBEDTLS_ASN1_GENERALIZED_TIME:"19910229000000Z":MBEDTLS_ERR_X509_INVALID_DATE:0:0:0:0:0:0