From 7df9bd366c7136abaa8deef978270809ba082595 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Fri, 9 Jun 2023 09:33:11 +0100
Subject: [PATCH] Add a test for an invalid group in the HRR

Test that if the client sends a key share for a group in the server's
supported_group list but is otherwise invalid, that we don't select it
in the HRR.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/21163)

(cherry picked from commit adf33f9e268b17ec1b4739707abb40b03b21ea6a)
---
 test/recipes/70-test_tls13hrr.t | 42 +++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/test/recipes/70-test_tls13hrr.t b/test/recipes/70-test_tls13hrr.t
index faf7302e42409..2c385a8a43ee5 100644
--- a/test/recipes/70-test_tls13hrr.t
+++ b/test/recipes/70-test_tls13hrr.t
@@ -38,7 +38,8 @@ my $proxy = TLSProxy::Proxy->new(
 use constant {
     CHANGE_HRR_CIPHERSUITE => 0,
     CHANGE_CH1_CIPHERSUITE => 1,
-    DUPLICATE_HRR => 2
+    DUPLICATE_HRR => 2,
+    INVALID_GROUP => 3
 };
 
 #Test 1: A client should fail if the server changes the ciphersuite between the
@@ -51,7 +52,7 @@ if (disabled("ec")) {
 }
 my $testtype = CHANGE_HRR_CIPHERSUITE;
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 3;
+plan tests => 4;
 ok(TLSProxy::Message->fail(), "Server ciphersuite changes");
 
 #Test 2: It is an error if the client changes the offered ciphersuites so that
@@ -80,6 +81,24 @@ $testtype = DUPLICATE_HRR;
 $proxy->start();
 ok($fatal_alert, "Server duplicated HRR");
 
+#Test 4: If the client sends a group that is in the supported_groups list but
+#        otherwise not valid (e.g. not suitable for TLSv1.3) we should reject it
+#        and not consider it when sending the HRR. We send brainpoolP512r1 in
+#        the ClientHello, which is acceptable to the server but is not valid in
+#        TLSv1.3. We expect the server to select X25519 in the HRR and the
+#        handshake to complete successfully
+SKIP: {
+    skip "EC/TLSv1.2 is disabled in this build", 1
+        if disabled("ec") || disabled("tls1_2");
+
+    $proxy->clear();
+    $proxy->clientflags("-groups P-256:brainpoolP512r1:X25519");
+    $proxy->serverflags("-groups brainpoolP512r1:X25519");
+    $testtype = INVALID_GROUP;
+    $proxy->start();
+    ok(TLSProxy::Message->success(), "Invalid group with HRR");
+}
+
 sub hrr_filter
 {
     my $proxy = shift;
@@ -133,16 +152,25 @@ sub hrr_filter
         return;
     }
 
-    # CHANGE_CH1_CIPHERSUITE
     if ($proxy->flight != 0) {
         return;
     }
 
     my $ch1 = ${$proxy->message_list}[0];
 
-    # The server will always pick TLS_AES_256_GCM_SHA384
-    my @ciphersuites = (TLSProxy::Message::CIPHER_TLS13_AES_128_GCM_SHA256);
-    $ch1->ciphersuite_len(2 * scalar @ciphersuites);
-    $ch1->ciphersuites(\@ciphersuites);
+    if ($testtype == CHANGE_CH1_CIPHERSUITE) {
+        # The server will always pick TLS_AES_256_GCM_SHA384
+        my @ciphersuites = (TLSProxy::Message::CIPHER_TLS13_AES_128_GCM_SHA256);
+        $ch1->ciphersuite_len(2 * scalar @ciphersuites);
+        $ch1->ciphersuites(\@ciphersuites);
+    } elsif ($testtype == INVALID_GROUP) {
+        # INVALID_GROUP
+        my $ext = pack "C7",
+            0x00, 0x05, #List Length
+            0x00, 0x1c, #brainpoolP512r1 (not compatible with TLSv1.3)
+            0x00, 0x01, 0xff; #key_exchange data
+        $ch1->set_extension(
+            TLSProxy::Message::EXT_KEY_SHARE, $ext);
+    }
     $ch1->repack();
 }