Merge pull request #4611 from gilles-peskine-arm/random-range-uniformity-3.0
Fix non-uniform random generation in a range
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 0000000..d5a527b
--- /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.
diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h
index 8501fb6..e066da7 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
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index fb57818..9cce3cd 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -759,40 +759,11 @@
*
* \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
/**
- * \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
*
* Enable deterministic ECDSA (RFC 6979).
diff --git a/include/mbedtls/ecdh.h b/include/mbedtls/ecdh.h
index 05855cd..765ac5e 100644
--- a/include/mbedtls/ecdh.h
+++ b/include/mbedtls/ecdh.h
@@ -40,6 +40,25 @@
#include "mbedtls/ecp.h"
+/*
+ * 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.
+ */
+
+#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"
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 8e163a9..ab8500b 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -1085,26 +1085,8 @@
# 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
- scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT
- 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 unset MBEDTLS_ECDH_LEGACY_CONTEXT
scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED
CC=clang cmake -D CMAKE_BUILD_TYPE:String=Asan .
make
@@ -1122,7 +1104,6 @@
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
@@ -2326,7 +2307,6 @@
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"
diff --git a/tests/suites/test_suite_ecdh.data b/tests/suites/test_suite_ecdh.data
index fb4a232..d9e81a6 100644
--- a/tests/suites/test_suite_ecdh.data
+++ b/tests/suites/test_suite_ecdh.data
@@ -76,10 +76,6 @@
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 da0531b..d52849a 100644
--- a/tests/suites/test_suite_ecdh.function
+++ b/tests/suites/test_suite_ecdh.function
@@ -473,47 +473,6 @@
}
/* 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,
