Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the default hash and curve selection for X.509 and TLS #4604

Merged
merged 20 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ffb92da
Upgrade the default X.509 profile to the former "next" profile
gilles-peskine-arm Jun 1, 2021
ae270bf
Upgrade the default TLS hash and curve selection, matching X.509
gilles-peskine-arm Jun 1, 2021
3758fd6
Changelog entry and migration guide for hash and curve profile upgrades
gilles-peskine-arm Jun 1, 2021
2c69fa2
Initializer element was not constant
gilles-peskine-arm Jun 1, 2021
12b5b38
Fix "PSA - ECDH with [non-default curve]"
gilles-peskine-arm Jun 2, 2021
5752e59
Reduce the default ECP window size
gilles-peskine-arm Jun 2, 2021
377c91e
Remove meaningless clause
gilles-peskine-arm Jun 2, 2021
b1940a7
In TLS, order curves by resource usage, not size
gilles-peskine-arm Jun 2, 2021
a28f0f5
Leave the preference order for hashes unspecified
gilles-peskine-arm Jun 2, 2021
c5b9510
Clarify test case descriptions
gilles-peskine-arm Jun 2, 2021
3beb72e
Add mbedtls_debug_print_mpi test case for 0
gilles-peskine-arm Jun 2, 2021
b26696b
Simplify mbedtls_debug_print_mpi and fix the case of empty bignums
gilles-peskine-arm Jun 2, 2021
799eee6
Update the expected default curve in ssl-opt.sh
gilles-peskine-arm Jun 2, 2021
3b3aa36
Indicate that the truncation from size_t to int is deliberate
gilles-peskine-arm Jun 3, 2021
4a02cef
Test restartable ECC with a curve that supports it
gilles-peskine-arm Jun 3, 2021
55cb9af
Add missing parentheses
gilles-peskine-arm Jun 7, 2021
6b1f64a
Wording clarifications
gilles-peskine-arm Jun 7, 2021
ec78bc4
Meld DEFAULT_ALLOW_SHA1_IN_CERTIFICATES removal migration guide
gilles-peskine-arm Jun 7, 2021
a03fb29
Document backward compatibility promises for the default TLS profile
gilles-peskine-arm Jun 17, 2021
3995750
Remove secp256k1 from the default X.509 and TLS profiles
gilles-peskine-arm Jun 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ChangeLog.d/default-curves.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
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. The default order in TLS now favors faster curves over larger
curves.

Removals
* Remove the compile-time option
MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE.
3 changes: 3 additions & 0 deletions ChangeLog.d/ecp-window-size.txt
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions ChangeLog.d/mbedtls_debug_print_mpi.txt
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 25 additions & 0 deletions docs/3.0-migration-guide.d/default-curves.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
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 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 (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 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;
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 over larger curves.

If you prefer a different order, call `mbedtls_ssl_conf_curves()` when configuring a TLS connection.
25 changes: 0 additions & 25 deletions docs/3.0-migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------------------------------------

Expand Down
19 changes: 1 addition & 18 deletions include/mbedtls/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions include/mbedtls/ecp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) )
Expand All @@ -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)
Expand Down Expand Up @@ -505,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
Expand Down
27 changes: 25 additions & 2 deletions include/mbedtls/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -2914,6 +2913,19 @@ 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.
Comment on lines +2916 to +2918
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the X.509 and TLS profiles are currently aligned, but we don't promise that they will remain so forever.

* 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.
Expand All @@ -2925,7 +2937,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
Expand All @@ -2937,6 +2948,18 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Here we state that the set is the same as for certificate verification. However, the above description includes SM3, whereas the certificate profile doesn't:
4e754c7#diff-2617ff9076cb3f3c76f4c4e86c40ed9dc2557905d0e2ec7a8a7844e845d88900R336
(I appreciate that SM3 is still in a PR and the TLS cipher suites are still in a draft stage.)

* for certificate verification
* (#mbedtls_x509_crt_profile_default).
* The preference order is currently unspecified and may
* change in future versions.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may or may not want change the documentation of mbedtls_md_list, which currently guarantees that the list “starts with the strongest available hashes” — which by the way means that the list has to be ordered from strongest to weakest, since disabling any subset must leave the strongest remaining hash at the top.

Related: where to add SM3 in mbedtls_md_list()

*
* \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.
Expand Down
12 changes: 11 additions & 1 deletion include/mbedtls/x509_crt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This excludes SHA-224. With 3-DES on its way out, I don't expect SHA-224 to gain in popularity.

* - Elliptic curves with 255 bits and above except secp256k1.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since X.509 support in Mbed TLS is mainly intended for use with TLS and secp256k1 is deprecated in TLS, I decided to remove secp256k1 from the default X.509 profile. What tipped the balance for me is that this is the safe choice from a backward compatibility perspective: we wouldn't remove a curve in a minor version unless there were serious security concerns, but we can add a curve in a minor version.

* - RSA with 2048 bits and above.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should up this to 3072.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well according to SSL pulse ("Key strength distribution"), 2048 is still the most frequent size so I don't think we can require more than that.

*
* New minor versions of Mbed TLS may extend this profile, for example if
* 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;

/**
* 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;

Expand Down
70 changes: 29 additions & 41 deletions library/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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 = (int) ( ( 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 */

Expand Down
4 changes: 2 additions & 2 deletions library/ecp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[] =
{
Expand Down
47 changes: 41 additions & 6 deletions library/ssl_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. 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[] = {
#if defined(MBEDTLS_SHA512_C)
MBEDTLS_MD_SHA512,
Expand All @@ -6108,13 +6113,43 @@ static int ssl_preset_default_hashes[] = {
#if defined(MBEDTLS_SHA256_C)
MBEDTLS_MD_SHA256,
#endif
#if defined(MBEDTLS_SHA224_C)
MBEDTLS_MD_SHA224,
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:
* curves with a lower resource usage come first.
* 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_CURVE25519_ENABLED)
MBEDTLS_ECP_DP_CURVE25519,
#endif
#if defined(MBEDTLS_SHA1_C) && defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE)
MBEDTLS_MD_SHA1,
#if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED)
MBEDTLS_ECP_DP_SECP256R1,
#endif
MBEDTLS_MD_NONE
#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
};
#endif

Expand Down Expand Up @@ -6281,7 +6316,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)
Expand Down
Loading