[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Fri Mar 10 15:29:41 UTC 2017
The branch master has been updated
via 717afd9337abb2ec8f4b59c7c700fe417e746346 (commit)
via 652a6b7ee1be26c1a5205a494b0245d41dc34e26 (commit)
from 0b1f26648671b94e2ae3e11d602556a4e46535ce (commit)
- Log -----------------------------------------------------------------
commit 717afd9337abb2ec8f4b59c7c700fe417e746346
Author: Matt Caswell <matt at openssl.org>
Date: Fri Mar 10 13:54:32 2017 +0000
Add a test to check that if a PSK extension is not last then we fail
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2896)
commit 652a6b7ee1be26c1a5205a494b0245d41dc34e26
Author: Matt Caswell <matt at openssl.org>
Date: Fri Mar 10 13:53:53 2017 +0000
Check that the PSK extension is last
We need to check that the PSK extension in a ClientHello is the last one.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2896)
-----------------------------------------------------------------------
Summary of changes:
ssl/statem/extensions.c | 8 +++-
...{70-test_renegotiation.t => 70-test_tls13psk.t} | 49 ++++++++++++----------
util/TLSProxy/ClientHello.pm | 37 ++++++++++++----
util/TLSProxy/Message.pm | 4 +-
4 files changed, 63 insertions(+), 35 deletions(-)
copy test/recipes/{70-test_renegotiation.t => 70-test_tls13psk.t} (52%)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index fab9bcb..ffacd41 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -447,10 +447,14 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
}
/*
* Verify this extension is allowed. We only check duplicates for
- * extensions that we recognise.
+ * extensions that we recognise. We also have a special case for the
+ * PSK extension, which must be the last one in the ClientHello.
*/
if (!verify_extension(s, context, type, exts, raw_extensions, &thisex)
- || (thisex != NULL && thisex->present == 1)) {
+ || (thisex != NULL && thisex->present == 1)
+ || (type == TLSEXT_TYPE_psk
+ && (context & EXT_CLIENT_HELLO) != 0
+ && PACKET_remaining(&extensions) != 0)) {
SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_BAD_EXTENSION);
*al = SSL_AD_ILLEGAL_PARAMETER;
goto err;
diff --git a/test/recipes/70-test_renegotiation.t b/test/recipes/70-test_tls13psk.t
similarity index 52%
copy from test/recipes/70-test_renegotiation.t
copy to test/recipes/70-test_tls13psk.t
index 9bd9026..2607d51 100644
--- a/test/recipes/70-test_renegotiation.t
+++ b/test/recipes/70-test_tls13psk.t
@@ -1,5 +1,5 @@
#! /usr/bin/env perl
-# Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
+# Copyright 2017 The OpenSSL Project Authors. All Rights Reserved.
#
# Licensed under the OpenSSL license (the "License"). You may not use
# this file except in compliance with the License. You can obtain a copy
@@ -7,11 +7,12 @@
# https://www.openssl.org/source/license.html
use strict;
-use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file bldtop_dir/;
+use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file srctop_dir bldtop_dir/;
use OpenSSL::Test::Utils;
+use File::Temp qw(tempfile);
use TLSProxy::Proxy;
-my $test_name = "test_renegotiation";
+my $test_name = "test_tls13psk";
setup($test_name);
plan skip_all => "TLSProxy isn't usable on $^O"
@@ -23,10 +24,12 @@ plan skip_all => "$test_name needs the dynamic engine feature enabled"
plan skip_all => "$test_name needs the sock feature enabled"
if disabled("sock");
-plan skip_all => "$test_name needs TLS <= 1.2 enabled"
- if alldisabled(("ssl3", "tls1", "tls1_1", "tls1_2"));
+plan skip_all => "$test_name needs TLSv1.3 enabled"
+ if disabled("tls1_3");
$ENV{OPENSSL_ia32cap} = '~0x200000200000000';
+$ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
+
my $proxy = TLSProxy::Proxy->new(
undef,
cmdstr(app(["openssl"]), display => 1),
@@ -34,36 +37,36 @@ my $proxy = TLSProxy::Proxy->new(
(!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
);
-#Test 1: A basic renegotiation test
-$proxy->clientflags("-no_tls1_3");
-$proxy->reneg(1);
+#Most PSK tests are done in test_ssl_new. This just checks sending a PSK
+#extension when it isn't in the last place in a ClientHello
+
+#Test 1: First get a session
+(undef, my $session) = tempfile();
+$proxy->clientflags("-sess_out ".$session);
+$proxy->sessionfile($session);
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
plan tests => 2;
-ok(TLSProxy::Message->success(), "Basic renegotiation");
+ok(TLSProxy::Message->success(), "Initial connection");
-#Test 2: Client does not send the Reneg SCSV. Reneg should fail
+#Test 2: Attempt a resume with PSK not in last place. Should fail
$proxy->clear();
-$proxy->filter(\&reneg_filter);
-$proxy->clientflags("-no_tls1_3");
-$proxy->reneg(1);
+$proxy->clientflags("-sess_in ".$session);
+$proxy->filter(\&modify_psk_filter);
$proxy->start();
-ok(TLSProxy::Message->fail(), "No client SCSV");
+ok(TLSProxy::Message->fail(), "PSK not last");
-sub reneg_filter
+unlink $session;
+
+sub modify_psk_filter
{
my $proxy = shift;
- # We're only interested in the initial ClientHello message
- if ($proxy->flight != 0) {
- return;
- }
+ # We're only interested in the initial ClientHello
+ return if ($proxy->flight != 0);
foreach my $message (@{$proxy->message_list}) {
if ($message->mt == TLSProxy::Message::MT_CLIENT_HELLO) {
- #Remove any SCSV ciphersuites - just leave AES128-SHA (0x002f)
- my @ciphersuite = (0x002f);
- $message->ciphersuites(\@ciphersuite);
- $message->ciphersuite_len(2);
+ $message->set_extension(TLSProxy::Message::EXT_FORCE_LAST, "");
$message->repack();
}
}
diff --git a/util/TLSProxy/ClientHello.pm b/util/TLSProxy/ClientHello.pm
index ec739d2..2ae9d6f 100644
--- a/util/TLSProxy/ClientHello.pm
+++ b/util/TLSProxy/ClientHello.pm
@@ -114,6 +114,24 @@ sub process_extensions
}
}
+sub extension_contents
+{
+ my $self = shift;
+ my $key = shift;
+ my $extension = "";
+
+ my $extdata = ${$self->extension_data}{$key};
+ $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;
+}
+
#Reconstruct the on-the-wire message data following changes
sub set_message_contents
{
@@ -131,15 +149,16 @@ sub set_message_contents
$data .= pack("C*", @{$self->comp_meths});
foreach my $key (keys %{$self->extension_data}) {
- my $extdata = ${$self->extension_data}{$key};
- $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;
- }
+ next if ($key == TLSProxy::Message::EXT_PSK);
+ $extensions .= $self->extension_contents($key);
+ }
+ #PSK extension always goes last...
+ if (defined ${$self->extension_data}{TLSProxy::Message::EXT_PSK}) {
+ $extensions .= $self->extension_contents(TLSProxy::Message::EXT_PSK);
+ }
+ #unless we have EXT_FORCE_LAST
+ if (defined ${$self->extension_data}{TLSProxy::Message::EXT_FORCE_LAST}) {
+ $extensions .= $self->extension_contents(TLSProxy::Message::EXT_FORCE_LAST);
}
$data .= pack('n', length($extensions));
diff --git a/util/TLSProxy/Message.pm b/util/TLSProxy/Message.pm
index 88de558..39123fa 100644
--- a/util/TLSProxy/Message.pm
+++ b/util/TLSProxy/Message.pm
@@ -85,7 +85,9 @@ use constant {
# 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_DUPLICATE_EXTENSION => 0xfde8,
+ #Unknown extension that should appear last
+ EXT_FORCE_LAST => 0xffff
};
use constant {
More information about the openssl-commits
mailing list