Test that we ignore a bad record version in a plaintext TLSv1.3 record
authorMatt Caswell <matt@openssl.org>
Wed, 24 Aug 2022 15:29:52 +0000 (16:29 +0100)
committerTomas Mraz <tomas@openssl.org>
Mon, 29 Aug 2022 10:21:34 +0000 (12:21 +0200)
The RFC requires us to ignore this field in plaintext records - so even
if it is set incorrectly we should tolerate it.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19058)

test/recipes/70-test_sslrecords.t

index 6d099f645dd130e799a86db6ec009adf3b87fbc6..9a7e3d8c06839263174375d0064be66b69e37916 100644 (file)
@@ -44,7 +44,7 @@ my $inject_recs_num = 1;
 $proxy->serverflags("-tls1_2");
 $proxy->clientflags("-no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 20;
+plan tests => 21;
 ok($fatal_alert, "Out of context empty records test");
 
 #Test 2: Injecting in context empty records should succeed
@@ -175,7 +175,7 @@ ok($fatal_alert, "Changed record version in TLS1.2");
 
 #TLS1.3 specific tests
 SKIP: {
-    skip "TLSv1.3 disabled", 8
+    skip "TLSv1.3 disabled", 9
         if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
     #Test 13: Sending a different record version in TLS1.3 should fail
@@ -247,6 +247,22 @@ SKIP: {
     $boundary_test_type = NO_DATA_BETWEEN_KEY_UPDATE;
     $proxy->start();
     ok(TLSProxy::Message->success(), "No data between KeyUpdate");
+
+    SKIP: {
+        skip "EC disabled", 1 if disabled("ec");
+
+        #Test 21: Force an HRR and change the "real" ServerHello to have a protocol
+        #         record version of 0x0301 (TLSv1.0). At this point we have already
+        #         decided that we are doing TLSv1.3 but are still using plaintext
+        #         records. The server should be sending a record version of 0x303
+        #         (TLSv1.2), but the RFC requires us to ignore this field so we
+        #         should tolerate the incorrect version.
+        $proxy->clear();
+        $proxy->filter(\&change_server_hello_version);
+        $proxy->serverflags("-groups P-256"); # Force an HRR
+        $proxy->start();
+        ok(TLSProxy::Message->success(), "Bad ServerHello record version after HRR");
+    }
  }
 
 
@@ -535,6 +551,26 @@ sub change_version
     }
 }
 
+sub change_server_hello_version
+{
+    my $proxy = shift;
+    my $records = $proxy->record_list;
+
+    # We're only interested in changing the ServerHello after an HRR
+    if ($proxy->flight != 3) {
+        return;
+    }
+
+    # The ServerHello has index 5
+    # 0 - ClientHello
+    # 1 - HRR
+    # 2 - CCS
+    # 3 - ClientHello(2)
+    # 4 - CCS
+    # 5 - ServerHello
+    @{$records}[5]->version(TLSProxy::Record::VERS_TLS_1_0);
+}
+
 sub change_outer_record_type
 {
     my $proxy = shift;