From 4076d3e9f3b46318a9eb5be1175c7dce2a6e605c Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 1 Mar 2021 15:34:18 +0100 Subject: [PATCH 01/66] Implement one-shot MAC functions Implement one-shot MAC APIs, psa_mac_compute and psa_mac_verify, introduced in PSA Crypto API 1.0. Signed-off-by: gabor-mezei-arm --- library/psa_crypto.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7921eb2313..a6697f6f40 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2444,7 +2444,68 @@ cleanup: return( status == PSA_SUCCESS ? abort_status : status ); } +psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + const uint8_t *input, + size_t input_length, + uint8_t *mac, + size_t mac_size, + size_t *mac_length) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + status = psa_mac_sign_setup( &operation, key, alg ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_mac_update( &operation, input, input_length ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_mac_sign_finish( &operation, mac, mac_size, mac_length ); + if( status != PSA_SUCCESS ) + goto exit; + +exit: + if ( status == PSA_SUCCESS ) + status = psa_mac_abort( &operation ); + else + psa_mac_abort( &operation ); + + return ( status ); +} + +psa_status_t psa_mac_verify( mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + const uint8_t *input, + size_t input_length, + const uint8_t *mac, + size_t mac_length) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + + status = psa_mac_verify_setup( &operation, key, alg ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_mac_update( &operation, input, input_length ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_mac_verify_finish( &operation, mac, mac_length ); + if( status != PSA_SUCCESS ) + goto exit; + +exit: + if ( status == PSA_SUCCESS ) + status = psa_mac_abort( &operation ); + else + psa_mac_abort( &operation ); + + return ( status ); +} /****************************************************************/ /* Asymmetric cryptography */ From 5f43f978f013381343d8084a5ce1ae633cf8d7e4 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 11:41:23 +0100 Subject: [PATCH 02/66] Removes tests from ssl-opt.sh Commit removes tests from ssl-opt.sh that were specific to MBEDTLS_SSL_TRUNCATED_HMAC extention. Signed-off-by: Thomas Daubney --- tests/ssl-opt.sh | 190 ----------------------------------------------- 1 file changed, 190 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index d1221112ae..89eece6f66 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1464,102 +1464,6 @@ run_test "DTLS: multiple records in same datagram, neither client nor server" -S "next record in same datagram" \ -C "next record in same datagram" -# Tests for Truncated HMAC extension - -run_test "Truncated HMAC: client default, server default" \ - "$P_SRV debug_level=4" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC: client disabled, server default" \ - "$P_SRV debug_level=4" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=0" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC: client enabled, server default" \ - "$P_SRV debug_level=4" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC: client enabled, server disabled" \ - "$P_SRV debug_level=4 trunc_hmac=0" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC: client disabled, server enabled" \ - "$P_SRV debug_level=4 trunc_hmac=1" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=0" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC: client enabled, server enabled" \ - "$P_SRV debug_level=4 trunc_hmac=1" \ - "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -S "dumping 'expected mac' (20 bytes)" \ - -s "dumping 'expected mac' (10 bytes)" - -run_test "Truncated HMAC, DTLS: client default, server default" \ - "$P_SRV dtls=1 debug_level=4" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC, DTLS: client disabled, server default" \ - "$P_SRV dtls=1 debug_level=4" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=0" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC, DTLS: client enabled, server default" \ - "$P_SRV dtls=1 debug_level=4" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC, DTLS: client enabled, server disabled" \ - "$P_SRV dtls=1 debug_level=4 trunc_hmac=0" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC, DTLS: client disabled, server enabled" \ - "$P_SRV dtls=1 debug_level=4 trunc_hmac=1" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=0" \ - 0 \ - -s "dumping 'expected mac' (20 bytes)" \ - -S "dumping 'expected mac' (10 bytes)" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Truncated HMAC, DTLS: client enabled, server enabled" \ - "$P_SRV dtls=1 debug_level=4 trunc_hmac=1" \ - "$P_CLI dtls=1 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA trunc_hmac=1" \ - 0 \ - -S "dumping 'expected mac' (20 bytes)" \ - -s "dumping 'expected mac' (10 bytes)" - # Tests for Context serialization requires_config_enabled MBEDTLS_SSL_CONTEXT_SERIALIZATION @@ -5660,22 +5564,6 @@ run_test "Small client packet TLS 1.2 BlockCipher larger MAC" \ 0 \ -s "Read from client: 1 bytes read" -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small client packet TLS 1.2 BlockCipher, truncated MAC" \ - "$P_SRV trunc_hmac=1" \ - "$P_CLI request_size=1 force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "Read from client: 1 bytes read" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small client packet TLS 1.2 BlockCipher, without EtM, truncated MAC" \ - "$P_SRV trunc_hmac=1" \ - "$P_CLI request_size=1 force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1 etm=0" \ - 0 \ - -s "Read from client: 1 bytes read" - run_test "Small client packet TLS 1.2 AEAD" \ "$P_SRV" \ "$P_CLI request_size=1 force_version=tls1_2 \ @@ -5708,24 +5596,6 @@ run_test "Small client packet DTLS 1.2, without EtM" \ 0 \ -s "Read from client: 1 bytes read" -requires_config_enabled MBEDTLS_SSL_PROTO_DTLS -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small client packet DTLS 1.2, truncated hmac" \ - "$P_SRV dtls=1 force_version=dtls1_2 trunc_hmac=1" \ - "$P_CLI dtls=1 request_size=1 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "Read from client: 1 bytes read" - -requires_config_enabled MBEDTLS_SSL_PROTO_DTLS -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small client packet DTLS 1.2, without EtM, truncated MAC" \ - "$P_SRV dtls=1 force_version=dtls1_2 trunc_hmac=1 etm=0" \ - "$P_CLI dtls=1 request_size=1 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1"\ - 0 \ - -s "Read from client: 1 bytes read" - # Tests for small server packets run_test "Small server packet TLS 1.2 BlockCipher" \ @@ -5749,22 +5619,6 @@ run_test "Small server packet TLS 1.2 BlockCipher larger MAC" \ 0 \ -c "Read from server: 1 bytes read" -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small server packet TLS 1.2 BlockCipher, truncated MAC" \ - "$P_SRV response_size=1 trunc_hmac=1" \ - "$P_CLI force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1" \ - 0 \ - -c "Read from server: 1 bytes read" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small server packet TLS 1.2 BlockCipher, without EtM, truncated MAC" \ - "$P_SRV response_size=1 trunc_hmac=1" \ - "$P_CLI force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1 etm=0" \ - 0 \ - -c "Read from server: 1 bytes read" - run_test "Small server packet TLS 1.2 AEAD" \ "$P_SRV response_size=1" \ "$P_CLI force_version=tls1_2 \ @@ -5797,24 +5651,6 @@ run_test "Small server packet DTLS 1.2, without EtM" \ 0 \ -c "Read from server: 1 bytes read" -requires_config_enabled MBEDTLS_SSL_PROTO_DTLS -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small server packet DTLS 1.2, truncated hmac" \ - "$P_SRV dtls=1 response_size=1 force_version=dtls1_2 trunc_hmac=1" \ - "$P_CLI dtls=1 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1" \ - 0 \ - -c "Read from server: 1 bytes read" - -requires_config_enabled MBEDTLS_SSL_PROTO_DTLS -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Small server packet DTLS 1.2, without EtM, truncated MAC" \ - "$P_SRV dtls=1 response_size=1 force_version=dtls1_2 trunc_hmac=1 etm=0" \ - "$P_CLI dtls=1 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1"\ - 0 \ - -c "Read from server: 1 bytes read" - # Test for large client packets # How many fragments do we expect to write $1 bytes? @@ -5845,23 +5681,6 @@ run_test "Large client packet TLS 1.2 BlockCipher larger MAC" \ -c "16384 bytes written in $(fragments_for_write 16384) fragments" \ -s "Read from client: $MAX_CONTENT_LEN bytes read" -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Large client packet TLS 1.2 BlockCipher, truncated MAC" \ - "$P_SRV trunc_hmac=1" \ - "$P_CLI request_size=16384 force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1" \ - 0 \ - -s "Read from client: $MAX_CONTENT_LEN bytes read" - -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Large client packet TLS 1.2 BlockCipher, without EtM, truncated MAC" \ - "$P_SRV trunc_hmac=1" \ - "$P_CLI request_size=16384 force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA trunc_hmac=1 etm=0" \ - 0 \ - -c "16384 bytes written in $(fragments_for_write 16384) fragments" \ - -s "Read from client: $MAX_CONTENT_LEN bytes read" - run_test "Large client packet TLS 1.2 AEAD" \ "$P_SRV" \ "$P_CLI request_size=16384 force_version=tls1_2 \ @@ -5900,15 +5719,6 @@ run_test "Large server packet TLS 1.2 BlockCipher larger MAC" \ 0 \ -c "Read from server: 16384 bytes read" -requires_config_enabled MBEDTLS_SSL_TRUNCATED_HMAC -run_test "Large server packet TLS 1.2 BlockCipher truncated MAC" \ - "$P_SRV response_size=16384" \ - "$P_CLI force_version=tls1_2 \ - force_ciphersuite=TLS-RSA-WITH-AES-256-CBC-SHA \ - trunc_hmac=1" \ - 0 \ - -c "Read from server: 16384 bytes read" - run_test "Large server packet TLS 1.2 BlockCipher, without EtM, truncated MAC" \ "$P_SRV response_size=16384 trunc_hmac=1" \ "$P_CLI force_version=tls1_2 \ From ce9e716a2bdc411df9d4bbf74d384d7013b527a6 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 14:04:25 +0100 Subject: [PATCH 03/66] Modifies tests in context-info.sh Commit modifies tests involving MBEDTLS_SSL_TRUNCATED_HMAC in the context-info.sh script. Signed-off-by: Thomas Daubney --- tests/context-info.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/context-info.sh b/tests/context-info.sh index e02d330841..88dfcaa5ea 100755 --- a/tests/context-info.sh +++ b/tests/context-info.sh @@ -210,7 +210,6 @@ run_test "Default configuration, server" \ -u "MBEDTLS_HAVE_TIME$" \ -u "MBEDTLS_X509_CRT_PARSE_C$" \ -u "MBEDTLS_SSL_MAX_FRAGMENT_LENGTH$" \ - -u "MBEDTLS_SSL_TRUNCATED_HMAC$" \ -u "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -u "MBEDTLS_SSL_SESSION_TICKETS$" \ -u "MBEDTLS_SSL_SESSION_TICKETS and client$" \ @@ -233,7 +232,6 @@ run_test "Default configuration, client" \ -u "MBEDTLS_HAVE_TIME$" \ -u "MBEDTLS_X509_CRT_PARSE_C$" \ -u "MBEDTLS_SSL_MAX_FRAGMENT_LENGTH$" \ - -u "MBEDTLS_SSL_TRUNCATED_HMAC$" \ -u "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -u "MBEDTLS_SSL_SESSION_TICKETS$" \ -u "MBEDTLS_SSL_SESSION_TICKETS and client$" \ @@ -339,7 +337,6 @@ run_test "Minimal configuration, server" \ "srv_min_cfg.txt" \ -n "ERROR" \ -n "MBEDTLS_SSL_MAX_FRAGMENT_LENGTH$" \ - -n "MBEDTLS_SSL_TRUNCATED_HMAC$" \ -n "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -n "MBEDTLS_SSL_SESSION_TICKETS$" \ -n "MBEDTLS_SSL_SESSION_TICKETS and client$" \ @@ -350,7 +347,6 @@ run_test "Minimal configuration, client" \ "cli_min_cfg.txt" \ -n "ERROR" \ -n "MBEDTLS_SSL_MAX_FRAGMENT_LENGTH$" \ - -n "MBEDTLS_SSL_TRUNCATED_HMAC$" \ -n "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -n "MBEDTLS_SSL_SESSION_TICKETS$" \ -n "MBEDTLS_SSL_SESSION_TICKETS and client$" \ From c46bf3c79c2a23d2f67f1a2d01dd980416f3286f Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 14:15:21 +0100 Subject: [PATCH 04/66] Modifies tests in test_suite_ssl.function Commit removes conditional compilation code blocks relating to MBEDTLS_SSL_TRUNCATED_HMAC config option. Signed-off-by: Thomas Daubney --- tests/suites/test_suite_ssl.function | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index c555d74a20..e175db277c 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1504,9 +1504,6 @@ static int ssl_populate_session( mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) session->mfl_code = 1; #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - session->trunc_hmac = 1; -#endif #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) session->encrypt_then_mac = 1; #endif @@ -4078,10 +4075,6 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file ) TEST_ASSERT( original.mfl_code == restored.mfl_code ); #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - TEST_ASSERT( original.trunc_hmac == restored.trunc_hmac ); -#endif - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) TEST_ASSERT( original.encrypt_then_mac == restored.encrypt_then_mac ); #endif From 22989d027a00f5c3a21dc2387fb6b4a559f127c9 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 15:34:28 +0100 Subject: [PATCH 05/66] Removes MBEDTLS_SSL_TRUNCATED_HMAC code from ssl programs Commit removes code dependent on MBEDTLS_SSL_TRUNCATED_HMAC from SSL client and sever example programs. Signed-off-by: Thomas Daubney --- programs/ssl/ssl_client2.c | 13 ------------- programs/ssl/ssl_server2.c | 13 ------------- 2 files changed, 26 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 98a3048685..ce9bcc302e 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -235,13 +235,6 @@ int main( void ) #define USAGE_SRTP "" #endif /* MBEDTLS_SSL_EXPORT_KEYS */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -#define USAGE_TRUNC_HMAC \ - " trunc_hmac=%%d default: library default\n" -#else -#define USAGE_TRUNC_HMAC "" -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) #define USAGE_MAX_FRAG_LEN \ " max_frag_len=%%d default: 16384 (tls default)\n" \ @@ -394,7 +387,6 @@ int main( void ) USAGE_TICKETS \ USAGE_EAP_TLS \ USAGE_MAX_FRAG_LEN \ - USAGE_TRUNC_HMAC \ USAGE_CONTEXT_CRT_CB \ USAGE_ALPN \ USAGE_EMS \ @@ -1721,11 +1713,6 @@ int main( int argc, char *argv[] ) } #endif /* MBEDTLS_SSL_DTLS_SRTP */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - if( opt.trunc_hmac != DFL_TRUNC_HMAC ) - mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac ); -#endif - #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( opt.extended_ms != DFL_EXTENDED_MS ) mbedtls_ssl_conf_extended_master_secret( &conf, opt.extended_ms ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index de4eb6d87e..f0ca72f2ca 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -334,13 +334,6 @@ int main( void ) #define USAGE_MAX_FRAG_LEN "" #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -#define USAGE_TRUNC_HMAC \ - " trunc_hmac=%%d default: library default\n" -#else -#define USAGE_TRUNC_HMAC "" -#endif - #if defined(MBEDTLS_SSL_ALPN) #define USAGE_ALPN \ " alpn=%%s default: \"\" (disabled)\n" \ @@ -487,7 +480,6 @@ int main( void ) USAGE_NSS_KEYLOG_FILE \ USAGE_CACHE \ USAGE_MAX_FRAG_LEN \ - USAGE_TRUNC_HMAC \ USAGE_ALPN \ USAGE_EMS \ USAGE_ETM \ @@ -2506,11 +2498,6 @@ int main( int argc, char *argv[] ) } #endif /* MBEDTLS_SSL_DTLS_SRTP */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - if( opt.trunc_hmac != DFL_TRUNC_HMAC ) - mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac ); -#endif - #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( opt.extended_ms != DFL_EXTENDED_MS ) mbedtls_ssl_conf_extended_master_secret( &conf, opt.extended_ms ); From 909d3bfa432c2c26834d9e9ba3e9373df0eddc65 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 15:37:45 +0100 Subject: [PATCH 06/66] Removes MBEDTLS_SSL_TRUNCATED_HMAC code from fuzz programs Commit removes conditional compilation code blocks dependent on MBEDTLS_SSL_TRUNCATED_HMAC config option. Signed-off-by: Thomas Daubney --- programs/fuzz/fuzz_client.c | 3 --- programs/fuzz/fuzz_server.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/programs/fuzz/fuzz_client.c b/programs/fuzz/fuzz_client.c index cbd9483f8e..ab2d134948 100644 --- a/programs/fuzz/fuzz_client.c +++ b/programs/fuzz/fuzz_client.c @@ -102,9 +102,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { { mbedtls_ssl_conf_authmode( &conf, MBEDTLS_SSL_VERIFY_NONE ); } -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - mbedtls_ssl_conf_truncated_hmac( &conf, (options & 8) ? MBEDTLS_SSL_TRUNC_HMAC_ENABLED : MBEDTLS_SSL_TRUNC_HMAC_DISABLED); -#endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) mbedtls_ssl_conf_extended_master_secret( &conf, (options & 0x10) ? MBEDTLS_SSL_EXTENDED_MS_DISABLED : MBEDTLS_SSL_EXTENDED_MS_ENABLED); #endif diff --git a/programs/fuzz/fuzz_server.c b/programs/fuzz/fuzz_server.c index 5480e3e876..e3bee2b65a 100644 --- a/programs/fuzz/fuzz_server.c +++ b/programs/fuzz/fuzz_server.c @@ -127,9 +127,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { &ticket_ctx ); } #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - mbedtls_ssl_conf_truncated_hmac( &conf, (options & 0x8) ? MBEDTLS_SSL_TRUNC_HMAC_ENABLED : MBEDTLS_SSL_TRUNC_HMAC_DISABLED); -#endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) mbedtls_ssl_conf_extended_master_secret( &conf, (options & 0x10) ? MBEDTLS_SSL_EXTENDED_MS_DISABLED : MBEDTLS_SSL_EXTENDED_MS_ENABLED); #endif From 40d49b1e54c7447354e498af5dd22ab1e7880ad5 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 15:46:45 +0100 Subject: [PATCH 07/66] Removes truncated HMAC code from ssl_context_info program Commit removes conditional compilation block which depends on MBEDTLS_SSL_TRUNCATED_HMAC config option. Signed-off-by: Thomas Daubney --- programs/ssl/ssl_context_info.c | 1 - 1 file changed, 1 deletion(-) diff --git a/programs/ssl/ssl_context_info.c b/programs/ssl/ssl_context_info.c index b360991152..a8a815235d 100644 --- a/programs/ssl/ssl_context_info.c +++ b/programs/ssl/ssl_context_info.c @@ -868,7 +868,6 @@ void print_deserialized_ssl_context( const uint8_t *ssl, size_t len ) print_if_bit( "MBEDTLS_HAVE_TIME", SESSION_CONFIG_TIME_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_X509_CRT_PARSE_C", SESSION_CONFIG_CRT_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_SSL_MAX_FRAGMENT_LENGTH", SESSION_CONFIG_MFL_BIT, session_cfg_flag ); - print_if_bit( "MBEDTLS_SSL_TRUNCATED_HMAC", SESSION_CONFIG_TRUNC_HMAC_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_SSL_ENCRYPT_THEN_MAC", SESSION_CONFIG_ETM_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_SSL_SESSION_TICKETS", SESSION_CONFIG_TICKET_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_SSL_SESSION_TICKETS and client", SESSION_CONFIG_CLIENT_TICKET_BIT, session_cfg_flag ); From 32fb900eee9b69abddbbe4a6115f0193858fffe6 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 14 Jun 2021 17:25:08 +0100 Subject: [PATCH 08/66] Removes truncated HMAC code from ssl_tls.c Removes conditional code compilation blocks and code paths relating to the MBEDTLS_SSL_TRUNCATED_HMAC config option. Signed-off-by: Thomas Daubney --- library/ssl_tls.c | 46 ---------------------------------------------- 1 file changed, 46 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5c1bc32078..eb5d9db1ce 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -672,9 +672,6 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) int encrypt_then_mac, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - int trunc_hmac, -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */ ssl_tls_prf_t tls_prf, const unsigned char randbytes[64], @@ -845,18 +842,6 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, mac_key_len = mbedtls_md_get_size( md_info ); transform->maclen = mac_key_len; -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - /* - * If HMAC is to be truncated, we shall keep the leftmost bytes, - * (rfc 6066 page 13 or rfc 2104 section 4), - * so we only need to adjust the length here. - */ - if( trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_ENABLED ) - { - transform->maclen = MBEDTLS_SSL_TRUNCATED_HMAC_LEN; - } -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - /* IV length */ transform->ivlen = cipher_info->iv_size; @@ -1368,9 +1353,6 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) ssl->session_negotiate->encrypt_then_mac, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - ssl->session_negotiate->trunc_hmac, -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */ ssl->handshake->tls_prf, ssl->handshake->randbytes, @@ -4138,13 +4120,6 @@ int mbedtls_ssl_conf_max_frag_len( mbedtls_ssl_config *conf, unsigned char mfl_c } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -void mbedtls_ssl_conf_truncated_hmac( mbedtls_ssl_config *conf, int truncate ) -{ - conf->trunc_hmac = truncate; -} -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - void mbedtls_ssl_conf_legacy_renegotiation( mbedtls_ssl_config *conf, int allow_legacy ) { conf->allow_legacy_renegotiation = allow_legacy; @@ -4519,11 +4494,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_co #define SSL_SERIALIZED_SESSION_CONFIG_MFL 0 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -#define SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC 1 -#else #define SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC 0 -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) #define SSL_SERIALIZED_SESSION_CONFIG_ETM 1 @@ -4766,13 +4737,6 @@ static int ssl_session_save( const mbedtls_ssl_session *session, *p++ = session->mfl_code; #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - used += 1; - - if( used <= buf_len ) - *p++ = (unsigned char)( ( session->trunc_hmac ) & 0xFF ); -#endif - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) used += 1; @@ -5008,13 +4972,6 @@ static int ssl_session_load( mbedtls_ssl_session *session, session->mfl_code = *p++; #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - if( 1 > (size_t)( end - p ) ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - - session->trunc_hmac = *p++; -#endif - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) if( 1 > (size_t)( end - p ) ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -5831,9 +5788,6 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) ssl->session->encrypt_then_mac, #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - ssl->session->trunc_hmac, -#endif #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */ ssl_tls12prf_from_cs( ssl->session->ciphersuite ), p, /* currently pointing to randbytes */ From e1c9a40bc41e8b868847879b111cce447419a882 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 15 Jun 2021 11:26:43 +0100 Subject: [PATCH 09/66] Removes truncated HMAC code from ssl_X.c Removes conditional code blocks relating to MBEDTLS_SSL_TRUNCATED_HMAC from ssl_cli.c and ssl_srv.c. Signed-off-by: Thomas Daubney --- library/ssl_cli.c | 78 ----------------------------------------------- library/ssl_srv.c | 64 +------------------------------------- 2 files changed, 1 insertion(+), 141 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9a441385d2..30e64c4843 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -559,36 +559,6 @@ static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -static int ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *olen ) -{ - unsigned char *p = buf; - - *olen = 0; - - if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ) - return( 0 ); - - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "client hello, adding truncated_hmac extension" ) ); - - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); - - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC ) & 0xFF ); - - *p++ = 0x00; - *p++ = 0x00; - - *olen = 4; - - return( 0 ); -} -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -1304,16 +1274,6 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ext_len += olen; #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - if( ( ret = ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_truncated_hmac_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) if( ( ret = ssl_write_encrypt_then_mac_ext( ssl, p + 2 + ext_len, end, &olen ) ) != 0 ) @@ -1479,31 +1439,6 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t len ) -{ - if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED || - len != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "non-matching truncated HMAC extension" ) ); - mbedtls_ssl_send_alert_message( - ssl, - MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); - return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); - } - - ((void) buf); - - ssl->session_negotiate->trunc_hmac = MBEDTLS_SSL_TRUNC_HMAC_ENABLED; - - return( 0 ); -} -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, @@ -2346,19 +2281,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) break; #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - case MBEDTLS_TLS_EXT_TRUNCATED_HMAC: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found truncated_hmac extension" ) ); - - if( ( ret = ssl_parse_truncated_hmac_ext( ssl, - ext + 4, ext_size ) ) != 0 ) - { - return( ret ); - } - - break; -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) case MBEDTLS_TLS_EXT_CID: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 29569d176e..47151298d4 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -543,28 +543,6 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t len ) -{ - if( len != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); - } - - ((void) buf); - - if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_ENABLED ) - ssl->session_negotiate->trunc_hmac = MBEDTLS_SSL_TRUNC_HMAC_ENABLED; - - return( 0 ); -} -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, @@ -1703,16 +1681,6 @@ read_record_header: break; #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - case MBEDTLS_TLS_EXT_TRUNCATED_HMAC: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found truncated hmac extension" ) ); - - ret = ssl_parse_truncated_hmac_ext( ssl, ext + 4, ext_size ); - if( ret != 0 ) - return( ret ); - break; -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) case MBEDTLS_TLS_EXT_CID: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) ); @@ -1721,7 +1689,7 @@ read_record_header: if( ret != 0 ) return( ret ); break; -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ +#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) case MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC: @@ -1967,31 +1935,6 @@ have_ciphersuite: return( 0 ); } -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) -{ - unsigned char *p = buf; - - if( ssl->session_negotiate->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ) - { - *olen = 0; - return; - } - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, adding truncated hmac extension" ) ); - - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC ) & 0xFF ); - - *p++ = 0x00; - *p++ = 0x00; - - *olen = 4; -} -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) static void ssl_write_cid_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -2654,11 +2597,6 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) ext_len += olen; #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len, &olen ); - ext_len += olen; -#endif - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) ssl_write_cid_ext( ssl, p + 2 + ext_len, &olen ); ext_len += olen; From d7171e9f59b5648b2ae9d0e96f84022fd7e5bc3b Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 15 Jun 2021 12:43:45 +0100 Subject: [PATCH 10/66] Removes truncated HMAC code from ssl.h Commit removes conditionally compiled code relating to MBEDTLS_SSL_TRUNCATED_HMAC from ssl.h. Signed-off-by: Thomas Daubney --- include/mbedtls/ssl.h | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b2f5c67a2d..a93a32588e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -956,10 +956,6 @@ struct mbedtls_ssl_session unsigned char MBEDTLS_PRIVATE(mfl_code); /*!< MaxFragmentLength negotiated by peer */ #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - int MBEDTLS_PRIVATE(trunc_hmac); /*!< flag for truncated hmac activation */ -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) int MBEDTLS_PRIVATE(encrypt_then_mac); /*!< flag for EtM activation */ #endif @@ -1180,9 +1176,6 @@ struct mbedtls_ssl_config #if defined(MBEDTLS_SSL_RENEGOTIATION) unsigned int MBEDTLS_PRIVATE(disable_renegotiation) : 1; /*!< disable renegotiation? */ #endif -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - unsigned int MBEDTLS_PRIVATE(trunc_hmac) : 1; /*!< negotiate truncated hmac? */ -#endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) unsigned int MBEDTLS_PRIVATE(session_tickets) : 1; /*!< use session tickets? */ #endif @@ -3315,18 +3308,6 @@ int mbedtls_ssl_conf_max_frag_len( mbedtls_ssl_config *conf, unsigned char mfl_c void mbedtls_ssl_conf_preference_order( mbedtls_ssl_config *conf, int order ); #endif /* MBEDTLS_SSL_SRV_C */ -#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -/** - * \brief Activate negotiation of truncated HMAC - * (Default: MBEDTLS_SSL_TRUNC_HMAC_DISABLED) - * - * \param conf SSL configuration - * \param truncate Enable or disable (MBEDTLS_SSL_TRUNC_HMAC_ENABLED or - * MBEDTLS_SSL_TRUNC_HMAC_DISABLED) - */ -void mbedtls_ssl_conf_truncated_hmac( mbedtls_ssl_config *conf, int truncate ); -#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ - #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) /** * \brief Enable / Disable session tickets (client only). From 4a7010d1aaae937dfa328ef253d70d04b3b260ff Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 15 Jun 2021 12:54:14 +0100 Subject: [PATCH 11/66] Removes MBEDTLS_SSL_TRUNCATED_HMAC config option Commit removes the MBEDTLS_SSL_TRUNCATED_HMAC config option from config.h and places a check that it is unset in check_config.h. Signed-off-by: Thomas Daubney --- include/mbedtls/check_config.h | 4 ++++ include/mbedtls/config.h | 9 --------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 90dee6c1af..b0d61a3fa7 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -837,6 +837,10 @@ #error "MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY was removed in Mbed TLS 3.0. See https://github.com/ARMmbed/mbedtls/issues/4335" #endif +#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) //no-check-names +#error "MBEDTLS_SSL_TRUNCATED_HMAC was removed in Mbed TLS 3.0. See https://github.com/ARMmbed/mbedtls/issues/4341" +#endif + /* * Avoid warning from -pedantic. This is a convenient place for this * workaround since this is included by every single file before the diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 16f8f8b350..2b4976de70 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1648,15 +1648,6 @@ */ #define MBEDTLS_SSL_SERVER_NAME_INDICATION -/** - * \def MBEDTLS_SSL_TRUNCATED_HMAC - * - * Enable support for RFC 6066 truncated HMAC in SSL. - * - * Comment this macro to disable support for truncated HMAC in SSL - */ -#define MBEDTLS_SSL_TRUNCATED_HMAC - /** * \def MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH * From 22ecf49e9a577cb3ad3a64dbc337b26917039a3d Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 15 Jun 2021 13:04:11 +0100 Subject: [PATCH 12/66] Adds ChangeLog entry Commit adds ChangeLog entry for removal of MBEDTLS_SSL_TRUNCATED_HMAC. Signed-off-by: Thomas Daubney --- ChangeLog.d/rm-truncated-hmac-ext.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/rm-truncated-hmac-ext.txt diff --git a/ChangeLog.d/rm-truncated-hmac-ext.txt b/ChangeLog.d/rm-truncated-hmac-ext.txt new file mode 100644 index 0000000000..e3391f885d --- /dev/null +++ b/ChangeLog.d/rm-truncated-hmac-ext.txt @@ -0,0 +1,3 @@ +Removals + * Remove MBEDTLS_SSL_TRUNCATED_HMAC config option since it is no longer + supported in 3.0. Addresses #4341. From 50afb4378f1282eaf3d62a579016787add81e2ec Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 17 Jun 2021 09:23:41 +0100 Subject: [PATCH 13/66] Adds Migration guide Commit adds a migraiton guide entry that was missing. Signed-off-by: Thomas Daubney --- .../remove-truncated-HMAC-extension.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md diff --git a/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md b/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md new file mode 100644 index 0000000000..12f7c2385e --- /dev/null +++ b/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md @@ -0,0 +1,10 @@ +Remove the truncated HMAC extension +----------------------------------- + +This affects all users who use the truncated HMAC extension for cryptographic +operations. + +The config option `MBEDTLS_SSL_TRUNCATED_HMAC` has been removed. Users concerned +about overhead are better served by using any of the CCM-8 ciphersuites rather +than a CBC ciphersuite with truncated HMAC, and so going forward this must be +the approach taken. From ffb92da622122c8d1f6dd9cd626ed3b098ccf181 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Jun 2021 00:03:26 +0200 Subject: [PATCH 14/66] 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 d383168d25..9c7016d8e9 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 d4e0ffd404..59692476bc 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 15/66] 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 16f8f8b350..fc10f9d703 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 b2f5c67a2d..df3974aea8 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 044bbe1d10..6bb955cc0f 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 5c1bc32078..be389f03b6 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 59692476bc..abd70e1e40 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 16/66] 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 0000000000..1a805623dc --- /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 0000000000..5db517ebd2 --- /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 17/66] 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 abd70e1e40..650044583c 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 18/66] 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 d1221112ae..5ccb467b15 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 19/66] 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 0000000000..909d4e8a42 --- /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 fc10f9d703..0c7e4ec063 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 49e85d9419..1c271c913d 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 20/66] 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 1c271c913d..dd3988eec8 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 21/66] 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 1a805623dc..bfb0fd0e03 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 5db517ebd2..5879d77a91 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 df3974aea8..d885d213ee 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 be389f03b6..07569b240e 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 22/66] 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 d885d213ee..b38bd72e00 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 07569b240e..3bbdcb086b 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 23/66] 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 0935c12448..f1d29ef7d7 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 24/66] 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 f1d29ef7d7..4a1a1be481 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 ad50e53fd1..fda6939871 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 25/66] 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 0000000000..d1b4f5b415 --- /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 4be2cba195..e520258519 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 26/66] 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 5ccb467b15..81cb774e7a 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 27/66] 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 e520258519..618d124e52 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 28/66] 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 81cb774e7a..d3f6d324b8 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 29/66] 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 618d124e52..fa60d13f30 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 30/66] 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 5879d77a91..825609e431 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 31/66] 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 825609e431..551e28721c 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 a4a59b8895..2ae74c910d 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 32/66] 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 b38bd72e00..4000369199 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 33/66] 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 551e28721c..928130d247 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 9c7016d8e9..f991251ca6 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 3bbdcb086b..2688af173e 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 650044583c..f12ac6b7e0 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 From a42bf29b2c125f10a01fad34f2e287ed44c39aa8 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 18 Jun 2021 09:13:53 +0100 Subject: [PATCH 34/66] Modifies ChangeLog entry Corrects wording in ChangeLog entry as requested in review. Signed-off-by: Thomas Daubney --- ChangeLog.d/rm-truncated-hmac-ext.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/rm-truncated-hmac-ext.txt b/ChangeLog.d/rm-truncated-hmac-ext.txt index e3391f885d..c82415ba7e 100644 --- a/ChangeLog.d/rm-truncated-hmac-ext.txt +++ b/ChangeLog.d/rm-truncated-hmac-ext.txt @@ -1,3 +1,4 @@ Removals - * Remove MBEDTLS_SSL_TRUNCATED_HMAC config option since it is no longer - supported in 3.0. Addresses #4341. + * Remove MBEDTLS_SSL_TRUNCATED_HMAC config option. Users are better served by + using a CCM-8 ciphersuite than a CBC ciphersuite with truncated HMAC. + Addresses #4341. From 379227cc596d8dbb293c8056e7c6ed9abe9724be Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 18 Jun 2021 10:46:12 +0100 Subject: [PATCH 35/66] Modifies ChangeLog and Migration Guide Entries in ChangeLog and Migration guide files have been merged to cover both the removal of MBEDTLS_SSL_TRUNCATED_HMAC and MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT. Signed-off-by: Thomas Daubney --- ChangeLog | 12 ------------ ChangeLog.d/rm-truncated-hmac-ext.txt | 5 +++-- .../remove-truncated-HMAC-extension.md | 10 ---------- docs/3.0-migration-guide.md | 16 ++++++---------- 4 files changed, 9 insertions(+), 34 deletions(-) delete mode 100644 docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md diff --git a/ChangeLog b/ChangeLog index ddaf3fd9f7..b525492fb9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -32,8 +32,6 @@ API changes * Drop support for parsing SSLv2 ClientHello (MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO). * Drop support for SSLv3 (MBEDTLS_SSL_PROTO_SSL3). - * Drop support for compatibility with our own previous buggy - implementation of truncated HMAC (MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT). * Drop support for TLS record-level compression (MBEDTLS_ZLIB_SUPPORT). * Drop support for RC4 TLS ciphersuites. * Drop support for single-DES ciphersuites. @@ -1688,16 +1686,6 @@ Changes = mbed TLS 2.8.0 branch released 2018-03-16 -Default behavior changes - * The truncated HMAC extension now conforms to RFC 6066. This means - that when both sides of a TLS connection negotiate the truncated - HMAC extension, Mbed TLS can now interoperate with other - compliant implementations, but this breaks interoperability with - prior versions of Mbed TLS. To restore the old behavior, enable - the (deprecated) option MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT in - config.h. Found by Andreas Walz (ivESK, Offenburg University of - Applied Sciences). - Security * Fix implementation of the truncated HMAC extension. The previous implementation allowed an offline 2^80 brute force attack on the diff --git a/ChangeLog.d/rm-truncated-hmac-ext.txt b/ChangeLog.d/rm-truncated-hmac-ext.txt index c82415ba7e..3739256957 100644 --- a/ChangeLog.d/rm-truncated-hmac-ext.txt +++ b/ChangeLog.d/rm-truncated-hmac-ext.txt @@ -1,4 +1,5 @@ Removals - * Remove MBEDTLS_SSL_TRUNCATED_HMAC config option. Users are better served by + * Remove MBEDTLS_SSL_TRUNCATED_HMAC and also remove + MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT config option. Users are better served by using a CCM-8 ciphersuite than a CBC ciphersuite with truncated HMAC. - Addresses #4341. + See issue #4341 for more details. diff --git a/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md b/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md deleted file mode 100644 index 12f7c2385e..0000000000 --- a/docs/3.0-migration-guide.d/remove-truncated-HMAC-extension.md +++ /dev/null @@ -1,10 +0,0 @@ -Remove the truncated HMAC extension ------------------------------------ - -This affects all users who use the truncated HMAC extension for cryptographic -operations. - -The config option `MBEDTLS_SSL_TRUNCATED_HMAC` has been removed. Users concerned -about overhead are better served by using any of the CCM-8 ciphersuites rather -than a CBC ciphersuite with truncated HMAC, and so going forward this must be -the approach taken. diff --git a/docs/3.0-migration-guide.md b/docs/3.0-migration-guide.md index a4a59b8895..a30e786771 100644 --- a/docs/3.0-migration-guide.md +++ b/docs/3.0-migration-guide.md @@ -137,17 +137,13 @@ and relied on that version in order to communicate with peers that are not up to date. If one of your peers is in that case, please try contacting them and encouraging them to upgrade their software. -Remove support for compatibility with old Mbed TLS's truncated HMAC -------------------------------------------------------------------- +Remove support for truncated HMAC +--------------------------------- -This doesn't affect people using the default configuration as it was already -disabled by default. - -This only affects TLS users who enabled `MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT` and -used the Truncated HMAC extension to communicate with peers using old version -of Mbed TLS. Please consider using a CCM-8 ciphersuite instead of the -Truncated HMAC extension, or convincing your peer to upgrade their version of -Mbed TLS. +This only affects TLS users who enabled `MBEDTLS_SSL_TRUNCATED_HMAC` +`MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT` and used the truncated HMAC extension. +Please consider using a CCM-8 ciphersuite instead of the Truncated HMAC +extension, or convincing your peer to upgrade their version of Mbed TLS. Remove support for TLS record-level compression ----------------------------------------------- From d596e99d61608df51ac4323148482a373ef0b25d Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 18 Jun 2021 11:50:56 +0100 Subject: [PATCH 36/66] Modifies ChangeLog Corrects erroneous removal from ChangeLog. Signed-off-by: Thomas Daubney --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index b525492fb9..565ce1c788 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1686,6 +1686,16 @@ Changes = mbed TLS 2.8.0 branch released 2018-03-16 +Default behavior changes + * The truncated HMAC extension now conforms to RFC 6066. This means + that when both sides of a TLS connection negotiate the truncated + HMAC extension, Mbed TLS can now interoperate with other + compliant implementations, but this breaks interoperability with + prior versions of Mbed TLS. To restore the old behavior, enable + the (deprecated) option MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT in + config.h. Found by Andreas Walz (ivESK, Offenburg University of + Applied Sciences). + Security * Fix implementation of the truncated HMAC extension. The previous implementation allowed an offline 2^80 brute force attack on the From 87db8a2676e07b2b31a9cdd21b23e594503a28c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 18 Jun 2021 13:30:14 +0200 Subject: [PATCH 37/66] Clean up old files before generating them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1423099571..7974f9b9fb 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -651,6 +651,9 @@ pre_check_tools () { } pre_generate_files() { + # since make doesn't have proper dependencies, remove any possibly outdate + # file that might be around before generating fresh ones + make neat make generated_files } From ac84469dd1f93e285cc571c0202b14b63a42ebfc Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 18 Jun 2021 14:08:56 +0100 Subject: [PATCH 38/66] Modifies Migration Guide entry Commit makes corrections to Migration Guide entry for this task. Signed-off-by: Thomas Daubney --- docs/3.0-migration-guide.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/3.0-migration-guide.md b/docs/3.0-migration-guide.md index a30e786771..6a21a653a6 100644 --- a/docs/3.0-migration-guide.md +++ b/docs/3.0-migration-guide.md @@ -140,10 +140,13 @@ encouraging them to upgrade their software. Remove support for truncated HMAC --------------------------------- -This only affects TLS users who enabled `MBEDTLS_SSL_TRUNCATED_HMAC` -`MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT` and used the truncated HMAC extension. -Please consider using a CCM-8 ciphersuite instead of the Truncated HMAC -extension, or convincing your peer to upgrade their version of Mbed TLS. +This affects users of truncated HMAC, that is, users who called +`mbedtls_ssl_conf_truncated_hmac( ..., MBEDTLS_SSL_TRUNC_HMAC_ENABLED)`, +regardless of whether the standard version was used or compatibility version +(`MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT`). + +The recommended migration path for people who want minimal overhead is to use a +CCM-8 ciphersuite. Remove support for TLS record-level compression ----------------------------------------------- From 534bb99f17dac336b404c7db1b6512cbf5c40d7e Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Mon, 1 Mar 2021 15:35:48 +0100 Subject: [PATCH 39/66] Add test for one-shot MAC functions Tests for psa_mac_compute and psa_mac_verify functions. Signed-off-by: gabor-mezei-arm Signed-off-by: Ronald Cron --- tests/suites/test_suite_psa_crypto.function | 45 +++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 5b5531f037..10426302dd 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1984,7 +1984,21 @@ void mac_sign( int key_type_arg, mbedtls_test_set_step( output_size ); ASSERT_ALLOC( actual_mac, output_size ); - /* Calculate the MAC. */ + /* Calculate the MAC, one-shot case. */ + TEST_EQUAL( psa_mac_compute( key, alg, + input->x, input->len, + actual_mac, output_size, &mac_length ), + expected_status ); + if( expected_status == PSA_SUCCESS ) + { + ASSERT_COMPARE( expected_mac->x, expected_mac->len, + actual_mac, mac_length ); + } + + if( output_size > 0 ) + memset( actual_mac, 0, output_size ); + + /* Calculate the MAC, multi-part case. */ PSA_ASSERT( psa_mac_sign_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, input->x, input->len ) ); @@ -2036,7 +2050,11 @@ void mac_verify( int key_type_arg, PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len, &key ) ); - /* Test the correct MAC. */ + /* Verify correct MAC, one-shot case. */ + PSA_ASSERT( psa_mac_verify( key, alg, input->x, input->len, + expected_mac->x, expected_mac->len ) ); + + /* Verify correct MAC, multi-part case. */ PSA_ASSERT( psa_mac_verify_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, input->x, input->len ) ); @@ -2044,7 +2062,14 @@ void mac_verify( int key_type_arg, expected_mac->x, expected_mac->len ) ); - /* Test a MAC that's too short. */ + /* Test a MAC that's too short, one-shot case. */ + TEST_EQUAL( psa_mac_verify( key, alg, + input->x, input->len, + expected_mac->x, + expected_mac->len - 1 ), + PSA_ERROR_INVALID_SIGNATURE ); + + /* Test a MAC that's too short, multi-part case. */ PSA_ASSERT( psa_mac_verify_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, input->x, input->len ) ); @@ -2053,9 +2078,15 @@ void mac_verify( int key_type_arg, expected_mac->len - 1 ), PSA_ERROR_INVALID_SIGNATURE ); - /* Test a MAC that's too long. */ + /* Test a MAC that's too long, one-shot case. */ ASSERT_ALLOC( perturbed_mac, expected_mac->len + 1 ); memcpy( perturbed_mac, expected_mac->x, expected_mac->len ); + TEST_EQUAL( psa_mac_verify( key, alg, + input->x, input->len, + perturbed_mac, expected_mac->len + 1 ), + PSA_ERROR_INVALID_SIGNATURE ); + + /* Test a MAC that's too long, multi-part case. */ PSA_ASSERT( psa_mac_verify_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, input->x, input->len ) ); @@ -2069,6 +2100,12 @@ void mac_verify( int key_type_arg, { mbedtls_test_set_step( i ); perturbed_mac[i] ^= 1; + + TEST_EQUAL( psa_mac_verify( key, alg, + input->x, input->len, + perturbed_mac, expected_mac->len ), + PSA_ERROR_INVALID_SIGNATURE ); + PSA_ASSERT( psa_mac_verify_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, input->x, input->len ) ); From 78ba2af7c2e24626bb01fece73513a06c4a4fa74 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 10:27:05 +0100 Subject: [PATCH 40/66] Remove old key export API Seems to be an oversight that this wasn't marked deprecated. Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 30 ------------------------------ library/ssl_tls.c | 16 ---------------- 2 files changed, 46 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 603615b3c8..6c24aab77b 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1033,9 +1033,6 @@ struct mbedtls_ssl_config #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) - /** Callback to export key block and master secret */ - int (*MBEDTLS_PRIVATE(f_export_keys))( void *, const unsigned char *, - const unsigned char *, size_t, size_t, size_t ); /** Callback to export key block, master secret, * tls_prf and random bytes. Should replace f_export_keys */ int (*MBEDTLS_PRIVATE(f_export_keys_ext))( void *, const unsigned char *, @@ -1919,33 +1916,6 @@ typedef int mbedtls_ssl_ticket_write_t( void *p_ticket, uint32_t *lifetime ); #if defined(MBEDTLS_SSL_EXPORT_KEYS) -/** - * \brief Callback type: Export key block and master secret - * - * \note This is required for certain uses of TLS, e.g. EAP-TLS - * (RFC 5216) and Thread. The key pointers are ephemeral and - * therefore must not be stored. The master secret and keys - * should not be used directly except as an input to a key - * derivation function. - * - * \param p_expkey Context for the callback - * \param ms Pointer to master secret (fixed length: 48 bytes) - * \param kb Pointer to key block, see RFC 5246 section 6.3 - * (variable length: 2 * maclen + 2 * keylen + 2 * ivlen). - * \param maclen MAC length - * \param keylen Key length - * \param ivlen IV length - * - * \return 0 if successful, or - * a specific MBEDTLS_ERR_XXX code. - */ -typedef int mbedtls_ssl_export_keys_t( void *p_expkey, - const unsigned char *ms, - const unsigned char *kb, - size_t maclen, - size_t keylen, - size_t ivlen ); - /** * \brief Callback type: Export key block, master secret, * handshake randbytes and the tls_prf function diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2961637a85..e6bc790fe5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -986,14 +986,6 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, ((void) mac_enc); #if defined(MBEDTLS_SSL_EXPORT_KEYS) - if( ssl->conf->f_export_keys != NULL ) - { - ssl->conf->f_export_keys( ssl->conf->p_export_keys, - master, keyblk, - mac_key_len, keylen, - iv_copy_len ); - } - if( ssl->conf->f_export_keys_ext != NULL ) { ssl->conf->f_export_keys_ext( ssl->conf->p_export_keys, @@ -4193,14 +4185,6 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) -void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ) -{ - conf->f_export_keys = f_export_keys; - conf->p_export_keys = p_export_keys; -} - void mbedtls_ssl_conf_export_keys_ext_cb( mbedtls_ssl_config *conf, mbedtls_ssl_export_keys_ext_t *f_export_keys_ext, void *p_export_keys ) From 2d6e6f8fec7fe3a116f7aefe39644bad0df6506b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 10:58:31 +0100 Subject: [PATCH 41/66] Remove '_ext' suffix from SSL key exporter API Signed-off-by: Hanno Becker --- include/mbedtls/config.h | 2 +- include/mbedtls/ssl.h | 12 ++++++------ library/ssl_tls.c | 10 +++++----- programs/ssl/ssl_client2.c | 6 +++--- programs/ssl/ssl_server2.c | 6 +++--- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 598e59fcbd..e9da07a6e2 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1556,7 +1556,7 @@ * (see Section 5 of RFC 5764), are not handled by this feature. * Instead, after successful completion of a handshake negotiating * the use of DTLS-SRTP, the extended key exporter API - * mbedtls_ssl_conf_export_keys_ext_cb() should be used to implement + * mbedtls_ssl_conf_export_keys_cb() should be used to implement * the key exporter described in Section 4.2 of RFC 5764 and RFC 5705 * (this is implemented in the SSL example programs). * The resulting key should then be passed to an SRTP stack. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 6c24aab77b..410ac0e0a2 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1035,7 +1035,7 @@ struct mbedtls_ssl_config #if defined(MBEDTLS_SSL_EXPORT_KEYS) /** Callback to export key block, master secret, * tls_prf and random bytes. Should replace f_export_keys */ - int (*MBEDTLS_PRIVATE(f_export_keys_ext))( void *, const unsigned char *, + int (*MBEDTLS_PRIVATE(f_export_keys))( void *, const unsigned char *, const unsigned char *, size_t, size_t, size_t, const unsigned char[32], const unsigned char[32], mbedtls_tls_prf_types ); @@ -1941,7 +1941,7 @@ typedef int mbedtls_ssl_ticket_write_t( void *p_ticket, * \return 0 if successful, or * a specific MBEDTLS_ERR_XXX code. */ -typedef int mbedtls_ssl_export_keys_ext_t( void *p_expkey, +typedef int mbedtls_ssl_export_keys_t( void *p_expkey, const unsigned char *ms, const unsigned char *kb, size_t maclen, @@ -2020,16 +2020,16 @@ void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, * \brief Configure extended key export callback. * (Default: none.) * - * \note See \c mbedtls_ssl_export_keys_ext_t. + * \note See \c mbedtls_ssl_export_keys_t. * \warning Exported key material must not be used for any purpose * before the (D)TLS handshake is completed * * \param conf SSL configuration context - * \param f_export_keys_ext Callback for exporting keys + * \param f_export_keys Callback for exporting keys * \param p_export_keys Context for the callback */ -void mbedtls_ssl_conf_export_keys_ext_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_ext_t *f_export_keys_ext, +void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, + mbedtls_ssl_export_keys_t *f_export_keys, void *p_export_keys ); #endif /* MBEDTLS_SSL_EXPORT_KEYS */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e6bc790fe5..94f906afcb 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -986,9 +986,9 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, ((void) mac_enc); #if defined(MBEDTLS_SSL_EXPORT_KEYS) - if( ssl->conf->f_export_keys_ext != NULL ) + if( ssl->conf->f_export_keys != NULL ) { - ssl->conf->f_export_keys_ext( ssl->conf->p_export_keys, + ssl->conf->f_export_keys( ssl->conf->p_export_keys, master, keyblk, mac_key_len, keylen, iv_copy_len, @@ -4185,11 +4185,11 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) -void mbedtls_ssl_conf_export_keys_ext_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_ext_t *f_export_keys_ext, +void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, + mbedtls_ssl_export_keys_t *f_export_keys, void *p_export_keys ) { - conf->f_export_keys_ext = f_export_keys_ext; + conf->f_export_keys = f_export_keys; conf->p_export_keys = p_export_keys; } #endif diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 322cef8b45..bc5ed2e9ee 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1739,19 +1739,19 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_EXPORT_KEYS) if( opt.eap_tls != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, eap_tls_key_derivation, + mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, &eap_tls_keying ); } else if( opt.nss_keylog != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, + mbedtls_ssl_conf_export_keys_cb( &conf, nss_keylog_export, NULL ); } #if defined( MBEDTLS_SSL_DTLS_SRTP ) else if( opt.use_srtp != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, dtls_srtp_key_derivation, + mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, &dtls_srtp_keying ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 51125bdb69..334eb7d445 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2528,19 +2528,19 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_EXPORT_KEYS) if( opt.eap_tls != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, eap_tls_key_derivation, + mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, &eap_tls_keying ); } else if( opt.nss_keylog != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, + mbedtls_ssl_conf_export_keys_cb( &conf, nss_keylog_export, NULL ); } #if defined( MBEDTLS_SSL_DTLS_SRTP ) else if( opt.use_srtp != 0 ) { - mbedtls_ssl_conf_export_keys_ext_cb( &conf, dtls_srtp_key_derivation, + mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, &dtls_srtp_keying ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ From 457d61602f4fcf6d0f50edc2c070a674646b96b5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 10:27:39 +0100 Subject: [PATCH 42/66] Define and implement new key export API for Mbed TLS 3.0 Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 151 ++++++++++++++++++++---------------------- library/ssl_tls.c | 15 ++--- 2 files changed, 79 insertions(+), 87 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 410ac0e0a2..38a4a64d36 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -458,18 +458,6 @@ typedef enum } mbedtls_ssl_states; -/* - * The tls_prf function types. - */ -typedef enum -{ - MBEDTLS_SSL_TLS_PRF_NONE, - MBEDTLS_SSL_TLS_PRF_TLS1, - MBEDTLS_SSL_TLS_PRF_SHA384, - MBEDTLS_SSL_TLS_PRF_SHA256 -} -mbedtls_tls_prf_types; - /** * \brief Callback type: send data on the network. * @@ -967,6 +955,58 @@ struct mbedtls_ssl_session #endif }; +/* + * Identifiers for PRFs used in various versions of TLS. + */ +typedef enum +{ + MBEDTLS_SSL_TLS_PRF_NONE, + MBEDTLS_SSL_TLS_PRF_TLS1, + MBEDTLS_SSL_TLS_PRF_SHA384, + MBEDTLS_SSL_TLS_PRF_SHA256, + MBEDTLS_SSL_HKDF_EXPAND_SHA384, + MBEDTLS_SSL_HKDF_EXPAND_SHA256 +} +mbedtls_tls_prf_types; + +#if defined(MBEDTLS_SSL_EXPORT_KEYS) +typedef enum +{ + MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET = 0, +#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL) + MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_EARLY_SECRET, + MBEDTLS_SSL_KEY_EXPORT_TLS13_EARLY_EXPORTER_SECRET, + MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_HANDSHAKE_TRAFFIC_SECRET, + MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_HANDSHAKE_TRAFFIC_SECRET, + MBEDTLS_SSL_KEY_EXPORT_TLS13_CLIENT_APPLICATION_TRAFFIC_SECRET, + MBEDTLS_SSL_KEY_EXPORT_TLS13_SERVER_APPLICATION_TRAFFIC_SECRET, +#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ +} mbedtls_ssl_key_export_type; + +/** + * \brief Callback type: Export key alongside random values for + * session identification, and PRF for + * implementation of TLS key exporters. + * + * \param p_expkey Context for the callback. + * \param type The type of the key that is being exported. + * \param client_random The client random bytes. + * \param server_random The server random bytes. + * \param tls_prf_type The identifier for the PRF used in the handshake + * to which the key belongs. + * + * \return \c 0 if successful. + * \return A negative error code on failure. + */ +typedef int mbedtls_ssl_export_keys_t( void *p_expkey, + mbedtls_ssl_key_export_type type, + const unsigned char *secret, + size_t secret_len, + const unsigned char client_random[32], + const unsigned char server_random[32], + mbedtls_tls_prf_types tls_prf_type ); +#endif /* MBEDTLS_SSL_EXPORT_KEYS */ + /** * SSL/TLS configuration to be shared between mbedtls_ssl_context structures. */ @@ -1033,12 +1073,8 @@ struct mbedtls_ssl_config #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) - /** Callback to export key block, master secret, - * tls_prf and random bytes. Should replace f_export_keys */ - int (*MBEDTLS_PRIVATE(f_export_keys))( void *, const unsigned char *, - const unsigned char *, size_t, size_t, size_t, - const unsigned char[32], const unsigned char[32], - mbedtls_tls_prf_types ); + /** Callback to export key block and master secret */ + mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ #endif @@ -1915,43 +1951,6 @@ typedef int mbedtls_ssl_ticket_write_t( void *p_ticket, size_t *tlen, uint32_t *lifetime ); -#if defined(MBEDTLS_SSL_EXPORT_KEYS) -/** - * \brief Callback type: Export key block, master secret, - * handshake randbytes and the tls_prf function - * used to derive keys. - * - * \note This is required for certain uses of TLS, e.g. EAP-TLS - * (RFC 5216) and Thread. The key pointers are ephemeral and - * therefore must not be stored. The master secret and keys - * should not be used directly except as an input to a key - * derivation function. - * - * \param p_expkey Context for the callback. - * \param ms Pointer to master secret (fixed length: 48 bytes). - * \param kb Pointer to key block, see RFC 5246 section 6.3. - * (variable length: 2 * maclen + 2 * keylen + 2 * ivlen). - * \param maclen MAC length. - * \param keylen Key length. - * \param ivlen IV length. - * \param client_random The client random bytes. - * \param server_random The server random bytes. - * \param tls_prf_type The tls_prf enum type. - * - * \return 0 if successful, or - * a specific MBEDTLS_ERR_XXX code. - */ -typedef int mbedtls_ssl_export_keys_t( void *p_expkey, - const unsigned char *ms, - const unsigned char *kb, - size_t maclen, - size_t keylen, - size_t ivlen, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type ); -#endif /* MBEDTLS_SSL_EXPORT_KEYS */ - /** * \brief Callback type: parse and load session ticket * @@ -2003,34 +2002,28 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_EXPORT_KEYS) /** - * \brief Configure key export callback. - * (Default: none.) + * \brief Configure a key export callback. + * (Default: none.) * - * \note See \c mbedtls_ssl_export_keys_t. + * This API can be used for two purposes: + * - Debugging: Use this API to e.g. generate an NSSKeylog + * file and use it to inspect encrypted traffic in tools + * such as Wireshark. + * - Application-specific export: Use this API to implement + * key exporters, e.g. for EAP-TLS or DTLS-SRTP. * - * \param conf SSL configuration context - * \param f_export_keys Callback for exporting keys - * \param p_export_keys Context for the callback + * + * \param conf The SSL configuration to which the export + * callback should be attached. All connections + * subsequently bound to this configuration will + * have their keys exported. + * \param f_export_keys The callback for the key export. + * \param p_export_keys The opaque context pointer to be passed to the + * callback \p f_export_keys. */ void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ); - -/** - * \brief Configure extended key export callback. - * (Default: none.) - * - * \note See \c mbedtls_ssl_export_keys_t. - * \warning Exported key material must not be used for any purpose - * before the (D)TLS handshake is completed - * - * \param conf SSL configuration context - * \param f_export_keys Callback for exporting keys - * \param p_export_keys Context for the callback - */ -void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ); + mbedtls_ssl_export_keys_t *f_export_keys, + void *p_export_keys ); #endif /* MBEDTLS_SSL_EXPORT_KEYS */ #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 94f906afcb..4c69a0872b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -989,12 +989,11 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, if( ssl->conf->f_export_keys != NULL ) { ssl->conf->f_export_keys( ssl->conf->p_export_keys, - master, keyblk, - mac_key_len, keylen, - iv_copy_len, - randbytes + 32, - randbytes, - tls_prf_get_type( tls_prf ) ); + MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET, + master, 48, + randbytes + 32, + randbytes, + tls_prf_get_type( tls_prf ) ); } #endif @@ -4186,8 +4185,8 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_EXPORT_KEYS) void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ) + mbedtls_ssl_export_keys_t *f_export_keys, + void *p_export_keys ) { conf->f_export_keys = f_export_keys; conf->p_export_keys = p_export_keys; From c4c38caca584098ffc98db4031d84ecd7a189fb2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 10:57:07 +0100 Subject: [PATCH 43/66] Adjust example programs to new key export API Signed-off-by: Hanno Becker --- programs/ssl/ssl_client2.c | 8 ++-- programs/ssl/ssl_server2.c | 8 ++-- programs/ssl/ssl_test_common_source.c | 67 ++++++++++++--------------- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index bc5ed2e9ee..c25b9ee10a 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1740,19 +1740,19 @@ int main( int argc, char *argv[] ) if( opt.eap_tls != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, - &eap_tls_keying ); + &eap_tls_keying ); } else if( opt.nss_keylog != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, - nss_keylog_export, - NULL ); + nss_keylog_export, + NULL ); } #if defined( MBEDTLS_SSL_DTLS_SRTP ) else if( opt.use_srtp != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, - &dtls_srtp_keying ); + &dtls_srtp_keying ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ #endif /* MBEDTLS_SSL_EXPORT_KEYS */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 334eb7d445..9cecf7f3d8 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2529,19 +2529,19 @@ int main( int argc, char *argv[] ) if( opt.eap_tls != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, - &eap_tls_keying ); + &eap_tls_keying ); } else if( opt.nss_keylog != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, - nss_keylog_export, - NULL ); + nss_keylog_export, + NULL ); } #if defined( MBEDTLS_SSL_DTLS_SRTP ) else if( opt.use_srtp != 0 ) { mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, - &dtls_srtp_keying ); + &dtls_srtp_keying ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ #endif /* MBEDTLS_SSL_EXPORT_KEYS */ diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index fa2c606975..6da0ba4a50 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -26,54 +26,48 @@ #if defined(MBEDTLS_SSL_EXPORT_KEYS) int eap_tls_key_derivation( void *p_expkey, - const unsigned char *ms, - const unsigned char *kb, - size_t maclen, - size_t keylen, - size_t ivlen, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, const unsigned char client_random[32], const unsigned char server_random[32], mbedtls_tls_prf_types tls_prf_type ) { eap_tls_keys *keys = (eap_tls_keys *)p_expkey; - ( ( void ) kb ); - memcpy( keys->master_secret, ms, sizeof( keys->master_secret ) ); + /* We're only interested in the TLS 1.2 master secret */ + if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) + return( 0 ); + if( secret_len != sizeof( keys->master_secret ) ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + + memcpy( keys->master_secret, secret, sizeof( keys->master_secret ) ); memcpy( keys->randbytes, client_random, 32 ); memcpy( keys->randbytes + 32, server_random, 32 ); keys->tls_prf_type = tls_prf_type; - if( opt.debug_level > 2 ) - { - mbedtls_printf("exported maclen is %u\n", (unsigned)maclen); - mbedtls_printf("exported keylen is %u\n", (unsigned)keylen); - mbedtls_printf("exported ivlen is %u\n", (unsigned)ivlen); - } return( 0 ); } int nss_keylog_export( void *p_expkey, - const unsigned char *ms, - const unsigned char *kb, - size_t maclen, - size_t keylen, - size_t ivlen, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, const unsigned char client_random[32], const unsigned char server_random[32], mbedtls_tls_prf_types tls_prf_type ) { char nss_keylog_line[ 200 ]; size_t const client_random_len = 32; - size_t const master_secret_len = 48; size_t len = 0; size_t j; int ret = 0; + /* We're only interested in the TLS 1.2 master secret */ + if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) + return( 0 ); + ((void) p_expkey); - ((void) kb); - ((void) maclen); - ((void) keylen); - ((void) ivlen); ((void) server_random); ((void) tls_prf_type); @@ -88,10 +82,10 @@ int nss_keylog_export( void *p_expkey, len += sprintf( nss_keylog_line + len, " " ); - for( j = 0; j < master_secret_len; j++ ) + for( j = 0; j < secret_len; j++ ) { len += sprintf( nss_keylog_line + len, - "%02x", ms[j] ); + "%02x", secret[j] ); } len += sprintf( nss_keylog_line + len, "\n" ); @@ -130,29 +124,26 @@ exit: #if defined( MBEDTLS_SSL_DTLS_SRTP ) int dtls_srtp_key_derivation( void *p_expkey, - const unsigned char *ms, - const unsigned char *kb, - size_t maclen, - size_t keylen, - size_t ivlen, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, const unsigned char client_random[32], const unsigned char server_random[32], mbedtls_tls_prf_types tls_prf_type ) { dtls_srtp_keys *keys = (dtls_srtp_keys *)p_expkey; - ( ( void ) kb ); - memcpy( keys->master_secret, ms, sizeof( keys->master_secret ) ); + /* We're only interested in the TLS 1.2 master secret */ + if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) + return( 0 ); + if( secret_len != sizeof( keys->master_secret ) ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + + memcpy( keys->master_secret, secret, sizeof( keys->master_secret ) ); memcpy( keys->randbytes, client_random, 32 ); memcpy( keys->randbytes + 32, server_random, 32 ); keys->tls_prf_type = tls_prf_type; - if( opt.debug_level > 2 ) - { - mbedtls_printf( "exported maclen is %u\n", (unsigned) maclen ); - mbedtls_printf( "exported keylen is %u\n", (unsigned) keylen ); - mbedtls_printf( "exported ivlen is %u\n", (unsigned) ivlen ); - } return( 0 ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ From d5c9cc7c90c91acb67693664aa0fc6a8728d55ce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 11:12:43 +0100 Subject: [PATCH 44/66] Add migration guide for modified key export API Signed-off-by: Hanno Becker --- docs/3.0-migration-guide.d/key-export.md | 28 ++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/3.0-migration-guide.d/key-export.md diff --git a/docs/3.0-migration-guide.d/key-export.md b/docs/3.0-migration-guide.d/key-export.md new file mode 100644 index 0000000000..43781a2fb1 --- /dev/null +++ b/docs/3.0-migration-guide.d/key-export.md @@ -0,0 +1,28 @@ +SSL key export interface change +------------------------------- + +This affects users of the SSL key export APIs: +``` + mbedtls_ssl_conf_export_keys_cb() + mbedtls_ssl_conf_export_keys_ext_cb() +``` + +The API `mbedtls_ssl_conf_export_keys_ext_cb()` has been removed, +and the function type of key export callback passed to +`mbedtls_ssl_conf_export_keys_cb()` has changed, as follows: +- It no longer exports raw keys and IV. +- A secret type parameter has been added to identify which key + is being exported. For TLS 1.2, only the master secret is + exported, but upcoming TLS 1.3 support will add other kinds of keys. + +For users which do not rely on raw keys and IV, adjusting to the new +callback type should be straightforward - see the example programs +programs/ssl/ssl_client2 and programs/ssl/ssl_server2 for callbacks +for NSSKeylog, EAP-TLS and DTLS-SRTP. + +Users which require access to the raw keys used to secure application +traffic may derive those by hand based on the master secret and the +handshake transcript hashes which can be obtained from the raw data +on the wire. Such users are also encouraged to reach out to the +Mbed TLS team on the mailing list, to let the team know about their +use case. From 5a234e8718741a660aec8d9ec718b0f6e0434b2e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 May 2021 11:15:29 +0100 Subject: [PATCH 45/66] Add ChangeLog entry Signed-off-by: Hanno Becker --- ChangeLog.d/key-export.txt | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 ChangeLog.d/key-export.txt diff --git a/ChangeLog.d/key-export.txt b/ChangeLog.d/key-export.txt new file mode 100644 index 0000000000..5882d231e4 --- /dev/null +++ b/ChangeLog.d/key-export.txt @@ -0,0 +1,8 @@ +API changes + * mbedtls_ssl_conf_export_keys_ext_cb() has been removed. + * The signature of key export callbacks configured via + mbedtls_ssl_conf_export_keys_cb() has changed, and raw + keys and IVs are no longer exported. Further, callbacks + now receive an additional parameter indicating the type + of secret that's being exported, paving the way for the + larger number of secrets in TLS 1.3. From 11a4c1abcd7a5da2341f2f4bb79d550874148d6f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 26 May 2021 04:46:20 +0100 Subject: [PATCH 46/66] Adapt key export test in ssl-opt.sh to reduced output Signed-off-by: Hanno Becker --- tests/ssl-opt.sh | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index d1221112ae..4210847dfa 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8555,12 +8555,6 @@ run_test "export keys functionality" \ "$P_SRV eap_tls=1 debug_level=3" \ "$P_CLI eap_tls=1 debug_level=3" \ 0 \ - -s "exported maclen is " \ - -s "exported keylen is " \ - -s "exported ivlen is " \ - -c "exported maclen is " \ - -c "exported keylen is " \ - -c "exported ivlen is " \ -c "EAP-TLS key material is:"\ -s "EAP-TLS key material is:"\ -c "EAP-TLS IV is:" \ From ddc739cac41f4c03ac1e137d839dd186b7b59588 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 28 May 2021 05:10:38 +0100 Subject: [PATCH 47/66] Add missing documentation for key export callback parameters Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 38a4a64d36..bf8a9e2b43 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -988,8 +988,11 @@ typedef enum * session identification, and PRF for * implementation of TLS key exporters. * - * \param p_expkey Context for the callback. - * \param type The type of the key that is being exported. + * \param p_expkey Context for the callback. + * \param type The type of the key that is being exported. + * \param secret The address of the buffer holding the secret + * that's being exporterd. + * \param secret_len The length of \p secret in bytes. * \param client_random The client random bytes. * \param server_random The server random bytes. * \param tls_prf_type The identifier for the PRF used in the handshake From 22b34f75cdc371ee05ea4130c54c8906810d3790 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 28 May 2021 05:11:25 +0100 Subject: [PATCH 48/66] Remote key export identifier used for TLS < 1.2. Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index bf8a9e2b43..540f5527cd 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -961,7 +961,6 @@ struct mbedtls_ssl_session typedef enum { MBEDTLS_SSL_TLS_PRF_NONE, - MBEDTLS_SSL_TLS_PRF_TLS1, MBEDTLS_SSL_TLS_PRF_SHA384, MBEDTLS_SSL_TLS_PRF_SHA256, MBEDTLS_SSL_HKDF_EXPAND_SHA384, From a7991f2e11290a66e1d5a759dbb7b35b792580d6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 28 May 2021 05:14:18 +0100 Subject: [PATCH 49/66] Remove all occurrences of TLS < 1.2 PRF identifier Signed-off-by: Hanno Becker --- tests/suites/test_suite_ssl.data | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index a497076c14..10f8f28bff 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -9278,14 +9278,6 @@ ssl_tls1_3_create_psk_binder:MBEDTLS_MD_SHA256:"4ecd0eb6ec3b4d87f5d6028f922ca4c5 SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_NONE ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_NONE:"":"":"test tls_prf label":"":MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE -SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_TLS1 TLS 1.0 enabled -depends_on:MBEDTLS_SSL_PROTO_TLS1 -ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_TLS1:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"8defca540d41d4c79d390027295bb4e6":0 - -SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_TLS1 TLS 1.1 enabled -depends_on:MBEDTLS_SSL_PROTO_TLS1_1 -ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_TLS1:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"8defca540d41d4c79d390027295bb4e6":0 - SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_SHA384 depends_on:MBEDTLS_SHA384_C:MBEDTLS_SSL_PROTO_TLS1_2 ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_SHA384:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"a4206a36eef93f496611c2b7806625c3":0 @@ -9294,10 +9286,6 @@ SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_SHA256 depends_on:MBEDTLS_SHA256_C:MBEDTLS_SSL_PROTO_TLS1_2 ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_SHA256:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"7f9998393198a02c8d731ccc2ef90b2c":0 -SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_TLS1 TLS 1.X not enabled -depends_on:!MBEDTLS_SSL_PROTO_TLS1:!MBEDTLS_SSL_PROTO_TLS1_1 -ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_TLS1:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"8defca540d41d4c79d390027295bb4e6":MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE - SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_SHA384 SHA-384 not enabled depends_on:!MBEDTLS_SHA384_C ssl_tls_prf:MBEDTLS_SSL_TLS_PRF_SHA384:"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef":"test tls_prf label":"a4206a36eef93f496611c2b7806625c3":MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE From 7e6c178b6dd200c0f36a603f9ab3fa4d4e83e709 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 8 Jun 2021 09:24:55 +0100 Subject: [PATCH 50/66] Make key export callback and context connection-specific Fixes #2188 Signed-off-by: Hanno Becker --- ChangeLog.d/key-export.txt | 13 +++++--- docs/3.0-migration-guide.d/key-export.md | 10 ++++-- include/mbedtls/ssl.h | 24 +++++++------- library/ssl_tls.c | 24 +++++++------- programs/ssl/ssl_client2.c | 42 ++++++++++++------------ programs/ssl/ssl_server2.c | 42 ++++++++++++------------ 6 files changed, 81 insertions(+), 74 deletions(-) diff --git a/ChangeLog.d/key-export.txt b/ChangeLog.d/key-export.txt index 5882d231e4..10d8c89913 100644 --- a/ChangeLog.d/key-export.txt +++ b/ChangeLog.d/key-export.txt @@ -1,8 +1,13 @@ API changes - * mbedtls_ssl_conf_export_keys_ext_cb() has been removed. + * mbedtls_ssl_conf_export_keys_ext_cb() and + mbedtls_ssl_conf_export_keys_cb() have been removed + and replaced by a new API + mbedtls_ssl_set_export_keys_cb(). * The signature of key export callbacks configured via - mbedtls_ssl_conf_export_keys_cb() has changed, and raw - keys and IVs are no longer exported. Further, callbacks + mbedtls_ssl_set_export_keys_cb() is different from that + of the previous mbedtls_ssl_conf_export_keys_cb(): First, + raw keys and IVs are no longer exported. Further, callbacks now receive an additional parameter indicating the type of secret that's being exported, paving the way for the - larger number of secrets in TLS 1.3. + larger number of secrets in TLS 1.3. Finally, the key export + callback and context are now connection-specific. diff --git a/docs/3.0-migration-guide.d/key-export.md b/docs/3.0-migration-guide.d/key-export.md index 43781a2fb1..967ecf87bf 100644 --- a/docs/3.0-migration-guide.d/key-export.md +++ b/docs/3.0-migration-guide.d/key-export.md @@ -7,9 +7,13 @@ This affects users of the SSL key export APIs: mbedtls_ssl_conf_export_keys_ext_cb() ``` -The API `mbedtls_ssl_conf_export_keys_ext_cb()` has been removed, -and the function type of key export callback passed to -`mbedtls_ssl_conf_export_keys_cb()` has changed, as follows: +Those APIs have been removed and replaced by the new API +`mbedtls_ssl_set_export_keys_cb()`. This API differs from +the previous key export API in the following ways: + +- It is no longer bound to an SSL configuration, but to an + SSL context. This allows users to more easily identify the + connection an exported key belongs to. - It no longer exports raw keys and IV. - A secret type parameter has been added to identify which key is being exported. For TLS 1.2, only the master secret is diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 540f5527cd..25800209e0 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1074,12 +1074,6 @@ struct mbedtls_ssl_config void *MBEDTLS_PRIVATE(p_ticket); /*!< context for the ticket callbacks */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ -#if defined(MBEDTLS_SSL_EXPORT_KEYS) - /** Callback to export key block and master secret */ - mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); - void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ -#endif - #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) size_t MBEDTLS_PRIVATE(cid_len); /*!< The length of CIDs for incoming DTLS records. */ #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ @@ -1260,6 +1254,12 @@ struct mbedtls_ssl_context int MBEDTLS_PRIVATE(minor_ver); /*!< one of MBEDTLS_SSL_MINOR_VERSION_x macros */ unsigned MBEDTLS_PRIVATE(badmac_seen); /*!< records with a bad MAC received */ +#if defined(MBEDTLS_SSL_EXPORT_KEYS) + /** Callback to export key block and master secret */ + mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); + void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ +#endif + #if defined(MBEDTLS_X509_CRT_PARSE_C) /** Callback to customize X.509 certificate chain verification */ int (*MBEDTLS_PRIVATE(f_vrfy))(void *, mbedtls_x509_crt *, int, uint32_t *); @@ -2015,17 +2015,15 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, * key exporters, e.g. for EAP-TLS or DTLS-SRTP. * * - * \param conf The SSL configuration to which the export - * callback should be attached. All connections - * subsequently bound to this configuration will - * have their keys exported. + * \param ssl The SSL context to which the export + * callback should be attached. * \param f_export_keys The callback for the key export. * \param p_export_keys The opaque context pointer to be passed to the * callback \p f_export_keys. */ -void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ); +void mbedtls_ssl_set_export_keys_cb( mbedtls_ssl_context *ssl, + mbedtls_ssl_export_keys_t *f_export_keys, + void *p_export_keys ); #endif /* MBEDTLS_SSL_EXPORT_KEYS */ #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4c69a0872b..9268ede285 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -986,14 +986,14 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, ((void) mac_enc); #if defined(MBEDTLS_SSL_EXPORT_KEYS) - if( ssl->conf->f_export_keys != NULL ) + if( ssl->f_export_keys != NULL ) { - ssl->conf->f_export_keys( ssl->conf->p_export_keys, - MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET, - master, 48, - randbytes + 32, - randbytes, - tls_prf_get_type( tls_prf ) ); + ssl->f_export_keys( ssl->p_export_keys, + MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET, + master, 48, + randbytes + 32, + randbytes, + tls_prf_get_type( tls_prf ) ); } #endif @@ -4184,12 +4184,12 @@ void mbedtls_ssl_conf_session_tickets_cb( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) -void mbedtls_ssl_conf_export_keys_cb( mbedtls_ssl_config *conf, - mbedtls_ssl_export_keys_t *f_export_keys, - void *p_export_keys ) +void mbedtls_ssl_set_export_keys_cb( mbedtls_ssl_context *ssl, + mbedtls_ssl_export_keys_t *f_export_keys, + void *p_export_keys ) { - conf->f_export_keys = f_export_keys; - conf->p_export_keys = p_export_keys; + ssl->f_export_keys = f_export_keys; + ssl->p_export_keys = p_export_keys; } #endif diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index c25b9ee10a..a7b7ece4f4 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1736,27 +1736,6 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_encrypt_then_mac( &conf, opt.etm ); #endif -#if defined(MBEDTLS_SSL_EXPORT_KEYS) - if( opt.eap_tls != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, - &eap_tls_keying ); - } - else if( opt.nss_keylog != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, - nss_keylog_export, - NULL ); - } -#if defined( MBEDTLS_SSL_DTLS_SRTP ) - else if( opt.use_srtp != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, - &dtls_srtp_keying ); - } -#endif /* MBEDTLS_SSL_DTLS_SRTP */ -#endif /* MBEDTLS_SSL_EXPORT_KEYS */ - #if defined(MBEDTLS_DHM_C) if( opt.dhmlen != DFL_DHMLEN ) mbedtls_ssl_conf_dhm_min_bitlen( &conf, opt.dhmlen ); @@ -1886,6 +1865,27 @@ int main( int argc, char *argv[] ) goto exit; } +#if defined(MBEDTLS_SSL_EXPORT_KEYS) + if( opt.eap_tls != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, eap_tls_key_derivation, + &eap_tls_keying ); + } + else if( opt.nss_keylog != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, + nss_keylog_export, + NULL ); + } +#if defined( MBEDTLS_SSL_DTLS_SRTP ) + else if( opt.use_srtp != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, dtls_srtp_key_derivation, + &dtls_srtp_keying ); + } +#endif /* MBEDTLS_SSL_DTLS_SRTP */ +#endif /* MBEDTLS_SSL_EXPORT_KEYS */ + #if defined(MBEDTLS_X509_CRT_PARSE_C) if( ( ret = mbedtls_ssl_set_hostname( &ssl, opt.server_name ) ) != 0 ) { diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 9cecf7f3d8..cb15866b04 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2525,27 +2525,6 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_encrypt_then_mac( &conf, opt.etm ); #endif -#if defined(MBEDTLS_SSL_EXPORT_KEYS) - if( opt.eap_tls != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, eap_tls_key_derivation, - &eap_tls_keying ); - } - else if( opt.nss_keylog != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, - nss_keylog_export, - NULL ); - } -#if defined( MBEDTLS_SSL_DTLS_SRTP ) - else if( opt.use_srtp != 0 ) - { - mbedtls_ssl_conf_export_keys_cb( &conf, dtls_srtp_key_derivation, - &dtls_srtp_keying ); - } -#endif /* MBEDTLS_SSL_DTLS_SRTP */ -#endif /* MBEDTLS_SSL_EXPORT_KEYS */ - #if defined(MBEDTLS_SSL_ALPN) if( opt.alpn_string != NULL ) if( ( ret = mbedtls_ssl_conf_alpn_protocols( &conf, alpn_list ) ) != 0 ) @@ -2872,6 +2851,27 @@ int main( int argc, char *argv[] ) goto exit; } +#if defined(MBEDTLS_SSL_EXPORT_KEYS) + if( opt.eap_tls != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, eap_tls_key_derivation, + &eap_tls_keying ); + } + else if( opt.nss_keylog != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, + nss_keylog_export, + NULL ); + } +#if defined( MBEDTLS_SSL_DTLS_SRTP ) + else if( opt.use_srtp != 0 ) + { + mbedtls_ssl_set_export_keys_cb( &ssl, dtls_srtp_key_derivation, + &dtls_srtp_keying ); + } +#endif /* MBEDTLS_SSL_DTLS_SRTP */ +#endif /* MBEDTLS_SSL_EXPORT_KEYS */ + io_ctx.ssl = &ssl; io_ctx.net = &client_fd; mbedtls_ssl_set_bio( &ssl, &io_ctx, send_cb, recv_cb, From e0dad720ee3962264abba2867a0620a8de4babb9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 11 Jun 2021 15:38:37 +0100 Subject: [PATCH 51/66] Remove return value from key export callback Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 25800209e0..a434912868 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -996,17 +996,14 @@ typedef enum * \param server_random The server random bytes. * \param tls_prf_type The identifier for the PRF used in the handshake * to which the key belongs. - * - * \return \c 0 if successful. - * \return A negative error code on failure. */ -typedef int mbedtls_ssl_export_keys_t( void *p_expkey, - mbedtls_ssl_key_export_type type, - const unsigned char *secret, - size_t secret_len, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type ); +typedef void mbedtls_ssl_export_keys_t( void *p_expkey, + mbedtls_ssl_key_export_type type, + const unsigned char *secret, + size_t secret_len, + const unsigned char client_random[32], + const unsigned char server_random[32], + mbedtls_tls_prf_types tls_prf_type ); #endif /* MBEDTLS_SSL_EXPORT_KEYS */ /** From 1e1c23d768a83827d6a0e86d363fd87c1bebcb7f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 11 Jun 2021 15:40:16 +0100 Subject: [PATCH 52/66] Improve ChangeLog wording for key export Signed-off-by: Hanno Becker --- ChangeLog.d/key-export.txt | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/ChangeLog.d/key-export.txt b/ChangeLog.d/key-export.txt index 10d8c89913..2fc01a4c52 100644 --- a/ChangeLog.d/key-export.txt +++ b/ChangeLog.d/key-export.txt @@ -1,13 +1,10 @@ API changes * mbedtls_ssl_conf_export_keys_ext_cb() and - mbedtls_ssl_conf_export_keys_cb() have been removed - and replaced by a new API - mbedtls_ssl_set_export_keys_cb(). - * The signature of key export callbacks configured via - mbedtls_ssl_set_export_keys_cb() is different from that - of the previous mbedtls_ssl_conf_export_keys_cb(): First, - raw keys and IVs are no longer exported. Further, callbacks - now receive an additional parameter indicating the type - of secret that's being exported, paving the way for the - larger number of secrets in TLS 1.3. Finally, the key export - callback and context are now connection-specific. + mbedtls_ssl_conf_export_keys_cb() have been removed and + replaced by a new API mbedtls_ssl_set_export_keys_cb(). + Raw keys and IVs are no longer passed to the callback. + Further, callbacks now receive an additional parameter + indicating the type of secret that's being exported, + paving the way for the larger number of secrets + in TLS 1.3. Finally, the key export callback and + context are now connection-specific. From d8f32e72b4ef90ab09e2b647932b182ac1a6ea53 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 17 Jun 2021 05:14:58 +0100 Subject: [PATCH 53/66] Move export callback and context to the end of SSL context This saves some code when compiling for Thumb, where access to fields with offset index > 127 requires intermediate address computations. Frequently used fields should therefore be located at the top of the structure, while less frequently used ones -- such as the export callback -- can be moved to the back. Signed-off-by: Hanno Becker --- include/mbedtls/ssl.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a434912868..512f0ccfd6 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1251,12 +1251,6 @@ struct mbedtls_ssl_context int MBEDTLS_PRIVATE(minor_ver); /*!< one of MBEDTLS_SSL_MINOR_VERSION_x macros */ unsigned MBEDTLS_PRIVATE(badmac_seen); /*!< records with a bad MAC received */ -#if defined(MBEDTLS_SSL_EXPORT_KEYS) - /** Callback to export key block and master secret */ - mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); - void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ -#endif - #if defined(MBEDTLS_X509_CRT_PARSE_C) /** Callback to customize X.509 certificate chain verification */ int (*MBEDTLS_PRIVATE(f_vrfy))(void *, mbedtls_x509_crt *, int, uint32_t *); @@ -1427,6 +1421,12 @@ struct mbedtls_ssl_context * Possible values are #MBEDTLS_SSL_CID_ENABLED * and #MBEDTLS_SSL_CID_DISABLED. */ #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ + +#if defined(MBEDTLS_SSL_EXPORT_KEYS) + /** Callback to export key block and master secret */ + mbedtls_ssl_export_keys_t *MBEDTLS_PRIVATE(f_export_keys); + void *MBEDTLS_PRIVATE(p_export_keys); /*!< context for key export callback */ +#endif }; /** From 28ea050cf4c695e627b9e7285e8005d846f2fc38 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 17 Jun 2021 16:10:24 +0200 Subject: [PATCH 54/66] psa: mac: Re-organize psa_mac_setup() internal function Re-organize psa_mac_setup() to prepare the move to a dedicated function of the additional checks on the algorithm and the key attributes done by this function. We want to move those checks in a dedicated function to be able to do them without duplicating them in psa_mac_compute(). Signed-off-by: Ronald Cron --- library/psa_crypto.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a6697f6f40..02b2f99a9e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2248,14 +2248,12 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; + uint8_t mac_size; /* A context must be freshly initialized before it can be set up. */ if( operation->id != 0 ) return( PSA_ERROR_BAD_STATE ); - if( ! PSA_ALG_IS_MAC( alg ) ) - return( PSA_ERROR_INVALID_ARGUMENT ); - status = psa_get_and_lock_key_slot_with_policy( key, &slot, @@ -2264,24 +2262,28 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - psa_key_attributes_t attributes = { + psa_key_attributes_t key_attributes = { .core = slot->attr }; + psa_key_attributes_t *attributes = &key_attributes; + + if( ! PSA_ALG_IS_MAC( alg ) ) + { + status = PSA_ERROR_INVALID_ARGUMENT; + goto exit; + } /* Validate the combination of key type and algorithm */ - status = psa_mac_key_can_do( alg, psa_get_key_type( &attributes ) ); + status = psa_mac_key_can_do( alg, psa_get_key_type( attributes ) ); if( status != PSA_SUCCESS ) goto exit; - operation->is_sign = is_sign; - /* Get the output length for the algorithm and key combination */ - operation->mac_size = PSA_MAC_LENGTH( - psa_get_key_type( &attributes ), - psa_get_key_bits( &attributes ), - alg ); + mac_size = PSA_MAC_LENGTH( psa_get_key_type( attributes ), + psa_get_key_bits( attributes ), + alg ); - if( operation->mac_size < 4 ) + if( mac_size < 4 ) { /* A very short MAC is too short for security since it can be * brute-forced. Ancient protocols with 32-bit MACs do exist, @@ -2291,9 +2293,9 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, goto exit; } - if( operation->mac_size > PSA_MAC_LENGTH( psa_get_key_type( &attributes ), - psa_get_key_bits( &attributes ), - PSA_ALG_FULL_LENGTH_MAC( alg ) ) ) + if( mac_size > PSA_MAC_LENGTH( psa_get_key_type( attributes ), + psa_get_key_bits( attributes ), + PSA_ALG_FULL_LENGTH_MAC( alg ) ) ) { /* It's impossible to "truncate" to a larger length than the full length * of the algorithm. */ @@ -2301,11 +2303,14 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, goto exit; } + operation->is_sign = is_sign; + operation->mac_size = mac_size; + /* Dispatch the MAC setup call with validated input */ if( is_sign ) { status = psa_driver_wrapper_mac_sign_setup( operation, - &attributes, + &key_attributes, slot->key.data, slot->key.bytes, alg ); @@ -2313,7 +2318,7 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, else { status = psa_driver_wrapper_mac_verify_setup( operation, - &attributes, + &key_attributes, slot->key.data, slot->key.bytes, alg ); From 2dff3b2a1807fc4f9f28993b9b479486687d4f3a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 17 Jun 2021 16:33:22 +0200 Subject: [PATCH 55/66] psa: mac: Split psa_mac_setup() Split out of psa_mac_setup() the final checks on the requested algorithm and the key attributes. Signed-off-by: Ronald Cron --- library/psa_crypto.c | 70 +++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 02b2f99a9e..e7320f7157 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2240,33 +2240,13 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) return( status ); } -static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, - mbedtls_svc_key_id_t key, - psa_algorithm_t alg, - int is_sign ) +static psa_status_t psa_mac_finalize_alg_and_key_validation( + psa_algorithm_t alg, + const psa_key_attributes_t *attributes ) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; - psa_key_slot_t *slot; uint8_t mac_size; - /* A context must be freshly initialized before it can be set up. */ - if( operation->id != 0 ) - return( PSA_ERROR_BAD_STATE ); - - status = psa_get_and_lock_key_slot_with_policy( - key, - &slot, - is_sign ? PSA_KEY_USAGE_SIGN_HASH : PSA_KEY_USAGE_VERIFY_HASH, - alg ); - if( status != PSA_SUCCESS ) - return( status ); - - psa_key_attributes_t key_attributes = { - .core = slot->attr - }; - psa_key_attributes_t *attributes = &key_attributes; - if( ! PSA_ALG_IS_MAC( alg ) ) { status = PSA_ERROR_INVALID_ARGUMENT; @@ -2303,14 +2283,50 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, goto exit; } - operation->is_sign = is_sign; - operation->mac_size = mac_size; + status = PSA_SUCCESS; +exit: + return( status ); +} + +static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, + mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + int is_sign ) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_slot_t *slot; + + /* A context must be freshly initialized before it can be set up. */ + if( operation->id != 0 ) + return( PSA_ERROR_BAD_STATE ); + + status = psa_get_and_lock_key_slot_with_policy( + key, + &slot, + is_sign ? PSA_KEY_USAGE_SIGN_HASH : PSA_KEY_USAGE_VERIFY_HASH, + alg ); + if( status != PSA_SUCCESS ) + return( status ); + + psa_key_attributes_t attributes = { + .core = slot->attr + }; + + status = psa_mac_finalize_alg_and_key_validation( alg, &attributes ); + if( status != PSA_SUCCESS ) + goto exit; + + operation->is_sign = is_sign; + operation->mac_size = PSA_MAC_LENGTH( psa_get_key_type( &attributes ), + psa_get_key_bits( &attributes ), + alg ); /* Dispatch the MAC setup call with validated input */ if( is_sign ) { status = psa_driver_wrapper_mac_sign_setup( operation, - &key_attributes, + &attributes, slot->key.data, slot->key.bytes, alg ); @@ -2318,7 +2334,7 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, else { status = psa_driver_wrapper_mac_verify_setup( operation, - &key_attributes, + &attributes, slot->key.data, slot->key.bytes, alg ); From 79bdd82eaa18d0bdf19063272ae06f38c5c176b5 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 17 Jun 2021 16:46:44 +0200 Subject: [PATCH 56/66] psa: mac: Improve implementation of psa_mac_finalize_alg_and_key_validation() Signed-off-by: Ronald Cron --- library/psa_crypto.c | 43 ++++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e7320f7157..9e5e009324 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2242,51 +2242,42 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) static psa_status_t psa_mac_finalize_alg_and_key_validation( psa_algorithm_t alg, - const psa_key_attributes_t *attributes ) + const psa_key_attributes_t *attributes, + uint8_t *mac_size ) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - uint8_t mac_size; + psa_key_type_t key_type = psa_get_key_type( attributes ); + size_t key_bits = psa_get_key_bits( attributes ); if( ! PSA_ALG_IS_MAC( alg ) ) - { - status = PSA_ERROR_INVALID_ARGUMENT; - goto exit; - } + return( PSA_ERROR_INVALID_ARGUMENT ); /* Validate the combination of key type and algorithm */ - status = psa_mac_key_can_do( alg, psa_get_key_type( attributes ) ); + status = psa_mac_key_can_do( alg, key_type ); if( status != PSA_SUCCESS ) - goto exit; + return( status ); /* Get the output length for the algorithm and key combination */ - mac_size = PSA_MAC_LENGTH( psa_get_key_type( attributes ), - psa_get_key_bits( attributes ), - alg ); + *mac_size = PSA_MAC_LENGTH( key_type, key_bits, alg ); - if( mac_size < 4 ) + if( *mac_size < 4 ) { /* A very short MAC is too short for security since it can be * brute-forced. Ancient protocols with 32-bit MACs do exist, * so we make this our minimum, even though 32 bits is still * too small for security. */ - status = PSA_ERROR_NOT_SUPPORTED; - goto exit; + return( PSA_ERROR_NOT_SUPPORTED ); } - if( mac_size > PSA_MAC_LENGTH( psa_get_key_type( attributes ), - psa_get_key_bits( attributes ), - PSA_ALG_FULL_LENGTH_MAC( alg ) ) ) + if( *mac_size > PSA_MAC_LENGTH( key_type, key_bits, + PSA_ALG_FULL_LENGTH_MAC( alg ) ) ) { /* It's impossible to "truncate" to a larger length than the full length * of the algorithm. */ - status = PSA_ERROR_INVALID_ARGUMENT; - goto exit; + return( PSA_ERROR_INVALID_ARGUMENT ); } - status = PSA_SUCCESS; - -exit: - return( status ); + return( PSA_SUCCESS ); } static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, @@ -2314,14 +2305,12 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, .core = slot->attr }; - status = psa_mac_finalize_alg_and_key_validation( alg, &attributes ); + status = psa_mac_finalize_alg_and_key_validation( alg, &attributes, + &operation->mac_size ); if( status != PSA_SUCCESS ) goto exit; operation->is_sign = is_sign; - operation->mac_size = PSA_MAC_LENGTH( psa_get_key_type( &attributes ), - psa_get_key_bits( &attributes ), - alg ); /* Dispatch the MAC setup call with validated input */ if( is_sign ) { From 76be3e08a61141b4ee94c2e7779ee0b04db35b3a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 17 Jun 2021 17:34:43 +0200 Subject: [PATCH 57/66] psa: mac: Add MAC compute builtin implementation Signed-off-by: Ronald Cron --- library/psa_crypto_mac.c | 62 ++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/library/psa_crypto_mac.c b/library/psa_crypto_mac.c index 20c56a0214..135fa352da 100644 --- a/library/psa_crypto_mac.c +++ b/library/psa_crypto_mac.c @@ -359,30 +359,6 @@ static psa_status_t mac_setup( mbedtls_psa_mac_operation_t *operation, return( status ); } -static psa_status_t mac_compute( - const psa_key_attributes_t *attributes, - const uint8_t *key_buffer, - size_t key_buffer_size, - psa_algorithm_t alg, - const uint8_t *input, - size_t input_length, - uint8_t *mac, - size_t mac_size, - size_t *mac_length ) -{ - /* One-shot MAC has not been implemented in this PSA implementation yet. */ - (void) attributes; - (void) key_buffer; - (void) key_buffer_size; - (void) alg; - (void) input; - (void) input_length; - (void) mac; - (void) mac_size; - (void) mac_length; - return( PSA_ERROR_NOT_SUPPORTED ); -} - static psa_status_t mac_update( mbedtls_psa_mac_operation_t *operation, const uint8_t *input, @@ -497,6 +473,44 @@ cleanup: return( status ); } + +static psa_status_t mac_compute( + const psa_key_attributes_t *attributes, + const uint8_t *key_buffer, + size_t key_buffer_size, + psa_algorithm_t alg, + const uint8_t *input, + size_t input_length, + uint8_t *mac, + size_t mac_size, + size_t *mac_length ) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + mbedtls_psa_mac_operation_t operation = MBEDTLS_PSA_MAC_OPERATION_INIT; + + status = mac_setup( &operation, + attributes, key_buffer, key_buffer_size, + alg ); + if( status != PSA_SUCCESS ) + goto exit; + + if( input_length > 0 ) + { + status = mac_update( &operation, input, input_length ); + if( status != PSA_SUCCESS ) + goto exit; + } + + status = mac_finish_internal( &operation, mac, mac_size ); + if( status == PSA_SUCCESS ) + *mac_length = mac_size; + +exit: + mac_abort( &operation ); + + return( status ); +} + #endif /* BUILTIN_ALG_HMAC || BUILTIN_ALG_CMAC */ #if defined(MBEDTLS_PSA_BUILTIN_MAC) From 51131b53fe755c8e4f8801e6151a87e79fc06b75 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 17 Jun 2021 17:17:20 +0200 Subject: [PATCH 58/66] psa: mac: Add driver delegation support for psa_mac_compute() Signed-off-by: Ronald Cron --- library/psa_crypto.c | 50 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9e5e009324..6d01b4c4b5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2463,27 +2463,59 @@ psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, size_t *mac_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_slot_t *slot; + uint8_t operation_mac_size = 0; - status = psa_mac_sign_setup( &operation, key, alg ); + status = psa_get_and_lock_key_slot_with_policy( + key, &slot, PSA_KEY_USAGE_SIGN_HASH, alg ); if( status != PSA_SUCCESS ) goto exit; - status = psa_mac_update( &operation, input, input_length ); + psa_key_attributes_t attributes = { + .core = slot->attr + }; + + status = psa_mac_finalize_alg_and_key_validation( alg, &attributes, + &operation_mac_size ); if( status != PSA_SUCCESS ) goto exit; - status = psa_mac_sign_finish( &operation, mac, mac_size, mac_length ); - if( status != PSA_SUCCESS ) + if( mac_size < operation_mac_size ) + { + status = PSA_ERROR_BUFFER_TOO_SMALL; goto exit; + } + + status = psa_driver_wrapper_mac_compute( + &attributes, + slot->key.data, slot->key.bytes, + alg, + input, input_length, + mac, operation_mac_size, mac_length ); exit: - if ( status == PSA_SUCCESS ) - status = psa_mac_abort( &operation ); + if( status == PSA_SUCCESS ) + { + /* Set the excess room in the output buffer to an invalid value, to + * avoid potentially leaking a longer MAC. */ + if( mac_size > operation_mac_size ) + memset( &mac[operation_mac_size], + '!', + mac_size - operation_mac_size ); + } else - psa_mac_abort( &operation ); + { + /* Set the output length and content to a safe default, such that in + * case the caller misses an error check, the output would be an + * unachievable MAC. */ + *mac_length = mac_size; + memset( mac, '!', mac_size ); + } - return ( status ); + unlock_status = psa_unlock_key_slot( slot ); + + return( ( status == PSA_SUCCESS ) ? unlock_status : status ); } psa_status_t psa_mac_verify( mbedtls_svc_key_id_t key, From c3dd75f71b7e0bef1e013ead0bbb15ca517b48e1 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Jun 2021 13:05:48 +0200 Subject: [PATCH 59/66] psa: mac: Improve MAC finalization code Signed-off-by: Ronald Cron --- library/psa_crypto.c | 50 +++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 6d01b4c4b5..514ab40dad 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2403,24 +2403,22 @@ psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, mac, operation->mac_size, mac_length ); - if( status == PSA_SUCCESS ) + /* In case of success, set the potential excess room in the output buffer + * to an invalid value, to avoid potentially leaking a longer MAC. + * In case of error, set the output length and content to a safe default, + * such that in case the caller misses an error check, the output would be + * an unachievable MAC. + */ + if( status != PSA_SUCCESS ) { - /* Set the excess room in the output buffer to an invalid value, to - * avoid potentially leaking a longer MAC. */ - if( mac_size > operation->mac_size ) - memset( &mac[operation->mac_size], - '!', - mac_size - operation->mac_size ); - } - else - { - /* Set the output length and content to a safe default, such that in - * case the caller misses an error check, the output would be an - * unachievable MAC. */ *mac_length = mac_size; - memset( mac, '!', mac_size ); + operation->mac_size = 0; } + if( mac_size > operation->mac_size ) + memset( &mac[operation->mac_size], '!', + mac_size - operation->mac_size ); + abort_status = psa_mac_abort( operation ); return( status == PSA_SUCCESS ? abort_status : status ); @@ -2495,23 +2493,19 @@ psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, mac, operation_mac_size, mac_length ); exit: - if( status == PSA_SUCCESS ) + /* In case of success, set the potential excess room in the output buffer + * to an invalid value, to avoid potentially leaking a longer MAC. + * In case of error, set the output length and content to a safe default, + * such that in case the caller misses an error check, the output would be + * an unachievable MAC. + */ + if( status != PSA_SUCCESS ) { - /* Set the excess room in the output buffer to an invalid value, to - * avoid potentially leaking a longer MAC. */ - if( mac_size > operation_mac_size ) - memset( &mac[operation_mac_size], - '!', - mac_size - operation_mac_size ); - } - else - { - /* Set the output length and content to a safe default, such that in - * case the caller misses an error check, the output would be an - * unachievable MAC. */ *mac_length = mac_size; - memset( mac, '!', mac_size ); + operation_mac_size = 0; } + if( mac_size > operation_mac_size ) + memset( &mac[operation_mac_size], '!', mac_size - operation_mac_size ); unlock_status = psa_unlock_key_slot( slot ); From 094b06a5729b98d047b8a13fe8409f9a3f509bf1 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Jun 2021 14:01:50 +0200 Subject: [PATCH 60/66] psa: mac: Add driver dispatch tests for psa_mac_compute Signed-off-by: Ronald Cron --- ..._suite_psa_crypto_driver_wrappers.function | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index e86309b065..c9a2a47de7 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -1118,7 +1118,31 @@ void mac_sign( int key_type_arg, ASSERT_ALLOC( actual_mac, mac_buffer_size ); mbedtls_test_driver_mac_hooks.forced_status = forced_status; - /* Calculate the MAC. */ + /* + * Calculate the MAC, one-shot case. + */ + status = psa_mac_compute( key, alg, + input->x, input->len, + actual_mac, mac_buffer_size, + &mac_length ); + + TEST_EQUAL( mbedtls_test_driver_mac_hooks.hits, 1 ); + if( forced_status == PSA_SUCCESS || + forced_status == PSA_ERROR_NOT_SUPPORTED ) + { + PSA_ASSERT( status ); + } + else + TEST_EQUAL( forced_status, status ); + + if( mac_buffer_size > 0 ) + memset( actual_mac, 0, mac_buffer_size ); + mbedtls_test_driver_mac_hooks = mbedtls_test_driver_mac_hooks_init(); + mbedtls_test_driver_mac_hooks.forced_status = forced_status; + + /* + * Calculate the MAC, multipart case. + */ status = psa_mac_sign_setup( &operation, key, alg ); TEST_EQUAL( mbedtls_test_driver_mac_hooks.hits, 1 ); From cd989b55983363893c340a38fdb22a5bd8238491 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Jun 2021 14:23:33 +0200 Subject: [PATCH 61/66] psa: mac: Introduce psa_mac_compute_internal Introduce psa_mac_compute_internal with an additional `is_sign` parameter compared to the psa_mac_compute API. The intent is to call psa_mac_compute_internal() from psa_mac_verify() as well to compute the message MAC. Signed-off-by: Ronald Cron --- library/psa_crypto.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 514ab40dad..ec4f7d94e9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2452,13 +2452,14 @@ cleanup: return( status == PSA_SUCCESS ? abort_status : status ); } -psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, - psa_algorithm_t alg, - const uint8_t *input, - size_t input_length, - uint8_t *mac, - size_t mac_size, - size_t *mac_length) +static psa_status_t psa_mac_compute_internal( mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + const uint8_t *input, + size_t input_length, + uint8_t *mac, + size_t mac_size, + size_t *mac_length, + int is_sign ) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; @@ -2466,7 +2467,9 @@ psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, uint8_t operation_mac_size = 0; status = psa_get_and_lock_key_slot_with_policy( - key, &slot, PSA_KEY_USAGE_SIGN_HASH, alg ); + key, &slot, + is_sign ? PSA_KEY_USAGE_SIGN_HASH : PSA_KEY_USAGE_VERIFY_HASH, + alg ); if( status != PSA_SUCCESS ) goto exit; @@ -2512,6 +2515,19 @@ exit: return( ( status == PSA_SUCCESS ) ? unlock_status : status ); } +psa_status_t psa_mac_compute( mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + const uint8_t *input, + size_t input_length, + uint8_t *mac, + size_t mac_size, + size_t *mac_length) +{ + return( psa_mac_compute_internal( key, alg, + input, input_length, + mac, mac_size, mac_length, 1 ) ); +} + psa_status_t psa_mac_verify( mbedtls_svc_key_id_t key, psa_algorithm_t alg, const uint8_t *input, From a587cbc3a4ae6063efc2aeacf6f5b43c043bb4c5 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Jun 2021 14:51:29 +0200 Subject: [PATCH 62/66] psa: mac: Add driver delegation support for psa_mac_verify() Signed-off-by: Ronald Cron --- library/psa_crypto.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ec4f7d94e9..466845d8a5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2536,25 +2536,29 @@ psa_status_t psa_mac_verify( mbedtls_svc_key_id_t key, size_t mac_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + uint8_t actual_mac[PSA_MAC_MAX_SIZE]; + size_t actual_mac_length; - status = psa_mac_verify_setup( &operation, key, alg ); + status = psa_mac_compute_internal( key, alg, + input, input_length, + actual_mac, sizeof( actual_mac ), + &actual_mac_length, 0 ); if( status != PSA_SUCCESS ) goto exit; - status = psa_mac_update( &operation, input, input_length ); - if( status != PSA_SUCCESS ) + if( mac_length != actual_mac_length ) + { + status = PSA_ERROR_INVALID_SIGNATURE; goto exit; - - status = psa_mac_verify_finish( &operation, mac, mac_length ); - if( status != PSA_SUCCESS ) + } + if( mbedtls_psa_safer_memcmp( mac, actual_mac, actual_mac_length ) != 0 ) + { + status = PSA_ERROR_INVALID_SIGNATURE; goto exit; + } exit: - if ( status == PSA_SUCCESS ) - status = psa_mac_abort( &operation ); - else - psa_mac_abort( &operation ); + mbedtls_platform_zeroize( actual_mac, sizeof( actual_mac ) ); return ( status ); } From fb01081212814cf243aa9097c6d2f2ebd4c4efdd Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Jun 2021 15:05:36 +0200 Subject: [PATCH 63/66] psa: mac: Add driver dispatch tests for psa_mac_verify Signed-off-by: Ronald Cron --- ..._suite_psa_crypto_driver_wrappers.function | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index c9a2a47de7..3a9eff9557 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -1238,7 +1238,27 @@ void mac_verify( int key_type_arg, mbedtls_test_driver_mac_hooks.forced_status = forced_status; - /* Test the correct MAC. */ + /* + * Verify the MAC, one-shot case. + */ + status = psa_mac_verify( key, alg, + input->x, input->len, + expected_mac->x, expected_mac->len ); + TEST_EQUAL( mbedtls_test_driver_mac_hooks.hits, 1 ); + if( forced_status == PSA_SUCCESS || + forced_status == PSA_ERROR_NOT_SUPPORTED ) + { + PSA_ASSERT( status ); + } + else + TEST_EQUAL( forced_status, status ); + + mbedtls_test_driver_mac_hooks = mbedtls_test_driver_mac_hooks_init(); + mbedtls_test_driver_mac_hooks.forced_status = forced_status; + + /* + * Verify the MAC, multi-part case. + */ status = psa_mac_verify_setup( &operation, key, alg ); TEST_EQUAL( mbedtls_test_driver_mac_hooks.hits, 1 ); From 4d91bcd4135587ecc62b18e0bb745bf7a5876b7c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 21 Jun 2021 09:58:03 +0200 Subject: [PATCH 64/66] Add change log Signed-off-by: Ronald Cron --- ChangeLog.d/one-shot-mac.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/one-shot-mac.txt diff --git a/ChangeLog.d/one-shot-mac.txt b/ChangeLog.d/one-shot-mac.txt new file mode 100644 index 0000000000..112891decc --- /dev/null +++ b/ChangeLog.d/one-shot-mac.txt @@ -0,0 +1,3 @@ +Features + * Implement psa_mac_compute() and psa_mac_verify() as defined in the + PSA Cryptograpy API 1.0.0 specification. From 296fefeb981f8c95d580abfb46d52e1838a082eb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 21 Jun 2021 09:32:27 +0100 Subject: [PATCH 65/66] Fix return type of example key export callbacks Signed-off-by: Hanno Becker --- programs/ssl/ssl_test_common_source.c | 60 ++++++++++++--------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index 6da0ba4a50..d30c36ea56 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -25,47 +25,44 @@ */ #if defined(MBEDTLS_SSL_EXPORT_KEYS) -int eap_tls_key_derivation( void *p_expkey, - mbedtls_ssl_key_export_type secret_type, - const unsigned char *secret, - size_t secret_len, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type ) +void eap_tls_key_derivation( void *p_expkey, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, + const unsigned char client_random[32], + const unsigned char server_random[32], + mbedtls_tls_prf_types tls_prf_type ) { eap_tls_keys *keys = (eap_tls_keys *)p_expkey; /* We're only interested in the TLS 1.2 master secret */ if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) - return( 0 ); + return; if( secret_len != sizeof( keys->master_secret ) ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + return; memcpy( keys->master_secret, secret, sizeof( keys->master_secret ) ); memcpy( keys->randbytes, client_random, 32 ); memcpy( keys->randbytes + 32, server_random, 32 ); keys->tls_prf_type = tls_prf_type; - - return( 0 ); } -int nss_keylog_export( void *p_expkey, - mbedtls_ssl_key_export_type secret_type, - const unsigned char *secret, - size_t secret_len, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type ) +void nss_keylog_export( void *p_expkey, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, + const unsigned char client_random[32], + const unsigned char server_random[32], + mbedtls_tls_prf_types tls_prf_type ) { char nss_keylog_line[ 200 ]; size_t const client_random_len = 32; size_t len = 0; size_t j; - int ret = 0; /* We're only interested in the TLS 1.2 master secret */ if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) - return( 0 ); + return; ((void) p_expkey); ((void) server_random); @@ -102,13 +99,11 @@ int nss_keylog_export( void *p_expkey, if( ( f = fopen( opt.nss_keylog_file, "a" ) ) == NULL ) { - ret = -1; goto exit; } if( fwrite( nss_keylog_line, 1, len, f ) != len ) { - ret = -1; fclose( f ); goto exit; } @@ -119,32 +114,29 @@ int nss_keylog_export( void *p_expkey, exit: mbedtls_platform_zeroize( nss_keylog_line, sizeof( nss_keylog_line ) ); - return( ret ); } #if defined( MBEDTLS_SSL_DTLS_SRTP ) -int dtls_srtp_key_derivation( void *p_expkey, - mbedtls_ssl_key_export_type secret_type, - const unsigned char *secret, - size_t secret_len, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type ) +void dtls_srtp_key_derivation( void *p_expkey, + mbedtls_ssl_key_export_type secret_type, + const unsigned char *secret, + size_t secret_len, + const unsigned char client_random[32], + const unsigned char server_random[32], + mbedtls_tls_prf_types tls_prf_type ) { dtls_srtp_keys *keys = (dtls_srtp_keys *)p_expkey; /* We're only interested in the TLS 1.2 master secret */ if( secret_type != MBEDTLS_SSL_KEY_EXPORT_TLS12_MASTER_SECRET ) - return( 0 ); + return; if( secret_len != sizeof( keys->master_secret ) ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + return; memcpy( keys->master_secret, secret, sizeof( keys->master_secret ) ); memcpy( keys->randbytes, client_random, 32 ); memcpy( keys->randbytes + 32, server_random, 32 ); keys->tls_prf_type = tls_prf_type; - - return( 0 ); } #endif /* MBEDTLS_SSL_DTLS_SRTP */ From 5ec50039927601005534460385bbe9206dcf7ad0 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 22 Jun 2021 13:41:56 +0100 Subject: [PATCH 66/66] Document the return type change in the migration guide Signed-off-by: Dave Rodgman --- docs/3.0-migration-guide.d/key-export.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/3.0-migration-guide.d/key-export.md b/docs/3.0-migration-guide.d/key-export.md index 967ecf87bf..f8b3505b51 100644 --- a/docs/3.0-migration-guide.d/key-export.md +++ b/docs/3.0-migration-guide.d/key-export.md @@ -18,6 +18,10 @@ the previous key export API in the following ways: - A secret type parameter has been added to identify which key is being exported. For TLS 1.2, only the master secret is exported, but upcoming TLS 1.3 support will add other kinds of keys. +- The callback now specifies a void return type, rather than + returning an error code. It is the responsibility of the application + to handle failures in the key export callback, for example by + shutting down the TLS connection. For users which do not rely on raw keys and IV, adjusting to the new callback type should be straightforward - see the example programs