From 0ecd719edf8067b4fe538f7387fa7440361e9612 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Jun 2021 21:24:26 +0200 Subject: [PATCH 1/6] Document more precisely what goes into the default profile Signed-off-by: Gilles Peskine --- include/mbedtls/x509_crt.h | 11 ++++++++++- library/x509_crt.c | 5 ++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 8e389f8c0..64ccb433b 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -263,12 +263,21 @@ 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. + * - All supported elliptic curves. + * - 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. */ extern const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_next; diff --git a/library/x509_crt.c b/library/x509_crt.c index 8086cc034..acdd54529 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -91,9 +91,8 @@ 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 = { #if defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES) From 646b78b92775f5f8b9d0b0c099c1040eaf847b34 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Jun 2021 21:26:41 +0200 Subject: [PATCH 2/6] Document more precisely what goes into the default preset Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4684a6009..8f11fa243 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2987,7 +2987,7 @@ 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.) + * (Default: all defined curves in order of decreasing size.) * * On server: this only affects selection of the ECDHE curve; * the curves used for ECDH and ECDSA are determined by the @@ -3019,7 +3019,7 @@ 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.) + * (Default: all SHA2 hashes, largest first.) * * \note This only affects which hashes are offered and can be used * for signatures during the handshake. Hashes for message From b3ca90bc449b851e73ed5d71824dbdecc718051f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 13:27:03 +0200 Subject: [PATCH 3/6] 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 d0e61c545..1ae51252e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3617,7 +3617,7 @@ /* ECP options */ //#define MBEDTLS_ECP_MAX_BITS 521 /**< Maximum bit size of groups */ -//#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 b974e5984..f278d105d 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -258,7 +258,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 ) ) @@ -275,7 +276,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 da728b31b06481992ff5bdc48e17a2e9f167fc77 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 14:37:57 +0200 Subject: [PATCH 4/6] 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 f278d105d..1e614edb9 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -467,8 +467,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 138d9f52cf18d6969e07126fc3fe254c2498427d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 21 Jun 2021 09:53:25 +0200 Subject: [PATCH 5/6] SHA-1 is allowed for handshake signatures by default Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8f11fa243..bd0f5d779 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3019,7 +3019,9 @@ 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 SHA2 hashes, largest first.) + * (Default: all SHA-2 hashes, largest first. Also SHA-1 if + * the compile-time option + * `MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE` is enabled.) * * \note This only affects which hashes are offered and can be used * for signatures during the handshake. Hashes for message From 5ea63a31c4b0f42adbebe92ed79003b459119e84 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 22 Jun 2021 10:50:04 +0200 Subject: [PATCH 6/6] Mention the Montgomery curve exception Montgomery curves are not in the expected place in the curve list. This is a bug (https://github.com/ARMmbed/mbedtls/issues/4698), but until this bug is fixed, document the current behavior and indicate that it's likely to change. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index bd0f5d779..30dfbce0f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2987,7 +2987,9 @@ 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 in order of decreasing size.) + * (Default: all defined curves in order of decreasing size, + * except that Montgomery curves come last. This order + * is likely to change in a future version.) * * On server: this only affects selection of the ECDHE curve; * the curves used for ECDH and ECDSA are determined by the