From a9eecf1b19f55c691d9d423203a34d81f123e8da Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Tue, 7 Jul 2020 11:11:02 +0200 Subject: [PATCH 1/4] If query_config is used only the config value is printed. The extra code running after the removed return instruction should not generate any output. Only the read config value must be printed. Signed-off-by: gabor-mezei-arm --- programs/ssl/ssl_client2.c | 22 +++++++++++++++----- programs/ssl/ssl_server2.c | 41 ++++++++++++++++++++++++++------------ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index a26dd5146ae9..d62d24ecb8de 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -149,6 +149,7 @@ int main( void ) #define DFL_NSS_KEYLOG 0 #define DFL_NSS_KEYLOG_FILE NULL #define DFL_SKIP_CLOSE_NOTIFY 0 +#define DFL_QUERY_CONFIG_MODE 0 #define GET_REQUEST "GET %s HTTP/1.0\r\nExtra-header: " #define GET_REQUEST_END "\r\n\r\n" @@ -539,6 +540,7 @@ struct options * after renegotiation */ int reproducible; /* make communication reproducible */ int skip_close_notify; /* skip sending the close_notify alert */ + int query_config_mode; /* whether to read config */ } opt; int query_config( const char *config ); @@ -1102,6 +1104,7 @@ int report_cid_usage( mbedtls_ssl_context *ssl, int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; + int query_config_ret = 0; mbedtls_net_context server_fd; io_ctx_t io_ctx; @@ -1300,6 +1303,7 @@ int main( int argc, char *argv[] ) opt.nss_keylog = DFL_NSS_KEYLOG; opt.nss_keylog_file = DFL_NSS_KEYLOG_FILE; opt.skip_close_notify = DFL_SKIP_CLOSE_NOTIFY; + opt.query_config_mode = DFL_QUERY_CONFIG_MODE; for( i = 1; i < argc; i++ ) { @@ -1686,7 +1690,9 @@ int main( int argc, char *argv[] ) } else if( strcmp( p, "query_config" ) == 0 ) { - mbedtls_exit( query_config( q ) ); + opt.query_config_mode = 1; + query_config_ret = query_config( q ); + mbedtls_exit( ret ); } else if( strcmp( p, "serialize") == 0 ) { @@ -3307,7 +3313,7 @@ int main( int argc, char *argv[] ) */ exit: #ifdef MBEDTLS_ERROR_C - if( ret != 0 ) + if( ret != 0 && opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) { char error_buf[100]; mbedtls_strerror( ret, error_buf, 100 ); @@ -3366,16 +3372,22 @@ int main( int argc, char *argv[] ) mbedtls_memory_buffer_alloc_free(); #endif + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + { #if defined(_WIN32) - mbedtls_printf( " + Press Enter to exit this program.\n" ); - fflush( stdout ); getchar(); + mbedtls_printf( " + Press Enter to exit this program.\n" ); + fflush( stdout ); getchar(); #endif + } // Shell can not handle large exit numbers -> 1 for errors if( ret < 0 ) ret = 1; - mbedtls_exit( ret ); + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + mbedtls_exit( ret ); + else + mbedtls_exit( query_config_ret ); } #endif /* MBEDTLS_BIGNUM_C && MBEDTLS_ENTROPY_C && MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_CLI_C && MBEDTLS_NET_C && MBEDTLS_RSA_C && diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index c445ddb042b9..3bc1712de6df 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -182,6 +182,7 @@ int main( void ) #define DFL_REPRODUCIBLE 0 #define DFL_NSS_KEYLOG 0 #define DFL_NSS_KEYLOG_FILE NULL +#define DFL_QUERY_CONFIG_MODE 0 #define LONG_RESPONSE "

