From 8523288e6d667f052bda092e01ab17986782fede Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 10 Aug 2016 00:45:51 -0400 Subject: [PATCH] Test CBC mode padding. This is a regression test for https://github.com/openssl/openssl/pull/1431. It tests a maximally-padded record with each possible invalid offset. This required fixing a bug in Message.pm where the client sending a fatal alert followed by close_notify was still treated as success. Reviewed-by: Rich Salz Reviewed-by: Matt Caswell --- test/recipes/70-test_sslcbcpadding.t | 107 +++++++++++++++++++++++++++ util/TLSProxy/Message.pm | 6 +- util/TLSProxy/Proxy.pm | 11 +++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 test/recipes/70-test_sslcbcpadding.t diff --git a/test/recipes/70-test_sslcbcpadding.t b/test/recipes/70-test_sslcbcpadding.t new file mode 100644 index 0000000000..c6eca2f713 --- /dev/null +++ b/test/recipes/70-test_sslcbcpadding.t @@ -0,0 +1,107 @@ +#! /usr/bin/env perl +# 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 +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + +use strict; +use OpenSSL::Test qw/:DEFAULT cmdstr srctop_file bldtop_dir/; +use OpenSSL::Test::Utils; +use TLSProxy::Proxy; + +my $test_name = "test_sslcbcpadding"; +setup($test_name); + +plan skip_all => "TLSProxy isn't usable on $^O" + if $^O =~ /^(VMS|MSWin32)$/; + +plan skip_all => "$test_name needs the dynamic engine feature enabled" + if disabled("engine") || disabled("dynamic-engine"); + +plan skip_all => "$test_name needs the sock feature enabled" + if disabled("sock"); + +plan skip_all => "$test_name needs TLSv1.2 enabled" + if disabled("tls1_2"); + +$ENV{OPENSSL_ia32cap} = '~0x200000200000000'; +my $proxy = TLSProxy::Proxy->new( + \&add_maximal_padding_filter, + cmdstr(app(["openssl"]), display => 1), + srctop_file("apps", "server.pem"), + (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE}) +); + +plan tests => 1 + 256; + +my $bad_padding_offset = -1; + +# Test 1: Maximally-padded records are accepted. +$proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; +ok(TLSProxy::Message->success(), "Maximally-padded record test"); + +# Tests 2 through 257: Invalid padding. +for ($bad_padding_offset = 0; $bad_padding_offset < 256; + $bad_padding_offset++) { + $proxy->clear(); + $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";; + ok(TLSProxy::Message->fail(), "Invalid padding byte $bad_padding_offset"); +} + +sub add_maximal_padding_filter +{ + my $proxy = shift; + + if ($proxy->flight == 0) { + # Disable Encrypt-then-MAC. + foreach my $message (@{$proxy->message_list}) { + if ($message->mt != TLSProxy::Message::MT_CLIENT_HELLO) { + next; + } + + $message->delete_extension(TLSProxy::Message::EXT_ENCRYPT_THEN_MAC); + $message->process_extensions(); + $message->repack(); + } + } + + if ($proxy->flight == 3) { + # Insert a maximally-padded record. Assume a block size of 16 (AES) and + # a MAC length of 20 (SHA-1). + my $block_size = 16; + my $mac_len = 20; + + # Size the plaintext so that 256 is a valid padding. + my $plaintext_len = $block_size - ($mac_len % $block_size); + my $plaintext = "A" x $plaintext_len; + + my $data = "B" x $block_size; # Explicit IV. + $data .= $plaintext; + $data .= TLSProxy::Proxy::fill_known_data($mac_len); # MAC. + + # Add padding. + for (my $i = 0; $i < 256; $i++) { + if ($i == $bad_padding_offset) { + $data .= "\xfe"; + } else { + $data .= "\xff"; + } + } + + my $record = TLSProxy::Record->new( + $proxy->flight, + TLSProxy::Record::RT_APPLICATION_DATA, + TLSProxy::Record::VERS_TLS_1_2, + length($data), + length($data), + $plaintext_len, + $data, + $plaintext, + ); + + # Send the record immediately after the server Finished. + push @{$proxy->record_list}, $record; + } +} diff --git a/util/TLSProxy/Message.pm b/util/TLSProxy/Message.pm index 321e080ea3..1810d8c30e 100644 --- a/util/TLSProxy/Message.pm +++ b/util/TLSProxy/Message.pm @@ -199,14 +199,14 @@ sub get_messages print " [".$record->decrypt_data."]\n"; } elsif ($record->content_type == TLSProxy::Record::RT_ALERT) { my ($alertlev, $alertdesc) = unpack('CC', $record->decrypt_data); - #All alerts end the test - $end = 1; #A CloseNotify from the client indicates we have finished successfully #(we assume) - if (!$server && $alertlev == AL_LEVEL_WARN + if (!$end && !$server && $alertlev == AL_LEVEL_WARN && $alertdesc == AL_DESC_CLOSE_NOTIFY) { $success = 1; } + #All alerts end the test + $end = 1; } return @messages; diff --git a/util/TLSProxy/Proxy.pm b/util/TLSProxy/Proxy.pm index e0ce43aa77..eeb83ed74f 100644 --- a/util/TLSProxy/Proxy.pm +++ b/util/TLSProxy/Proxy.pm @@ -493,4 +493,15 @@ sub serverpid } return $self->{serverpid}; } + +sub fill_known_data +{ + my $length = shift; + my $ret = ""; + for (my $i = 0; $i < $length; $i++) { + $ret .= chr($i); + } + return $ret; +} + 1; -- 2.25.1