Merge pull request #3145 from mpg/fix-reconnect-2.7

[backport 2.7] Fix issues in handling of client reconnecting from the same port

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.

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 );

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 );
 }
 

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" \