From 301b870546d1c7b2d8f0d66e04a2596142f0399f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 21 Jun 2024 14:29:26 +0100 Subject: [PATCH 10/10] Add a test for an empty NextProto message It is valid according to the spec for a NextProto message to have no protocols listed in it. The OpenSSL implementation however does not allow us to create such a message. In order to check that we work as expected when communicating with a client that does generate such messages we have to use a TLSProxy test. Follow on from CVE-2024-5535 Reviewed-by: Neil Horman Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/24717) --- test/recipes/70-test_npn.t | 73 +++++++++++++++++++++++++++++++++ util/perl/TLSProxy/Message.pm | 9 ++++ util/perl/TLSProxy/NextProto.pm | 54 ++++++++++++++++++++++++ util/perl/TLSProxy/Proxy.pm | 1 + 4 files changed, 137 insertions(+) create mode 100644 test/recipes/70-test_npn.t create mode 100644 util/perl/TLSProxy/NextProto.pm diff --git a/test/recipes/70-test_npn.t b/test/recipes/70-test_npn.t new file mode 100644 index 0000000000..f82e71af6a --- /dev/null +++ b/test/recipes/70-test_npn.t @@ -0,0 +1,73 @@ +#! /usr/bin/env perl +# Copyright 2024 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the Apache License 2.0 (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + +use strict; +use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file/; +use OpenSSL::Test::Utils; + +use TLSProxy::Proxy; + +my $test_name = "test_npn"; +setup($test_name); + +plan skip_all => "TLSProxy isn't usable on $^O" + if $^O =~ /^(VMS)$/; + +plan skip_all => "$test_name needs the dynamic engine feature enabled" + if disabled("engine") || disabled("dynamic-engine"); + +plan skip_all => "$test_name needs the sock feature enabled" + if disabled("sock"); + +plan skip_all => "$test_name needs NPN enabled" + if disabled("nextprotoneg"); + +plan skip_all => "$test_name needs TLSv1.2 enabled" + if disabled("tls1_2"); + +my $proxy = TLSProxy::Proxy->new( + undef, + cmdstr(app(["openssl"]), display => 1), + srctop_file("apps", "server.pem"), + (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE}) +); + +$proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; +plan tests => 1; + +my $npnseen = 0; + +# Test 1: Check sending an empty NextProto message from the client works. This is +# valid as per the spec, but OpenSSL does not allow you to send it. +# Therefore we must be prepared to receive such a message but we cannot +# generate it except via TLSProxy +$proxy->clear(); +$proxy->filter(\&npn_filter); +$proxy->clientflags("-nextprotoneg foo -no_tls1_3"); +$proxy->serverflags("-nextprotoneg foo"); +$proxy->start(); +ok($npnseen && TLSProxy::Message->success(), "Empty NPN message"); + +sub npn_filter +{ + my $proxy = shift; + my $message; + + # The NextProto message always appears in flight 2 + return if $proxy->flight != 2; + + foreach my $message (@{$proxy->message_list}) { + if ($message->mt == TLSProxy::Message::MT_NEXT_PROTO) { + # Our TLSproxy NextProto message support doesn't support parsing of + # the message. If we repack it just creates an empty NextProto + # message - which is exactly the scenario we want to test here. + $message->repack(); + $npnseen = 1; + } + } +} diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm index ce22187569..fb41b2ffc8 100644 --- a/util/perl/TLSProxy/Message.pm +++ b/util/perl/TLSProxy/Message.pm @@ -384,6 +384,15 @@ sub create_message [@message_frag_lens] ); $message->parse(); + } elsif ($mt == MT_NEXT_PROTO) { + $message = TLSProxy::NextProto->new( + $server, + $data, + [@message_rec_list], + $startoffset, + [@message_frag_lens] + ); + $message->parse(); } else { #Unknown message type $message = TLSProxy::Message->new( diff --git a/util/perl/TLSProxy/NextProto.pm b/util/perl/TLSProxy/NextProto.pm new file mode 100644 index 0000000000..0e18347546 --- /dev/null +++ b/util/perl/TLSProxy/NextProto.pm @@ -0,0 +1,54 @@ +# Copyright 2024 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the Apache License 2.0 (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + +use strict; + +package TLSProxy::NextProto; + +use vars '@ISA'; +push @ISA, 'TLSProxy::Message'; + +sub new +{ + my $class = shift; + my ($server, + $data, + $records, + $startoffset, + $message_frag_lens) = @_; + + my $self = $class->SUPER::new( + $server, + TLSProxy::Message::MT_NEXT_PROTO, + $data, + $records, + $startoffset, + $message_frag_lens); + + return $self; +} + +sub parse +{ + # We don't support parsing at the moment +} + +# This is supposed to reconstruct the on-the-wire message data following changes. +# For now though since we don't support parsing we just create an empty NextProto +# message - this capability is used in test_npn +sub set_message_contents +{ + my $self = shift; + my $data; + + $data = pack("C32", 0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00); + $self->data($data); +} +1; diff --git a/util/perl/TLSProxy/Proxy.pm b/util/perl/TLSProxy/Proxy.pm index 3de10eccb9..b707722b6b 100644 --- a/util/perl/TLSProxy/Proxy.pm +++ b/util/perl/TLSProxy/Proxy.pm @@ -23,6 +23,7 @@ use TLSProxy::CertificateRequest; use TLSProxy::CertificateVerify; use TLSProxy::ServerKeyExchange; use TLSProxy::NewSessionTicket; +use TLSProxy::NextProto; my $have_IPv6; my $IP_factory; -- 2.46.0