From ed0620cb1372f8e17da6d771bc26a8f41089bfe2 Mon Sep 17 00:00:00 2001 From: Xiaokang Qian Date: Wed, 12 Oct 2022 06:58:13 +0000 Subject: [PATCH] Refine code base on comments Move code to proper macro guards protection Fix typo issues Signed-off-by: Xiaokang Qian --- library/ssl_client.c | 6 +-- library/ssl_misc.h | 3 +- library/ssl_tls.c | 101 +++++++++++++++++++------------------------ tests/ssl-opt.sh | 2 +- 4 files changed, 51 insertions(+), 61 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index baf82a69d..341e882cc 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -885,9 +885,9 @@ static int ssl_prepare_client_hello( mbedtls_ssl_context *ssl ) if( hostname_mismatch ) { - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "hostname mismatch the session ticket," - " should not resume " ) ); + MBEDTLS_SSL_DEBUG_MSG( + 1, ( "Hostname mismatch the session ticket, " + "disable session resumption." ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } } diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 6ba9ad519..82a0d5c8a 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2496,7 +2496,8 @@ int mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext( #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ - defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ + defined(MBEDTLS_SSL_TLS_C) MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_set_hostname( mbedtls_ssl_session *session, const char *hostname ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6563b907c..521b922b7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -295,12 +295,9 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, memcpy( dst->ticket, src->ticket, src->ticket_len ); } -#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ - defined(MBEDTLS_SSL_SESSION_TICKETS) && \ - defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ - defined(MBEDTLS_SSL_CLI_C) + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( src->endpoint == MBEDTLS_SSL_IS_CLIENT ) { dst->hostname = NULL; @@ -308,6 +305,7 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, src->hostname ); } #endif +#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */ return( 0 ); } @@ -1974,10 +1972,9 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, { unsigned char *p = buf; #if defined(MBEDTLS_SSL_CLI_C) && \ - defined(MBEDTLS_SSL_SESSION_TICKETS) && \ defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) size_t hostname_len = ( session->hostname == NULL ) ? - 0 : strlen( session->hostname ); + 0 : strlen( session->hostname ) + 1; #endif size_t needed = 1 /* endpoint */ + 2 /* ciphersuite */ @@ -2029,24 +2026,7 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, memcpy( p, session->resumption_key, session->resumption_key_len ); p += session->resumption_key_len; -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ - defined(MBEDTLS_SSL_SESSION_TICKETS) && \ - defined(MBEDTLS_SSL_CLI_C) - if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - MBEDTLS_PUT_UINT16_BE( hostname_len, p, 0 ); - p += 2; - if ( hostname_len > 0 && - session->hostname != NULL ) - { - /* save host name */ - memcpy( p, session->hostname, hostname_len ); - p += hostname_len; - } - } -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && - MBEDTLS_SSL_SESSION_TICKETS && - MBEDTLS_SSL_CLI_C */ + #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if( session->endpoint == MBEDTLS_SSL_IS_SERVER ) @@ -2059,6 +2039,17 @@ static int ssl_tls13_session_save( const mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_CLI_C) if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) { +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + MBEDTLS_PUT_UINT16_BE( hostname_len, p, 0 ); + p += 2; + if ( hostname_len > 0 ) + { + /* save host name */ + memcpy( p, session->hostname, hostname_len ); + p += hostname_len; + } +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + #if defined(MBEDTLS_HAVE_TIME) MBEDTLS_PUT_UINT64_BE( (uint64_t) session->ticket_received, p, 0 ); p += 8; @@ -2106,33 +2097,6 @@ static int ssl_tls13_session_load( mbedtls_ssl_session *session, memcpy( session->resumption_key, p, session->resumption_key_len ); p += session->resumption_key_len; -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ - defined(MBEDTLS_SSL_SESSION_TICKETS) && \ - defined(MBEDTLS_SSL_CLI_C) - if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) - { - size_t hostname_len; - /* load host name */ - if( end - p < 2 ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - hostname_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; - - if( end - p < ( long int )hostname_len ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - if( hostname_len > 0 ) - { - session->hostname = mbedtls_calloc( 1, hostname_len + 1 ); - if( session->hostname == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - memcpy( session->hostname, p, hostname_len ); - p += hostname_len; - } - } -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && - MBEDTLS_SSL_SESSION_TICKETS && - MBEDTLS_SSL_CLI_C */ - #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_SRV_C) if( session->endpoint == MBEDTLS_SSL_IS_SERVER ) { @@ -2146,6 +2110,28 @@ static int ssl_tls13_session_load( mbedtls_ssl_session *session, #if defined(MBEDTLS_SSL_CLI_C) if( session->endpoint == MBEDTLS_SSL_IS_CLIENT ) { +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ + defined(MBEDTLS_SSL_SESSION_TICKETS) + size_t hostname_len; + /* load host name */ + if( end - p < 2 ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + hostname_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + if( end - p < ( long int )hostname_len ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + if( hostname_len > 0 ) + { + session->hostname = mbedtls_calloc( 1, hostname_len ); + if( session->hostname == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + memcpy( session->hostname, p, hostname_len ); + p += hostname_len; + } +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && + MBEDTLS_SSL_SESSION_TICKETS */ + #if defined(MBEDTLS_HAVE_TIME) if( end - p < 8 ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -2475,6 +2461,7 @@ 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 ) ); @@ -2482,6 +2469,7 @@ 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; @@ -3745,13 +3733,12 @@ void mbedtls_ssl_session_free( mbedtls_ssl_session *session ) #endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) - mbedtls_free( session->ticket ); -#endif - #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) mbedtls_free( session->hostname ); #endif + mbedtls_free( session->ticket ); +#endif mbedtls_platform_zeroize( session, sizeof( mbedtls_ssl_session ) ); } @@ -8874,7 +8861,8 @@ int mbedtls_ssl_write_alpn_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ - defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) && \ + defined(MBEDTLS_SSL_TLS_C) int mbedtls_ssl_session_set_hostname( mbedtls_ssl_session *session, const char *hostname ) { @@ -8918,6 +8906,7 @@ int mbedtls_ssl_session_set_hostname( mbedtls_ssl_session *session, } #endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS && - MBEDTLS_SSL_SERVER_NAME_INDICATION */ + MBEDTLS_SSL_SERVER_NAME_INDICATION && + MBEDTLS_SSL_TLS_C */ #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2be5506a4..f6437f515 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -12909,7 +12909,7 @@ run_test "TLS 1.3: NewSessionTicket: servername negative check, m->m" \ -c "got new session ticket." \ -c "Saving session for reuse... ok" \ -c "Reconnecting with saved session" \ - -c "hostname mismatch the session ticket, should not resume" \ + -c "Hostname mismatch the session ticket, disable session resumption." \ -s "=> write NewSessionTicket msg" \ -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET" \ -s "server state: MBEDTLS_SSL_NEW_SESSION_TICKET_FLUSH"