[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Mon Aug 15 22:28:01 UTC 2016
The branch master has been updated
via bb982ce7532eb5f5f8d66211d556940a7f407496 (commit)
via 78fcddbb8d690c515f4525b5220fc63448ecd1a9 (commit)
via a2a0c86bb0d602253d02ded2a848ed69e8cc425a (commit)
via a01c86a25198921c5b8adb45c9379088ace4e42e (commit)
via 44efb88a21d464dba3ac5084c8d4553d696fab33 (commit)
from c35d339d98f969aa88b75124389ba86344eb7e2a (commit)
- Log -----------------------------------------------------------------
commit bb982ce7532eb5f5f8d66211d556940a7f407496
Author: Matt Caswell <matt at openssl.org>
Date: Thu Aug 4 11:31:57 2016 +0100
Remove a stray unneeded line in 70-test_sslrecords.t
Reviewed-by: Tim Hudson <tjh at openssl.org>
commit 78fcddbb8d690c515f4525b5220fc63448ecd1a9
Author: Matt Caswell <matt at openssl.org>
Date: Wed Aug 3 13:03:25 2016 +0100
Address feedback on SSLv2 ClientHello processing
Reviewed-by: Tim Hudson <tjh at openssl.org>
commit a2a0c86bb0d602253d02ded2a848ed69e8cc425a
Author: Matt Caswell <matt at openssl.org>
Date: Tue Aug 2 17:24:54 2016 +0100
Add some SSLv2 ClientHello tests
Test that we handle a TLS ClientHello in an SSLv2 record correctly.
Reviewed-by: Tim Hudson <tjh at openssl.org>
commit a01c86a25198921c5b8adb45c9379088ace4e42e
Author: Matt Caswell <matt at openssl.org>
Date: Tue Aug 2 17:43:32 2016 +0100
Send an alert if we get a non-initial record with the wrong version
If we receive a non-initial record but the version number isn't right then
we should send an alert.
Reviewed-by: Tim Hudson <tjh at openssl.org>
commit 44efb88a21d464dba3ac5084c8d4553d696fab33
Author: Matt Caswell <matt at openssl.org>
Date: Mon Aug 1 17:15:13 2016 +0100
Address feedback on SSLv2 ClientHello processing
Feedback on the previous SSLv2 ClientHello processing fix was that it
breaks layering by reading init_num in the record layer. It also does not
detect if there was a previous non-fatal warning.
This is an alternative approach that directly tracks in the record layer
whether this is the first record.
GitHub Issue #1298
Reviewed-by: Tim Hudson <tjh at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/record/rec_layer_s3.c | 1 +
ssl/record/record.h | 3 +
ssl/record/record_locl.h | 3 +
ssl/record/ssl3_record.c | 29 +++---
test/recipes/70-test_sslrecords.t | 197 +++++++++++++++++++++++++++++++++++++-
util/TLSProxy/Message.pm | 3 +-
util/TLSProxy/Record.pm | 14 ++-
util/TLSProxy/ServerHello.pm | 20 +++-
8 files changed, 248 insertions(+), 22 deletions(-)
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 2d0fca2..c2f9666 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -33,6 +33,7 @@
void RECORD_LAYER_init(RECORD_LAYER *rl, SSL *s)
{
rl->s = s;
+ RECORD_LAYER_set_first_record(&s->rlayer);
SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);
}
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 9177fb4..ce60a15 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -199,6 +199,9 @@ typedef struct record_layer_st {
unsigned char read_sequence[SEQ_NUM_SIZE];
unsigned char write_sequence[SEQ_NUM_SIZE];
+ /* Set to true if this is the first record in a connection */
+ unsigned int is_first_record;
+
DTLS_RECORD_LAYER *d;
} RECORD_LAYER;
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index 435e92a..f9dbc33 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -31,6 +31,9 @@
#define RECORD_LAYER_reset_empty_record_count(rl) \
((rl)->empty_record_count = 0)
#define RECORD_LAYER_get_empty_record_count(rl) ((rl)->empty_record_count)
+#define RECORD_LAYER_is_first_record(rl) ((rl)->is_first_record)
+#define RECORD_LAYER_set_first_record(rl) ((rl)->is_first_record = 1)
+#define RECORD_LAYER_clear_first_record(rl) ((rl)->is_first_record = 0)
#define DTLS_RECORD_LAYER_get_r_epoch(rl) ((rl)->d->r_epoch)
__owur int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold);
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index 49c6756..5f9ce7a 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -159,19 +159,9 @@ int ssl3_get_record(SSL *s)
p = RECORD_LAYER_get_packet(&s->rlayer);
/*
- * Check whether this is a regular record or an SSLv2 style record.
- * The latter can only be used in the first record of an initial
- * ClientHello for old clients. Initial ClientHello means
- * s->first_packet is set and s->server is true. The first record
- * means we've not received any data so far (s->init_num == 0) and
- * have had no empty records. We check s->read_hash and
- * s->enc_read_ctx to ensure this does not apply during
- * renegotiation.
+ * The first record received by the server may be a V2ClientHello.
*/
- if (s->first_packet && s->server
- && s->init_num == 0
- && RECORD_LAYER_get_empty_record_count(&s->rlayer) == 0
- && s->read_hash == NULL && s->enc_read_ctx == NULL
+ if (s->server && RECORD_LAYER_is_first_record(&s->rlayer)
&& (p[0] & 0x80) && (p[2] == SSL2_MT_CLIENT_HELLO)) {
/*
* SSLv2 style record
@@ -239,7 +229,7 @@ int ssl3_get_record(SSL *s)
}
if ((version >> 8) != SSL3_VERSION_MAJOR) {
- if (s->first_packet) {
+ if (RECORD_LAYER_is_first_record(&s->rlayer)) {
/* Go back to start of packet, look at the five bytes
* that we have. */
p = RECORD_LAYER_get_packet(&s->rlayer);
@@ -254,9 +244,17 @@ int ssl3_get_record(SSL *s)
SSL_R_HTTPS_PROXY_REQUEST);
goto err;
}
+
+ /* Doesn't look like TLS - don't send an alert */
+ SSLerr(SSL_F_SSL3_GET_RECORD,
+ SSL_R_WRONG_VERSION_NUMBER);
+ goto err;
+ } else {
+ SSLerr(SSL_F_SSL3_GET_RECORD,
+ SSL_R_WRONG_VERSION_NUMBER);
+ al = SSL_AD_PROTOCOL_VERSION;
+ goto f_err;
}
- SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
- goto err;
}
if (rr[num_recs].length >
@@ -335,6 +333,7 @@ int ssl3_get_record(SSL *s)
/* we have pulled in a full packet so zero things */
RECORD_LAYER_reset_packet_length(&s->rlayer);
+ RECORD_LAYER_clear_first_record(&s->rlayer);
} while (num_recs < max_recs
&& rr[num_recs-1].type == SSL3_RT_APPLICATION_DATA
&& SSL_USE_EXPLICIT_IV(s)
diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t
index 0ae018a..d1c8d3a 100644
--- a/test/recipes/70-test_sslrecords.t
+++ b/test/recipes/70-test_sslrecords.t
@@ -38,7 +38,7 @@ my $proxy = TLSProxy::Proxy->new(
my $content_type = TLSProxy::Record::RT_APPLICATION_DATA;
my $inject_recs_num = 1;
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 4;
+plan tests => 9;
ok(TLSProxy::Message->fail(), "Out of context empty records test");
#Test 2: Injecting in context empty records should succeed
@@ -62,6 +62,51 @@ $proxy->filter(\&add_frag_alert_filter);
$proxy->start();
ok(!TLSProxy::Message->end(), "Fragmented alert records test");
+#Run some SSLv2 ClientHello tests
+
+use constant {
+ TLSV1_2_IN_SSLV2 => 0,
+ SSLV2_IN_SSLV2 => 1,
+ FRAGMENTED_IN_TLSV1_2 => 2,
+ FRAGMENTED_IN_SSLV2 => 3,
+ ALERT_BEFORE_SSLV2 => 4
+};
+#Test 5: Inject an SSLv2 style record format for a TLSv1.2 ClientHello
+my $sslv2testtype = TLSV1_2_IN_SSLV2;
+$proxy->clear();
+$proxy->filter(\&add_sslv2_filter);
+$proxy->start();
+ok(TLSProxy::Message->success(), "TLSv1.2 in SSLv2 ClientHello test");
+
+#Test 6: Inject an SSLv2 style record format for an SSLv2 ClientHello. We don't
+# support this so it should fail. We actually treat it as an unknown
+# protocol so we don't even send an alert in this case.
+$sslv2testtype = SSLV2_IN_SSLV2;
+$proxy->clear();
+$proxy->start();
+ok(!TLSProxy::Message->end(), "SSLv2 in SSLv2 ClientHello test");
+
+#Test 7: Sanity check ClientHello fragmentation. This isn't really an SSLv2 test
+# at all, but it gives us confidence that Test 8 fails for the right
+# reasons
+$sslv2testtype = FRAGMENTED_IN_TLSV1_2;
+$proxy->clear();
+$proxy->start();
+ok(TLSProxy::Message->success(), "Fragmented ClientHello in TLSv1.2 test");
+
+#Test 8: Fragment a TLSv1.2 ClientHello across a TLS1.2 record; an SSLv2
+# record; and another TLS1.2 record. This isn't allowed so should fail
+$sslv2testtype = FRAGMENTED_IN_SSLV2;
+$proxy->clear();
+$proxy->start();
+ok(TLSProxy::Message->fail(), "Fragmented ClientHello in TLSv1.2/SSLv2 test");
+
+#Test 9: Send a TLS warning alert before an SSLv2 ClientHello. This should
+# fail because an SSLv2 ClientHello must be the first record.
+$sslv2testtype = ALERT_BEFORE_SSLV2;
+$proxy->clear();
+$proxy->start();
+ok(TLSProxy::Message->fail(), "Alert before SSLv2 ClientHello test");
sub add_empty_recs_filter
{
my $proxy = shift;
@@ -79,6 +124,7 @@ sub add_empty_recs_filter
0,
0,
0,
+ 0,
"",
""
);
@@ -117,6 +163,7 @@ sub add_frag_alert_filter
TLSProxy::Record::RT_ALERT,
TLSProxy::Record::VERS_TLS_1_2,
1,
+ 0,
1,
1,
$byte,
@@ -131,6 +178,7 @@ sub add_frag_alert_filter
TLSProxy::Record::RT_ALERT,
TLSProxy::Record::VERS_TLS_1_2,
1,
+ 0,
1,
1,
$byte,
@@ -138,3 +186,150 @@ sub add_frag_alert_filter
);
push @{$proxy->record_list}, $record;
}
+
+sub add_sslv2_filter
+{
+ my $proxy = shift;
+ my $clienthello;
+ my $record;
+
+ # We're only interested in the initial ClientHello
+ if ($proxy->flight != 0) {
+ return;
+ }
+
+ # Ditch the real ClientHello - we're going to replace it with our own
+ shift @{$proxy->record_list};
+
+ if ($sslv2testtype == ALERT_BEFORE_SSLV2) {
+ my $alert = pack('CC', TLSProxy::Message::AL_LEVEL_FATAL,
+ TLSProxy::Message::AL_DESC_NO_RENEGOTIATION);
+ my $alertlen = length $alert;
+ $record = TLSProxy::Record->new(
+ 0,
+ TLSProxy::Record::RT_ALERT,
+ TLSProxy::Record::VERS_TLS_1_2,
+ $alertlen,
+ 0,
+ $alertlen,
+ $alertlen,
+ $alert,
+ $alert
+ );
+
+ push @{$proxy->record_list}, $record;
+ }
+
+ if ($sslv2testtype == ALERT_BEFORE_SSLV2
+ || $sslv2testtype == TLSV1_2_IN_SSLV2
+ || $sslv2testtype == SSLV2_IN_SSLV2) {
+ # This is an SSLv2 format ClientHello
+ $clienthello =
+ pack "C44",
+ 0x01, # ClientHello
+ 0x03, 0x03, #TLSv1.2
+ 0x00, 0x03, # Ciphersuites len
+ 0x00, 0x00, # Session id len
+ 0x00, 0x20, # Challenge len
+ 0x00, 0x00, 0x2f, #AES128-SHA
+ 0x01, 0x18, 0x9F, 0x76, 0xEC, 0x57, 0xCE, 0xE5, 0xB3, 0xAB, 0x79, 0x90,
+ 0xAD, 0xAC, 0x6E, 0xD1, 0x58, 0x35, 0x03, 0x97, 0x16, 0x10, 0x82, 0x56,
+ 0xD8, 0x55, 0xFF, 0xE1, 0x8A, 0xA3, 0x2E, 0xF6; # Challenge
+
+ if ($sslv2testtype == SSLV2_IN_SSLV2) {
+ # Set the version to "real" SSLv2
+ vec($clienthello, 1, 8) = 0x00;
+ vec($clienthello, 2, 8) = 0x02;
+ }
+
+ my $chlen = length $clienthello;
+
+ $record = TLSProxy::Record->new(
+ 0,
+ TLSProxy::Record::RT_HANDSHAKE,
+ TLSProxy::Record::VERS_TLS_1_2,
+ $chlen,
+ 1, #SSLv2
+ $chlen,
+ $chlen,
+ $clienthello,
+ $clienthello
+ );
+
+ push @{$proxy->record_list}, $record;
+ } else {
+ # For this test we're using a real TLS ClientHello
+ $clienthello =
+ pack "C49",
+ 0x01, # ClientHello
+ 0x00, 0x00, 0x2D, # Message length
+ 0x03, 0x03, # TLSv1.2
+ 0x01, 0x18, 0x9F, 0x76, 0xEC, 0x57, 0xCE, 0xE5, 0xB3, 0xAB, 0x79, 0x90,
+ 0xAD, 0xAC, 0x6E, 0xD1, 0x58, 0x35, 0x03, 0x97, 0x16, 0x10, 0x82, 0x56,
+ 0xD8, 0x55, 0xFF, 0xE1, 0x8A, 0xA3, 0x2E, 0xF6, # Random
+ 0x00, # Session id len
+ 0x00, 0x04, # Ciphersuites len
+ 0x00, 0x2f, # AES128-SHA
+ 0x00, 0xff, # Empty reneg info SCSV
+ 0x01, # Compression methods len
+ 0x00, # Null compression
+ 0x00, 0x00; # Extensions len
+
+ # Split this into 3: A TLS record; a SSLv2 record and a TLS record.
+ # We deliberately split the second record prior to the Challenge/Random
+ # and set the first byte of the random to 1. This makes the second SSLv2
+ # record look like an SSLv2 ClientHello
+ my $frag1 = substr $clienthello, 0, 6;
+ my $frag2 = substr $clienthello, 6, 32;
+ my $frag3 = substr $clienthello, 38;
+
+ my $fraglen = length $frag1;
+ $record = TLSProxy::Record->new(
+ 0,
+ TLSProxy::Record::RT_HANDSHAKE,
+ TLSProxy::Record::VERS_TLS_1_2,
+ $fraglen,
+ 0,
+ $fraglen,
+ $fraglen,
+ $frag1,
+ $frag1
+ );
+ push @{$proxy->record_list}, $record;
+
+ $fraglen = length $frag2;
+ my $recvers;
+ if ($sslv2testtype == FRAGMENTED_IN_SSLV2) {
+ $recvers = 1;
+ } else {
+ $recvers = 0;
+ }
+ $record = TLSProxy::Record->new(
+ 0,
+ TLSProxy::Record::RT_HANDSHAKE,
+ TLSProxy::Record::VERS_TLS_1_2,
+ $fraglen,
+ $recvers,
+ $fraglen,
+ $fraglen,
+ $frag2,
+ $frag2
+ );
+ push @{$proxy->record_list}, $record;
+
+ $fraglen = length $frag3;
+ $record = TLSProxy::Record->new(
+ 0,
+ TLSProxy::Record::RT_HANDSHAKE,
+ TLSProxy::Record::VERS_TLS_1_2,
+ $fraglen,
+ 0,
+ $fraglen,
+ $fraglen,
+ $frag3,
+ $frag3
+ );
+ push @{$proxy->record_list}, $record;
+ }
+
+}
diff --git a/util/TLSProxy/Message.pm b/util/TLSProxy/Message.pm
index b8db22f..321e080 100644
--- a/util/TLSProxy/Message.pm
+++ b/util/TLSProxy/Message.pm
@@ -37,7 +37,8 @@ use constant {
#Alert descriptions
use constant {
AL_DESC_CLOSE_NOTIFY => 0,
- AL_DESC_UNEXPECTED_MESSAGE => 10
+ AL_DESC_UNEXPECTED_MESSAGE => 10,
+ AL_DESC_NO_RENEGOTIATION => 100
};
my %message_type = (
diff --git a/util/TLSProxy/Record.pm b/util/TLSProxy/Record.pm
index 2a605e3..423bad3 100644
--- a/util/TLSProxy/Record.pm
+++ b/util/TLSProxy/Record.pm
@@ -98,6 +98,7 @@ sub get_records
$content_type,
$version,
$len,
+ 0,
$len_real,
$decrypt_len,
substr($packet, TLS_RECORD_HEADER_LENGTH, $len_real),
@@ -167,6 +168,7 @@ sub new
$content_type,
$version,
$len,
+ $sslv2,
$len_real,
$decrypt_len,
$data,
@@ -177,6 +179,7 @@ sub new
content_type => $content_type,
version => $version,
len => $len,
+ sslv2 => $sslv2,
len_real => $len_real,
decrypt_len => $decrypt_len,
data => $data,
@@ -247,7 +250,11 @@ sub reconstruct_record
my $self = shift;
my $data;
- $data = pack('Cnn', $self->content_type, $self->version, $self->len);
+ if ($self->sslv2) {
+ $data = pack('n', $self->len | 0x8000);
+ } else {
+ $data = pack('Cnn', $self->content_type, $self->version, $self->len);
+ }
$data .= $self->data;
return $data;
@@ -269,6 +276,11 @@ sub version
my $self = shift;
return $self->{version};
}
+sub sslv2
+{
+ my $self = shift;
+ return $self->{sslv2};
+}
sub len_real
{
my $self = shift;
diff --git a/util/TLSProxy/ServerHello.pm b/util/TLSProxy/ServerHello.pm
index ee2fd72..79a8be9 100644
--- a/util/TLSProxy/ServerHello.pm
+++ b/util/TLSProxy/ServerHello.pm
@@ -56,13 +56,25 @@ sub parse
my $comp_meth = unpack('C', substr($self->data, $ptr));
$ptr++;
my $extensions_len = unpack('n', substr($self->data, $ptr));
- $ptr += 2;
+ if (!defined $extensions_len) {
+ $extensions_len = 0;
+ } else {
+ $ptr += 2;
+ }
#For now we just deal with this as a block of data. In the future we will
#want to parse this
- my $extension_data = substr($self->data, $ptr);
+ my $extension_data;
+ if ($extensions_len != 0) {
+ $extension_data = substr($self->data, $ptr);
- if (length($extension_data) != $extensions_len) {
- die "Invalid extension length\n";
+ if (length($extension_data) != $extensions_len) {
+ die "Invalid extension length\n";
+ }
+ } else {
+ if (length($self->data) != $ptr) {
+ die "Invalid extension length\n";
+ }
+ $extension_data = "";
}
my %extensions = ();
while (length($extension_data) >= 4) {
More information about the openssl-commits
mailing list