Skip to content

Commit

Permalink
Add a test for an invalid group in the HRR
Browse files Browse the repository at this point in the history
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 #21163)

(cherry picked from commit adf33f9)
  • Loading branch information
mattcaswell committed Jun 23, 2023
1 parent 98f43f4 commit 7df9bd3
Showing 1 changed file with 35 additions and 7 deletions.
42 changes: 35 additions & 7 deletions test/recipes/70-test_tls13hrr.t
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

0 comments on commit 7df9bd3

Please sign in to comment.