From: Matt Caswell <matt@openssl.org>
Date: Fri, 4 Jan 2019 16:55:15 +0000 (+0000)
Subject: Add a test for correct handling of the cryptopro bug extension
X-Git-Tag: openssl-3.0.0-alpha1~2653
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=9effc496ad8a9b0ec737c69cc0fddf610a045ea4;p=oweals%2Fopenssl.git

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)
---

diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 79466b6109..e725b44f9c 100644
--- 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 70c9faea72..03f6619954 100644
--- 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 7ae3dba901..c49bc23671 100644
--- 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 f56f3c4270..4fd445b41e 100644
--- 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 642afb58cb..71803698c2 100644
--- 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 94e7ab5f39..14eb813d5e 100644
--- 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;