Add a test to check that if a PSK extension is not last then we fail

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2896)

3 files changed, 104 insertions, 10 deletions

diff --git a/test/recipes/70-test_tls13psk.t b/test/recipes/70-test_tls13psk.t
new file mode 100644
index 0000000000..2607d51f56
--- /dev/null
+++ b/test/recipes/70-test_tls13psk.t
@@ -0,0 +1,73 @@
+#! /usr/bin/env perl
+# 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
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+use strict;
+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_tls13psk";
+setup($test_name);
+
+plan skip_all => "TLSProxy isn't usable on $^O"
+    if $^O =~ /^(VMS|MSWin32)$/;
+
+plan skip_all => "$test_name needs the dynamic engine feature enabled"
+    if disabled("engine") || disabled("dynamic-engine");
+
+plan skip_all => "$test_name needs the sock feature enabled"
+    if disabled("sock");
+
+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),
+    srctop_file("apps", "server.pem"),
+    (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
+);
+
+#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(), "Initial connection");
+
+#Test 2: Attempt a resume with PSK not in last place. Should fail
+$proxy->clear();
+$proxy->clientflags("-sess_in ".$session);
+$proxy->filter(\&modify_psk_filter);
+$proxy->start();
+ok(TLSProxy::Message->fail(), "PSK not last");
+
+unlink $session;
+
+sub modify_psk_filter
+{
+    my $proxy = shift;
+
+    # 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) {
+            $message->set_extension(TLSProxy::Message::EXT_FORCE_LAST, "");
+            $message->repack();
+        }
+    }
+}
diff --git a/util/TLSProxy/ClientHello.pm b/util/TLSProxy/ClientHello.pm
index ec739d2970..2ae9d6f55d 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 88de55844b..39123fabef 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 {