Add a test for correct handling of the cryptopro bug extension

This was complicated by the fact that we were using this extension for our
duplicate extension handling tests. In order to add tests for cryptopro
bug the duplicate extension handling tests needed to change first.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7984)

(cherry picked from commit 9effc496ad8a9b0ec737c69cc0fddf610a045ea4)

diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 20e1933f0054162277d18f367a6dd68aeefba9f7..525de06f2a95745920ac332794d33dedd5539eee 100644 (file)
--- a/test/recipes/70-test_sslextension.t
+++ b/test/recipes/70-test_sslextension.t
@@ -88,9 +88,11 @@ sub inject_duplicate_extension
     foreach my $message (@{$proxy->message_list}) {
         if ($message->mt == $message_type) {
           my %extensions = %{$message->extension_data};
-            # Add a duplicate (unknown) extension.
-            $message->set_extension(TLSProxy::Message::EXT_DUPLICATE_EXTENSION, "");
-            $message->set_extension(TLSProxy::Message::EXT_DUPLICATE_EXTENSION, "");
+            # Add a duplicate extension. We use cryptopro_bug since we never
+            # normally write that one, and it is allowed as unsolicited in the
+            # ServerHello
+            $message->set_extension(TLSProxy::Message::EXT_CRYPTOPRO_BUG_EXTENSION, "");
+            $message->dupext(TLSProxy::Message::EXT_CRYPTOPRO_BUG_EXTENSION);
             $message->repack();
         }
     }
@@ -173,9 +175,23 @@ sub inject_unsolicited_extension
     $sent_unsolisited_extension = 1;
 }
 
+sub inject_cryptopro_extension
+{
+    my $proxy = shift;
+
+    # We're only interested in the initial ClientHello
+    if ($proxy->flight != 0) {
+        return;
+    }
+
+    my $message = ${$proxy->message_list}[0];
+    $message->set_extension(TLSProxy::Message::EXT_CRYPTOPRO_BUG_EXTENSION, "");
+    $message->repack();
+}
+
 # Test 1-2: Sending a duplicate extension should fail.
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 7;
+plan tests => 8;
 ok($fatal_alert, "Duplicate ClientHello extension");
 
 $fatal_alert = 0;
@@ -234,3 +250,11 @@ SKIP: {
     $proxy->start();
     ok($fatal_alert, "Unsolicited server name extension (TLSv1.3)");
 }
+
+#Test 8: Send the cryptopro extension in a ClientHello. Normally this is an
+#        unsolicited extension only ever seen in the ServerHello. We should
+#        ignore it in a ClientHello
+$proxy->clear();
+$proxy->filter(\&inject_cryptopro_extension);
+$proxy->start();
+ok(TLSProxy::Message->success(), "Cryptopro extension in ClientHello");

diff --git a/util/perl/TLSProxy/Certificate.pm b/util/perl/TLSProxy/Certificate.pm
index d3bf7f21804471e47fc52cbb61b0e0262ff11b6c..a415897352a236c32d3a962a8b38ec5f5e35ef33 100644 (file)
--- a/util/perl/TLSProxy/Certificate.pm
+++ b/util/perl/TLSProxy/Certificate.pm
@@ -138,11 +138,6 @@ sub set_message_contents
             $extensions .= pack("n", $key);
             $extensions .= pack("n", length($extdata));
             $extensions .= $extdata;
-            if ($key == TLSProxy::Message::EXT_DUPLICATE_EXTENSION) {
-              $extensions .= pack("n", $key);
-              $extensions .= pack("n", length($extdata));
-              $extensions .= $extdata;
-            }
         }
         $data = pack('C', length($self->context()));
         $data .= $self->context;

diff --git a/util/perl/TLSProxy/ClientHello.pm b/util/perl/TLSProxy/ClientHello.pm
index 2ae9d6f55d2b9a5c83e0705b376c2075fc01e21a..ec4846966cb7cede31174b8231510653933b7a8e 100644 (file)
--- a/util/perl/TLSProxy/ClientHello.pm
+++ b/util/perl/TLSProxy/ClientHello.pm
@@ -124,11 +124,6 @@ sub extension_contents
     $extension .= pack("n", $key);
     $extension .= pack("n", length($extdata));
     $extension .= $extdata;
