diff --git a/ChangeLog b/ChangeLog index b4d2d4481..9e0abd76f 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 +Security + * Fix bug in DTLS handling of new associations with the same parameters + (RFC 6347 section 4.2.8): after sending its HelloVerifyRequest, the + server would end up with corrupted state and only send invalid records to + the client. An attacker able to send forged UDP packets to the server + could use that to obtain a Denial of Service. This could only happen when + MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE was enabled in config.h (which it is + by default). + Bugfix * Fix compilation failure when both MBEDTLS_SSL_PROTO_DTLS and MBEDTLS_SSL_HW_RECORD_ACCEL are enabled. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1188e5399..5f6ae6200 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3598,29 +3598,38 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) int ret; size_t len; + /* Use out_msg as temporary buffer for writing out HelloVerifyRequest, + * because the output buffer's already around. Don't use out_buf though, + * as we don't want to overwrite out_ctr. */ ret = ssl_check_dtls_clihlo_cookie( ssl->conf->f_cookie_write, ssl->conf->f_cookie_check, ssl->conf->p_cookie, ssl->cli_id, ssl->cli_id_len, ssl->in_buf, ssl->in_left, - ssl->out_buf, MBEDTLS_SSL_MAX_CONTENT_LEN, &len ); + ssl->out_msg, MBEDTLS_SSL_MAX_CONTENT_LEN, &len ); MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_dtls_clihlo_cookie", ret ); if( ret == MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED ) { + int send_ret; + MBEDTLS_SSL_DEBUG_MSG( 1, ( "sending HelloVerifyRequest" ) ); + MBEDTLS_SSL_DEBUG_BUF( 4, "output record sent to network", + ssl->out_msg, len ); /* Don't check write errors as we can't do anything here. * If the error is permanent we'll catch it later, * if it's not, then hopefully it'll work next time. */ - (void) ssl->f_send( ssl->p_bio, ssl->out_buf, len ); + send_ret = ssl->f_send( ssl->p_bio, ssl->out_msg, len ); + MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", send_ret ); + (void) send_ret; return( MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED ); } if( ret == 0 ) { - /* Got a valid cookie, partially reset context */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "cookie is valid, resetting context" ) ); if( ( ret = ssl_session_reset_int( ssl, 1 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "reset", ret ); diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 02428b9dd..5b5809d79 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -107,7 +107,8 @@ 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" \ + " inject_clihlo=0/1 default: 0 (don't inject fake ClientHello)\n" \ "\n" \ " seed=%%d default: (use current time)\n" \ "\n" @@ -130,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 inject_clihlo; /* inject fake ClientHello after handshake */ unsigned int seed; /* seed for "random" events */ } opt; @@ -219,6 +220,12 @@ static void get_options( int argc, char *argv[] ) if( opt.protect_len < 0 ) exit_usage( p, q ); } + else if( strcmp( p, "inject_clihlo" ) == 0 ) + { + opt.inject_clihlo = atoi( q ); + if( opt.inject_clihlo < 0 || opt.inject_clihlo > 1 ) + exit_usage( p, q ); + } else if( strcmp( p, "seed" ) == 0 ) { opt.seed = atoi( q ); @@ -311,11 +318,41 @@ void print_packet( const packet *p, const char *why ) fflush( stdout ); } +/* + * In order to test the server's behaviour when receiving a ClientHello after + * the connection is established (this could be a hard reset from the client, + * but the server must not drop the existing connection before establishing + * client reachability, see RFC 6347 Section 4.2.8), we memorize the first + * ClientHello we see (which can't have a cookie), then replay it after the + * first ApplicationData record - then we're done. + * + * This is controlled by the inject_clihlo option. + * + * We want an explicit state and a place to store the packet. + */ +typedef enum { + ICH_INIT, /* haven't seen the first ClientHello yet */ + ICH_CACHED, /* cached the initial ClientHello */ + ICH_INJECTED, /* ClientHello already injected, done */ +} inject_clihlo_state_t; + +static inject_clihlo_state_t inject_clihlo_state; +static packet initial_clihlo; + int send_packet( const packet *p, const char *why ) { int ret; mbedtls_net_context *dst = p->dst; + /* save initial ClientHello? */ + if( opt.inject_clihlo != 0 && + inject_clihlo_state == ICH_INIT && + strcmp( p->type, "ClientHello" ) == 0 ) + { + memcpy( &initial_clihlo, p, sizeof( packet ) ); + inject_clihlo_state = ICH_CACHED; + } + /* insert corrupted ApplicationData record? */ if( opt.bad_ad && strcmp( p->type, "ApplicationData" ) == 0 ) @@ -353,6 +390,23 @@ int send_packet( const packet *p, const char *why ) } } + /* Inject ClientHello after first ApplicationData */ + if( opt.inject_clihlo != 0 && + inject_clihlo_state == ICH_CACHED && + strcmp( p->type, "ApplicationData" ) == 0 ) + { + print_packet( &initial_clihlo, "injected" ); + + if( ( ret = mbedtls_net_send( dst, initial_clihlo.buf, + initial_clihlo.len ) ) <= 0 ) + { + mbedtls_printf( " ! mbedtls_net_send returned %d\n", ret ); + return( ret ); + } + + inject_clihlo_state = ICH_INJECTED; + } + return( 0 ); } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1434156ec..f304fdc52 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4972,8 +4972,8 @@ run_test "DTLS cookie: enabled, nbio" \ not_with_valgrind # spurious resend run_test "DTLS client reconnect from same port: reference" \ - "$P_SRV dtls=1 exchanges=2 read_timeout=1000" \ - "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=500-1000" \ + "$P_SRV dtls=1 exchanges=2 read_timeout=20000 hs_timeout=10000-20000" \ + "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=10000-20000" \ 0 \ -C "resend" \ -S "The operation timed out" \ @@ -4981,8 +4981,8 @@ run_test "DTLS client reconnect from same port: reference" \ not_with_valgrind # spurious resend run_test "DTLS client reconnect from same port: reconnect" \ - "$P_SRV dtls=1 exchanges=2 read_timeout=1000" \ - "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=500-1000 reconnect_hard=1" \ + "$P_SRV dtls=1 exchanges=2 read_timeout=20000 hs_timeout=10000-20000" \ + "$P_CLI dtls=1 exchanges=2 debug_level=2 hs_timeout=10000-20000 reconnect_hard=1" \ 0 \ -C "resend" \ -S "The operation timed out" \ @@ -5011,6 +5011,14 @@ run_test "DTLS client reconnect from same port: no cookies" \ -s "The operation timed out" \ -S "Client initiated reconnection from same port" +run_test "DTLS client reconnect from same port: attacker-injected" \ + -p "$P_PXY inject_clihlo=1" \ + "$P_SRV dtls=1 exchanges=2 debug_level=1" \ + "$P_CLI dtls=1 exchanges=2" \ + 0 \ + -s "possible client reconnect from the same port" \ + -S "Client initiated reconnection from same port" + # Tests for various cases of client authentication with DTLS # (focused on handshake flows and message parsing) @@ -5135,8 +5143,8 @@ run_test "DTLS reassembly: fragmentation, nbio (openssl server)" \ not_with_valgrind # spurious resend due to timeout run_test "DTLS proxy: reference" \ -p "$P_PXY" \ - "$P_SRV dtls=1 debug_level=2" \ - "$P_CLI dtls=1 debug_level=2" \ + "$P_SRV dtls=1 debug_level=2 hs_timeout=10000-20000" \ + "$P_CLI dtls=1 debug_level=2 hs_timeout=10000-20000" \ 0 \ -C "replayed record" \ -S "replayed record" \ @@ -5151,8 +5159,8 @@ run_test "DTLS proxy: reference" \ not_with_valgrind # spurious resend due to timeout run_test "DTLS proxy: duplicate every packet" \ -p "$P_PXY duplicate=1" \ - "$P_SRV dtls=1 debug_level=2" \ - "$P_CLI dtls=1 debug_level=2" \ + "$P_SRV dtls=1 debug_level=2 hs_timeout=10000-20000" \ + "$P_CLI dtls=1 debug_level=2 hs_timeout=10000-20000" \ 0 \ -c "replayed record" \ -s "replayed record" \