TLSProxy: Handle partial messages with DTLS

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26532)

diff --git a/test/recipes/70-test_dtls13epoch.t b/test/recipes/70-test_dtls13epoch.t
index 663067bd44b71b9e45d9d7458cf307d4033847ae..6b6d6fd8f0903aaad2c6d640387928b0138cf28d 100644 (file)
--- a/test/recipes/70-test_dtls13epoch.t
+++ b/test/recipes/70-test_dtls13epoch.t
@@ -27,10 +27,6 @@ plan skip_all => "$test_name needs the module feature enabled"
 plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
-# TODO(DTLSv1.3): Implement support for partial messages for DTLS
-plan skip_all => "DTLSProxy does not support partial messages"
-    if disabled("ec");
-
 plan skip_all => "$test_name needs DTLSv1.3 enabled"
     if disabled("dtls1_3");
 

diff --git a/test/recipes/70-test_tls13alerts.t b/test/recipes/70-test_tls13alerts.t
index f3fc5f8464eedc0c17a5426cda31e0e058433130..6848ad355cc921b624e8e803291568ebee63ac40 100644 (file)
--- a/test/recipes/70-test_tls13alerts.t
+++ b/test/recipes/70-test_tls13alerts.t
@@ -41,8 +41,6 @@ SKIP: {
 
 SKIP: {
     skip "DTLS 1.3 is disabled", $testcount if disabled("dtls1_3");
-    skip "DTLSProxy does not support partial messages that are sent when EC is disabled",
-        $testcount if disabled("ec");
     skip "DTLSProxy does not work on Windows", $testcount if $^O =~ /^(MSWin32)$/;
     run_tests(1);
 }

diff --git a/test/recipes/70-test_tls13certcomp.t b/test/recipes/70-test_tls13certcomp.t
index d2f3c09c564fecc4efe55704d2450479c3b131a9..19a0bbdb7571e6868d06ef467d559adb81f6d2a4 100644 (file)
--- a/test/recipes/70-test_tls13certcomp.t
+++ b/test/recipes/70-test_tls13certcomp.t
@@ -283,7 +283,7 @@ sub run_tests
         "Both support certificate compression, but no client auth");
 
     SKIP: {
-        skip "TLSProxy does not support partial messages for dtls", 2
+        skip "TODO(DTLSv1.3): Server hangs on client certificate + finish", 2
             if $run_test_as_dtls == 1;
         #Test 4: Both send cert comp, with client auth
         $proxy->clear();

diff --git a/test/recipes/70-test_tls13hrr.t b/test/recipes/70-test_tls13hrr.t
index 7498b38d83baebad13bf5a54204d5c48ead90554..a5ff57f02d8dcdea12f53e20d2e58f2507e01ca2 100644 (file)
--- a/test/recipes/70-test_tls13hrr.t
+++ b/test/recipes/70-test_tls13hrr.t
@@ -53,10 +53,6 @@ SKIP: {
 
 SKIP: {
     skip "DTLS 1.3 is disabled", $testcount if disabled("dtls1_3");
-    skip "DTLSProxy does not support partial messages that are sent when EC is disabled",
-        $testcount if disabled("ec");
-    skip "This test fails in several configurations because DTLSProxy does not support"
-         ." partial messages that are sent", $testcount;
     skip "DTLSProxy does not work on Windows", $testcount if $^O =~ /^(MSWin32)$/;
     run_tests(1);
 }
@@ -64,12 +60,11 @@ SKIP: {
 sub run_tests
 {
     my $run_test_as_dtls = shift;
-    my $proxy_start_success = 0;
 
     my $proxy;
     if ($run_test_as_dtls == 1) {
         $proxy = TLSProxy::Proxy->new_dtls(
-            undef,
+            \&hrr_filter,
             cmdstr(app([ "openssl" ]), display => 1),
             srctop_file("apps", "server.pem"),
             (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
@@ -77,7 +72,7 @@ sub run_tests
     }
     else {
         $proxy = TLSProxy::Proxy->new(
-            undef,
+            \&hrr_filter,
             cmdstr(app([ "openssl" ]), display => 1),
             srctop_file("apps", "server.pem"),
             (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
@@ -85,12 +80,11 @@ sub run_tests
     }
 
     SKIP: {
-        skip "TODO(DTLSv1.3): When ECX is disabled running this test with DTLS will hang"
-            ." waiting for s_server to close", 2 if $run_test_as_dtls == 1 && disabled("ecx");
+        skip "TODO(DTLSv1.3): Test stalls when server sends its ServerHello.",
+            1 if $run_test_as_dtls == 1 && disabled("ecx");
         #Test 1: A client should fail if the server changes the ciphersuite between the
         #        HRR and the SH
         $proxy->clear();
-        $proxy->filter(\&hrr_filter);
         if (disabled("ec")) {
             $proxy->serverflags("-curves ffdhe3072");
         }
@@ -98,28 +92,30 @@ sub run_tests
             $proxy->serverflags("-curves P-256");
         }
         $testtype = CHANGE_HRR_CIPHERSUITE;
-        $proxy_start_success = $proxy->start();
-        skip "TLSProxy did not start correctly", 2 if $proxy_start_success == 0;
+        # Skip tests if TLSProxy if it fails to start. For DTLS this return is always
+        # false when the handshake fails so we skip the check.
+        skip "TLSProxy did not start correctly", $testcount if $proxy->start() == 0
+            && $run_test_as_dtls == 0;
         ok(TLSProxy::Message->fail(), "Server ciphersuite changes");
+    }
 
-        #Test 2: It is an error if the client changes the offered ciphersuites so that
-        #        we end up selecting a different ciphersuite between HRR and the SH
-        $proxy->clear();
-        if (disabled("ec")) {
-            $proxy->serverflags("-curves ffdhe3072");
-        }
-        else {
-            $proxy->serverflags("-curves P-384");
-        }
-        $proxy->ciphersuitess("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384");
-        $testtype = CHANGE_CH1_CIPHERSUITE;
-        $proxy->start();
-        ok(TLSProxy::Message->fail(), "Client ciphersuite changes");
+    #Test 2: It is an error if the client changes the offered ciphersuites so that
+    #        we end up selecting a different ciphersuite between HRR and the SH
+    $proxy->clear();
+    if (disabled("ec")) {
+        $proxy->serverflags("-curves ffdhe3072");
     }
+    else {
+        $proxy->serverflags("-curves P-384");
+    }
+    $proxy->ciphersuitess("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384");
+    $testtype = CHANGE_CH1_CIPHERSUITE;
+    $proxy->start();
+    ok(TLSProxy::Message->fail(), "Client ciphersuite changes");
 
     SKIP: {
-        skip "DTLSProxy does not support partial messages that are sent when ECX is disabled",
-            1 if $run_test_as_dtls == 1 && disabled("ecx");
+        skip "TODO(DTLSv1.3): Re-enable when #26465 has been merged.",
+             1 if $run_test_as_dtls == 1;
         #Test 3: A client should fail with unexpected_message alert if the server
         #        sends more than 1 HRR
         $fatal_alert = 0;
@@ -144,7 +140,9 @@ sub run_tests
     SKIP: {
         skip "EC/(D)TLSv1.2 is disabled in this build", 1
             if disabled("ec") || ($run_test_as_dtls == 0 && disabled("tls1_2"))
-                              || ($run_test_as_dtls == 1 && disabled("dtls1_2"));
+                              || $run_test_as_dtls == 1;
+    #TODO(DTLSv1.3): Fails when running with DTLS.
+    #                         || ($run_test_as_dtls == 1 && disabled("dtls1_2"));
 
         $proxy->clear();
         $proxy->clientflags("-groups P-256:brainpoolP512r1:P-521");

diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index 7a219f18f7924b21ca6b4003d555de318844560b..2832b0a60b529c828883aeffb8a7a3aee98c75f5 100644 (file)
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -292,55 +292,47 @@ sub run_tests
             checkhandshake::DEFAULT_EXTENSIONS,
             "status_request handshake test (server)");
 
-        SKIP: {
-            skip "TLSProxy does not support partial messages for dtls", 2
-                if $run_test_as_dtls == 1;
-            #Test 5: A status_request handshake (client and server)
-            $proxy->clear();
-            $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
-            $proxy->clientflags("-no_rx_cert_comp -status");
-            $proxy->serverflags("-no_rx_cert_comp -status_file "
-                . srctop_file("test", "recipes", "ocsp-response.der"));
-            $proxy->start();
-            checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
-                checkhandshake::DEFAULT_EXTENSIONS
-                    | checkhandshake::STATUS_REQUEST_CLI_EXTENSION
-                    | checkhandshake::STATUS_REQUEST_SRV_EXTENSION,
-                "status_request handshake test");
-
-            #Test 6: A status_request handshake (client and server) with client auth
-            $proxy->clear();
-            $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
-            $proxy->clientflags("-no_rx_cert_comp -status -enable_pha -cert "
-                . srctop_file("apps", "server.pem"));
-            $proxy->serverflags("-no_rx_cert_comp -Verify 5 -status_file "
-                . srctop_file("test", "recipes", "ocsp-response.der"));
-            $proxy->start();
-            checkhandshake($proxy, checkhandshake::CLIENT_AUTH_HANDSHAKE,
-                checkhandshake::DEFAULT_EXTENSIONS
-                    | checkhandshake::STATUS_REQUEST_CLI_EXTENSION
-                    | checkhandshake::STATUS_REQUEST_SRV_EXTENSION
-                    | checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION,
-                "status_request handshake with client auth test");
-        }
-    }
+        #Test 5: A status_request handshake (client and server)
+        $proxy->clear();
+        $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
+        $proxy->clientflags("-no_rx_cert_comp -status");
+        $proxy->serverflags("-no_rx_cert_comp -status_file "
+            . srctop_file("test", "recipes", "ocsp-response.der"));
+        $proxy->start();
+        checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
+            checkhandshake::DEFAULT_EXTENSIONS
+                | checkhandshake::STATUS_REQUEST_CLI_EXTENSION
+                | checkhandshake::STATUS_REQUEST_SRV_EXTENSION,
+            "status_request handshake test");
 
-    SKIP: {
-        skip "TLSProxy does not support partial messages for dtls", 1
-            if $run_test_as_dtls == 1;
-        #Test 7: A client auth handshake
+        #Test 6: A status_request handshake (client and server) with client auth
         $proxy->clear();
         $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
-        $proxy->clientflags("-no_rx_cert_comp -enable_pha -cert " . srctop_file("apps", "server.pem"));
-        $proxy->serverflags("-no_rx_cert_comp -Verify 5");
-        $proxy_start_success = $proxy->start();
-        skip "TLSProxy did not start correctly", $testcount - 6 if $proxy_start_success == 0;
+        $proxy->clientflags("-no_rx_cert_comp -status -enable_pha -cert "
+            . srctop_file("apps", "server.pem"));
+        $proxy->serverflags("-no_rx_cert_comp -Verify 5 -status_file "
+            . srctop_file("test", "recipes", "ocsp-response.der"));
+        $proxy->start();
         checkhandshake($proxy, checkhandshake::CLIENT_AUTH_HANDSHAKE,
-            checkhandshake::DEFAULT_EXTENSIONS |
-                checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION,
-            "Client auth handshake test");
+            checkhandshake::DEFAULT_EXTENSIONS
+                | checkhandshake::STATUS_REQUEST_CLI_EXTENSION
+                | checkhandshake::STATUS_REQUEST_SRV_EXTENSION
+                | checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION,
+            "status_request handshake with client auth test");
     }
 
+    #Test 7: A client auth handshake
+    $proxy->clear();
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
+    $proxy->clientflags("-no_rx_cert_comp -enable_pha -cert " . srctop_file("apps", "server.pem"));
+    $proxy->serverflags("-no_rx_cert_comp -Verify 5");
+    $proxy_start_success = $proxy->start();
+    skip "TLSProxy did not start correctly", $testcount - 6 if $proxy_start_success == 0;
+    checkhandshake($proxy, checkhandshake::CLIENT_AUTH_HANDSHAKE,
+                   checkhandshake::DEFAULT_EXTENSIONS
+                   | checkhandshake::POST_HANDSHAKE_AUTH_CLI_EXTENSION,
+                   "Client auth handshake test");
+
     #Test 8: Server name handshake (no client request)
     $proxy->clear();
     $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
@@ -406,10 +398,11 @@ sub run_tests
         "ALPN handshake test");
 
     SKIP: {
+        # TODO(DTLSv1.3): When ecx is disabled the test reports "Invalid
+        # CertificateVerify signature length" when running with DTLS.
         skip "No CT, EC or OCSP support in this OpenSSL build", 1
-            if disabled("ct") || disabled("ec") || disabled("ocsp");
-        skip "TLSProxy does not support partial messages for dtls", 1
-            if $run_test_as_dtls == 1;
+            if disabled("ct") || disabled("ec") || disabled("ocsp")
+               || ($run_test_as_dtls == 1 && disabled("ecx"));
 
         #Test 14: SCT handshake (client request only)
         $proxy->clear();
@@ -430,7 +423,7 @@ sub run_tests
     }
 
     SKIP: {
-        skip "TLSProxy does not support partial messages for dtls", 1
+        skip "TODO(DTLSv1.3): Re-enable when #26465 is merged.", 1
             if $run_test_as_dtls == 1;
         #Test 15: HRR Handshake
         $proxy->clear();
@@ -461,20 +454,15 @@ sub run_tests
             "Resumption handshake with HRR test");
     }
 
-
-    SKIP: {
-        skip "TLSProxy does not support partial messages for dtls", 1
-            if $run_test_as_dtls == 1;
-        #Test 17: Acceptable but non preferred key_share
-        $proxy->clear();
-        $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
-        $proxy->clientflags("-no_rx_cert_comp -curves P-384");
-        $proxy->start();
-        checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
-            checkhandshake::DEFAULT_EXTENSIONS
-                | checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION,
-            "Acceptable but non preferred key_share");
-    }
+    #Test 17: Acceptable but non preferred key_share
+    $proxy->clear();
+    $proxy->cipherc("DEFAULT:\@SECLEVEL=2");
+    $proxy->clientflags("-no_rx_cert_comp -curves P-384");
+    $proxy->start();
+    checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
+                   checkhandshake::DEFAULT_EXTENSIONS
+                   | checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION,
+                    "Acceptable but non preferred key_share");
 
     unlink $session;
 }

diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm
index 4a3fe22971c28d4683e905dde1836eedb0a258bd..526afc9b4aaf718323e6904b8fbba66d1c6fd226 100644 (file)
--- a/util/perl/TLSProxy/Message.pm
+++ b/util/perl/TLSProxy/Message.pm
@@ -149,6 +149,9 @@ use constant {
 
 my $payload = "";
 my $messlen = -1;
+my $messseq = -1;
+my $messfraglen = -1;
+my $messfragoffs = -1;
 my $mt;
 my $startoffset = -1;
 my $server = 0;
@@ -164,6 +167,9 @@ sub clear
 {
     $payload = "";
     $messlen = -1;
+    $messseq = -1;
+    $messfraglen = -1;
+    $messfragoffs = -1;
     $startoffset = -1;
     $server = 0;
     $success = 0;
@@ -225,13 +231,8 @@ sub get_messages
                     $recoffset = $messlen - length($payload);
                     $payload .= substr($record->decrypt_data, 0, $recoffset);
                     push @message_frag_lens, $recoffset;
-                    if ($isdtls) {
-                        # We must set $msgseq, $msgfraglen, $msgfragoffs
-                        die "Internal error: cannot handle partial dtls messages\n"
-                    }
                     $message = create_message($server, $mt,
-                        #$msgseq, $msgfraglen, $msgfragoffs,
-                        0, 0, 0,
+                        $messseq, $messfraglen, $messfragoffs,
                         $payload, $startoffset, $isdtls);
                     push @messages, $message;
 
@@ -256,18 +257,15 @@ sub get_messages
                 @message_rec_list = ($record);
                 my $lenhi;
                 my $lenlo;
-                my $msgseq;
-                my $msgfraglen;
-                my $msgfragoffs;
                 if ($isdtls) {
                     my $msgfraglenhi;
                     my $msgfraglenlo;
                     my $msgfragoffshi;
                     my $msgfragoffslo;
-                    ($mt, $lenhi, $lenlo, $msgseq, $msgfragoffshi, $msgfragoffslo, $msgfraglenhi, $msgfraglenlo) =
+                    ($mt, $lenhi, $lenlo, $messseq, $msgfragoffshi, $msgfragoffslo, $msgfraglenhi, $msgfraglenlo) =
                         unpack('CnCnnCnC', substr($record->decrypt_data, $recoffset));
-                    $msgfraglen = ($msgfraglenhi << 8) | $msgfraglenlo;
-                    $msgfragoffs = ($msgfragoffshi << 8) | $msgfragoffslo;
+                    $messfraglen = ($msgfraglenhi << 8) | $msgfraglenlo;
+                    $messfragoffs = ($msgfragoffshi << 8) | $msgfragoffslo;
                 } else {
                     ($mt, $lenhi, $lenlo) =
                         unpack('CnC', substr($record->decrypt_data, $recoffset));
@@ -276,8 +274,8 @@ sub get_messages
                 print "  Message type: $message_type{$mt}($mt)\n";
                 print "  Message Length: $messlen\n";
                 if ($isdtls) {
-                    print "  Message fragment length: $msgfraglen\n";
-                    print "  Message fragment offset: $msgfragoffs\n";
+                    print "  Message fragment length: $messfraglen\n";
+                    print "  Message fragment offset: $messfragoffs\n";
                 }
                 $startoffset = $recoffset;
                 $recoffset += $msgheaderlen;
@@ -291,8 +289,8 @@ sub get_messages
                                            $messlen);
                         $recoffset += $messlen;
                         push @message_frag_lens, $messlen;
-                        $message = create_message($server, $mt, $msgseq,
-                                                  $msgfraglen, $msgfragoffs,
+                        $message = create_message($server, $mt, $messseq,
+                                                  $messfraglen, $messfragoffs,
                                                   $payload, $startoffset, $isdtls);
                         push @messages, $message;