From 17c0493ca887091e39e541de4b2c6955e0a62036 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 14:44:53 +0100 Subject: [PATCH 01/55] Allow default arguments for client/server/proxy in ssl-opt.sh ssl-opt.sh checks whether the client, server and proxy commands are names of executable files, forbidding the use of default arguments by by e.g. setting P_SRV="ssl_server2 debug_level=3". This commit relaxes this check, only considering the part of the command string prior to the first whitespace. --- tests/ssl-opt.sh | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 280fc6348..7f5510cce 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -597,16 +597,22 @@ fi get_options "$@" # sanity checks, avoid an avalanche of errors -if [ ! -x "$P_SRV" ]; then - echo "Command '$P_SRV' is not an executable file" +P_SRV_BIN=$(echo "$P_SRV" | sed -r -n "s/^([^ ]*).*$/\1/p") +echo "Server binary: ${P_SRV_BIN}" +P_CLI_BIN=$(echo "$P_CLI" | sed -r -n "s/^([^ ]*).*$/\1/p") +echo "Client binary: ${P_CLI_BIN}" +P_PXY_BIN=$(echo "$P_PXY" | sed -r -n "s/^([^ ]*).*$/\1/p") +echo "Proxy binary: ${P_PXY_BIN}" +if [ ! -x "$P_SRV_BIN" ]; then + echo "Command '$P_SRV_BIN' is not an executable file" exit 1 fi -if [ ! -x "$P_CLI" ]; then - echo "Command '$P_CLI' is not an executable file" +if [ ! -x "$P_CLI_BIN" ]; then + echo "Command '$P_CLI_BIN' is not an executable file" exit 1 fi -if [ ! -x "$P_PXY" ]; then - echo "Command '$P_PXY' is not an executable file" +if [ ! -x "$P_PXY_BIN" ]; then + echo "Command '$P_PXY_BIN' is not an executable file" exit 1 fi if [ "$MEMCHECK" -gt 0 ]; then From f65ca329b6e9b75694fce075f5eef8d19681e4a6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 14:44:57 +0100 Subject: [PATCH 02/55] Introduce UDP proxy wrapper script This commit introduces the script `programs/test/udp_proxy_wrapper.sh` which can be used to wrap the SSL server binary `programs/ssl/ssl_server2` by the UDP proxy application `programs/test/udp_proxy` while maintaining the same interface from the command line. Specifically, given UDP proxy arguments ARGS_UDP and SSL server arguments ARGS_SSL, the command line > ./udp_proxy_wrapper.sh ARGS_UDP -- ARGS_SSL behaves like > ./ssl_server2 ARGS_SSL wrapped by > ./udp_proxy ARGS_UDP The motivation and benefit of this is that scripts like `ssl-opt.sh` can be used with the server command line `P_SRV` modified to `./udp_proxy_wrapper.sh ARGS_UDP -- DEFAULT_ARGS_SSL` which will result in all tests being executed for an SSL server behind a UDP proxy. --- programs/test/udp_proxy_wrapper.sh | 103 +++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100755 programs/test/udp_proxy_wrapper.sh diff --git a/programs/test/udp_proxy_wrapper.sh b/programs/test/udp_proxy_wrapper.sh new file mode 100755 index 000000000..415f88399 --- /dev/null +++ b/programs/test/udp_proxy_wrapper.sh @@ -0,0 +1,103 @@ +#!/bin/sh + +set -u + +MBEDTLS_BASE="$(pwd)/$(dirname $0)/../../" +TPXY_BIN="$MBEDTLS_BASE/test/udp_proxy" +SRV_BIN="$MBEDTLS_BASE/programs/ssl/ssl_server2" + +: ${VERBOSE:=0} +VERBOSE=1 + +PARAM_SEP="^(.*)--(.*)$" +PROXY_PARAMS=$(echo $@ | sed -n -r "s/$PARAM_SEP/\1/p") +SERVER_PARAMS=$(echo $@ | sed -n -r "s/$PARAM_SEP/\2/p") + +stop_proxy() { + test -n "${TPXY_PID:-}" && + ( + echo "\n * Killing proxy (pid $TPXY_PID) ..." + kill $TPXY_PID + ) +} + +stop_server() { + test -n "${SRV_PID:-}" && + ( + echo "\n * Killing server (pid $SRV_PID) ..." + kill $SRV_PID >/dev/null 2>/dev/null + ) +} + +cleanup() { + stop_server + stop_proxy + return 1 +} + +trap cleanup INT TERM HUP + +DTLS_ENABLED=$(echo "$SERVER_PARAMS" | grep -v "::1" | grep "dtls=1") +if [ -z "$DTLS_ENABLED" ]; then + echo " * Couldn't find DTLS enabling, or IPv6 is in use - immediate fallback to server application..." + if [ $VERBOSE -gt 0 ]; then + echo "[ $SRV_BIN $SERVER_PARAMS ]" + fi + $SRV_BIN $SERVER_PARAMS >&1 2>&1 & + SRV_PID=$! + wait $SRV_PID + exit 0 +fi + +SERVER_PORT_ORIG=$(echo "$SERVER_PARAMS" | sed -n -r "s/^.*server_port=([0-9]+).*$/\1/p") +if [ -z "$SERVER_PORT_ORIG" ]; then + echo " * No server port specified - exit" + exit 1 +fi + +SERVER_ADDR_ORIG=$(echo "$SERVER_PARAMS" | sed -n -r "s/^.*server_addr=([a-zA-Z0-9\.]+).*$/\1/p") +if [ -z "$SERVER_ADDR_ORIG" ]; then + echo " * No server address specified - exit" + exit 1 +fi + +echo " * Server address: $SERVER_ADDR_ORIG" +echo " * Server port: $SERVER_PORT_ORIG" + +SERVER_PORT=$(( $SERVER_PORT_ORIG + 1 )) +echo " * Intermediate port: $SERVER_PORT" + +TPXY_CMD=\ +"$TPXY_BIN $PROXY_PARAMS "\ +"listen_port=$SERVER_PORT_ORIG "\ +"server_port=$SERVER_PORT "\ +"server_addr=$SERVER_ADDR_ORIG "\ +"listen_addr=$SERVER_ADDR_ORIG" + +echo " * Start proxy in background ..." +if [ $VERBOSE -gt 0 ]; then + echo "[ $TPXY_CMD ]" +fi + +$TPXY_CMD >/dev/null 2>&1 & +TPXY_PID=$! + +if [ $VERBOSE -gt 0 ]; then + echo " * Proxy ID: $TPXY_PID" +fi + +SERVER_PARAMS_NEW=$(echo $SERVER_PARAMS | sed -n -r "s/^(.*server_port=)[0-9]+(.*)$/\1$SERVER_PORT\2/p") +SRV_CMD="$SRV_BIN $SERVER_PARAMS_NEW" + +echo " * Starting server ..." +if [ $VERBOSE -gt 0 ]; then + echo "[ $SRV_CMD ]" +fi + +$SRV_CMD >&2 & +SRV_PID=$! + +wait $SRV_PID + +stop_proxy +return 0 From 1dd62ea81139e9fff902b6ee9e5701f342d4e022 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 May 2017 14:30:59 +0100 Subject: [PATCH 03/55] Add packing option to UDP proxy This commit provides the new option pack=TIME for the udp proxy ./programs/test/udp_proxy. If used, udp packets with the same destination will be queued and concatenated for up to TIME milliseconds before being delivered. This is useful to test how mbed TLS's deals with multiple DTLS records within a single datagram. --- programs/test/udp_proxy.c | 137 +++++++++++++++++++++++++++++++++++--- 1 file changed, 128 insertions(+), 9 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 20624d227..bb5537ff1 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -85,6 +85,7 @@ int main( void ) #define DFL_SERVER_PORT "4433" #define DFL_LISTEN_ADDR "localhost" #define DFL_LISTEN_PORT "5556" +#define DFL_PACK 0 #define USAGE \ "\n usage: udp_proxy param=<>...\n" \ @@ -108,6 +109,8 @@ int main( void ) " protect_len=%%d default: (don't protect packets of this size)\n" \ "\n" \ " seed=%%d default: (use current time)\n" \ + " pack=%%d default: 0 (don't merge)\n" \ + " options: t > 0 (merge for t milliseconds)\n" \ "\n" /* @@ -128,6 +131,8 @@ static struct options int bad_ad; /* inject corrupted ApplicationData record */ int protect_hvr; /* never drop or delay HelloVerifyRequest */ int protect_len; /* never drop/delay packet of the given size*/ + int merge; /* merge packets into single datagram for + * at most \c merge milliseconds if > 0 */ unsigned int seed; /* seed for "random" events */ } opt; @@ -152,6 +157,7 @@ static void get_options( int argc, char *argv[] ) opt.server_port = DFL_SERVER_PORT; opt.listen_addr = DFL_LISTEN_ADDR; opt.listen_port = DFL_LISTEN_PORT; + opt.merge = DFL_PACK; /* Other members default to 0 */ for( i = 1; i < argc; i++ ) @@ -193,6 +199,10 @@ static void get_options( int argc, char *argv[] ) if( opt.drop < 0 || opt.drop > 20 || opt.drop == 1 ) exit_usage( p, q ); } + else if( strcmp( p, "pack" ) == 0 ) + { + opt.merge = atoi( q ); + } else if( strcmp( p, "mtu" ) == 0 ) { opt.mtu = atoi( q ); @@ -288,6 +298,94 @@ static unsigned long ellapsed_time( void ) #endif } +typedef struct +{ + mbedtls_net_context *ctx; + + const char *description; + + unsigned long packet_lifetime; + size_t num_datagrams; + + unsigned char data[MAX_MSG_SIZE]; + unsigned len; + +} ctx_buffer; + +static ctx_buffer outbuf[2]; + +static int ctx_buffer_flush( ctx_buffer *buf ) +{ + int ret; + + mbedtls_printf( " %05lu flush %s: %u bytes, %lu datagrams, " + "last %ld ms\n", ellapsed_time(), + buf->description, buf->len, buf->num_datagrams, + ellapsed_time() - buf->packet_lifetime ); + + ret = mbedtls_net_send( buf->ctx, buf->data, buf->len ); + + buf->len = 0; + buf->num_datagrams = 0; + + return( ret ); +} + +static inline int ctx_buffer_check( ctx_buffer *buf ) +{ + if( buf->len > 0 && + ellapsed_time() - buf->packet_lifetime >= (size_t) opt.merge ) + { + return( ctx_buffer_flush( buf ) ); + } + + return( 0 ); +} + +static int ctx_buffer_append( ctx_buffer *buf, + const unsigned char * data, + size_t len ) +{ + int ret; + + if( len > sizeof( buf->data ) ) + { + mbedtls_printf( " ! buffer size %lu too large (max %lu)\n", + len, sizeof( buf->data ) ); + return( -1 ); + } + + if( sizeof( buf->data ) - buf->len < len ) + { + if( ( ret = ctx_buffer_flush( buf ) ) <= 0 ) + return( ret ); + } + + memcpy( buf->data + buf->len, data, len ); + + buf->len += len; + if( ++buf->num_datagrams == 1 ) + buf->packet_lifetime = ellapsed_time(); + + return( len ); +} + +static int dispatch_data( mbedtls_net_context *ctx, + const unsigned char * data, + size_t len ) +{ + ctx_buffer *buf = NULL; + if( outbuf[0].ctx == ctx ) + buf = &outbuf[0]; + else if( outbuf[1].ctx == ctx ) + buf = &outbuf[1]; + + if( buf == NULL ) + return( mbedtls_net_send( ctx, data, len ) ); + + return( ctx_buffer_append( buf, data, len ) ); +} + typedef struct { mbedtls_net_context *dst; @@ -301,10 +399,10 @@ typedef struct void print_packet( const packet *p, const char *why ) { if( why == NULL ) - mbedtls_printf( " %05lu %s %s (%u bytes)\n", + mbedtls_printf( " %05lu dispatch %s %s (%u bytes)\n", ellapsed_time(), p->way, p->type, p->len ); else - mbedtls_printf( " %s %s (%u bytes): %s\n", + mbedtls_printf( " dispatch %s %s (%u bytes): %s\n", p->way, p->type, p->len, why ); fflush( stdout ); } @@ -323,17 +421,17 @@ int send_packet( const packet *p, const char *why ) ++buf[p->len - 1]; print_packet( p, "corrupted" ); - if( ( ret = mbedtls_net_send( dst, buf, p->len ) ) <= 0 ) + if( ( ret = dispatch_data( dst, buf, p->len ) ) <= 0 ) { - mbedtls_printf( " ! mbedtls_net_send returned %d\n", ret ); + mbedtls_printf( " ! dispatch returned %d\n", ret ); return( ret ); } } print_packet( p, why ); - if( ( ret = mbedtls_net_send( dst, p->buf, p->len ) ) <= 0 ) + if( ( ret = dispatch_data( dst, p->buf, p->len ) ) <= 0 ) { - mbedtls_printf( " ! mbedtls_net_send returned %d\n", ret ); + mbedtls_printf( " ! dispatch returned %d\n", ret ); return( ret ); } @@ -344,9 +442,9 @@ int send_packet( const packet *p, const char *why ) { print_packet( p, "duplicated" ); - if( ( ret = mbedtls_net_send( dst, p->buf, p->len ) ) <= 0 ) + if( ( ret = dispatch_data( dst, p->buf, p->len ) ) <= 0 ) { - mbedtls_printf( " ! mbedtls_net_send returned %d\n", ret ); + mbedtls_printf( " ! dispatch returned %d\n", ret ); return( ret ); } } @@ -471,10 +569,14 @@ int main( int argc, char *argv[] ) int ret; mbedtls_net_context listen_fd, client_fd, server_fd; + struct timeval tm; int nb_fds; fd_set read_fds; + tm.tv_sec = 0; + tm.tv_usec = 0; + mbedtls_net_init( &listen_fd ); mbedtls_net_init( &client_fd ); mbedtls_net_init( &server_fd ); @@ -560,6 +662,19 @@ accept: nb_fds = listen_fd.fd; ++nb_fds; + if( opt.merge > 0 ) + { + outbuf[0].ctx = &server_fd; + outbuf[0].description = "S <- C"; + outbuf[0].num_datagrams = 0; + outbuf[0].len = 0; + + outbuf[1].ctx = &client_fd; + outbuf[1].description = "S -> C"; + outbuf[1].num_datagrams = 0; + outbuf[1].len = 0; + } + while( 1 ) { FD_ZERO( &read_fds ); @@ -567,7 +682,10 @@ accept: FD_SET( client_fd.fd, &read_fds ); FD_SET( listen_fd.fd, &read_fds ); - if( ( ret = select( nb_fds, &read_fds, NULL, NULL, NULL ) ) <= 0 ) + ctx_buffer_check( &outbuf[0] ); + ctx_buffer_check( &outbuf[1] ); + + if( ( ret = select( nb_fds, &read_fds, NULL, NULL, &tm ) ) < 0 ) { perror( "select" ); goto exit; @@ -589,6 +707,7 @@ accept: &client_fd, &server_fd ) ) != 0 ) goto accept; } + } exit: From fbb0b701e4d6ee7c5cd30394ddde8cf1e4def5d7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 26 May 2017 16:55:07 +0100 Subject: [PATCH 04/55] Corrupt application data in the beginning instead of the end in UDP proxy The UDP proxy corrupts application data at the end of the datagram. If there are multiple DTLS records within the same datagram, this leads to the wrong message being corrupted. This commit always corrupts the beginning of the message to prevent this. Overall, the UDP proxy needs reworking if it is supposed to reliably support multiple records within a single datagram, because it determines its actions from the type of the first record in the current datagram only. --- programs/test/udp_proxy.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index bb5537ff1..c978f9047 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -418,9 +418,17 @@ int send_packet( const packet *p, const char *why ) { unsigned char buf[MAX_MSG_SIZE]; memcpy( buf, p->buf, p->len ); - ++buf[p->len - 1]; - print_packet( p, "corrupted" ); + if( p->len <= 13 ) + { + mbedtls_printf( " ! can't corrupt empty AD record" ); + } + else + { + ++buf[13]; + print_packet( p, "corrupted" ); + } + if( ( ret = dispatch_data( dst, buf, p->len ) ) <= 0 ) { mbedtls_printf( " ! dispatch returned %d\n", ret ); From e65ce7862a40a9abbfda6ae374cb755033029a8d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 May 2017 14:47:48 +0100 Subject: [PATCH 05/55] Enhance debugging output in ssl_tls.c Give a note on the debugging output on the following occasions: (1) The timer expires in mbedtls_ssl_fetch_input (2) There's more than one records within a single datagram --- library/ssl_tls.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 661ae7065..759dca013 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2294,7 +2294,10 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) * that will end up being dropped. */ if( ssl_check_timer( ssl ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "timer has expired" ) ); ret = MBEDTLS_ERR_SSL_TIMEOUT; + } else { len = MBEDTLS_SSL_BUFFER_LEN - ( ssl->in_hdr - ssl->in_buf ); @@ -3921,7 +3924,13 @@ read_record_header: /* Done reading this record, get ready for the next one */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { ssl->next_record_offset = ssl->in_msglen + mbedtls_ssl_hdr_len( ssl ); + if( ssl->next_record_offset < ssl->in_left ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "more than one record within datagram" ) ); + } + } else #endif ssl->in_left = 0; From e09ca3d9b68486fa9a4c368fd6578c68fc54c242 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 22 May 2017 15:06:06 +0100 Subject: [PATCH 06/55] Add polling function for network contexts This commit adds a function `mbedtls_net_poll` to the network module allowing to check if a network context is available for read or write. --- include/mbedtls/net_sockets.h | 28 ++++++++++++++++++ library/error.c | 4 +++ library/net_sockets.c | 56 +++++++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index de335526f..2777b79e4 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -45,12 +45,17 @@ #define MBEDTLS_ERR_NET_UNKNOWN_HOST -0x0052 /**< Failed to get an IP address for the given hostname. */ #define MBEDTLS_ERR_NET_BUFFER_TOO_SMALL -0x0043 /**< Buffer is too small to hold the data. */ #define MBEDTLS_ERR_NET_INVALID_CONTEXT -0x0045 /**< The context is invalid, eg because it was free()ed. */ +#define MBEDTLS_ERR_NET_POLL_FAILED -0x0047 /**< Polling the net context failed. */ +#define MBEDTLS_ERR_NET_BAD_INPUT_DATA -0x0049 /**< Input invalid. */ #define MBEDTLS_NET_LISTEN_BACKLOG 10 /**< The backlog that listen() should use. */ #define MBEDTLS_NET_PROTO_TCP 0 /**< The TCP transport protocol */ #define MBEDTLS_NET_PROTO_UDP 1 /**< The UDP transport protocol */ +#define MBEDTLS_NET_POLL_READ 1 /**< Used in \c mbedtls_net_poll to check for pending data */ +#define MBEDTLS_NET_POLL_WRITE 2 /**< Used in \c mbedtls_net_poll to check if write possible */ + #ifdef __cplusplus extern "C" { #endif @@ -131,6 +136,29 @@ int mbedtls_net_accept( mbedtls_net_context *bind_ctx, mbedtls_net_context *client_ctx, void *client_ip, size_t buf_size, size_t *ip_len ); +/** + * \brief Check and wait for the context to be ready for read/write + * + * \param ctx Socket to check + * \param rw Bitflag composed of MBEDTLS_NET_POLL_READ and + * MBEDTLS_NET_POLL_WRITE specifying the events + * to wait for: + * - If MBEDTLS_NET_POLL_READ is set, the function + * will return as soon as the net context is available + * for reading. + * - If MBEDTLS_NET_POLL_WRITE is set, the function + * will return as soon as the net context is available + * for writing. + * \param timeout Maximal amount of time to wait before returning, + * in milliseconds. If \c timeout is zero, the + * function returns immediately. If \c timeout is + * -1u, the function blocks potentially indefinitely. + * + * \return Bitmask composed of MBEDTLS_NET_POLL_READ/WRITE + * on success or timeout, or a negative return code otherwise. + */ +int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ); + /** * \brief Set the socket blocking * diff --git a/library/error.c b/library/error.c index db42381c4..8977cc4e5 100644 --- a/library/error.c +++ b/library/error.c @@ -654,6 +654,10 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "NET - Buffer is too small to hold the data" ); if( use_ret == -(MBEDTLS_ERR_NET_INVALID_CONTEXT) ) mbedtls_snprintf( buf, buflen, "NET - The context is invalid, eg because it was free()ed" ); + if( use_ret == -(MBEDTLS_ERR_NET_POLL_FAILED) ) + mbedtls_snprintf( buf, buflen, "NET - Polling the net context failed" ); + if( use_ret == -(MBEDTLS_ERR_NET_BAD_INPUT_DATA) ) + mbedtls_snprintf( buf, buflen, "NET - Input invalid" ); #endif /* MBEDTLS_NET_C */ #if defined(MBEDTLS_OID_C) diff --git a/library/net_sockets.c b/library/net_sockets.c index 80be6ec6a..edd084416 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -433,6 +433,58 @@ int mbedtls_net_set_nonblock( mbedtls_net_context *ctx ) #endif } +/* + * Check if data is available on the socket + */ + +int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) +{ + int ret; + struct timeval tv; + + fd_set read_fds; + fd_set write_fds; + + int fd = ctx->fd; + + if( fd < 0 ) + return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + + FD_ZERO( &read_fds ); + if( rw & MBEDTLS_NET_POLL_READ ) + { + rw &= ~MBEDTLS_NET_POLL_READ; + FD_SET( fd, &read_fds ); + } + + FD_ZERO( &write_fds ); + if( rw & MBEDTLS_NET_POLL_WRITE ) + { + rw &= ~MBEDTLS_NET_POLL_WRITE; + FD_SET( fd, &write_fds ); + } + + if( rw != 0 ) + return( MBEDTLS_ERR_NET_BAD_INPUT_DATA ); + + tv.tv_sec = timeout / 1000; + tv.tv_usec = ( timeout % 1000 ) * 1000; + + ret = select( fd + 1, &read_fds, &write_fds, NULL, + timeout == (uint32_t) -1u ? NULL : &tv ); + + if( ret < 0 ) + return( MBEDTLS_ERR_NET_POLL_FAILED ); + + ret = 0; + if( FD_ISSET( fd, &read_fds ) ) + ret |= MBEDTLS_NET_POLL_READ; + if( FD_ISSET( fd, &write_fds ) ) + ret |= MBEDTLS_NET_POLL_WRITE; + + return( ret ); +} + /* * Portable usleep helper */ @@ -492,8 +544,8 @@ int mbedtls_net_recv( void *ctx, unsigned char *buf, size_t len ) /* * Read at most 'len' characters, blocking for at most 'timeout' ms */ -int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, size_t len, - uint32_t timeout ) +int mbedtls_net_recv_timeout( void *ctx, unsigned char *buf, + size_t len, uint32_t timeout ) { int ret; struct timeval tv; From 16970d29127332e109f928fbe9efecc0a118c8dc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 15:56:37 +0100 Subject: [PATCH 07/55] Add support for event-driven IO in ssl_client2 and ssl_server2 --- programs/ssl/ssl_client2.c | 302 +++++++++++++++++++++++++++++++------ programs/ssl/ssl_server2.c | 185 +++++++++++++++++++++-- 2 files changed, 429 insertions(+), 58 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 5032a9f3d..e82adaa7b 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -70,6 +70,7 @@ int main( void ) #define DFL_REQUEST_SIZE -1 #define DFL_DEBUG_LEVEL 0 #define DFL_NBIO 0 +#define DFL_EVENT 0 #define DFL_READ_TIMEOUT 0 #define DFL_MAX_RESEND 0 #define DFL_CA_FILE "" @@ -243,23 +244,25 @@ int main( void ) " server_port=%%d default: 4433\n" \ " request_page=%%s default: \".\"\n" \ " request_size=%%d default: about 34 (basic request)\n" \ - " (minimum: 0, max: 16384)\n" \ - " debug_level=%%d default: 0 (disabled)\n" \ - " nbio=%%d default: 0 (blocking I/O)\n" \ - " options: 1 (non-blocking), 2 (added delays)\n" \ - " read_timeout=%%d default: 0 ms (no timeout)\n" \ + " (minimum: 0, max: 16384)\n" \ + " debug_level=%%d default: 0 (disabled)\n" \ + " nbio=%%d default: 0 (blocking I/O)\n" \ + " options: 1 (non-blocking), 2 (added delays)\n" \ + " event=%%d default: 0 (loop)\n" \ + " options: 1 (level-triggered, implies nbio=1),\n" \ + " read_timeout=%%d default: 0 ms (no timeout)\n" \ " max_resend=%%d default: 0 (no resend on timeout)\n" \ "\n" \ USAGE_DTLS \ "\n" \ - " auth_mode=%%s default: (library default: none)\n" \ + " auth_mode=%%s default: (library default: none)\n" \ " options: none, optional, required\n" \ USAGE_IO \ "\n" \ USAGE_PSK \ USAGE_ECJPAKE \ "\n" \ - " allow_legacy=%%d default: (library default: no)\n" \ + " allow_legacy=%%d default: (library default: no)\n" \ USAGE_RENEGO \ " exchanges=%%d default: 1\n" \ " reconnect=%%d default: 0 (disabled)\n" \ @@ -299,7 +302,8 @@ struct options const char *server_port; /* port on which the ssl service runs */ int debug_level; /* level of debugging */ int nbio; /* should I/O be blocking? */ - uint32_t read_timeout; /* timeout on mbedtls_ssl_read() in milliseconds */ + int event; /* loop or event-driven IO? level or edge triggered? */ + uint32_t read_timeout; /* timeout on mbedtls_ssl_read() in milliseconds */ int max_resend; /* DTLS times to resend on read timeout */ const char *request_page; /* page on server to request */ int request_size; /* pad request with header to requested size */ @@ -433,6 +437,78 @@ static int ssl_sig_hashes_for_test[] = { }; #endif /* MBEDTLS_X509_CRT_PARSE_C */ +/* + * Wait for an event from the underlying transport or the timer + * (Used in event-driven IO mode). + */ +#if !defined(MBEDTLS_TIMING_C) +void idle( mbedtls_ssl_context *ssl, + mbedtls_net_context *fd, + int idle_reason ) +{ +#else +void idle( mbedtls_ssl_context *ssl, + mbedtls_net_context *fd, + mbedtls_timing_delay_context *timer, + int idle_reason ) +{ +#if defined(MBEDTLS_DEBUG_C) + struct mbedtls_timing_hr_time tm; + unsigned long time_elapsed; +#endif +#endif + + int poll_type = 0; + + if( idle_reason == MBEDTLS_ERR_SSL_WANT_WRITE ) + poll_type = MBEDTLS_NET_POLL_WRITE; + else if( idle_reason == MBEDTLS_ERR_SSL_WANT_READ ) + poll_type = MBEDTLS_NET_POLL_READ; +#if !defined(MBEDTLS_TIMING_C) + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: No reason for idling given" ) ); + return; + } +#endif + + /* One should not idle on the underlying transport + * if data is still pending to be processed. */ + if( mbedtls_ssl_check_pending( ssl ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, " + "but idling requested!" ) ); + } + MBEDTLS_SSL_DEBUG_MSG( 3, ( "idle, waiting for event... " ) ); + +#if defined(MBEDTLS_TIMING_C) && defined(MBEDTLS_DEBUG_C) + mbedtls_timing_get_timer( &tm, 1 /* restart */ ); +#endif + + while( 1 ) + { +#if defined(MBEDTLS_TIMING_C) +#if defined(MBEDTLS_DEBUG_C) + time_elapsed = mbedtls_timing_get_timer( &tm, 0 ); +#endif + if( mbedtls_timing_get_delay( timer ) == 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] timer expired - continue", + time_elapsed ) ); + break; + } +#endif + + if( poll_type != 0 && + mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - " + "continue", time_elapsed ) ); + break; + } + } +} + int main( int argc, char *argv[] ) { int ret = 0, len, tail_len, i, written, frags, retry_left; @@ -516,6 +592,7 @@ int main( int argc, char *argv[] ) opt.server_port = DFL_SERVER_PORT; opt.debug_level = DFL_DEBUG_LEVEL; opt.nbio = DFL_NBIO; + opt.event = DFL_EVENT; opt.read_timeout = DFL_READ_TIMEOUT; opt.max_resend = DFL_MAX_RESEND; opt.request_page = DFL_REQUEST_PAGE; @@ -589,6 +666,12 @@ int main( int argc, char *argv[] ) if( opt.nbio < 0 || opt.nbio > 2 ) goto usage; } + else if( strcmp( p, "event" ) == 0 ) + { + opt.event = atoi( q ); + if( opt.event < 0 || opt.event > 2 ) + goto usage; + } else if( strcmp( p, "read_timeout" ) == 0 ) opt.read_timeout = atoi( q ); else if( strcmp( p, "max_resend" ) == 0 ) @@ -858,6 +941,16 @@ int main( int argc, char *argv[] ) goto usage; } + /* Event-driven IO is incompatible with the above custom + * receive and send functions, as the polling builds on + * refers to the underlying net_context. */ + if( opt.event == 1 && opt.nbio != 1 ) + { + mbedtls_printf( "Warning: event-driven IO mandates nbio=1" + " - overwrite\n" ); + opt.nbio = 1; + } + #if defined(MBEDTLS_DEBUG_C) mbedtls_debug_set_threshold( opt.debug_level ); #endif @@ -1092,7 +1185,8 @@ int main( int argc, char *argv[] ) #endif if( ret < 0 ) { - mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1115,7 +1209,8 @@ int main( int argc, char *argv[] ) else #endif #if defined(MBEDTLS_CERTS_C) - ret = mbedtls_x509_crt_parse( &clicert, (const unsigned char *) mbedtls_test_cli_crt, + ret = mbedtls_x509_crt_parse( &clicert, + (const unsigned char *) mbedtls_test_cli_crt, mbedtls_test_cli_crt_len ); #else { @@ -1125,7 +1220,8 @@ int main( int argc, char *argv[] ) #endif if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1138,7 +1234,8 @@ int main( int argc, char *argv[] ) else #endif #if defined(MBEDTLS_CERTS_C) - ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_cli_key, + ret = mbedtls_pk_parse_key( &pkey, + (const unsigned char *) mbedtls_test_cli_key, mbedtls_test_cli_key_len, NULL, 0 ); #else { @@ -1148,7 +1245,8 @@ int main( int argc, char *argv[] ) #endif if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_pk_parse_key " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1166,11 +1264,13 @@ int main( int argc, char *argv[] ) opt.server_addr, opt.server_port ); fflush( stdout ); - if( ( ret = mbedtls_net_connect( &server_fd, opt.server_addr, opt.server_port, - opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ? - MBEDTLS_NET_PROTO_TCP : MBEDTLS_NET_PROTO_UDP ) ) != 0 ) + if( ( ret = mbedtls_net_connect( &server_fd, + opt.server_addr, opt.server_port, + opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ? + MBEDTLS_NET_PROTO_TCP : MBEDTLS_NET_PROTO_UDP ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_net_connect returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_net_connect " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1180,7 +1280,8 @@ int main( int argc, char *argv[] ) ret = mbedtls_net_set_block( &server_fd ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! net_set_(non)block() returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! net_set_(non)block() " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1197,7 +1298,8 @@ int main( int argc, char *argv[] ) opt.transport, MBEDTLS_SSL_PRESET_DEFAULT ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_config_defaults returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_config_defaults " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1220,13 +1322,15 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( opt.hs_to_min != DFL_HS_TO_MIN || opt.hs_to_max != DFL_HS_TO_MAX ) - mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, opt.hs_to_max ); + mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, + opt.hs_to_max ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) if( ( ret = mbedtls_ssl_conf_max_frag_len( &conf, opt.mfl_code ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_max_frag_len returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_max_frag_len " + "returned %d\n\n", ret ); goto exit; } #endif @@ -1249,8 +1353,8 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_CBC_RECORD_SPLITTING) if( opt.recsplit != DFL_RECSPLIT ) mbedtls_ssl_conf_cbc_record_splitting( &conf, opt.recsplit - ? MBEDTLS_SSL_CBC_RECORD_SPLITTING_ENABLED - : MBEDTLS_SSL_CBC_RECORD_SPLITTING_DISABLED ); + ? MBEDTLS_SSL_CBC_RECORD_SPLITTING_ENABLED + : MBEDTLS_SSL_CBC_RECORD_SPLITTING_DISABLED ); #endif #if defined(MBEDTLS_DHM_C) @@ -1262,7 +1366,8 @@ int main( int argc, char *argv[] ) if( opt.alpn_string != NULL ) if( ( ret = mbedtls_ssl_conf_alpn_protocols( &conf, alpn_list ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_alpn_protocols returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_alpn_protocols " + "returned %d\n\n", ret ); goto exit; } #endif @@ -1301,7 +1406,8 @@ int main( int argc, char *argv[] ) { if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &clicert, &pkey ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_own_cert returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_own_cert " + "returned %d\n\n", ret ); goto exit; } } @@ -1320,16 +1426,19 @@ int main( int argc, char *argv[] ) (const unsigned char *) opt.psk_identity, strlen( opt.psk_identity ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_psk returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_psk " + "returned %d\n\n", ret ); goto exit; } #endif if( opt.min_version != DFL_MIN_VERSION ) - mbedtls_ssl_conf_min_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.min_version ); + mbedtls_ssl_conf_min_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, + opt.min_version ); if( opt.max_version != DFL_MAX_VERSION ) - mbedtls_ssl_conf_max_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.max_version ); + mbedtls_ssl_conf_max_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, + opt.max_version ); #if defined(MBEDTLS_SSL_FALLBACK_SCSV) if( opt.fallback != DFL_FALLBACK ) @@ -1338,14 +1447,16 @@ int main( int argc, char *argv[] ) if( ( ret = mbedtls_ssl_setup( &ssl, &conf ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_setup returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_setup " + "returned -0x%x\n\n", -ret ); goto exit; } #if defined(MBEDTLS_X509_CRT_PARSE_C) if( ( ret = mbedtls_ssl_set_hostname( &ssl, opt.server_name ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_set_hostname returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_set_hostname " + "returned %d\n\n", ret ); goto exit; } #endif @@ -1357,7 +1468,8 @@ int main( int argc, char *argv[] ) (const unsigned char *) opt.ecjpake_pw, strlen( opt.ecjpake_pw ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_set_hs_ecjpake_password returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_set_hs_ecjpake_password " + "returned %d\n\n", ret ); goto exit; } } @@ -1366,7 +1478,8 @@ int main( int argc, char *argv[] ) if( opt.nbio == 2 ) mbedtls_ssl_set_bio( &ssl, &server_fd, my_send, my_recv, NULL ); else - mbedtls_ssl_set_bio( &ssl, &server_fd, mbedtls_net_send, mbedtls_net_recv, + mbedtls_ssl_set_bio( &ssl, &server_fd, + mbedtls_net_send, mbedtls_net_recv, opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); #if defined(MBEDTLS_TIMING_C) @@ -1384,9 +1497,11 @@ int main( int argc, char *argv[] ) while( ( ret = mbedtls_ssl_handshake( &ssl ) ) != 0 ) { - if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_handshake " + "returned -0x%x\n", -ret ); if( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ) mbedtls_printf( " Unable to verify the server's certificate. " @@ -1398,10 +1513,21 @@ int main( int argc, char *argv[] ) mbedtls_printf( "\n" ); goto exit; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } } mbedtls_printf( " ok\n [ Protocol is %s ]\n [ Ciphersuite is %s ]\n", - mbedtls_ssl_get_version( &ssl ), mbedtls_ssl_get_ciphersuite( &ssl ) ); + mbedtls_ssl_get_version( &ssl ), + mbedtls_ssl_get_ciphersuite( &ssl ) ); if( ( ret = mbedtls_ssl_get_record_expansion( &ssl ) ) >= 0 ) mbedtls_printf( " [ Record expansion is %d ]\n", ret ); @@ -1429,7 +1555,8 @@ int main( int argc, char *argv[] ) if( ( ret = mbedtls_ssl_get_session( &ssl, &saved_session ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_get_session returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_get_session " + "returned -0x%x\n\n", -ret ); goto exit; } @@ -1448,7 +1575,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( " failed\n" ); - mbedtls_x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", flags ); + mbedtls_x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), + " ! ", flags ); mbedtls_printf( "%s\n", vrfy_buf ); } @@ -1478,9 +1606,21 @@ int main( int argc, char *argv[] ) if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_renegotiate returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_renegotiate " + "returned %d\n\n", ret ); goto exit; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } + } mbedtls_printf( " ok\n" ); } @@ -1524,27 +1664,54 @@ send_request: { for( written = 0, frags = 0; written < len; written += ret, frags++ ) { - while( ( ret = mbedtls_ssl_write( &ssl, buf + written, len - written ) ) - <= 0 ) + while( ( ret = mbedtls_ssl_write( &ssl, buf + written, + len - written ) ) <= 0 ) { if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_write returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_write " + "returned -0x%x\n\n", -ret ); goto exit; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } } } } else /* Not stream, so datagram */ { - do ret = mbedtls_ssl_write( &ssl, buf, len ); - while( ret == MBEDTLS_ERR_SSL_WANT_READ || - ret == MBEDTLS_ERR_SSL_WANT_WRITE ); + while( 1 ) + { + ret = mbedtls_ssl_write( &ssl, buf, len ); + + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + break; + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } + } if( ret < 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_write returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_write " + "returned %d\n\n", ret ); goto exit; } @@ -1553,7 +1720,8 @@ send_request: } buf[written] = '\0'; - mbedtls_printf( " %d bytes written in %d fragments\n\n%s\n", written, frags, (char *) buf ); + mbedtls_printf( " %d bytes written in %d fragments\n\n%s\n", + written, frags, (char *) buf ); /* * 7. Read the HTTP response @@ -1574,7 +1742,18 @@ send_request: if( ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE ) + { + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } continue; + } if( ret <= 0 ) { @@ -1616,9 +1795,24 @@ send_request: len = sizeof( buf ) - 1; memset( buf, 0, sizeof( buf ) ); - do ret = mbedtls_ssl_read( &ssl, buf, len ); - while( ret == MBEDTLS_ERR_SSL_WANT_READ || - ret == MBEDTLS_ERR_SSL_WANT_WRITE ); + while( 1 ) + { + ret = mbedtls_ssl_read( &ssl, buf, len ); + + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + break; + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } + } if( ret <= 0 ) { @@ -1671,6 +1865,16 @@ send_request: mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n\n", -ret ); goto exit; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &server_fd, &timer, ret ); +#else + idle( &ssl, &server_fd, ret ); +#endif + } } mbedtls_printf( " ok\n" ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a25886824..b317bcca3 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -101,6 +101,7 @@ int main( void ) #define DFL_SERVER_PORT "4433" #define DFL_DEBUG_LEVEL 0 #define DFL_NBIO 0 +#define DFL_EVENT 0 #define DFL_READ_TIMEOUT 0 #define DFL_CA_FILE "" #define DFL_CA_PATH "" @@ -331,6 +332,8 @@ int main( void ) " debug_level=%%d default: 0 (disabled)\n" \ " nbio=%%d default: 0 (blocking I/O)\n" \ " options: 1 (non-blocking), 2 (added delays)\n" \ + " event=%%d default: 0 (loop)\n" \ + " options: 1 (level-triggered, implies nbio=1),\n" \ " read_timeout=%%d default: 0 ms (no timeout)\n" \ "\n" \ USAGE_DTLS \ @@ -399,6 +402,7 @@ struct options const char *server_port; /* port on which the ssl service runs */ int debug_level; /* level of debugging */ int nbio; /* should I/O be blocking? */ + int event; /* loop or event-driven IO? level or edge triggered? */ uint32_t read_timeout; /* timeout on mbedtls_ssl_read() in milliseconds */ const char *ca_file; /* the file with the CA certificate(s) */ const char *ca_path; /* the path with the CA certificate(s) reside */ @@ -837,6 +841,78 @@ static int ssl_sig_hashes_for_test[] = { }; #endif /* MBEDTLS_X509_CRT_PARSE_C */ +/* + * Wait for an event from the underlying transport or the timer + * (Used in event-driven IO mode). + */ +#if !defined(MBEDTLS_TIMING_C) +void idle( mbedtls_ssl_context *ssl, + mbedtls_net_context *fd, + int idle_reason ) +{ +#else +void idle( mbedtls_ssl_context *ssl, + mbedtls_net_context *fd, + mbedtls_timing_delay_context *timer, + int idle_reason ) +{ +#if defined(MBEDTLS_DEBUG_C) + struct mbedtls_timing_hr_time tm; + unsigned long time_elapsed; +#endif +#endif + + int poll_type = 0; + + if( idle_reason == MBEDTLS_ERR_SSL_WANT_WRITE ) + poll_type = MBEDTLS_NET_POLL_WRITE; + else if( idle_reason == MBEDTLS_ERR_SSL_WANT_READ ) + poll_type = MBEDTLS_NET_POLL_READ; +#if !defined(MBEDTLS_TIMING_C) + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: No reason for idling given" ) ); + return; + } +#endif + + /* One should not idle on the underlying transport + * if data is still pending to be processed. */ + if( mbedtls_ssl_check_pending( ssl ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, " + "but idling requested!" ) ); + } + MBEDTLS_SSL_DEBUG_MSG( 3, ( "idle, waiting for event... " ) ); + +#if defined(MBEDTLS_TIMING_C) && defined(MBEDTLS_DEBUG_C) + mbedtls_timing_get_timer( &tm, 1 /* restart */ ); +#endif + + while( 1 ) + { +#if defined(MBEDTLS_TIMING_C) +#if defined(MBEDTLS_DEBUG_C) + time_elapsed = mbedtls_timing_get_timer( &tm, 0 ); +#endif + if( mbedtls_timing_get_delay( timer ) == 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] timer expired - continue", + time_elapsed ) ); + break; + } +#endif + + if( poll_type != 0 && + mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - " + "continue", time_elapsed ) ); + break; + } + } +} + int main( int argc, char *argv[] ) { int ret = 0, len, written, frags, exchanges_left; @@ -969,6 +1045,7 @@ int main( int argc, char *argv[] ) opt.server_addr = DFL_SERVER_ADDR; opt.server_port = DFL_SERVER_PORT; opt.debug_level = DFL_DEBUG_LEVEL; + opt.event = DFL_EVENT; opt.nbio = DFL_NBIO; opt.read_timeout = DFL_READ_TIMEOUT; opt.ca_file = DFL_CA_FILE; @@ -1047,6 +1124,12 @@ int main( int argc, char *argv[] ) if( opt.nbio < 0 || opt.nbio > 2 ) goto usage; } + else if( strcmp( p, "event" ) == 0 ) + { + opt.event = atoi( q ); + if( opt.event < 0 || opt.event > 2 ) + goto usage; + } else if( strcmp( p, "read_timeout" ) == 0 ) opt.read_timeout = atoi( q ); else if( strcmp( p, "ca_file" ) == 0 ) @@ -1328,6 +1411,16 @@ int main( int argc, char *argv[] ) goto usage; } + /* Event-driven IO is incompatible with the above custom + * receive and send functions, as the polling builds on + * refers to the underlying net_context. */ + if( opt.event == 1 && opt.nbio != 1 ) + { + mbedtls_printf( "Warning: event-driven IO mandates nbio=1" + " - overwrite\n" ); + opt.nbio = 1; + } + #if defined(MBEDTLS_DEBUG_C) mbedtls_debug_set_threshold( opt.debug_level ); #endif @@ -2113,9 +2206,22 @@ handshake: mbedtls_printf( " . Performing the SSL/TLS handshake..." ); fflush( stdout ); - do ret = mbedtls_ssl_handshake( &ssl ); - while( ret == MBEDTLS_ERR_SSL_WANT_READ || - ret == MBEDTLS_ERR_SSL_WANT_WRITE ); + while( ( ret = mbedtls_ssl_handshake( &ssl ) ) != 0 ) + { + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + break; + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } + } if( ret == MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED ) { @@ -2221,7 +2327,18 @@ data_exchange: if( ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE ) + { + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } + continue; + } if( ret <= 0 ) { @@ -2309,9 +2426,24 @@ data_exchange: len = sizeof( buf ) - 1; memset( buf, 0, sizeof( buf ) ); - do ret = mbedtls_ssl_read( &ssl, buf, len ); - while( ret == MBEDTLS_ERR_SSL_WANT_READ || - ret == MBEDTLS_ERR_SSL_WANT_WRITE ); + while( 1 ) + { + ret = mbedtls_ssl_read( &ssl, buf, len ); + + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + break; + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } + } if( ret <= 0 ) { @@ -2352,6 +2484,16 @@ data_exchange: mbedtls_printf( " failed\n ! mbedtls_ssl_renegotiate returned %d\n\n", ret ); goto reset; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } } mbedtls_printf( " ok\n" ); @@ -2386,14 +2528,39 @@ data_exchange: mbedtls_printf( " failed\n ! mbedtls_ssl_write returned %d\n\n", ret ); goto reset; } + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } } } } else /* Not stream, so datagram */ { - do ret = mbedtls_ssl_write( &ssl, buf, len ); - while( ret == MBEDTLS_ERR_SSL_WANT_READ || - ret == MBEDTLS_ERR_SSL_WANT_WRITE ); + while( 1 ) + { + ret = mbedtls_ssl_write( &ssl, buf, len ); + + if( ret != MBEDTLS_ERR_SSL_WANT_READ && + ret != MBEDTLS_ERR_SSL_WANT_WRITE ) + break; + + /* For event-driven IO, wait for socket to become available */ + if( opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &ssl, &client_fd, &timer, ret ); +#else + idle( &ssl, &client_fd, ret ); +#endif + } + } if( ret < 0 ) { From 8b170a0a0b3b1f2b0cf27e572c0125ab3123e04d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 11:51:19 +0100 Subject: [PATCH 08/55] Enhance and extend checking of message processing state - Enhances the documentation of mbedtls_ssl_get_bytes_avail (return the number of bytes left in the current application data record, if there is any). - Introduces a new public function mbedtls_ssl_check_pending for checking whether any data in the internal buffers still needs to be processed. This is necessary for users implementing event-driven IO to decide when they can safely idle until they receive further events from the underlying transport. --- include/mbedtls/ssl.h | 47 +++++++++++++++++++++++++++++++-- library/ssl_tls.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index cc0007006..8b82eff8f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2235,11 +2235,54 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_RENEGOTIATION */ /** - * \brief Return the number of data bytes available to read + * \brief Check if there is data already read from the + * underlying transport but not yet processed. * * \param ssl SSL context * - * \return how many bytes are available in the read buffer + * \return 0 if nothing's pending, 1 otherwise. + * + * \note This function is essential when using the library + * with event-driven I/O. The user should not idle + * (waiting for events from the underlying transport + * or from timers) before this function's check passes. + * Otherwise, it's possible to run into a deadlock + * (if processing the pending data involves essential + * communication with the peer) or to accumulate and + * potentially lose data. + * + * \note This is different in purpose and behaviour from + * \c mbedtls_ssl_get_bytes_avail in that it considers + * any kind of unprocessed data, not only unread + * application data. If \c mbedtls_ssl_get_bytes + * returns a non-zero value, this function will + * also signal pending data, but the converse does + * not hold. For example, in DTLS there might be + * further records waiting to be processed from + * the current underlying transport's datagram. + * + * \note If this function returns 0 (data pending), this + * does not imply that a subsequent call to + * \c mbedtls_ssl_read will provide any data; + * e.g., the unprocessed data might turn out + * to be an alert or a handshake message. + */ +int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ); + +/** + * \brief Return the number of application data bytes + * remaining to be read from the current record. + * + * \param ssl SSL context + * + * \return How many bytes are available in the application + * data record read buffer. + * + * \note When working over a datagram transport, this is + * useful to detect the current datagram's boundary + * in case \c mbedtls_ssl_read has written the maximal + * amount of data fitting into the input buffer. + * */ size_t mbedtls_ssl_get_bytes_avail( const mbedtls_ssl_context *ssl ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 759dca013..7e8476cc6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6392,6 +6392,67 @@ size_t mbedtls_ssl_get_bytes_avail( const mbedtls_ssl_context *ssl ) return( ssl->in_offt == NULL ? 0 : ssl->in_msglen ); } +int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) +{ + /* + * Case A: We're currently holding back + * a message for further processing. + */ + + if( ssl->keep_current_message == 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: record " + "held back for processing" ) ); + return( 1 ); + } + + /* + * Case B: Further records are pending in the current datagram. + */ + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ssl->in_left > ssl->next_record_offset ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more records " + "within current datagram" ) ); + return( 1 ); + } +#endif /* MBEDTLS_SSL_PROTO_DTLS */ + + /* + * Case C: A handshake message is being processed. + */ + + /* TODO This needs correction in the same way as + * read_record_layer, see IOTSSL-1414 */ + if( ssl->in_hslen > 0 && ssl->in_hslen < ssl->in_msglen ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more handshake " + "messages within current record" ) ); + return( 1 ); + } + + /* + * Case D: An application data message is being processed + */ + if( ssl->in_offt != NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: application data " + "record is being processed" ) ); + return( 1 ); + } + + /* + * In all other cases, the rest of the message can be dropped. + * As in ssl_read_record_layer, this needs to be adapted if + * we implement support for multiple alerts in single records. + */ + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: nothing pending" ) ); + return( 0 ); +} + uint32_t mbedtls_ssl_get_verify_result( const mbedtls_ssl_context *ssl ) { if( ssl->session != NULL ) From cadb5bbe3c332ee460e9a0f60e563cb1cf01d48d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 26 May 2017 13:56:10 +0100 Subject: [PATCH 09/55] Add slight delay before killing server in ssl-opt.sh for log output It seems that tests from ssl-opt.sh are sometimes failing because the server is killed before its output has been finalized. This commit adds a small delay in ssl-opt.sh before killing the server to prevent that. --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7f5510cce..821df212c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -443,6 +443,8 @@ run_test() { eval "$CLI_CMD" >> $CLI_OUT 2>&1 & wait_client_done + sleep 0.05 + # terminate the server (and the proxy) kill $SRV_PID wait $SRV_PID From d82d84664af1da94aef06febf860418f82f97358 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 29 May 2017 21:37:46 +0100 Subject: [PATCH 10/55] ssl-opt.sh: Kill server via KILL signal if TERM doesn't succeed --- tests/ssl-opt.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 821df212c..fbb689a0b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -447,10 +447,19 @@ run_test() { # terminate the server (and the proxy) kill $SRV_PID - wait $SRV_PID + sleep 0.01 + if kill -0 $SRV_PID >/dev/null 2>&1; then + kill -KILL $SRV_PID + wait $SRV_PID + fi + if [ -n "$PXY_CMD" ]; then kill $PXY_PID >/dev/null 2>&1 - wait $PXY_PID + sleep 0.01 + if kill -0 $PXY_PID >/dev/null 2>&1; then + kill -KILL $pXY_PID + wait $PXY_PID + fi fi # retry only on timeouts From 52c6dc64c675d24b1be65e06d43dd67dc111e762 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 26 May 2017 16:07:36 +0100 Subject: [PATCH 11/55] Correct length check for DTLS records from old epochs. DTLS records from previous epochs were incorrectly checked against the current epoch transform's minimal content length, leading to the rejection of entire datagrams. This commit fixed that and adapts two test cases accordingly. Internal reference: IOTSSL-1417 --- library/ssl_tls.c | 143 ++++++++++++++++++++++++---------------------- tests/ssl-opt.sh | 10 ++-- 2 files changed, 79 insertions(+), 74 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7e8476cc6..c6aac473c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3522,81 +3522,23 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } - /* Check length against bounds of the current transform and version */ - if( ssl->transform_in == NULL ) - { - if( ssl->in_msglen < 1 || - ssl->in_msglen > MBEDTLS_SSL_MAX_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } - } - else - { - if( ssl->in_msglen < ssl->transform_in->minlen ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } - -#if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && - ssl->in_msglen > ssl->transform_in->minlen + MBEDTLS_SSL_MAX_CONTENT_LEN ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ - defined(MBEDTLS_SSL_PROTO_TLS1_2) - /* - * TLS encrypted messages can have up to 256 bytes of padding - */ - if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 && - ssl->in_msglen > ssl->transform_in->minlen + - MBEDTLS_SSL_MAX_CONTENT_LEN + 256 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); - return( MBEDTLS_ERR_SSL_INVALID_RECORD ); - } -#endif - } - /* - * DTLS-related tests done last, because most of them may result in - * silently dropping the record (but not the whole datagram), and we only - * want to consider that after ensuring that the "basic" fields (type, - * version, length) are sane. + * DTLS-related tests. + * Check epoch before checking length constraint because + * the latter varies with the epoch. E.g., if a ChangeCipherSpec + * message gets duplicated before the corresponding Finished message, + * the second ChangeCipherSpec should be discarded because it belongs + * to an old epoch, but not because its length is shorter than + * the minimum record length for packets using the new record transform. + * Note that these two kinds of failures are handled differently, + * as an unexpected record is silently skipped but an invalid + * record leads to the entire datagram being dropped. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1]; - /* Drop unexpected ChangeCipherSpec messages */ - if( ssl->in_msgtype == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC && - ssl->state != MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC && - ssl->state != MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ChangeCipherSpec" ) ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); - } - - /* Drop unexpected ApplicationData records, - * except at the beginning of renegotiations */ - if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA && - ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER -#if defined(MBEDTLS_SSL_RENEGOTIATION) - && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS && - ssl->state == MBEDTLS_SSL_SERVER_HELLO ) -#endif - ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); - } - /* Check epoch (and sequence number) with DTLS */ if( rec_epoch != ssl->in_epoch ) { @@ -3636,9 +3578,74 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); } #endif + + /* Drop unexpected ChangeCipherSpec messages */ + if( ssl->in_msgtype == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC && + ssl->state != MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC && + ssl->state != MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ChangeCipherSpec" ) ); + return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); + } + + /* Drop unexpected ApplicationData records, + * except at the beginning of renegotiations */ + if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA && + ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER +#if defined(MBEDTLS_SSL_RENEGOTIATION) + && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS && + ssl->state == MBEDTLS_SSL_SERVER_HELLO ) +#endif + ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) ); + return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); + } } #endif /* MBEDTLS_SSL_PROTO_DTLS */ + + /* Check length against bounds of the current transform and version */ + if( ssl->transform_in == NULL ) + { + if( ssl->in_msglen < 1 || + ssl->in_msglen > MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } + } + else + { + if( ssl->in_msglen < ssl->transform_in->minlen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } + +#if defined(MBEDTLS_SSL_PROTO_SSL3) + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && + ssl->in_msglen > ssl->transform_in->minlen + MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ + defined(MBEDTLS_SSL_PROTO_TLS1_2) + /* + * TLS encrypted messages can have up to 256 bytes of padding + */ + if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 && + ssl->in_msglen > ssl->transform_in->minlen + + MBEDTLS_SSL_MAX_CONTENT_LEN + 256 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); + return( MBEDTLS_ERR_SSL_INVALID_RECORD ); + } +#endif + } + return( 0 ); } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index fbb689a0b..57d5e6053 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -3702,8 +3702,8 @@ run_test "DTLS proxy: duplicate every packet" \ 0 \ -c "replayed record" \ -s "replayed record" \ - -c "discarding invalid record" \ - -s "discarding invalid record" \ + -c "record from another epoch" \ + -s "record from another epoch" \ -S "resend" \ -s "Extra-header:" \ -c "HTTP/1.0 200 OK" @@ -3715,8 +3715,8 @@ run_test "DTLS proxy: duplicate every packet, server anti-replay off" \ 0 \ -c "replayed record" \ -S "replayed record" \ - -c "discarding invalid record" \ - -s "discarding invalid record" \ + -c "record from another epoch" \ + -s "record from another epoch" \ -c "resend" \ -s "resend" \ -s "Extra-header:" \ @@ -3777,8 +3777,6 @@ run_test "DTLS proxy: delay ChangeCipherSpec" \ 0 \ -c "record from another epoch" \ -s "record from another epoch" \ - -c "discarding invalid record" \ - -s "discarding invalid record" \ -s "Extra-header:" \ -c "HTTP/1.0 200 OK" From 4cb1f4d49cff999d0c853bc696ad7eea68888c35 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 15:59:57 +0100 Subject: [PATCH 12/55] Style corrections --- library/ssl_srv.c | 8 ++-- programs/ssl/ssl_client2.c | 94 ++++++++++++++++++++++++-------------- programs/ssl/ssl_server2.c | 41 +++++++++++------ 3 files changed, 90 insertions(+), 53 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f137c3dce..be961af71 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -785,7 +785,7 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, const mbedtls_ssl_ciphersuite_t *suite_info; #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ - defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) + defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) mbedtls_pk_type_t sig_type; #endif @@ -2955,7 +2955,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) return( ret ); } -#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) +#if defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) dig_signed = p; dig_signed_len = len; #endif @@ -3044,7 +3044,7 @@ curve_matching_done: /* * 3.1: Choose hash algorithm: - * A: For TLS 1.2, obey signature-hash-algorithm extension + * A: For TLS 1.2, obey signature-hash-algorithm extension * to choose appropriate hash. * B: For SSL3, TLS1.0, TLS1.1 and ECDHE_ECDSA, use SHA1 * (RFC 4492, Sec. 5.4) @@ -3065,7 +3065,7 @@ curve_matching_done: sig_alg ) ) == MBEDTLS_MD_NONE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - /* (... because we choose a cipher suite + /* (... because we choose a cipher suite * only if there is a matching hash.) */ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index e82adaa7b..5b82693ff 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -354,7 +354,8 @@ static void my_debug( void *ctx, int level, if( *p == '/' || *p == '\\' ) basename = p + 1; - mbedtls_fprintf( (FILE *) ctx, "%s:%04d: |%d| %s", basename, line, level, str ); + mbedtls_fprintf( (FILE *) ctx, "%s:%04d: |%d| %s", + basename, line, level, str ); fflush( (FILE *) ctx ); } @@ -400,7 +401,8 @@ static int my_send( void *ctx, const unsigned char *buf, size_t len ) /* * Enabled if debug_level > 1 in code below */ -static int my_verify( void *data, mbedtls_x509_crt *crt, int depth, uint32_t *flags ) +static int my_verify( void *data, mbedtls_x509_crt *crt, + int depth, uint32_t *flags ) { char buf[1024]; ((void) data); @@ -685,7 +687,8 @@ int main( int argc, char *argv[] ) else if( strcmp( p, "request_size" ) == 0 ) { opt.request_size = atoi( q ); - if( opt.request_size < 0 || opt.request_size > MBEDTLS_SSL_MAX_CONTENT_LEN ) + if( opt.request_size < 0 || + opt.request_size > MBEDTLS_SSL_MAX_CONTENT_LEN ) goto usage; } else if( strcmp( p, "ca_file" ) == 0 ) @@ -715,16 +718,23 @@ int main( int argc, char *argv[] ) } else if( strcmp( p, "renegotiation" ) == 0 ) { - opt.renegotiation = (atoi( q )) ? MBEDTLS_SSL_RENEGOTIATION_ENABLED : - MBEDTLS_SSL_RENEGOTIATION_DISABLED; + opt.renegotiation = (atoi( q )) ? + MBEDTLS_SSL_RENEGOTIATION_ENABLED : + MBEDTLS_SSL_RENEGOTIATION_DISABLED; } else if( strcmp( p, "allow_legacy" ) == 0 ) { switch( atoi( q ) ) { - case -1: opt.allow_legacy = MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE; break; - case 0: opt.allow_legacy = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; break; - case 1: opt.allow_legacy = MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION; break; + case -1: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE; + break; + case 0: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; + break; + case 1: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION; + break; default: goto usage; } } @@ -781,8 +791,12 @@ int main( int argc, char *argv[] ) { switch( atoi( q ) ) { - case 0: opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_DISABLED; break; - case 1: opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; break; + case 0: + opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_DISABLED; + break; + case 1: + opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + break; default: goto usage; } } @@ -958,19 +972,20 @@ int main( int argc, char *argv[] ) if( opt.force_ciphersuite[0] > 0 ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info; - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); + ciphersuite_info = + mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); if( opt.max_version != -1 && ciphersuite_info->min_minor_ver > opt.max_version ) { - mbedtls_printf("forced ciphersuite not allowed with this protocol version\n"); + mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } if( opt.min_version != -1 && ciphersuite_info->max_minor_ver < opt.min_version ) { - mbedtls_printf("forced ciphersuite not allowed with this protocol version\n"); + mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } @@ -996,7 +1011,7 @@ int main( int argc, char *argv[] ) { if( opt.arc4 == MBEDTLS_SSL_ARC4_DISABLED ) { - mbedtls_printf("forced RC4 ciphersuite with RC4 disabled\n"); + mbedtls_printf( "forced RC4 ciphersuite with RC4 disabled\n" ); ret = 2; goto usage; } @@ -1016,7 +1031,7 @@ int main( int argc, char *argv[] ) if( strlen( opt.psk ) % 2 != 0 ) { - mbedtls_printf("pre-shared key not valid hex\n"); + mbedtls_printf( "pre-shared key not valid hex\n" ); goto exit; } @@ -1033,7 +1048,7 @@ int main( int argc, char *argv[] ) c -= 'A' - 10; else { - mbedtls_printf("pre-shared key not valid hex\n"); + mbedtls_printf( "pre-shared key not valid hex\n" ); goto exit; } psk[ j / 2 ] = c << 4; @@ -1047,7 +1062,7 @@ int main( int argc, char *argv[] ) c -= 'A' - 10; else { - mbedtls_printf("pre-shared key not valid hex\n"); + mbedtls_printf( "pre-shared key not valid hex\n" ); goto exit; } psk[ j / 2 ] |= c; @@ -1138,11 +1153,12 @@ int main( int argc, char *argv[] ) fflush( stdout ); mbedtls_entropy_init( &entropy ); - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - strlen( pers ) ) ) != 0 ) + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, + &entropy, (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%x\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%x\n", + -ret ); goto exit; } @@ -1180,13 +1196,13 @@ int main( int argc, char *argv[] ) #else { ret = 1; - mbedtls_printf("MBEDTLS_CERTS_C not defined."); + mbedtls_printf( "MBEDTLS_CERTS_C not defined." ); } #endif if( ret < 0 ) { - mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", + -ret ); goto exit; } @@ -1771,7 +1787,8 @@ send_request: goto reconnect; default: - mbedtls_printf( " mbedtls_ssl_read returned -0x%x\n", -ret ); + mbedtls_printf( " mbedtls_ssl_read returned -0x%x\n", + -ret ); goto exit; } } @@ -1853,7 +1870,8 @@ send_request: if( ( ret = mbedtls_ssl_session_reset( &ssl ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_session_reset returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_session_reset returned -0x%x\n\n", + -ret ); goto exit; } @@ -1862,7 +1880,8 @@ send_request: if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n\n", + -ret ); goto exit; } @@ -1921,21 +1940,25 @@ reconnect: if( ( ret = mbedtls_ssl_session_reset( &ssl ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_session_reset returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_session_reset returned -0x%x\n\n", + -ret ); goto exit; } if( ( ret = mbedtls_ssl_set_session( &ssl, &saved_session ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_session returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_session returned %d\n\n", + ret ); goto exit; } - if( ( ret = mbedtls_net_connect( &server_fd, opt.server_addr, opt.server_port, - opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ? - MBEDTLS_NET_PROTO_TCP : MBEDTLS_NET_PROTO_UDP ) ) != 0 ) + if( ( ret = mbedtls_net_connect( &server_fd, + opt.server_addr, opt.server_port, + opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ? + MBEDTLS_NET_PROTO_TCP : MBEDTLS_NET_PROTO_UDP ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_net_connect returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_net_connect returned -0x%x\n\n", + -ret ); goto exit; } @@ -1946,7 +1969,7 @@ reconnect: if( ret != 0 ) { mbedtls_printf( " failed\n ! net_set_(non)block() returned -0x%x\n\n", - -ret ); + -ret ); goto exit; } @@ -1955,7 +1978,8 @@ reconnect: if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n\n", + -ret ); goto exit; } } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index b317bcca3..d16c53419 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1171,16 +1171,23 @@ int main( int argc, char *argv[] ) opt.version_suites = q; else if( strcmp( p, "renegotiation" ) == 0 ) { - opt.renegotiation = (atoi( q )) ? MBEDTLS_SSL_RENEGOTIATION_ENABLED : - MBEDTLS_SSL_RENEGOTIATION_DISABLED; + opt.renegotiation = (atoi( q )) ? + MBEDTLS_SSL_RENEGOTIATION_ENABLED : + MBEDTLS_SSL_RENEGOTIATION_DISABLED; } else if( strcmp( p, "allow_legacy" ) == 0 ) { switch( atoi( q ) ) { - case -1: opt.allow_legacy = MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE; break; - case 0: opt.allow_legacy = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; break; - case 1: opt.allow_legacy = MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION; break; + case -1: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE; + break; + case 0: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; + break; + case 1: + opt.allow_legacy = MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION; + break; default: goto usage; } } @@ -1337,8 +1344,12 @@ int main( int argc, char *argv[] ) { switch( atoi( q ) ) { - case 0: opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_DISABLED; break; - case 1: opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; break; + case 0: + opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_DISABLED; + break; + case 1: + opt.extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + break; default: goto usage; } } @@ -1428,19 +1439,20 @@ int main( int argc, char *argv[] ) if( opt.force_ciphersuite[0] > 0 ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info; - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); + ciphersuite_info = + mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); if( opt.max_version != -1 && ciphersuite_info->min_minor_ver > opt.max_version ) { - mbedtls_printf("forced ciphersuite not allowed with this protocol version\n"); + mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } if( opt.min_version != -1 && ciphersuite_info->max_minor_ver < opt.min_version ) { - mbedtls_printf("forced ciphersuite not allowed with this protocol version\n"); + mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } @@ -1619,11 +1631,12 @@ int main( int argc, char *argv[] ) fflush( stdout ); mbedtls_entropy_init( &entropy ); - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - strlen( pers ) ) ) != 0 ) + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, + &entropy, (const unsigned char *) pers, + strlen( pers ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%x\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned -0x%x\n", + -ret ); goto exit; } From 8ec8102c9a6356b420aebf5074d042902b060a2c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 10:35:08 +0100 Subject: [PATCH 13/55] Split WANT_READ in two error codes This commit restricts WANT_READ to indicate that no data is available on the underlying transport. To signal the need for further processing - which was previously also handled through this error code - a new internal error code MBEDTLS_ERR_SSL_CONTINUE_PROCESSING is introduced. --- include/mbedtls/ssl.h | 63 ++++++++++++++++++++++++++++++++++--------- library/error.c | 4 ++- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8b82eff8f..e811bb907 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -102,13 +102,14 @@ #define MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED -0x6A80 /**< DTLS client must retry for hello verification */ #define MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL -0x6A00 /**< A buffer is too small to receive or write a message */ #define MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE -0x6980 /**< None of the common ciphersuites is usable (eg, no suitable certificate, see debug messages). */ -#define MBEDTLS_ERR_SSL_WANT_READ -0x6900 /**< Connection requires a read call. */ +#define MBEDTLS_ERR_SSL_WANT_READ -0x6900 /**< No data of requested type currently available on underlying transport. */ #define MBEDTLS_ERR_SSL_WANT_WRITE -0x6880 /**< Connection requires a write call. */ #define MBEDTLS_ERR_SSL_TIMEOUT -0x6800 /**< The operation timed out. */ #define MBEDTLS_ERR_SSL_CLIENT_RECONNECT -0x6780 /**< The client initiated a reconnect from the same port. */ #define MBEDTLS_ERR_SSL_UNEXPECTED_RECORD -0x6700 /**< Record header looks valid but is not expected. */ #define MBEDTLS_ERR_SSL_NON_FATAL -0x6680 /**< The alert message received indicates a non-fatal error. */ #define MBEDTLS_ERR_SSL_INVALID_VERIFY_HASH -0x6600 /**< Couldn't set the hash for verifying CertificateVerify */ +#define MBEDTLS_ERR_SSL_CONTINUE_PROCESSING -0x6580 /**< Internal-only message signaling that further message-processing should be done */ /* * Various constants @@ -2397,6 +2398,19 @@ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session * MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED (see below), or * a specific SSL error code. * + * If MBEDTLS_ERR_SSL_WANT_READ is returned, the handshake is + * unfinished and no further data is available from the underlying + * transport. In this case, the function needs to be called again + * at some later stage. + * + * \note Remarks regarding event-driven DTLS: + * If the function returns MBEDTLS_ERR_SSL_WANT_READ, no datagram + * from the underlying transport layer is currently being processed, + * and it is safe to idle until the timer or the underlying transport + * signal a new event. This is not true for a successful handshake, + * in which case the currently processed underlying transport's datagram + * might or might not contain further DTLS records. + * * \note If this function returns something other than 0 or * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context * becomes unusable, and you should either free it or call @@ -2460,20 +2474,20 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * \param buf buffer that will hold the data * \param len maximum number of bytes to read * - * \return the number of bytes read, or - * 0 for EOF, or - * MBEDTLS_ERR_SSL_WANT_READ or MBEDTLS_ERR_SSL_WANT_WRITE, or - * MBEDTLS_ERR_SSL_CLIENT_RECONNECT (see below), or - * another negative error code. + * \return One of the following: + * - 0 for EOF, or + * - the (positive) number of bytes read, or + * - a negative error code on failure. * - * \note If this function returns something other than a positive - * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE or - * MBEDTLS_ERR_SSL_CLIENT_RECONNECT, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * If MBEDTLS_ERR_SSL_WANT_READ is returned, no application data + * is available from the underlying transport. In this case, + * the function needs to be called again at some later stage. * - * \note When this function return MBEDTLS_ERR_SSL_CLIENT_RECONNECT + * If MBEDTLS_ERR_SSL_WANT_WRITE is returned, a write is pending + * but the underlying transport isn't available for writing. In this + * case, the function needs to be called again at some later stage. + * + * When this function return MBEDTLS_ERR_SSL_CLIENT_RECONNECT * (which can only happen server-side), it means that a client * is initiating a new connection using the same source port. * You can either treat that as a connection close and wait @@ -2486,6 +2500,29 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * again. WARNING: not validating the identity of the client * again, or not transmitting the new identity to the * application layer, would allow authentication bypass! + * + * If this function returns something other than a positive + * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE or + * MBEDTLS_ERR_SSL_CLIENT_RECONNECT, then the ssl context + * becomes unusable, and you should either free it or call + * \c mbedtls_ssl_session_reset() on it before re-using it for + * a new connection; the current connection must be closed. + * + * \note Remarks regarding event-driven DTLS: + * - If the function returns MBEDTLS_ERR_SSL_WANT_READ, no datagram + * from the underlying transport layer is currently being processed, + * and it is safe to idle until the timer or the underlying transport + * signal a new event. + * - If the function returns MBEDTLS_ERR_SSL_WANT_READ this does not mean + * that no data was available from the underlying transport in the first place, + * as there might have been delayed or duplicated messages, or a renegotiation + * request from the peer. Therefore, the user must be prepared to receive + * MBEDTLS_ERR_SSL_WANT_READ even when reacting to an incoming-data event + * from the underlying transport. + * - On success, the currently processed underlying transport's datagram + * might or might not contain further DTLS records, and the user should + * consult \c mbedtls_ssl_check_pending in that regard. + * */ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ); diff --git a/library/error.c b/library/error.c index 8977cc4e5..c42642467 100644 --- a/library/error.c +++ b/library/error.c @@ -426,7 +426,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) if( use_ret == -(MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE) ) mbedtls_snprintf( buf, buflen, "SSL - None of the common ciphersuites is usable (eg, no suitable certificate, see debug messages)" ); if( use_ret == -(MBEDTLS_ERR_SSL_WANT_READ) ) - mbedtls_snprintf( buf, buflen, "SSL - Connection requires a read call" ); + mbedtls_snprintf( buf, buflen, "SSL - No data of requested type currently available on underlying transport" ); if( use_ret == -(MBEDTLS_ERR_SSL_WANT_WRITE) ) mbedtls_snprintf( buf, buflen, "SSL - Connection requires a write call" ); if( use_ret == -(MBEDTLS_ERR_SSL_TIMEOUT) ) @@ -439,6 +439,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "SSL - The alert message received indicates a non-fatal error" ); if( use_ret == -(MBEDTLS_ERR_SSL_INVALID_VERIFY_HASH) ) mbedtls_snprintf( buf, buflen, "SSL - Couldn't set the hash for verifying CertificateVerify" ); + if( use_ret == -(MBEDTLS_ERR_SSL_CONTINUE_PROCESSING) ) + mbedtls_snprintf( buf, buflen, "SSL - Internal-only message signalling that further message-processing should be done" ); #endif /* MBEDTLS_SSL_TLS_C */ #if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C) From 90333dab855c8f5f5fa02149e23b95183253650e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 10 Oct 2017 11:27:13 +0100 Subject: [PATCH 14/55] Replace wrong usage of WANT_READ by CONTINUE_PROCESSING --- library/ssl_srv.c | 8 ++++-- library/ssl_tls.c | 67 +++++++++++++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index be961af71..c52aa4737 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3790,7 +3790,10 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) /* Read the message without adding it to the checksum */ do { - if( ( ret = mbedtls_ssl_read_record_layer( ssl ) ) != 0 ) + do ret = mbedtls_ssl_read_record_layer( ssl ); + while( ret == MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); + + if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_read_record_layer" ), ret ); return( ret ); @@ -3798,7 +3801,8 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) ret = mbedtls_ssl_handle_message_type( ssl ); - } while( MBEDTLS_ERR_SSL_NON_FATAL == ret ); + } while( MBEDTLS_ERR_SSL_NON_FATAL == ret || + MBEDTLS_ERR_SSL_CONTINUE_PROCESSING == ret ); if( 0 != ret ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c6aac473c..e2df82242 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3020,7 +3020,7 @@ static int ssl_reassemble_dtls_handshake( mbedtls_ssl_context *ssl ) if( ssl_bitmask_check( bitmask, msg_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "message is not complete yet" ) ); - return( MBEDTLS_ERR_SSL_WANT_READ ); + return( MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); } MBEDTLS_SSL_DEBUG_MSG( 2, ( "handshake message completed" ) ); @@ -3126,7 +3126,7 @@ int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ) ssl->handshake->in_msg_seq ) ); } - return( MBEDTLS_ERR_SSL_WANT_READ ); + return( MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); } /* Wait until message completion to increment in_msg_seq */ @@ -3734,7 +3734,10 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl ) { do { - if( ( ret = mbedtls_ssl_read_record_layer( ssl ) ) != 0 ) + do ret = mbedtls_ssl_read_record_layer( ssl ); + while( ret == MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); + + if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_read_record_layer" ), ret ); return( ret ); @@ -3742,7 +3745,8 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl ) ret = mbedtls_ssl_handle_message_type( ssl ); - } while( MBEDTLS_ERR_SSL_NON_FATAL == ret ); + } while( MBEDTLS_ERR_SSL_NON_FATAL == ret || + MBEDTLS_ERR_SSL_CONTINUE_PROCESSING == ret ); if( 0 != ret ) { @@ -3872,12 +3876,6 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) return( 0 ); } - /* Need to fetch a new record */ - -#if defined(MBEDTLS_SSL_PROTO_DTLS) -read_record_header: -#endif - /* Current record either fully processed or to be discarded. */ if( ( ret = mbedtls_ssl_fetch_input( ssl, mbedtls_ssl_hdr_len( ssl ) ) ) != 0 ) @@ -3912,7 +3910,7 @@ read_record_header: } /* Get next record */ - goto read_record_header; + return( MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); } #endif return( ret ); @@ -3984,7 +3982,7 @@ read_record_header: ssl->in_left = 0; MBEDTLS_SSL_DEBUG_MSG( 1, ( "discarding invalid record (mac)" ) ); - goto read_record_header; + return( MBEDTLS_ERR_SSL_CONTINUE_PROCESSING ); } return( ret ); @@ -4089,7 +4087,7 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) if( ssl->in_msg[0] == MBEDTLS_SSL_ALERT_LEVEL_WARNING && ssl->in_msg[1] == MBEDTLS_SSL_ALERT_MSG_NO_RENEGOTIATION ) { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "is a SSLv3 no_cert" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "is a SSLv3 no renegotiation alert" ) ); /* Will be handled when trying to parse ServerHello */ return( 0 ); } @@ -6868,25 +6866,16 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) } /* - * TODO - * - * The logic should be streamlined here: - * - * Instead of - * + * The logic could be streamlined here. Instead of * - Manually checking whether ssl->in_offt is NULL * - Fetching a new record if yes * - Setting ssl->in_offt if one finds an application record * - Resetting keep_current_message after handling the application data - * * one should - * * - Adapt read_record to set ssl->in_offt automatically * when a new application data record is processed. * - Always call mbedtls_ssl_read_record here. - * * This way, the logic of ssl_read would be much clearer: - * * (1) Always call record layer and see what kind of record is on * and have it ready for consumption (in particular, in_offt * properly set for application data records). @@ -6896,13 +6885,11 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) * (3) If it's something different from application data, * handle it accordingly, e.g. potentially start a * renegotiation. - * * This will also remove the need to manually reset * ssl->keep_current_message = 0 below. - * */ - if( ssl->in_offt == NULL ) + while( ssl->in_offt == NULL ) { /* Start timer if not already running */ if( ssl->f_get_timer != NULL && @@ -6957,7 +6944,9 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) /* With DTLS, drop the packet (probably from last handshake) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - return( MBEDTLS_ERR_SSL_WANT_READ ); + { + continue; + } #endif return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -6972,7 +6961,9 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) /* With DTLS, drop the packet (probably from last handshake) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - return( MBEDTLS_ERR_SSL_WANT_READ ); + { + continue; + } #endif return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -7044,7 +7035,25 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) } } - return( MBEDTLS_ERR_SSL_WANT_READ ); + /* At this point, we don't know whether the renegotiation has been + * completed or not. The cases to consider are the following: + * 1) The renegotiation is complete. In this case, no new record + * has been read yet. + * 2) The renegotiation is incomplete because the client received + * an application data record while awaiting the ServerHello. + * 3) The renegotiation is incomplete because the client received + * a non-handshake, non-application data message while awaiting + * the ServerHello. + * In each of these case, looping will be the proper action: + * - For 1), the next iteration will read a new record and check + * if it's application data. + * - For 2), the loop condition isn't satisfied as application data + * is present, hence continue is the same as break + * - For 3), the loop condition is satisfied and read_record + * will re-deliver the message that was held back by the client + * when expecting the ServerHello. + */ + continue; } else if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING ) { From c76c619dd08eba3210caa6d13a8cca43e3a697fa Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 6 Jun 2017 10:03:17 +0100 Subject: [PATCH 15/55] Reconcile resending of previous flights This commit reconciles the code path responsible for resending the final DTLS handshake flight with the path for handling resending of the other flights. --- library/ssl_tls.c | 55 +++++++++++------------------------------------ 1 file changed, 13 insertions(+), 42 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e2df82242..83d3c9698 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3097,9 +3097,11 @@ int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ) int ret; unsigned int recv_msg_seq = ( ssl->in_msg[4] << 8 ) | ssl->in_msg[5]; - /* ssl->handshake is NULL when receiving ClientHello for renego */ if( ssl->handshake != NULL && - recv_msg_seq != ssl->handshake->in_msg_seq ) + ( ( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER && + recv_msg_seq != ssl->handshake->in_msg_seq ) || + ( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER && + ssl->in_msg[0] != MBEDTLS_SSL_HS_CLIENT_HELLO ) ) ) { /* Retransmit only on last message from previous flight, to avoid * too many retransmissions. @@ -4003,46 +4005,6 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) } } - /* - * When we sent the last flight of the handshake, we MUST respond to a - * retransmit of the peer's previous flight with a retransmit. (In - * practice, only the Finished message will make it, other messages - * including CCS use the old transform so they're dropped as invalid.) - * - * If the record we received is not a handshake message, however, it - * means the peer received our last flight so we can clean up - * handshake info. - * - * This check needs to be done before prepare_handshake() due to an edge - * case: if the client immediately requests renegotiation, this - * finishes the current handshake first, avoiding the new ClientHello - * being mistaken for an ancient message in the current handshake. - */ -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && - ssl->handshake != NULL && - ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER ) - { - if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && - ssl->in_msg[0] == MBEDTLS_SSL_HS_FINISHED ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "received retransmit of last flight" ) ); - - if( ( ret = mbedtls_ssl_resend( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_resend", ret ); - return( ret ); - } - - return( MBEDTLS_ERR_SSL_WANT_READ ); - } - else - { - ssl_handshake_wrapup_free_hs_transform( ssl ); - } - } -#endif - return( 0 ); } @@ -4109,6 +4071,15 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) return MBEDTLS_ERR_SSL_NON_FATAL; } +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ssl->handshake != NULL && + ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER ) + { + ssl_handshake_wrapup_free_hs_transform( ssl ); + } +#endif + return( 0 ); } From 6ea44fabc543867aa76cf9901a0a2aa09ed68561 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 12 Oct 2017 07:46:10 +0100 Subject: [PATCH 16/55] Adapt ChangeLog: API extended by `net_poll` and `check_pending` --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index 227faed6b..50fefacb0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +API changes + * Add function mbedtls_net_poll to public API allowing to wait for a + network context to become ready for reading or writing. + * Add function mbedtls_ssl_check_pending to public API allowing to check + if more data is pending to be processed in the internal message buffers. + This function is necessary to determine when it is safe to idle on the + underlying transport in case event-driven IO is used. + = mbed TLS 2.6.0 branch released 2017-08-10 Security From c53826b459b174bc6be4d6a9f52fc8f528c1494f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 12 Oct 2017 07:46:41 +0100 Subject: [PATCH 17/55] Adapt ChangeLog: Usage restriction for WANT_READ --- ChangeLog | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ChangeLog b/ChangeLog index 50fefacb0..6b0fe3ba5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,15 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Bugfix + * Restrict usage of error code MBEDTLS_ERR_SSL_WANT_READ to situations + where data needs to be fetched from the underlying transport in order + to make progress. Previously, this error code was also occasionally + returned when unexpected messages were being discarded, ignoring that + further messages could potentially already be pending to be processed + in the internal buffers; these cases lead to deadlocks in case + event-driven I/O was used. Found by Hubert Mis. + API changes * Add function mbedtls_net_poll to public API allowing to wait for a network context to become ready for reading or writing. From ffb1e1ab3da9b235ccf8e28629aa143b6db36f38 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 13:17:42 +0100 Subject: [PATCH 18/55] Documentation improvements --- include/mbedtls/ssl.h | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index e811bb907..43ba67cd5 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2244,7 +2244,7 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf, * \return 0 if nothing's pending, 1 otherwise. * * \note This function is essential when using the library - * with event-driven I/O. The user should not idle + * with event-driven I/O. You should not idle * (waiting for events from the underlying transport * or from timers) before this function's check passes. * Otherwise, it's possible to run into a deadlock @@ -2398,18 +2398,19 @@ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session * MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED (see below), or * a specific SSL error code. * - * If MBEDTLS_ERR_SSL_WANT_READ is returned, the handshake is - * unfinished and no further data is available from the underlying - * transport. In this case, the function needs to be called again - * at some later stage. + * If this function returns MBEDTLS_ERR_SSL_WANT_READ, the + * handshake is unfinished and no further data is available + * from the underlying transport. In this case, you must call + * the function again at some later stage. * * \note Remarks regarding event-driven DTLS: * If the function returns MBEDTLS_ERR_SSL_WANT_READ, no datagram * from the underlying transport layer is currently being processed, * and it is safe to idle until the timer or the underlying transport * signal a new event. This is not true for a successful handshake, - * in which case the currently processed underlying transport's datagram - * might or might not contain further DTLS records. + * in which case the datagram of the underlying transport that is + * currently being processed might or might not contain further + * DTLS records. * * \note If this function returns something other than 0 or * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context @@ -2475,7 +2476,7 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * \param len maximum number of bytes to read * * \return One of the following: - * - 0 for EOF, or + * - 0 if the read end of the underlying transport was closed, * - the (positive) number of bytes read, or * - a negative error code on failure. * @@ -2506,22 +2507,21 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * MBEDTLS_ERR_SSL_CLIENT_RECONNECT, then the ssl context * becomes unusable, and you should either free it or call * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * a new connection. * * \note Remarks regarding event-driven DTLS: * - If the function returns MBEDTLS_ERR_SSL_WANT_READ, no datagram * from the underlying transport layer is currently being processed, * and it is safe to idle until the timer or the underlying transport * signal a new event. - * - If the function returns MBEDTLS_ERR_SSL_WANT_READ this does not mean - * that no data was available from the underlying transport in the first place, - * as there might have been delayed or duplicated messages, or a renegotiation - * request from the peer. Therefore, the user must be prepared to receive - * MBEDTLS_ERR_SSL_WANT_READ even when reacting to an incoming-data event - * from the underlying transport. - * - On success, the currently processed underlying transport's datagram - * might or might not contain further DTLS records, and the user should - * consult \c mbedtls_ssl_check_pending in that regard. + * - This function may return MBEDTLS_ERR_SSL_WANT_READ even if data was + * initially available on the underlying transport, as this data may have + * been only e.g. duplicated messages or a renegotiation request. + * Therefore, you must be prepared to receive MBEDTLS_ERR_SSL_WANT_READ even + * when reacting to an incoming-data event from the underlying transport. + * - On success, the datagram of the underlying transport that is currently + * being processed may contain further DTLS records. You should call + * \c mbedtls_ssl_check_pending to check for remaining records. * */ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ); From a6fb089efc6b6b30434f7db5d4c330e204e03896 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 13:17:48 +0100 Subject: [PATCH 19/55] Don't split debug messages --- library/ssl_tls.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 83d3c9698..caa1cd32e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6377,8 +6377,7 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) if( ssl->keep_current_message == 1 ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: record " - "held back for processing" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: record held back for processing" ) ); return( 1 ); } @@ -6390,8 +6389,7 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && ssl->in_left > ssl->next_record_offset ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more records " - "within current datagram" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more records within current datagram" ) ); return( 1 ); } #endif /* MBEDTLS_SSL_PROTO_DTLS */ @@ -6404,8 +6402,7 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) * read_record_layer, see IOTSSL-1414 */ if( ssl->in_hslen > 0 && ssl->in_hslen < ssl->in_msglen ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more handshake " - "messages within current record" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more handshake messages within current record" ) ); return( 1 ); } @@ -6414,8 +6411,7 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) */ if( ssl->in_offt != NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: application data " - "record is being processed" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: application data record is being processed" ) ); return( 1 ); } From e72489de11067b033a444c13d3b4c305d160cb3c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 13:23:50 +0100 Subject: [PATCH 20/55] Remove internal references and use milder wording for some comments --- library/ssl_tls.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index caa1cd32e..80a06fe30 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3790,11 +3790,6 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) * (2) Alert messages: * Consume whole record content, in_msglen = 0. * - * NOTE: This needs to be fixed, since like for - * handshake messages it is allowed to have - * multiple alerts witin a single record. - * Internal reference IOTSSL-1321. - * * (3) Change cipher spec: * Consume whole record content, in_msglen = 0. * @@ -3822,12 +3817,12 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl ) */ /* Notes: - * (1) in_hslen is *NOT* necessarily the size of the + * (1) in_hslen is not necessarily the size of the * current handshake content: If DTLS handshake * fragmentation is used, that's the fragment * size instead. Using the total handshake message - * size here is FAULTY and should be changed at - * some point. Internal reference IOTSSL-1414. + * size here is faulty and should be changed at + * some point. * (2) While it doesn't seem to cause problems, one * has to be very careful not to assume that in_hslen * is always <= in_msglen in a sensible communication. @@ -6398,8 +6393,6 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) * Case C: A handshake message is being processed. */ - /* TODO This needs correction in the same way as - * read_record_layer, see IOTSSL-1414 */ if( ssl->in_hslen > 0 && ssl->in_hslen < ssl->in_msglen ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more handshake messages within current record" ) ); From e41158ba10b82da1006509edb1f59f2b5cb435a0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 13:30:32 +0100 Subject: [PATCH 21/55] Add comment on the meaning of ssl->in_offt == NULL --- library/ssl_tls.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 80a06fe30..a0c19c936 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6849,6 +6849,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) * ssl->keep_current_message = 0 below. */ + /* Loop as long as no application data record is available */ while( ssl->in_offt == NULL ) { /* Start timer if not already running */ From 4ac73e78048235902727ed420f04dc2d7f296135 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 15:27:37 +0100 Subject: [PATCH 22/55] Use shell string processing instead of sed in ssl-opt.sh --- tests/ssl-opt.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 57d5e6053..5078c0bcd 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -457,7 +457,7 @@ run_test() { kill $PXY_PID >/dev/null 2>&1 sleep 0.01 if kill -0 $PXY_PID >/dev/null 2>&1; then - kill -KILL $pXY_PID + kill -KILL $PXY_PID wait $PXY_PID fi fi @@ -608,12 +608,9 @@ fi get_options "$@" # sanity checks, avoid an avalanche of errors -P_SRV_BIN=$(echo "$P_SRV" | sed -r -n "s/^([^ ]*).*$/\1/p") -echo "Server binary: ${P_SRV_BIN}" -P_CLI_BIN=$(echo "$P_CLI" | sed -r -n "s/^([^ ]*).*$/\1/p") -echo "Client binary: ${P_CLI_BIN}" -P_PXY_BIN=$(echo "$P_PXY" | sed -r -n "s/^([^ ]*).*$/\1/p") -echo "Proxy binary: ${P_PXY_BIN}" +P_SRV_BIN="${P_SRV%%[ ]*}" +P_CLI_BIN="${P_CLI%%[ ]*}" +P_PXY_BIN="${P_PXY%%[ ]*}" if [ ! -x "$P_SRV_BIN" ]; then echo "Command '$P_SRV_BIN' is not an executable file" exit 1 From 22829e9860029eb6e3e333ee481ef232fce0c97a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 15:28:55 +0100 Subject: [PATCH 23/55] Don't use sed -r in udp_proxy_wrapper.sh --- programs/test/udp_proxy_wrapper.sh | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/programs/test/udp_proxy_wrapper.sh b/programs/test/udp_proxy_wrapper.sh index 415f88399..d0a366095 100755 --- a/programs/test/udp_proxy_wrapper.sh +++ b/programs/test/udp_proxy_wrapper.sh @@ -2,16 +2,14 @@ set -u -MBEDTLS_BASE="$(pwd)/$(dirname $0)/../../" -TPXY_BIN="$MBEDTLS_BASE/test/udp_proxy" +MBEDTLS_BASE="$(dirname -- "$0")/../.." +TPXY_BIN="$MBEDTLS_BASE/programs/test/udp_proxy" SRV_BIN="$MBEDTLS_BASE/programs/ssl/ssl_server2" : ${VERBOSE:=0} -VERBOSE=1 - -PARAM_SEP="^(.*)--(.*)$" -PROXY_PARAMS=$(echo $@ | sed -n -r "s/$PARAM_SEP/\1/p") -SERVER_PARAMS=$(echo $@ | sed -n -r "s/$PARAM_SEP/\2/p") +FULL_PARAMS=$* +PROXY_PARAMS=${FULL_PARAMS%%" -- "*} +SERVER_PARAMS=${FULL_PARAMS#*" -- "} stop_proxy() { test -n "${TPXY_PID:-}" && @@ -49,13 +47,13 @@ if [ -z "$DTLS_ENABLED" ]; then exit 0 fi -SERVER_PORT_ORIG=$(echo "$SERVER_PARAMS" | sed -n -r "s/^.*server_port=([0-9]+).*$/\1/p") +SERVER_PORT_ORIG=$(echo "$SERVER_PARAMS" | sed -n "s/^.*server_port=\([0-9]*\).*$/\1/p") if [ -z "$SERVER_PORT_ORIG" ]; then echo " * No server port specified - exit" exit 1 fi -SERVER_ADDR_ORIG=$(echo "$SERVER_PARAMS" | sed -n -r "s/^.*server_addr=([a-zA-Z0-9\.]+).*$/\1/p") +SERVER_ADDR_ORIG=$(echo "$SERVER_PARAMS" | sed -n "s/^.*server_addr=\([a-zA-Z0-9\.]*\).*$/\1/p") if [ -z "$SERVER_ADDR_ORIG" ]; then echo " * No server address specified - exit" exit 1 @@ -86,7 +84,7 @@ if [ $VERBOSE -gt 0 ]; then echo " * Proxy ID: $TPXY_PID" fi -SERVER_PARAMS_NEW=$(echo $SERVER_PARAMS | sed -n -r "s/^(.*server_port=)[0-9]+(.*)$/\1$SERVER_PORT\2/p") +SERVER_PARAMS_NEW=$(echo "$SERVER_PARAMS" | sed -n "s/^\(.*server_port=\)[0-9]*\(.*\)$/\1$SERVER_PORT\2/p") SRV_CMD="$SRV_BIN $SERVER_PARAMS_NEW" echo " * Starting server ..." From a677cdd4592ebd7d0ef725109e76ca3662d8c477 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 23 Oct 2017 15:29:31 +0100 Subject: [PATCH 24/55] Detect IPv6 in udp_proxy_wrapper.sh grepping for `server_addr=::1` --- programs/test/udp_proxy_wrapper.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/programs/test/udp_proxy_wrapper.sh b/programs/test/udp_proxy_wrapper.sh index d0a366095..987b8a4ca 100755 --- a/programs/test/udp_proxy_wrapper.sh +++ b/programs/test/udp_proxy_wrapper.sh @@ -35,8 +35,10 @@ cleanup() { trap cleanup INT TERM HUP -DTLS_ENABLED=$(echo "$SERVER_PARAMS" | grep -v "::1" | grep "dtls=1") -if [ -z "$DTLS_ENABLED" ]; then +DTLS_ENABLED=$(echo " $SERVER_PARAMS" | grep " dtls=1") +IPV6_IN_USE=$(echo " $SERVER_PARAMS" | grep " server_addr=::1" ) + +if [ -z "$DTLS_ENABLED" ] || [ -n "$IPV6_IN_USE" ]; then echo " * Couldn't find DTLS enabling, or IPv6 is in use - immediate fallback to server application..." if [ $VERBOSE -gt 0 ]; then echo "[ $SRV_BIN $SERVER_PARAMS ]" From afc4f892d1f3afb1f1bd40a5392609392fb5eb12 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 24 Oct 2017 10:00:17 +0200 Subject: [PATCH 25/55] udp_proxy_wrapper.sh: more robust Don't mangle arguments containing spaces and other special characters, pass them unchanged to the proxy or server as applicable. More robust parsing of server parameters: don't hit on partial words; use ssl_server2's default values. Minor style improvements. --- programs/test/udp_proxy_wrapper.sh | 124 ++++++++++++++++------------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/programs/test/udp_proxy_wrapper.sh b/programs/test/udp_proxy_wrapper.sh index 987b8a4ca..fa13596d6 100755 --- a/programs/test/udp_proxy_wrapper.sh +++ b/programs/test/udp_proxy_wrapper.sh @@ -1,4 +1,6 @@ #!/bin/sh +# -*-sh-basic-offset: 4-*- +# Usage: udp_proxy_wrapper.sh [PROXY_PARAM...] -- [SERVER_PARAM...] set -u @@ -7,24 +9,21 @@ TPXY_BIN="$MBEDTLS_BASE/programs/test/udp_proxy" SRV_BIN="$MBEDTLS_BASE/programs/ssl/ssl_server2" : ${VERBOSE:=0} -FULL_PARAMS=$* -PROXY_PARAMS=${FULL_PARAMS%%" -- "*} -SERVER_PARAMS=${FULL_PARAMS#*" -- "} stop_proxy() { - test -n "${TPXY_PID:-}" && - ( - echo "\n * Killing proxy (pid $TPXY_PID) ..." - kill $TPXY_PID - ) + if [ -n "${tpxy_pid:-}" ]; then + echo + echo " * Killing proxy (pid $tpxy_pid) ..." + kill $tpxy_pid + fi } stop_server() { - test -n "${SRV_PID:-}" && - ( - echo "\n * Killing server (pid $SRV_PID) ..." - kill $SRV_PID >/dev/null 2>/dev/null - ) + if [ -n "${srv_pid:-}" ]; then + echo + echo " * Killing server (pid $srv_pid) ..." + kill $srv_pid >/dev/null 2>/dev/null + fi } cleanup() { @@ -35,69 +34,84 @@ cleanup() { trap cleanup INT TERM HUP -DTLS_ENABLED=$(echo " $SERVER_PARAMS" | grep " dtls=1") -IPV6_IN_USE=$(echo " $SERVER_PARAMS" | grep " server_addr=::1" ) +# Extract the proxy parameters +tpxy_cmd_snippet='"$TPXY_BIN"' +while [ $# -ne 0 ] && [ "$1" != "--" ]; do + tail="$1" quoted="" + while [ -n "$tail" ]; do + case "$tail" in + *\'*) quoted="${quoted}${tail%%\'*}'\\''" tail="${tail#*\'}";; + *) quoted="${quoted}${tail}"; tail=; false;; + esac + done + tpxy_cmd_snippet="$tpxy_cmd_snippet '$quoted'" + shift +done +unset tail quoted +if [ $# -eq 0 ]; then + echo " * No server arguments (must be preceded by \" -- \") - exit" + exit 3 +fi +shift -if [ -z "$DTLS_ENABLED" ] || [ -n "$IPV6_IN_USE" ]; then - echo " * Couldn't find DTLS enabling, or IPv6 is in use - immediate fallback to server application..." +dtls_enabled= +ipv6_in_use= +server_port_orig= +server_addr_orig= +for param; do + case "$param" in + server_port=*) server_port_orig="${param#*=}";; + server_addr=*:*) server_addr_orig="${param#*=}"; ipv6_in_use=1;; + server_addr=*) server_addr_orig="${param#*=}";; + dtls=[!0]*) dtls_enabled=1;; + esac +done + +if [ -z "$dtls_enabled" ] || [ -n "$ipv6_in_use" ]; then + echo >&2 "$0: Couldn't find DTLS enabling, or IPv6 is in use - immediate fallback to server application..." if [ $VERBOSE -gt 0 ]; then - echo "[ $SRV_BIN $SERVER_PARAMS ]" + echo "[ $SRV_BIN $* ]" fi - $SRV_BIN $SERVER_PARAMS >&1 2>&1 & - SRV_PID=$! - wait $SRV_PID - exit 0 + exec "$SRV_BIN" "$@" fi -SERVER_PORT_ORIG=$(echo "$SERVER_PARAMS" | sed -n "s/^.*server_port=\([0-9]*\).*$/\1/p") -if [ -z "$SERVER_PORT_ORIG" ]; then - echo " * No server port specified - exit" - exit 1 +if [ -z "$server_port_orig" ]; then + server_port_orig=4433 +fi +echo " * Server port: $server_port_orig" +tpxy_cmd_snippet="$tpxy_cmd_snippet \"listen_port=\$server_port_orig\"" +tpxy_cmd_snippet="$tpxy_cmd_snippet \"server_port=\$server_port\"" + +if [ -n "$server_addr_orig" ]; then + echo " * Server address: $server_addr_orig" + tpxy_cmd_snippet="$tpxy_cmd_snippet \"server_addr=\$server_addr_orig\"" + tpxy_cmd_snippet="$tpxy_cmd_snippet \"listen_addr=\$server_addr_orig\"" fi -SERVER_ADDR_ORIG=$(echo "$SERVER_PARAMS" | sed -n "s/^.*server_addr=\([a-zA-Z0-9\.]*\).*$/\1/p") -if [ -z "$SERVER_ADDR_ORIG" ]; then - echo " * No server address specified - exit" - exit 1 -fi - -echo " * Server address: $SERVER_ADDR_ORIG" -echo " * Server port: $SERVER_PORT_ORIG" - -SERVER_PORT=$(( $SERVER_PORT_ORIG + 1 )) -echo " * Intermediate port: $SERVER_PORT" - -TPXY_CMD=\ -"$TPXY_BIN $PROXY_PARAMS "\ -"listen_port=$SERVER_PORT_ORIG "\ -"server_port=$SERVER_PORT "\ -"server_addr=$SERVER_ADDR_ORIG "\ -"listen_addr=$SERVER_ADDR_ORIG" +server_port=$(( server_port_orig + 1 )) +set -- "$@" "server_port=$server_port" +echo " * Intermediate port: $server_port" echo " * Start proxy in background ..." if [ $VERBOSE -gt 0 ]; then - echo "[ $TPXY_CMD ]" + echo "[ $tpxy_cmd_snippet ]" fi - -$TPXY_CMD >/dev/null 2>&1 & -TPXY_PID=$! +eval "$tpxy_cmd_snippet" >/dev/null 2>&1 & +tpxy_pid=$! if [ $VERBOSE -gt 0 ]; then echo " * Proxy ID: $TPXY_PID" fi -SERVER_PARAMS_NEW=$(echo "$SERVER_PARAMS" | sed -n "s/^\(.*server_port=\)[0-9]*\(.*\)$/\1$SERVER_PORT\2/p") -SRV_CMD="$SRV_BIN $SERVER_PARAMS_NEW" - echo " * Starting server ..." if [ $VERBOSE -gt 0 ]; then - echo "[ $SRV_CMD ]" + echo "[ $SRV_BIN $* ]" fi -$SRV_CMD >&2 & -SRV_PID=$! +"$SRV_BIN" "$@" >&2 & +srv_pid=$! -wait $SRV_PID +wait $srv_pid stop_proxy return 0 From 8149321fedd0085f18783d41490a7d4043f7716c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 24 Oct 2017 12:22:40 +0200 Subject: [PATCH 26/55] udp_proxy_wrapper.sh: fix cleanup not cleaning up Fixed cleanup leaving the actual udp_proxy behind and only killing an intermediate shell process. Fixed trap handler cleaning up but then not dying. --- programs/test/udp_proxy_wrapper.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/test/udp_proxy_wrapper.sh b/programs/test/udp_proxy_wrapper.sh index fa13596d6..29033d5d1 100755 --- a/programs/test/udp_proxy_wrapper.sh +++ b/programs/test/udp_proxy_wrapper.sh @@ -29,7 +29,7 @@ stop_server() { cleanup() { stop_server stop_proxy - return 1 + exit 129 } trap cleanup INT TERM HUP @@ -96,7 +96,7 @@ echo " * Start proxy in background ..." if [ $VERBOSE -gt 0 ]; then echo "[ $tpxy_cmd_snippet ]" fi -eval "$tpxy_cmd_snippet" >/dev/null 2>&1 & +eval exec "$tpxy_cmd_snippet" >/dev/null 2>&1 & tpxy_pid=$! if [ $VERBOSE -gt 0 ]; then @@ -108,7 +108,7 @@ if [ $VERBOSE -gt 0 ]; then echo "[ $SRV_BIN $* ]" fi -"$SRV_BIN" "$@" >&2 & +exec "$SRV_BIN" "$@" >&2 & srv_pid=$! wait $srv_pid From df4180a235de1990d9769a1010b03cfe9cbed8c2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 27 Oct 2017 13:43:58 +0100 Subject: [PATCH 27/55] Don't break debug messages --- programs/ssl/ssl_client2.c | 78 +++++++++++++++++++------------------- programs/ssl/ssl_server2.c | 24 ++++++------ programs/test/udp_proxy.c | 5 +-- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 5b82693ff..ed3966495 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -478,8 +478,7 @@ void idle( mbedtls_ssl_context *ssl, * if data is still pending to be processed. */ if( mbedtls_ssl_check_pending( ssl ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, " - "but idling requested!" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, but idling requested!" ) ); } MBEDTLS_SSL_DEBUG_MSG( 3, ( "idle, waiting for event... " ) ); @@ -504,8 +503,8 @@ void idle( mbedtls_ssl_context *ssl, if( poll_type != 0 && mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - " - "continue", time_elapsed ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - continue", + time_elapsed ) ); break; } } @@ -960,8 +959,7 @@ int main( int argc, char *argv[] ) * refers to the underlying net_context. */ if( opt.event == 1 && opt.nbio != 1 ) { - mbedtls_printf( "Warning: event-driven IO mandates nbio=1" - " - overwrite\n" ); + mbedtls_printf( "Warning: event-driven IO mandates nbio=1 - overwrite\n" ); opt.nbio = 1; } @@ -1236,8 +1234,8 @@ int main( int argc, char *argv[] ) #endif if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", + -ret ); goto exit; } @@ -1261,8 +1259,8 @@ int main( int argc, char *argv[] ) #endif if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_pk_parse_key " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", + -ret ); goto exit; } @@ -1285,8 +1283,8 @@ int main( int argc, char *argv[] ) opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM ? MBEDTLS_NET_PROTO_TCP : MBEDTLS_NET_PROTO_UDP ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_net_connect " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_net_connect returned -0x%x\n\n", + -ret ); goto exit; } @@ -1296,8 +1294,8 @@ int main( int argc, char *argv[] ) ret = mbedtls_net_set_block( &server_fd ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! net_set_(non)block() " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! net_set_(non)block() returned -0x%x\n\n", + -ret ); goto exit; } @@ -1314,8 +1312,8 @@ int main( int argc, char *argv[] ) opt.transport, MBEDTLS_SSL_PRESET_DEFAULT ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_config_defaults " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_config_defaults returned -0x%x\n\n", + -ret ); goto exit; } @@ -1345,8 +1343,8 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) if( ( ret = mbedtls_ssl_conf_max_frag_len( &conf, opt.mfl_code ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_max_frag_len " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_max_frag_len returned %d\n\n", + ret ); goto exit; } #endif @@ -1382,8 +1380,8 @@ int main( int argc, char *argv[] ) if( opt.alpn_string != NULL ) if( ( ret = mbedtls_ssl_conf_alpn_protocols( &conf, alpn_list ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_alpn_protocols " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_alpn_protocols returned %d\n\n", + ret ); goto exit; } #endif @@ -1422,8 +1420,8 @@ int main( int argc, char *argv[] ) { if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &clicert, &pkey ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_own_cert " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_own_cert returned %d\n\n", + ret ); goto exit; } } @@ -1442,8 +1440,8 @@ int main( int argc, char *argv[] ) (const unsigned char *) opt.psk_identity, strlen( opt.psk_identity ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_conf_psk " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_conf_psk returned %d\n\n", + ret ); goto exit; } #endif @@ -1463,16 +1461,16 @@ int main( int argc, char *argv[] ) if( ( ret = mbedtls_ssl_setup( &ssl, &conf ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_setup " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_setup returned -0x%x\n\n", + -ret ); goto exit; } #if defined(MBEDTLS_X509_CRT_PARSE_C) if( ( ret = mbedtls_ssl_set_hostname( &ssl, opt.server_name ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_set_hostname " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_set_hostname returned %d\n\n", + ret ); goto exit; } #endif @@ -1484,8 +1482,8 @@ int main( int argc, char *argv[] ) (const unsigned char *) opt.ecjpake_pw, strlen( opt.ecjpake_pw ) ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_set_hs_ecjpake_password " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_set_hs_ecjpake_password returned %d\n\n", + ret ); goto exit; } } @@ -1516,8 +1514,8 @@ int main( int argc, char *argv[] ) if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_handshake " - "returned -0x%x\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_handshake returned -0x%x\n", + -ret ); if( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ) mbedtls_printf( " Unable to verify the server's certificate. " @@ -1571,8 +1569,8 @@ int main( int argc, char *argv[] ) if( ( ret = mbedtls_ssl_get_session( &ssl, &saved_session ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_get_session " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_get_session returned -0x%x\n\n", + -ret ); goto exit; } @@ -1622,8 +1620,8 @@ int main( int argc, char *argv[] ) if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_renegotiate " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_renegotiate returned %d\n\n", + ret ); goto exit; } @@ -1686,8 +1684,8 @@ send_request: if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_write " - "returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_write returned -0x%x\n\n", + -ret ); goto exit; } @@ -1726,8 +1724,8 @@ send_request: if( ret < 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ssl_write " - "returned %d\n\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_write returned %d\n\n", + ret ); goto exit; } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index d16c53419..d70046c84 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1427,8 +1427,7 @@ int main( int argc, char *argv[] ) * refers to the underlying net_context. */ if( opt.event == 1 && opt.nbio != 1 ) { - mbedtls_printf( "Warning: event-driven IO mandates nbio=1" - " - overwrite\n" ); + mbedtls_printf( "Warning: event-driven IO mandates nbio=1 - overwrite\n" ); opt.nbio = 1; } @@ -1733,7 +1732,7 @@ int main( int argc, char *argv[] ) if( ( ret = mbedtls_pk_parse_keyfile( &pkey2, opt.key_file2, "" ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_pk_parse_keyfile(2) returned -0x%x\n\n", - -ret ); + -ret ); goto exit; } } @@ -1751,8 +1750,7 @@ int main( int argc, char *argv[] ) strcmp( opt.key_file2, "none" ) != 0 ) { #if !defined(MBEDTLS_CERTS_C) - mbedtls_printf( "Not certificated or key provided, and \n" - "MBEDTLS_CERTS_C not defined!\n" ); + mbedtls_printf( "Not certificated or key provided, and \nMBEDTLS_CERTS_C not defined!\n" ); goto exit; #else #if defined(MBEDTLS_RSA_C) @@ -1760,14 +1758,16 @@ int main( int argc, char *argv[] ) (const unsigned char *) mbedtls_test_srv_crt_rsa, mbedtls_test_srv_crt_rsa_len ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_x509_crt_parse returned -0x%x\n\n", + -ret ); goto exit; } if( ( ret = mbedtls_pk_parse_key( &pkey, (const unsigned char *) mbedtls_test_srv_key_rsa, mbedtls_test_srv_key_rsa_len, NULL, 0 ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_pk_parse_key returned -0x%x\n\n", + -ret ); goto exit; } key_cert_init = 2; @@ -1777,14 +1777,16 @@ int main( int argc, char *argv[] ) (const unsigned char *) mbedtls_test_srv_crt_ec, mbedtls_test_srv_crt_ec_len ) ) != 0 ) { - mbedtls_printf( " failed\n ! x509_crt_parse2 returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! x509_crt_parse2 returned -0x%x\n\n", + -ret ); goto exit; } if( ( ret = mbedtls_pk_parse_key( &pkey2, (const unsigned char *) mbedtls_test_srv_key_ec, mbedtls_test_srv_key_ec_len, NULL, 0 ) ) != 0 ) { - mbedtls_printf( " failed\n ! pk_parse_key2 returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! pk_parse_key2 returned -0x%x\n\n", + -ret ); goto exit; } key_cert_init2 = 2; @@ -2190,8 +2192,8 @@ reset: if( ( ret = mbedtls_ssl_set_client_transport_id( &ssl, client_ip, cliip_len ) ) != 0 ) { - mbedtls_printf( " failed\n ! " - "mbedtls_ssl_set_client_transport_id() returned -0x%x\n\n", -ret ); + mbedtls_printf( " failed\n ! mbedtls_ssl_set_client_transport_id() returned -0x%x\n\n", + -ret ); goto exit; } } diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index c978f9047..386b1fcad 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -318,9 +318,8 @@ static int ctx_buffer_flush( ctx_buffer *buf ) { int ret; - mbedtls_printf( " %05lu flush %s: %u bytes, %lu datagrams, " - "last %ld ms\n", ellapsed_time(), - buf->description, buf->len, buf->num_datagrams, + mbedtls_printf( " %05lu flush %s: %u bytes, %lu datagrams, last %ld ms\n", + ellapsed_time(), buf->description, buf->len, buf->num_datagrams, ellapsed_time() - buf->packet_lifetime ); ret = mbedtls_net_send( buf->ctx, buf->data, buf->len ); From 197a91cd82c6351beacb23d2bea0522066c9b332 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 31 Oct 2017 10:58:53 +0000 Subject: [PATCH 28/55] Clean up idle() function in ssl_client2 and ssl_server2 --- programs/ssl/ssl_client2.c | 67 ++++++++++++-------------------------- programs/ssl/ssl_server2.c | 64 +++++++++++------------------------- 2 files changed, 40 insertions(+), 91 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index ed3966495..289920cbd 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -444,20 +444,14 @@ static int ssl_sig_hashes_for_test[] = { * (Used in event-driven IO mode). */ #if !defined(MBEDTLS_TIMING_C) -void idle( mbedtls_ssl_context *ssl, - mbedtls_net_context *fd, +void idle( mbedtls_net_context *fd, int idle_reason ) { #else -void idle( mbedtls_ssl_context *ssl, - mbedtls_net_context *fd, +void idle( mbedtls_net_context *fd, mbedtls_timing_delay_context *timer, int idle_reason ) { -#if defined(MBEDTLS_DEBUG_C) - struct mbedtls_timing_hr_time tm; - unsigned long time_elapsed; -#endif #endif int poll_type = 0; @@ -468,43 +462,24 @@ void idle( mbedtls_ssl_context *ssl, poll_type = MBEDTLS_NET_POLL_READ; #if !defined(MBEDTLS_TIMING_C) else - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: No reason for idling given" ) ); return; - } -#endif - - /* One should not idle on the underlying transport - * if data is still pending to be processed. */ - if( mbedtls_ssl_check_pending( ssl ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, but idling requested!" ) ); - } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "idle, waiting for event... " ) ); - -#if defined(MBEDTLS_TIMING_C) && defined(MBEDTLS_DEBUG_C) - mbedtls_timing_get_timer( &tm, 1 /* restart */ ); #endif while( 1 ) { + /* Check if timer has expired */ #if defined(MBEDTLS_TIMING_C) -#if defined(MBEDTLS_DEBUG_C) - time_elapsed = mbedtls_timing_get_timer( &tm, 0 ); -#endif - if( mbedtls_timing_get_delay( timer ) == 2 ) + if( timer != NULL && + mbedtls_timing_get_delay( timer ) == 2 ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] timer expired - continue", - time_elapsed ) ); break; } -#endif +#endif /* MBEDTLS_TIMING_C */ + /* Check if underlying transport became available */ if( poll_type != 0 && mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - continue", - time_elapsed ) ); break; } } @@ -1532,9 +1507,9 @@ int main( int argc, char *argv[] ) if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } } @@ -1629,9 +1604,9 @@ int main( int argc, char *argv[] ) if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } @@ -1693,9 +1668,9 @@ send_request: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } } @@ -1715,9 +1690,9 @@ send_request: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } } @@ -1761,9 +1736,9 @@ send_request: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } continue; @@ -1822,9 +1797,9 @@ send_request: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } } @@ -1887,9 +1862,9 @@ send_request: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &server_fd, &timer, ret ); + idle( &server_fd, &timer, ret ); #else - idle( &ssl, &server_fd, ret ); + idle( &server_fd, ret ); #endif } } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index d70046c84..c3321d13a 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -846,20 +846,14 @@ static int ssl_sig_hashes_for_test[] = { * (Used in event-driven IO mode). */ #if !defined(MBEDTLS_TIMING_C) -void idle( mbedtls_ssl_context *ssl, - mbedtls_net_context *fd, +void idle( mbedtls_net_context *fd, int idle_reason ) { #else -void idle( mbedtls_ssl_context *ssl, - mbedtls_net_context *fd, +void idle( mbedtls_net_context *fd, mbedtls_timing_delay_context *timer, int idle_reason ) { -#if defined(MBEDTLS_DEBUG_C) - struct mbedtls_timing_hr_time tm; - unsigned long time_elapsed; -#endif #endif int poll_type = 0; @@ -870,44 +864,24 @@ void idle( mbedtls_ssl_context *ssl, poll_type = MBEDTLS_NET_POLL_READ; #if !defined(MBEDTLS_TIMING_C) else - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: No reason for idling given" ) ); return; - } -#endif - - /* One should not idle on the underlying transport - * if data is still pending to be processed. */ - if( mbedtls_ssl_check_pending( ssl ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "WARNING: Data still pending, " - "but idling requested!" ) ); - } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "idle, waiting for event... " ) ); - -#if defined(MBEDTLS_TIMING_C) && defined(MBEDTLS_DEBUG_C) - mbedtls_timing_get_timer( &tm, 1 /* restart */ ); #endif while( 1 ) { + /* Check if timer has expired */ #if defined(MBEDTLS_TIMING_C) -#if defined(MBEDTLS_DEBUG_C) - time_elapsed = mbedtls_timing_get_timer( &tm, 0 ); -#endif - if( mbedtls_timing_get_delay( timer ) == 2 ) + if( timer != NULL && + mbedtls_timing_get_delay( timer ) == 2 ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] timer expired - continue", - time_elapsed ) ); break; } -#endif +#endif /* MBEDTLS_TIMING_C */ + /* Check if underlying transport became available */ if( poll_type != 0 && mbedtls_net_poll( fd, poll_type, 0 ) == poll_type ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "[%lu ms] net_context signals data - " - "continue", time_elapsed ) ); break; } } @@ -2231,9 +2205,9 @@ handshake: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } } @@ -2346,9 +2320,9 @@ data_exchange: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } @@ -2453,9 +2427,9 @@ data_exchange: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } } @@ -2504,9 +2478,9 @@ data_exchange: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } } @@ -2548,9 +2522,9 @@ data_exchange: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } } @@ -2570,9 +2544,9 @@ data_exchange: if( opt.event == 1 /* level triggered IO */ ) { #if defined(MBEDTLS_TIMING_C) - idle( &ssl, &client_fd, &timer, ret ); + idle( &client_fd, &timer, ret ); #else - idle( &ssl, &client_fd, ret ); + idle( &client_fd, ret ); #endif } } From 9b19a1253f56b53e6b093197bf6947bb7b51c344 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 31 Oct 2017 13:00:14 +0000 Subject: [PATCH 29/55] Clarify use of mbedtls_ssl_check_pending --- include/mbedtls/ssl.h | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 43ba67cd5..594c7d6b1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2243,15 +2243,6 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf, * * \return 0 if nothing's pending, 1 otherwise. * - * \note This function is essential when using the library - * with event-driven I/O. You should not idle - * (waiting for events from the underlying transport - * or from timers) before this function's check passes. - * Otherwise, it's possible to run into a deadlock - * (if processing the pending data involves essential - * communication with the peer) or to accumulate and - * potentially lose data. - * * \note This is different in purpose and behaviour from * \c mbedtls_ssl_get_bytes_avail in that it considers * any kind of unprocessed data, not only unread @@ -2262,11 +2253,25 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf, * further records waiting to be processed from * the current underlying transport's datagram. * - * \note If this function returns 0 (data pending), this + * \note If this function returns 1 (data pending), this * does not imply that a subsequent call to * \c mbedtls_ssl_read will provide any data; * e.g., the unprocessed data might turn out * to be an alert or a handshake message. + * + * \note This function is useful in the following situation: + * If the SSL/TLS module successfully returns from an + * operation - e.g. a handshake or an application record + * read - and you're awaiting incoming data next, you + * must not immediately idle on the underlying transport + * to have data ready, but you need to check the value + * of this function first. The reason is that the desired + * data might already be read but not yet processed. + * If, in contrast, a previous call to the SSL/TLS module + * returned MBEDTLS_ERR_SSL_WANT_READ, it is not necessary + * to call this function, as the latter error code entails + * that all internal data has been processed. + * */ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ); From 211f44c928293203c82b6781ff58346d9f00739e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 31 Oct 2017 14:08:10 +0000 Subject: [PATCH 30/55] Rename `merge` option in UDP proxy to `pack` --- programs/test/udp_proxy.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 386b1fcad..d0c5b9450 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -131,7 +131,7 @@ static struct options int bad_ad; /* inject corrupted ApplicationData record */ int protect_hvr; /* never drop or delay HelloVerifyRequest */ int protect_len; /* never drop/delay packet of the given size*/ - int merge; /* merge packets into single datagram for + int pack; /* merge packets into single datagram for * at most \c merge milliseconds if > 0 */ unsigned int seed; /* seed for "random" events */ @@ -157,7 +157,7 @@ static void get_options( int argc, char *argv[] ) opt.server_port = DFL_SERVER_PORT; opt.listen_addr = DFL_LISTEN_ADDR; opt.listen_port = DFL_LISTEN_PORT; - opt.merge = DFL_PACK; + opt.pack = DFL_PACK; /* Other members default to 0 */ for( i = 1; i < argc; i++ ) @@ -201,7 +201,7 @@ static void get_options( int argc, char *argv[] ) } else if( strcmp( p, "pack" ) == 0 ) { - opt.merge = atoi( q ); + opt.pack = atoi( q ); } else if( strcmp( p, "mtu" ) == 0 ) { @@ -333,7 +333,7 @@ static int ctx_buffer_flush( ctx_buffer *buf ) static inline int ctx_buffer_check( ctx_buffer *buf ) { if( buf->len > 0 && - ellapsed_time() - buf->packet_lifetime >= (size_t) opt.merge ) + ellapsed_time() - buf->packet_lifetime >= (size_t) opt.pack ) { return( ctx_buffer_flush( buf ) ); } @@ -669,7 +669,7 @@ accept: nb_fds = listen_fd.fd; ++nb_fds; - if( opt.merge > 0 ) + if( opt.pack > 0 ) { outbuf[0].ctx = &server_fd; outbuf[0].description = "S <- C"; From 92474da0a22548b05340857d44020856bc6f3be7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 31 Oct 2017 14:09:30 +0000 Subject: [PATCH 31/55] Use Mbed TLS timing module to obtain ellapsed time in udp_proxy --- programs/test/udp_proxy.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index d0c5b9450..39b3bed4a 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -280,22 +280,17 @@ static const char *msg_type( unsigned char *msg, size_t len ) /* Return elapsed time in milliseconds since the first call */ static unsigned long ellapsed_time( void ) { -#if defined(_WIN32) - return( 0 ); -#else - static struct timeval ref = { 0, 0 }; - struct timeval now; + static int initialized = 0; + static struct mbedtls_timing_hr_time hires; - if( ref.tv_sec == 0 && ref.tv_usec == 0 ) + if( initialized == 0 ) { - gettimeofday( &ref, NULL ); + (void) mbedtls_timing_get_timer( &hires, 1 ); + initialized = 1; return( 0 ); } - gettimeofday( &now, NULL ); - return( 1000 * ( now.tv_sec - ref.tv_sec ) - + ( now.tv_usec - ref.tv_usec ) / 1000 ); -#endif + return( mbedtls_timing_get_timer( &hires, 0 ) ); } typedef struct From 0cc7774dab85d3938b04ead8814e585f76c82f13 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 31 Oct 2017 14:10:07 +0000 Subject: [PATCH 32/55] Only add pack option to UDP proxy if MBEDTLS_TIMING_C is enabled --- programs/test/udp_proxy.c | 57 ++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 39b3bed4a..7e8d309f4 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -53,6 +53,7 @@ int main( void ) #include "mbedtls/net_sockets.h" #include "mbedtls/error.h" #include "mbedtls/ssl.h" +#include "mbedtls/timing.h" #include @@ -74,11 +75,6 @@ int main( void ) #include #endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */ -/* For gettimeofday() */ -#if !defined(_WIN32) -#include -#endif - #define MAX_MSG_SIZE 16384 + 2048 /* max record/datagram size */ #define DFL_SERVER_ADDR "localhost" @@ -87,6 +83,14 @@ int main( void ) #define DFL_LISTEN_PORT "5556" #define DFL_PACK 0 +#if defined(MBEDTLS_TIMING_C) +#define USAGE_PACK \ + " pack=%%d default: 0 (don't pack)\n" \ + " options: t > 0 (pack for t milliseconds)\n" +#else +#define USAGE_PACK +#endif + #define USAGE \ "\n usage: udp_proxy param=<>...\n" \ "\n acceptable parameters:\n" \ @@ -106,11 +110,10 @@ int main( void ) " drop packets larger than N bytes\n" \ " bad_ad=0/1 default: 0 (don't add bad ApplicationData)\n" \ " protect_hvr=0/1 default: 0 (don't protect HelloVerifyRequest)\n" \ - " protect_len=%%d default: (don't protect packets of this size)\n" \ + " protect_len=%%d default: (don't protect packets of this size)\n" \ "\n" \ " seed=%%d default: (use current time)\n" \ - " pack=%%d default: 0 (don't merge)\n" \ - " options: t > 0 (merge for t milliseconds)\n" \ + USAGE_PACK \ "\n" /* @@ -133,7 +136,6 @@ static struct options int protect_len; /* never drop/delay packet of the given size*/ int pack; /* merge packets into single datagram for * at most \c merge milliseconds if > 0 */ - unsigned int seed; /* seed for "random" events */ } opt; @@ -201,7 +203,12 @@ static void get_options( int argc, char *argv[] ) } else if( strcmp( p, "pack" ) == 0 ) { +#if defined(MBEDTLS_TIMING_C) opt.pack = atoi( q ); +#else + mbedtls_printf( " option pack only defined if MBEDTLS_TIMING_C is enabled\n" ); + exit( 1 ); +#endif } else if( strcmp( p, "mtu" ) == 0 ) { @@ -277,6 +284,7 @@ static const char *msg_type( unsigned char *msg, size_t len ) } } +#if defined(MBEDTLS_TIMING_C) /* Return elapsed time in milliseconds since the first call */ static unsigned long ellapsed_time( void ) { @@ -369,6 +377,7 @@ static int dispatch_data( mbedtls_net_context *ctx, size_t len ) { ctx_buffer *buf = NULL; + if( outbuf[0].ctx == ctx ) buf = &outbuf[0]; else if( outbuf[1].ctx == ctx ) @@ -380,6 +389,17 @@ static int dispatch_data( mbedtls_net_context *ctx, return( ctx_buffer_append( buf, data, len ) ); } +#else /* MBEDTLS_TIMING_C */ + +static int dispatch_data( mbedtls_net_context *ctx, + const unsigned char * data, + size_t len ) +{ + return( mbedtls_net_send( ctx, data, len ) ); +} + +#endif /* MBEDTLS_TIMING_C */ + typedef struct { mbedtls_net_context *dst; @@ -392,12 +412,22 @@ typedef struct /* Print packet. Outgoing packets come with a reason (forward, dupl, etc.) */ void print_packet( const packet *p, const char *why ) { +#if defined(MBEDTLS_TIMING_C) if( why == NULL ) mbedtls_printf( " %05lu dispatch %s %s (%u bytes)\n", ellapsed_time(), p->way, p->type, p->len ); + else + mbedtls_printf( " %05lu dispatch %s %s (%u bytes): %s\n", + ellapsed_time(), p->way, p->type, p->len, why ); +#else + if( why == NULL ) + mbedtls_printf( " dispatch %s %s (%u bytes)\n", + p->way, p->type, p->len ); else mbedtls_printf( " dispatch %s %s (%u bytes): %s\n", p->way, p->type, p->len, why ); +#endif + fflush( stdout ); } @@ -664,6 +694,7 @@ accept: nb_fds = listen_fd.fd; ++nb_fds; +#if defined(MBEDTLS_TIMING_C) if( opt.pack > 0 ) { outbuf[0].ctx = &server_fd; @@ -676,6 +707,12 @@ accept: outbuf[1].num_datagrams = 0; outbuf[1].len = 0; } + else + { + outbuf[0].ctx = NULL; + outbuf[1].ctx = NULL; + } +#endif /* MBEDTLS_TIMING_C */ while( 1 ) { @@ -684,8 +721,10 @@ accept: FD_SET( client_fd.fd, &read_fds ); FD_SET( listen_fd.fd, &read_fds ); +#if defined(MBEDTLS_TIMING_C) ctx_buffer_check( &outbuf[0] ); ctx_buffer_check( &outbuf[1] ); +#endif if( ( ret = select( nb_fds, &read_fds, NULL, NULL, &tm ) ) < 0 ) { From 77abef5cba14ebd2f396e786615fb0e9de4b9338 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 2 Nov 2017 10:50:28 +0000 Subject: [PATCH 33/55] Don't use busy-waiting in udp_proxy Also, correct inconsistent use of unsigned integer types in udp_proxy. --- programs/test/udp_proxy.c | 125 +++++++++++++++++++++++--------------- 1 file changed, 76 insertions(+), 49 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 7e8d309f4..0dec40932 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -134,7 +134,7 @@ static struct options int bad_ad; /* inject corrupted ApplicationData record */ int protect_hvr; /* never drop or delay HelloVerifyRequest */ int protect_len; /* never drop/delay packet of the given size*/ - int pack; /* merge packets into single datagram for + unsigned pack; /* merge packets into single datagram for * at most \c merge milliseconds if > 0 */ unsigned int seed; /* seed for "random" events */ } opt; @@ -204,7 +204,7 @@ static void get_options( int argc, char *argv[] ) else if( strcmp( p, "pack" ) == 0 ) { #if defined(MBEDTLS_TIMING_C) - opt.pack = atoi( q ); + opt.pack = (unsigned) atoi( q ); #else mbedtls_printf( " option pack only defined if MBEDTLS_TIMING_C is enabled\n" ); exit( 1 ); @@ -286,7 +286,7 @@ static const char *msg_type( unsigned char *msg, size_t len ) #if defined(MBEDTLS_TIMING_C) /* Return elapsed time in milliseconds since the first call */ -static unsigned long ellapsed_time( void ) +static unsigned ellapsed_time( void ) { static int initialized = 0; static struct mbedtls_timing_hr_time hires; @@ -307,8 +307,8 @@ typedef struct const char *description; - unsigned long packet_lifetime; - size_t num_datagrams; + unsigned packet_lifetime; + unsigned num_datagrams; unsigned char data[MAX_MSG_SIZE]; unsigned len; @@ -321,8 +321,9 @@ static int ctx_buffer_flush( ctx_buffer *buf ) { int ret; - mbedtls_printf( " %05lu flush %s: %u bytes, %lu datagrams, last %ld ms\n", - ellapsed_time(), buf->description, buf->len, buf->num_datagrams, + mbedtls_printf( " %05u flush %s: %u bytes, %u datagrams, last %u ms\n", + ellapsed_time(), buf->description, + buf->len, buf->num_datagrams, ellapsed_time() - buf->packet_lifetime ); ret = mbedtls_net_send( buf->ctx, buf->data, buf->len ); @@ -333,15 +334,17 @@ static int ctx_buffer_flush( ctx_buffer *buf ) return( ret ); } -static inline int ctx_buffer_check( ctx_buffer *buf ) +static unsigned ctx_buffer_time_remaining( ctx_buffer *buf ) { - if( buf->len > 0 && - ellapsed_time() - buf->packet_lifetime >= (size_t) opt.pack ) - { - return( ctx_buffer_flush( buf ) ); - } + unsigned const cur_time = ellapsed_time(); - return( 0 ); + if( buf->num_datagrams == 0 ) + return( (unsigned) -1 ); + + if( cur_time - buf->packet_lifetime >= opt.pack ) + return( 0 ); + + return( opt.pack - ( cur_time - buf->packet_lifetime ) ); } static int ctx_buffer_append( ctx_buffer *buf, @@ -352,8 +355,8 @@ static int ctx_buffer_append( ctx_buffer *buf, if( len > sizeof( buf->data ) ) { - mbedtls_printf( " ! buffer size %lu too large (max %lu)\n", - len, sizeof( buf->data ) ); + mbedtls_printf( " ! buffer size %u too large (max %u)\n", + (unsigned) len, (unsigned) sizeof( buf->data ) ); return( -1 ); } @@ -371,35 +374,31 @@ static int ctx_buffer_append( ctx_buffer *buf, return( len ); } +#endif /* MBEDTLS_TIMING_C */ static int dispatch_data( mbedtls_net_context *ctx, const unsigned char * data, size_t len ) { +#if defined(MBEDTLS_TIMING_C) ctx_buffer *buf = NULL; + if( opt.pack > 0 ) + { + if( outbuf[0].ctx == ctx ) + buf = &outbuf[0]; + else if( outbuf[1].ctx == ctx ) + buf = &outbuf[1]; - if( outbuf[0].ctx == ctx ) - buf = &outbuf[0]; - else if( outbuf[1].ctx == ctx ) - buf = &outbuf[1]; + if( buf == NULL ) + return( -1 ); - if( buf == NULL ) - return( mbedtls_net_send( ctx, data, len ) ); + return( ctx_buffer_append( buf, data, len ) ); + } +#endif /* MBEDTLS_TIMING_C */ - return( ctx_buffer_append( buf, data, len ) ); -} - -#else /* MBEDTLS_TIMING_C */ - -static int dispatch_data( mbedtls_net_context *ctx, - const unsigned char * data, - size_t len ) -{ return( mbedtls_net_send( ctx, data, len ) ); } -#endif /* MBEDTLS_TIMING_C */ - typedef struct { mbedtls_net_context *dst; @@ -414,10 +413,10 @@ void print_packet( const packet *p, const char *why ) { #if defined(MBEDTLS_TIMING_C) if( why == NULL ) - mbedtls_printf( " %05lu dispatch %s %s (%u bytes)\n", + mbedtls_printf( " %05u dispatch %s %s (%u bytes)\n", ellapsed_time(), p->way, p->type, p->len ); else - mbedtls_printf( " %05lu dispatch %s %s (%u bytes): %s\n", + mbedtls_printf( " %05u dispatch %s %s (%u bytes): %s\n", ellapsed_time(), p->way, p->type, p->len, why ); #else if( why == NULL ) @@ -601,14 +600,16 @@ int main( int argc, char *argv[] ) int ret; mbedtls_net_context listen_fd, client_fd, server_fd; + +#if defined( MBEDTLS_TIMING_C ) struct timeval tm; +#endif + + struct timeval *tm_ptr = NULL; int nb_fds; fd_set read_fds; - tm.tv_sec = 0; - tm.tv_usec = 0; - mbedtls_net_init( &listen_fd ); mbedtls_net_init( &client_fd ); mbedtls_net_init( &server_fd ); @@ -707,26 +708,52 @@ accept: outbuf[1].num_datagrams = 0; outbuf[1].len = 0; } - else - { - outbuf[0].ctx = NULL; - outbuf[1].ctx = NULL; - } #endif /* MBEDTLS_TIMING_C */ while( 1 ) { +#if defined(MBEDTLS_TIMING_C) + if( opt.pack > 0 ) + { + unsigned max_wait_server, max_wait_client, max_wait; + max_wait_server = ctx_buffer_time_remaining( &outbuf[0] ); + max_wait_client = ctx_buffer_time_remaining( &outbuf[1] ); + + max_wait = (unsigned) -1; + + if( max_wait_server == 0 ) + ctx_buffer_flush( &outbuf[0] ); + else + max_wait = max_wait_server; + + if( max_wait_client == 0 ) + ctx_buffer_flush( &outbuf[1] ); + else + { + if( max_wait_client < max_wait ) + max_wait = max_wait_client; + } + + if( max_wait != (unsigned) -1 ) + { + tm.tv_sec = max_wait / 1000; + tm.tv_usec = ( max_wait % 1000 ) * 1000; + + tm_ptr = &tm; + } + else + { + tm_ptr = NULL; + } + } +#endif /* MBEDTLS_TIMING_C */ + FD_ZERO( &read_fds ); FD_SET( server_fd.fd, &read_fds ); FD_SET( client_fd.fd, &read_fds ); FD_SET( listen_fd.fd, &read_fds ); -#if defined(MBEDTLS_TIMING_C) - ctx_buffer_check( &outbuf[0] ); - ctx_buffer_check( &outbuf[1] ); -#endif - - if( ( ret = select( nb_fds, &read_fds, NULL, NULL, &tm ) ) < 0 ) + if( ( ret = select( nb_fds, &read_fds, NULL, NULL, tm_ptr ) ) < 0 ) { perror( "select" ); goto exit; From 298a7b214dd2cb8f7f82a90c1638e00f34f46807 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 6 Nov 2017 10:45:26 +0000 Subject: [PATCH 34/55] Change wording of directions on the usage of SSL context after error --- include/mbedtls/ssl.h | 62 +++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 594c7d6b1..cf98a3cc6 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2418,10 +2418,10 @@ int mbedtls_ssl_get_session( const mbedtls_ssl_context *ssl, mbedtls_ssl_session * DTLS records. * * \note If this function returns something other than 0 or - * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. * * \note If DTLS is in use, then you may choose to handle * MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED specially for logging @@ -2438,10 +2438,10 @@ int mbedtls_ssl_handshake( mbedtls_ssl_context *ssl ); * call this function if state is MBEDTLS_SSL_HANDSHAKE_OVER. * * \note If this function returns something other than 0 or - * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. * * \param ssl SSL context * @@ -2465,10 +2465,10 @@ int mbedtls_ssl_handshake_step( mbedtls_ssl_context *ssl ); * value. * * \note If this function returns something other than 0 or - * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. */ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_RENEGOTIATION */ @@ -2507,12 +2507,12 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * again, or not transmitting the new identity to the * application layer, would allow authentication bypass! * - * If this function returns something other than a positive - * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE or - * MBEDTLS_ERR_SSL_CLIENT_RECONNECT, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection. + * \note If this function returns something other than a positive value + * or MBEDTLS_ERR_SSL_WANT_READ/WRITE or MBEDTLS_ERR_SSL_CLIENT_RECONNECT, + * you must stop using the SSL context for reading or writing, + * and either free it or call \c mbedtls_ssl_session_reset() on it + * before re-using it for a new connection; the current connection + * must be closed. * * \note Remarks regarding event-driven DTLS: * - If the function returns MBEDTLS_ERR_SSL_WANT_READ, no datagram @@ -2548,11 +2548,11 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) * or MBEDTLS_ERR_SSL_WANT_WRITE or MBEDTLS_ERR_SSL_WANT_READ, * or another negative error code. * - * \note If this function returns something other than a positive - * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE, the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * \note If this function returns something other than a positive value + * or MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. * * \note When this function returns MBEDTLS_ERR_SSL_WANT_WRITE/READ, * it must be called later with the *same* arguments, @@ -2579,10 +2579,10 @@ int mbedtls_ssl_write( mbedtls_ssl_context *ssl, const unsigned char *buf, size_ * \return 0 if successful, or a specific SSL error code. * * \note If this function returns something other than 0 or - * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. */ int mbedtls_ssl_send_alert_message( mbedtls_ssl_context *ssl, unsigned char level, @@ -2595,10 +2595,10 @@ int mbedtls_ssl_send_alert_message( mbedtls_ssl_context *ssl, * \return 0 if successful, or a specific SSL error code. * * \note If this function returns something other than 0 or - * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context - * becomes unusable, and you should either free it or call - * \c mbedtls_ssl_session_reset() on it before re-using it for - * a new connection; the current connection must be closed. + * MBEDTLS_ERR_SSL_WANT_READ/WRITE, you must stop using + * the SSL context for reading or writing, and either free it or + * call \c mbedtls_ssl_session_reset() on it before re-using it + * for a new connection; the current connection must be closed. */ int mbedtls_ssl_close_notify( mbedtls_ssl_context *ssl ); From 05c4fc860805c1ffd7c8f3c42eb475105c07d05c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 9 Nov 2017 14:34:06 +0000 Subject: [PATCH 35/55] Correct typo in debugging message --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a0c19c936..abd6f09d0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3752,7 +3752,7 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl ) if( 0 != ret ) { - MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_read_record_layer" ), ret ); + MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_handle_message_type" ), ret ); return( ret ); } From 000767123f640648158e4c61564826c9969352ed Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 15 Nov 2017 16:39:08 +0000 Subject: [PATCH 36/55] Add tests for event-driven I/O --- tests/ssl-opt.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 5078c0bcd..2ff411092 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2469,6 +2469,64 @@ run_test "Non-blocking I/O: session-id resume" \ -C "mbedtls_ssl_handshake returned" \ -c "Read from server: .* bytes read" +# Tests for event-driven I/O: exercise a variety of handshake flows + +run_test "Event-driven I/O: basic handshake" \ + "$P_SRV event=1 tickets=0 auth_mode=none" \ + "$P_CLI event=1 tickets=0" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: client auth" \ + "$P_SRV event=1 tickets=0 auth_mode=required" \ + "$P_CLI event=1 tickets=0" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: ticket" \ + "$P_SRV event=1 tickets=1 auth_mode=none" \ + "$P_CLI event=1 tickets=1" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: ticket + client auth" \ + "$P_SRV event=1 tickets=1 auth_mode=required" \ + "$P_CLI event=1 tickets=1" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: ticket + client auth + resume" \ + "$P_SRV event=1 tickets=1 auth_mode=required" \ + "$P_CLI event=1 tickets=1 reconnect=1" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: ticket + resume" \ + "$P_SRV event=1 tickets=1 auth_mode=none" \ + "$P_CLI event=1 tickets=1 reconnect=1" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O: session-id resume" \ + "$P_SRV event=1 tickets=0 auth_mode=none" \ + "$P_CLI event=1 tickets=0 reconnect=1" \ + 0 \ + -S "mbedtls_ssl_handshake returned" \ + -C "mbedtls_ssl_handshake returned" \ + -c "Read from server: .* bytes read" + # Tests for version negotiation run_test "Version check: all -> 1.2" \ From 72a4f0338d08712209c909d7cdf9853d2cb4d3cf Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 15 Nov 2017 16:39:20 +0000 Subject: [PATCH 37/55] Add tests for UDP proxy packing option --- tests/ssl-opt.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2ff411092..34aa43f99 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -3777,6 +3777,22 @@ run_test "DTLS proxy: duplicate every packet, server anti-replay off" \ -s "Extra-header:" \ -c "HTTP/1.0 200 OK" +run_test "DTLS proxy: multiple records in same datagram" \ + -p "$P_PXY pack=10" \ + "$P_SRV dtls=1 debug_level=2" \ + "$P_CLI dtls=1 debug_level=2" \ + 0 \ + -c "next record in same datagram" \ + -s "next record in same datagram" + +run_test "DTLS proxy: multiple records in same datagram, duplicate every packet" \ + -p "$P_PXY pack=10 duplicate=1" \ + "$P_SRV dtls=1 debug_level=2" \ + "$P_CLI dtls=1 debug_level=2" \ + 0 \ + -c "next record in same datagram" \ + -s "next record in same datagram" + run_test "DTLS proxy: inject invalid AD record, default badmac_limit" \ -p "$P_PXY bad_ad=1" \ "$P_SRV dtls=1 debug_level=1" \ From 6e5dd79a437c5ea899b0c14d256caeb4e5f0a1ce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 28 Nov 2017 14:34:04 +0000 Subject: [PATCH 38/55] Fix compilation warning on MSVC MSVC complains about the negation in `(uint32_t) -1u`. This commit fixes this by using `(uint32_t) -1` instead. --- library/net_sockets.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index edd084416..2d1c1082a 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -471,7 +471,7 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) tv.tv_usec = ( timeout % 1000 ) * 1000; ret = select( fd + 1, &read_fds, &write_fds, NULL, - timeout == (uint32_t) -1u ? NULL : &tv ); + timeout == (uint32_t) -1 ? NULL : &tv ); if( ret < 0 ) return( MBEDTLS_ERR_NET_POLL_FAILED ); From a5e68979cabc0883935fafd3e5cc86418e6a4239 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 6 Dec 2017 08:35:02 +0000 Subject: [PATCH 39/55] Resolve integer type conversion problem on MSVC MSVC rightfully complained that there was some conversion from `size_t` to `unsigned int` that could come with a loss of data. This commit re-types the corresponding struct field `ctx_buffer::len` to `size_t`. Also, the function `ctx_buffer_append` has an integer return value which is supposed to be the (positive) length of the appended data on success, and a check is inserted that the data to be appended does not exceed MAX_INT in length. --- programs/test/udp_proxy.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 0dec40932..5797f3d69 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -311,7 +311,7 @@ typedef struct unsigned num_datagrams; unsigned char data[MAX_MSG_SIZE]; - unsigned len; + size_t len; } ctx_buffer; @@ -323,7 +323,7 @@ static int ctx_buffer_flush( ctx_buffer *buf ) mbedtls_printf( " %05u flush %s: %u bytes, %u datagrams, last %u ms\n", ellapsed_time(), buf->description, - buf->len, buf->num_datagrams, + (unsigned) buf->len, buf->num_datagrams, ellapsed_time() - buf->packet_lifetime ); ret = mbedtls_net_send( buf->ctx, buf->data, buf->len ); @@ -353,6 +353,9 @@ static int ctx_buffer_append( ctx_buffer *buf, { int ret; + if( len > (size_t) INT_MAX ) + return( -1 ); + if( len > sizeof( buf->data ) ) { mbedtls_printf( " ! buffer size %u too large (max %u)\n", @@ -372,7 +375,7 @@ static int ctx_buffer_append( ctx_buffer *buf, if( ++buf->num_datagrams == 1 ) buf->packet_lifetime = ellapsed_time(); - return( len ); + return( (int) len ); } #endif /* MBEDTLS_TIMING_C */ From 62dcbaf567e9f015fc533d2ef29c39ee9271527b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Mar 2018 10:54:43 +0000 Subject: [PATCH 40/55] Improve crediting in ChangeLog --- ChangeLog | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 6b0fe3ba5..b6f61fa71 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,7 +9,8 @@ Bugfix returned when unexpected messages were being discarded, ignoring that further messages could potentially already be pending to be processed in the internal buffers; these cases lead to deadlocks in case - event-driven I/O was used. Found by Hubert Mis. + event-driven I/O was used. + Found and reported by Hubert Mis in #772. API changes * Add function mbedtls_net_poll to public API allowing to wait for a From 6a33f59f76092fe86094b71ec4a47cfff481d65b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Mar 2018 11:38:46 +0000 Subject: [PATCH 41/55] Add tests for event-driven I/O in DTLS to ssl-opt.sh --- tests/ssl-opt.sh | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 34aa43f99..4c6512142 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2527,6 +2527,47 @@ run_test "Event-driven I/O: session-id resume" \ -C "mbedtls_ssl_handshake returned" \ -c "Read from server: .* bytes read" +run_test "Event-driven I/O, DTLS: basic handshake" \ + "$P_SRV dtls=1 event=1 tickets=0 auth_mode=none" \ + "$P_CLI dtls=1 event=1 tickets=0" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: client auth" \ + "$P_SRV dtls=1 event=1 tickets=0 auth_mode=required" \ + "$P_CLI dtls=1 event=1 tickets=0" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: ticket" \ + "$P_SRV dtls=1 event=1 tickets=1 auth_mode=none" \ + "$P_CLI dtls=1 event=1 tickets=1" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: ticket + client auth" \ + "$P_SRV dtls=1 event=1 tickets=1 auth_mode=required" \ + "$P_CLI dtls=1 event=1 tickets=1" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: ticket + client auth + resume" \ + "$P_SRV dtls=1 event=1 tickets=1 auth_mode=required" \ + "$P_CLI dtls=1 event=1 tickets=1 reconnect=1" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: ticket + resume" \ + "$P_SRV dtls=1 event=1 tickets=1 auth_mode=none" \ + "$P_CLI dtls=1 event=1 tickets=1 reconnect=1" \ + 0 \ + -c "Read from server: .* bytes read" + +run_test "Event-driven I/O, DTLS: session-id resume" \ + "$P_SRV dtls=1 event=1 tickets=0 auth_mode=none" \ + "$P_CLI dtls=1 event=1 tickets=0 reconnect=1" \ + 0 \ + -c "Read from server: .* bytes read" # Tests for version negotiation run_test "Version check: all -> 1.2" \ From ddc3ebbc3f698a6db77da59805c3a02f891454e0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Mar 2018 11:39:09 +0000 Subject: [PATCH 42/55] Exemplify use of `mbedtls_ssl_check_pending` in `ssl_server2.c` --- programs/ssl/ssl_server2.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index c3321d13a..74a114271 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2417,21 +2417,37 @@ data_exchange: while( 1 ) { + /* Without the call to `mbedtls_ssl_check_pending`, it might + * happen that the client sends application data in the same + * datagram as the Finished message concluding the handshake. + * In this case, the application data would be ready to be + * processed while the underlying transport wouldn't signal + * any further incoming data. + * + * See the test 'Event-driven I/O: session-id resume, UDP packing' + * in tests/ssl-opt.sh. + */ + + /* For event-driven IO, wait for socket to become available */ + if( mbedtls_ssl_check_pending( &ssl ) == 0 && + opt.event == 1 /* level triggered IO */ ) + { +#if defined(MBEDTLS_TIMING_C) + idle( &client_fd, &timer, MBEDTLS_ERR_SSL_WANT_READ ); +#else + idle( &client_fd, MBEDTLS_ERR_SSL_WANT_READ ); +#endif + } + ret = mbedtls_ssl_read( &ssl, buf, len ); + /* Note that even if `mbedtls_ssl_check_pending` returns true, + * it can happen that the subsequent call to `mbedtls_ssl_read` + * returns `MBEDTLS_ERR_SSL_WANT_READ`, because the pending messages + * might be discarded (e.g. because they are retransmissions). */ if( ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE ) break; - - /* For event-driven IO, wait for socket to become available */ - if( opt.event == 1 /* level triggered IO */ ) - { -#if defined(MBEDTLS_TIMING_C) - idle( &client_fd, &timer, ret ); -#else - idle( &client_fd, ret ); -#endif - } } if( ret <= 0 ) From bc6c1101399a31058cf93b3f31be65a8d11e4bb2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Mar 2018 11:39:40 +0000 Subject: [PATCH 43/55] Add test to ssl-opt.sh demonstrating the need for ssl_check_pending --- tests/ssl-opt.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4c6512142..4afc527a1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2568,6 +2568,19 @@ run_test "Event-driven I/O, DTLS: session-id resume" \ "$P_CLI dtls=1 event=1 tickets=0 reconnect=1" \ 0 \ -c "Read from server: .* bytes read" + +# This test demonstrates the need for the mbedtls_ssl_check_pending function. +# During session resumption, the client will send its ApplicationData record +# within the same datagram as the Finished messages. In this situation, the +# server MUST NOT idle on the underlying transport after handshake completion, +# because the ApplicationData request has already been queued internally. +run_test "Event-driven I/O, DTLS: session-id resume, UDP packing" \ + -p "$P_PXY pack=10" \ + "$P_SRV dtls=1 event=1 tickets=0 auth_mode=required" \ + "$P_CLI dtls=1 event=1 tickets=0 reconnect=1" \ + 0 \ + -c "Read from server: .* bytes read" + # Tests for version negotiation run_test "Version check: all -> 1.2" \ From b6f880b63bc6afd192bd280fc178484b77cf710b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 13 Mar 2018 12:48:37 +0000 Subject: [PATCH 44/55] Revert whitespace change to ease merging --- programs/ssl/ssl_client2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 289920cbd..3d03269e6 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -661,8 +661,7 @@ int main( int argc, char *argv[] ) else if( strcmp( p, "request_size" ) == 0 ) { opt.request_size = atoi( q ); - if( opt.request_size < 0 || - opt.request_size > MBEDTLS_SSL_MAX_CONTENT_LEN ) + if( opt.request_size < 0 || opt.request_size > MBEDTLS_SSL_MAX_CONTENT_LEN ) goto usage; } else if( strcmp( p, "ca_file" ) == 0 ) From 7b6582b63196bd18f5dccdcaebd9a6bd97858aa3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 09:37:27 +0000 Subject: [PATCH 45/55] Kill server and proxy via SIGQUIT in ssl-opt.sh SIGKILL interferes with memory checking in valgrind. --- tests/ssl-opt.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4afc527a1..8f64e5423 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -449,7 +449,7 @@ run_test() { kill $SRV_PID sleep 0.01 if kill -0 $SRV_PID >/dev/null 2>&1; then - kill -KILL $SRV_PID + kill -3 $SRV_PID wait $SRV_PID fi @@ -457,7 +457,7 @@ run_test() { kill $PXY_PID >/dev/null 2>&1 sleep 0.01 if kill -0 $PXY_PID >/dev/null 2>&1; then - kill -KILL $PXY_PID + kill -3 $PXY_PID wait $PXY_PID fi fi From 8d83218b702e78e1b403a8a3c2bb0abd9bd2a51a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 10:14:19 +0000 Subject: [PATCH 46/55] Increase UDP record packing time in ssl-opt.sh The UDP tests involving the merging of multiple records into single datagrams accumulate records for 10ms, which can be less than the total flight preparation time if e.g. the tests are being run with valgrind. This commit increases the packing time for the relevant tests from 10ms to 50ms. --- tests/ssl-opt.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 8f64e5423..a1155e8d0 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2575,7 +2575,7 @@ run_test "Event-driven I/O, DTLS: session-id resume" \ # server MUST NOT idle on the underlying transport after handshake completion, # because the ApplicationData request has already been queued internally. run_test "Event-driven I/O, DTLS: session-id resume, UDP packing" \ - -p "$P_PXY pack=10" \ + -p "$P_PXY pack=50" \ "$P_SRV dtls=1 event=1 tickets=0 auth_mode=required" \ "$P_CLI dtls=1 event=1 tickets=0 reconnect=1" \ 0 \ @@ -3832,7 +3832,7 @@ run_test "DTLS proxy: duplicate every packet, server anti-replay off" \ -c "HTTP/1.0 200 OK" run_test "DTLS proxy: multiple records in same datagram" \ - -p "$P_PXY pack=10" \ + -p "$P_PXY pack=50" \ "$P_SRV dtls=1 debug_level=2" \ "$P_CLI dtls=1 debug_level=2" \ 0 \ @@ -3840,7 +3840,7 @@ run_test "DTLS proxy: multiple records in same datagram" \ -s "next record in same datagram" run_test "DTLS proxy: multiple records in same datagram, duplicate every packet" \ - -p "$P_PXY pack=10 duplicate=1" \ + -p "$P_PXY pack=50 duplicate=1" \ "$P_SRV dtls=1 debug_level=2" \ "$P_CLI dtls=1 debug_level=2" \ 0 \ From adfa64f0c4d99db9cf08ad927843c564209e8506 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 11:35:07 +0000 Subject: [PATCH 47/55] 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 3d03269e6..023c0c5d1 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 74a114271..e29633972 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 a1155e8d0..1682a8476 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 From 9ac640326b5ec7bf1140cc542a91b61d10ba2d51 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 12:19:31 +0000 Subject: [PATCH 48/55] Don't exit mbedtls_net_poll on interruption of select If the select UNIX system call is interrupted by a signal handler, it is not automatically restarted but returns EINTR. This commit modifies the use of select in mbedtls_net_poll from net_sockets.c to retry the select call in this case. --- library/net_sockets.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 2d1c1082a..e63e496b9 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -470,8 +470,11 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) tv.tv_sec = timeout / 1000; tv.tv_usec = ( timeout % 1000 ) * 1000; - ret = select( fd + 1, &read_fds, &write_fds, NULL, - timeout == (uint32_t) -1 ? NULL : &tv ); + do + { + ret = select( fd + 1, &read_fds, &write_fds, NULL, + timeout == (uint32_t) -1 ? NULL : &tv ); + } while( ret == EINTR ); if( ret < 0 ) return( MBEDTLS_ERR_NET_POLL_FAILED ); From 9b2b66ebd250e63e51c87d9b75fd67bad4e1e8f9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 12:21:15 +0000 Subject: [PATCH 49/55] Minor style corrections Move function block brace outside conditional compilation to not confuse some editors, and correct indentation. --- programs/ssl/ssl_client2.c | 9 ++++----- programs/ssl/ssl_server2.c | 10 ++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 023c0c5d1..232dc6445 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -445,14 +445,13 @@ static int ssl_sig_hashes_for_test[] = { */ #if !defined(MBEDTLS_TIMING_C) int idle( mbedtls_net_context *fd, - int idle_reason ) -{ + int idle_reason ) #else int idle( mbedtls_net_context *fd, - mbedtls_timing_delay_context *timer, - int idle_reason ) -{ + mbedtls_timing_delay_context *timer, + int idle_reason ) #endif +{ int ret; int poll_type = 0; diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index e29633972..3a6b9dcf1 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -847,15 +847,13 @@ static int ssl_sig_hashes_for_test[] = { */ #if !defined(MBEDTLS_TIMING_C) int idle( mbedtls_net_context *fd, - int idle_reason ) -{ + int idle_reason ) #else int idle( mbedtls_net_context *fd, - mbedtls_timing_delay_context *timer, - int idle_reason ) -{ + mbedtls_timing_delay_context *timer, + int idle_reason ) #endif - +{ int ret; int poll_type = 0; From 80e06d77d95329c1a43e7d9dba73e289bdeec1ec Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 14:41:55 +0000 Subject: [PATCH 50/55] Use WSAEINTR instead of EINTR on Windows --- library/net_sockets.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index e63e496b9..96cfa35cd 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -474,7 +474,13 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) { ret = select( fd + 1, &read_fds, &write_fds, NULL, timeout == (uint32_t) -1 ? NULL : &tv ); - } while( ret == EINTR ); + } +#if ( defined(_WIN32) || defined(_WIN32_WCE) ) && !defined(EFIX64) && \ + !defined(EFI32) + while( ret == WSAEINTR ); +#else + while( ret == EINTR ); +#endif if( ret < 0 ) return( MBEDTLS_ERR_NET_POLL_FAILED ); From ef52796537c89bfb06d4eb5daecab7d013a57749 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 15 Mar 2018 15:49:24 +0000 Subject: [PATCH 51/55] Fix missing return statement ssl_server2 idling Also, introduce MBEDTLS_EINTR locally in net_sockets.c for the platform-dependent return code macro used by the `select` call to indicate that the poll was interrupted by a signal handler: On Unix, the corresponding macro is EINTR, while on Windows, it's WSAEINTR. --- library/net_sockets.c | 11 +++++------ programs/ssl/ssl_client2.c | 2 +- programs/ssl/ssl_server2.c | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 96cfa35cd..10b5456be 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -45,6 +45,8 @@ #if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(EFIX64) && \ !defined(EFI32) +#define MBEDTLS_EINTR WSAEINTR + #ifdef _WIN32_WINNT #undef _WIN32_WINNT #endif @@ -82,6 +84,8 @@ static int wsa_init_done = 0; #include #include +#define MBEDTLS_EINTR EINTR + #endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */ /* Some MS functions want int and MSVC warns if we pass size_t, @@ -475,12 +479,7 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) ret = select( fd + 1, &read_fds, &write_fds, NULL, timeout == (uint32_t) -1 ? NULL : &tv ); } -#if ( defined(_WIN32) || defined(_WIN32_WCE) ) && !defined(EFIX64) && \ - !defined(EFI32) - while( ret == WSAEINTR ); -#else - while( ret == EINTR ); -#endif + while( ret == MBEDTLS_EINTR ); if( ret < 0 ) return( MBEDTLS_ERR_NET_POLL_FAILED ); diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 232dc6445..58f12c986 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -462,7 +462,7 @@ int idle( mbedtls_net_context *fd, poll_type = MBEDTLS_NET_POLL_READ; #if !defined(MBEDTLS_TIMING_C) else - return; + return( 0 ); #endif while( 1 ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 3a6b9dcf1..ed38a321b 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -863,7 +863,7 @@ int idle( mbedtls_net_context *fd, poll_type = MBEDTLS_NET_POLL_READ; #if !defined(MBEDTLS_TIMING_C) else - return; + return( 0 ); #endif while( 1 ) From c9f4d6d44899a26f001c28b93e86cf7d6452e693 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 19 Mar 2018 09:23:13 +0000 Subject: [PATCH 52/55] Correct error.c --- library/error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/error.c b/library/error.c index c42642467..63cabb1f9 100644 --- a/library/error.c +++ b/library/error.c @@ -440,7 +440,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) if( use_ret == -(MBEDTLS_ERR_SSL_INVALID_VERIFY_HASH) ) mbedtls_snprintf( buf, buflen, "SSL - Couldn't set the hash for verifying CertificateVerify" ); if( use_ret == -(MBEDTLS_ERR_SSL_CONTINUE_PROCESSING) ) - mbedtls_snprintf( buf, buflen, "SSL - Internal-only message signalling that further message-processing should be done" ); + mbedtls_snprintf( buf, buflen, "SSL - Internal-only message signaling that further message-processing should be done" ); #endif /* MBEDTLS_SSL_TLS_C */ #if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C) From ea7dbbe0de008c1844f8f48fde08542e6a222fcc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 15 Mar 2018 23:25:21 +0100 Subject: [PATCH 53/55] Replace MBEDTLS_EINTR by IS_EINTR check-names.sh reserves the prefix MBEDTLS_ for macros defined in config.h so this name (or check-names.sh) had to change. This is also more flexible because it allows for platforms that don't have an EINTR equivalent or have multiple such values. --- library/net_sockets.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 10b5456be..6ce9eee7b 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -45,7 +45,7 @@ #if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(EFIX64) && \ !defined(EFI32) -#define MBEDTLS_EINTR WSAEINTR +#define IS_EINTR( ret ) ( ( ret ) == WSAEINTR ) #ifdef _WIN32_WINNT #undef _WIN32_WINNT @@ -84,7 +84,7 @@ static int wsa_init_done = 0; #include #include -#define MBEDTLS_EINTR EINTR +#define IS_EINTR( ret ) ( ( ret ) == EINTR ) #endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */ @@ -479,7 +479,7 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) ret = select( fd + 1, &read_fds, &write_fds, NULL, timeout == (uint32_t) -1 ? NULL : &tv ); } - while( ret == MBEDTLS_EINTR ); + while( IS_EINTR( ret ) ); if( ret < 0 ) return( MBEDTLS_ERR_NET_POLL_FAILED ); From f4e5b7e87de2484f0e3dbb9d11e87dd275874cd0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 3 Apr 2018 16:28:09 +0100 Subject: [PATCH 54/55] Additionally initialize fd_set's via memset in mbedtls_net_poll The initialization via FD_SET is not seen by memory sanitizers if FD_SET is implemented through assembly. Additionally zeroizing the respective fd_set's before calling FD_SET contents the sanitizers and comes at a negligible computational overhead. --- library/net_sockets.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index cdc237642..f99d339ff 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -275,7 +275,7 @@ static int net_would_block( const mbedtls_net_context *ctx ) static int net_would_block( const mbedtls_net_context *ctx ) { int err = errno; - + /* * Never return 'WOULD BLOCK' on a non-blocking socket */ @@ -459,6 +459,12 @@ int mbedtls_net_poll( mbedtls_net_context *ctx, uint32_t rw, uint32_t timeout ) if( fd < 0 ) return( MBEDTLS_ERR_NET_INVALID_CONTEXT ); + /* Ensure that memory sanitizers consider + * read_fds and write_fds as initialized even + * if FD_ZERO is implemented in assembly. */ + memset( &read_fds, 0, sizeof( read_fds ) ); + memset( &write_fds, 0, sizeof( write_fds ) ); + FD_ZERO( &read_fds ); if( rw & MBEDTLS_NET_POLL_READ ) { From e4d3b7f86074e32d907867d72ee52507b47b3457 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 4 Apr 2018 09:28:48 +0200 Subject: [PATCH 55/55] Fix merge glitch in ChangeLog --- ChangeLog | 1 - 1 file changed, 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 70e9bb679..b772c3fd2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -326,7 +326,6 @@ Changes implementation. * Add explicit warnings for the use of MD2, MD4, MD5, SHA-1, DES and ARC4 throughout the library. ->>>>>>> development = mbed TLS 2.6.0 branch released 2017-08-10