[openssl] master update
Matt Caswell
matt at openssl.org
Tue Feb 19 09:41:49 UTC 2019
The branch master has been updated
via 73e62d40eb53f2bad98dea0083c217dbfad1a335 (commit)
via 3d35e3a253a2895f263333bb4355760630a31955 (commit)
from 4ce738d083a377e0788e5d6cf92e3756d436b2f4 (commit)
- Log -----------------------------------------------------------------
commit 73e62d40eb53f2bad98dea0083c217dbfad1a335
Author: Matt Caswell <matt at openssl.org>
Date: Fri Feb 8 17:25:58 2019 +0000
Add a test for interleaving app data with handshake data in TLSv1.3
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/8191)
commit 3d35e3a253a2895f263333bb4355760630a31955
Author: Matt Caswell <matt at openssl.org>
Date: Fri Feb 8 16:36:32 2019 +0000
Don't interleave handshake and other record types in TLSv1.3
In TLSv1.3 it is illegal to interleave handshake records with non handshake
records.
Fixes #8189
Reviewed-by: Ben Kaduk <kaduk at mit.edu>
(Merged from https://github.com/openssl/openssl/pull/8191)
-----------------------------------------------------------------------
Summary of changes:
crypto/err/openssl.txt | 2 +
include/openssl/sslerr.h | 3 +-
ssl/record/rec_layer_s3.c | 8 +++
ssl/ssl_err.c | 4 +-
test/recipes/70-test_sslrecords.t | 102 +++++++++++++++++++++++++++++++++++---
5 files changed, 111 insertions(+), 8 deletions(-)
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index e944b57..82c33f0 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2796,6 +2796,8 @@ SSL_R_MISSING_SRP_PARAM:358:can't find SRP server param
SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION:209:missing supported groups extension
SSL_R_MISSING_TMP_DH_KEY:171:missing tmp dh key
SSL_R_MISSING_TMP_ECDH_KEY:311:missing tmp ecdh key
+SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA:293:\
+ mixed handshake and non handshake data
SSL_R_NOT_ON_RECORD_BOUNDARY:182:not on record boundary
SSL_R_NOT_REPLACING_CERTIFICATE:289:not replacing certificate
SSL_R_NOT_SERVER:284:not server
diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index f878371..6305751 100644
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
*
* Licensed under the Apache License 2.0 (the "License"). You may not use
* this file except in compliance with the License. You can obtain a copy
@@ -596,6 +596,7 @@ int ERR_load_SSL_strings(void);
# define SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION 209
# define SSL_R_MISSING_TMP_DH_KEY 171
# define SSL_R_MISSING_TMP_ECDH_KEY 311
+# define SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA 293
# define SSL_R_NOT_ON_RECORD_BOUNDARY 182
# define SSL_R_NOT_REPLACING_CERTIFICATE 289
# define SSL_R_NOT_SERVER 284
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 2f5987b..feca76e 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1363,6 +1363,14 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
} while (num_recs == 0);
rr = &rr[curr_rec];
+ if (s->rlayer.handshake_fragment_len > 0
+ && SSL3_RECORD_get_type(rr) != SSL3_RT_HANDSHAKE
+ && SSL_IS_TLS13(s)) {
+ SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_READ_BYTES,
+ SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA);
+ return -1;
+ }
+
/*
* Reset the count of consecutive warning alerts if we've got a non-empty
* record that isn't an alert.
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 7b06878..ceae87b 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
*
* Licensed under the Apache License 2.0 (the "License"). You may not use
* this file except in compliance with the License. You can obtain a copy
@@ -965,6 +965,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_TMP_DH_KEY), "missing tmp dh key"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_TMP_ECDH_KEY),
"missing tmp ecdh key"},
+ {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA),
+ "mixed handshake and non handshake data"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NOT_ON_RECORD_BOUNDARY),
"not on record boundary"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NOT_REPLACING_CERTIFICATE),
diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t
index 0e38308..b0ad026 100644
--- a/test/recipes/70-test_sslrecords.t
+++ b/test/recipes/70-test_sslrecords.t
@@ -44,7 +44,7 @@ my $content_type = TLSProxy::Record::RT_APPLICATION_DATA;
my $inject_recs_num = 1;
$proxy->serverflags("-tls1_2");
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 18;
+plan tests => 20;
ok($fatal_alert, "Out of context empty records test");
#Test 2: Injecting in context empty records should succeed
@@ -155,7 +155,7 @@ ok($fatal_alert, "Changed record version in TLS1.2");
#TLS1.3 specific tests
SKIP: {
- skip "TLSv1.3 disabled", 6 if disabled("tls1_3");
+ skip "TLSv1.3 disabled", 8 if disabled("tls1_3");
#Test 13: Sending a different record version in TLS1.3 should fail
$proxy->clear();
@@ -181,7 +181,9 @@ SKIP: {
use constant {
DATA_AFTER_SERVER_HELLO => 0,
DATA_AFTER_FINISHED => 1,
- DATA_AFTER_KEY_UPDATE => 2
+ DATA_AFTER_KEY_UPDATE => 2,
+ DATA_BETWEEN_KEY_UPDATE => 3,
+ NO_DATA_BETWEEN_KEY_UPDATE => 4,
};
#Test 16: Sending a ServerHello which doesn't end on a record boundary
@@ -198,7 +200,6 @@ SKIP: {
$fatal_alert = 0;
$proxy->clear();
$boundary_test_type = DATA_AFTER_FINISHED;
- $proxy->filter(\¬_on_record_boundary);
$proxy->start();
ok($fatal_alert, "Record not on boundary in TLS1.3 (Finished)");
@@ -207,9 +208,24 @@ SKIP: {
$fatal_alert = 0;
$proxy->clear();
$boundary_test_type = DATA_AFTER_KEY_UPDATE;
- $proxy->filter(\¬_on_record_boundary);
$proxy->start();
ok($fatal_alert, "Record not on boundary in TLS1.3 (KeyUpdate)");
+
+ #Test 19: Sending application data in the middle of a fragmented KeyUpdate
+ # should fail. Strictly speaking this is not a record boundary test
+ # but we use the same filter.
+ $fatal_alert = 0;
+ $proxy->clear();
+ $boundary_test_type = DATA_BETWEEN_KEY_UPDATE;
+ $proxy->start();
+ ok($fatal_alert, "Data between KeyUpdate");
+
+ #Test 20: Fragmented KeyUpdate. This should succeed. Strictly speaking this
+ # is not a record boundary test but we use the same filter.
+ $proxy->clear();
+ $boundary_test_type = NO_DATA_BETWEEN_KEY_UPDATE;
+ $proxy->start();
+ ok(TLSProxy::Message->success(), "No data between KeyUpdate");
}
@@ -573,7 +589,7 @@ sub not_on_record_boundary
#Update the record
$last_record->data($data);
$last_record->len(length $data);
- } else {
+ } elsif ($boundary_test_type == DATA_AFTER_KEY_UPDATE) {
return if @{$proxy->{message_list}}[-1]->{mt}
!= TLSProxy::Message::MT_FINISHED;
@@ -605,5 +621,79 @@ sub not_on_record_boundary
$record->data($data);
$record->len(length $data);
push @{$records}, $record;
+ } else {
+ return if @{$proxy->{message_list}}[-1]->{mt}
+ != TLSProxy::Message::MT_FINISHED;
+
+ my $record = TLSProxy::Record->new(
+ 1,
+ TLSProxy::Record::RT_APPLICATION_DATA,
+ TLSProxy::Record::VERS_TLS_1_2,
+ 0,
+ 0,
+ 0,
+ 0,
+ "",
+ ""
+ );
+
+ #Add a partial KeyUpdate message into the record
+ $data = pack "C1",
+ 0x18; # KeyUpdate message type. Omit the rest of the message header
+
+ #Add content type and tag
+ $data .= pack("C", TLSProxy::Record::RT_HANDSHAKE).("\0"x16);
+
+ $record->data($data);
+ $record->len(length $data);
+ push @{$records}, $record;
+
+ if ($boundary_test_type == DATA_BETWEEN_KEY_UPDATE) {
+ #Now add an app data record
+ $record = TLSProxy::Record->new(
+ 1,
+ TLSProxy::Record::RT_APPLICATION_DATA,
+ TLSProxy::Record::VERS_TLS_1_2,
+ 0,
+ 0,
+ 0,
+ 0,
+ "",
+ ""
+ );
+
+ #Add an empty app data record (just content type and tag)
+ $data = pack("C", TLSProxy::Record::RT_APPLICATION_DATA).("\0"x16);
+
+ $record->data($data);
+ $record->len(length $data);
+ push @{$records}, $record;
+ }
+
+ #Now add the rest of the KeyUpdate message
+ $record = TLSProxy::Record->new(
+ 1,
+ TLSProxy::Record::RT_APPLICATION_DATA,
+ TLSProxy::Record::VERS_TLS_1_2,
+ 0,
+ 0,
+ 0,
+ 0,
+ "",
+ ""
+ );
+
+ #Add the last 4 bytes of the KeyUpdate record
+ $data = pack "C4",
+ 0x00, 0x00, 0x01, # Message length
+ 0x00; # Update not requested
+
+ #Add content type and tag
+ $data .= pack("C", TLSProxy::Record::RT_HANDSHAKE).("\0"x16);
+
+ $record->data($data);
+ $record->len(length $data);
+ push @{$records}, $record;
+
}
}
More information about the openssl-commits
mailing list