From ffb92da622122c8d1f6dd9cd626ed3b098ccf181 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 00:03:26 +0200 Subject: [PATCH 01/20] Upgrade the default X.509 profile to the former "next" profile Upgrade the default X.509 certificate verification profile mbedtls_x509_crt_profile_default to the former value of mbedtls_x509_crt_profile_next, which is hashes and curves with at least 255 bits (Curve25519 included), and RSA 2048 and above. Document more precisely what goes into the default profile. Keep the "next" profile unchanged for now. Signed-off-by: Gilles Peskine --- include/mbedtls/x509_crt.h | 12 +++++++++++- library/x509_crt.c | 25 +++++++------------------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index d383168d2..9c7016d8e 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -332,12 +332,22 @@ typedef void mbedtls_x509_crt_restart_ctx; /** * Default security profile. Should provide a good balance between security * and compatibility with current deployments. + * + * This profile permits: + * - SHA2 hashes with at least 256 bits: SHA-256, SHA-384, SHA-512. + * - Elliptic curves with 255 bits and above. + * - RSA with 2048 bits and above. + * + * New minor versions of Mbed TLS may extend this profile, for example if + * new curves are added to the library. New minor versions of Mbed TLS will + * not reduce this profile unless serious security concerns require it. */ extern const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default; /** * Expected next default profile. Recommended for new deployments. - * Currently targets a 128-bit security level, except for RSA-2048. + * Currently targets a 128-bit security level, except for allowing RSA-2048. + * This profile may change at any time. */ extern const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next; diff --git a/library/x509_crt.c b/library/x509_crt.c index d4e0ffd40..59692476b 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -95,25 +95,9 @@ typedef struct { */ #define X509_MAX_VERIFY_CHAIN_SIZE ( MBEDTLS_X509_MAX_INTERMEDIATE_CA + 2 ) -/* - * Default profile - */ +/* Default profile. Do not remove items unless there are serious security + * concerns. */ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default = -{ - /* Only SHA-2 hashes */ - MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA224 ) | - MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA256 ) | - MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA384 ) | - MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA512 ), - 0xFFFFFFF, /* Any PK alg */ - 0xFFFFFFF, /* Any curve */ - 2048, -}; - -/* - * Next-default profile - */ -const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next = { /* Hashes from SHA-256 and above */ MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA256 ) | @@ -135,6 +119,11 @@ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next = 2048, }; +/* Next-generation profile. Currently identical to the default, but may + * be tightened at any time. */ +const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next = + mbedtls_x509_crt_profile_default; + /* * NSA Suite B Profile */ From ae270bf3860aad2c68430d6381be586a72c4f1ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 00:05:29 +0200 Subject: [PATCH 02/20] Upgrade the default TLS hash and curve selection, matching X.509 Upgrade the default list of hashes and curves allowed for TLS. The list is now aligned with X.509 certificate verification: hashes and curves with at least 255 bits (Curve25519 included), and RSA 2048 and above. Remove MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE which would no longer do anything. Document more precisely what is allowed by default. Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 17 ------------- include/mbedtls/ssl.h | 14 +++++++++-- library/ecp.c | 4 ++-- library/ssl_tls.c | 52 ++++++++++++++++++++++++++++++++++------ library/x509_crt.c | 6 +++-- 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 16f8f8b35..fc10f9d70 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3326,23 +3326,6 @@ //#define MBEDTLS_X509_MAX_INTERMEDIATE_CA 8 /**< Maximum number of intermediate CAs in a verification chain. */ //#define MBEDTLS_X509_MAX_FILE_PATH_LEN 512 /**< Maximum length of a path/filename string in bytes including the null terminator character ('\0'). */ -/** - * Allow SHA-1 in the default TLS configuration for TLS 1.2 handshake - * signature and ciphersuite selection. Without this build-time option, SHA-1 - * support must be activated explicitly through mbedtls_ssl_conf_sig_hashes. - * The use of SHA-1 in TLS <= 1.1 and in HMAC-SHA-1 is always allowed by - * default. At the time of writing, there is no practical attack on the use - * of SHA-1 in handshake signatures, hence this option is turned on by default - * to preserve compatibility with existing peers, but the general - * warning applies nonetheless: - * - * \warning SHA-1 is considered a weak message digest and its use constitutes - * a security risk. If possible, we recommend avoiding dependencies - * on it, and considering stronger message digests instead. - * - */ -#define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE - /** * Uncomment the macro to let mbed TLS use your alternate implementation of * mbedtls_platform_zeroize(). This replaces the default implementation in diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b2f5c67a2..df3974aea 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2893,7 +2893,6 @@ void mbedtls_ssl_conf_dhm_min_bitlen( mbedtls_ssl_config *conf, #if defined(MBEDTLS_ECP_C) /** * \brief Set the allowed curves in order of preference. - * (Default: all defined curves.) * * On server: this only affects selection of the ECDHE curve; * the curves used for ECDH and ECDSA are determined by the @@ -2914,6 +2913,12 @@ void mbedtls_ssl_conf_dhm_min_bitlen( mbedtls_ssl_config *conf, * \note This list should be ordered by decreasing preference * (preferred curve first). * + * \note The default list is the same set of curves that + * #mbedtls_x509_crt_profile_default allows, plus + * ECDHE-only curves selected according to the same criteria. + * Larger (generally more secure but slower) curves are + * preferred over smaller curves. + * * \param conf SSL configuration * \param curves Ordered list of allowed curves, * terminated by MBEDTLS_ECP_DP_NONE. @@ -2925,7 +2930,6 @@ void mbedtls_ssl_conf_curves( mbedtls_ssl_config *conf, #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /** * \brief Set the allowed hashes for signatures during the handshake. - * (Default: all available hashes except MD5.) * * \note This only affects which hashes are offered and can be used * for signatures during the handshake. Hashes for message @@ -2937,6 +2941,12 @@ void mbedtls_ssl_conf_curves( mbedtls_ssl_config *conf, * \note This list should be ordered by decreasing preference * (preferred hash first). * + * \note By default, all supported hashes whose length is at least + * 256 bits are allowed. This is the same set as the default + * for certificate verification + * (#mbedtls_x509_crt_profile_default). Larger hashes are + * preferred. + * * \param conf SSL configuration * \param hashes Ordered list of allowed signature hashes, * terminated by \c MBEDTLS_MD_NONE. diff --git a/library/ecp.c b/library/ecp.c index 044bbe1d1..6bb955cc0 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -509,9 +509,9 @@ int mbedtls_ecp_check_budget( const mbedtls_ecp_group *grp, * - readable name * * Curves are listed in order: largest curves first, and for a given size, - * fastest curves first. This provides the default order for the SSL module. + * fastest curves first. * - * Reminder: update profiles in x509_crt.c when adding a new curves! + * Reminder: update profiles in x509_crt.c and ssl_tls.c when adding a new curve! */ static const mbedtls_ecp_curve_info ecp_supported_curves[] = { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5c1bc3207..be389f03b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6098,6 +6098,11 @@ void mbedtls_ssl_config_init( mbedtls_ssl_config *conf ) } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +/* The selection should be the same as mbedtls_x509_crt_profile_default in + * x509_crt.c. Here, the order matters: larger hashes first, for consistency + * with curves. + * See the documentation of mbedtls_ssl_conf_curves() for what we promise + * about this list. */ static int ssl_preset_default_hashes[] = { #if defined(MBEDTLS_SHA512_C) MBEDTLS_MD_SHA512, @@ -6107,17 +6112,50 @@ static int ssl_preset_default_hashes[] = { #endif #if defined(MBEDTLS_SHA256_C) MBEDTLS_MD_SHA256, -#endif -#if defined(MBEDTLS_SHA224_C) - MBEDTLS_MD_SHA224, -#endif -#if defined(MBEDTLS_SHA1_C) && defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE) - MBEDTLS_MD_SHA1, #endif MBEDTLS_MD_NONE }; #endif +#if defined(MBEDTLS_ECP_C) +/* The selection should be the same as mbedtls_x509_crt_profile_default in + * x509_crt.c, plus Montgomery curves for ECDHE. Here, the order matters: + * larger curves first, like ecp_supported_curves in ecp.c. + * See the documentation of mbedtls_ssl_conf_curves() for what we promise + * about this list. */ +static mbedtls_ecp_group_id ssl_preset_default_curves[] = { +#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) + MBEDTLS_ECP_DP_SECP521R1, +#endif +#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) + MBEDTLS_ECP_DP_BP512R1, +#endif +#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) + MBEDTLS_ECP_DP_CURVE448, +#endif +#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) + MBEDTLS_ECP_DP_SECP384R1, +#endif +#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) + MBEDTLS_ECP_DP_BP384R1, +#endif +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) + // Positioned in the list as a fast 256-bit curve, not as a 255-bit curve + MBEDTLS_ECP_DP_CURVE25519, +#endif +#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) + MBEDTLS_ECP_DP_SECP256R1, +#endif +#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) + MBEDTLS_ECP_DP_SECP256K1, +#endif +#if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) + MBEDTLS_ECP_DP_BP256R1, +#endif + MBEDTLS_ECP_DP_NONE +}; +#endif + static int ssl_preset_suiteb_ciphersuites[] = { MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, @@ -6281,7 +6319,7 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #endif #if defined(MBEDTLS_ECP_C) - conf->curve_list = mbedtls_ecp_grp_id_list(); + conf->curve_list = ssl_preset_default_curves; #endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_CLI_C) diff --git a/library/x509_crt.c b/library/x509_crt.c index 59692476b..abd70e1e4 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -99,13 +99,15 @@ typedef struct { * concerns. */ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default = { - /* Hashes from SHA-256 and above */ + /* Hashes from SHA-256 and above. Note that this selection + * should be aligned with ssl_preset_default_hashes in ssl_tls.c. */ MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA256 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA384 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA512 ), 0xFFFFFFF, /* Any PK alg */ #if defined(MBEDTLS_ECP_C) - /* Curves at or above 128-bit security level */ + /* Curves at or above 128-bit security level. Note that this selection + * should be aligned with ssl_preset_default_curves in ssl_tls.c. */ MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP256R1 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP384R1 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP521R1 ) | From 3758fd6b79914d060124b5492da85a5693672bb5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 00:07:17 +0200 Subject: [PATCH 03/20] Changelog entry and migration guide for hash and curve profile upgrades Signed-off-by: Gilles Peskine --- ChangeLog.d/default-curves.txt | 8 ++++++++ docs/3.0-migration-guide.d/default-curves.md | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 ChangeLog.d/default-curves.txt create mode 100644 docs/3.0-migration-guide.d/default-curves.md diff --git a/ChangeLog.d/default-curves.txt b/ChangeLog.d/default-curves.txt new file mode 100644 index 000000000..1a805623d --- /dev/null +++ b/ChangeLog.d/default-curves.txt @@ -0,0 +1,8 @@ +Default behavior changes + * Some default policies for X.509 certificate verification and TLS have + changed: curves and hashes weaker than 255 bits are no longer accepted + by default. + +Removals + * Remove the compile-time option + MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE. diff --git a/docs/3.0-migration-guide.d/default-curves.md b/docs/3.0-migration-guide.d/default-curves.md new file mode 100644 index 000000000..5db517ebd --- /dev/null +++ b/docs/3.0-migration-guide.d/default-curves.md @@ -0,0 +1,16 @@ +Weak curves are now disabled by default for X.509 and TLS +--------------------------------------------------------- + +The default X.509 verification profile (`mbedtls_x509_crt_profile_default`) and the default curve and hash selection have changed. X.509 and TLS now allow the same algorithms by default (except that the X.509 profile only lists curves that support signature verification). + +Hashes and curves weaker than 255 bits are no longer accepted by default. The following algorithms have been removed: SHA-1 (formerly only accepted for key exchanges but not for certificate signatures), SHA-224, secp192r1, secp224r1, secp192k1, secp224k1 (weaker hashes were already not accepted). + +The compile-time option `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` is no longer available. + +If you still need to accept certificates signed with algorithms that have been removed from the default profile, call `mbedtls_x509_crt_verify_with_profile` instead of `mbedtls_x509_crt_verify` and pass a profile that allows the curves you want. For example, to allow SHA-224: +``` +mbedtls_x509_crt_profile my_profile = mbedtls_x509_crt_profile_default; +my_profile.allowed_mds |= MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA224 ); +``` + +If you still need to allow hashes and curves in TLS that have been removed from the default configuration, call `mbedtls_ssl_conf_sig_hashes()` and `mbedtls_ssl_conf_curves()` with the desired lists. From 2c69fa245c42acf7e7ab226c36a804f86275a723 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 00:33:33 +0200 Subject: [PATCH 04/20] Initializer element was not constant Signed-off-by: Gilles Peskine --- library/x509_crt.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index abd70e1e4..650044583 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -124,7 +124,26 @@ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default = /* Next-generation profile. Currently identical to the default, but may * be tightened at any time. */ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next = - mbedtls_x509_crt_profile_default; +{ + /* Hashes from SHA-256 and above. */ + MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA256 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA384 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA512 ), + 0xFFFFFFF, /* Any PK alg */ +#if defined(MBEDTLS_ECP_C) + /* Curves at or above 128-bit security level. */ + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP256R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP384R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP521R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP256R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP384R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP512R1 ) | + MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP256K1 ), +#else + 0, +#endif + 2048, +}; /* * NSA Suite B Profile From 12b5b389817ae77ded92c3f799ef09432cc834de Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 10:00:42 +0200 Subject: [PATCH 05/20] Fix "PSA - ECDH with [non-default curve]" Signed-off-by: Gilles Peskine --- 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 d1221112a..5ccb467b1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1008,7 +1008,7 @@ run_test_psa() { run_test_psa_force_curve() { requires_config_enabled MBEDTLS_USE_PSA_CRYPTO run_test "PSA - ECDH with $1" \ - "$P_SRV debug_level=4 force_version=tls1_2" \ + "$P_SRV debug_level=4 force_version=tls1_2 curves=$1" \ "$P_CLI debug_level=4 force_version=tls1_2 force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-128-GCM-SHA256 curves=$1" \ 0 \ -c "Successfully setup PSA-based decryption cipher context" \ From 5752e599b32dc0322fa3352f6c86281c01dc9ee1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 13:27:03 +0200 Subject: [PATCH 06/20] Reduce the default ECP window size MBEDTLS_ECP_WINDOW_SIZE is a compromise between memory usage (growing based on the value) and performance (faster with larger values). There are disminishing returns as the value grows larger. Based on Manuel's benchmarks recorded in https://github.com/ARMmbed/mbedtls/issues/4127, 4 is a good compromise point, with larger values bringing little advantage. So reduce the default from 6 to 4. Document the default value as in optimized for performance mostly, but don't document the specific value, so we may change it later or make it platform-dependent. Signed-off-by: Gilles Peskine --- ChangeLog.d/ecp-window-size.txt | 3 +++ include/mbedtls/config.h | 2 +- include/mbedtls/ecp.h | 5 +++-- 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 ChangeLog.d/ecp-window-size.txt diff --git a/ChangeLog.d/ecp-window-size.txt b/ChangeLog.d/ecp-window-size.txt new file mode 100644 index 000000000..909d4e8a4 --- /dev/null +++ b/ChangeLog.d/ecp-window-size.txt @@ -0,0 +1,3 @@ +Changes + * Reduce the default value of MBEDTLS_ECP_WINDOW_SIZE. This reduces RAM usage + during ECC operations at a negligible performance cost. diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index fc10f9d70..0c7e4ec06 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3145,7 +3145,7 @@ //#define MBEDTLS_HMAC_DRBG_MAX_SEED_INPUT 384 /**< Maximum size of (re)seed buffer */ /* ECP options */ -//#define MBEDTLS_ECP_WINDOW_SIZE 6 /**< Maximum window size used */ +//#define MBEDTLS_ECP_WINDOW_SIZE 4 /**< Maximum window size used */ //#define MBEDTLS_ECP_FIXED_POINT_OPTIM 1 /**< Enable fixed-point speed-up */ /* Entropy options */ diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 49e85d941..1c271c913 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -255,7 +255,8 @@ mbedtls_ecp_group; #if !defined(MBEDTLS_ECP_WINDOW_SIZE) /* * Maximum "window" size used for point multiplication. - * Default: 6. + * Default: a point where higher memory usage yields disminishing performance + * returns. * Minimum value: 2. Maximum value: 7. * * Result is an array of at most ( 1 << ( MBEDTLS_ECP_WINDOW_SIZE - 1 ) ) @@ -272,7 +273,7 @@ mbedtls_ecp_group; * 224 475 475 453 398 342 * 192 640 640 633 587 476 */ -#define MBEDTLS_ECP_WINDOW_SIZE 6 /**< The maximum window size used. */ +#define MBEDTLS_ECP_WINDOW_SIZE 4 /**< The maximum window size used. */ #endif /* MBEDTLS_ECP_WINDOW_SIZE */ #if !defined(MBEDTLS_ECP_FIXED_POINT_OPTIM) From 377c91e1b73e5a55f5e976f357bd9ac000b7aba0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 14:37:57 +0200 Subject: [PATCH 07/20] Remove meaningless clause We stated that curves were listed "in order of preference", but we never explained what the preference was, so this was not meaningful. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 1c271c913..dd3988eec 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -506,8 +506,7 @@ mbedtls_ecp_curve_type mbedtls_ecp_get_type( const mbedtls_ecp_group *grp ); /** * \brief This function retrieves the information defined in - * mbedtls_ecp_curve_info() for all supported curves in order - * of preference. + * mbedtls_ecp_curve_info() for all supported curves. * * \note This function returns information about all curves * supported by the library. Some curves may not be From b1940a76ad50248a536b16c3689719e6d2a5c5f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 15:18:12 +0200 Subject: [PATCH 08/20] In TLS, order curves by resource usage, not size TLS used to prefer larger curves, under the idea that a larger curve has a higher security strength and is therefore harder to attack. However, brute force attacks are not a practical concern, so this was not particularly meaningful. If a curve is considered secure enough to be allowed, then we might as well use it. So order curves by resource usage. The exact definition of what this means is purposefully left open. It may include criteria such as performance and memory usage. Risk of side channels could be a factor as well, although it didn't affect the current choice. The current list happens to exactly correspond to the numbers reported by one run of the benchmark program for "full handshake/s" on my machine. Signed-off-by: Gilles Peskine --- ChangeLog.d/default-curves.txt | 3 +- docs/3.0-migration-guide.d/default-curves.md | 7 ++++ include/mbedtls/ssl.h | 3 +- library/ssl_tls.c | 36 ++++++++++---------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/ChangeLog.d/default-curves.txt b/ChangeLog.d/default-curves.txt index 1a805623d..bfb0fd0e0 100644 --- a/ChangeLog.d/default-curves.txt +++ b/ChangeLog.d/default-curves.txt @@ -1,7 +1,8 @@ Default behavior changes * Some default policies for X.509 certificate verification and TLS have changed: curves and hashes weaker than 255 bits are no longer accepted - by default. + by default. The default order in TLS now favors faster curves over larger + curves. Removals * Remove the compile-time option diff --git a/docs/3.0-migration-guide.d/default-curves.md b/docs/3.0-migration-guide.d/default-curves.md index 5db517ebd..5879d77a9 100644 --- a/docs/3.0-migration-guide.d/default-curves.md +++ b/docs/3.0-migration-guide.d/default-curves.md @@ -14,3 +14,10 @@ my_profile.allowed_mds |= MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA224 ); ``` If you still need to allow hashes and curves in TLS that have been removed from the default configuration, call `mbedtls_ssl_conf_sig_hashes()` and `mbedtls_ssl_conf_curves()` with the desired lists. + +TLS now favors faster curves over larger curves +----------------------------------------------- + +The default preference order for curves in TLS now favors resource usage (performance and memory consumption) over size. The exact order is unspecified and may change, but generally you can expect 256-bit curves to be preferred. + +If you prefer a different order, call `mbedtls_ssl_conf_curves()` when configuring a TLS connection. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index df3974aea..d885d213e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2916,8 +2916,7 @@ void mbedtls_ssl_conf_dhm_min_bitlen( mbedtls_ssl_config *conf, * \note The default list is the same set of curves that * #mbedtls_x509_crt_profile_default allows, plus * ECDHE-only curves selected according to the same criteria. - * Larger (generally more secure but slower) curves are - * preferred over smaller curves. + * The order favors curves with the lowest resource usage. * * \param conf SSL configuration * \param curves Ordered list of allowed curves, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index be389f03b..07569b240 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6120,27 +6120,12 @@ static int ssl_preset_default_hashes[] = { #if defined(MBEDTLS_ECP_C) /* The selection should be the same as mbedtls_x509_crt_profile_default in * x509_crt.c, plus Montgomery curves for ECDHE. Here, the order matters: - * larger curves first, like ecp_supported_curves in ecp.c. + * curves with a lower resource usage come first. * See the documentation of mbedtls_ssl_conf_curves() for what we promise - * about this list. */ + * about this list. + */ static mbedtls_ecp_group_id ssl_preset_default_curves[] = { -#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) - MBEDTLS_ECP_DP_SECP521R1, -#endif -#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) - MBEDTLS_ECP_DP_BP512R1, -#endif -#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) - MBEDTLS_ECP_DP_CURVE448, -#endif -#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) - MBEDTLS_ECP_DP_SECP384R1, -#endif -#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) - MBEDTLS_ECP_DP_BP384R1, -#endif #if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) - // Positioned in the list as a fast 256-bit curve, not as a 255-bit curve MBEDTLS_ECP_DP_CURVE25519, #endif #if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) @@ -6149,8 +6134,23 @@ static mbedtls_ecp_group_id ssl_preset_default_curves[] = { #if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) MBEDTLS_ECP_DP_SECP256K1, #endif +#if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) + MBEDTLS_ECP_DP_SECP384R1, +#endif +#if defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) + MBEDTLS_ECP_DP_CURVE448, +#endif +#if defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) + MBEDTLS_ECP_DP_SECP521R1, +#endif #if defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) MBEDTLS_ECP_DP_BP256R1, +#endif +#if defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) + MBEDTLS_ECP_DP_BP384R1, +#endif +#if defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) + MBEDTLS_ECP_DP_BP512R1, #endif MBEDTLS_ECP_DP_NONE }; From a28f0f50823fca84afecf1fd4069c1ead6414088 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 15:29:38 +0200 Subject: [PATCH 09/20] Leave the preference order for hashes unspecified We don't seem to have strong feelings about this, so allow ourselves to change the order later. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 5 +++-- library/ssl_tls.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d885d213e..b38bd72e0 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2943,8 +2943,9 @@ void mbedtls_ssl_conf_curves( mbedtls_ssl_config *conf, * \note By default, all supported hashes whose length is at least * 256 bits are allowed. This is the same set as the default * for certificate verification - * (#mbedtls_x509_crt_profile_default). Larger hashes are - * preferred. + * (#mbedtls_x509_crt_profile_default). + * The preference order is currently unspecified and may + * change in future versions. * * \param conf SSL configuration * \param hashes Ordered list of allowed signature hashes, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 07569b240..3bbdcb086 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6099,8 +6099,8 @@ void mbedtls_ssl_config_init( mbedtls_ssl_config *conf ) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* The selection should be the same as mbedtls_x509_crt_profile_default in - * x509_crt.c. Here, the order matters: larger hashes first, for consistency - * with curves. + * x509_crt.c. Here, the order matters. Currently we favor stronger hashes, + * for no fundamental reason. * See the documentation of mbedtls_ssl_conf_curves() for what we promise * about this list. */ static int ssl_preset_default_hashes[] = { From c5b9510114b3c78f27f1ff68c93b05b0b20a637b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 20:14:59 +0200 Subject: [PATCH 10/20] Clarify test case descriptions Reorder test cases and make their descriptions more explicit. No change in test data. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_debug.data | 36 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/suites/test_suite_debug.data b/tests/suites/test_suite_debug.data index 0935c1244..f1d29ef7d 100644 --- a/tests/suites/test_suite_debug.data +++ b/tests/suites/test_suite_debug.data @@ -37,6 +37,24 @@ mbedtls_debug_print_buf:"MyFile":999:"Test return value":"000102030405060708090A Debug print buffer #5 mbedtls_debug_print_buf:"MyFile":999:"Test return value":"000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F30":"MyFile(0999)\: dumping 'Test return value' (49 bytes)\nMyFile(0999)\: 0000\: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................\nMyFile(0999)\: 0010\: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................\nMyFile(0999)\: 0020\: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./\nMyFile(0999)\: 0030\: 30 0\n" +Debug print mbedtls_mpi: 0 (non-empty representation) +mbedtls_debug_print_mpi:16:"00000000000000":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (0 bits) is\:\nMyFile(0999)\: 00\n" + +Debug print mbedtls_mpi #2: 3 bits +mbedtls_debug_print_mpi:16:"00000000000007":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (3 bits) is\:\nMyFile(0999)\: 07\n" + +Debug print mbedtls_mpi: 49 bits +mbedtls_debug_print_mpi:16:"01020304050607":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (49 bits) is\:\nMyFile(0999)\: 01 02 03 04 05 06 07\n" + +Debug print mbedtls_mpi: 759 bits +mbedtls_debug_print_mpi:16:"0000000000000000000000000000000000000000000000000000000041379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (759 bits) is\:\nMyFile(0999)\: 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a 14\nMyFile(0999)\: 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90 ff\nMyFile(0999)\: e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c 09\nMyFile(0999)\: 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89 af\nMyFile(0999)\: 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b 52\nMyFile(0999)\: 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" + +Debug print mbedtls_mpi: 764 bits #1 +mbedtls_debug_print_mpi:16:"0941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (764 bits) is\:\nMyFile(0999)\: 09 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a\nMyFile(0999)\: 14 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90\nMyFile(0999)\: ff e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c\nMyFile(0999)\: 09 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89\nMyFile(0999)\: af 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b\nMyFile(0999)\: 52 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" + +Debug print mbedtls_mpi: 764 bits #2 +mbedtls_debug_print_mpi:16:"0000000000000000000000000000000000000000000000000000000941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (764 bits) is\:\nMyFile(0999)\: 09 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a\nMyFile(0999)\: 14 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90\nMyFile(0999)\: ff e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c\nMyFile(0999)\: 09 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89\nMyFile(0999)\: af 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b\nMyFile(0999)\: 52 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" + Debug print certificate #1 (RSA) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_BASE64_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C:!MBEDTLS_X509_REMOVE_INFO mbedtls_debug_print_crt:"data_files/server1.crt":"MyFile":999:"PREFIX_":"MyFile(0999)\: PREFIX_ #1\:\nMyFile(0999)\: cert. version \: 3\nMyFile(0999)\: serial number \: 01\nMyFile(0999)\: issuer name \: C=NL, O=PolarSSL, CN=PolarSSL Test CA\nMyFile(0999)\: subject name \: C=NL, O=PolarSSL, CN=PolarSSL Server 1\nMyFile(0999)\: issued on \: 2019-02-10 14\:44\:06\nMyFile(0999)\: expires on \: 2029-02-10 14\:44\:06\nMyFile(0999)\: signed using \: RSA with SHA1\nMyFile(0999)\: RSA key size \: 2048 bits\nMyFile(0999)\: basic constraints \: CA=false\nMyFile(0999)\: value of 'crt->rsa.N' (2048 bits) is\:\nMyFile(0999)\: a9 02 1f 3d 40 6a d5 55 53 8b fd 36 ee 82 65 2e\nMyFile(0999)\: 15 61 5e 89 bf b8 e8 45 90 db ee 88 16 52 d3 f1\nMyFile(0999)\: 43 50 47 96 12 59 64 87 6b fd 2b e0 46 f9 73 be\nMyFile(0999)\: dd cf 92 e1 91 5b ed 66 a0 6f 89 29 79 45 80 d0\nMyFile(0999)\: 83 6a d5 41 43 77 5f 39 7c 09 04 47 82 b0 57 39\nMyFile(0999)\: 70 ed a3 ec 15 19 1e a8 33 08 47 c1 05 42 a9 fd\nMyFile(0999)\: 4c c3 b4 df dd 06 1f 4d 10 51 40 67 73 13 0f 40\nMyFile(0999)\: f8 6d 81 25 5f 0a b1 53 c6 30 7e 15 39 ac f9 5a\nMyFile(0999)\: ee 7f 92 9e a6 05 5b e7 13 97 85 b5 23 92 d9 d4\nMyFile(0999)\: 24 06 d5 09 25 89 75 07 dd a6 1a 8f 3f 09 19 be\nMyFile(0999)\: ad 65 2c 64 eb 95 9b dc fe 41 5e 17 a6 da 6c 5b\nMyFile(0999)\: 69 cc 02 ba 14 2c 16 24 9c 4a dc cd d0 f7 52 67\nMyFile(0999)\: 73 f1 2d a0 23 fd 7e f4 31 ca 2d 70 ca 89 0b 04\nMyFile(0999)\: db 2e a6 4f 70 6e 9e ce bd 58 89 e2 53 59 9e 6e\nMyFile(0999)\: 5a 92 65 e2 88 3f 0c 94 19 a3 dd e5 e8 9d 95 13\nMyFile(0999)\: ed 29 db ab 70 12 dc 5a ca 6b 17 ab 52 82 54 b1\nMyFile(0999)\: value of 'crt->rsa.E' (17 bits) is\:\nMyFile(0999)\: 01 00 01\n" @@ -44,21 +62,3 @@ mbedtls_debug_print_crt:"data_files/server1.crt":"MyFile":999:"PREFIX_":"MyFile( Debug print certificate #2 (EC) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_BASE64_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_SHA256_C:!MBEDTLS_X509_REMOVE_INFO mbedtls_debug_print_crt:"data_files/test-ca2.crt":"MyFile":999:"PREFIX_":"MyFile(0999)\: PREFIX_ #1\:\nMyFile(0999)\: cert. version \: 3\nMyFile(0999)\: serial number \: C1\:43\:E2\:7E\:62\:43\:CC\:E8\nMyFile(0999)\: issuer name \: C=NL, O=PolarSSL, CN=Polarssl Test EC CA\nMyFile(0999)\: subject name \: C=NL, O=PolarSSL, CN=Polarssl Test EC CA\nMyFile(0999)\: issued on \: 2019-02-10 14\:44\:00\nMyFile(0999)\: expires on \: 2029-02-10 14\:44\:00\nMyFile(0999)\: signed using \: ECDSA with SHA256\nMyFile(0999)\: EC key size \: 384 bits\nMyFile(0999)\: basic constraints \: CA=true\nMyFile(0999)\: value of 'crt->eckey.Q(X)' (384 bits) is\:\nMyFile(0999)\: c3 da 2b 34 41 37 58 2f 87 56 fe fc 89 ba 29 43\nMyFile(0999)\: 4b 4e e0 6e c3 0e 57 53 33 39 58 d4 52 b4 91 95\nMyFile(0999)\: 39 0b 23 df 5f 17 24 62 48 fc 1a 95 29 ce 2c 2d\nMyFile(0999)\: value of 'crt->eckey.Q(Y)' (384 bits) is\:\nMyFile(0999)\: 87 c2 88 52 80 af d6 6a ab 21 dd b8 d3 1c 6e 58\nMyFile(0999)\: b8 ca e8 b2 69 8e f3 41 ad 29 c3 b4 5f 75 a7 47\nMyFile(0999)\: 6f d5 19 29 55 69 9a 53 3b 20 b4 66 16 60 33 1e\n" - -Debug print mbedtls_mpi #1 -mbedtls_debug_print_mpi:16:"01020304050607":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (49 bits) is\:\nMyFile(0999)\: 01 02 03 04 05 06 07\n" - -Debug print mbedtls_mpi #2 -mbedtls_debug_print_mpi:16:"00000000000007":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (3 bits) is\:\nMyFile(0999)\: 07\n" - -Debug print mbedtls_mpi #3 -mbedtls_debug_print_mpi:16:"00000000000000":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (0 bits) is\:\nMyFile(0999)\: 00\n" - -Debug print mbedtls_mpi #4 -mbedtls_debug_print_mpi:16:"0941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (764 bits) is\:\nMyFile(0999)\: 09 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a\nMyFile(0999)\: 14 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90\nMyFile(0999)\: ff e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c\nMyFile(0999)\: 09 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89\nMyFile(0999)\: af 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b\nMyFile(0999)\: 52 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" - -Debug print mbedtls_mpi #5 -mbedtls_debug_print_mpi:16:"0000000000000000000000000000000000000000000000000000000941379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (764 bits) is\:\nMyFile(0999)\: 09 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a\nMyFile(0999)\: 14 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90\nMyFile(0999)\: ff e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c\nMyFile(0999)\: 09 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89\nMyFile(0999)\: af 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b\nMyFile(0999)\: 52 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" - -Debug print mbedtls_mpi #6 -mbedtls_debug_print_mpi:16:"0000000000000000000000000000000000000000000000000000000041379d00fed1491fe15df284dfde4a142f68aa8d412023195cee66883e6290ffe703f4ea5963bf212713cee46b107c09182b5edcd955adac418bf4918e2889af48e1099d513830cec85c26ac1e158b52620e33ba8692f893efbb2f958b4424":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (759 bits) is\:\nMyFile(0999)\: 41 37 9d 00 fe d1 49 1f e1 5d f2 84 df de 4a 14\nMyFile(0999)\: 2f 68 aa 8d 41 20 23 19 5c ee 66 88 3e 62 90 ff\nMyFile(0999)\: e7 03 f4 ea 59 63 bf 21 27 13 ce e4 6b 10 7c 09\nMyFile(0999)\: 18 2b 5e dc d9 55 ad ac 41 8b f4 91 8e 28 89 af\nMyFile(0999)\: 48 e1 09 9d 51 38 30 ce c8 5c 26 ac 1e 15 8b 52\nMyFile(0999)\: 62 0e 33 ba 86 92 f8 93 ef bb 2f 95 8b 44 24\n" From 3beb72eeaf492adedfb718b640ad6e7f32ef898f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 20:16:53 +0200 Subject: [PATCH 11/20] Add mbedtls_debug_print_mpi test case for 0 There was already a test case for 0 but with a non-empty representation (X->n == 1). Add a test case with X->n == 0 (freshly initialized mpi). Signed-off-by: Gilles Peskine --- tests/suites/test_suite_debug.data | 3 +++ tests/suites/test_suite_debug.function | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_debug.data b/tests/suites/test_suite_debug.data index f1d29ef7d..4a1a1be48 100644 --- a/tests/suites/test_suite_debug.data +++ b/tests/suites/test_suite_debug.data @@ -37,6 +37,9 @@ mbedtls_debug_print_buf:"MyFile":999:"Test return value":"000102030405060708090A Debug print buffer #5 mbedtls_debug_print_buf:"MyFile":999:"Test return value":"000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F30":"MyFile(0999)\: dumping 'Test return value' (49 bytes)\nMyFile(0999)\: 0000\: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................\nMyFile(0999)\: 0010\: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f ................\nMyFile(0999)\: 0020\: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f !"#$%&'()*+,-./\nMyFile(0999)\: 0030\: 30 0\n" +Debug print mbedtls_mpi: 0 (empty representation) +mbedtls_debug_print_mpi:16:"":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (0 bits) is\:\nMyFile(0999)\: 00\n" + Debug print mbedtls_mpi: 0 (non-empty representation) mbedtls_debug_print_mpi:16:"00000000000000":"MyFile":999:"VALUE":"MyFile(0999)\: value of 'VALUE' (0 bits) is\:\nMyFile(0999)\: 00\n" diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index ad50e53fd..fda693987 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -179,7 +179,9 @@ void mbedtls_debug_print_mpi( int radix, char * value, char * file, int line, TEST_ASSERT( mbedtls_ssl_setup( &ssl, &conf ) == 0 ); - TEST_ASSERT( mbedtls_mpi_read_string( &val, radix, value ) == 0 ); + /* If value is empty, keep val->n == 0. */ + if( value[0] != 0 ) + TEST_ASSERT( mbedtls_mpi_read_string( &val, radix, value ) == 0 ); mbedtls_ssl_conf_dbg( &conf, string_debug, &buffer); From b26696bafb03eead55dd2b0f0835c335aca571f8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 20:17:46 +0200 Subject: [PATCH 12/20] Simplify mbedtls_debug_print_mpi and fix the case of empty bignums Rewrite mbedtls_debug_print_mpi to be simpler and smaller. Leverage mbedtls_mpi_bitlen() instead of manually looking for the leading zeros. Fix #4608: the old code made an invalid memory dereference when X->n==0 (freshly initialized bignum with the value 0). Signed-off-by: Gilles Peskine --- ChangeLog.d/mbedtls_debug_print_mpi.txt | 5 ++ library/debug.c | 70 ++++++++++--------------- 2 files changed, 34 insertions(+), 41 deletions(-) create mode 100644 ChangeLog.d/mbedtls_debug_print_mpi.txt diff --git a/ChangeLog.d/mbedtls_debug_print_mpi.txt b/ChangeLog.d/mbedtls_debug_print_mpi.txt new file mode 100644 index 000000000..d1b4f5b41 --- /dev/null +++ b/ChangeLog.d/mbedtls_debug_print_mpi.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix a crash in mbedtls_mpi_debug_mpi on a bignum having 0 limbs. This + could notably be triggered by setting the TLS debug level to 3 or above + and using a Montgomery curve for the key exchange. Reported by lhuang04 + in #4578. Fixes #4608. diff --git a/library/debug.c b/library/debug.c index 4be2cba19..e52025851 100644 --- a/library/debug.c +++ b/library/debug.c @@ -220,8 +220,8 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level, const char *text, const mbedtls_mpi *X ) { char str[DEBUG_BUF_SIZE]; - int j, k, zeros = 1; - size_t i, n, idx = 0; + size_t bitlen; + size_t idx = 0; if( NULL == ssl || NULL == ssl->conf || @@ -232,55 +232,43 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level, return; } - for( n = X->n - 1; n > 0; n-- ) - if( X->p[n] != 0 ) - break; - - for( j = ( sizeof(mbedtls_mpi_uint) << 3 ) - 1; j >= 0; j-- ) - if( ( ( X->p[n] >> j ) & 1 ) != 0 ) - break; - - mbedtls_snprintf( str + idx, sizeof( str ) - idx, "value of '%s' (%d bits) is:\n", - text, (int) ( ( n * ( sizeof(mbedtls_mpi_uint) << 3 ) ) + j + 1 ) ); + bitlen = mbedtls_mpi_bitlen( X ); + mbedtls_snprintf( str, sizeof( str ), "value of '%s' (%u bits) is:\n", + text, (unsigned) bitlen ); debug_send_line( ssl, level, file, line, str ); - idx = 0; - for( i = n + 1, j = 0; i > 0; i-- ) + if( bitlen == 0 ) { - if( zeros && X->p[i - 1] == 0 ) - continue; - - for( k = sizeof( mbedtls_mpi_uint ) - 1; k >= 0; k-- ) + str[0] = ' '; str[1] = '0'; str[2] = '0'; + idx = 3; + } + else + { + int n; + for( n = ( bitlen - 1 ) / 8; n >= 0; n-- ) { - if( zeros && ( ( X->p[i - 1] >> ( k << 3 ) ) & 0xFF ) == 0 ) - continue; - else - zeros = 0; - - if( j % 16 == 0 ) + size_t limb_offset = n / sizeof( mbedtls_mpi_uint ); + size_t offset_in_limb = n % sizeof( mbedtls_mpi_uint ); + unsigned char octet = + ( X->p[limb_offset] >> ( offset_in_limb * 8 ) ) & 0xff; + mbedtls_snprintf( str + idx, sizeof( str ) - idx, " %02x", octet ); + idx += 3; + /* Wrap lines after 16 octets that each take 3 columns */ + if( idx >= 3 * 16 ) { - if( j > 0 ) - { - mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" ); - debug_send_line( ssl, level, file, line, str ); - idx = 0; - } + mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" ); + debug_send_line( ssl, level, file, line, str ); + idx = 0; } - - idx += mbedtls_snprintf( str + idx, sizeof( str ) - idx, " %02x", (unsigned int) - ( X->p[i - 1] >> ( k << 3 ) ) & 0xFF ); - - j++; } - } - if( zeros == 1 ) - idx += mbedtls_snprintf( str + idx, sizeof( str ) - idx, " 00" ); - - mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" ); - debug_send_line( ssl, level, file, line, str ); + if( idx != 0 ) + { + mbedtls_snprintf( str + idx, sizeof( str ) - idx, "\n" ); + debug_send_line( ssl, level, file, line, str ); + } } #endif /* MBEDTLS_BIGNUM_C */ From 799eee65fd9d58e86fd9d8996f80b00003c73517 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 22:14:15 +0200 Subject: [PATCH 13/20] Update the expected default curve in ssl-opt.sh Signed-off-by: Gilles Peskine --- 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 5ccb467b1..81cb774e7 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1233,7 +1233,7 @@ trap cleanup INT TERM HUP # Checks that: # - things work with all ciphersuites active (used with config-full in all.sh) -# - the expected (highest security) parameters are selected +# - the expected parameters are selected # ("signature_algorithm ext: 6" means SHA-512 (highest common hash)) run_test "Default" \ "$P_SRV debug_level=3" \ @@ -1242,7 +1242,7 @@ run_test "Default" \ -s "Protocol is TLSv1.2" \ -s "Ciphersuite is TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256" \ -s "client hello v3, signature_algorithm ext: 6" \ - -s "ECDHE curve: secp521r1" \ + -s "ECDHE curve: x25519" \ -S "error" \ -C "error" From 3b3aa36962fb9278adb52ab0cd9def18d58d01e7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 3 Jun 2021 11:12:04 +0200 Subject: [PATCH 14/20] Indicate that the truncation from size_t to int is deliberate MPI sizes do fit in int. Let MSVC know this conversion is deliberate. Signed-off-by: Gilles Peskine --- library/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/debug.c b/library/debug.c index e52025851..618d124e5 100644 --- a/library/debug.c +++ b/library/debug.c @@ -246,7 +246,7 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level, else { int n; - for( n = ( bitlen - 1 ) / 8; n >= 0; n-- ) + for( n = (int) ( bitlen - 1 ) / 8; n >= 0; n-- ) { size_t limb_offset = n / sizeof( mbedtls_mpi_uint ); size_t offset_in_limb = n % sizeof( mbedtls_mpi_uint ); From 4a02cef40270b16c4e7aa482dd751ea7c90c5457 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 3 Jun 2021 11:12:40 +0200 Subject: [PATCH 15/20] Test restartable ECC with a curve that supports it The default curve is now Curve25519, which doesn't support restartable ECC. So run the restartable ECC tests with a curve that does support it. Use secp256r1 which is required for these tests anyway for the server's certificate. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 81cb774e7..d3f6d324b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5933,9 +5933,12 @@ run_test "Large server packet TLS 1.2 AEAD shorter tag" \ # Tests for restartable ECC +# Force the use of a curve that supports restartable ECC (secp256r1). + requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, default" \ - "$P_SRV auth_mode=required" \ + "$P_SRV curves=secp256r1 auth_mode=required" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ key_file=data_files/server5.key crt_file=data_files/server5.crt \ debug_level=1" \ @@ -5946,8 +5949,9 @@ run_test "EC restart: TLS, default" \ -C "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=0" \ - "$P_SRV auth_mode=required" \ + "$P_SRV curves=secp256r1 auth_mode=required" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ key_file=data_files/server5.key crt_file=data_files/server5.crt \ debug_level=1 ec_max_ops=0" \ @@ -5958,8 +5962,9 @@ run_test "EC restart: TLS, max_ops=0" \ -C "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=65535" \ - "$P_SRV auth_mode=required" \ + "$P_SRV curves=secp256r1 auth_mode=required" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ key_file=data_files/server5.key crt_file=data_files/server5.crt \ debug_level=1 ec_max_ops=65535" \ @@ -5970,8 +5975,9 @@ run_test "EC restart: TLS, max_ops=65535" \ -C "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000" \ - "$P_SRV auth_mode=required" \ + "$P_SRV curves=secp256r1 auth_mode=required" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ key_file=data_files/server5.key crt_file=data_files/server5.crt \ debug_level=1 ec_max_ops=1000" \ @@ -5982,8 +5988,9 @@ run_test "EC restart: TLS, max_ops=1000" \ -c "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000, badsign" \ - "$P_SRV auth_mode=required \ + "$P_SRV curves=secp256r1 auth_mode=required \ crt_file=data_files/server5-badsign.crt \ key_file=data_files/server5.key" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ @@ -5999,8 +6006,9 @@ run_test "EC restart: TLS, max_ops=1000, badsign" \ -c "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000, auth_mode=optional badsign" \ - "$P_SRV auth_mode=required \ + "$P_SRV curves=secp256r1 auth_mode=required \ crt_file=data_files/server5-badsign.crt \ key_file=data_files/server5.key" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ @@ -6016,8 +6024,9 @@ run_test "EC restart: TLS, max_ops=1000, auth_mode=optional badsign" \ -C "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000, auth_mode=none badsign" \ - "$P_SRV auth_mode=required \ + "$P_SRV curves=secp256r1 auth_mode=required \ crt_file=data_files/server5-badsign.crt \ key_file=data_files/server5.key" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ @@ -6033,8 +6042,9 @@ run_test "EC restart: TLS, max_ops=1000, auth_mode=none badsign" \ -C "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: DTLS, max_ops=1000" \ - "$P_SRV auth_mode=required dtls=1" \ + "$P_SRV curves=secp256r1 auth_mode=required dtls=1" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ key_file=data_files/server5.key crt_file=data_files/server5.crt \ dtls=1 debug_level=1 ec_max_ops=1000" \ @@ -6045,8 +6055,9 @@ run_test "EC restart: DTLS, max_ops=1000" \ -c "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000 no client auth" \ - "$P_SRV" \ + "$P_SRV curves=secp256r1" \ "$P_CLI force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ debug_level=1 ec_max_ops=1000" \ 0 \ @@ -6056,8 +6067,9 @@ run_test "EC restart: TLS, max_ops=1000 no client auth" \ -C "mbedtls_pk_sign.*4b00" requires_config_enabled MBEDTLS_ECP_RESTARTABLE +requires_config_enabled MBEDTLS_ECP_DP_SECP256R1_ENABLED run_test "EC restart: TLS, max_ops=1000, ECDHE-PSK" \ - "$P_SRV psk=abc123" \ + "$P_SRV curves=secp256r1 psk=abc123" \ "$P_CLI force_ciphersuite=TLS-ECDHE-PSK-WITH-AES-128-CBC-SHA256 \ psk=abc123 debug_level=1 ec_max_ops=1000" \ 0 \ From 55cb9af910a0da7af067b152aa37eb149c6f7ac1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Jun 2021 20:56:20 +0200 Subject: [PATCH 16/20] Add missing parentheses Signed-off-by: Gilles Peskine --- library/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/debug.c b/library/debug.c index 618d124e5..fa60d13f3 100644 --- a/library/debug.c +++ b/library/debug.c @@ -246,7 +246,7 @@ void mbedtls_debug_print_mpi( const mbedtls_ssl_context *ssl, int level, else { int n; - for( n = (int) ( bitlen - 1 ) / 8; n >= 0; n-- ) + for( n = (int) ( ( bitlen - 1 ) / 8 ); n >= 0; n-- ) { size_t limb_offset = n / sizeof( mbedtls_mpi_uint ); size_t offset_in_limb = n % sizeof( mbedtls_mpi_uint ); From 6b1f64a1506672a02b2780ea28fe7b503a675ce6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Jun 2021 21:05:37 +0200 Subject: [PATCH 17/20] Wording clarifications Signed-off-by: Gilles Peskine --- docs/3.0-migration-guide.d/default-curves.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/3.0-migration-guide.d/default-curves.md b/docs/3.0-migration-guide.d/default-curves.md index 5879d77a9..825609e43 100644 --- a/docs/3.0-migration-guide.d/default-curves.md +++ b/docs/3.0-migration-guide.d/default-curves.md @@ -1,13 +1,13 @@ -Weak curves are now disabled by default for X.509 and TLS ---------------------------------------------------------- +Strengthen default algorithm selection for X.509 and TLS +-------------------------------------------------------- -The default X.509 verification profile (`mbedtls_x509_crt_profile_default`) and the default curve and hash selection have changed. X.509 and TLS now allow the same algorithms by default (except that the X.509 profile only lists curves that support signature verification). +The default X.509 verification profile (`mbedtls_x509_crt_profile_default`) and the default curve and hash selection in TLS have changed. They are now aligned, except that the X.509 profile only lists curves that support signature verification. -Hashes and curves weaker than 255 bits are no longer accepted by default. The following algorithms have been removed: SHA-1 (formerly only accepted for key exchanges but not for certificate signatures), SHA-224, secp192r1, secp224r1, secp192k1, secp224k1 (weaker hashes were already not accepted). +Hashes and curves weaker than 255 bits (security strength less than 128 bits) are no longer accepted by default. The following hashes have been removed: SHA-1 (formerly only accepted for key exchanges but not for certificate signatures), SHA-224 (weaker hashes were already not accepted). The following curves have been removed: secp192r1, secp224r1, secp192k1, secp224k1. The compile-time option `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` is no longer available. -If you still need to accept certificates signed with algorithms that have been removed from the default profile, call `mbedtls_x509_crt_verify_with_profile` instead of `mbedtls_x509_crt_verify` and pass a profile that allows the curves you want. For example, to allow SHA-224: +If you still need to accept certificates signed with algorithms that have been removed from the default profile, call `mbedtls_x509_crt_verify_with_profile` instead of `mbedtls_x509_crt_verify` and pass a profile that allows the curves and hashes you want. For example, to allow SHA-224: ``` mbedtls_x509_crt_profile my_profile = mbedtls_x509_crt_profile_default; my_profile.allowed_mds |= MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA224 ); @@ -18,6 +18,6 @@ If you still need to allow hashes and curves in TLS that have been removed from TLS now favors faster curves over larger curves ----------------------------------------------- -The default preference order for curves in TLS now favors resource usage (performance and memory consumption) over size. The exact order is unspecified and may change, but generally you can expect 256-bit curves to be preferred. +The default preference order for curves in TLS now favors resource usage (performance and memory consumption) over size. The exact order is unspecified and may change, but generally you can expect 256-bit curves to be preferred over larger curves. If you prefer a different order, call `mbedtls_ssl_conf_curves()` when configuring a TLS connection. From ec78bc47b53b290c0c833744671433cdd42be241 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Jun 2021 21:11:27 +0200 Subject: [PATCH 18/20] Meld DEFAULT_ALLOW_SHA1_IN_CERTIFICATES removal migration guide Meld the migration guide for the removal of MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES into the migration guide for the strengthening of TLS and X.509 defaults, which is more general. The information in the SHA-1 section was largely already present in the strengthening section. It is now less straightforward to figure out how to enable SHA-1 in certificates, but that's a good thing, since no one should still be doing this in 2021. Signed-off-by: Gilles Peskine --- docs/3.0-migration-guide.d/default-curves.md | 2 +- docs/3.0-migration-guide.md | 25 -------------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/docs/3.0-migration-guide.d/default-curves.md b/docs/3.0-migration-guide.d/default-curves.md index 825609e43..551e28721 100644 --- a/docs/3.0-migration-guide.d/default-curves.md +++ b/docs/3.0-migration-guide.d/default-curves.md @@ -5,7 +5,7 @@ The default X.509 verification profile (`mbedtls_x509_crt_profile_default`) and Hashes and curves weaker than 255 bits (security strength less than 128 bits) are no longer accepted by default. The following hashes have been removed: SHA-1 (formerly only accepted for key exchanges but not for certificate signatures), SHA-224 (weaker hashes were already not accepted). The following curves have been removed: secp192r1, secp224r1, secp192k1, secp224k1. -The compile-time option `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` is no longer available. +The compile-time options `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES` and `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` are no longer available. If you still need to accept certificates signed with algorithms that have been removed from the default profile, call `mbedtls_x509_crt_verify_with_profile` instead of `mbedtls_x509_crt_verify` and pass a profile that allows the curves and hashes you want. For example, to allow SHA-224: ``` diff --git a/docs/3.0-migration-guide.md b/docs/3.0-migration-guide.md index a4a59b889..2ae74c910 100644 --- a/docs/3.0-migration-guide.md +++ b/docs/3.0-migration-guide.md @@ -65,31 +65,6 @@ If you're a library user and used to rely on having access to a structure or function that's now in a private header, please reach out on the mailing list and explain your need; we'll consider adding a new API in a future version. -Remove the option to allow SHA-1 by default in certificates ------------------------------------------------------------ - -This does not affect users who use the default `config.h`, as this option was -already off by default. - -If you used to enable `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES` in your -`config.h`, first please take a moment to consider whether you really still -want to accept certificates signed with SHA-1 as those are considered insecure -and no CA has issued them for a while. If you really need to allow SHA-1 in -certificates, please set up a custom profile as follows: - -``` -const mbedtls_x509_crt_profile mbedtls_x509_crt_custom = { - MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA1 ) | - MBEDTLS_X509_ID_FLAG( /* other hash */ ) /* | etc */, - 0xFFFFFFF, /* Or specific PK algs */ - 0xFFFFFFF, /* Or specific curves */ - 2048 /* Or another RSA min bitlen */ -}; -``` -Then pass it to `mbedtls_x509_crt_verify_with_profile()` if you're verifying -a certificate chain directly, or to `mbedtls_ssl_conf_cert_profile()` if the -verification happens during a TLS handshake. - Remove the certs module from the library ---------------------------------------- From a03fb29666317a397fa76db9f0d826162b3225cb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Jun 2021 23:17:05 +0200 Subject: [PATCH 19/20] Document backward compatibility promises for the default TLS profile Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b38bd72e0..400036919 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2918,6 +2918,14 @@ void mbedtls_ssl_conf_dhm_min_bitlen( mbedtls_ssl_config *conf, * ECDHE-only curves selected according to the same criteria. * The order favors curves with the lowest resource usage. * + * \note New minor versions of Mbed TLS may extend this list, + * for example if new curves are added to the library. + * New minor versions of Mbed TLS will not remove items + * from this list unless serious security concerns require it. + * New minor versions of Mbed TLS may change the order in + * keeping with the general principle of favoring the lowest + * resource usage. + * * \param conf SSL configuration * \param curves Ordered list of allowed curves, * terminated by MBEDTLS_ECP_DP_NONE. @@ -2947,6 +2955,11 @@ void mbedtls_ssl_conf_curves( mbedtls_ssl_config *conf, * The preference order is currently unspecified and may * change in future versions. * + * \note New minor versions of Mbed TLS may extend this list, + * for example if new curves are added to the library. + * New minor versions of Mbed TLS will not remove items + * from this list unless serious security concerns require it. + * * \param conf SSL configuration * \param hashes Ordered list of allowed signature hashes, * terminated by \c MBEDTLS_MD_NONE. From 39957503c56a1f1e8a7d76c64ec6ef25e30d561a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Jun 2021 23:17:52 +0200 Subject: [PATCH 20/20] Remove secp256k1 from the default X.509 and TLS profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For TLS, secp256k1 is deprecated by RFC 8422 ยง5.1.1. For X.509, secp256k1 is not deprecated, but it isn't used in practice, especially in the context of TLS where there isn't much point in having an X.509 certificate which most peers do not support. So remove it from the default profile. We can add it back later if there is demand. Signed-off-by: Gilles Peskine --- docs/3.0-migration-guide.d/default-curves.md | 2 ++ include/mbedtls/x509_crt.h | 4 ++-- library/ssl_tls.c | 3 --- library/x509_crt.c | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/3.0-migration-guide.d/default-curves.md b/docs/3.0-migration-guide.d/default-curves.md index 551e28721..928130d24 100644 --- a/docs/3.0-migration-guide.d/default-curves.md +++ b/docs/3.0-migration-guide.d/default-curves.md @@ -7,6 +7,8 @@ Hashes and curves weaker than 255 bits (security strength less than 128 bits) ar The compile-time options `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES` and `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` are no longer available. +The curve secp256k1 has also been removed from the default X.509 and TLS profiles. [RFC 8422](https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.1) deprecates it in TLS, and it is very rarely used, although it is not known to be weak at the time of writing. + If you still need to accept certificates signed with algorithms that have been removed from the default profile, call `mbedtls_x509_crt_verify_with_profile` instead of `mbedtls_x509_crt_verify` and pass a profile that allows the curves and hashes you want. For example, to allow SHA-224: ``` mbedtls_x509_crt_profile my_profile = mbedtls_x509_crt_profile_default; diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 9c7016d8e..f991251ca 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -335,11 +335,11 @@ typedef void mbedtls_x509_crt_restart_ctx; * * This profile permits: * - SHA2 hashes with at least 256 bits: SHA-256, SHA-384, SHA-512. - * - Elliptic curves with 255 bits and above. + * - Elliptic curves with 255 bits and above except secp256k1. * - RSA with 2048 bits and above. * * New minor versions of Mbed TLS may extend this profile, for example if - * new curves are added to the library. New minor versions of Mbed TLS will + * new algorithms are added to the library. New minor versions of Mbed TLS will * not reduce this profile unless serious security concerns require it. */ extern const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3bbdcb086..2688af173 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6131,9 +6131,6 @@ static mbedtls_ecp_group_id ssl_preset_default_curves[] = { #if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) MBEDTLS_ECP_DP_SECP256R1, #endif -#if defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) - MBEDTLS_ECP_DP_SECP256K1, -#endif #if defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) MBEDTLS_ECP_DP_SECP384R1, #endif diff --git a/library/x509_crt.c b/library/x509_crt.c index 650044583..f12ac6b7e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -114,7 +114,7 @@ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default = MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP256R1 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP384R1 ) | MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_BP512R1 ) | - MBEDTLS_X509_ID_FLAG( MBEDTLS_ECP_DP_SECP256K1 ), + 0, #else 0, #endif