From c32b3b73c4aacb3a7eef66df9d10df644c2cce4d Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 14 Feb 2018 19:30:48 +0200 Subject: [PATCH 1/5] Add ecc extensions only if ecc ciphersuite is used Fix compliancy to RFC4492. ECC extensions should be included only if ec ciphersuites are used. Interoperability issue with bouncy castle. #1157 --- library/ssl_ciphersuites.c | 6 ++++-- library/ssl_cli.c | 20 ++++++++++++++++---- library/ssl_srv.c | 8 ++++++-- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 95e6163cc..800b5f84d 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -1837,7 +1837,8 @@ mbedtls_pk_type_t mbedtls_ssl_get_ciphersuite_sig_alg( const mbedtls_ssl_ciphers #endif /* MBEDTLS_PK_C */ -#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) int mbedtls_ssl_ciphersuite_uses_ec( const mbedtls_ssl_ciphersuite_t *info ) { switch( info->key_exchange ) @@ -1847,13 +1848,14 @@ int mbedtls_ssl_ciphersuite_uses_ec( const mbedtls_ssl_ciphersuite_t *info ) case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + case MBEDTLS_KEY_EXCHANGE_ECJPAKE: return( 1 ); default: return( 0 ); } } -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED*/ #if defined(MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED) int mbedtls_ssl_ciphersuite_uses_psk( const mbedtls_ssl_ciphersuite_t *info ) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 15599c9df..a430a36a0 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -769,6 +769,10 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) unsigned char offer_compress; const int *ciphersuites; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + int uses_ec = 0; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write client hello" ) ); @@ -920,6 +924,11 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, add ciphersuite: %04x", ciphersuites[i] ) ); +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + uses_ec |= mbedtls_ssl_ciphersuite_uses_ec( ciphersuite_info ); +#endif + n++; *p++ = (unsigned char)( ciphersuites[i] >> 8 ); *p++ = (unsigned char)( ciphersuites[i] ); @@ -1013,11 +1022,14 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - ssl_write_supported_elliptic_curves_ext( ssl, p + 2 + ext_len, &olen ); - ext_len += olen; + if( uses_ec ) + { + ssl_write_supported_elliptic_curves_ext( ssl, p + 2 + ext_len, &olen ); + ext_len += olen; - ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); - ext_len += olen; + ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); + ext_len += olen; + } #endif #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 14b5f2990..45f625127 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2570,8 +2570,12 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); - ext_len += olen; + if ( mbedtls_ssl_ciphersuite_uses_ec( + mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ) ) ) + { + ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); + ext_len += olen; + } #endif #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) From 335cf423f94eabd1ed26cb736d8eb0e2ecfca51d Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 21 Jun 2018 16:40:24 +0300 Subject: [PATCH 2/5] Add entry in ChangeLog Add an entry in the ChangeLog, describing the fix. --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 1e2f9b871..ea5740c56 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,9 @@ Bugfix Philippe Antoine. * Clarify documentation for mbedtls_ssl_write() to include 0 as a valid return value. Found by @davidwu2000. #839 + * Add ecc extensions only if an ecc based ciphersuite is used. + Affects interoperability with BouncyCastle and other peers. + Raised by milenamil in #1157. = mbed TLS 2.7.4 branch released 2018-06-18 From 6a5d6e22951479176e7423933977cac5916196e8 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 28 Jun 2018 11:09:09 +0300 Subject: [PATCH 3/5] Update ChangeLog Update ChangeLog with a less ambigous description. --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index ea5740c56..b80af6ef0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,8 +10,8 @@ Bugfix * Clarify documentation for mbedtls_ssl_write() to include 0 as a valid return value. Found by @davidwu2000. #839 * Add ecc extensions only if an ecc based ciphersuite is used. - Affects interoperability with BouncyCastle and other peers. - Raised by milenamil in #1157. + This improves compliance to RFC 4492, and as a result, solves + interoperability issues with BouncyCastle. Raised by milenamil in #1157. = mbed TLS 2.7.4 branch released 2018-06-18 From c7f1523a9e44f6deee160ff4025b408bea78d036 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 28 Jun 2018 13:22:05 +0300 Subject: [PATCH 4/5] Add ECC extensions test in ssl-opts.sh Add test to verify if an ecc based extension exists or not if an ecc based ciphersuite is used or not. --- tests/ssl-opt.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2f8ea4f0b..68e94b643 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4085,6 +4085,40 @@ run_test "Large packet TLS 1.2 AEAD shorter tag" \ -c "16384 bytes written in 1 fragments" \ -s "Read from client: 16384 bytes read" +# Tests for ECC extensions (rfc 4492) + +run_test "Force a non ECC ciphersuite in the client side" \ + "$P_SRV debug_level=3" \ + "$P_CLI debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -C "client hello, adding supported_elliptic_curves extension" \ + -C "client hello, adding supported_point_formats extension" \ + -S "found supported elliptic curves extension" \ + -S "found supported point formats extension" + +run_test "Force a non ECC ciphersuite in the server side" \ + "$P_SRV debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + "$P_CLI debug_level=3" \ + 0 \ + -C "found supported_point_formats extension" \ + -S "server hello, supported_point_formats extension" + +run_test "Force an ECC ciphersuite in the client side" \ + "$P_SRV debug_level=3" \ + "$P_CLI debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ + 0 \ + -c "client hello, adding supported_elliptic_curves extension" \ + -c "client hello, adding supported_point_formats extension" \ + -s "found supported elliptic curves extension" \ + -s "found supported point formats extension" + +run_test "Force an ECC ciphersuite in the server side" \ + "$P_SRV debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ + "$P_CLI debug_level=3" \ + 0 \ + -c "found supported_point_formats extension" \ + -s "server hello, supported_point_formats extension" + # Tests for DTLS HelloVerifyRequest run_test "DTLS cookie: enabled" \ From 94226d8e61817fd0f716cc93c0e0dcbfd81ed237 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 28 Jun 2018 16:17:00 +0300 Subject: [PATCH 5/5] Update ssl-opt.sh test to run condition 1. Update the test script to un the ECC tests only if the relevant configurations are defined in `config.h` file 2. Change the HASH of the ciphersuite from SHA1 based to SHA256 for better example --- tests/ssl-opt.sh | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 68e94b643..43e30341a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4087,22 +4087,34 @@ run_test "Large packet TLS 1.2 AEAD shorter tag" \ # Tests for ECC extensions (rfc 4492) +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ - "$P_CLI debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + "$P_CLI debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA256" \ 0 \ -C "client hello, adding supported_elliptic_curves extension" \ -C "client hello, adding supported_point_formats extension" \ -S "found supported elliptic curves extension" \ -S "found supported point formats extension" +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_RSA_ENABLED run_test "Force a non ECC ciphersuite in the server side" \ - "$P_SRV debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + "$P_SRV debug_level=3 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA256" \ "$P_CLI debug_level=3" \ 0 \ -C "found supported_point_formats extension" \ -S "server hello, supported_point_formats extension" +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED run_test "Force an ECC ciphersuite in the client side" \ "$P_SRV debug_level=3" \ "$P_CLI debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ @@ -4112,6 +4124,10 @@ run_test "Force an ECC ciphersuite in the client side" \ -s "found supported elliptic curves extension" \ -s "found supported point formats extension" +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED run_test "Force an ECC ciphersuite in the server side" \ "$P_SRV debug_level=3 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256" \ "$P_CLI debug_level=3" \