From de1adee51a7927cbd89028387881487bad0ff780 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 16:20:30 +0100 Subject: [PATCH 01/21] Rename ssl_cli/srv.c Rename ssl_cli.c and ssl_srv.c to reflect the fact that they are TLS 1.2 specific now. Align there new names with the TLS 1.3 ones. Signed-off-by: Ronald Cron --- include/mbedtls/mbedtls_config.h | 60 +++++++++++++---------- include/mbedtls/ssl.h | 5 +- library/CMakeLists.txt | 4 +- library/Makefile | 4 +- library/ssl_misc.h | 6 +-- library/{ssl_cli.c => ssl_tls12_client.c} | 0 library/{ssl_srv.c => ssl_tls12_server.c} | 0 tests/scripts/all.sh | 4 +- 8 files changed, 44 insertions(+), 39 deletions(-) rename library/{ssl_cli.c => ssl_tls12_client.c} (100%) rename library/{ssl_srv.c => ssl_tls12_server.c} (100%) diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 1c631b5267..996d9f6a57 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -2181,9 +2181,10 @@ * Enable the debug functions. * * Module: library/debug.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/ssl_msg.c * library/ssl_tls.c + * library/ssl_tls12_*.c + * library/ssl_tls13_*.c * * This module provides debugging functions. */ @@ -2211,8 +2212,9 @@ * Enable the Diffie-Hellman-Merkle module. * * Module: library/dhm.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c * * This module is used by the following key exchanges: * DHE-RSA, DHE-PSK @@ -2232,8 +2234,10 @@ * Enable the elliptic curve Diffie-Hellman library. * * Module: library/ecdh.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/psa_crypto.c + * library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c * * This module is used by the following key exchanges: * ECDHE-ECDSA, ECDHE-RSA, DHE-PSK @@ -2519,9 +2523,11 @@ * Enable the generic public (asymetric) key layer. * * Module: library/pk.c - * Caller: library/ssl_tls.c - * library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/psa_crypto_rsa.c + * library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c + * library/x509.c * * Requires: MBEDTLS_RSA_C or MBEDTLS_ECP_C * @@ -2689,10 +2695,11 @@ * * Module: library/rsa.c * library/rsa_alt_helpers.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/pk.c + * library/psa_crypto.c * library/ssl_tls.c - * library/x509.c + * library/ssl*_client.c + * library/ssl*_server.c * * This module is used by the following key exchanges: * RSA, DHE-RSA, ECDHE-RSA, RSA-PSK @@ -2708,10 +2715,7 @@ * * Module: library/sha1.c * Caller: library/md.c - * library/ssl_cli.c - * library/ssl_srv.c - * library/ssl_tls.c - * library/x509write_crt.c + * library/psa_crypto_hash.c * * This module is required for TLS 1.2 depending on the handshake parameters, * and for SHA1-signed certificates. @@ -2750,9 +2754,9 @@ * Module: library/sha256.c * Caller: library/entropy.c * library/md.c - * library/ssl_cli.c - * library/ssl_srv.c * library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c * * This module adds support for SHA-256. * This module is required for the SSL/TLS 1.2 PRF function. @@ -2818,8 +2822,10 @@ * * Module: library/sha512.c * Caller: library/md.c - * library/ssl_cli.c - * library/ssl_srv.c + * library/psa_crypto_hash.c + * library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c * * Comment to disable SHA-384 */ @@ -2879,7 +2885,7 @@ * * Enable the SSL/TLS client code. * - * Module: library/ssl_cli.c + * Module: library/ssl*_client.c * Caller: * * Requires: MBEDTLS_SSL_TLS_C @@ -2893,7 +2899,7 @@ * * Enable the SSL/TLS server code. * - * Module: library/ssl_srv.c + * Module: library/ssl*_server.c * Caller: * * Requires: MBEDTLS_SSL_TLS_C @@ -2908,8 +2914,8 @@ * Enable the generic SSL/TLS code. * * Module: library/ssl_tls.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c + * Caller: library/ssl*_client.c + * library/ssl*_server.c * * Requires: MBEDTLS_CIPHER_C, MBEDTLS_MD_C * and at least one of the MBEDTLS_SSL_PROTO_XXX defines @@ -2994,9 +3000,9 @@ * Enable X.509 certificate parsing. * * Module: library/x509_crt.c - * Caller: library/ssl_cli.c - * library/ssl_srv.c - * library/ssl_tls.c + * Caller: library/ssl_tls.c + * library/ssl*_client.c + * library/ssl*_server.c * * Requires: MBEDTLS_X509_USE_C * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b819bbad8f..a16c8e6f34 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -41,9 +41,8 @@ #endif /* Adding guard for MBEDTLS_ECDSA_C to ensure no compile errors due - * to guards also being in ssl_srv.c and ssl_cli.c. There is a gap - * in functionality that access to ecdh_ctx structure is needed for - * MBEDTLS_ECDSA_C which does not seem correct. + * to guards in TLS code. There is a gap in functionality that access to + * ecdh_ctx structure is needed for MBEDTLS_ECDSA_C which does not seem correct. */ #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) #include "mbedtls/ecdh.h" diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index ddede03901..6c3d7fdb49 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -99,12 +99,12 @@ set(src_tls net_sockets.c ssl_cache.c ssl_ciphersuites.c - ssl_cli.c ssl_cookie.c ssl_msg.c - ssl_srv.c ssl_ticket.c ssl_tls.c + ssl_tls12_client.c + ssl_tls12_server.c ssl_tls13_keys.c ssl_tls13_server.c ssl_tls13_client.c diff --git a/library/Makefile b/library/Makefile index e9c0a11744..d49d20cbcb 100644 --- a/library/Makefile +++ b/library/Makefile @@ -168,12 +168,12 @@ OBJS_TLS= \ net_sockets.o \ ssl_cache.o \ ssl_ciphersuites.o \ - ssl_cli.o \ ssl_cookie.o \ ssl_msg.o \ - ssl_srv.o \ ssl_ticket.o \ ssl_tls.o \ + ssl_tls12_client.o \ + ssl_tls12_server.o \ ssl_tls13_keys.o \ ssl_tls13_client.o \ ssl_tls13_server.o \ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 42563921eb..7b4311f218 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -623,9 +623,9 @@ struct mbedtls_ssl_handshake_params #endif /* Adding guard for MBEDTLS_ECDSA_C to ensure no compile errors due - * to guards also being in ssl_srv.c and ssl_cli.c. There is a gap - * in functionality that access to ecdh_ctx structure is needed for - * MBEDTLS_ECDSA_C which does not seem correct. + * to guards in client and server code. There is a gap in functionality that + * access to ecdh_ctx structure is needed for MBEDTLS_ECDSA_C which does not + * seem correct. */ #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) mbedtls_ecdh_context ecdh_ctx; /*!< ECDH key exchange */ diff --git a/library/ssl_cli.c b/library/ssl_tls12_client.c similarity index 100% rename from library/ssl_cli.c rename to library/ssl_tls12_client.c diff --git a/library/ssl_srv.c b/library/ssl_tls12_server.c similarity index 100% rename from library/ssl_srv.c rename to library/ssl_tls12_server.c diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 69b1fc83e2..e732ae47db 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2169,14 +2169,14 @@ component_build_no_std_function () { } component_build_no_ssl_srv () { - msg "build: full config except ssl_srv.c, make, gcc" # ~ 30s + msg "build: full config except SSL server, make, gcc" # ~ 30s scripts/config.py full scripts/config.py unset MBEDTLS_SSL_SRV_C make CC=gcc CFLAGS='-Werror -Wall -Wextra -O1' } component_build_no_ssl_cli () { - msg "build: full config except ssl_cli.c, make, gcc" # ~ 30s + msg "build: full config except SSL client, make, gcc" # ~ 30s scripts/config.py full scripts/config.py unset MBEDTLS_SSL_CLI_C make CC=gcc CFLAGS='-Werror -Wall -Wextra -O1' From a0855a6d135cd7c7e601507a79337c5fdd08ed2c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 10:04:54 +0100 Subject: [PATCH 02/21] ssl_tls13_client.c: alpn: Add missing return value assignment Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 165aa9d842..f9e5a134df 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -903,7 +903,7 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += output_len; #if defined(MBEDTLS_SSL_ALPN) - ssl_tls13_write_alpn_ext( ssl, p, end, &output_len ); + ret = ssl_tls13_write_alpn_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; From 13d8ea1dd92eadf545ab07635eb279000793a56a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 10:48:18 +0100 Subject: [PATCH 03/21] ssl_tls13_client.c: alpn: Loop only once over protocol names This has although the benefit of getting rid of a potential integer overflow (though very unlikely and probably harmless). Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index f9e5a134df..0c26b2dfaa 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -128,7 +128,7 @@ static int ssl_tls13_write_alpn_ext( mbedtls_ssl_context *ssl, size_t *olen ) { unsigned char *p = buf; - size_t alpnlen = 0; + size_t protocol_name_len; const char **cur; *olen = 0; @@ -138,13 +138,14 @@ static int ssl_tls13_write_alpn_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) ); - for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) - alpnlen += strlen( *cur ) + 1; - - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 + alpnlen ); + /* Check we have enough space for the extension type (2 bytes), the + * extension length (2 bytes) and the protocol_name_list length (2 bytes). + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_ALPN, p, 0 ); - p += 2; + /* Skip writing extension and list length for now */ + p += 6; /* * opaque ProtocolName<1..2^8-1>; @@ -153,19 +154,17 @@ static int ssl_tls13_write_alpn_ext( mbedtls_ssl_context *ssl, * ProtocolName protocol_name_list<2..2^16-1> * } ProtocolNameList; */ - - /* Skip writing extension and list length for now */ - p += 4; - for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) { /* * mbedtls_ssl_conf_set_alpn_protocols() checked that the length of * protocol names is less than 255. */ - *p = (unsigned char)strlen( *cur ); - memcpy( p + 1, *cur, *p ); - p += 1 + *p; + protocol_name_len = strlen( *cur ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + protocol_name_len ); + *p++ = (unsigned char)protocol_name_len; + memcpy( p, *cur, protocol_name_len ); + p += protocol_name_len; } *olen = p - buf; From 60ff79424eec7fd3c5a023729d57e7233748ebe8 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 13:56:48 +0100 Subject: [PATCH 04/21] ssl_tls13_client.c: alpn: Miscellanous minor improvements Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 0c26b2dfaa..5c07bc058a 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -113,7 +113,10 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_ALPN) /* - * ssl_tls13_write_alpn_ext( ) structure: + * ssl_tls13_write_alpn_ext() + * + * Structure of the application_layer_protocol_negotiation extension in + * ClientHello: * * opaque ProtocolName<1..2^8-1>; * @@ -123,15 +126,13 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, * */ static int ssl_tls13_write_alpn_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *olen ) + unsigned char *buf, + const unsigned char *end, + size_t *out_len ) { unsigned char *p = buf; - size_t protocol_name_len; - const char **cur; - *olen = 0; + *out_len = 0; if( ssl->conf->alpn_list == NULL ) return( 0 ); @@ -154,26 +155,27 @@ static int ssl_tls13_write_alpn_ext( mbedtls_ssl_context *ssl, * ProtocolName protocol_name_list<2..2^16-1> * } ProtocolNameList; */ - for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) + for( const char **cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) { /* * mbedtls_ssl_conf_set_alpn_protocols() checked that the length of * protocol names is less than 255. */ - protocol_name_len = strlen( *cur ); + size_t protocol_name_len = strlen( *cur ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + protocol_name_len ); *p++ = (unsigned char)protocol_name_len; memcpy( p, *cur, protocol_name_len ); p += protocol_name_len; } - *olen = p - buf; + *out_len = p - buf; - /* List length = olen - 2 (ext_type) - 2 (ext_len) - 2 (list_len) */ - MBEDTLS_PUT_UINT16_BE( *olen - 6, buf, 4 ); + /* List length = *out_len - 2 (ext_type) - 2 (ext_len) - 2 (list_len) */ + MBEDTLS_PUT_UINT16_BE( *out_len - 6, buf, 4 ); - /* Extension length = olen - 2 (ext_type) - 2 (ext_len) */ - MBEDTLS_PUT_UINT16_BE( *olen - 4, buf, 2 ); + /* Extension length = *out_len - 2 (ext_type) - 2 (ext_len) */ + MBEDTLS_PUT_UINT16_BE( *out_len - 4, buf, 2 ); return( 0 ); } From 8540cf66acad6283f97a0f35426488741d8b85aa Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 16 Mar 2022 08:01:09 +0100 Subject: [PATCH 05/21] ssl_tls.c: Propose PKCS1 v1.5 signatures with SHA_384/512 In case of TLS 1.3 and hybrid TLS 1.2/1.3, propose PKCS1 v1.5 signatures with SHA_384/512 not only SHA_256. There is no point in not proposing them if they are available. In TLS 1.3 those could be useful for certificate signature verification. In hybrid TLS 1.2/1.3 this allows to propose for TLS 1.2 the same set of signature algorithms. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5c65cc535f..3cd6f2726e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3939,6 +3939,14 @@ static uint16_t ssl_preset_default_sig_algs[] = { MBEDTLS_TLS1_3_SIG_RSA_PSS_RSAE_SHA256, #endif /* MBEDTLS_X509_RSASSA_PSS_SUPPORT && MBEDTLS_SHA256_C */ +#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_SHA512_C) + MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA512, +#endif /* MBEDTLS_RSA_C && MBEDTLS_SHA512_C */ + +#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_SHA384_C) + MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA384, +#endif /* MBEDTLS_RSA_C && MBEDTLS_SHA384_C */ + #if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_SHA256_C) MBEDTLS_TLS1_3_SIG_RSA_PKCS1_SHA256, #endif /* MBEDTLS_RSA_C && MBEDTLS_SHA256_C */ From 5b98ac9c64316e07befb197b923c1ac7f8c04255 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 15 Mar 2022 10:19:18 +0100 Subject: [PATCH 06/21] TLS 1.3: Move PSA ECDH private key destroy to dedicated function Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 15 ++++++++++++++- library/ssl_tls13_generic.c | 13 ------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5c07bc058a..7d1c826eb1 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -231,13 +231,26 @@ static int ssl_tls13_parse_alpn_ext( mbedtls_ssl_context *ssl, static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl ) { uint16_t group_id = ssl->handshake->offered_group_id; + if( group_id == 0 ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); #if defined(MBEDTLS_ECDH_C) if( mbedtls_ssl_tls13_named_group_is_ecdhe( group_id ) ) { - mbedtls_ecdh_free( &ssl->handshake->ecdh_ctx ); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + /* Destroy generated private key. */ + status = psa_destroy_key( ssl->handshake->ecdh_psa_privkey ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); + return( ret ); + } + + ssl->handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; return( 0 ); } else diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 856b4ea863..dab98a34f3 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1519,7 +1519,6 @@ int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ) size_t hash_len; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; uint16_t cipher_suite = ssl->session_negotiate->ciphersuite; - psa_status_t status = PSA_ERROR_GENERIC_ERROR; ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "Reset SSL session for HRR" ) ); @@ -1574,18 +1573,6 @@ int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ) ssl->handshake->update_checksum( ssl, hash_transcript, hash_len ); #endif /* MBEDTLS_SHA256_C || MBEDTLS_SHA384_C */ - /* Destroy generated private key. */ - status = psa_destroy_key( ssl->handshake->ecdh_psa_privkey ); - - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); - return( ret ); - } - - ssl->handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; - return( ret ); } From f12b81d387f78368dc081d453a61fab445644825 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 15 Mar 2022 10:42:41 +0100 Subject: [PATCH 07/21] ssl_tls.c: Fix PSA ECDH private key destruction In TLS 1.3, a PSA ECDH private key may be created even if MBEDTLS_SSL_USA_PSA_CRYPTO is disabled. We must destroy this key if still referenced by an handshake context when we free such context. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3cd6f2726e..9105830d1b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3111,8 +3111,8 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) mbedtls_ssl_buffering_free( ssl ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ -#if defined(MBEDTLS_ECDH_C) && \ - defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_ECDH_C) && \ + ( defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) ) psa_destroy_key( handshake->ecdh_psa_privkey ); #endif /* MBEDTLS_ECDH_C && MBEDTLS_USE_PSA_CRYPTO */ From 81591aa0f31d96f8576615239e10751bb9c2a6fe Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 09:05:51 +0100 Subject: [PATCH 08/21] ssl_tls.c: Remove ssl_set_handshake_prfs unnecessary minor_ver param ssl_set_handshake_prfs() is TLS 1.2 specific and only called from TLS 1.2 only code thus no need to pass the TLS minor version of the currebt session. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9105830d1b..0679a70ad3 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5226,20 +5226,16 @@ static int tls_prf_sha384( const unsigned char *secret, size_t slen, * Set appropriate PRF function and other SSL / TLS1.2 functions * * Inputs: - * - SSL/TLS minor version * - hash associated with the ciphersuite (only used by TLS 1.2) * * Outputs: * - the tls_prf, calc_verify and calc_finished members of handshake structure */ static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, - int minor_ver, mbedtls_md_type_t hash ) { - #if defined(MBEDTLS_SHA384_C) - if( minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 && - hash == MBEDTLS_MD_SHA384 ) + if( hash == MBEDTLS_MD_SHA384 ) { handshake->tls_prf = tls_prf_sha384; handshake->calc_verify = ssl_calc_verify_tls_sha384; @@ -5248,20 +5244,19 @@ static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, else #endif #if defined(MBEDTLS_SHA256_C) - if( minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { + (void) hash; handshake->tls_prf = tls_prf_sha256; handshake->calc_verify = ssl_calc_verify_tls_sha256; handshake->calc_finished = ssl_calc_finished_tls_sha256; } - else -#endif +#else { - (void) hash; - (void) minor_ver; (void) handshake; + (void) hash; return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } +#endif return( 0 ); } @@ -5439,7 +5434,6 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) /* Set PRF, calc_verify and calc_finished function pointers */ ret = ssl_set_handshake_prfs( ssl->handshake, - ssl->minor_ver, ciphersuite_info->mac ); if( ret != 0 ) { From 4dcbca952e4ad318ad3057a7190b50a8802f2ca3 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 10:21:40 +0100 Subject: [PATCH 09/21] ssl_tls.c: Move mbedtls_ssl_set_calc_verify_md() to TLS 1.2 section In ssl_tls.c, move mbedtls_ssl_set_calc_verify_md() under the "if defined(MBEDTLS_SSL_PROTO_TLS1_2)" pre-processor directive as it is specific to TLS 1.2. Signed-off-by: Ronald Cron --- library/ssl_misc.h | 3 +++ library/ssl_tls.c | 62 +++++++++++++++++++++++----------------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 7b4311f218..47976a868e 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1336,7 +1336,10 @@ mbedtls_pk_type_t mbedtls_ssl_pk_alg_from_sig( unsigned char sig ); mbedtls_md_type_t mbedtls_ssl_md_alg_from_hash( unsigned char hash ); unsigned char mbedtls_ssl_hash_from_md_alg( int md ); + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ); +#endif int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id ); #if defined(MBEDTLS_ECP_C) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0679a70ad3..c4a9a29b9c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4517,37 +4517,6 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, } #endif /* MBEDTLS_X509_CRT_PARSE_C */ -int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) -{ -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) - return( -1 ); - - switch( md ) - { -#if defined(MBEDTLS_SHA384_C) - case MBEDTLS_SSL_HASH_SHA384: - ssl->handshake->calc_verify = ssl_calc_verify_tls_sha384; - break; -#endif -#if defined(MBEDTLS_SHA256_C) - case MBEDTLS_SSL_HASH_SHA256: - ssl->handshake->calc_verify = ssl_calc_verify_tls_sha256; - break; -#endif - default: - return( -1 ); - } - - return 0; -#else /* !MBEDTLS_SSL_PROTO_TLS1_2 */ - (void) ssl; - (void) md; - - return( -1 ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ -} - #if defined(MBEDTLS_USE_PSA_CRYPTO) int mbedtls_ssl_get_handshake_transcript( mbedtls_ssl_context *ssl, const mbedtls_md_type_t md, @@ -5491,6 +5460,37 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) return( 0 ); } +int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) +{ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + return( -1 ); + + switch( md ) + { +#if defined(MBEDTLS_SHA384_C) + case MBEDTLS_SSL_HASH_SHA384: + ssl->handshake->calc_verify = ssl_calc_verify_tls_sha384; + break; +#endif +#if defined(MBEDTLS_SHA256_C) + case MBEDTLS_SSL_HASH_SHA256: + ssl->handshake->calc_verify = ssl_calc_verify_tls_sha256; + break; +#endif + default: + return( -1 ); + } + + return 0; +#else /* !MBEDTLS_SSL_PROTO_TLS1_2 */ + (void) ssl; + (void) md; + + return( -1 ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +} + #if defined(MBEDTLS_SHA256_C) void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *ssl, unsigned char *hash, From c2f13a05680247b939079f0d02af8c02f89b85e8 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 10:25:24 +0100 Subject: [PATCH 10/21] ssl_tls.c: Modify mbedtls_ssl_set_calc_verify_md() Modify mbedtls_ssl_set_calc_verify_md() taking into account that it is an TLS 1.2 only function. Signed-off-by: Ronald Cron --- library/ssl_tls.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c4a9a29b9c..82dd7b59d1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5462,10 +5462,6 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) { -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) - return( -1 ); - switch( md ) { #if defined(MBEDTLS_SHA384_C) @@ -5482,13 +5478,7 @@ int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) return( -1 ); } - return 0; -#else /* !MBEDTLS_SSL_PROTO_TLS1_2 */ - (void) ssl; - (void) md; - - return( -1 ); -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + return( 0 ); } #if defined(MBEDTLS_SHA256_C) From a25cf58681434c547081b5df77225659e7c90fc9 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 11:10:36 +0100 Subject: [PATCH 11/21] ssl_tls.c: Remove one unnecessary minor version check Signed-off-by: Ronald Cron --- library/ssl_tls.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 82dd7b59d1..e174219d45 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5264,7 +5264,7 @@ static int ssl_use_opaque_psk( mbedtls_ssl_context const *ssl ) * [in] ssl: optionally used for debugging, EMS and PSA-PSK * debug: conf->f_dbg, conf->p_dbg * EMS: passed to calc_verify (debug + session_negotiate) - * PSA-PSA: minor_ver, conf + * PSA-PSA: conf */ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, unsigned char *master, @@ -5325,7 +5325,6 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, #if defined(MBEDTLS_USE_PSA_CRYPTO) && \ defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) if( handshake->ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK && - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 && ssl_use_opaque_psk( ssl ) == 1 ) { /* Perform PSK-to-MS expansion in a single step. */ From 90915f2a21c78d297570205f36af1b537f126360 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 11:11:36 +0100 Subject: [PATCH 12/21] ssl_tls12_client.c: Remove some unnecessary checks on TLS minor version ssl_tls12_client.c contains only TLS 1.2 specific code thus remove some checks on the minor version version being MBEDTLS_SSL_MINOR_VERSION_3. No aim for completeness, ssl_parse_server_hello() is not reworked here for example. Signed-off-by: Ronald Cron --- library/ssl_tls12_client.c | 194 ++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 111 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 88427effb5..ab8a69f7d7 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2531,12 +2531,6 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, *md_alg = MBEDTLS_MD_NONE; *pk_alg = MBEDTLS_PK_NONE; - /* Only in TLS 1.2 */ - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) - { - return( 0 ); - } - if( (*p) + 2 > end ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); @@ -2903,36 +2897,28 @@ start_processing: /* * Handle the digitally-signed structure */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( ssl_parse_signature_algorithm( ssl, &p, end, + &md_alg, &pk_alg ) != 0 ) { - if( ssl_parse_signature_algorithm( ssl, &p, end, - &md_alg, &pk_alg ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( - ssl, - MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } - - if( pk_alg != - mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( - ssl, - MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - else + + if( pk_alg != + mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } /* @@ -3074,6 +3060,10 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) size_t cert_type_len = 0, dn_len = 0; const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->handshake->ciphersuite_info; + size_t sig_alg_len; +#if defined(MBEDTLS_DEBUG_C) + unsigned char *sig_alg; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); @@ -3170,52 +3160,43 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) } /* supported_signature_algorithms */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + sig_alg_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) + | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); + + /* + * The furthest access in buf is in the loop few lines below: + * sig_alg[i + 1], + * where: + * sig_alg = buf + ...hdr_len + 3 + n, + * max(i) = sig_alg_len - 1. + * Therefore the furthest access is: + * buf[...hdr_len + 3 + n + sig_alg_len - 1 + 1], + * which reduces to: + * buf[...hdr_len + 3 + n + sig_alg_len], + * which is one less than we need the buf to be. + */ + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n + sig_alg_len ) { - size_t sig_alg_len = - ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) - | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); -#if defined(MBEDTLS_DEBUG_C) - unsigned char* sig_alg; - size_t i; -#endif - - /* - * The furthest access in buf is in the loop few lines below: - * sig_alg[i + 1], - * where: - * sig_alg = buf + ...hdr_len + 3 + n, - * max(i) = sig_alg_len - 1. - * Therefore the furthest access is: - * buf[...hdr_len + 3 + n + sig_alg_len - 1 + 1], - * which reduces to: - * buf[...hdr_len + 3 + n + sig_alg_len], - * which is one less than we need the buf to be. - */ - if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) - + 3 + n + sig_alg_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - mbedtls_ssl_send_alert_message( - ssl, - MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - -#if defined(MBEDTLS_DEBUG_C) - sig_alg = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n; - for( i = 0; i < sig_alg_len; i += 2 ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "Supported Signature Algorithm found: %d,%d", - sig_alg[i], sig_alg[i + 1] ) ); - } -#endif - - n += 2 + sig_alg_len; + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } +#if defined(MBEDTLS_DEBUG_C) + sig_alg = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n; + for( size_t i = 0; i < sig_alg_len; i += 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "Supported Signature Algorithm found: %d,%d", + sig_alg[i], sig_alg[i + 1] ) ); + } +#endif + + n += 2 + sig_alg_len; + /* certificate_authorities */ dn_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); @@ -3612,7 +3593,6 @@ ecdh_calc_secret: #if defined(MBEDTLS_USE_PSA_CRYPTO) && \ defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK && - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 && ssl_conf_has_static_raw_psk( ssl->conf ) == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, @@ -3783,45 +3763,37 @@ sign: ssl->handshake->calc_verify( ssl, hash, &hashlen ); - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + /* + * digitally-signed struct { + * opaque handshake_messages[handshake_messages_length]; + * }; + * + * Taking shortcut here. We assume that the server always allows the + * PRF Hash function and has sent it in the allowed signature + * algorithms list received in the Certificate Request message. + * + * Until we encounter a server that does not, we will take this + * shortcut. + * + * Reason: Otherwise we should have running hashes for SHA512 and + * SHA224 in order to satisfy 'weird' needs from the server + * side. + */ + if( ssl->handshake->ciphersuite_info->mac == MBEDTLS_MD_SHA384 ) { - /* - * digitally-signed struct { - * opaque handshake_messages[handshake_messages_length]; - * }; - * - * Taking shortcut here. We assume that the server always allows the - * PRF Hash function and has sent it in the allowed signature - * algorithms list received in the Certificate Request message. - * - * Until we encounter a server that does not, we will take this - * shortcut. - * - * Reason: Otherwise we should have running hashes for SHA512 and - * SHA224 in order to satisfy 'weird' needs from the server - * side. - */ - if( ssl->handshake->ciphersuite_info->mac == MBEDTLS_MD_SHA384 ) - { - md_alg = MBEDTLS_MD_SHA384; - ssl->out_msg[4] = MBEDTLS_SSL_HASH_SHA384; - } - else - { - md_alg = MBEDTLS_MD_SHA256; - ssl->out_msg[4] = MBEDTLS_SSL_HASH_SHA256; - } - ssl->out_msg[5] = mbedtls_ssl_sig_from_pk( mbedtls_ssl_own_key( ssl ) ); - - /* Info from md_alg will be used instead */ - hashlen = 0; - offset = 2; + md_alg = MBEDTLS_MD_SHA384; + ssl->out_msg[4] = MBEDTLS_SSL_HASH_SHA384; } else { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + md_alg = MBEDTLS_MD_SHA256; + ssl->out_msg[4] = MBEDTLS_SSL_HASH_SHA256; } + ssl->out_msg[5] = mbedtls_ssl_sig_from_pk( mbedtls_ssl_own_key( ssl ) ); + + /* Info from md_alg will be used instead */ + hashlen = 0; + offset = 2; #if defined(MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED) if( ssl->handshake->ecrs_enabled ) From b894ac7f9919aa742371fd35da000d708e5584ab Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 11:56:06 +0100 Subject: [PATCH 13/21] ssl_tls12_server.c: Remove some dead code for versions of TLS < 1.2 Signed-off-by: Ronald Cron --- library/ssl_tls12_server.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index e9fa63311f..deab271d3b 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1011,23 +1011,6 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, } #endif - /* - * Try to select a SHA-1 certificate for pre-1.2 clients, but still - * present them a SHA-higher cert rather than failing if it's the only - * one we got that satisfies the other conditions. - */ - if( ssl->minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 && - cur->cert->sig_md != MBEDTLS_MD_SHA1 ) - { - if( fallback == NULL ) - fallback = cur; - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "certificate not preferred: " - "sha-2 with pre-TLS 1.2 client" ) ); - continue; - } - } - /* If we get there, we got a winner */ break; } From 8457c1212795ca25d05198d0bcfb003cbb996545 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Mon, 7 Mar 2022 11:32:54 +0100 Subject: [PATCH 14/21] ssl_tls12_server.c: Remove some unnecessary checks on TLS minor version Signed-off-by: Ronald Cron --- library/ssl_tls12_server.c | 223 ++++++++++++++++--------------------- 1 file changed, 95 insertions(+), 128 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index deab271d3b..5bbcd63319 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1103,16 +1103,13 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* If the ciphersuite requires signing, check whether * a suitable hash algorithm is present. */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + sig_type = mbedtls_ssl_get_ciphersuite_sig_alg( suite_info ); + if( sig_type != MBEDTLS_PK_NONE && + mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_type ) == MBEDTLS_MD_NONE ) { - sig_type = mbedtls_ssl_get_ciphersuite_sig_alg( suite_info ); - if( sig_type != MBEDTLS_PK_NONE && - mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, sig_type ) == MBEDTLS_MD_NONE ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: no suitable hash algorithm " - "for signature algorithm %u", (unsigned) sig_type ) ); - return( 0 ); - } + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: no suitable hash algorithm " + "for signature algorithm %u", (unsigned) sig_type ) ); + return( 0 ); } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ @@ -1945,21 +1942,18 @@ have_ciphersuite: /* Debugging-only output for testsuite */ #if defined(MBEDTLS_DEBUG_C) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_alg( ciphersuite_info ); + if( sig_alg != MBEDTLS_PK_NONE ) { - mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_alg( ciphersuite_info ); - if( sig_alg != MBEDTLS_PK_NONE ) - { - mbedtls_md_type_t md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, - sig_alg ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", - mbedtls_ssl_hash_from_md_alg( md_alg ) ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm " - "%u - should not happen", (unsigned) sig_alg ) ); - } + mbedtls_md_type_t md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, + sig_alg ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, signature_algorithm ext: %d", + mbedtls_ssl_hash_from_md_alg( md_alg ) ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "no hash algorithm for signature algorithm " + "%u - should not happen", (unsigned) sig_alg ) ); } #endif @@ -2794,33 +2788,27 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) * enum { (255) } HashAlgorithm; * enum { (255) } SignatureAlgorithm; */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + const uint16_t *sig_alg = mbedtls_ssl_get_sig_algs( ssl ); + if( sig_alg == NULL ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + + for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) { - /* - * Supported signature algorithms - */ - const uint16_t *sig_alg = mbedtls_ssl_get_sig_algs( ssl ); - if( sig_alg == NULL ) - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + unsigned char hash = MBEDTLS_BYTE_1( *sig_alg ); - for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) - { - unsigned char hash = MBEDTLS_BYTE_1( *sig_alg ); + if( mbedtls_ssl_set_calc_verify_md( ssl, hash ) ) + continue; + if( ! mbedtls_ssl_sig_alg_is_supported( ssl, *sig_alg ) ) + continue; - if( mbedtls_ssl_set_calc_verify_md( ssl, hash ) ) - continue; - if( ! mbedtls_ssl_sig_alg_is_supported( ssl, *sig_alg ) ) - continue; - - MBEDTLS_PUT_UINT16_BE( *sig_alg, p, sa_len ); - sa_len += 2; - } - - MBEDTLS_PUT_UINT16_BE( sa_len, p, 0 ); + MBEDTLS_PUT_UINT16_BE( *sig_alg, p, sa_len ); sa_len += 2; - p += sa_len; } + MBEDTLS_PUT_UINT16_BE( sa_len, p, 0 ); + sa_len += 2; + p += sa_len; + /* * DistinguishedName certificate_authorities<0..2^16-1>; * opaque DistinguishedName<1..2^16-1>; @@ -3243,26 +3231,18 @@ curve_matching_done: */ mbedtls_md_type_t md_alg; - mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) - { - /* For TLS 1.2, obey signature-hash-algorithm extension - * (RFC 5246, Sec. 7.4.1.4.1). */ - if( sig_alg == MBEDTLS_PK_NONE || - ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, - sig_alg ) ) == MBEDTLS_MD_NONE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - /* (... because we choose a cipher suite - * only if there is a matching hash.) */ - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - } - else + + /* For TLS 1.2, obey signature-hash-algorithm extension + * (RFC 5246, Sec. 7.4.1.4.1). */ + if( sig_alg == MBEDTLS_PK_NONE || + ( md_alg = mbedtls_ssl_sig_hash_set_find( &ssl->handshake->hash_algs, + sig_alg ) ) == MBEDTLS_MD_NONE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + /* (... because we choose a cipher suite + * only if there is a matching hash.) */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } @@ -3291,29 +3271,24 @@ curve_matching_done: /* * 2.3: Compute and add the signature */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) - { - /* - * For TLS 1.2, we need to specify signature and hash algorithm - * explicitly through a prefix to the signature. - * - * struct { - * HashAlgorithm hash; - * SignatureAlgorithm signature; - * } SignatureAndHashAlgorithm; - * - * struct { - * SignatureAndHashAlgorithm algorithm; - * opaque signature<0..2^16-1>; - * } DigitallySigned; - * - */ + /* + * We need to specify signature and hash algorithm explicitly through + * a prefix to the signature. + * + * struct { + * HashAlgorithm hash; + * SignatureAlgorithm signature; + * } SignatureAndHashAlgorithm; + * + * struct { + * SignatureAndHashAlgorithm algorithm; + * opaque signature<0..2^16-1>; + * } DigitallySigned; + * + */ - ssl->out_msg[ssl->out_msglen++] = - mbedtls_ssl_hash_from_md_alg( md_alg ); - ssl->out_msg[ssl->out_msglen++] = - mbedtls_ssl_sig_from_pk_alg( sig_alg ); - } + ssl->out_msg[ssl->out_msglen++] = mbedtls_ssl_hash_from_md_alg( md_alg ); + ssl->out_msg[ssl->out_msglen++] = mbedtls_ssl_sig_from_pk_alg( sig_alg ); #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) if( ssl->conf->f_async_sign_start != NULL ) @@ -4261,64 +4236,56 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) * opaque signature<0..2^16-1>; * } DigitallySigned; */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( i + 2 > ssl->in_hslen ) { - if( i + 2 > ssl->in_hslen ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate verify message" ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate verify message" ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } - /* - * Hash - */ - md_alg = mbedtls_ssl_md_alg_from_hash( ssl->in_msg[i] ); + /* + * Hash + */ + md_alg = mbedtls_ssl_md_alg_from_hash( ssl->in_msg[i] ); - if( md_alg == MBEDTLS_MD_NONE || mbedtls_ssl_set_calc_verify_md( ssl, ssl->in_msg[i] ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "peer not adhering to requested sig_alg" - " for verify message" ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } + if( md_alg == MBEDTLS_MD_NONE || mbedtls_ssl_set_calc_verify_md( ssl, ssl->in_msg[i] ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "peer not adhering to requested sig_alg" + " for verify message" ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } #if !defined(MBEDTLS_MD_SHA1) - if( MBEDTLS_MD_SHA1 == md_alg ) - hash_start += 16; + if( MBEDTLS_MD_SHA1 == md_alg ) + hash_start += 16; #endif - /* Info from md_alg will be used instead */ - hashlen = 0; + /* Info from md_alg will be used instead */ + hashlen = 0; - i++; + i++; - /* - * Signature - */ - if( ( pk_alg = mbedtls_ssl_pk_alg_from_sig( ssl->in_msg[i] ) ) - == MBEDTLS_PK_NONE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "peer not adhering to requested sig_alg" - " for verify message" ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } - - /* - * Check the certificate's key type matches the signature alg - */ - if( !mbedtls_pk_can_do( peer_pk, pk_alg ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "sig_alg doesn't match cert key" ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - } - - i++; - } - else + /* + * Signature + */ + if( ( pk_alg = mbedtls_ssl_pk_alg_from_sig( ssl->in_msg[i] ) ) + == MBEDTLS_PK_NONE ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "peer not adhering to requested sig_alg" + " for verify message" ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } + /* + * Check the certificate's key type matches the signature alg + */ + if( !mbedtls_pk_can_do( peer_pk, pk_alg ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "sig_alg doesn't match cert key" ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + + i++; + if( i + 2 > ssl->in_hslen ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate verify message" ) ); From 086ee0be0e41cee2bc8b8681920be58d256f07c5 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 15 Mar 2022 15:18:51 +0100 Subject: [PATCH 15/21] ssl_tls.c: Reject TLS 1.3 version configuration for server Signed-off-by: Ronald Cron --- library/ssl_tls.c | 19 ++++++++++++++----- tests/ssl-opt.sh | 15 ++++++--------- tests/suites/test_suite_ssl.data | 1 + 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e174219d45..6a1bfa8f5a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -853,12 +853,21 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ) static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) { + const mbedtls_ssl_config *conf = ssl->conf; + #if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( mbedtls_ssl_conf_is_tls13_only( ssl->conf ) ) + if( mbedtls_ssl_conf_is_tls13_enabled( conf ) && + ( conf->endpoint == MBEDTLS_SSL_IS_SERVER ) ) { - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 server is not supported yet." ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } + + if( mbedtls_ssl_conf_is_tls13_only( conf ) ) + { + if( conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS 1.3 is not yet supported" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS 1.3 is not yet supported." ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is tls13 only." ) ); @@ -867,7 +876,7 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( mbedtls_ssl_conf_is_tls12_only( ssl->conf ) ) + if( mbedtls_ssl_conf_is_tls12_only( conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is tls12 only." ) ); return( 0 ); @@ -875,7 +884,7 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( mbedtls_ssl_conf_is_hybrid_tls12_tls13( ssl->conf ) ) + if( mbedtls_ssl_conf_is_hybrid_tls12_tls13( conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Hybrid TLS 1.2 + TLS 1.3 configurations are not yet supported" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index aff2411e6e..03351d419d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9630,26 +9630,23 @@ run_test "TLS 1.3: Test gnutls tls1_3 feature" \ -c "Version: TLS1.3" # TLS1.3 test cases -# TODO: remove or rewrite this test case if #4832 is resolved. requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 skip_handshake_stage_check run_test "TLS 1.3: Not supported version check: tls12 and tls13" \ - "$P_SRV debug_level=1 min_version=tls12 max_version=tls13" \ + "$P_SRV debug_level=1" \ "$P_CLI debug_level=1 min_version=tls12 max_version=tls13" \ 1 \ - -s "SSL - The requested feature is not available" \ -c "SSL - The requested feature is not available" \ - -s "Hybrid TLS 1.2 + TLS 1.3 configurations are not yet supported" \ -c "Hybrid TLS 1.2 + TLS 1.3 configurations are not yet supported" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -run_test "TLS 1.3: handshake dispatch test: tls13 only" \ - "$P_SRV debug_level=2 min_version=tls13 max_version=tls13" \ - "$P_CLI debug_level=2 min_version=tls13 max_version=tls13" \ +skip_handshake_stage_check +run_test "TLS 1.3: No server support" \ + "$P_SRV debug_level=2 force_version=tls13" \ + "$P_CLI debug_level=2 force_version=tls13" \ 1 \ - -s "tls13 server state: MBEDTLS_SSL_HELLO_REQUEST" \ - -c "tls13 client state: MBEDTLS_SSL_HELLO_REQUEST" + -s "TLS 1.3 server is not supported yet." requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 7de562afd1..e330016f51 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -101,6 +101,7 @@ Test mbedtls_endpoint sanity for the client mbedtls_endpoint_sanity:MBEDTLS_SSL_IS_CLIENT Test mbedtls_endpoint sanity for the server +depends_on:MBEDTLS_SSL_PROTO_TLS1_2 mbedtls_endpoint_sanity:MBEDTLS_SSL_IS_SERVER Test moving clients handshake to state: HELLO_REQUEST From 90f012037dda434ab75a81f74c950616f5752f3c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 15 Mar 2022 15:37:13 +0100 Subject: [PATCH 16/21] ssl_tls12_server.c: Simplify TLS version check in ClientHello The TLS server code only support TLS 1.2 thus simplify the check of the version proposed by the client. Signed-off-by: Ronald Cron --- library/ssl_misc.h | 2 -- library/ssl_tls12_server.c | 24 +++++------------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 47976a868e..289a64a1d6 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -539,8 +539,6 @@ struct mbedtls_ssl_handshake_params { /* Frequently-used boolean or byte fields (placed early to take * advantage of smaller code size for indirect access on Arm Thumb) */ - uint8_t max_major_ver; /*!< max. major version client*/ - uint8_t max_minor_ver; /*!< max. minor version client*/ uint8_t resume; /*!< session resume indicator*/ uint8_t cli_exts; /*!< client extension presence*/ diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 5bbcd63319..fe81e34d39 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1415,29 +1415,15 @@ read_record_header: ssl->conf->transport, buf ); ssl->session_negotiate->minor_ver = ssl->minor_ver; - ssl->handshake->max_major_ver = ssl->major_ver; - ssl->handshake->max_minor_ver = ssl->minor_ver; - - if( ssl->major_ver < ssl->conf->min_major_ver || - ssl->minor_ver < ssl->conf->min_minor_ver ) + if( ( ssl->major_ver != MBEDTLS_SSL_MAJOR_VERSION_3 ) || + ( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" - " [%d:%d] < [%d:%d]", - ssl->major_ver, ssl->minor_ver, - ssl->conf->min_major_ver, ssl->conf->min_minor_ver ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "server only supports TLS 1.2" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); return( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); } - if( ssl->major_ver > ssl->conf->max_major_ver ) - { - ssl->major_ver = ssl->conf->max_major_ver; - ssl->minor_ver = ssl->conf->max_minor_ver; - } - else if( ssl->minor_ver > ssl->conf->max_minor_ver ) - ssl->minor_ver = ssl->conf->max_minor_ver; - /* * Save client random (inc. Unix time) */ @@ -3660,8 +3646,8 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, return( ret ); #endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ - mbedtls_ssl_write_version( ssl->handshake->max_major_ver, - ssl->handshake->max_minor_ver, + mbedtls_ssl_write_version( MBEDTLS_SSL_MAJOR_VERSION_3, + MBEDTLS_SSL_MINOR_VERSION_3, ssl->conf->transport, ver ); /* Avoid data-dependent branches while checking for invalid From 4e263fd49cc750dac2b09e20b68f8b9304a42534 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Tue, 15 Mar 2022 15:54:17 +0100 Subject: [PATCH 17/21] ssl_tls12_client.c: Simplify TLS version in encrypted PMS This can only be TLS 1.2 now in this structure and when adding support for TLS 1.2 or 1.3 version negotiation the highest configured version can be TLS 1.3. Signed-off-by: Ronald Cron --- library/ssl_tls12_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index ab8a69f7d7..67bcd9a99b 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -2461,8 +2461,8 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, * opaque random[46]; * } PreMasterSecret; */ - mbedtls_ssl_write_version( ssl->conf->max_major_ver, - ssl->conf->max_minor_ver, + mbedtls_ssl_write_version( MBEDTLS_SSL_MAJOR_VERSION_3, + MBEDTLS_SSL_MINOR_VERSION_3, ssl->conf->transport, p ); if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, p + 2, 46 ) ) != 0 ) From 12dcdf0d6e779d3d13c2033aa43b2d063a8e3c87 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 16 Feb 2022 15:28:22 +0100 Subject: [PATCH 18/21] ssl_tls12_client.c: Move writing of TLS 1.2 specific extensions Signed-off-by: Ronald Cron --- library/ssl_tls12_client.c | 219 ++++++++++++++++++++----------------- 1 file changed, 121 insertions(+), 98 deletions(-) diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 67bcd9a99b..7b609e9f2a 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -681,6 +681,118 @@ static int ssl_validate_ciphersuite( return( 0 ); } +static int ssl_tls12_write_client_hello_exts( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + int uses_ec, + size_t *out_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *p = buf; + size_t ext_len = 0; + + (void) ssl; + (void) end; + (void) uses_ec; + (void) ret; + (void) ext_len; + + *out_len = 0; + + /* Note that TLS_EMPTY_RENEGOTIATION_INFO_SCSV is always added + * even if MBEDTLS_SSL_RENEGOTIATION is not defined. */ +#if defined(MBEDTLS_SSL_RENEGOTIATION) + if( ( ret = ssl_write_renegotiation_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_renegotiation_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( uses_ec ) + { + if( ( ret = ssl_write_supported_point_formats_ext( ssl, p, end, + &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_point_formats_ext", ret ); + return( ret ); + } + p += ext_len; + } +#endif + +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( ( ret = ssl_write_ecjpake_kkpp_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_ecjpake_kkpp_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) + if( ( ret = ssl_write_cid_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_cid_ext", ret ); + return( ret ); + } + p += ext_len; +#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ + +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + if( ( ret = ssl_write_max_fragment_length_ext( ssl, p, end, + &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_max_fragment_length_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) + if( ( ret = ssl_write_encrypt_then_mac_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_encrypt_then_mac_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) + if( ( ret = ssl_write_extended_ms_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_extended_ms_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_SSL_DTLS_SRTP) + if( ( ret = ssl_write_use_srtp_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_use_srtp_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + +#if defined(MBEDTLS_SSL_SESSION_TICKETS) + if( ( ret = ssl_write_session_ticket_ext( ssl, p, end, &ext_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_session_ticket_ext", ret ); + return( ret ); + } + p += ext_len; +#endif + + *out_len = p - buf; + + return( 0 ); +} + static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -692,10 +804,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) const int *ciphersuites; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; -#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ - defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) int uses_ec = 0; -#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write client hello" ) ); @@ -946,13 +1055,10 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ext_len += olen; #endif - /* Note that TLS_EMPTY_RENEGOTIATION_INFO_SCSV is always added - * even if MBEDTLS_SSL_RENEGOTIATION is not defined. */ -#if defined(MBEDTLS_SSL_RENEGOTIATION) - if( ( ret = ssl_write_renegotiation_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) +#if defined(MBEDTLS_SSL_ALPN) + if( ( ret = ssl_write_alpn_ext( ssl, p + 2 + ext_len, end, &olen ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_renegotiation_ext", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_alpn_ext", ret ); return( ret ); } ext_len += olen; @@ -979,101 +1085,18 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) return( ret ); } ext_len += olen; - - if( ( ret = ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_point_formats_ext", ret ); - return( ret ); - } - ext_len += olen; } #endif -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( ( ret = ssl_write_ecjpake_kkpp_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_ecjpake_kkpp_ext", ret ); + ret = ssl_tls12_write_client_hello_exts( ssl, p + 2 + ext_len, end, uses_ec, + &olen ); + if( ret != 0 ) return( ret ); - } ext_len += olen; -#endif -#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) - if( ( ret = ssl_write_cid_ext( ssl, p + 2 + ext_len, end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_cid_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ - -#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) - if( ( ret = ssl_write_max_fragment_length_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_max_fragment_length_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 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_encrypt_then_mac_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) - if( ( ret = ssl_write_extended_ms_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_extended_ms_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - -#if defined(MBEDTLS_SSL_ALPN) - if( ( ret = ssl_write_alpn_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_alpn_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - -#if defined(MBEDTLS_SSL_DTLS_SRTP) - if( ( ret = ssl_write_use_srtp_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_use_srtp_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - -#if defined(MBEDTLS_SSL_SESSION_TICKETS) - if( ( ret = ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, - end, &olen ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_session_ticket_ext", ret ); - return( ret ); - } - ext_len += olen; -#endif - - /* olen unused if all extensions are disabled */ - ((void) olen); - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, total extension length: %" MBEDTLS_PRINTF_SIZET, - ext_len ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, total extension length: %" MBEDTLS_PRINTF_SIZET, + ext_len ) ); if( ext_len > 0 ) { From 04fbd2b2ff33900f126042feb08eccf4902104de Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 18 Feb 2022 12:06:07 +0100 Subject: [PATCH 19/21] ssl_tls13_client.c: Move writing of TLS 1.3 specific extensions Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 70 +++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7d1c826eb1..79ad9772d1 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -805,6 +805,49 @@ static int ssl_tls13_write_client_hello_cipher_suites( return( 0 ); } +static int ssl_tls13_write_client_hello_exts( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *p = buf; + size_t ext_len; + + *out_len = 0; + + /* Write supported_versions extension + * + * Supported Versions Extension is mandatory with TLS 1.3. + */ + ret = ssl_tls13_write_supported_versions_ext( ssl, p, end, &ext_len ); + if( ret != 0 ) + return( ret ); + p += ext_len; + + /* Echo the cookie if the server provided one in its preceding + * HelloRetryRequest message. + */ + ret = ssl_tls13_write_cookie_ext( ssl, p, end, &ext_len ); + if( ret != 0 ) + return( ret ); + p += ext_len; + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) + { + ret = ssl_tls13_write_key_share_ext( ssl, p, end, &ext_len ); + if( ret != 0 ) + return( ret ); + p += ext_len; + } +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + + *out_len = p - buf; + + return( 0 ); +} + /* * Structure of ClientHello message: * @@ -907,11 +950,7 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p_extensions_len = p; p += 2; - /* Write supported_versions extension - * - * Supported Versions Extension is mandatory with TLS 1.3. - */ - ret = ssl_tls13_write_supported_versions_ext( ssl, p, end, &output_len ); + ret = ssl_tls13_write_client_hello_exts( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; @@ -923,32 +962,17 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += output_len; #endif /* MBEDTLS_SSL_ALPN */ - /* Echo the cookie if the server provided one in its preceding - * HelloRetryRequest message. - */ - ret = ssl_tls13_write_cookie_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - return( ret ); - p += output_len; - #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - - /* - * Add the extensions related to (EC)DHE ephemeral key establishment only if - * enabled as per the configuration. - */ if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { ret = mbedtls_ssl_write_supported_groups_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; + } - ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); - if( ret != 0 ) - return( ret ); - p += output_len; - + if( mbedtls_ssl_conf_tls13_ephemeral_enabled( ssl ) ) + { ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); From 7ffe7ebe380dfd5cd860a4b7914c942f92117924 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 9 Mar 2022 15:26:31 +0100 Subject: [PATCH 20/21] ssl_tls13_client.c: Add some MBEDTLS_SSL_PROTO_TLS1_3 guards Add some MBEDTLS_SSL_PROTO_TLS1_3 guards that will be necessary when the ClientHello writing code is made available when MBEDTLS_SSL_PROTO_TLS1_2 is enabled. Signed-off-by: Ronald Cron --- library/ssl_tls13_client.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 79ad9772d1..7d3bdf2408 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -942,18 +942,22 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, /* Write extensions */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) /* Keeping track of the included extensions */ ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; +#endif /* First write extensions, then the total length */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); p_extensions_len = p; p += 2; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) ret = ssl_tls13_write_client_hello_exts( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; +#endif #if defined(MBEDTLS_SSL_ALPN) ret = ssl_tls13_write_alpn_ext( ssl, p, end, &output_len ); @@ -962,6 +966,7 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += output_len; #endif /* MBEDTLS_SSL_ALPN */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { @@ -979,6 +984,7 @@ static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, p += output_len; } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) /* Write server name extension */ From 8f6d39a81d1cb95503215761672d979584d28f7a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 10 Mar 2022 18:56:50 +0100 Subject: [PATCH 21/21] Make some handshake TLS 1.3 utility routines available for TLS 1.2 Signed-off-by: Ronald Cron --- docs/architecture/tls13-support.md | 16 ++--- library/ssl_misc.h | 62 ++++++++--------- library/ssl_msg.c | 34 ++++++++++ library/ssl_tls.c | 24 +++++++ library/ssl_tls13_client.c | 27 ++++---- library/ssl_tls13_generic.c | 103 ++++++----------------------- 6 files changed, 124 insertions(+), 142 deletions(-) diff --git a/docs/architecture/tls13-support.md b/docs/architecture/tls13-support.md index 39e46c4562..3154ac1077 100644 --- a/docs/architecture/tls13-support.md +++ b/docs/architecture/tls13-support.md @@ -388,10 +388,10 @@ General coding rules: Example: ``` - int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char **buf, - size_t *buf_len ); + int mbedtls_ssl_start_handshake_msg( mbedtls_ssl_context *ssl, + unsigned hs_type, + unsigned char **buf, + size_t *buf_len ); ``` - When a function's parameters span several lines, group related parameters @@ -400,12 +400,12 @@ General coding rules: For example, prefer: ``` - mbedtls_ssl_tls13_start_handshake_msg( ssl, hs_type, - buf, buf_len ); + mbedtls_ssl_start_handshake_msg( ssl, hs_type, + buf, buf_len ); ``` over ``` - mbedtls_ssl_tls13_start_handshake_msg( ssl, hs_type, buf, - buf_len ); + mbedtls_ssl_start_handshake_msg( ssl, hs_type, buf, + buf_len ); ``` even if it fits. diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 289a64a1d6..6329e0d5e5 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1152,6 +1152,11 @@ int mbedtls_ssl_write_hostname_ext( mbedtls_ssl_context *ssl, int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ); int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ); void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ); +static inline void mbedtls_ssl_handshake_set_state( mbedtls_ssl_context *ssl, + mbedtls_ssl_states state ) +{ + ssl->state = ( int ) state; +} int mbedtls_ssl_send_fatal_handshake_failure( mbedtls_ssl_context *ssl ); @@ -1245,6 +1250,12 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, unsigned update_hs_digest ); int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); +/* + * Write handshake message header + */ +int mbedtls_ssl_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, + unsigned char **buf, size_t *buf_len ); + int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, int update_checksum, int force_flush ); @@ -1253,6 +1264,12 @@ static inline int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) return( mbedtls_ssl_write_handshake_msg_ext( ssl, 1 /* update checksum */, 1 /* force flush */ ) ); } +/* + * Write handshake message tail + */ +int mbedtls_ssl_finish_handshake_msg( mbedtls_ssl_context *ssl, + size_t buf_len, size_t msg_len ); + int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, int force_flush ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); @@ -1268,8 +1285,17 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ); void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t *ciphersuite_info ); +/* + * Update checksum of handshake messages. + */ +void mbedtls_ssl_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, + unsigned hs_type, + unsigned char const *msg, + size_t msg_len ); + #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) -int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex ); +int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, + mbedtls_key_exchange_type_t key_ex ); /** * Get the first defined PSK by order of precedence: @@ -1727,13 +1753,6 @@ static inline int mbedtls_ssl_tls13_some_psk_enabled( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ALL ) ); } - -static inline void mbedtls_ssl_handshake_set_state( mbedtls_ssl_context *ssl, - mbedtls_ssl_states state ) -{ - ssl->state = ( int ) state; -} - /* * Fetch TLS 1.3 handshake message header */ @@ -1742,14 +1761,6 @@ int mbedtls_ssl_tls13_fetch_handshake_msg( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ); -/* - * Write TLS 1.3 handshake message header - */ -int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char **buf, - size_t *buf_len ); - /* * Handler of TLS 1.3 server certificate message */ @@ -1778,25 +1789,6 @@ int mbedtls_ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ); */ int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ); -/* - * Write TLS 1.3 handshake message tail - */ -int mbedtls_ssl_tls13_finish_handshake_msg( mbedtls_ssl_context *ssl, - size_t buf_len, - size_t msg_len ); - -void mbedtls_ssl_tls13_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl, - unsigned hs_type, - size_t total_hs_len ); - -/* - * Update checksum of handshake messages. - */ -void mbedtls_ssl_tls13_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char const *msg, - size_t msg_len ); - int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index c2effb6d23..ae3dc13303 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2445,6 +2445,24 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) /* * Handshake layer functions */ +int mbedtls_ssl_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, + unsigned char **buf, size_t *buf_len ) +{ + /* + * Reserve 4 bytes for hanshake header. ( Section 4,RFC 8446 ) + * ... + * HandshakeType msg_type; + * uint24 length; + * ... + */ + *buf = ssl->out_msg + 4; + *buf_len = MBEDTLS_SSL_OUT_CONTENT_LEN - 4; + + ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; + ssl->out_msg[0] = hs_type; + + return( 0 ); +} /* * Write (DTLS: or queue) current handshake (including CCS) message. @@ -2609,6 +2627,22 @@ int mbedtls_ssl_write_handshake_msg_ext( mbedtls_ssl_context *ssl, return( 0 ); } +int mbedtls_ssl_finish_handshake_msg( mbedtls_ssl_context *ssl, + size_t buf_len, size_t msg_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t msg_with_header_len; + ((void) buf_len); + + /* Add reserved 4 bytes for handshake header */ + msg_with_header_len = msg_len + 4; + ssl->out_msglen = msg_with_header_len; + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg_ext( ssl, 0, 0 ) ); + +cleanup: + return( ret ); +} + /* * Record layer functions */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6a1bfa8f5a..b14848527f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -475,6 +475,30 @@ void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, } } +static void mbedtls_ssl_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl, + unsigned hs_type, + size_t total_hs_len ) +{ + unsigned char hs_hdr[4]; + + /* Build HS header for checksum update. */ + hs_hdr[0] = MBEDTLS_BYTE_0( hs_type ); + hs_hdr[1] = MBEDTLS_BYTE_2( total_hs_len ); + hs_hdr[2] = MBEDTLS_BYTE_1( total_hs_len ); + hs_hdr[3] = MBEDTLS_BYTE_0( total_hs_len ); + + ssl->handshake->update_checksum( ssl, hs_hdr, sizeof( hs_hdr ) ); +} + +void mbedtls_ssl_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, + unsigned hs_type, + unsigned char const *msg, + size_t msg_len ) +{ + mbedtls_ssl_add_hs_hdr_to_checksum( ssl, hs_type, msg_len ); + ssl->handshake->update_checksum( ssl, msg, msg_len ); +} + void mbedtls_ssl_reset_checksum( mbedtls_ssl_context *ssl ) { ((void) ssl); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7d3bdf2408..9f22e1dcc6 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1060,7 +1060,7 @@ static int ssl_tls13_write_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_client_hello( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, &buf, &buf_len ) ); @@ -1068,14 +1068,12 @@ static int ssl_tls13_write_client_hello( mbedtls_ssl_context *ssl ) buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_hdr_to_checksum( ssl, - MBEDTLS_SSL_HS_CLIENT_HELLO, - msg_len ); - ssl->handshake->update_checksum( ssl, buf, msg_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, + buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, - buf_len, - msg_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, + buf_len, + msg_len ) ); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); @@ -1707,9 +1705,8 @@ static int ssl_tls13_process_server_hello( mbedtls_ssl_context *ssl ) if( is_hrr ) MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_reset_transcript_for_hrr( ssl ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, - MBEDTLS_SSL_HS_SERVER_HELLO, - buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, + buf, buf_len ); if( is_hrr ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_hrr( ssl ) ); @@ -1762,8 +1759,8 @@ static int ssl_tls13_process_encrypted_extensions( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_encrypted_extensions( ssl, buf, buf + buf_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, + buf, buf_len ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_encrypted_extensions( ssl ) ); @@ -2059,8 +2056,8 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate_request( ssl, buf, buf + buf_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, + buf, buf_len ); } else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index dab98a34f3..6623e7f705 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -72,68 +72,6 @@ cleanup: return( ret ); } -int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char **buf, - size_t *buf_len ) -{ - /* - * Reserve 4 bytes for hanshake header. ( Section 4,RFC 8446 ) - * ... - * HandshakeType msg_type; - * uint24 length; - * ... - */ - *buf = ssl->out_msg + 4; - *buf_len = MBEDTLS_SSL_OUT_CONTENT_LEN - 4; - - ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; - ssl->out_msg[0] = hs_type; - - return( 0 ); -} - -int mbedtls_ssl_tls13_finish_handshake_msg( mbedtls_ssl_context *ssl, - size_t buf_len, - size_t msg_len ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t msg_with_header_len; - ((void) buf_len); - - /* Add reserved 4 bytes for handshake header */ - msg_with_header_len = msg_len + 4; - ssl->out_msglen = msg_with_header_len; - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_write_handshake_msg_ext( ssl, 0, 0 ) ); - -cleanup: - return( ret ); -} - -void mbedtls_ssl_tls13_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char const *msg, - size_t msg_len ) -{ - mbedtls_ssl_tls13_add_hs_hdr_to_checksum( ssl, hs_type, msg_len ); - ssl->handshake->update_checksum( ssl, msg, msg_len ); -} - -void mbedtls_ssl_tls13_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl, - unsigned hs_type, - size_t total_hs_len ) -{ - unsigned char hs_hdr[4]; - - /* Build HS header for checksum update. */ - hs_hdr[0] = MBEDTLS_BYTE_0( hs_type ); - hs_hdr[1] = MBEDTLS_BYTE_2( total_hs_len ); - hs_hdr[2] = MBEDTLS_BYTE_1( total_hs_len ); - hs_hdr[3] = MBEDTLS_BYTE_0( total_hs_len ); - - ssl->handshake->update_checksum( ssl, hs_hdr, sizeof( hs_hdr ) ); -} - #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* mbedtls_ssl_tls13_parse_sig_alg_ext() * @@ -479,8 +417,8 @@ int mbedtls_ssl_tls13_process_certificate_verify( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate_verify( ssl, buf, buf + buf_len, verify_buffer, verify_buffer_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, - MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, + buf, buf_len ); cleanup: @@ -796,8 +734,8 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) /* Validate the certificate chain and set the verification results. */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, - buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, + buf, buf_len ); cleanup: @@ -904,7 +842,7 @@ int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate" ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_CERTIFICATE, &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_body( ssl, @@ -912,12 +850,10 @@ int mbedtls_ssl_tls13_write_certificate( mbedtls_ssl_context *ssl ) buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, - MBEDTLS_SSL_HS_CERTIFICATE, - buf, - msg_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, + buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); cleanup: @@ -1161,16 +1097,16 @@ int mbedtls_ssl_tls13_write_certificate_verify( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate verify" ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_verify_body( ssl, buf, buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, msg_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, + buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); cleanup: @@ -1340,8 +1276,8 @@ int mbedtls_ssl_tls13_process_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_finished_message( ssl, buf, buf + buf_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_FINISHED, buf, buf_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, + buf, buf_len ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_finished_message( ssl ) ); cleanup: @@ -1418,19 +1354,18 @@ int mbedtls_ssl_tls13_write_finished_message( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_finished_message( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_FINISHED, &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_finished_message_body( ssl, buf, buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, - buf, msg_len ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_FINISHED, + buf, msg_len ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_finished_message( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, - buf_len, msg_len ) ); - + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( + ssl, buf_len, msg_len ) ); cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished message" ) );