From 2e19981e17eb40d6ed421fc651b4b519f1637c58 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 1 Dec 2022 18:57:19 +0800 Subject: [PATCH 1/4] tls13: guards transform negotiate Signed-off-by: Jerry Yu --- include/mbedtls/ssl.h | 2 ++ library/ssl_msg.c | 3 +++ library/ssl_tls.c | 40 +++++++++++++++++++++++++++++----------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3f48377b6..b8e739bf4 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1656,9 +1656,11 @@ struct mbedtls_ssl_context mbedtls_ssl_transform *MBEDTLS_PRIVATE(transform); /*!< negotiated transform params * This pointer owns the transform * it references. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) mbedtls_ssl_transform *MBEDTLS_PRIVATE(transform_negotiate); /*!< transform params in negotiation * This pointer owns the transform * it references. */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) /*! The application data transform in TLS 1.3. diff --git a/library/ssl_msg.c b/library/ssl_msg.c index e4d50dbfd..c9ef8e49d 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5097,9 +5097,12 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) * data. */ MBEDTLS_SSL_DEBUG_MSG( 3, ( "switching to new transform spec for inbound data" ) ); +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) ssl->transform_in = ssl->transform_negotiate; +#endif ssl->session_in = ssl->session_negotiate; + #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9bb9dc23c..eba5e3ba9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -965,13 +965,16 @@ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handshake_init( mbedtls_ssl_context *ssl ) { /* Clear old handshake information if present */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->transform_negotiate ) mbedtls_ssl_transform_free( ssl->transform_negotiate ); +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ if( ssl->session_negotiate ) mbedtls_ssl_session_free( ssl->session_negotiate ); if( ssl->handshake ) mbedtls_ssl_handshake_free( ssl ); +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) /* * Either the pointers are now NULL or cleared properly and can be freed. * Now allocate missing structures. @@ -980,6 +983,7 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) { ssl->transform_negotiate = mbedtls_calloc( 1, sizeof(mbedtls_ssl_transform) ); } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ if( ssl->session_negotiate == NULL ) { @@ -998,18 +1002,23 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) #endif /* All pointers should exist and can be directly freed without issue */ - if( ssl->handshake == NULL || + if( ssl->handshake == NULL || +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) ssl->transform_negotiate == NULL || - ssl->session_negotiate == NULL ) +#endif + ssl->session_negotiate == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc() of ssl sub-contexts failed" ) ); mbedtls_free( ssl->handshake ); - mbedtls_free( ssl->transform_negotiate ); - mbedtls_free( ssl->session_negotiate ); - ssl->handshake = NULL; + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + mbedtls_free( ssl->transform_negotiate ); ssl->transform_negotiate = NULL; +#endif + + mbedtls_free( ssl->session_negotiate ); ssl->session_negotiate = NULL; return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); @@ -1017,9 +1026,12 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) /* Initialize structures */ mbedtls_ssl_session_init( ssl->session_negotiate ); - mbedtls_ssl_transform_init( ssl->transform_negotiate ); ssl_handshake_params_init( ssl->handshake ); +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + mbedtls_ssl_transform_init( ssl->transform_negotiate ); +#endif + #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) @@ -3975,7 +3987,7 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_hash_abort( &handshake->fin_sha256_psa ); #else - mbedtls_sha256_free( &handshake->fin_sha256 ); + mbedtls_sha256_free( &handshake->fin_sha256 ); #endif #endif #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) @@ -4512,10 +4524,12 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, /* This has been allocated by ssl_handshake_init(), called by * by either mbedtls_ssl_session_reset_int() or mbedtls_ssl_setup(). */ - ssl->transform = ssl->transform_negotiate; +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + ssl->transform = ssl->transform_negotiate; ssl->transform_in = ssl->transform; ssl->transform_out = ssl->transform; ssl->transform_negotiate = NULL; +#endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) prf_func = ssl_tls12prf_from_cs( ssl->session->ciphersuite ); @@ -4748,14 +4762,18 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) mbedtls_free( ssl->transform ); } + if( ssl->handshake ) { mbedtls_ssl_handshake_free( ssl ); - mbedtls_ssl_transform_free( ssl->transform_negotiate ); - mbedtls_ssl_session_free( ssl->session_negotiate ); - mbedtls_free( ssl->handshake ); + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + mbedtls_ssl_transform_free( ssl->transform_negotiate ); mbedtls_free( ssl->transform_negotiate ); +#endif + + mbedtls_ssl_session_free( ssl->session_negotiate ); mbedtls_free( ssl->session_negotiate ); } From ddda0506040a6d1f02a998edbf824c6e324638a6 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 1 Dec 2022 19:43:12 +0800 Subject: [PATCH 2/4] tls13: Upstream various fix in prototype - Adjust max input_max_frag_len - Guard transform_negotiate - Adjust function position - update comments - fix wrong requirements Signed-off-by: Jerry Yu --- library/ssl_misc.h | 10 ++++++---- library/ssl_tls.c | 8 ++++++-- library/ssl_tls13_client.c | 7 +++---- tests/ssl-opt.sh | 10 +++++++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 5808cab08..b20b72d69 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2058,6 +2058,12 @@ int mbedtls_ssl_tls13_write_early_data_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) +/* + * Write Signature Algorithm extension + */ +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, + const unsigned char *end, size_t *out_len ); /* * Parse TLS Signature Algorithm extension */ @@ -2605,10 +2611,6 @@ int mbedtls_ssl_validate_ciphersuite( mbedtls_ssl_protocol_version min_tls_version, mbedtls_ssl_protocol_version max_tls_version ); -MBEDTLS_CHECK_RETURN_CRITICAL -int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, - const unsigned char *end, size_t *out_len ); - #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_server_name_ext( mbedtls_ssl_context *ssl, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index eba5e3ba9..7723363df 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3198,12 +3198,14 @@ size_t mbedtls_ssl_get_input_max_frag_len( const mbedtls_ssl_context *ssl ) size_t max_len = MBEDTLS_SSL_IN_CONTENT_LEN; size_t read_mfl; +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) /* Use the configured MFL for the client if we're past SERVER_HELLO_DONE */ if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && ssl->state >= MBEDTLS_SSL_SERVER_HELLO_DONE ) { return ssl_mfl_code_to_length( ssl->conf->mfl_code ); } +#endif /* Check if a smaller max length was negotiated */ if( ssl->session_out != NULL ) @@ -3215,7 +3217,7 @@ size_t mbedtls_ssl_get_input_max_frag_len( const mbedtls_ssl_context *ssl ) } } - // During a handshake, use the value being negotiated + /* During a handshake, use the value being negotiated */ if( ssl->session_negotiate != NULL ) { read_mfl = ssl_mfl_code_to_length( ssl->session_negotiate->mfl_code ); @@ -3486,6 +3488,8 @@ static unsigned char ssl_serialized_session_header[] = { * * case MBEDTLS_SSL_VERSION_TLS1_2: * serialized_session_tls12 data; + * case MBEDTLS_SSL_MINOR_VERSION_4: + * serialized_session_tls13 data; * * }; * @@ -4525,7 +4529,7 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, /* This has been allocated by ssl_handshake_init(), called by * by either mbedtls_ssl_session_reset_int() or mbedtls_ssl_setup(). */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - ssl->transform = ssl->transform_negotiate; + ssl->transform = ssl->transform_negotiate; ssl->transform_in = ssl->transform; ssl->transform_out = ssl->transform; ssl->transform_negotiate = NULL; diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 0109f776c..3295e67c3 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2805,11 +2805,10 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) switch( ssl->state ) { - /* - * ssl->state is initialized as HELLO_REQUEST. It is the same - * as CLIENT_HELLO state. - */ case MBEDTLS_SSL_HELLO_REQUEST: + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); + break; + case MBEDTLS_SSL_CLIENT_HELLO: ret = mbedtls_ssl_write_client_hello( ssl ); break; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1fe8baeb6..9e6930635 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2090,6 +2090,8 @@ run_test "Opaque keys for server authentication: EC + RSA, force ECDHE-ECDSA" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3 opaque key: no suitable algorithm found" \ "$P_SRV debug_level=4 force_version=tls13 auth_mode=required key_opaque=1 key_opaque_algs=rsa-decrypt,none" \ "$P_CLI debug_level=4 key_opaque=1 key_opaque_algs=rsa-decrypt,rsa-sign-pss" \ @@ -2103,6 +2105,8 @@ run_test "TLS 1.3 opaque key: no suitable algorithm found" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3 opaque key: suitable algorithm found" \ "$P_SRV debug_level=4 force_version=tls13 auth_mode=required key_opaque=1 key_opaque_algs=rsa-decrypt,rsa-sign-pss" \ "$P_CLI debug_level=4 key_opaque=1 key_opaque_algs=rsa-decrypt,rsa-sign-pss" \ @@ -2111,11 +2115,13 @@ run_test "TLS 1.3 opaque key: suitable algorithm found" \ -c "key type: Opaque" \ -s "key types: Opaque, Opaque" \ -C "error" \ - -S "error" \ + -S "error" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3 opaque key: first client sig alg not suitable" \ "$P_SRV debug_level=4 force_version=tls13 auth_mode=required key_opaque=1 key_opaque_algs=rsa-sign-pss-sha512,none" \ "$P_CLI debug_level=4 sig_algs=rsa_pss_rsae_sha256,rsa_pss_rsae_sha512" \ @@ -2130,6 +2136,8 @@ run_test "TLS 1.3 opaque key: first client sig alg not suitable" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_USE_PSA_CRYPTO requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3 opaque key: 2 keys on server, suitable algorithm found" \ "$P_SRV debug_level=4 force_version=tls13 auth_mode=required key_opaque=1 key_opaque_algs2=ecdsa-sign,none key_opaque_algs=rsa-decrypt,rsa-sign-pss" \ "$P_CLI debug_level=4 key_opaque=1 key_opaque_algs=rsa-decrypt,rsa-sign-pss" \ From 141bbe7beeabad6b212f16a615b7f5d191586b3e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 1 Dec 2022 20:30:41 +0800 Subject: [PATCH 3/4] tls13: Adjust include files - remove duplicate and unused included - Adjust the order to system, mbedtls global, local. Signed-off-by: Jerry Yu --- library/ssl_client.c | 6 +----- library/ssl_tls13_generic.c | 6 ++---- library/ssl_tls13_server.c | 10 ---------- 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index 2c4ce4316..b7de4bcec 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -24,15 +24,11 @@ #if defined(MBEDTLS_SSL_CLI_C) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) || defined(MBEDTLS_SSL_PROTO_TLS1_2) -#include "mbedtls/platform.h" - #include #include "mbedtls/debug.h" #include "mbedtls/error.h" -#if defined(MBEDTLS_HAVE_TIME) -#include "mbedtls/platform_time.h" -#endif +#include "mbedtls/platform.h" #include "ssl_client.h" #include "ssl_misc.h" diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 761c00ec5..0aecb3544 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -28,16 +28,14 @@ #include "mbedtls/oid.h" #include "mbedtls/platform.h" #include "mbedtls/constant_time.h" -#include +#include "psa/crypto.h" +#include "mbedtls/psa_util.h" #include "ssl_misc.h" #include "ssl_tls13_invasive.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" -#include "psa/crypto.h" -#include "mbedtls/psa_util.h" - const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ MBEDTLS_SERVER_HELLO_RANDOM_LEN ] = { 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 6caae89b4..685e10ba1 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -30,16 +30,6 @@ #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" -#if defined(MBEDTLS_ECP_C) -#include "mbedtls/ecp.h" -#endif /* MBEDTLS_ECP_C */ - -#include "mbedtls/platform.h" - -#include "ssl_misc.h" -#include "ssl_tls13_keys.h" -#include "ssl_debug_helpers.h" - static const mbedtls_ssl_ciphersuite_t *ssl_tls13_validate_peer_ciphersuite( mbedtls_ssl_context *ssl, From 0c2a738c2305496e29d5abe9ba185c7722bdd3e3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Tue, 6 Dec 2022 13:27:25 +0800 Subject: [PATCH 4/4] fix various issues Signed-off-by: Jerry Yu --- library/ssl_msg.c | 1 - library/ssl_tls.c | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index c9ef8e49d..d1e8887d7 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -5102,7 +5102,6 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) #endif ssl->session_in = ssl->session_negotiate; - #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7723363df..f141f03ec 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3482,13 +3482,14 @@ static unsigned char ssl_serialized_session_header[] = { * // the structure of mbedtls_ssl_session. * * uint8_t minor_ver; // Protocol minor version. Possible values: - * // - TLS 1.2 (3) + * // - TLS 1.2 (0x0303) + * // - TLS 1.3 (0x0304) * * select (serialized_session.tls_version) { * * case MBEDTLS_SSL_VERSION_TLS1_2: * serialized_session_tls12 data; - * case MBEDTLS_SSL_MINOR_VERSION_4: + * case MBEDTLS_SSL_VERSION_TLS1_3: * serialized_session_tls13 data; * * }; @@ -4766,7 +4767,6 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) mbedtls_free( ssl->transform ); } - if( ssl->handshake ) { mbedtls_ssl_handshake_free( ssl );