01-blah-blah-blah-blah-blah-blah-blah-blah-blah\r\n" \ "02-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah-blah\r\n" \ @@ -643,6 +644,7 @@ struct options const char *cid_val_renego; /* the CID to use for incoming messages * after renegotiation */ int reproducible; /* make communication reproducible */ + int query_config_mode; /* whether to read config */ } opt; int query_config( const char *config ); @@ -1723,6 +1725,7 @@ int report_cid_usage( mbedtls_ssl_context *ssl, int main( int argc, char *argv[] ) { int ret = 0, len, written, frags, exchanges_left; + int query_config_ret = 0; int version_suites[4][2]; io_ctx_t io_ctx; unsigned char* buf = 0; @@ -1972,6 +1975,7 @@ int main( int argc, char *argv[] ) opt.reproducible = DFL_REPRODUCIBLE; opt.nss_keylog = DFL_NSS_KEYLOG; opt.nss_keylog_file = DFL_NSS_KEYLOG_FILE; + opt.query_config_mode = DFL_QUERY_CONFIG_MODE; for( i = 1; i < argc; i++ ) { @@ -2386,7 +2390,9 @@ int main( int argc, char *argv[] ) } else if( strcmp( p, "query_config" ) == 0 ) { - mbedtls_exit( query_config( q ) ); + opt.query_config_mode = 1; + query_config_ret = query_config( q ); + mbedtls_exit( ret ); } else if( strcmp( p, "serialize") == 0 ) { @@ -4252,17 +4258,20 @@ int main( int argc, char *argv[] ) * Cleanup and exit */ exit: -#ifdef MBEDTLS_ERROR_C - if( ret != 0 ) + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) { - char error_buf[100]; - mbedtls_strerror( ret, error_buf, 100 ); - mbedtls_printf("Last error was: -0x%X - %s\n\n", (unsigned int) -ret, error_buf ); - } +#ifdef MBEDTLS_ERROR_C + if( ret != 0 ) + { + char error_buf[100]; + mbedtls_strerror( ret, error_buf, 100 ); + mbedtls_printf("Last error was: -0x%X - %s\n\n", (unsigned int) -ret, error_buf ); + } #endif - mbedtls_printf( " . Cleaning up..." ); - fflush( stdout ); + mbedtls_printf( " . Cleaning up..." ); + fflush( stdout ); + } mbedtls_net_free( &client_fd ); mbedtls_net_free( &listen_fd ); @@ -4347,18 +4356,24 @@ int main( int argc, char *argv[] ) mbedtls_memory_buffer_alloc_free(); #endif - mbedtls_printf( " done.\n" ); + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + { + mbedtls_printf( " done.\n" ); #if defined(_WIN32) - mbedtls_printf( " + Press Enter to exit this program.\n" ); - fflush( stdout ); getchar(); + mbedtls_printf( " + Press Enter to exit this program.\n" ); + fflush( stdout ); getchar(); #endif + } // Shell can not handle large exit numbers -> 1 for errors if( ret < 0 ) ret = 1; - mbedtls_exit( ret ); + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + mbedtls_exit( ret ); + else + mbedtls_exit( query_config_ret ); } #endif /* MBEDTLS_BIGNUM_C && MBEDTLS_ENTROPY_C && MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_SRV_C && MBEDTLS_NET_C && MBEDTLS_RSA_C && From 785958577e2b2d34aeb0ab3bd979df96d0523b9d Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Tue, 28 Apr 2020 10:40:30 +0200 Subject: [PATCH 2/4] Use goto exit instead of direct return Signed-off-by: gabor-mezei-arm --- programs/ssl/ssl_client2.c | 4 ++-- programs/ssl/ssl_server2.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d62d24ecb8de..d624b196ce7f 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1692,7 +1692,7 @@ int main( int argc, char *argv[] ) { opt.query_config_mode = 1; query_config_ret = query_config( q ); - mbedtls_exit( ret ); + goto exit; } else if( strcmp( p, "serialize") == 0 ) { @@ -2691,7 +2691,7 @@ int main( int argc, char *argv[] ) { mbedtls_printf( " failed\n ! mbedtls_ssl_set_cid returned %d\n\n", ret ); - return( ret ); + goto exit; } } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 3bc1712de6df..1ccdeaecacf0 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2392,7 +2392,7 @@ int main( int argc, char *argv[] ) { opt.query_config_mode = 1; query_config_ret = query_config( q ); - mbedtls_exit( ret ); + goto exit; } else if( strcmp( p, "serialize") == 0 ) { From f1f7b29d76ff8198d952a279e9c0a016af7198e8 Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Thu, 11 Jun 2020 12:18:55 +0200 Subject: [PATCH 3/4] Fix overiding of return value. If MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED is defined, then the return value will be overridden by the extra code running after the removed return instruction. Signed-off-by: gabor-mezei-arm --- programs/ssl/ssl_server2.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 1ccdeaecacf0..c8c52d5021b5 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1734,6 +1734,7 @@ int main( int argc, char *argv[] ) psa_algorithm_t alg = 0; psa_key_handle_t psk_slot = 0; #endif /* MBEDTLS_USE_PSA_CRYPTO */ + int psk_free_ret = 0; unsigned char psk[MBEDTLS_PSK_MAX_LEN]; size_t psk_len = 0; psk_entry *psk_info = NULL; @@ -4301,8 +4302,8 @@ int main( int argc, char *argv[] ) sni_free( sni_info ); #endif #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) - if( ( ret = psk_free( psk_info ) ) != 0 ) - mbedtls_printf( "Failed to list of opaque PSKs - error was %d\n", ret ); + if( ( psk_free_ret = psk_free( psk_info ) ) != 0 ) + mbedtls_printf( "Failed to list of opaque PSKs - error was %d\n", psk_free_ret ); #endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_FS_IO) mbedtls_dhm_free( &dhm ); From de47217580846250583045caf5e2a6d2dd13ca7d Mon Sep 17 00:00:00 2001 From: gabor-mezei-arm Date: Fri, 4 Sep 2020 14:44:25 +0200 Subject: [PATCH 4/4] Do not print any messages if query_config option is used To preserve the behaviour of the query_config option all message is omitted it it is used. Signed-off-by: gabor-mezei-arm --- programs/ssl/ssl_client2.c | 9 +++++---- programs/ssl/ssl_server2.c | 25 +++++++++++++------------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d624b196ce7f..b9047df1d190 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -3313,7 +3313,7 @@ int main( int argc, char *argv[] ) */ exit: #ifdef MBEDTLS_ERROR_C - if( ret != 0 && opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + if( ret != 0 ) { char error_buf[100]; mbedtls_strerror( ret, error_buf, 100 ); @@ -3354,7 +3354,8 @@ int main( int argc, char *argv[] ) * immediately because of bad cmd line params, * for example). */ status = psa_destroy_key( slot ); - if( status != PSA_SUCCESS ) + if( ( status != PSA_SUCCESS ) && + ( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) ) { mbedtls_printf( "Failed to destroy key slot %u - error was %d", (unsigned) slot, (int) status ); @@ -3372,13 +3373,13 @@ int main( int argc, char *argv[] ) mbedtls_memory_buffer_alloc_free(); #endif +#if defined(_WIN32) if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) { -#if defined(_WIN32) mbedtls_printf( " + Press Enter to exit this program.\n" ); fflush( stdout ); getchar(); -#endif } +#endif // Shell can not handle large exit numbers -> 1 for errors if( ret < 0 ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index c8c52d5021b5..a98aec1191eb 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1734,7 +1734,6 @@ int main( int argc, char *argv[] ) psa_algorithm_t alg = 0; psa_key_handle_t psk_slot = 0; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - int psk_free_ret = 0; unsigned char psk[MBEDTLS_PSK_MAX_LEN]; size_t psk_len = 0; psk_entry *psk_info = NULL; @@ -4259,17 +4258,17 @@ int main( int argc, char *argv[] ) * Cleanup and exit */ exit: - if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) - { #ifdef MBEDTLS_ERROR_C - if( ret != 0 ) - { - char error_buf[100]; - mbedtls_strerror( ret, error_buf, 100 ); - mbedtls_printf("Last error was: -0x%X - %s\n\n", (unsigned int) -ret, error_buf ); - } + if( ret != 0 ) + { + char error_buf[100]; + mbedtls_strerror( ret, error_buf, 100 ); + mbedtls_printf("Last error was: -0x%X - %s\n\n", (unsigned int) -ret, error_buf ); + } #endif + if( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) + { mbedtls_printf( " . Cleaning up..." ); fflush( stdout ); } @@ -4302,8 +4301,9 @@ int main( int argc, char *argv[] ) sni_free( sni_info ); #endif #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) - if( ( psk_free_ret = psk_free( psk_info ) ) != 0 ) - mbedtls_printf( "Failed to list of opaque PSKs - error was %d\n", psk_free_ret ); + ret = psk_free( psk_info ); + if( ( ret != 0 ) && ( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) ) + mbedtls_printf( "Failed to list of opaque PSKs - error was %d\n", ret ); #endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_FS_IO) mbedtls_dhm_free( &dhm ); @@ -4318,7 +4318,8 @@ int main( int argc, char *argv[] ) * immediately because of bad cmd line params, * for example). */ status = psa_destroy_key( psk_slot ); - if( status != PSA_SUCCESS ) + if( ( status != PSA_SUCCESS ) && + ( opt.query_config_mode == DFL_QUERY_CONFIG_MODE ) ) { mbedtls_printf( "Failed to destroy key slot %u - error was %d", (unsigned) psk_slot, (int) status );