From 5d2511c4d48eb197697466d1bd6b776cf09b0e7c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 May 2017 13:16:40 +0200 Subject: [PATCH] SHA-1 deprecation: allow it in key exchange By default, keep allowing SHA-1 in key exchange signatures. Disabling it causes compatibility issues, especially with clients that use TLS1.2 but don't send the signature_algorithms extension. SHA-1 is forbidden in certificates by default, since it's vulnerable to offline collision-based attacks. --- ChangeLog | 9 +++------ include/mbedtls/config.h | 23 +++++++++++++++++------ include/mbedtls/x509_crt.h | 2 +- library/ssl_tls.c | 2 +- library/x509_crt.c | 2 +- tests/ssl-opt.sh | 9 ++++++++- tests/suites/test_suite_x509parse.data | 2 +- 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3befcade5..265205cdb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,12 +4,9 @@ mbed TLS 2.x.x branch released xxxx-xx-xx Security - * SHA-1 deprecation: remove it from the default allowed hash - algorithms for certificate verification and TLS 1.2 handshake - signatures. It can be turned back on at compile time with - MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 or explicitly with ssl_conf functions. - * Removed RIPEMD-160 from the default hash algorithms for - certificate verification. + * Removed SHA-1 and RIPEMD-160 from the default hash algorithms for + certificate verification. SHA-1 can be turned back on with a compile-time + option if needed. Bugfix * Remove invalid use of size zero arrays in ECJPAKE test suite. diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d52026e88..c4b8995c1 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -2638,13 +2638,24 @@ //#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 certificate signing and - * TLS 1.2 handshake signature. Without this build-time option, SHA-1 - * support must be activated explicitly through mbedtls_ssl_conf_cert_profile - * and mbedtls_ssl_conf_sig_hashes. The use of SHA-1 in TLS <= 1.1 and in - * HMAC-SHA-1 for XXX_SHA ciphersuites is always allowed by default. + * Allow SHA-1 in the default TLS configuration for certificate signing. + * Without this build-time option, SHA-1 support must be activated explicitly + * through mbedtls_ssl_conf_cert_profile. Turning on this option is not + * recommended because of it is possible to generte SHA-1 collisions, however + * this may be safe for legacy infrastructure where additional controls apply. */ -// #define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 +// #define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES + +/** + * 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 + * for compatibility with existing peers. + */ +#define MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE /* \} name SECTION: Customisation configuration options */ diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 78ee6e2aa..a27f8c549 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -651,7 +651,7 @@ int mbedtls_x509write_crt_pem( mbedtls_x509write_cert *ctx, unsigned char *buf, } #endif -#ifndef MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 +#ifndef MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES /* The test infrastructure requires a positive define */ #define MBEDTLS_X509__DEFAULT_FORBID_SHA1 #endif diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d5510472d..ddedabb96 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7162,7 +7162,7 @@ static int ssl_preset_default_hashes[] = { MBEDTLS_MD_SHA256, MBEDTLS_MD_SHA224, #endif -#if defined(MBEDTLS_SHA1_C) && defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1) +#if defined(MBEDTLS_SHA1_C) && defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_KEY_EXCHANGE) MBEDTLS_MD_SHA1, #endif MBEDTLS_MD_NONE diff --git a/library/x509_crt.c b/library/x509_crt.c index 4de9e85d7..d86857de8 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -85,7 +85,7 @@ static void mbedtls_zeroize( void *v, size_t n ) { */ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default = { -#if defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1) +#if defined(MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES) /* Allow SHA-1 (weak, but still safe in controlled environments) */ MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA1 ) | #endif diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f9fbfa2db..61c382742 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2908,12 +2908,19 @@ run_test "Per-version suites: TLS 1.2" \ # Test for ClientHello without extensions requires_gnutls -run_test "ClientHello without extensions" \ +run_test "ClientHello without extensions, SHA-1 allowed" \ "$P_SRV debug_level=3" \ "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION" \ 0 \ -s "dumping 'client hello extensions' (0 bytes)" +requires_gnutls +run_test "ClientHello without extensions, SHA-1 forbidden in certificates on server" \ + "$P_SRV debug_level=3 key_file=data_files/server2.key crt_file=data_files/server2.crt allow_sha1=0" \ + "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION" \ + 0 \ + -s "dumping 'client hello extensions' (0 bytes)" + # Tests for mbedtls_ssl_get_bytes_avail() run_test "mbedtls_ssl_get_bytes_avail: no extra data" \ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index f8118b0a3..44a781608 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -432,7 +432,7 @@ depends_on:MBEDTLS_SHA1_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDT x509_verify:"data_files/cert_sha1.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"compat":"NULL" X509 Certificate verification #14 (Valid Cert SHA1 Digest allowed in compile-time default profile) -depends_on:MBEDTLS_SHA1_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 +depends_on:MBEDTLS_SHA1_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES x509_verify:"data_files/cert_sha1.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"default":"NULL" X509 Certificate verification #14 (Valid Cert SHA1 Digest forbidden in default profile)