[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  8e47ee18c8f7e59575effdd8dfcfbfff1a365ede (commit)
       via  3c9539d294b931bc430a01510753e10b7a201f11 (commit)
       via  185c29b14eafb9ddacffb82b10c4609e49686e66 (commit)
      from  5d71f7ea291761777a2b2a84f340ffb38b3ea14a (commit)


- Log -----------------------------------------------------------------
commit 8e47ee18c8f7e59575effdd8dfcfbfff1a365ede
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Nov 7 14:26:41 2016 +0000

    Add a test for the wrong version number in a record
    
    Prior to TLS1.3 we check that the received record version number is correct.
    In TLS1.3 we need to ignore the record version number. This adds a test to
    make sure we do it correctly.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 3c9539d294b931bc430a01510753e10b7a201f11
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Nov 7 13:49:18 2016 +0000

    Ignore the record version in TLS1.3
    
    The record layer version field must be ignored in TLSv1.3, so we remove the
    check when using that version.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit 185c29b14eafb9ddacffb82b10c4609e49686e66
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Nov 7 14:44:38 2016 +0000

    test_sslcbcpadding only makes sense <TLS1.3
    
    We may get failures if we run it in TLS1.3, and it makes no sense anyway
    so force TLS1.2
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 ssl/record/ssl3_record.c             |  5 +++--
 test/recipes/70-test_sslcbcpadding.t |  1 +
 test/recipes/70-test_sslrecords.t    | 32 +++++++++++++++++++++++++++++++-
 util/TLSProxy/Record.pm              | 13 ++++++++-----
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index f160c06..181ebbb 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -204,8 +204,9 @@ int ssl3_get_record(SSL *s)
                 rr[num_recs].rec_version = version;
                 n2s(p, rr[num_recs].length);
 
-                /* Lets check version */
-                if (!s->first_packet && version != s->version) {
+                /* Lets check version. In TLSv1.3 we ignore this field */
+                if (!s->first_packet && s->version != TLS1_3_VERSION
+                        && version != s->version) {
                     SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_WRONG_VERSION_NUMBER);
                     if ((s->version & 0xFF00) == (version & 0xFF00)
                         && !s->enc_write_ctx && !s->write_hash) {
diff --git a/test/recipes/70-test_sslcbcpadding.t b/test/recipes/70-test_sslcbcpadding.t
index 22825a0..8d3d6fc 100644
--- a/test/recipes/70-test_sslcbcpadding.t
+++ b/test/recipes/70-test_sslcbcpadding.t
@@ -48,6 +48,7 @@ ok(TLSProxy::Message->success(), "Maximally-padded record test");
 # Test that invalid padding is rejected.
 foreach my $offset (@test_offsets) {
     $proxy->clear();
+    $proxy->serverflags("-tls1_2");
     $bad_padding_offset = $offset;
     $proxy->start();
     ok(TLSProxy::Message->fail(), "Invalid padding byte $bad_padding_offset");
diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t
index b282dbd..cafa30c 100644
--- a/test/recipes/70-test_sslrecords.t
+++ b/test/recipes/70-test_sslrecords.t
@@ -39,10 +39,13 @@ 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";
-my $num_tests = 10;
+my $num_tests = 11;
 if (!disabled("tls1_1")) {
     $num_tests++;
 }
+if (!disabled("tls1_3")) {
+    $num_tests++;
+}
 plan tests => $num_tests;
 ok(TLSProxy::Message->fail(), "Out of context empty records test");
 
@@ -137,6 +140,21 @@ if (!disabled("tls1_1")) {
     ok(TLSProxy::Message->fail(), "Unrecognised record type in TLS1.1");
 }
 
+#Test 12: Sending a different record version in TLS1.2 should fail
+$proxy->clear();
+$proxy->clientflags("-tls1_2");
+$proxy->filter(\&change_version);
+$proxy->start();
+ok(TLSProxy::Message->fail(), "Changed record version in TLS1.2");
+
+#Test 13: Sending a different record version in TLS1.3 should succeed
+if (!disabled("tls1_3")) {
+    $proxy->clear();
+    $proxy->filter(\&change_version);
+    $proxy->start();
+    ok(TLSProxy::Message->success(), "Changed record version in TLS1.3");
+}
+
 sub add_empty_recs_filter
 {
     my $proxy = shift;
@@ -388,3 +406,15 @@ sub add_unknown_record_type
 
     unshift @{$proxy->record_list}, $record;
 }
+
+sub change_version
+{
+    my $proxy = shift;
+
+    # We'll change a version after the initial version neg has taken place
+    if ($proxy->flight != 2) {
+        return;
+    }
+
+    (${$proxy->record_list}[-1])->version(TLSProxy::Record::VERS_TLS_1_1);
+}
diff --git a/util/TLSProxy/Record.pm b/util/TLSProxy/Record.pm
index 106fa74..a4e7adc 100644
--- a/util/TLSProxy/Record.pm
+++ b/util/TLSProxy/Record.pm
@@ -278,11 +278,6 @@ sub content_type
     my $self = shift;
     return $self->{content_type};
 }
-sub version
-{
-    my $self = shift;
-    return $self->{version};
-}
 sub sslv2
 {
     my $self = shift;
@@ -332,4 +327,12 @@ sub len
     }
     return $self->{len};
 }
+sub version
+{
+    my $self = shift;
+    if (@_) {
+      $self->{version} = shift;
+    }
+    return $self->{version};
+}
 1;