From 0c37be3a7cc318fdeb23d65d2491ffaa5b1ac86e Mon Sep 17 00:00:00 2001 From: Frederik Wedel-Heinen Date: Wed, 22 Jan 2025 16:48:06 +0100 Subject: [PATCH] TLSProxy: Handle partial messages with DTLS Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/26532) --- test/recipes/70-test_dtls13epoch.t | 4 - test/recipes/70-test_tls13alerts.t | 2 - test/recipes/70-test_tls13certcomp.t | 2 +- test/recipes/70-test_tls13hrr.t | 54 +++++++------ test/recipes/70-test_tls13messages.t | 110 ++++++++++++--------------- util/perl/TLSProxy/Message.pm | 30 ++++---- 6 files changed, 90 insertions(+), 112 deletions(-) diff --git a/test/recipes/70-test_dtls13epoch.t b/test/recipes/70-test_dtls13epoch.t index e383d8775cf..dd7d070cca1 100644 --- a/test/recipes/70-test_dtls13epoch.t +++ b/test/recipes/70-test_dtls13epoch.t @@ -26,10 +26,6 @@ plan skip_all => "$test_name needs the dynamic engine 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 d9a7e89da32..ca0fd864f79 100644 --- a/test/recipes/70-test_tls13alerts.t +++ b/test/recipes/70-test_tls13alerts.t @@ -38,8 +38,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 aa663708b3f..f3a456aa847 100644 --- a/test/recipes/70-test_tls13certcomp.t +++ b/test/recipes/70-test_tls13certcomp.t @@ -280,7 +280,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 eed0d9df4da..7ef1c598da7 100644 --- a/test/recipes/70-test_tls13hrr.t +++ b/test/recipes/70-test_tls13hrr.t @@ -50,10 +50,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); } @@ -61,12 +57,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}) @@ -74,7 +69,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}) @@ -82,12 +77,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"); } @@ -95,28 +89,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; @@ -141,7 +137,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 d0ac5163c23..d451e046822 100644 --- a/test/recipes/70-test_tls13messages.t +++ b/test/recipes/70-test_tls13messages.t @@ -289,55 +289,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"); @@ -403,10 +395,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(); @@ -427,7 +420,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(); @@ -458,20 +451,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 0239b509522..6831b778e80 100644 --- 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; -- 2.47.2