Address comments of various issues

Improve comments
Change coding style
Rename functions

Change-Id: Ia111aef303932cfeee693431c3d48f90342b32e5
Signed-off-by: XiaokangQian <xiaokang.qian@arm.com>
This commit is contained in:
XiaokangQian 2022-07-19 09:51:50 +00:00 committed by Ronald Cron
parent adab9a6440
commit 8698195566
4 changed files with 53 additions and 40 deletions

View File

@ -645,6 +645,9 @@ static int ssl_write_client_hello_body( mbedtls_ssl_context *ssl,
#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \
defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED)
/* The "pre_shared_key" extension (RFC 8446 Section 4.2.11)
* MUST be the last extension in the ClientHello.
*/
if( propose_tls13 && mbedtls_ssl_conf_tls13_some_psk_enabled( ssl ) ) if( propose_tls13 && mbedtls_ssl_conf_tls13_some_psk_enabled( ssl ) )
{ {
ret = mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders( ret = mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
@ -907,11 +910,11 @@ int mbedtls_ssl_write_client_hello( mbedtls_ssl_context *ssl )
if( binders_len > 0 ) if( binders_len > 0 )
{ {
MBEDTLS_SSL_PROC_CHK( MBEDTLS_SSL_PROC_CHK(
mbedtls_ssl_tls13_write_pre_shared_key_ext_binders( mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext(
ssl, buf + msg_len - binders_len, buf + msg_len ) ); ssl, buf + msg_len - binders_len, buf + msg_len ) );
ssl->handshake->update_checksum( ssl, buf + msg_len - binders_len,
binders_len );
} }
ssl->handshake->update_checksum( ssl, buf + msg_len - binders_len,
binders_len );
#endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */
MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl,

View File

@ -1336,9 +1336,6 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl );
void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl,
const mbedtls_ssl_ciphersuite_t *ciphersuite_info ); const mbedtls_ssl_ciphersuite_t *ciphersuite_info );
void mbedtls_ssl_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl,
unsigned hs_type,
size_t total_hs_len );
/* /*
* Update checksum of handshake messages. * Update checksum of handshake messages.
*/ */
@ -1347,6 +1344,10 @@ void mbedtls_ssl_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl,
unsigned char const *msg, unsigned char const *msg,
size_t msg_len ); size_t msg_len );
void mbedtls_ssl_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl,
unsigned hs_type,
size_t total_hs_len );
#if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED)
#if !defined(MBEDTLS_USE_PSA_CRYPTO) #if !defined(MBEDTLS_USE_PSA_CRYPTO)
MBEDTLS_CHECK_RETURN_CRITICAL MBEDTLS_CHECK_RETURN_CRITICAL
@ -2426,6 +2427,7 @@ int mbedtls_ssl_check_dtls_clihlo_cookie(
/* Check if we have any PSK to offer, returns 0 if PSK is available. /* Check if we have any PSK to offer, returns 0 if PSK is available.
* Assign the psk and ticket if pointers are present. * Assign the psk and ticket if pointers are present.
*/ */
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_get_psk_to_offer( int mbedtls_ssl_get_psk_to_offer(
const mbedtls_ssl_context *ssl, const mbedtls_ssl_context *ssl,
int *psk_type, int *psk_type,
@ -2445,6 +2447,7 @@ int mbedtls_ssl_get_psk_to_offer(
* \param[out] binders_len Length of the binders to be written at the end of * \param[out] binders_len Length of the binders to be written at the end of
* the extension. * the extension.
*/ */
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders( int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
mbedtls_ssl_context *ssl, mbedtls_ssl_context *ssl,
unsigned char *buf, unsigned char *end, unsigned char *buf, unsigned char *end,
@ -2459,7 +2462,8 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
* \param[in] buf Base address of the buffer where to write the binders * \param[in] buf Base address of the buffer where to write the binders
* \param[in] end End address of the buffer where to write the binders * \param[in] end End address of the buffer where to write the binders
*/ */
int mbedtls_ssl_tls13_write_pre_shared_key_ext_binders( MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext(
mbedtls_ssl_context *ssl, mbedtls_ssl_context *ssl,
unsigned char *buf, unsigned char *end ); unsigned char *buf, unsigned char *end );
#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */

View File

@ -605,15 +605,12 @@ static int ssl_tls13_write_cookie_ext( mbedtls_ssl_context *ssl,
* PskKeyExchangeMode ke_modes<1..255>; * PskKeyExchangeMode ke_modes<1..255>;
* } PskKeyExchangeModes; * } PskKeyExchangeModes;
*/ */
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl, static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl,
unsigned char *buf, unsigned char *buf,
unsigned char *end, unsigned char *end,
size_t *out_len ) size_t *out_len )
{ {
const unsigned char *psk;
size_t psk_len;
const unsigned char *psk_identity;
size_t psk_identity_len;
unsigned char *p = buf; unsigned char *p = buf;
int ke_modes_len = 0; int ke_modes_len = 0;
@ -623,9 +620,7 @@ static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl,
/* Skip writing extension if no PSK key exchange mode /* Skip writing extension if no PSK key exchange mode
* is enabled in the config or there is no PSK to offer. * is enabled in the config or there is no PSK to offer.
*/ */
if( !mbedtls_ssl_conf_tls13_some_psk_enabled( ssl ) || if( !mbedtls_ssl_conf_tls13_some_psk_enabled( ssl ) )
mbedtls_ssl_get_psk_to_offer( ssl, NULL, &psk, &psk_len,
&psk_identity, &psk_identity_len ) != 0 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip psk_key_exchange_modes extension" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip psk_key_exchange_modes extension" ) );
return( 0 ); return( 0 );
@ -682,10 +677,15 @@ static int ssl_tls13_write_psk_key_exchange_modes_ext( mbedtls_ssl_context *ssl,
* opaque PskBinderEntry<32..255>; * opaque PskBinderEntry<32..255>;
* *
* struct { * struct {
* PskIdentity identities<7..2^16-1>;
* PskBinderEntry binders<33..2^16-1>;
* } OfferedPsks;
* *
* PskIdentity identities<7..2^16-1>; * struct {
* PskBinderEntry binders<33..2^16-1>; * select (Handshake.msg_type) {
* * case client_hello: OfferedPsks;
* ...
* };
* } PreSharedKeyExtension; * } PreSharedKeyExtension;
* *
*/ */
@ -702,6 +702,7 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
size_t psk_len; size_t psk_len;
const unsigned char *psk_identity; const unsigned char *psk_identity;
size_t psk_identity_len; size_t psk_identity_len;
int psk_type;
const mbedtls_ssl_ciphersuite_t *ciphersuite_info = NULL; const mbedtls_ssl_ciphersuite_t *ciphersuite_info = NULL;
const int *ciphersuites; const int *ciphersuites;
psa_algorithm_t psa_hash_alg; psa_algorithm_t psa_hash_alg;
@ -725,7 +726,7 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
* - Otherwise, skip the PSK extension. * - Otherwise, skip the PSK extension.
*/ */
if( mbedtls_ssl_get_psk_to_offer( ssl, NULL, &psk, &psk_len, if( mbedtls_ssl_get_psk_to_offer( ssl, &psk_type, &psk, &psk_len,
&psk_identity, &psk_identity_len ) != 0 ) &psk_identity, &psk_identity_len ) != 0 )
{ {
MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip pre_shared_key extensions" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip pre_shared_key extensions" ) );
@ -736,19 +737,25 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
* Ciphersuite list * Ciphersuite list
*/ */
ciphersuites = ssl->conf->ciphersuite_list; ciphersuites = ssl->conf->ciphersuite_list;
for ( int i = 0; ciphersuites[i] != 0; i++ ) if( psk_type == MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL )
{ {
ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( ciphersuites[i] ); for( int i = 0; ciphersuites[i] != 0; i++ )
{
ciphersuite_info = mbedtls_ssl_ciphersuite_from_id(
ciphersuites[i] );
if( mbedtls_ssl_validate_ciphersuite( if( mbedtls_ssl_validate_ciphersuite(
ssl, ciphersuite_info, ssl, ciphersuite_info,
MBEDTLS_SSL_VERSION_TLS1_3, MBEDTLS_SSL_VERSION_TLS1_3,
MBEDTLS_SSL_VERSION_TLS1_3 ) != 0 ) MBEDTLS_SSL_VERSION_TLS1_3 ) != 0 )
continue; continue;
/* In this implementation we only add one pre-shared-key extension. */ /* In this implementation we only add one pre-shared-key
ssl->session_negotiate->ciphersuite = ciphersuites[i]; * extension.
break; */
ssl->session_negotiate->ciphersuite = ciphersuites[i];
break;
}
} }
ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( ciphersuite_info = mbedtls_ssl_ciphersuite_from_id(
@ -804,7 +811,7 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_without_binders(
return( 0 ); return( 0 );
} }
int mbedtls_ssl_tls13_write_pre_shared_key_ext_binders( int mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext(
mbedtls_ssl_context *ssl, mbedtls_ssl_context *ssl,
unsigned char *buf, unsigned char *end ) unsigned char *buf, unsigned char *end )
{ {
@ -847,11 +854,6 @@ int mbedtls_ssl_tls13_write_pre_shared_key_ext_binders(
/* 1 bytes length field for next psk binder */ /* 1 bytes length field for next psk binder */
*p++ = MBEDTLS_BYTE_0( hash_len ); *p++ = MBEDTLS_BYTE_0( hash_len );
if( ssl->handshake->resume == 1 )
psk_type = MBEDTLS_SSL_TLS1_3_PSK_RESUMPTION;
else
psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL;
/* Get current state of handshake transcript. */ /* Get current state of handshake transcript. */
ret = mbedtls_ssl_get_handshake_transcript( ssl, ciphersuite_info->mac, ret = mbedtls_ssl_get_handshake_transcript( ssl, ciphersuite_info->mac,
transcript, sizeof( transcript ), transcript, sizeof( transcript ),
@ -1234,15 +1236,19 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss
* *
* struct { * struct {
* *
* uint16 selected_identity; * select (Handshake.msg_type) {
* ...
* case server_hello: uint16 selected_identity;
* };
* *
* } PreSharedKeyExtension; * } PreSharedKeyExtension;
* *
*/ */
static int ssl_tls13_parse_server_psk_identity_ext( mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL
const unsigned char *buf, static int ssl_tls13_parse_server_pre_shared_key_ext( mbedtls_ssl_context *ssl,
const unsigned char *end ) const unsigned char *buf,
const unsigned char *end )
{ {
int ret = 0; int ret = 0;
size_t selected_identity; size_t selected_identity;
@ -1524,11 +1530,11 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl,
goto cleanup; goto cleanup;
} }
if( ( ret = ssl_tls13_parse_server_psk_identity_ext( if( ( ret = ssl_tls13_parse_server_pre_shared_key_ext(
ssl, p, extension_data_end ) ) != 0 ) ssl, p, extension_data_end ) ) != 0 )
{ {
MBEDTLS_SSL_DEBUG_RET( MBEDTLS_SSL_DEBUG_RET(
1, ( "ssl_tls13_parse_server_psk_identity_ext" ), ret ); 1, ( "ssl_tls13_parse_server_pre_shared_key_ext" ), ret );
return( ret ); return( ret );
} }
break; break;

View File

@ -1509,6 +1509,7 @@ int mbedtls_ssl_tls13_generate_and_write_ecdh_key_exchange(
/* Check if we have any PSK to offer, returns 0 if PSK is available. /* Check if we have any PSK to offer, returns 0 if PSK is available.
* Assign the psk and ticket if pointers are present. * Assign the psk and ticket if pointers are present.
*/ */
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_get_psk_to_offer( int mbedtls_ssl_get_psk_to_offer(
const mbedtls_ssl_context *ssl, const mbedtls_ssl_context *ssl,
int *psk_type, int *psk_type,
@ -1517,7 +1518,7 @@ int mbedtls_ssl_get_psk_to_offer(
{ {
int ptrs_present = 0; int ptrs_present = 0;
if( psk != NULL && psk_len != NULL && if( psk_type != NULL && psk != NULL && psk_len != NULL &&
psk_identity != NULL && psk_identity_len != NULL ) psk_identity != NULL && psk_identity_len != NULL )
{ {
ptrs_present = 1; ptrs_present = 1;
@ -1528,14 +1529,13 @@ int mbedtls_ssl_get_psk_to_offer(
{ {
if( ptrs_present ) if( ptrs_present )
{ {
*psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL;
*psk = ssl->conf->psk; *psk = ssl->conf->psk;
*psk_len = ssl->conf->psk_len; *psk_len = ssl->conf->psk_len;
*psk_identity = ssl->conf->psk_identity; *psk_identity = ssl->conf->psk_identity;
*psk_identity_len = ssl->conf->psk_identity_len; *psk_identity_len = ssl->conf->psk_identity_len;
} }
if( psk_type != NULL )
*psk_type = MBEDTLS_SSL_TLS1_3_PSK_EXTERNAL;
return( 0 ); return( 0 );
} }