diff --git a/ChangeLog b/ChangeLog index bcceebb7d..917c521cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,14 @@ New deprecations * Deprecate MBEDTLS_SSL_HW_RECORD_ACCEL that enables function hooks in the SSL module for hardware acceleration of individual records. +Security + * Fix issue in DTLS handling of new associations with the same parameters + (RFC 6347 section 4.2.8): an attacker able to send forged UDP packets to + the server could cause it to drop established associations with + legitimate clients, resulting in 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_msg.c b/library/ssl_msg.c index 18fa55574..428ace5ba 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3197,16 +3197,17 @@ static int ssl_check_dtls_clihlo_cookie( * that looks like a ClientHello. * * - if the input looks like a ClientHello without cookies, - * send back HelloVerifyRequest, then - * return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED + * send back HelloVerifyRequest, then return 0 * - if the input looks like a ClientHello with a valid cookie, * reset the session of the current context, and * return MBEDTLS_ERR_SSL_CLIENT_RECONNECT * - if anything goes wrong, return a specific error code * - * mbedtls_ssl_read_record() will ignore the record if anything else than - * MBEDTLS_ERR_SSL_CLIENT_RECONNECT or 0 is returned, although this function - * cannot not return 0. + * This function is called (through ssl_check_client_reconnect()) when an + * unexpected record is found in ssl_get_next_record(), which will discard the + * record if we return 0, and bubble up the return value otherwise (this + * includes the case of MBEDTLS_ERR_SSL_CLIENT_RECONNECT and of unexpected + * errors, and is the right thing to do in both cases). */ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) { @@ -3218,6 +3219,8 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) { /* If we can't use cookies to verify reachability of the peer, * drop the record. */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no cookie callbacks, " + "can't check reconnect validity" ) ); return( 0 ); } @@ -3233,16 +3236,23 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) 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_buf, 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 ); - ret = 0; + send_ret = ssl->f_send( ssl->p_bio, ssl->out_buf, len ); + MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", send_ret ); + (void) send_ret; + + return( 0 ); } if( ret == 0 ) { - /* Got a valid cookie, partially reset context */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "cookie is valid, resetting context" ) ); if( ( ret = mbedtls_ssl_session_reset_int( ssl, 1 ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "reset", ret ); @@ -4415,6 +4425,7 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) ssl->in_msglen = rec.data_len; ret = ssl_check_client_reconnect( ssl ); + MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_client_reconnect", ret ); if( ret != 0 ) return( ret ); #endif diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 979910e6b..7447571f4 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -133,6 +133,7 @@ int main( void ) " modifying CID in first instance of the packet.\n" \ " protect_hvr=0/1 default: 0 (don't protect HelloVerifyRequest)\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" \ USAGE_PACK \ @@ -166,6 +167,7 @@ static struct options unsigned bad_cid; /* inject corrupted CID 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 pack; /* merge packets into single datagram for * at most \c merge milliseconds if > 0 */ unsigned int seed; /* seed for "random" events */ @@ -314,6 +316,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 ); @@ -523,11 +531,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 CID record? */ if( opt.bad_cid != 0 && strcmp( p->type, "CID" ) == 0 && @@ -592,6 +630,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 = dispatch_data( dst, initial_clihlo.buf, + initial_clihlo.len ) ) <= 0 ) + { + mbedtls_printf( " ! dispatch 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 35f742f67..9b6eee152 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7279,8 +7279,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" \ @@ -7288,8 +7288,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" \ @@ -7318,6 +7318,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) @@ -8387,8 +8395,8 @@ run_test "DTLS fragmenting: 3d, openssl client, DTLS 1.0" \ 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" \ @@ -8405,8 +8413,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 dgram_packing=0 debug_level=2" \ - "$P_CLI dtls=1 dgram_packing=0 debug_level=2" \ + "$P_SRV dtls=1 dgram_packing=0 debug_level=2 hs_timeout=10000-20000" \ + "$P_CLI dtls=1 dgram_packing=0 debug_level=2 hs_timeout=10000-20000" \ 0 \ -c "replayed record" \ -s "replayed record" \