From 8f4eacaac6a7f2b6345ad11c0cba375c9d38eb96 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 27 May 2021 13:45:38 +0100 Subject: [PATCH 1/8] Removes MBEDTLS_ECDH_LEGACY_CONTEXT from config.h Commit removes the definition of MBEDTLS_ECDH_LEGACY_CONTEXT from config.h. Additionally removes the unset calls to MBEDTLS_ECDH_LEGACY_CONTEXT in all.sh. Signed-off-by: Thomas Daubney --- include/mbedtls/config.h | 28 ---------------------------- tests/scripts/all.sh | 4 ---- 2 files changed, 32 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 715c73ada..c6882dbb2 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -784,34 +784,6 @@ */ //#define MBEDTLS_ECP_RESTARTABLE -/** - * \def MBEDTLS_ECDH_LEGACY_CONTEXT - * - * Use a backward compatible ECDH context. - * - * Mbed TLS supports two formats for ECDH contexts (#mbedtls_ecdh_context - * defined in `ecdh.h`). For most applications, the choice of format makes - * no difference, since all library functions can work with either format, - * except that the new format is incompatible with MBEDTLS_ECP_RESTARTABLE. - - * The new format used when this option is disabled is smaller - * (56 bytes on a 32-bit platform). In future versions of the library, it - * will support alternative implementations of ECDH operations. - * The new format is incompatible with applications that access - * context fields directly and with restartable ECP operations. - * - * Define this macro if you enable MBEDTLS_ECP_RESTARTABLE or if you - * want to access ECDH context fields directly. Otherwise you should - * comment out this macro definition. - * - * This option has no effect if #MBEDTLS_ECDH_C is not enabled. - * - * \note This configuration option is experimental. Future versions of the - * library may modify the way the ECDH context layout is configured - * and may modify the layout of the new context type. - */ -#define MBEDTLS_ECDH_LEGACY_CONTEXT - /** * \def MBEDTLS_ECDSA_DETERMINISTIC * diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index ef2b6363b..aff186b1d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1078,7 +1078,6 @@ component_test_ecp_restartable_no_internal_rng () { component_test_new_ecdh_context () { msg "build: new ECDH context (ASan build)" # ~ 6 min - scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . make @@ -1095,7 +1094,6 @@ component_test_new_ecdh_context () { component_test_everest () { msg "build: Everest ECDH context (ASan build)" # ~ 6 min - scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED CC=clang cmake -D CMAKE_BUILD_TYPE:String=Asan . make @@ -1113,7 +1111,6 @@ component_test_everest () { component_test_everest_curve25519_only () { msg "build: Everest ECDH context, only Curve25519" # ~ 6 min - scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED scripts/config.py unset MBEDTLS_ECDSA_C scripts/config.py unset MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED @@ -2317,7 +2314,6 @@ support_test_m32_o1 () { component_test_m32_everest () { msg "build: i386, Everest ECDH context (ASan build)" # ~ 6 min - scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O2" LDFLAGS="-m32 $ASAN_CFLAGS" From c8901ed98d445de91a021e91b621d430adcfc06f Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 27 May 2021 15:31:15 +0100 Subject: [PATCH 2/8] Removes MBEDTLS_ECDH_LEGACY_CONTEXT from check_config.h Commit removes MBEDTLS_ECDH_LEGACY_CONTEXT checks from check_config.h. Signed-off-by: Thomas Daubney --- include/mbedtls/check_config.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 389ae2a71..298c91d60 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -130,16 +130,6 @@ #error "MBEDTLS_ECP_RESTARTABLE defined, but it cannot coexist with an alternative or PSA-based ECP implementation" #endif -#if defined(MBEDTLS_ECP_RESTARTABLE) && \ - ! defined(MBEDTLS_ECDH_LEGACY_CONTEXT) -#error "MBEDTLS_ECP_RESTARTABLE defined, but not MBEDTLS_ECDH_LEGACY_CONTEXT" -#endif - -#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) && \ - defined(MBEDTLS_ECDH_LEGACY_CONTEXT) -#error "MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED defined, but MBEDTLS_ECDH_LEGACY_CONTEXT not disabled" -#endif - #if defined(MBEDTLS_ECDSA_DETERMINISTIC) && !defined(MBEDTLS_HMAC_DRBG_C) #error "MBEDTLS_ECDSA_DETERMINISTIC defined, but not all prerequisites" #endif From 416c46ffe524edec3c8c45f1fc47445056ec8f33 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 27 May 2021 15:51:44 +0100 Subject: [PATCH 3/8] Defines MBEDTLS_ECDH_LEGACY_CONTEXT in ecdh.h Commit adds the conditional definition of MBEDTLS_ECDH_LEGACY_CONTEXT to ecdh.h. MBEDTLS_ECDH_LEGACY_CONTEXT is only defined if MBEDTLS_ECP_RESTARTABLE is definied. Signed-off-by: Thomas Daubney --- include/mbedtls/ecdh.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h index 05855cdf1..82d3de008 100644 --- a/include/mbedtls/ecdh.h +++ b/include/mbedtls/ecdh.h @@ -40,6 +40,39 @@ #include "mbedtls/ecp.h" +/** + * \def MBEDTLS_ECDH_LEGACY_CONTEXT + * + * Use a backward compatible ECDH context. + * + * Mbed TLS supports two formats for ECDH contexts (#mbedtls_ecdh_context + * defined in `ecdh.h`). For most applications, the choice of format makes + * no difference, since all library functions can work with either format, + * except that the new format is incompatible with MBEDTLS_ECP_RESTARTABLE. + + * The new format used when this option is disabled is smaller + * (56 bytes on a 32-bit platform). In future versions of the library, it + * will support alternative implementations of ECDH operations. + * The new format is incompatible with applications that access + * context fields directly and with restartable ECP operations. + * + * Define this macro if you enable MBEDTLS_ECP_RESTARTABLE or if you + * want to access ECDH context fields directly. Otherwise you should + * comment out this macro definition. + * + * This option has no effect if #MBEDTLS_ECDH_C is not enabled. + * + * \note This configuration option is experimental. Future versions of the + * library may modify the way the ECDH context layout is configured + * and may modify the layout of the new context type. + */ + +#if defined(MBEDTLS_ECP_RESTARTABLE) +#define MBEDTLS_ECDH_LEGACY_CONTEXT +#else +#undef MBEDTLS_ECDH_LEGACY_CONTEXT +#endif + #if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) #undef MBEDTLS_ECDH_LEGACY_CONTEXT #include "everest/everest.h" From 42aaf7a7183914f64f1446477c821eb611084b7a Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 1 Jun 2021 17:48:40 +0100 Subject: [PATCH 4/8] Removes component_test_new_ecdh_context in all.sh Commit removes the component_test_new_new_ecdh_context in all.sh. Signed-off-by: Thomas Daubney --- tests/scripts/all.sh | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index aff186b1d..c187af6ba 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1076,22 +1076,6 @@ component_test_ecp_restartable_no_internal_rng () { # no SSL tests as they all depend on having a DRBG } -component_test_new_ecdh_context () { - msg "build: new ECDH context (ASan build)" # ~ 6 min - CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . - make - - msg "test: new ECDH context - main suites (inc. selftests) (ASan build)" # ~ 50s - make test - - msg "test: new ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s - if_build_succeeded tests/ssl-opt.sh -f ECDH - - msg "test: new ECDH context - compat.sh with some ECDH ciphersuites (ASan build)" # ~ 3 min - # Exclude some symmetric ciphers that are redundant here to gain time. - if_build_succeeded tests/compat.sh -f ECDH -V NO -e 'ARCFOUR\|ARIA\|CAMELLIA\|CHACHA\|DES\|RC4' -} - component_test_everest () { msg "build: Everest ECDH context (ASan build)" # ~ 6 min scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED From 3726db4750e2990762b29ac0195a49fed85dcd0e Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 1 Jun 2021 19:03:08 +0100 Subject: [PATCH 5/8] Removes obsolete test Removal of obsolete test in test_suite_ecdh.function and corresponding .data file. Signed-off-by: Thomas Daubney --- tests/suites/test_suite_ecdh.data | 4 --- tests/suites/test_suite_ecdh.function | 41 --------------------------- 2 files changed, 45 deletions(-) diff --git a/tests/suites/test_suite_ecdh.data b/tests/suites/test_suite_ecdh.data index fb4a232fc..d9e81a6b0 100644 --- a/tests/suites/test_suite_ecdh.data +++ b/tests/suites/test_suite_ecdh.data @@ -76,10 +76,6 @@ ECDH restartable rfc 5903 p256 restart disabled max_ops=250 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED ecdh_restart:MBEDTLS_ECP_DP_SECP256R1:"C88F01F510D9AC3F70A292DAA2316DE544E9AAB8AFE84049C62A9C57862D1433":"C6EF9C5D78AE012A011164ACB397CE2088685D8F06BF9BE0B283AB46476BEE53":"D6840F6B42F6EDAFD13116E0E12565202FEF8E9ECE7DCE03812464D04B9442DE":0:250:0:0 -ECDH exchange legacy context -depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED -ecdh_exchange_legacy:MBEDTLS_ECP_DP_SECP192R1 - ECDH calc_secret: ours first, SECP256R1 (RFC 5903) depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED ecdh_exchange_calc_secret:MBEDTLS_ECP_DP_SECP256R1:"c6ef9c5d78ae012a011164acb397ce2088685d8f06bf9be0b283ab46476bee53":"04dad0b65394221cf9b051e1feca5787d098dfe637fc90b9ef945d0c37725811805271a0461cdb8252d61f1c456fa3e59ab1f45b33accf5f58389e0577b8990bb3":0:"d6840f6b42f6edafd13116e0e12565202fef8e9ece7dce03812464d04b9442de" diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function index 3ab96fa11..cd8eca855 100644 --- a/tests/suites/test_suite_ecdh.function +++ b/tests/suites/test_suite_ecdh.function @@ -465,47 +465,6 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_ECDH_LEGACY_CONTEXT */ -void ecdh_exchange_legacy( int id ) -{ - mbedtls_ecdh_context srv, cli; - unsigned char buf[1000]; - const unsigned char *vbuf; - size_t len; - - mbedtls_test_rnd_pseudo_info rnd_info; - - mbedtls_ecdh_init( &srv ); - mbedtls_ecdh_init( &cli ); - memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) ); - - TEST_ASSERT( mbedtls_ecp_group_load( &srv.grp, id ) == 0 ); - - memset( buf, 0x00, sizeof( buf ) ); vbuf = buf; - TEST_ASSERT( mbedtls_ecdh_make_params( &srv, &len, buf, 1000, - &mbedtls_test_rnd_pseudo_rand, - &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_ecdh_read_params( &cli, &vbuf, buf + len ) == 0 ); - - memset( buf, 0x00, sizeof( buf ) ); - TEST_ASSERT( mbedtls_ecdh_make_public( &cli, &len, buf, 1000, - &mbedtls_test_rnd_pseudo_rand, - &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_ecdh_read_public( &srv, buf, len ) == 0 ); - - TEST_ASSERT( mbedtls_ecdh_calc_secret( &srv, &len, buf, 1000, - &mbedtls_test_rnd_pseudo_rand, - &rnd_info ) == 0 ); - TEST_ASSERT( mbedtls_ecdh_calc_secret( &cli, &len, buf, 1000, NULL, - NULL ) == 0 ); - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &srv.z, &cli.z ) == 0 ); - -exit: - mbedtls_ecdh_free( &srv ); - mbedtls_ecdh_free( &cli ); -} -/* END_CASE */ - /* BEGIN_CASE */ void ecdh_exchange_calc_secret( int grp_id, data_t *our_private_key, From adb93d732f4f77b092c4f54f1fd2b98eb7fd7276 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 2 Jun 2021 13:45:57 +0100 Subject: [PATCH 6/8] Adds ChangeLog entry Commit adds required ChangeLog entry. Signed-off-by: Thomas Daubney --- ChangeLog.d/rm-ecdh-legacy-context-option.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/rm-ecdh-legacy-context-option.txt diff --git a/ChangeLog.d/rm-ecdh-legacy-context-option.txt b/ChangeLog.d/rm-ecdh-legacy-context-option.txt new file mode 100644 index 000000000..d5a527b94 --- /dev/null +++ b/ChangeLog.d/rm-ecdh-legacy-context-option.txt @@ -0,0 +1,3 @@ +Removals + * Remove MBEDTLS_ECDH_LEGACY_CONTEXT config option since this was purely for + backward compatibility which is no longer supported. Addresses #4404. From 4e9fb3985e214e66b0319baf79416895fa33fcd4 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 3 Jun 2021 11:51:08 +0100 Subject: [PATCH 7/8] Corrects documentation in ecdh.h Commit corrects the documentation for MBEDTLS_ECDH_LEGACY_CONTEXT in ecdh.h. Signed-off-by: Thomas Daubney --- include/mbedtls/ecdh.h | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h index 82d3de008..e5edb46e6 100644 --- a/include/mbedtls/ecdh.h +++ b/include/mbedtls/ecdh.h @@ -40,11 +40,7 @@ #include "mbedtls/ecp.h" -/** - * \def MBEDTLS_ECDH_LEGACY_CONTEXT - * - * Use a backward compatible ECDH context. - * +/* * Mbed TLS supports two formats for ECDH contexts (#mbedtls_ecdh_context * defined in `ecdh.h`). For most applications, the choice of format makes * no difference, since all library functions can work with either format, @@ -56,15 +52,8 @@ * The new format is incompatible with applications that access * context fields directly and with restartable ECP operations. * - * Define this macro if you enable MBEDTLS_ECP_RESTARTABLE or if you - * want to access ECDH context fields directly. Otherwise you should - * comment out this macro definition. - * * This option has no effect if #MBEDTLS_ECDH_C is not enabled. * - * \note This configuration option is experimental. Future versions of the - * library may modify the way the ECDH context layout is configured - * and may modify the layout of the new context type. */ #if defined(MBEDTLS_ECP_RESTARTABLE) From 537e64305d167449f999cbd434eb5666974731e3 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 3 Jun 2021 15:46:33 +0100 Subject: [PATCH 8/8] Corrects documentation issues Commit corrects incorrect docs in ecdh.h and config.h. Signed-off-by: Thomas Daubney --- include/mbedtls/config.h | 3 +-- include/mbedtls/ecdh.h | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index c6882dbb2..79165876b 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -779,8 +779,7 @@ * * \note This option only works with the default software implementation of * elliptic curve functionality. It is incompatible with - * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT, MBEDTLS_ECDSA_XXX_ALT - * and MBEDTLS_ECDH_LEGACY_CONTEXT. + * MBEDTLS_ECP_ALT, MBEDTLS_ECDH_XXX_ALT, MBEDTLS_ECDSA_XXX_ALT. */ //#define MBEDTLS_ECP_RESTARTABLE diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h index e5edb46e6..765ac5e62 100644 --- a/include/mbedtls/ecdh.h +++ b/include/mbedtls/ecdh.h @@ -51,9 +51,6 @@ * will support alternative implementations of ECDH operations. * The new format is incompatible with applications that access * context fields directly and with restartable ECP operations. - * - * This option has no effect if #MBEDTLS_ECDH_C is not enabled. - * */ #if defined(MBEDTLS_ECP_RESTARTABLE)