-    if ($key == TLSProxy::Message::EXT_DUPLICATE_EXTENSION) {
-        $extension .= pack("n", $key);
-        $extension .= pack("n", length($extdata));
-        $extension .= $extdata;
-    }
     return $extension;
 }
 
@@ -151,6 +146,8 @@ sub set_message_contents
     foreach my $key (keys %{$self->extension_data}) {
         next if ($key == TLSProxy::Message::EXT_PSK);
         $extensions .= $self->extension_contents($key);
+        #Add extension twice if we are duplicating that extension
+        $extensions .= $self->extension_contents($key) if ($key == $self->dupext);
     }
     #PSK extension always goes last...
     if (defined ${$self->extension_data}{TLSProxy::Message::EXT_PSK}) {

diff --git a/util/perl/TLSProxy/EncryptedExtensions.pm b/util/perl/TLSProxy/EncryptedExtensions.pm
index 81242e29ff9de8f8fbdc1ab65bb8a6d3355f5541..cd529eed8e505c44d97b2bbfab17ed763588ddd7 100644 (file)
--- a/util/perl/TLSProxy/EncryptedExtensions.pm
+++ b/util/perl/TLSProxy/EncryptedExtensions.pm
@@ -81,11 +81,6 @@ sub set_message_contents
         $extensions .= pack("n", $key);
         $extensions .= pack("n", length($extdata));
         $extensions .= $extdata;
-        if ($key == TLSProxy::Message::EXT_DUPLICATE_EXTENSION) {
-            $extensions .= pack("n", $key);
-            $extensions .= pack("n", length($extdata));
-            $extensions .= $extdata;
-        }
     }
 
     $data = pack('n', length($extensions));

diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm
index 16ed0120669b884c95112137f395f670c04ebad5..ee507f9e21d814dd491cd460fcb6e4d1ba6e43f6 100644 (file)
--- a/util/perl/TLSProxy/Message.pm
+++ b/util/perl/TLSProxy/Message.pm
@@ -86,10 +86,7 @@ use constant {
     EXT_SIG_ALGS_CERT => 50,
     EXT_RENEGOTIATE => 65281,
     EXT_NPN => 13172,
-    # This extension is an unofficial extension only ever written by OpenSSL
-    # (i.e. not read), and even then only when enabled. We use it to test
-    # handling of duplicate extensions.
-    EXT_DUPLICATE_EXTENSION => 0xfde8,
+    EXT_CRYPTOPRO_BUG_EXTENSION => 0xfde8,
     EXT_UNKNOWN => 0xfffe,
     #Unknown extension that should appear last
     EXT_FORCE_LAST => 0xffff
@@ -420,7 +417,8 @@ sub new
         records => $records,
         mt => $mt,
         startoffset => $startoffset,
-        message_frag_lens => $message_frag_lens
+        message_frag_lens => $message_frag_lens,
+        dupext => -1
     };
 
     return bless $self, $class;
@@ -575,6 +573,14 @@ sub encoded_length
     my $self = shift;
     return TLS_MESSAGE_HEADER_LENGTH + length($self->data);
 }
+sub dupext
+{
+    my $self = shift;
+    if (@_) {
+        $self->{dupext} = shift;
+    }
+    return $self->{dupext};
+}
 sub successondata
 {
     my $class = shift;

diff --git a/util/perl/TLSProxy/ServerHello.pm b/util/perl/TLSProxy/ServerHello.pm
index 84f2faab058bd58683ee7c44da85f995ec5bc670..c95bbeeaeb7d1305d058d44aaa3440df1b724628 100644 (file)
--- a/util/perl/TLSProxy/ServerHello.pm
+++ b/util/perl/TLSProxy/ServerHello.pm
@@ -154,7 +154,7 @@ sub set_message_contents
         $extensions .= pack("n", $key);
         $extensions .= pack("n", length($extdata));
         $extensions .= $extdata;
-        if ($key == TLSProxy::Message::EXT_DUPLICATE_EXTENSION) {
+        if ($key == $self->dupext) {
           $extensions .= pack("n", $key);
           $extensions .= pack("n", length($extdata));
           $extensions .= $extdata;