From 5b64ae9badcea709be984d083cc80530200ac2bc Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 30 Mar 2022 17:15:02 +0800 Subject: [PATCH 01/18] tls13:server:Add base framework for serverhello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8d1b1d81e5..f9f9b6bcc3 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -727,6 +727,15 @@ cleanup: return( ret ); } +/* + * StateHanler: MBEDTLS_SSL_SERVER_HELLO + */ +static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) +{ + ((void) ssl); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} + /* * TLS 1.3 State Machine -- server side */ @@ -758,6 +767,10 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) break; + case MBEDTLS_SSL_SERVER_HELLO: + ret = ssl_tls13_write_server_hello( ssl ); + break; + default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); From f4b27e4351da33bbdf9b277b8fa1ecaf819c5ded Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 30 Mar 2022 17:32:21 +0800 Subject: [PATCH 02/18] tls13:server:Add prepare write_server_hello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 64 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f9f9b6bcc3..8254e9d1a6 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -730,10 +730,70 @@ cleanup: /* * StateHanler: MBEDTLS_SSL_SERVER_HELLO */ +static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + + if( ssl->conf->f_rng == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no RNG provided" ) ); + return( MBEDTLS_ERR_SSL_NO_RNG ); + } + + if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, + ssl->handshake->randbytes, + MBEDTLS_SERVER_HELLO_RANDOM_LEN ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", + ssl->handshake->randbytes, + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + +#if defined(MBEDTLS_HAVE_TIME) + ssl->session_negotiate->start = time( NULL ); +#endif /* MBEDTLS_HAVE_TIME */ + + return( ret ); +} + static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) { - ((void) ssl); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + int ret = 0; + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server hello" ) ); + + /* Preprocessing */ + + /* This might lead to ssl_tls13_process_server_hello() being called + * multiple times. The implementation of + * ssl_tls13_process_server_hello_preprocess() must either be safe to be + * called multiple times, or we need to add state to omit this call once + * we're calling ssl_tls13_process_server_hello() multiple times. + */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_server_hello( ssl ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, buf_len, + &msg_len ) ); + + mbedtls_ssl_tls13_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + ssl, buf_len, msg_len ) ); +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write server hello" ) ); + return( ret ); } /* From 56404d70c4f8e635b2ac238bce24db3880662623 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 30 Mar 2022 17:36:13 +0800 Subject: [PATCH 03/18] tls13:server:Add finalize write_server_hello and dummy body Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8254e9d1a6..ccc66a9460 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -759,6 +759,21 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) return( ret ); } +static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} + + +static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) +{ + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); + return( 0 ); +} + static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) { int ret = 0; From 3bf2c6449d31f2f2ea9c15ba97aebc5d35069468 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 30 Mar 2022 22:02:12 +0800 Subject: [PATCH 04/18] tls13: write server hello compile pass Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 289 +++++++++++++++++++++++++++++++++++-- 1 file changed, 277 insertions(+), 12 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ccc66a9460..ae49eff68e 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -26,7 +26,8 @@ #include "ssl_misc.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" -#include +#include "ecdh_misc.h" + #if defined(MBEDTLS_ECP_C) #include "mbedtls/ecp.h" #endif /* MBEDTLS_ECP_C */ @@ -733,23 +734,22 @@ cleanup: static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) { int ret = 0; - + unsigned char *server_randbyes = + ssl->handshake->randbytes + MBEDTLS_CLIENT_HELLO_RANDOM_LEN; if( ssl->conf->f_rng == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "no RNG provided" ) ); return( MBEDTLS_ERR_SSL_NO_RNG ); } - if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, - ssl->handshake->randbytes, + if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, server_randbyes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); return( ret ); } - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", - ssl->handshake->randbytes, + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", server_randbyes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); #if defined(MBEDTLS_HAVE_TIME) @@ -759,12 +759,276 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) return( ret ); } -static int ssl_tls13_write_client_hello_body( mbedtls_ssl_context *ssl, +/* + * ssl_tls13_write_supported_versions_ext(): + * + * struct { + * ProtocolVersion versions<2..254>; + * } SupportedVersions; + */ +static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + unsigned char *p = buf; + + *out_len = 0; + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, write supported versions extension" ) ); + + /* Check if we have space to write the extension: + * - extension_type (2 bytes) + * - extension_data_length (2 bytes) + * - versions_length (1 byte ) + * - versions (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 7 ); + + /* Write extension_type */ + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS, p, 0 ); + + /* Write extension_data_length */ + MBEDTLS_PUT_UINT16_BE( 3, p, 2 ); + p += 4; + + /* Length of versions */ + *p++ = 0x2; + + /* Write values of supported versions. + * + * They are defined by the configuration. + * + * Currently, only one version is advertised. + */ + mbedtls_ssl_write_version( p, ssl->tls_version, ssl->conf->transport ); + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "supported version: [%04x]", + ssl->tls_version ) ); + + *out_len = 7; + + return( 0 ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + +/* Generate and export a single key share. For hybrid KEMs, this can + * be called multiple times with the different components of the hybrid. */ +static int ssl_tls13_key_share_encapsulate( mbedtls_ssl_context *ssl, + uint16_t named_group, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + if( mbedtls_ssl_tls13_named_group_is_ecdhe( named_group ) ) + { + ret = mbedtls_ecdh_tls13_make_params( &ssl->handshake->ecdh_ctx, + out_len, buf, end - buf, ssl->conf->f_rng, ssl->conf->p_rng ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_tls13_params", ret ); + return( ret ); + } + + MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Q ); + } + else if( 0 /* Other kinds of KEMs */ ) + { + } + else + { + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + } + + return( ret ); +} + +/* + * ssl_tls13_write_key_share_ext + * + * Structure of key_share extension in ServerHello: + * + * struct { + * NamedGroup group; + * opaque key_exchange<1..2^16-1>; + * } KeyShareEntry; + * struct { + * KeyShareEntry server_share; + * } KeyShareServerHello; + */ +static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + unsigned char *p = buf; + unsigned char *start = buf; + uint16_t group = ssl->handshake->offered_group_id ; + unsigned char *server_share = buf + 4; + unsigned char *key_exchange = buf + 6; + size_t key_exchange_length; + int ret; + + *out_len = 0; + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, adding key share extension" ) ); + + /* Check if we have space for header and length fields: + * - extension_type (2 bytes) + * - extension_data_length (2 bytes) + * - group (2 bytes) + * - key_exchange_length (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 8 ); + + p += 8; + /* When we introduce PQC-ECDHE hybrids, we'll want to call this + * function multiple times. */ + ret = ssl_tls13_key_share_encapsulate( ssl, group, key_exchange + 2, + end, &key_exchange_length ); + if( ret != 0 ) + return( ret ); + p += key_exchange_length; + /* Write length of key_exchange */ + MBEDTLS_PUT_UINT16_BE( key_exchange_length, key_exchange, 0 ); + + *out_len = p - start; + + /* Write group ID */ + MBEDTLS_PUT_UINT16_BE( group, server_share, 0 ); + + /* Write extension header */ + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, start, 0 ); + + /* Write total extension length */ + MBEDTLS_PUT_UINT16_BE( p - server_share, start, 2 ); + + return( 0 ); +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + +/* + * Structure of ServerHello message: + * + * struct { + * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 + * Random random; + * opaque legacy_session_id_echo<0..32>; + * CipherSuite cipher_suite; + * uint8 legacy_compression_method = 0; + * Extension extensions<6..2^16-1>; + * } ServerHello; + */ +static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, unsigned char *buf, unsigned char *end, size_t *out_len ) { - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + int ret = 0; + size_t output_len; /* Length of buffer used by function */ + unsigned char *server_randbyes = + ssl->handshake->randbytes + MBEDTLS_CLIENT_HELLO_RANDOM_LEN; + + /* Buffer management */ + unsigned char *p = buf; + unsigned char *start = buf; + unsigned char *extension_start; + + *out_len = 0; + + /* + * Write legacy_version + * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 + * + * For TLS 1.3 we use the legacy version number {0x03, 0x03} + * instead of the true version number. + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( 0x0303, p, 0 ); + p += 2; + + /* Write the random bytes ( random ).*/ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + memcpy( p, server_randbyes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", + p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; + +#if defined(MBEDTLS_HAVE_TIME) + ssl->session_negotiate->start = time( NULL ); +#endif /* MBEDTLS_HAVE_TIME */ + + /* + * Write legacy_session_id_echo + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + ssl->session_negotiate->id_len ); + *p++ = (unsigned char)ssl->session_negotiate->id_len; + if( ssl->session_negotiate->id_len > 0 ) + { + memcpy( p, &ssl->session_negotiate->id[0], + ssl->session_negotiate->id_len ); + p += ssl->session_negotiate->id_len; + MBEDTLS_SSL_DEBUG_MSG( 3, ( "session id length ( %" + MBEDTLS_PRINTF_SIZET " )", + ssl->session_negotiate->id_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "session id", ssl->session_negotiate->id, + ssl->session_negotiate->id_len ); + } + + /* + * Write ciphersuite + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + MBEDTLS_PUT_UINT16_BE( ssl->session_negotiate->ciphersuite, p, 0 ); + p += 2; + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "server hello, chosen ciphersuite: %s ( id=%d )", + mbedtls_ssl_get_ciphersuite_name( + ssl->session_negotiate->ciphersuite ), + ssl->session_negotiate->ciphersuite ) ); + + /* write legacy_compression_method = ( 0 ) */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + *p++ = 0x0; + + /* Extensions */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + extension_start = p; + p += 2; + + /* Add supported_version extension */ + if( ( ret = ssl_tls13_write_supported_versions_ext( + ssl, p, end, &output_len ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_supported_versions_ext", + ret ); + return( ret ); + } + p += output_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, &output_len ); + if( ret != 0 ) + return( ret ); + p += output_len; + } +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + + /* Write length information */ + MBEDTLS_PUT_UINT16_BE( p - extension_start - 2, extension_start, 0 ); + + MBEDTLS_SSL_DEBUG_BUF( 4, "server hello extensions", extension_start, p - extension_start ); + + *out_len = p - start; + + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello", start, *out_len ); + + return( ret ); } @@ -792,18 +1056,19 @@ static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_server_hello( 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_SERVER_HELLO, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, buf_len, + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_server_hello_body( ssl, buf, + buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); - 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: From 89e103c54c78b53b1c7f075cebd979c54c42f50f Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 30 Mar 2022 22:43:29 +0800 Subject: [PATCH 05/18] tls13: Share write ecdh_key_exchange function Signed-off-by: Jerry Yu --- library/ssl_misc.h | 10 ++++++ library/ssl_tls13_client.c | 63 ++----------------------------------- library/ssl_tls13_generic.c | 57 +++++++++++++++++++++++++++++++++ library/ssl_tls13_server.c | 21 ++++++++----- 4 files changed, 83 insertions(+), 68 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 46d85d94fa..fa336e4c74 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1781,6 +1781,16 @@ int mbedtls_ssl_tls13_write_change_cipher_spec( mbedtls_ssl_context *ssl ); int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ); +#if defined(MBEDTLS_ECDH_C) +int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( + mbedtls_ssl_context *ssl, + uint16_t named_group, + unsigned char *buf, + unsigned char *end, + size_t *out_len ); +#endif /* MBEDTLS_ECDH_C */ + + #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index cf5b382851..d024abf180 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -204,65 +204,6 @@ static int ssl_tls13_reset_key_share( mbedtls_ssl_context *ssl ) /* * Functions for writing key_share extension. */ -#if defined(MBEDTLS_ECDH_C) -static int ssl_tls13_generate_and_write_ecdh_key_exchange( - mbedtls_ssl_context *ssl, - uint16_t named_group, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - psa_status_t status = PSA_ERROR_GENERIC_ERROR; - int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; - psa_key_attributes_t key_attributes; - size_t own_pubkey_len; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; - size_t ecdh_bits = 0; - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); - - /* Convert EC group to PSA key type. */ - if( ( handshake->ecdh_psa_type = - mbedtls_psa_parse_tls_ecc_group( named_group, &ecdh_bits ) ) == 0 ) - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - - ssl->handshake->ecdh_bits = ecdh_bits; - - key_attributes = psa_key_attributes_init(); - psa_set_key_usage_flags( &key_attributes, PSA_KEY_USAGE_DERIVE ); - psa_set_key_algorithm( &key_attributes, PSA_ALG_ECDH ); - psa_set_key_type( &key_attributes, handshake->ecdh_psa_type ); - psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); - - /* Generate ECDH private key. */ - status = psa_generate_key( &key_attributes, - &handshake->ecdh_psa_privkey ); - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_generate_key", ret ); - return( ret ); - - } - - /* Export the public part of the ECDH private key from PSA. */ - status = psa_export_public_key( handshake->ecdh_psa_privkey, - buf, (size_t)( end - buf ), - &own_pubkey_len ); - if( status != PSA_SUCCESS ) - { - ret = psa_ssl_status_to_mbedtls( status ); - MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); - return( ret ); - - } - - *out_len = own_pubkey_len; - - return( 0 ); -} -#endif /* MBEDTLS_ECDH_C */ - static int ssl_tls13_get_default_group_id( mbedtls_ssl_context *ssl, uint16_t *group_id ) { @@ -367,8 +308,8 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); p += 4; - ret = ssl_tls13_generate_and_write_ecdh_key_exchange( ssl, group_id, p, end, - &key_exchange_len ); + ret = mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( + ssl, group_id, p, end, &key_exchange_len ); p += key_exchange_len; if( ret != 0 ) return( ret ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 4bee319dca..f5d791f1bf 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1535,6 +1535,63 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, return( 0 ); } + +int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( + mbedtls_ssl_context *ssl, + uint16_t named_group, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) +{ + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + psa_key_attributes_t key_attributes; + size_t own_pubkey_len; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + size_t ecdh_bits = 0; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Perform PSA-based ECDH computation." ) ); + + /* Convert EC group to PSA key type. */ + if( ( handshake->ecdh_psa_type = + mbedtls_psa_parse_tls_ecc_group( named_group, &ecdh_bits ) ) == 0 ) + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + + ssl->handshake->ecdh_bits = ecdh_bits; + + key_attributes = psa_key_attributes_init(); + psa_set_key_usage_flags( &key_attributes, PSA_KEY_USAGE_DERIVE ); + psa_set_key_algorithm( &key_attributes, PSA_ALG_ECDH ); + psa_set_key_type( &key_attributes, handshake->ecdh_psa_type ); + psa_set_key_bits( &key_attributes, handshake->ecdh_bits ); + + /* Generate ECDH private key. */ + status = psa_generate_key( &key_attributes, + &handshake->ecdh_psa_privkey ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_generate_key", ret ); + return( ret ); + + } + + /* Export the public part of the ECDH private key from PSA. */ + status = psa_export_public_key( handshake->ecdh_psa_privkey, + buf, (size_t)( end - buf ), + &own_pubkey_len ); + if( status != PSA_SUCCESS ) + { + ret = psa_ssl_status_to_mbedtls( status ); + MBEDTLS_SSL_DEBUG_RET( 1, "psa_export_public_key", ret ); + return( ret ); + + } + + *out_len = own_pubkey_len; + + return( 0 ); +} #endif /* MBEDTLS_ECDH_C */ #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ae49eff68e..d6450a508e 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -822,20 +822,27 @@ static int ssl_tls13_key_share_encapsulate( mbedtls_ssl_context *ssl, size_t *out_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - + ((void) ssl); + ((void) named_group); + ((void) buf); + ((void) end); + ((void) out_len); +#if defined(MBEDTLS_ECDH_C) if( mbedtls_ssl_tls13_named_group_is_ecdhe( named_group ) ) { - ret = mbedtls_ecdh_tls13_make_params( &ssl->handshake->ecdh_ctx, - out_len, buf, end - buf, ssl->conf->f_rng, ssl->conf->p_rng ); + ret = mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange( + ssl, named_group, buf, end, out_len ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_make_tls13_params", ret ); + MBEDTLS_SSL_DEBUG_RET( + 1, "mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange", + ret ); return( ret ); } - - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, MBEDTLS_DEBUG_ECDH_Q ); } - else if( 0 /* Other kinds of KEMs */ ) + else +#endif /* MBEDTLS_ECDH_C */ + if( 0 /* Other kinds of KEMs */ ) { } else From fb9f54db8c3d5cc295bd38261efcfb93de9280d3 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 6 Apr 2022 10:08:34 +0800 Subject: [PATCH 06/18] fix comments issue Co-authored-by: Ronald Cron Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d6450a508e..acd536abee 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -763,7 +763,7 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) * ssl_tls13_write_supported_versions_ext(): * * struct { - * ProtocolVersion versions<2..254>; + * ProtocolVersion selected_version; * } SupportedVersions; */ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, From abf20c756462b39fc9dc1559ba99f97058c788ec Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 14 Apr 2022 18:36:14 +0800 Subject: [PATCH 07/18] add state check Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index d207e54cfb..b69002c7c1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10481,9 +10481,10 @@ run_test "TLS 1.3: Server side check - openssl" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ - -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s " SSL - The requested feature is not available" \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -10496,9 +10497,10 @@ run_test "TLS 1.3: Server side check - gnutls" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ 1 \ - -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ - -s " SSL - The requested feature is not available" \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" From 349a61388bb66f048cbaadebadb4172466dfc82c Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 14 Apr 2022 20:52:56 +0800 Subject: [PATCH 08/18] fix write selected_version fail And rename write_supported_versions to write selected_version Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 39 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index acd536abee..7117e7ecc4 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -760,40 +760,31 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) } /* - * ssl_tls13_write_supported_versions_ext(): + * ssl_tls13_write_selected_version_ext(): * * struct { * ProtocolVersion selected_version; * } SupportedVersions; */ -static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) +static int ssl_tls13_write_selected_version_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) { - unsigned char *p = buf; - *out_len = 0; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, write supported versions extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, write selected_version" ) ); /* Check if we have space to write the extension: * - extension_type (2 bytes) * - extension_data_length (2 bytes) - * - versions_length (1 byte ) - * - versions (2 bytes) + * - selected_version (2 bytes) */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 7 ); + MBEDTLS_SSL_CHK_BUF_PTR( buf, end, 6 ); - /* Write extension_type */ - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS, p, 0 ); + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS, buf, 0 ); - /* Write extension_data_length */ - MBEDTLS_PUT_UINT16_BE( 3, p, 2 ); - p += 4; - - /* Length of versions */ - *p++ = 0x2; + MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); /* Write values of supported versions. * @@ -801,12 +792,14 @@ static int ssl_tls13_write_supported_versions_ext( mbedtls_ssl_context *ssl, * * Currently, only one version is advertised. */ - mbedtls_ssl_write_version( p, ssl->tls_version, ssl->conf->transport ); + mbedtls_ssl_write_version( buf + 4, + ssl->conf->transport, + ssl->tls_version ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "supported version: [%04x]", ssl->tls_version ) ); - *out_len = 7; + *out_len = 6; return( 0 ); } @@ -1007,10 +1000,10 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, p += 2; /* Add supported_version extension */ - if( ( ret = ssl_tls13_write_supported_versions_ext( + if( ( ret = ssl_tls13_write_selected_version_ext( ssl, p, end, &output_len ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_supported_versions_ext", + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_selected_version_ext", ret ); return( ret ); } From 8b9fd374b8cfa8fe8c4109edc994846b2d83beda Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 14 Apr 2022 20:55:12 +0800 Subject: [PATCH 09/18] Add P_CLI test to easy debug Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b69002c7c1..c93cec121a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10479,7 +10479,7 @@ requires_config_enabled MBEDTLS_SSL_SRV_C requires_openssl_tls1_3 run_test "TLS 1.3: Server side check - openssl" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ - "$O_NEXT_CLI -msg -tls1_3" \ + "$O_NEXT_CLI -msg -debug -tls1_3" \ 1 \ -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ @@ -10504,6 +10504,23 @@ run_test "TLS 1.3: Server side check - gnutls" \ -s "=> parse client hello" \ -s "<= parse client hello" +requires_gnutls_tls1_3 +requires_gnutls_next_no_ticket +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +run_test "TLS 1.3: Server side check - mbedtls" \ + "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ + "$P_CLI debug_level=4 force_version=tls13" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -c "client state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "SSL - The requested feature is not available" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + for i in opt-testcases/*.sh do TEST_SUITE_NAME=${i##*/} From 1c3e688df1bf3cdded0bfe8c0cf6189564f3757b Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Apr 2022 21:23:40 +0800 Subject: [PATCH 10/18] fix comments issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 7117e7ecc4..96daa1c469 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -729,7 +729,7 @@ cleanup: } /* - * StateHanler: MBEDTLS_SSL_SERVER_HELLO + * Handler for MBEDTLS_SSL_SERVER_HELLO */ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) { @@ -786,12 +786,6 @@ static int ssl_tls13_write_selected_version_ext( mbedtls_ssl_context *ssl, MBEDTLS_PUT_UINT16_BE( 2, buf, 2 ); - /* Write values of supported versions. - * - * They are defined by the configuration. - * - * Currently, only one version is advertised. - */ mbedtls_ssl_write_version( buf + 4, ssl->conf->transport, ssl->tls_version ); @@ -851,13 +845,13 @@ static int ssl_tls13_key_share_encapsulate( mbedtls_ssl_context *ssl, * * Structure of key_share extension in ServerHello: * - * struct { - * NamedGroup group; - * opaque key_exchange<1..2^16-1>; - * } KeyShareEntry; - * struct { - * KeyShareEntry server_share; - * } KeyShareServerHello; + * struct { + * NamedGroup group; + * opaque key_exchange<1..2^16-1>; + * } KeyShareEntry; + * struct { + * KeyShareEntry server_share; + * } KeyShareServerHello; */ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -950,7 +944,10 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, MBEDTLS_PUT_UINT16_BE( 0x0303, p, 0 ); p += 2; - /* Write the random bytes ( random ).*/ + /* ... + * Random random; + * ... + */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); memcpy( p, server_randbyes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", @@ -1046,14 +1043,6 @@ static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server hello" ) ); - /* Preprocessing */ - - /* This might lead to ssl_tls13_process_server_hello() being called - * multiple times. The implementation of - * ssl_tls13_process_server_hello_preprocess() must either be safe to be - * called multiple times, or we need to add state to omit this call once - * we're calling ssl_tls13_process_server_hello() multiple times. - */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_prepare_server_hello( ssl ) ); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, From 637a3f1090a74fcdd649e733a68607bbd44fd970 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Apr 2022 21:37:58 +0800 Subject: [PATCH 11/18] fix various issues typo issue, variable `ret` init value and remove finalize_server_hello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 96daa1c469..0810f333db 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -733,8 +733,8 @@ cleanup: */ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) { - int ret = 0; - unsigned char *server_randbyes = + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *server_randbytes = ssl->handshake->randbytes + MBEDTLS_CLIENT_HELLO_RANDOM_LEN; if( ssl->conf->f_rng == NULL ) { @@ -742,14 +742,14 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_NO_RNG ); } - if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, server_randbyes, + if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, server_randbytes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "f_rng", ret ); return( ret ); } - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", server_randbyes, + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", server_randbytes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); #if defined(MBEDTLS_HAVE_TIME) @@ -923,7 +923,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, { int ret = 0; size_t output_len; /* Length of buffer used by function */ - unsigned char *server_randbyes = + unsigned char *server_randbytes = ssl->handshake->randbytes + MBEDTLS_CLIENT_HELLO_RANDOM_LEN; /* Buffer management */ @@ -949,7 +949,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, * ... */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - memcpy( p, server_randbyes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + memcpy( p, server_randbytes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; @@ -1028,16 +1028,9 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, return( ret ); } - -static int ssl_tls13_finalize_server_hello( mbedtls_ssl_context *ssl ) -{ - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); - return( 0 ); -} - static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *buf; size_t buf_len, msg_len; @@ -1055,10 +1048,10 @@ static int ssl_tls13_write_server_hello( mbedtls_ssl_context *ssl ) mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_finalize_server_hello( ssl ) ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS ); cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write server hello" ) ); From 57d4841eda86f6cbe62c1410dad4f8cf2a942860 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Apr 2022 21:50:42 +0800 Subject: [PATCH 12/18] fix write key_share issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 0810f333db..ed1acbd24c 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -859,10 +859,9 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, size_t *out_len ) { unsigned char *p = buf; - unsigned char *start = buf; - uint16_t group = ssl->handshake->offered_group_id ; + uint16_t group = ssl->handshake->offered_group_id; unsigned char *server_share = buf + 4; - unsigned char *key_exchange = buf + 6; + unsigned char *p_key_exchange_len = buf + 6; size_t key_exchange_length; int ret; @@ -877,29 +876,22 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, * - key_exchange_length (2 bytes) */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 8 ); - + MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, p, 0 ); + MBEDTLS_PUT_UINT16_BE( group, server_share, 0 ); p += 8; + /* When we introduce PQC-ECDHE hybrids, we'll want to call this * function multiple times. */ - ret = ssl_tls13_key_share_encapsulate( ssl, group, key_exchange + 2, + ret = ssl_tls13_key_share_encapsulate( ssl, group, p_key_exchange_len + 2, end, &key_exchange_length ); if( ret != 0 ) return( ret ); p += key_exchange_length; - /* Write length of key_exchange */ - MBEDTLS_PUT_UINT16_BE( key_exchange_length, key_exchange, 0 ); + MBEDTLS_PUT_UINT16_BE( key_exchange_length, p_key_exchange_len, 0 ); - *out_len = p - start; - - /* Write group ID */ - MBEDTLS_PUT_UINT16_BE( group, server_share, 0 ); - - /* Write extension header */ - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_KEY_SHARE, start, 0 ); - - /* Write total extension length */ - MBEDTLS_PUT_UINT16_BE( p - server_share, start, 2 ); + MBEDTLS_PUT_UINT16_BE( p - server_share, buf, 2 ); + *out_len = p - buf; return( 0 ); } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ From d9436a1baa7aa667e8cde88457b57bd22638a9f9 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 20 Apr 2022 22:28:09 +0800 Subject: [PATCH 13/18] remove guards for write_key_share Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ed1acbd24c..a41ccea124 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -798,7 +798,7 @@ static int ssl_tls13_write_selected_version_ext( mbedtls_ssl_context *ssl, return( 0 ); } -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + /* Generate and export a single key share. For hybrid KEMs, this can * be called multiple times with the different components of the hybrid. */ @@ -894,7 +894,7 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, *out_len = p - buf; return( 0 ); } -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + /* * Structure of ServerHello message: @@ -920,8 +920,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, /* Buffer management */ unsigned char *p = buf; - unsigned char *start = buf; - unsigned char *extension_start; + unsigned char *p_extensions_len; *out_len = 0; @@ -985,7 +984,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, /* Extensions */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - extension_start = p; + p_extensions_len = p; p += 2; /* Add supported_version extension */ @@ -998,7 +997,6 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, } p += output_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, &output_len ); @@ -1006,16 +1004,16 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, return( ret ); p += output_len; } -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* Write length information */ - MBEDTLS_PUT_UINT16_BE( p - extension_start - 2, extension_start, 0 ); + MBEDTLS_PUT_UINT16_BE( p - p_extensions_len - 2, p_extensions_len, 0 ); - MBEDTLS_SSL_DEBUG_BUF( 4, "server hello extensions", extension_start, p - extension_start ); + MBEDTLS_SSL_DEBUG_BUF( 4, "server hello extensions", + p_extensions_len, p - p_extensions_len ); - *out_len = p - start; + *out_len = p - buf; - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello", start, *out_len ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello", buf, *out_len ); return( ret ); } From e74e04af1aca4ed096289478f01ed49528a39651 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 21 Apr 2022 09:23:16 +0800 Subject: [PATCH 14/18] Rename write supported_versions ext Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index a41ccea124..1b3fc19897 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -760,16 +760,17 @@ static int ssl_tls13_prepare_server_hello( mbedtls_ssl_context *ssl ) } /* - * ssl_tls13_write_selected_version_ext(): + * ssl_tls13_write_server_hello_supported_versions_ext (): * * struct { * ProtocolVersion selected_version; * } SupportedVersions; */ -static int ssl_tls13_write_selected_version_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) +static int ssl_tls13_write_server_hello_supported_versions_ext( + mbedtls_ssl_context *ssl, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) { *out_len = 0; @@ -988,7 +989,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, p += 2; /* Add supported_version extension */ - if( ( ret = ssl_tls13_write_selected_version_ext( + if( ( ret = ssl_tls13_write_server_hello_supported_versions_ext( ssl, p, end, &output_len ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_selected_version_ext", From cfc04b35417d11142c38d15c827776a41d7fd8db Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 21 Apr 2022 09:31:58 +0800 Subject: [PATCH 15/18] Update comments in write server hello Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 41 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 1b3fc19897..421e45d8a6 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -925,12 +925,11 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, *out_len = 0; - /* - * Write legacy_version - * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 - * - * For TLS 1.3 we use the legacy version number {0x03, 0x03} - * instead of the true version number. + /* ... + * ProtocolVersion legacy_version = 0x0303; // TLS 1.2 + * ... + * with ProtocolVersion defined as: + * uint16 ProtocolVersion; */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); MBEDTLS_PUT_UINT16_BE( 0x0303, p, 0 ); @@ -939,6 +938,8 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, /* ... * Random random; * ... + * with Random defined as: + * opaque Random[MBEDTLS_SERVER_HELLO_RANDOM_LEN]; */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); memcpy( p, server_randbytes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); @@ -950,8 +951,9 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, ssl->session_negotiate->start = time( NULL ); #endif /* MBEDTLS_HAVE_TIME */ - /* - * Write legacy_session_id_echo + /* ... + * opaque legacy_session_id_echo<0..32>; + * ... */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 + ssl->session_negotiate->id_len ); *p++ = (unsigned char)ssl->session_negotiate->id_len; @@ -967,8 +969,11 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, ssl->session_negotiate->id_len ); } - /* - * Write ciphersuite + /* ... + * CipherSuite cipher_suite; + * ... + * with CipherSuite defined as: + * uint8 CipherSuite[2]; */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); MBEDTLS_PUT_UINT16_BE( ssl->session_negotiate->ciphersuite, p, 0 ); @@ -979,11 +984,21 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, ssl->session_negotiate->ciphersuite ), ssl->session_negotiate->ciphersuite ) ); - /* write legacy_compression_method = ( 0 ) */ + /* ... + * uint8 legacy_compression_method = 0; + * ... + */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); *p++ = 0x0; - /* Extensions */ + /* ... + * Extension extensions<6..2^16-1>; + * ... + * struct { + * ExtensionType extension_type; (2 bytes) + * opaque extension_data<0..2^16-1>; + * } Extension; + */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); p_extensions_len = p; p += 2; @@ -1000,13 +1015,13 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { + /* Add key_share extension */ ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; } - /* Write length information */ MBEDTLS_PUT_UINT16_BE( p - p_extensions_len - 2, p_extensions_len, 0 ); MBEDTLS_SSL_DEBUG_BUF( 4, "server hello extensions", From a09f5e98efa2f5d0c69dea575d291689cbcd0499 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 22 Apr 2022 16:46:03 +0800 Subject: [PATCH 16/18] fix build fail Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 421e45d8a6..1697124900 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -26,7 +26,6 @@ #include "ssl_misc.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" -#include "ecdh_misc.h" #if defined(MBEDTLS_ECP_C) #include "mbedtls/ecp.h" From 955ddd75a38fbbc42e15eecdf01397eeb32be84f Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 22 Apr 2022 22:27:33 +0800 Subject: [PATCH 17/18] fix various issues Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 61 ++++++++++++++++---------------------- tests/ssl-opt.sh | 3 +- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 1697124900..ebbfb6f912 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -773,7 +773,7 @@ static int ssl_tls13_write_server_hello_supported_versions_ext( { *out_len = 0; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, write selected_version" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, write selected version" ) ); /* Check if we have space to write the extension: * - extension_type (2 bytes) @@ -802,18 +802,16 @@ static int ssl_tls13_write_server_hello_supported_versions_ext( /* Generate and export a single key share. For hybrid KEMs, this can * be called multiple times with the different components of the hybrid. */ -static int ssl_tls13_key_share_encapsulate( mbedtls_ssl_context *ssl, - uint16_t named_group, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) +static int ssl_tls13_generate_and_write_key_share( mbedtls_ssl_context *ssl, + uint16_t named_group, + unsigned char *buf, + unsigned char *end, + size_t *out_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - ((void) ssl); - ((void) named_group); - ((void) buf); - ((void) end); - ((void) out_len); + + *out_len = 0; + #if defined(MBEDTLS_ECDH_C) if( mbedtls_ssl_tls13_named_group_is_ecdhe( named_group ) ) { @@ -834,6 +832,10 @@ static int ssl_tls13_key_share_encapsulate( mbedtls_ssl_context *ssl, } else { + ((void) ssl); + ((void) named_group); + ((void) buf); + ((void) end); ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; } @@ -858,12 +860,11 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, unsigned char *end, size_t *out_len ) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; uint16_t group = ssl->handshake->offered_group_id; unsigned char *server_share = buf + 4; - unsigned char *p_key_exchange_len = buf + 6; size_t key_exchange_length; - int ret; *out_len = 0; @@ -882,16 +883,17 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, /* When we introduce PQC-ECDHE hybrids, we'll want to call this * function multiple times. */ - ret = ssl_tls13_key_share_encapsulate( ssl, group, p_key_exchange_len + 2, - end, &key_exchange_length ); + ret = ssl_tls13_generate_and_write_key_share( + ssl, group, p_key_exchange_len + 2, end, &key_exchange_length ); if( ret != 0 ) return( ret ); p += key_exchange_length; - MBEDTLS_PUT_UINT16_BE( key_exchange_length, p_key_exchange_len, 0 ); + MBEDTLS_PUT_UINT16_BE( key_exchange_length, server_share + 2, 0 ); MBEDTLS_PUT_UINT16_BE( p - server_share, buf, 2 ); *out_len = p - buf; + return( 0 ); } @@ -913,14 +915,10 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, unsigned char *end, size_t *out_len ) { - int ret = 0; - size_t output_len; /* Length of buffer used by function */ - unsigned char *server_randbytes = - ssl->handshake->randbytes + MBEDTLS_CLIENT_HELLO_RANDOM_LEN; - - /* Buffer management */ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; unsigned char *p_extensions_len; + size_t output_len; /* Length of buffer used by function */ *out_len = 0; @@ -941,15 +939,12 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, * opaque Random[MBEDTLS_SERVER_HELLO_RANDOM_LEN]; */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - memcpy( p, server_randbytes, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", + memcpy( p, &ssl->handshake->randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN], + MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; -#if defined(MBEDTLS_HAVE_TIME) - ssl->session_negotiate->start = time( NULL ); -#endif /* MBEDTLS_HAVE_TIME */ - /* ... * opaque legacy_session_id_echo<0..32>; * ... @@ -961,9 +956,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, memcpy( p, &ssl->session_negotiate->id[0], ssl->session_negotiate->id_len ); p += ssl->session_negotiate->id_len; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "session id length ( %" - MBEDTLS_PRINTF_SIZET " )", - ssl->session_negotiate->id_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "session id", ssl->session_negotiate->id, ssl->session_negotiate->id_len ); } @@ -1002,19 +995,17 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, p_extensions_len = p; p += 2; - /* Add supported_version extension */ if( ( ret = ssl_tls13_write_server_hello_supported_versions_ext( ssl, p, end, &output_len ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_write_selected_version_ext", - ret ); + MBEDTLS_SSL_DEBUG_RET( + 1, "ssl_tls13_write_server_hello_supported_versions_ext", ret ); return( ret ); } p += output_len; if( mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) { - /* Add key_share extension */ ret = ssl_tls13_write_key_share_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c93cec121a..bcbc0a0ebb 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10504,11 +10504,10 @@ run_test "TLS 1.3: Server side check - gnutls" \ -s "=> parse client hello" \ -s "<= parse client hello" -requires_gnutls_tls1_3 -requires_gnutls_next_no_ticket requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C run_test "TLS 1.3: Server side check - mbedtls" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$P_CLI debug_level=4 force_version=tls13" \ From e65d80158095eb829764d5434fa8d077fc8fdc46 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Sat, 23 Apr 2022 10:34:35 +0800 Subject: [PATCH 18/18] fix undeclare error Signed-off-by: Jerry Yu --- library/ssl_tls13_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ebbfb6f912..d06b9a88ef 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -884,7 +884,7 @@ static int ssl_tls13_write_key_share_ext( mbedtls_ssl_context *ssl, /* When we introduce PQC-ECDHE hybrids, we'll want to call this * function multiple times. */ ret = ssl_tls13_generate_and_write_key_share( - ssl, group, p_key_exchange_len + 2, end, &key_exchange_length ); + ssl, group, server_share + 4, end, &key_exchange_length ); if( ret != 0 ) return( ret ); p += key_exchange_length;