From adfa64f0c4d99db9cf08ad927843c564209e8506 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 11:35:07 +0000 Subject: [PATCH] Abort idle-loop in ssl_server2 if sockets gets invalid Previously, the idling loop in ssl_server2 didn't check whether the underlying call to mbedtls_net_poll signalled that the socket became invalid. This had the consequence that during idling, the server couldn't be terminated through a SIGTERM, as the corresponding handler would only close the sockets and expect the remainder of the program to shutdown gracefully as a consequence of this. This was subsequently attempted to be fixed through a change in ssl-opt.sh by terminating the server through a KILL signal, which however lead to other problems when the latter was run under valgrind. This commit changes the idling loop in ssl_server2 and ssl_client2 to obey the return code of mbedtls_net_poll and gracefully shutdown if an error occurs, e.g. because the socket was closed. As a consequence, the server termination via a KILL signal in ssl-opt.sh is no longer necessary, with the previous `kill; wait` pattern being sufficient. The commit reverts the corresponding change. --- programs/ssl/ssl_client2.c | 22 +++++++++++++++------- programs/ssl/ssl_server2.c | 22 +++++++++++++++------- tests/ssl-opt.sh | 12 ++---------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 3d03269e64..023c0c5d12 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -444,16 +444,17 @@ static int ssl_sig_hashes_for_test[] = { * (Used in event-driven IO mode). */ #if !defined(MBEDTLS_TIMING_C) -void idle( mbedtls_net_context *fd, +int idle( mbedtls_net_context *fd, int idle_reason ) { #else -void idle( mbedtls_net_context *fd, +int idle( mbedtls_net_context *fd, mbedtls_timing_delay_context *timer, int idle_reason ) { #endif + int ret; int poll_type = 0; if( idle_reason == MBEDTLS_ERR_SSL_WANT_WRITE ) @@ -477,12 +478,17 @@ void idle( mbedtls_net_context *fd, #endif /* MBEDTLS_TIMING_C */ /* Check if underlying transport became available */ - if( poll_type != 0 && - mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) + if( poll_type != 0 ) { - break; + ret = mbedtls_net_poll( fd, poll_type, 0 ); + if( ret < 0 ) + return( ret ); + if( ret == poll_type ) + break; } } + + return( 0 ); } int main( int argc, char *argv[] ) @@ -1506,10 +1512,12 @@ int main( int argc, char *argv[] ) if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &server_fd, &timer, ret ); + ret = idle( &server_fd, &timer, ret ); #else - idle( &server_fd, ret ); + ret = idle( &server_fd, ret ); #endif + if( ret != 0 ) + goto exit; } } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 74a1142719..e296339720 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -846,16 +846,17 @@ static int ssl_sig_hashes_for_test[] = { * (Used in event-driven IO mode). */ #if !defined(MBEDTLS_TIMING_C) -void idle( mbedtls_net_context *fd, +int idle( mbedtls_net_context *fd, int idle_reason ) { #else -void idle( mbedtls_net_context *fd, +int idle( mbedtls_net_context *fd, mbedtls_timing_delay_context *timer, int idle_reason ) { #endif + int ret; int poll_type = 0; if( idle_reason == MBEDTLS_ERR_SSL_WANT_WRITE ) @@ -879,12 +880,17 @@ void idle( mbedtls_net_context *fd, #endif /* MBEDTLS_TIMING_C */ /* Check if underlying transport became available */ - if( poll_type != 0 && - mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) + if( poll_type != 0 ) { - break; + ret = mbedtls_net_poll( fd, poll_type, 0 ); + if( ret < 0 ) + return( ret ); + if( ret == poll_type ) + break; } } + + return( 0 ); } int main( int argc, char *argv[] ) @@ -2205,10 +2211,12 @@ handshake: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &client_fd, &timer, ret ); + ret = idle( &client_fd, &timer, ret ); #else - idle( &client_fd, ret ); + ret = idle( &client_fd, ret ); #endif + if( ret != 0 ) + goto reset; } } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index a1155e8d0a..1682a8476f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -447,19 +447,11 @@ run_test() { # terminate the server (and the proxy) kill $SRV_PID - sleep 0.01 - if kill -0 $SRV_PID >/dev/null 2>&1; then - kill -3 $SRV_PID - wait $SRV_PID - fi + wait $SRV_PID if [ -n "$PXY_CMD" ]; then kill $PXY_PID >/dev/null 2>&1 - sleep 0.01 - if kill -0 $PXY_PID >/dev/null 2>&1; then - kill -3 $PXY_PID - wait $PXY_PID - fi + wait $PXY_PID fi # retry only on timeouts