From a3b451f9502b495171a2df825dc78be6452b286f Mon Sep 17 00:00:00 2001 From: Xiaokang Qian Date: Tue, 11 Oct 2022 06:20:56 +0000 Subject: [PATCH] Adress kinds of comments base on review Rename function name to mbedtls_ssl_session_set_hostname Add two extra check cases for server name Fix some coding styles Signed-off-by: Xiaokang Qian --- library/ssl_client.c | 81 ++++++++++---------------------------------- library/ssl_misc.h | 7 ++++ library/ssl_tls.c | 67 +++++++++++++++++++++++++++++------- 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index 0fda2712e4..993539573b 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -714,50 +714,6 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) return( ret ); } -static int ssl_session_set_hostname( mbedtls_ssl_session *session, - const char *hostname ) -{ - /* Initialize to suppress unnecessary compiler warning */ - size_t hostname_len = 0; - - /* Check if new hostname is valid before - * making any change to current one */ - if( hostname != NULL ) - { - hostname_len = strlen( hostname ); - - if( hostname_len > MBEDTLS_SSL_MAX_HOST_NAME_LEN ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - } - - /* Now it's clear that we will overwrite the old hostname, - * so we can free it safely */ - - if( session->hostname != NULL ) - { - mbedtls_platform_zeroize( session->hostname, - strlen( session->hostname ) ); - mbedtls_free( session->hostname ); - } - - /* Passing NULL as hostname shall clear the old one */ - - if( hostname == NULL ) - { - session->hostname = NULL; - } - else - { - session->hostname = mbedtls_calloc( 1, hostname_len + 1 ); - if( session->hostname == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - - memcpy( session->hostname, hostname, hostname_len ); - } - - return( 0 ); -} - MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_client_hello( mbedtls_ssl_context *ssl ) { @@ -917,34 +873,33 @@ static int ssl_prepare_client_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if( ssl->handshake->resume ) + if( ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_3 && + ssl->handshake->resume ) { - if( ssl->hostname != NULL && ssl->session_negotiate->hostname != NULL ) + int hostname_mismatch = 0; + if( ssl->session_negotiate->hostname != NULL ) { - size_t hostname_len = strlen( ssl->hostname ); - size_t negotiate_hostname_len = - strlen( ssl->session_negotiate->hostname ); - if( hostname_len != negotiate_hostname_len || \ - strncmp( ssl->hostname, ssl->session_negotiate->hostname, - hostname_len ) ) + if( ssl->hostname != NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "hostname mismatch the session ticket, should not resume " ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + if( strcmp( ssl->hostname, ssl->session_negotiate->hostname) ) + hostname_mismatch = 1; } + else + hostname_mismatch = 1; } - else if( ssl->session_negotiate->hostname != NULL ) + else + hostname_mismatch = ssl->hostname != NULL; + + if( hostname_mismatch ) { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "hostname missed, should not resume " ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "hostname mismatch the session ticket, should not resume " ) ); + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } } else - { - ssl_session_set_hostname( ssl->session_negotiate, - ssl->hostname ); - } + mbedtls_ssl_session_set_hostname( ssl->session_negotiate, + ssl->hostname ); #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SERVER_NAME_INDICATION */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 3ac81a365d..f20897d1c3 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2200,6 +2200,7 @@ static inline int mbedtls_ssl_tls13_sig_alg_is_supported( } return( 1 ); } + #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) @@ -2493,4 +2494,10 @@ int mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext( unsigned char *buf, unsigned char *end ); #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +int mbedtls_ssl_session_set_hostname( mbedtls_ssl_session *session, + const char *hostname ); +#endif + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 35e9fecf5d..062259b5cd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -300,14 +300,11 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ defined(MBEDTLS_SSL_CLI_C) - if( src->endpoint == MBEDTLS_SSL_IS_CLIENT && src->hostname != NULL ) + if( src->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - size_t hostname_len = strlen( src->hostname ); - dst->hostname = mbedtls_calloc( 1, hostname_len + 1 ); - if( dst->hostname == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - - strncpy( dst->hostname, src->hostname, hostname_len ); + dst->hostname = NULL; + mbedtls_ssl_session_set_hostname( dst, + src->hostname ); } #endif @@ -1975,6 +1972,11 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, size_t *olen ) { unsigned char *p = buf; +#if defined(MBEDTLS_SSL_CLI_C) && \ + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + size_t hostname_len = ( session->hostname == NULL ) ? + 0 : strlen( session->hostname ); +#endif size_t needed = 1 /* endpoint */ + 2 /* ciphersuite */ + 4 /* ticket_age_add */ @@ -1993,7 +1995,6 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_CLI_C) if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - size_t hostname_len = session->hostname == NULL?0:strlen( session->hostname ); #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) needed += 2 /* hostname_len */ + hostname_len; /* hostname */ @@ -2028,7 +2029,6 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && defined(MBEDTLS_SSL_CLI_C) if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) { - size_t hostname_len = session->hostname == NULL?0:strlen(session->hostname); MBEDTLS_PUT_UINT16_BE( hostname_len, p, 0 ); p += 2; if ( hostname_len > 0 && @@ -2106,7 +2106,7 @@ static int ssl_tls13_session_load( mbedtls_ssl_session *session, /* load host name */ if( end - p < 2 ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - hostname_len = MBEDTLS_GET_UINT16_BE( p, 0); + hostname_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; if( end - p < ( long int )hostname_len ) @@ -2464,7 +2464,6 @@ int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ) /* Now it's clear that we will overwrite the old hostname, * so we can free it safely */ - if( ssl->hostname != NULL ) { mbedtls_platform_zeroize( ssl->hostname, strlen( ssl->hostname ) ); @@ -2472,7 +2471,6 @@ int mbedtls_ssl_set_hostname( mbedtls_ssl_context *ssl, const char *hostname ) } /* Passing NULL as hostname shall clear the old one */ - if( hostname == NULL ) { ssl->hostname = NULL; @@ -8863,4 +8861,49 @@ int mbedtls_ssl_write_alpn_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_ALPN */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +int mbedtls_ssl_session_set_hostname( mbedtls_ssl_session *session, + const char *hostname ) +{ + /* Initialize to suppress unnecessary compiler warning */ + size_t hostname_len = 0; + + /* Check if new hostname is valid before + * making any change to current one */ + if( hostname != NULL ) + { + hostname_len = strlen( hostname ); + + if( hostname_len > MBEDTLS_SSL_MAX_HOST_NAME_LEN ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + + /* Now it's clear that we will overwrite the old hostname, + * so we can free it safely */ + if( session->hostname != NULL ) + { + mbedtls_platform_zeroize( session->hostname, + strlen( session->hostname ) ); + mbedtls_free( session->hostname ); + } + + /* Passing NULL as hostname shall clear the old one */ + if( hostname == NULL ) + { + session->hostname = NULL; + } + else + { + session->hostname = mbedtls_calloc( 1, hostname_len + 1 ); + if( session->hostname == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + + memcpy( session->hostname, hostname, hostname_len ); + } + + return( 0 ); +} +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SERVER_NAME_INDICATION */ + #endif /* MBEDTLS_SSL_TLS_C */