[openssl-commits] [openssl] master update
Emilia Kasper
emilia at openssl.org
Mon Sep 28 14:06:03 UTC 2015
The branch master has been updated
via cf7f85927c756978f8a032aa870db47078dd29ab (commit)
via 7f6d90ac751e2dff6c1a7aad94ce9c5fdd0eb725 (commit)
via e711da714b0add2c9c3cb67a8c2500002fff9105 (commit)
from 51cbee35162aecb4ca37ea9688461c79f975aff5 (commit)
- Log -----------------------------------------------------------------
commit cf7f85927c756978f8a032aa870db47078dd29ab
Author: Emilia Kasper <emilia at openssl.org>
Date: Wed Sep 16 17:47:55 2015 +0200
Empty NewSessionTicket: test session resumption
Reviewed-by: Matt Caswell <matt at openssl.org>
commit 7f6d90ac751e2dff6c1a7aad94ce9c5fdd0eb725
Author: Emilia Kasper <emilia at openssl.org>
Date: Tue Sep 15 12:06:12 2015 +0200
Empty session ticket: add a test
Reviewed-by: Matt Caswell <matt at openssl.org>
commit e711da714b0add2c9c3cb67a8c2500002fff9105
Author: Emilia Kasper <emilia at openssl.org>
Date: Thu Sep 10 16:32:51 2015 +0200
RT2772: accept empty SessionTicket
RFC 5077 section 3.3 says:
If the server determines that it does not want to include a
ticket after it has included the SessionTicket extension in the
ServerHello, then it sends a zero-length ticket in the
NewSessionTicket handshake message.
Previously the client would fail upon attempting to allocate a
zero-length buffer. Now, we have the client ignore the empty ticket and
keep the existing session.
Reviewed-by: Matt Caswell <matt at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/s3_clnt.c | 23 ++++--
test/recipes/70-test_sslsessiontick.t | 77 +++++++++++++++++-
util/TLSProxy/Message.pm | 46 ++++++++---
.../TLSProxy/NewSessionTicket.pm | 94 ++++++++++++++--------
util/TLSProxy/Proxy.pm | 24 +++---
util/TLSProxy/ServerHello.pm | 26 +++---
6 files changed, 210 insertions(+), 80 deletions(-)
copy test/recipes/70-test_sslskewith0p.t => util/TLSProxy/NewSessionTicket.pm (66%)
mode change 100755 => 100644
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 036a531..2c93bd0 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -2195,6 +2195,7 @@ int ssl3_get_new_session_ticket(SSL *s)
{
int ok, al, ret = 0;
unsigned int ticklen;
+ unsigned long ticket_lifetime_hint;
long n;
PACKET pkt;
@@ -2212,6 +2213,18 @@ int ssl3_get_new_session_ticket(SSL *s)
goto f_err;
}
+ if (!PACKET_get_net_4(&pkt, &ticket_lifetime_hint)
+ || !PACKET_get_net_2(&pkt, &ticklen)
+ || PACKET_remaining(&pkt) != ticklen) {
+ al = SSL_AD_DECODE_ERROR;
+ SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, SSL_R_LENGTH_MISMATCH);
+ goto f_err;
+ }
+
+ /* Server is allowed to change its mind and send an empty ticket. */
+ if (ticklen == 0)
+ return 1;
+
if (s->session->session_id_length > 0) {
int i = s->session_ctx->session_cache_mode;
SSL_SESSION *new_sess;
@@ -2243,15 +2256,9 @@ int ssl3_get_new_session_ticket(SSL *s)
s->session = new_sess;
}
- if (!PACKET_get_net_4(&pkt, &s->session->tlsext_tick_lifetime_hint)
- || !PACKET_get_net_2(&pkt, &ticklen)
- || PACKET_remaining(&pkt) != ticklen) {
- al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, SSL_R_LENGTH_MISMATCH);
- goto f_err;
- }
OPENSSL_free(s->session->tlsext_tick);
s->session->tlsext_ticklen = 0;
+
s->session->tlsext_tick = OPENSSL_malloc(ticklen);
if (!s->session->tlsext_tick) {
SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);
@@ -2262,6 +2269,8 @@ int ssl3_get_new_session_ticket(SSL *s)
SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, SSL_R_LENGTH_MISMATCH);
goto f_err;
}
+
+ s->session->tlsext_tick_lifetime_hint = ticket_lifetime_hint;
s->session->tlsext_ticklen = ticklen;
/*
* There are two ways to detect a resumed ticket session. One is to set
diff --git a/test/recipes/70-test_sslsessiontick.t b/test/recipes/70-test_sslsessiontick.t
index 7f90bea..a7a450a 100755
--- a/test/recipes/70-test_sslsessiontick.t
+++ b/test/recipes/70-test_sslsessiontick.t
@@ -82,7 +82,7 @@ my $proxy = TLSProxy::Proxy->new(
top_file("apps", "server.pem")
);
-plan tests => 5;
+plan tests => 8;
#Test 1: By default with no existing session we should get a session ticket
#Expected result: ClientHello extension seen; ServerHello extension seen
@@ -135,6 +135,79 @@ $proxy->clientstart();
checkmessages(5, "Session resumption with ticket capable client without a "
."ticket", 1, 1, 1, 0);
+#Test 6: Client accepts empty ticket.
+#Expected result: ClientHello extension seen; ServerHello extension seen;
+# NewSessionTicket message seen; Full handshake.
+clearall();
+$proxy->filter(\&ticket_filter);
+$proxy->start();
+checkmessages(6, "Empty ticket test", 1, 1, 1, 1);
+
+#Test 7-8: Client keeps existing ticket on empty ticket.
+clearall();
+($fh, $session) = tempfile();
+$proxy->serverconnects(3);
+$proxy->filter(undef);
+$proxy->clientflags("-sess_out ".$session);
+$proxy->start();
+$proxy->clear();
+$proxy->clientflags("-sess_in ".$session." -sess_out ".$session);
+$proxy->filter(\&inject_empty_ticket_filter);
+$proxy->clientstart();
+#Expected result: ClientHello extension seen; ServerHello extension seen;
+# NewSessionTicket message seen; Abbreviated handshake.
+checkmessages(7, "Empty ticket resumption test", 1, 1, 1, 0);
+clearall();
+$proxy->clientflags("-sess_in ".$session);
+$proxy->filter(undef);
+$proxy->clientstart();
+#Expected result: ClientHello extension seen; ServerHello extension not seen;
+# NewSessionTicket message not seen; Abbreviated handshake.
+checkmessages(8, "Empty ticket resumption test", 1, 0, 0, 0);
+
+
+sub ticket_filter
+{
+ my $proxy = shift;
+
+ foreach my $message (@{$proxy->message_list}) {
+ if ($message->mt == TLSProxy::Message::MT_NEW_SESSION_TICKET) {
+ $message->ticket("");
+ $message->repack();
+ }
+ }
+}
+
+sub inject_empty_ticket_filter {
+ my $proxy = shift;
+
+ foreach my $message (@{$proxy->message_list}) {
+ if ($message->mt == TLSProxy::Message::MT_NEW_SESSION_TICKET) {
+ # Only inject the message first time we're called.
+ return;
+ }
+ }
+
+ my @new_message_list = ();
+ foreach my $message (@{$proxy->message_list}) {
+ push @new_message_list, $message;
+ if ($message->mt == TLSProxy::Message::MT_SERVER_HELLO) {
+ $message->set_extension(TLSProxy::ClientHello::EXT_SESSION_TICKET, "");
+ $message->repack();
+ # Tack NewSessionTicket onto the ServerHello record.
+ # This only works if the ServerHello is exactly one record.
+ my $record = ${$message->records}[0];
+
+ my $offset = $message->startoffset + $message->encoded_length;
+ my $newsessionticket = TLSProxy::NewSessionTicket->new(
+ 1, "", [$record], $offset, []);
+ $newsessionticket->repack();
+ push @new_message_list, $newsessionticket;
+ }
+ }
+ $proxy->message_list([@new_message_list]);
+}
+
sub checkmessages($$$$$$)
{
my ($testno, $testname, $testch, $testsh, $testtickseen, $testhand) = @_;
@@ -164,7 +237,7 @@ sub checkmessages($$$$$$)
plan tests => 5;
- ok(TLSProxy::Message->success, "Hanshake");
+ ok(TLSProxy::Message->success, "Handshake");
ok(($testch && $chellotickext) || (!$testch && !$chellotickext),
"ClientHello extension Session Ticket check");
ok(($testsh && $shellotickext) || (!$testsh && !$shellotickext),
diff --git a/util/TLSProxy/Message.pm b/util/TLSProxy/Message.pm
index 6376219..ddd0a6d 100644
--- a/util/TLSProxy/Message.pm
+++ b/util/TLSProxy/Message.pm
@@ -282,6 +282,15 @@ sub create_message
[@message_frag_lens]
);
$message->parse();
+ } elsif ($mt == MT_NEW_SESSION_TICKET) {
+ $message = TLSProxy::NewSessionTicket->new(
+ $server,
+ $data,
+ [@message_rec_list],
+ $startoffset,
+ [@message_frag_lens]
+ );
+ $message->parse();
} else {
#Unknown message type
$message = TLSProxy::Message->new(
@@ -361,24 +370,34 @@ sub repack
$lenhi = length($self->data) >> 8;
$msgdata = pack('CnC', $self->mt, $lenhi, $lenlo).$self->data;
-
if ($numrecs == 0) {
#The message is fully contained within one record
my ($rec) = @{$self->records};
my $recdata = $rec->decrypt_data;
- if (length($msgdata) != ${$self->message_frag_lens}[0]
- + TLS_MESSAGE_HEADER_LENGTH) {
- #Message length has changed! Better adjust the record length
- my $diff = length($msgdata) - ${$self->message_frag_lens}[0]
- - TLS_MESSAGE_HEADER_LENGTH;
- $rec->len($rec->len + $diff);
+ my $old_length;
+
+ # We use empty message_frag_lens to indicates that pre-repacking,
+ # the message wasn't present. The first fragment length doesn't include
+ # the TLS header, so we need to check and compute the right length.
+ if (@{$self->message_frag_lens}) {
+ $old_length = ${$self->message_frag_lens}[0] +
+ TLS_MESSAGE_HEADER_LENGTH;
+ } else {
+ $old_length = 0;
}
- $rec->data(substr($recdata, 0, $self->startoffset)
- .($msgdata)
- .substr($recdata, ${$self->message_frag_lens}[0]
- + TLS_MESSAGE_HEADER_LENGTH));
+ my $prefix = substr($recdata, 0, $self->startoffset);
+ my $suffix = substr($recdata, $self->startoffset + $old_length);
+
+ $rec->decrypt_data($prefix.($msgdata).($suffix));
+ # TODO(openssl-team): don't keep explicit lengths.
+ # (If a length override is ever needed to construct invalid packets,
+ # use an explicit override field instead.)
+ $rec->decrypt_len(length($rec->decrypt_data));
+ $rec->len($rec->len + length($msgdata) - $old_length);
+ # Don't support re-encryption.
+ $rec->data($rec->decrypt_data);
#Update the fragment len in case we changed it above
${$self->message_frag_lens}[0] = length($msgdata)
@@ -462,5 +481,10 @@ sub message_frag_lens
}
return $self->{message_frag_lens};
}
+sub encoded_length
+{
+ my $self = shift;
+ return TLS_MESSAGE_HEADER_LENGTH + length($self->data);
+}
1;
diff --git a/test/recipes/70-test_sslskewith0p.t b/util/TLSProxy/NewSessionTicket.pm
old mode 100755
new mode 100644
similarity index 66%
copy from test/recipes/70-test_sslskewith0p.t
copy to util/TLSProxy/NewSessionTicket.pm
index d8d74b3..75dbf23
--- a/test/recipes/70-test_sslskewith0p.t
+++ b/util/TLSProxy/NewSessionTicket.pm
@@ -1,5 +1,3 @@
-#!/usr/bin/perl
-# Written by Matt Caswell for the OpenSSL project.
# ====================================================================
# Copyright (c) 1998-2015 The OpenSSL Project. All rights reserved.
#
@@ -53,49 +51,75 @@
# Hudson (tjh at cryptsoft.com).
use strict;
-use OpenSSL::Test qw/:DEFAULT cmdstr top_file top_dir/;
-use TLSProxy::Proxy;
-my $test_name = "test_sslskewith0p";
-setup($test_name);
+package TLSProxy::NewSessionTicket;
-plan skip_all => "$test_name can only be performed with OpenSSL configured shared"
- unless (map { chomp; s/^SHARED_LIBS=\s*//; $_ }
- grep { /^SHARED_LIBS=/ }
- do { local @ARGV = ( top_file("Makefile") ); <> })[0] ne "";
+use parent 'TLSProxy::Message';
-$ENV{OPENSSL_ENGINES} = top_dir("engines");
-$ENV{OPENSSL_ia32cap} = '~0x200000200000000';
-my $proxy = TLSProxy::Proxy->new(
- \&ske_0_p_filter,
- cmdstr(app(["openssl"])),
- top_file("apps", "server.pem")
-);
+sub new
+{
+ my $class = shift;
+ my ($server,
+ $data,
+ $records,
+ $startoffset,
+ $message_frag_lens) = @_;
-plan tests => 1;
+ my $self = $class->SUPER::new(
+ $server,
+ TLSProxy::Message::MT_NEW_SESSION_TICKET,
+ $data,
+ $records,
+ $startoffset,
+ $message_frag_lens);
-#We must use an anon DHE cipher for this test
-$proxy->cipherc('ADH-AES128-SHA:@SECLEVEL=0');
-$proxy->ciphers('ADH-AES128-SHA:@SECLEVEL=0');
+ $self->{ticket_lifetime_hint} = 0;
+ $self->{ticket} = "";
-$proxy->start();
-ok(TLSProxy::Message->fail, "ServerKeyExchange with 0 p");
+ return $self;
+}
-sub ske_0_p_filter
+sub parse
{
- my $proxy = shift;
+ my $self = shift;
- # We're only interested in the SKE - always in flight 1
- if ($proxy->flight != 1) {
- return;
- }
+ my $ticket_lifetime_hint = unpack('N', $self->data);
+ my $ticket_len = unpack('n', $self->data);
+ my $ticket = substr($self->data, 6, $ticket_len);
+
+ $self->ticket_lifetime_hint($ticket_lifetime_hint);
+ $self->ticket($ticket);
+}
+
+
+#Reconstruct the on-the-wire message data following changes
+sub set_message_contents
+{
+ my $self = shift;
+ my $data;
+
+ $data = pack('N', $self->ticket_lifetime_hint);
+ $data .= pack('n', length($self->ticket));
+ $data .= $self->ticket;
- foreach my $message (@{$proxy->message_list}) {
- if ($message->mt == TLSProxy::Message::MT_SERVER_KEY_EXCHANGE) {
- #Set p to a value of 0
- $message->p(pack('C', 0));
+ $self->data($data);
+}
- $message->repack();
- }
+#Read/write accessors
+sub ticket_lifetime_hint
+{
+ my $self = shift;
+ if (@_) {
+ $self->{ticket_lifetime_hint} = shift;
+ }
+ return $self->{ticket_lifetime_hint};
+}
+sub ticket
+{
+ my $self = shift;
+ if (@_) {
+ $self->{ticket} = shift;
}
+ return $self->{ticket};
}
+1;
diff --git a/util/TLSProxy/Proxy.pm b/util/TLSProxy/Proxy.pm
index 6c1ea77..1e90e66 100644
--- a/util/TLSProxy/Proxy.pm
+++ b/util/TLSProxy/Proxy.pm
@@ -63,6 +63,7 @@ use TLSProxy::Message;
use TLSProxy::ClientHello;
use TLSProxy::ServerHello;
use TLSProxy::ServerKeyExchange;
+use TLSProxy::NewSessionTicket;
sub new
{
@@ -92,9 +93,6 @@ sub new
flight => 0,
record_list => [],
message_list => [],
-
- #Private
- message_rec_list => []
};
return bless $self, $class;
@@ -109,7 +107,6 @@ sub clear
$self->{flight} = 0;
$self->{record_list} = [];
$self->{message_list} = [];
- $self->{message_rec_list} = [];
$self->{serverflags} = "";
$self->{clientflags} = "";
$self->{serverconnects} = 1;
@@ -273,7 +270,6 @@ sub clientstart
}
}
-
sub process_packet
{
my ($self, $server, $packet) = @_;
@@ -295,7 +291,6 @@ sub process_packet
#list of messages in those records
my @ret = TLSProxy::Record->get_records($server, $self->flight, $packet);
push @{$self->record_list}, @{$ret[0]};
- $self->{message_rec_list} = $ret[0];
push @{$self->{message_list}}, @{$ret[1]};
print "\n";
@@ -348,11 +343,6 @@ sub record_list
my $self = shift;
return $self->{record_list};
}
-sub message_list
-{
- my $self = shift;
- return $self->{message_list};
-}
sub success
{
my $self = shift;
@@ -445,4 +435,16 @@ sub serverconnects
}
return $self->{serverconnects};
}
+# This is a bit ugly because the caller is responsible for keeping the records
+# in sync with the updated message list; simply updating the message list isn't
+# sufficient to get the proxy to forward the new message.
+# But it does the trick for the one test (test_sslsessiontick) that needs it.
+sub message_list
+{
+ my $self = shift;
+ if (@_) {
+ $self->{message_list} = shift;
+ }
+ return $self->{message_list};
+}
1;
diff --git a/util/TLSProxy/ServerHello.pm b/util/TLSProxy/ServerHello.pm
index 693430e..56b8a34 100644
--- a/util/TLSProxy/ServerHello.pm
+++ b/util/TLSProxy/ServerHello.pm
@@ -80,7 +80,6 @@ sub new
$self->{session} = "";
$self->{ciphersuite} = 0;
$self->{comp_meth} = 0;
- $self->{extensions_len} = 0;
$self->{extensions_data} = "";
return $self;
@@ -124,7 +123,6 @@ sub parse
$self->session($session);
$self->ciphersuite($ciphersuite);
$self->comp_meth($comp_meth);
- $self->extensions_len($extensions_len);
$self->extension_data(\%extensions);
$self->process_data();
@@ -149,6 +147,7 @@ sub set_message_contents
{
my $self = shift;
my $data;
+ my $extensions = "";
$data = pack('n', $self->server_version);
$data .= $self->random;
@@ -156,14 +155,16 @@ sub set_message_contents
$data .= $self->session;
$data .= pack('n', $self->ciphersuite);
$data .= pack('C', $self->comp_meth);
- $data .= pack('n', $self->extensions_len);
+
foreach my $key (keys %{$self->extension_data}) {
my $extdata = ${$self->extension_data}{$key};
- $data .= pack("n", $key);
- $data .= pack("n", length($extdata));
- $data .= $extdata;
+ $extensions .= pack("n", $key);
+ $extensions .= pack("n", length($extdata));
+ $extensions .= $extdata;
}
+ $data .= pack('n', length($extensions));
+ $data .= $extensions;
$self->data($data);
}
@@ -216,14 +217,6 @@ sub comp_meth
}
return $self->{comp_meth};
}
-sub extensions_len
-{
- my $self = shift;
- if (@_) {
- $self->{extensions_len} = shift;
- }
- return $self->{extensions_len};
-}
sub extension_data
{
my $self = shift;
@@ -232,4 +225,9 @@ sub extension_data
}
return $self->{extension_data};
}
+sub set_extension
+{
+ my ($self, $ext_type, $ext_data) = @_;
+ $self->{extension_data}{$ext_type} = $ext_data;
+}
1;
More information about the openssl-commits
mailing list