[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Tue Jun 7 21:09:39 UTC 2016
The branch master has been updated
via 4f0c475719defd7c051964ef9964cc6e5b3a63bf (commit)
via 255cfeacd88bcba13688da17fab72b344a78d24f (commit)
via 0aac3a6b1979dbebd5325bb48c01f584bf35017e (commit)
from f44310e9ce2cdab64a9269ad8014be978e333db6 (commit)
- Log -----------------------------------------------------------------
commit 4f0c475719defd7c051964ef9964cc6e5b3a63bf
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jun 1 17:44:19 2016 +0100
Add empty record tests
The previous commit changed how we handle out-of-context empty records.
This commit adds some tests for the various scenarios. There are three
tests:
1: Check that if we inject an out-of-context empty record then we fail
2: Check that if we inject an in-context empty record then we succeed
3: Check that if we inject too many in-context empty records then we fail.
Reviewed-by: Andy Polyakov <appro at openssl.org>
commit 255cfeacd88bcba13688da17fab72b344a78d24f
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jun 1 16:31:11 2016 +0100
Reject out of context empty records
Previously if we received an empty record we just threw it away and
ignored it. Really though if we get an empty record of a different content
type to what we are expecting then that should be an error, i.e. we should
reject out of context empty records. This commit makes the necessary changes
to achieve that.
RT#4395
Reviewed-by: Andy Polyakov <appro at openssl.org>
commit 0aac3a6b1979dbebd5325bb48c01f584bf35017e
Author: Matt Caswell <matt at openssl.org>
Date: Wed Jun 1 16:25:31 2016 +0100
Fix pipelining bug
The number of read pipelines should be reset in the event of reuse of an
SSL object.
Reviewed-by: Andy Polyakov <appro at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/record/rec_layer_s3.c | 10 ++++-
ssl/record/record.h | 7 +++
ssl/record/record_locl.h | 6 +++
ssl/record/ssl3_record.c | 20 ++++-----
.../{70-test_sslvertol.t => 70-test_sslrecords.t} | 51 ++++++++++++++--------
5 files changed, 62 insertions(+), 32 deletions(-)
copy test/recipes/{70-test_sslvertol.t => 70-test_sslrecords.t} (52%)
mode change 100755 => 100644
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 8c02efd..7326076 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -63,6 +63,7 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
for(pipes = 0; pipes < rl->numwpipes; pipes++)
SSL3_BUFFER_clear(&rl->wbuf[pipes]);
rl->numwpipes = 0;
+ rl->numrpipes = 0;
SSL3_RECORD_clear(rl->rrec, SSL_MAX_PIPELINES);
RECORD_LAYER_reset_read_sequence(rl);
@@ -1056,9 +1057,9 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
goto f_err;
}
}
- /* Skip over any records we have already used or are zero in length */
+ /* Skip over any records we have already read */
for (curr_rec = 0;
- curr_rec < num_recs && SSL3_RECORD_get_length(&rr[curr_rec]) == 0;
+ curr_rec < num_recs && SSL3_RECORD_is_read(&rr[curr_rec]);
curr_rec++);
if (curr_rec == num_recs) {
RECORD_LAYER_set_numrpipes(&s->rlayer, 0);
@@ -1136,6 +1137,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
if (SSL3_RECORD_get_length(rr) == 0) {
s->rlayer.rstate = SSL_ST_READ_HEADER;
SSL3_RECORD_set_off(rr, 0);
+ SSL3_RECORD_set_read(rr);
}
}
if (SSL3_RECORD_get_length(rr) == 0
@@ -1146,6 +1148,10 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
read_bytes += n;
} while (type == SSL3_RT_APPLICATION_DATA && curr_rec < num_recs
&& read_bytes < (unsigned int)len);
+ if (read_bytes == 0) {
+ /* We must have read empty records. Get more data */
+ goto start;
+ }
if (!peek && curr_rec == num_recs
&& (s->mode & SSL_MODE_RELEASE_BUFFERS)
&& SSL3_BUFFER_get_left(rbuf) == 0)
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 9e19822..cda4eff 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -65,6 +65,10 @@ typedef struct ssl3_record_st {
/* r */
unsigned char *comp;
+ /* Whether the data from this record has already been read or not */
+ /* r */
+ unsigned int read;
+
/* epoch number, needed by DTLS1 */
/* r */
unsigned long epoch;
@@ -181,6 +185,9 @@ typedef struct record_layer_st {
unsigned char handshake_fragment[4];
unsigned int handshake_fragment_len;
+ /* The number of consecutive empty records we have received */
+ unsigned int empty_record_count;
+
/* partial write - check the numbers match */
/* number bytes written */
int wpend_tot;
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index 67ae1f4..ff1eb32 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -27,6 +27,10 @@
#define RECORD_LAYER_get_write_sequence(rl) ((rl)->write_sequence)
#define RECORD_LAYER_get_numrpipes(rl) ((rl)->numrpipes)
#define RECORD_LAYER_set_numrpipes(rl, n) ((rl)->numrpipes = (n))
+#define RECORD_LAYER_inc_empty_record_count(rl) ((rl)->empty_record_count++)
+#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 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);
@@ -89,6 +93,8 @@ int ssl3_release_write_buffer(SSL *s);
#define SSL3_RECORD_get_epoch(r) ((r)->epoch)
#define SSL3_RECORD_is_sslv2_record(r) \
((r)->rec_version == SSL2_VERSION)
+#define SSL3_RECORD_is_read(r) ((r)->read)
+#define SSL3_RECORD_set_read(r) ((r)->read = 1)
void SSL3_RECORD_clear(SSL3_RECORD *r, unsigned int num_recs);
void SSL3_RECORD_release(SSL3_RECORD *r, unsigned int num_recs);
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index 872a381..d3b2bea 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -134,7 +134,6 @@ int ssl3_get_record(SSL *s)
unsigned char md[EVP_MAX_MD_SIZE];
short version;
unsigned mac_size;
- unsigned empty_record_count = 0, curr_empty = 0;
unsigned int num_recs = 0;
unsigned int max_recs;
unsigned int j;
@@ -146,7 +145,6 @@ int ssl3_get_record(SSL *s)
max_recs = 1;
sess = s->session;
- again:
do {
/* check if we have the header */
if ((RECORD_LAYER_get_rstate(&s->rlayer) != SSL_ST_READ_BODY) ||
@@ -323,6 +321,10 @@ int ssl3_get_record(SSL *s)
/* decrypt in place in 'rr->input' */
rr[num_recs].data = rr[num_recs].input;
rr[num_recs].orig_len = rr[num_recs].length;
+
+ /* Mark this record as not read by upper layers yet */
+ rr[num_recs].read = 0;
+
num_recs++;
/* we have pulled in a full packet so zero things */
@@ -486,21 +488,17 @@ int ssl3_get_record(SSL *s)
/* just read a 0 length packet */
if (rr[j].length == 0) {
- curr_empty++;
- empty_record_count++;
- if (empty_record_count > MAX_EMPTY_RECORDS) {
+ RECORD_LAYER_inc_empty_record_count(&s->rlayer);
+ if (RECORD_LAYER_get_empty_record_count(&s->rlayer)
+ > MAX_EMPTY_RECORDS) {
al = SSL_AD_UNEXPECTED_MESSAGE;
SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_RECORD_TOO_SMALL);
goto f_err;
}
+ } else {
+ RECORD_LAYER_reset_empty_record_count(&s->rlayer);
}
}
- if (curr_empty == num_recs) {
- /* We have no data - do it all again */
- num_recs = 0;
- curr_empty = 0;
- goto again;
- }
RECORD_LAYER_set_numrpipes(&s->rlayer, num_recs);
return 1;
diff --git a/test/recipes/70-test_sslvertol.t b/test/recipes/70-test_sslrecords.t
old mode 100755
new mode 100644
similarity index 52%
copy from test/recipes/70-test_sslvertol.t
copy to test/recipes/70-test_sslrecords.t
index af82a8c..beacc4a
--- a/test/recipes/70-test_sslvertol.t
+++ b/test/recipes/70-test_sslrecords.t
@@ -1,5 +1,5 @@
#! /usr/bin/env perl
-# Copyright 2015-2016 The OpenSSL Project Authors. All Rights Reserved.
+# Copyright 2016 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
@@ -11,7 +11,7 @@ use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file bldtop_dir/;
use OpenSSL::Test::Utils;
use TLSProxy::Proxy;
-my $test_name = "test_sslextension";
+my $test_name = "test_sslrecords";
setup($test_name);
plan skip_all => "TLSProxy isn't usable on $^O"
@@ -28,26 +28,34 @@ plan skip_all => "$test_name needs TLS enabled"
$ENV{OPENSSL_ia32cap} = '~0x200000200000000';
my $proxy = TLSProxy::Proxy->new(
- \&vers_tolerance_filter,
+ \&add_empty_recs_filter,
cmdstr(app(["openssl"]), display => 1),
srctop_file("apps", "server.pem"),
(!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE})
);
-plan tests => 2;
+plan tests => 3;
-#Test 1: Asking for TLS1.3 should pass
-my $client_version = TLSProxy::Record::VERS_TLS_1_3;
+#Test 1: Injecting out of context empty records should fail
+my $content_type = TLSProxy::Record::RT_APPLICATION_DATA;
+my $inject_recs_num = 1;
$proxy->start();
-ok(TLSProxy::Message->success(), "Version tolerance test, TLS 1.3");
+ok(TLSProxy::Message->fail(), "Out of context empty records test");
-#Test 2: Testing something below SSLv3 should fail
-$client_version = TLSProxy::Record::VERS_SSL_3_0 - 1;
+#Test 2: Injecting in context empty records should succeed
$proxy->clear();
+$content_type = TLSProxy::Record::RT_HANDSHAKE;
$proxy->start();
-ok(TLSProxy::Message->fail(), "Version tolerance test, SSL < 3.0");
+ok(TLSProxy::Message->success(), "In context empty records test");
-sub vers_tolerance_filter
+#Test 3: Injecting too many in context empty records should fail
+$proxy->clear();
+#We allow 32 consecutive in context empty records
+$inject_recs_num = 33;
+$proxy->start();
+ok(TLSProxy::Message->fail(), "Too many in context empty records test");
+
+sub add_empty_recs_filter
{
my $proxy = shift;
@@ -56,13 +64,18 @@ sub vers_tolerance_filter
return;
}
- foreach my $message (@{$proxy->message_list}) {
- if ($message->mt == TLSProxy::Message::MT_CLIENT_HELLO) {
- #Set the client version
- #Anything above the max supported version (TLS1.2) should succeed
- #Anything below SSLv3 should fail
- $message->client_version($client_version);
- $message->repack();
- }
+ for (my $i = 0; $i < $inject_recs_num; $i++) {
+ my $record = TLSProxy::Record->new(
+ 0,
+ $content_type,
+ TLSProxy::Record::VERS_TLS_1_2,
+ 0,
+ 0,
+ 0,
+ "",
+ ""
+ );
+
+ push @{$proxy->record_list}, $record;
}
}
More information about the openssl-commits
mailing list