From 755bb6af5f6fdfcabaddd018d149c2819125d7b3 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 2e9a0fd79..dc4f0bbad 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -2242,7 +2242,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 ) @@ -2252,13 +2253,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 e537f9d2e..ad11292a0 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -766,6 +766,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" ) ); @@ -917,6 +921,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] ); @@ -1010,11 +1019,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 0ccab588e..91079f17a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2564,8 +2564,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 3f38cf7c7b7695d83d9049c630bd63a44d5f45e2 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 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 027a97174..c28f806a7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * 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.11.0 branch released 2018-06-18 Features From 84e62f88a2c170e0e1f58d42d3d3bbc9f68d0741 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 c28f806a7..380b289c9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,8 +4,8 @@ mbed TLS ChangeLog (Sorted per branch, date) Bugfix * 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.11.0 branch released 2018-06-18 From 58093c8bec6a410e6f7bbdccf1abd1fa01574b93 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 9faeb6703..7fade04ec 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4551,6 +4551,40 @@ run_test "SSL async private: renegotiation: server-initiated; decrypt" \ -s "Async decrypt callback: using key slot " \ -s "Async resume (slot [0-9]): decrypt done, status=0" +# 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 643df7c8a1003c7a190fab411ba8ac43f5a81210 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 7fade04ec..2366117e3 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4553,22 +4553,34 @@ run_test "SSL async private: renegotiation: server-initiated; decrypt" \ # 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" \ @@ -4578,6 +4590,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" \