Merge pull request #3158 from mpg/improve-make-tags-2.7
[backport 2.7] Improve ctags invocation in Makefile
diff --git a/ChangeLog b/ChangeLog
index b4d2d44..9e0abd7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,15 @@
= 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 1188e53..5f6ae62 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -3598,29 +3598,38 @@
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 02428b9..5b5809d 100644
--- a/programs/test/udp_proxy.c
+++ b/programs/test/udp_proxy.c
@@ -107,7 +107,8 @@
" 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 @@
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 @@
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 @@
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 @@
}
}
+ /* 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 1434156..f304fdc 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -4972,8 +4972,8 @@
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 @@
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 @@
-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 @@
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 @@
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" \
diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function
index 042085f..8f134ff 100644
--- a/tests/suites/main_test.function
+++ b/tests/suites/main_test.function
@@ -348,7 +348,7 @@
testfile_index++ )
{
int unmet_dep_count = 0;
- char *unmet_dependencies[20];
+ char *unmet_dependencies[20]; /* only filled when verbose != 0 */
test_filename = test_files[ testfile_index ];
@@ -369,6 +369,7 @@
mbedtls_exit( MBEDTLS_EXIT_FAILURE );
}
unmet_dep_count = 0;
+ memset( unmet_dependencies, 0, sizeof( unmet_dependencies ) );
if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 )
break;
@@ -391,18 +392,16 @@
{
if( dep_check( params[i] ) != DEPENDENCY_SUPPORTED )
{
- if( 0 == option_verbose )
+ if( 0 != option_verbose )
{
- /* Only one count is needed if not verbose */
- unmet_dep_count++;
- break;
- }
-
- unmet_dependencies[ unmet_dep_count ] = strdup(params[i]);
- if( unmet_dependencies[ unmet_dep_count ] == NULL )
- {
- mbedtls_fprintf( stderr, "FATAL: Out of memory\n" );
- mbedtls_exit( MBEDTLS_EXIT_FAILURE );
+ unmet_dependencies[unmet_dep_count] =
+ strdup( params[i] );
+ if( unmet_dependencies[unmet_dep_count] == NULL )
+ {
+ mbedtls_fprintf( stderr,
+ "FATAL: Out of memory\n" );
+ mbedtls_exit( MBEDTLS_EXIT_FAILURE );
+ }
}
unmet_dep_count++;
}
