[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Mon Jan 7 09:47:54 UTC 2019
The branch master has been updated
via 9effc496ad8a9b0ec737c69cc0fddf610a045ea4 (commit)
via 23fed8ba0ec895e1b2a089cae380697f15170afc (commit)
from 67ee899cb51d3e3d7b5f00b878f8f82a097b93f0 (commit)
- Log -----------------------------------------------------------------
commit 9effc496ad8a9b0ec737c69cc0fddf610a045ea4
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)
commit 23fed8ba0ec895e1b2a089cae380697f15170afc
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)
-----------------------------------------------------------------------
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 ffa4b46..773309a 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 79466b6..e725b44 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 70c9fae..03f6619 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 7ae3dba..c49bc23 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 f56f3c4..4fd445b 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 642afb5..7180369 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 94e7ab5..14eb813 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