[openssl-commits] [openssl] OpenSSL_1_1_1-stable update
Matt Caswell
matt at openssl.org
Mon Jan 7 09:48:08 UTC 2019
The branch OpenSSL_1_1_1-stable has been updated
via d3b574fee1c4ad887a219fadb1674349ae0ce4b7 (commit)
via fe5a516b72942f5eacda8c9c7f032e8c76e0cb7b (commit)
from 053aedf1536267b621cb8d7bceaafece4df03c41 (commit)
- Log -----------------------------------------------------------------
commit d3b574fee1c4ad887a219fadb1674349ae0ce4b7
Author: Matt Caswell <matt at openssl.org>
Date: Fri Jan 4 16:55:15 2019 +0000
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 at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7984)
(cherry picked from commit 9effc496ad8a9b0ec737c69cc0fddf610a045ea4)
commit fe5a516b72942f5eacda8c9c7f032e8c76e0cb7b
Author: Matt Caswell <matt at openssl.org>
Date: Fri Jan 4 16:54:03 2019 +0000
Don't complain if we receive the cryptopro extension in the ClientHello
The cryptopro extension is supposed to be unsolicited and appears in the
ServerHello only. Additionally it is unofficial and unregistered - therefore
we should really treat it like any other unknown extension if we see it in
the ClientHello.
Fixes #7747
Reviewed-by: Paul Dale <paul.dale at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7984)
(cherry picked from commit 23fed8ba0ec895e1b2a089cae380697f15170afc)
-----------------------------------------------------------------------
Summary of changes:
ssl/statem/extensions.c | 6 ++++--
test/recipes/70-test_sslextension.t | 32 +++++++++++++++++++++++++++----
util/perl/TLSProxy/Certificate.pm | 5 -----
util/perl/TLSProxy/ClientHello.pm | 7 ++-----
util/perl/TLSProxy/EncryptedExtensions.pm | 5 -----
util/perl/TLSProxy/Message.pm | 16 +++++++++++-----
util/perl/TLSProxy/ServerHello.pm | 2 +-
7 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 11feae5..0916395 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -348,10 +348,12 @@ static const EXTENSION_DEFINITION ext_defs[] = {
{
/*
* Special unsolicited ServerHello extension only used when
- * SSL_OP_CRYPTOPRO_TLSEXT_BUG is set
+ * SSL_OP_CRYPTOPRO_TLSEXT_BUG is set. We allow it in a ClientHello but
+ * ignore it.
*/
TLSEXT_TYPE_cryptopro_bug,
- SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_TLS1_2_AND_BELOW_ONLY,
+ SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO
+ | SSL_EXT_TLS1_2_AND_BELOW_ONLY,
NULL, NULL, NULL, tls_construct_stoc_cryptopro_bug, NULL, NULL
},
{
diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
index 20e1933..525de06 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 d3bf7f2..a415897 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 2ae9d6f..ec48469 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 81242e2..cd529ee 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 16ed012..ee507f9 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 84f2faa..c95bbee 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;
More information about the openssl-commits
mailing list