From 26182edd0cd5a09f2435b1123e384e8a22fac52d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 29 Sep 2017 15:45:12 +0200 Subject: [PATCH 01/30] Allow comments in test data files --- ChangeLog | 5 +++++ tests/scripts/generate_code.pl | 21 +++++++++++++++++++++ tests/suites/main_test.function | 19 ++++++++++++------- tests/suites/test_suite_md.data | 1 + tests/suites/test_suite_mdx.data | 1 + tests/suites/test_suite_rsa.data | 3 +++ tests/suites/test_suite_shax.data | 1 + 7 files changed, 44 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 227faed6b..2bbc4c333 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Features + * Allow comments in test data files. + = mbed TLS 2.6.0 branch released 2017-08-10 Security diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 84e949dfa..a48631946 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -49,6 +49,27 @@ # file name is used to replace the symbol 'TESTCASE_FILENAME' in the main # code file above. # +# A test data file consists of a sequence of paragraphs separated by +# a single empty line. Line breaks may be in Unix (LF) or Windows (CRLF) +# format. Lines starting with the character '#' are ignored +# (the parser behaves as if they were not present). +# +# Each paragraph describes one test case and must consist of: (1) one +# line which is the test case name; (2) an optional line starting with +# the 11-character prefix "depends_on:"; (3) a line containing the test +# function to execute and its parameters. +# +# A depends_on: line consists of a list of compile-time options +# separated by the character ':', with no whitespace. The test case +# is executed only if this compilation option is enabled in config.h. +# +# The last line of each paragraph contains a test function name and +# a list of parameters separated by the character ':'. Running the +# test case calls this function with the specified parameters. Each +# parameter may either be an integer written in decimal or hexadecimal, +# or a string surrounded by double quotes which may not contain the +# ':' character. +# use strict; diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index a7bb41de3..551f239d2 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -140,14 +140,19 @@ int get_line( FILE *f, char *buf, size_t len ) { char *ret; - ret = fgets( buf, len, f ); - if( ret == NULL ) - return( -1 ); + buf[0] = '#'; - if( strlen( buf ) && buf[strlen(buf) - 1] == '\n' ) - buf[strlen(buf) - 1] = '\0'; - if( strlen( buf ) && buf[strlen(buf) - 1] == '\r' ) - buf[strlen(buf) - 1] = '\0'; + while( buf[0] == '#' ) + { + ret = fgets( buf, len, f ); + if( ret == NULL ) + return( -1 ); + + if( strlen( buf ) && buf[strlen(buf) - 1] == '\n' ) + buf[strlen(buf) - 1] = '\0'; + if( strlen( buf ) && buf[strlen(buf) - 1] == '\r' ) + buf[strlen(buf) - 1] = '\0'; + } return( 0 ); } diff --git a/tests/suites/test_suite_md.data b/tests/suites/test_suite_md.data index 71d1f6dde..abd8e55d9 100644 --- a/tests/suites/test_suite_md.data +++ b/tests/suites/test_suite_md.data @@ -1,3 +1,4 @@ +# Tests of the generic message digest interface MD process mbedtls_md_process: diff --git a/tests/suites/test_suite_mdx.data b/tests/suites/test_suite_mdx.data index 2d403b410..3d063a477 100644 --- a/tests/suites/test_suite_mdx.data +++ b/tests/suites/test_suite_mdx.data @@ -1,3 +1,4 @@ +# Test MD2, MD4, MD5 and RIPEMD160 mbedtls_md2 Test vector RFC1319 #1 md2_text:"":"8350e5a3e24c153df2275c9f80692773" diff --git a/tests/suites/test_suite_rsa.data b/tests/suites/test_suite_rsa.data index 5013ac8b0..fc7d93588 100644 --- a/tests/suites/test_suite_rsa.data +++ b/tests/suites/test_suite_rsa.data @@ -1,5 +1,6 @@ RSA PKCS1 Verify v1.5 CAVS #1 depends_on:MBEDTLS_SHA1_C:MBEDTLS_PKCS1_V15 +# Good padding but wrong hash mbedtls_rsa_pkcs1_verify:"d6248c3e96b1a7e5fea978870fcc4c9786b4e5156e16b7faef4557d667f730b8bc4c784ef00c624df5309513c3a5de8ca94c2152e0459618666d3148092562ebc256ffca45b27fd2d63c68bd5e0a0aefbe496e9e63838a361b1db6fc272464f191490bf9c029643c49d2d9cd08833b8a70b4b3431f56fb1eb55ccd39e77a9c92":MBEDTLS_RSA_PKCS_V15:MBEDTLS_MD_SHA1:1024:16:"e28a13548525e5f36dccb24ecb7cc332cc689dfd64012604c9c7816d72a16c3f5fcdc0e86e7c03280b1c69b586ce0cd8aec722cc73a5d3b730310bf7dfebdc77ce5d94bbc369dc18a2f7b07bd505ab0f82224aef09fdc1e5063234255e0b3c40a52e9e8ae60898eb88a766bdd788fe9493d8fd86bcdd2884d5c06216c65469e5":16:"3":"3203b7647fb7e345aa457681e5131777f1adc371f2fba8534928c4e52ef6206a856425d6269352ecbf64db2f6ad82397768cafdd8cd272e512d617ad67992226da6bc291c31404c17fd4b7e2beb20eff284a44f4d7af47fd6629e2c95809fa7f2241a04f70ac70d3271bb13258af1ed5c5988c95df7fa26603515791075feccd":MBEDTLS_ERR_RSA_VERIFY_FAILED RSA PKCS1 Verify v1.5 CAVS #2 @@ -24,6 +25,7 @@ mbedtls_rsa_pkcs1_verify:"44637d3b8de525fd589237bc81229c8966d3af24540850c2403633 RSA PKCS1 Verify v1.5 CAVS #7 depends_on:MBEDTLS_SHA512_C:MBEDTLS_PKCS1_V15 +# Bad padding after performing the public key operation mbedtls_rsa_pkcs1_verify:"d03f12276f6ba7545b8fce719471bd253791878809694e8754f3b389f26c9253a758ed28b4c62535a8d5702d7a778731d5759ff2b3b39b192db680e791632918b6093c0e8ca25c2bf756a07fde4144a37f769fe4054455a45cb8cefe4462e7a9a45ce71f2189b4fef01b47aee8585d44dc9d6fa627a3e5f08801871731f234cd":MBEDTLS_RSA_PKCS_V15:MBEDTLS_MD_SHA384:1024:16:"e28a13548525e5f36dccb24ecb7cc332cc689dfd64012604c9c7816d72a16c3f5fcdc0e86e7c03280b1c69b586ce0cd8aec722cc73a5d3b730310bf7dfebdc77ce5d94bbc369dc18a2f7b07bd505ab0f82224aef09fdc1e5063234255e0b3c40a52e9e8ae60898eb88a766bdd788fe9493d8fd86bcdd2884d5c06216c65469e5":16:"3":"d93a878c1ce86571590b0e43794b3edb23552797c4b8c9e3da4fe1cc4ac0566acd3b10541fe9a7a79f5ea4892d3069ca6903efb5c40c47eb8a9c781eb4249281d40c3d96aae16da1bb4daaece6a26eca5f41c062b4124a64fc9d340cba5ab0d1f5affff6515a87f0933774fd4322d2fa497cd6f708a429ca56dcb1fd3db623d0":MBEDTLS_ERR_RSA_INVALID_PADDING RSA PKCS1 Verify v1.5 CAVS #8 @@ -365,6 +367,7 @@ RSA Generate Key - 2048 bit key mbedtls_rsa_gen_key:2048:3:0 RSA Generate Key - 1025 bit key +# mbedtls_rsa_gen_key only supports even-sized keys mbedtls_rsa_gen_key:1025:3:MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA PKCS1 Encrypt Bad RNG diff --git a/tests/suites/test_suite_shax.data b/tests/suites/test_suite_shax.data index ea2a18380..ee8074dc0 100644 --- a/tests/suites/test_suite_shax.data +++ b/tests/suites/test_suite_shax.data @@ -1,3 +1,4 @@ +# Test the operation of SHA-1 and SHA-2 SHA-1 Test Vector NIST CAVS #1 depends_on:MBEDTLS_SHA1_C mbedtls_sha1:"":"da39a3ee5e6b4b0d3255bfef95601890afd80709" From 5b7ee07ff6a29f70c8a26b2f4641d9d0759f2667 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 29 Sep 2017 18:00:25 +0200 Subject: [PATCH 02/30] Cleaned up get_line for test data files Look, ma, a use for do...while! Also removed 1-3 calls to strlen. --- tests/suites/main_test.function | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 551f239d2..20add3c77 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -136,23 +136,31 @@ DISPATCH_FUNCTION "TESTCASE_FILENAME" +/** Retrieve one input line into buf, which must have room for len + * bytes. The trailing line break (if any) is stripped from the result. + * Lines beginning with the character '#' are skipped. Lines that are + * more than len-1 bytes long including the trailing line break are + * truncated; note that the following bytes remain in the input stream. + * + * \return 0 on success, -1 on error or end of file + */ int get_line( FILE *f, char *buf, size_t len ) { char *ret; - buf[0] = '#'; - - while( buf[0] == '#' ) + do { ret = fgets( buf, len, f ); if( ret == NULL ) return( -1 ); - - if( strlen( buf ) && buf[strlen(buf) - 1] == '\n' ) - buf[strlen(buf) - 1] = '\0'; - if( strlen( buf ) && buf[strlen(buf) - 1] == '\r' ) - buf[strlen(buf) - 1] = '\0'; } + while( buf[0] == '#' ); + + ret = buf + strlen( buf ); + if( ret-- > buf && *ret == '\n' ) + *ret = '\0'; + if( ret-- > buf && *ret == '\r' ) + *ret = '\0'; return( 0 ); } From 00afe1c0469f331098288263bbc30aa8bc8cc0a2 Mon Sep 17 00:00:00 2001 From: Xinyu Chen Date: Tue, 22 Nov 2016 14:56:18 +0800 Subject: [PATCH 03/30] Correct the printf message of the DTLS handshake. Make it consistent with dtls_server.c --- programs/ssl/dtls_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/dtls_client.c b/programs/ssl/dtls_client.c index e18ee42a1..f271bad30 100644 --- a/programs/ssl/dtls_client.c +++ b/programs/ssl/dtls_client.c @@ -203,7 +203,7 @@ int main( int argc, char *argv[] ) /* * 4. Handshake */ - mbedtls_printf( " . Performing the SSL/TLS handshake..." ); + mbedtls_printf( " . Performing the DTLS handshake..." ); fflush( stdout ); do ret = mbedtls_ssl_handshake( &ssl ); From 376f7f5fe19b1ed354467adc31060b9bb41ae679 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 23 Aug 2017 16:04:40 +0300 Subject: [PATCH 04/30] Fix typo in configs/README.txt file Fix typo in Readme file: ajust->adjust --- configs/README.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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. From 713fe7f66c4b393e1905568786d6d615a761edbd Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 May 2017 11:24:30 +0100 Subject: [PATCH 05/30] Add test case calling ssl_set_hostname twice Add a test case calling ssl_set_hostname twice to test_suite_ssl. When run in CMake build mode ASan, this catches the current leak, but will hopefully be fine with the new version. --- tests/suites/test_suite_ssl.data | 3 +++ tests/suites/test_suite_ssl.function | 13 +++++++++++++ 2 files changed, 16 insertions(+) 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 From 39f5d359f5bdebd76c4491920002dda8c4789fb8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 7 Apr 2017 13:25:49 +0100 Subject: [PATCH 06/30] Make mbedtls_ssl_set_hostname safe to be called multiple times Zeroize and free previously set hostnames before overwriting them. Also, allow clearance of hostname by providing NULL parameter. --- library/ssl_tls.c | 55 +++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 661ae7065..8d143a383 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6159,7 +6159,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) /* @@ -6170,32 +6170,51 @@ 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 ); } From f5f9d11accdbd95ec82d28785c14019eb8c925d9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 7 Apr 2017 12:59:32 +0100 Subject: [PATCH 07/30] Enhance documentation of mbedtls_ssl_set_hostname (1) Add missing error condition (2) Specify allowance and effect of of NULL hostname parameter (3) Describe effect of function on failure --- include/mbedtls/ssl.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index cc0007006..87ea00dbb 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1794,15 +1794,23 @@ void mbedtls_ssl_conf_sig_hashes( mbedtls_ssl_config *conf, #if defined(MBEDTLS_X509_CRT_PARSE_C) /** - * \brief Set the hostname to check against the received server - * certificate. It sets the ServerName TLS extension too, - * if the extension is enabled. - * (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, MBEDTLS_ERR_SSL_ALLOC_FAILED on + * allocation failure, MBEDTLS_ERR_BAD_INPUT_DATA on + * too long input hostname. + * + * \post 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. * - * \return 0 if successful or MBEDTLS_ERR_SSL_ALLOC_FAILED */ int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ From 2f38a43d3a53173d3129457d5c93d99834fb0ca9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 7 Apr 2017 13:02:16 +0100 Subject: [PATCH 08/30] Enhance documentation of ssl_write_hostname_ext, adapt ChangeLog. Add a reference to the relevant RFC, adapt ChangeLog. --- ChangeLog | 2 ++ include/mbedtls/ssl.h | 5 ++--- library/ssl_cli.c | 8 ++++++++ library/ssl_tls.c | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2bbc4c333..565ea2904 100644 --- a/ChangeLog +++ b/ChangeLog @@ -194,6 +194,8 @@ Security team. #569 CVE-2017-2784 Bugfix + * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. + Found by jethrogb, #836. * Fix output certificate verification flags set by x509_crt_verify_top() when traversing a chain of trusted CA. The issue would cause both flags, MBEDTLS_X509_BADCERT_NOT_TRUSTED and MBEDTLS_X509_BADCERT_EXPIRED, to be diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 87ea00dbb..e98101e19 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1804,13 +1804,12 @@ void mbedtls_ssl_conf_sig_hashes( mbedtls_ssl_config *conf, * \note Maximum hostname length MBEDTLS_SSL_MAX_HOST_NAME_LEN. * * \return 0 if successful, MBEDTLS_ERR_SSL_ALLOC_FAILED on - * allocation failure, MBEDTLS_ERR_BAD_INPUT_DATA on + * allocation failure, MBEDTLS_ERR_SSL_BAD_INPUT_DATA on * too long input hostname. * - * \post Hostname set to the one provided on success (cleared + * 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 a2b9f8cfe..19bf021e2 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_tls.c b/library/ssl_tls.c index 8d143a383..de2490ced 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6218,7 +6218,7 @@ int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ) 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, From 83ce8201dcd2739a2350fcfc20d28083653c4e3f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Sat, 30 Sep 2017 23:39:46 +0100 Subject: [PATCH 09/30] Update ChangeLog for fix to #836 --- ChangeLog | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 565ea2904..ec9125961 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,10 @@ mbed TLS ChangeLog (Sorted per branch, date) Features * Allow comments in test data files. +Bugfix + * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. + Found by projectgus and jethrogb, #836. + = mbed TLS 2.6.0 branch released 2017-08-10 Security @@ -194,8 +198,6 @@ Security team. #569 CVE-2017-2784 Bugfix - * Fix memory leak in mbedtls_ssl_set_hostname() when called multiple times. - Found by jethrogb, #836. * Fix output certificate verification flags set by x509_crt_verify_top() when traversing a chain of trusted CA. The issue would cause both flags, MBEDTLS_X509_BADCERT_NOT_TRUSTED and MBEDTLS_X509_BADCERT_EXPIRED, to be From 7da7cb399e415ba68676964c1eecafbe0da9163f Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 26 Sep 2017 11:29:11 +0300 Subject: [PATCH 10/30] Fix ssl_server2 sample application prompt FIx the type of server_addr parameter from %d to %s. Issue reported by Email by Bei Jin --- programs/ssl/ssl_server2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a25886824..1285abcbd 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -326,7 +326,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" \ From 967a60502e02fddd587ada912842d82c7505757f Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 2 Oct 2017 19:12:54 +0100 Subject: [PATCH 11/30] Fix changelog for ssl_server2.c usage fix --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index ec9125961..b3d4d519a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,6 +8,7 @@ Features Bugfix * 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. = mbed TLS 2.6.0 branch released 2017-08-10 From 82759aa1c7d59e074eb9e2a0d28ca2a6b3ffb0fc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 16 Jun 2017 14:52:39 +0200 Subject: [PATCH 12/30] Note in README that GNU make is required Our README claims that we only use basic Make functionality, but in fact GNU make is required for conditional compilation. Document this. Addresses issue #967 --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 11b4ebf6a..624e03fe3 100644 --- a/README.md +++ b/README.md @@ -14,11 +14,11 @@ Compiling There are currently four active build systems used within mbed TLS releases: - yotta -- Make +- GNU Make - CMake - Microsoft Visual Studio (Visual Studio 6 and Visual Studio 2010) -The main systems used for development are CMake and Make. Those systems are always complete and up-to-date. The others should reflect all changes present in the CMake and Make build system, although features may not be ported there automatically. +The main systems used for development are CMake and GNU Make. Those systems are always complete and up-to-date. The others should reflect all changes present in the CMake and Make build system, although features may not be ported there automatically. Yotta, as a build system, is slightly different from the other build systems: @@ -54,9 +54,9 @@ For more details on the yotta/mbed OS edition of mbed TLS, including example pro ### Make -We intentionally only use the minimum of `Make` functionality, as a lot of `Make` features are not supported on all different implementations of Make or on different platforms. As such, the Makefiles sometimes require some manual changes or export statements in order to work for your platform. +We require GNU Make. To build the library and the sample programs, GNU Make and a C compiler are sufficient. Some of the more advanced build targets require some Unix/Linux tools. -In order to build from the source code using Make, just enter at the command line: +In order to build from the source code using GNU Make, just enter at the command line: make From ec82da4cb2fde14244fca2d8583a0159b49cdefa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 2 Oct 2017 10:52:50 +0200 Subject: [PATCH 13/30] Restored note about using minimum functionality in makefiles --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 624e03fe3..5ffd2ae56 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,8 @@ For more details on the yotta/mbed OS edition of mbed TLS, including example pro We require GNU Make. To build the library and the sample programs, GNU Make and a C compiler are sufficient. Some of the more advanced build targets require some Unix/Linux tools. +We intentionally only use a minimum of functionality in the makefiles in order to keep them as simple and independent of different toolchains as possible, to allow users to more easily move between different platforms. Users who need more features are recommended to use CMake. + In order to build from the source code using GNU Make, just enter at the command line: make From 074c58f08bceeb50fd59c131ed54c07db356df37 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 6 Sep 2017 15:33:34 +0100 Subject: [PATCH 14/30] Always print gmt_unix_time in TLS client Change ssl_parse_server_hello() so that the parsed first four random bytes from the ServerHello message are printed by the TLS client as a Unix timestamp regardless of whether MBEDTLS_DEBUG_C is defined. The debug message will only be printed if debug_level is 3 or higher. Unconditionally enabling the debug print enabled testing of this value. --- library/ssl_cli.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 19bf021e2..544c8cf5c 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1448,9 +1448,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #endif int handshake_failure = 0; const mbedtls_ssl_ciphersuite_t *suite_info; -#if defined(MBEDTLS_DEBUG_C) - uint32_t t; -#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server hello" ) ); @@ -1553,13 +1550,11 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); } -#if defined(MBEDTLS_DEBUG_C) - t = ( (uint32_t) buf[2] << 24 ) - | ( (uint32_t) buf[3] << 16 ) - | ( (uint32_t) buf[4] << 8 ) - | ( (uint32_t) buf[5] ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", t ) ); -#endif + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", + ( (uint32_t) buf[2] << 24 ) | + ( (uint32_t) buf[3] << 16 ) | + ( (uint32_t) buf[4] << 8 ) | + ( (uint32_t) buf[5] ) ) ); memcpy( ssl->handshake->randbytes + 32, buf + 2, 32 ); From a46a58ab942c324c414d46bc78fb4eb4db26f4ee Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 6 Sep 2017 15:38:07 +0100 Subject: [PATCH 15/30] Extend ssl-opt.h so that run_test takes function Extend the run_test function in ssl-opt.sh so that it accepts the -f and -F options. These parameters take an argument which is the name of a shell function that will be called by run_test and will be given the client input and output debug log. The idea is that these functions are defined by each test and they can be used to do some custom check beyon those allowed by the pattern matching capabilities of the run_test function. --- tests/ssl-opt.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 280fc6348..b349512cc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -357,9 +357,11 @@ detect_dtls() { # Options: -s pattern pattern that must be present in server output # -c pattern pattern that must be present in client output # -u pattern lines after pattern must be unique in client output +# -f call shell function on client output # -S pattern pattern that must be absent in server output # -C pattern pattern that must be absent in client output # -U pattern lines after pattern must be unique in server output +# -F call shell function on server output run_test() { NAME="$1" shift 1 @@ -546,6 +548,18 @@ run_test() { return fi ;; + "-F") + if ! $2 "$SRV_OUT"; then + fail "function call to '$2' failed on Server output" + return + fi + ;; + "-f") + if ! $2 "$CLI_OUT"; then + fail "function call to '$2' failed on Client output" + return + fi + ;; *) echo "Unknown test: $1" >&2 From ac36e382a97a54b06cca1f61f70eddc8c3904f4d Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 6 Sep 2017 15:44:01 +0100 Subject: [PATCH 16/30] Add ssl-opt.sh test to check gmt_unix_time is good Add a test to ssl-opt.sh that parses the client and server debug output and then checks that the Unix timestamp in the ServerHello message is within acceptable bounds. --- tests/ssl-opt.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b349512cc..e23daeeaf 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -321,6 +321,33 @@ wait_server_start() { fi } +# Given the client or server debug output, parse the unix timestamp that is +# included in the first 4 bytes of the random bytes and check that its within +# acceptable bounds +check_server_hello_time() { + # Extract the time from the debug (lvl 3) output of the client + SERVER_HELLO_TIME="$(cat "$1" | sed -n 's/.*server hello, current time: \([0-9]\+\)$/\1/p')" + # Get the Unix timestamp for now + CUR_TIME=$(date +'%s') + THRESHOLD_IN_SECS=300 + + # Check if the ServerHello time was printed + if [ -z "$SERVER_HELLO_TIME" ]; then + return 1 + fi + + # Check the time in ServerHello is within acceptable bounds + if [ $SERVER_HELLO_TIME -lt $(( $CUR_TIME - $THRESHOLD_IN_SECS )) ]; then + # The time in ServerHello is at least 5 minutes before now + return 1 + elif [ $SERVER_HELLO_TIME -gt $(( $CUR_TIME + $THRESHOLD_IN_SECS )) ]; then + # The time in ServerHello is at least 5 minues later than now + return 1 + else + return 0 + fi +} + # wait for client to terminate and set CLI_EXIT # must be called right after starting the client wait_client_done() { @@ -696,6 +723,21 @@ run_test "Default, DTLS" \ -s "Protocol is DTLSv1.2" \ -s "Ciphersuite is TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384" +# Test current time in ServerHello +requires_config_enabled MBEDTLS_HAVE_TIME +run_test "Default, ServerHello contains gmt_unix_time" \ + "$P_SRV debug_level=3" \ + "$P_CLI debug_level=3" \ + 0 \ + -s "Protocol is TLSv1.2" \ + -s "Ciphersuite is TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384" \ + -s "client hello v3, signature_algorithm ext: 6" \ + -s "ECDHE curve: secp521r1" \ + -S "error" \ + -C "error" \ + -f "check_server_hello_time" \ + -F "check_server_hello_time" + # Test for uniqueness of IVs in AEAD ciphersuites run_test "Unique IV in GCM" \ "$P_SRV exchanges=20 debug_level=4" \ From 5987ef451ca75fa492cbd1e8c4a8d3fed5aaf42f Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Thu, 14 Sep 2017 12:41:29 +0100 Subject: [PATCH 17/30] Fix typos in ssl-opt.sh comments --- tests/ssl-opt.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e23daeeaf..1a9482f10 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -322,7 +322,7 @@ wait_server_start() { } # Given the client or server debug output, parse the unix timestamp that is -# included in the first 4 bytes of the random bytes and check that its within +# included in the first 4 bytes of the random bytes and check that it's within # acceptable bounds check_server_hello_time() { # Extract the time from the debug (lvl 3) output of the client @@ -341,7 +341,7 @@ check_server_hello_time() { # The time in ServerHello is at least 5 minutes before now return 1 elif [ $SERVER_HELLO_TIME -gt $(( $CUR_TIME + $THRESHOLD_IN_SECS )) ]; then - # The time in ServerHello is at least 5 minues later than now + # The time in ServerHello is at least 5 minutes later than now return 1 else return 0 From acdae0cb33fe51e2693a92fa8a015bb784cac833 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 15 Sep 2017 15:49:24 +0100 Subject: [PATCH 18/30] Remove use of GNU sed features from ssl-opt.sh --- tests/ssl-opt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1a9482f10..7fcca685b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -326,7 +326,7 @@ wait_server_start() { # acceptable bounds check_server_hello_time() { # Extract the time from the debug (lvl 3) output of the client - SERVER_HELLO_TIME="$(cat "$1" | sed -n 's/.*server hello, current time: \([0-9]\+\)$/\1/p')" + SERVER_HELLO_TIME="$(sed -n 's/.*server hello, current time: //p' < "$1")" # Get the Unix timestamp for now CUR_TIME=$(date +'%s') THRESHOLD_IN_SECS=300 From def0339db2ffb08abb7e26db9d6523d584566f17 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Sun, 1 Oct 2017 16:42:29 +0100 Subject: [PATCH 19/30] Ensure failed test_suite output is sent to stdout The change modifies the template code in tests/suites/helpers.function and tests/suites/main.function so that error messages are printed to stdout instead of being discarded. This makes errors visible regardless of the --verbose flag being passed or not to the test suite programs. --- tests/suites/helpers.function | 19 +++++++++++++------ tests/suites/main_test.function | 24 +++++++++++++++++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 63815df85..cac104a3b 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -99,7 +99,15 @@ typedef UINT32 uint32_t; /*----------------------------------------------------------------------------*/ /* Global variables */ -static int test_errors = 0; + +static struct +{ + int failed; + const char *test; + const char *filename; + int line_no; +} +test_info; /*----------------------------------------------------------------------------*/ @@ -395,10 +403,9 @@ static int rnd_pseudo_rand( void *rng_state, unsigned char *output, size_t len ) static void test_fail( const char *test, int line_no, const char* filename ) { - test_errors++; - if( test_errors == 1 ) - mbedtls_fprintf( stdout, "FAILED\n" ); - mbedtls_fprintf( stdout, " %s\n at line %d, %s\n", test, line_no, - filename ); + test_info.failed = 1; + test_info.test = test; + test_info.line_no = line_no; + test_info.filename = filename; } diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 20add3c77..120247e53 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -339,6 +339,9 @@ int main(int argc, const char *argv[]) testfile_count = 1; } + /* Initialize the struct that holds information about the last test */ + memset( &test_info, 0, sizeof( test_info ) ); + /* Now begin to execute the tests in the testfiles */ for ( testfile_index = 0; testfile_index < testfile_count; @@ -369,7 +372,7 @@ int main(int argc, const char *argv[]) if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) break; - mbedtls_fprintf( stdout, "%s%.66s", test_errors ? "\n" : "", buf ); + mbedtls_fprintf( stdout, "%s%.66s", test_info.failed ? "\n" : "", buf ); mbedtls_fprintf( stdout, " " ); for( i = strlen( buf ) + 1; i < 67; i++ ) mbedtls_fprintf( stdout, "." ); @@ -409,11 +412,11 @@ int main(int argc, const char *argv[]) break; cnt = parse_arguments( buf, strlen(buf), params ); } - + // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { - test_errors = 0; + test_info.failed = 0; #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) /* Suppress all output from the library unless we're verbose @@ -467,9 +470,20 @@ int main(int argc, const char *argv[]) unmet_dep_count = 0; } - else if( ret == DISPATCH_TEST_SUCCESS && test_errors == 0 ) + else if( ret == DISPATCH_TEST_SUCCESS ) { - mbedtls_fprintf( stdout, "PASS\n" ); + if( test_info.failed == 0 ) + { + mbedtls_fprintf( stdout, "PASS\n" ); + } + else + { + total_errors++; + mbedtls_fprintf( stdout, "FAILED\n" ); + mbedtls_fprintf( stdout, " %s\n at line %d, %s\n", + test_info.test, test_info.line_no, + test_info.filename ); + } fflush( stdout ); } else if( ret == DISPATCH_INVALID_TEST_DATA ) From 86968c6dd1d5b272de78060a6dca7f7f2f961574 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 12 Jul 2017 14:04:40 +0100 Subject: [PATCH 20/30] Fix typo and bracketing in macro args --- library/net_sockets.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 80be6ec6a..31c42db05 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -63,8 +63,8 @@ #endif #endif /* _MSC_VER */ -#define read(fd,buf,len) recv(fd,(char*)buf,(int) len,0) -#define write(fd,buf,len) send(fd,(char*)buf,(int) len,0) +#define read(fd,buf,len) recv( fd, (char*)( buf ), (int)( len ), 0 ) +#define write(fd,buf,len) send( fd, (char*)( buf ), (int)( len ), 0 ) #define close(fd) closesocket(fd) static int wsa_init_done = 0; @@ -85,7 +85,7 @@ static int wsa_init_done = 0; #endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */ /* Some MS functions want int and MSVC warns if we pass size_t, - * but the standard fucntions use socklen_t, so cast only for MSVC */ + * but the standard functions use socklen_t, so cast only for MSVC */ #if defined(_MSC_VER) #define MSVC_INT_CAST (int) #else From 134a082455a9d1405422a4afd40e7992d25530c1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Oct 2017 16:51:24 +0200 Subject: [PATCH 21/30] Fixed "config.pl get" for options with no value Between 2.5.0 and 2.6.0, "scripts/config.pl get MBEDTLS_XXX" was fixed for config.h lines with a comment at the end, but that broke the case of macros with an empty expansion. Support all cases. --- scripts/config.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/config.pl b/scripts/config.pl index 406413bd5..4cf4ac8b8 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -205,7 +205,7 @@ for my $line (@config_lines) { $done = 1; } } elsif (!$done && $action eq "get") { - if ($line =~ /^\s*#define\s*$name\s*([^\s]+)\s*\b/) { + if ($line =~ /^\s*#define\s*$name(?:\s+(.*?))\s*(?:$|\/\*|\/\/)/) { $value = $1; $done = 1; } From 58e5fdc0ca76029c48f9523b4e4d7af4ae71abd2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Oct 2017 16:54:28 +0200 Subject: [PATCH 22/30] config.pl get: don't rewrite config.h; detect write errors scripts/config.pl would always rewrite config.h if it was reading it. This commit changes it to not modify the file when only reading is required, i.e. for the get command. Also, die if writing config.h fails (e.g. disk full). --- scripts/config.pl | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/config.pl b/scripts/config.pl index 4cf4ac8b8..9fc606278 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -175,7 +175,10 @@ if ($action eq "realfull") { $no_exclude_re = join '|', @non_excluded; } -open my $config_write, '>', $config_file or die "write $config_file: $!\n"; +my $config_write = undef; +if ($action ne "get") { + open $config_write, '>', $config_file or die "write $config_file: $!\n"; +} my $done; for my $line (@config_lines) { @@ -211,7 +214,9 @@ for my $line (@config_lines) { } } - print $config_write $line; + if (defined $config_write) { + print $config_write $line or die "write $config_file: $!\n";; + } } # Did the set command work? @@ -223,10 +228,12 @@ if ($action eq "set"&& $force_option && !$done) { $line .= "\n"; $done = 1; - print $config_write $line; + print $config_write $line or die "write $config_file: $!\n"; } -close $config_write; +if (defined $config_write) { + close $config_write or die "close $config_file: $!\n"; +} if ($action eq "get") { if($done) { From ad8b9ec9e9924929752af3769a64b8867f5c39a6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Oct 2017 16:56:18 +0200 Subject: [PATCH 23/30] config.pl get: be better behaved When printing an option's value, print a newline at the end. When the requested option is missing, fail with status 1 (the usual convention for "not found") rather than -1 (which has a system-dependent effect). --- scripts/config.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/config.pl b/scripts/config.pl index 9fc606278..b99140a37 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -56,7 +56,7 @@ Commands unset - Comments out the #define for the given symbol if present in the configuration file. get - Finds the #define for the given symbol, returning - an exitcode of 0 if the symbol is found, and -1 if + an exitcode of 0 if the symbol is found, and 1 if not. The value of the symbol is output if one is specified in the configuration file. full - Uncomments all #define's in the configuration file @@ -220,7 +220,7 @@ for my $line (@config_lines) { } # Did the set command work? -if ($action eq "set"&& $force_option && !$done) { +if ($action eq "set" && $force_option && !$done) { # If the force option was set, append the symbol to the end of the file my $line = "#define $name"; @@ -236,14 +236,14 @@ if (defined $config_write) { } if ($action eq "get") { - if($done) { + if ($done) { if ($value ne '') { - print $value; + print "$value\n"; } exit 0; } else { # If the symbol was not found, return an error - exit -1; + exit 1; } } From ae98d4aa397016e15ce2e2f8ba455322712157ec Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 10 Oct 2017 11:26:45 +0200 Subject: [PATCH 24/30] Minor style fix --- scripts/config.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/config.pl b/scripts/config.pl index b99140a37..5a06a3338 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -215,7 +215,7 @@ for my $line (@config_lines) { } if (defined $config_write) { - print $config_write $line or die "write $config_file: $!\n";; + print $config_write $line or die "write $config_file: $!\n"; } } From 8dd73e62d21dc47d8b520ab23795885ff3f1d4bc Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 3 Oct 2017 15:58:26 +0300 Subject: [PATCH 25/30] Parse Signature Algorithm ext when renegotiating Signature algorithm extension was skipped when renegotiation was in progress, causing the signature algorithm not to be known when renegotiating, and failing the handshake. Fix removes the renegotiation step check before parsing the extension. --- ChangeLog | 3 +++ library/ssl_srv.c | 7 ++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index b3d4d519a..c4e3998d0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,9 @@ Bugfix * 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. = mbed TLS 2.6.0 branch released 2017-08-10 diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f137c3dce..37f415dd1 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1694,11 +1694,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 ); From 88f5808c135485308fdaeebeafb18a7490a3585b Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 5 Oct 2017 12:29:42 +0100 Subject: [PATCH 26/30] Renegotiation: Add tests for SigAlg ext parsing This commit adds regression tests for the bug when we didn't parse the Signature Algorithm extension when renegotiating. (By nature, this bug affected only the server) The tests check for the fallback hash (SHA1) in the server log to detect that the Signature Algorithm extension hasn't been parsed at least in one of the handshakes. A more direct way of testing is not possible with the current test framework, since the Signature Algorithm extension is parsed in the first handshake and any corresponding debug message is present in the logs. --- tests/ssl-opt.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7fcca685b..64f26a0cf 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1470,6 +1470,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" \ From 106637fc2d654ef2032f78746be0291affb66b84 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 21 Nov 2016 15:38:02 +0000 Subject: [PATCH 27/30] Correctly handle leap year in x509_date_is_valid() This patch ensures that invalid dates on leap years with 100 or 400 years intervals are handled correctly. --- ChangeLog | 3 +++ library/x509.c | 14 ++++++++++---- tests/suites/test_suite_x509parse.data | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index c4e3998d0..e7abd5ce6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,9 @@ Bugfix * 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.6.0 branch released 2017-08-10 diff --git a/library/x509.c b/library/x509.c index e94a8a329..371d6da1d 100644 --- a/library/x509.c +++ b/library/x509.c @@ -496,9 +496,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 ); @@ -508,17 +509,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/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index b8c902e23..a49137bb7 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1670,3 +1670,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 invalid leap year multiple of 4 and 100) +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 and not multiple of 100) +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) +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 From 47e7b56fb6c06ca3c30b44f9a0c324f1e43c5900 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 6 Oct 2017 17:05:24 +0100 Subject: [PATCH 28/30] Improve leap year test names in x509parse.data --- tests/suites/test_suite_x509parse.data | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index a49137bb7..d4cc11a08 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1670,15 +1670,15 @@ 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 invalid leap year multiple of 4 and 100) +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 and not multiple of 100) +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) +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 From 77f1b109ec205a6f5866169cc3523da57c21815e Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:24:44 +0100 Subject: [PATCH 29/30] Fix typo in asn1.h --- include/mbedtls/asn1.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 005939db984168406d8fed04874379677cdd630f Mon Sep 17 00:00:00 2001 From: RonEld Date: Tue, 17 Oct 2017 20:19:48 +0300 Subject: [PATCH 30/30] update README file (#1144) * update README file update VS 2010 as the minimal version of required Visual Studio * Rephrase the MS VS requirement Rephrase the VS version sentence --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5ffd2ae56..75639e930 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ There are currently four active build systems used within mbed TLS releases: - yotta - GNU Make - CMake -- Microsoft Visual Studio (Visual Studio 6 and Visual Studio 2010) +- Microsoft Visual Studio (Microsoft Visual Studio 2010 or later) The main systems used for development are CMake and GNU Make. Those systems are always complete and up-to-date. The others should reflect all changes present in the CMake and Make build system, although features may not be ported there automatically.