TLS: Split CBC+HMAC modes to standalone AEAD_Mode

Now record layer only deals with an AEAD, and the weird complications
of CBC modes mostly hidden in tls_cbc.cpp

8 files changed, 850 insertions, 454 deletions

diff --git a/src/lib/tls/info.txt b/src/lib/tls/info.txt
index 667726318..ad0d266fa 100644
--- a/src/lib/tls/info.txt
+++ b/src/lib/tls/info.txt
@@ -22,6 +22,7 @@ tls_version.h
 </header:public>
 
 <header:internal>
+tls_cbc.h
 tls_extensions.h
 tls_handshake_hash.h
 tls_handshake_io.h
diff --git a/src/lib/tls/tls_cbc.cpp b/src/lib/tls/tls_cbc.cpp
new file mode 100644
index 000000000..c7203003b
--- /dev/null
+++ b/src/lib/tls/tls_cbc.cpp
@@ -0,0 +1,403 @@
+/*
+* TLS CBC Record Handling
+* (C) 2012,2013,2014,2015,2016 Jack Lloyd
+*     2016 Juraj Somorovsky
+*     2016 Matthias Gierlings
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#include <botan/internal/tls_cbc.h>
+#include <botan/internal/rounding.h>
+#include <botan/internal/ct_utils.h>
+#include <botan/tls_alert.h>
+#include <botan/tls_magic.h>
+#include <botan/tls_exceptn.h>
+
+namespace Botan {
+
+namespace TLS {
+
+/*
+* TLS_CBC_HMAC_AEAD_Mode Constructor
+*/
+TLS_CBC_HMAC_AEAD_Mode::TLS_CBC_HMAC_AEAD_Mode(const std::string& cipher_name,
+                                               size_t cipher_keylen,
+                                               const std::string& mac_name,
+                                               size_t mac_keylen,
+                                               bool use_explicit_iv,
+                                               bool use_encrypt_then_mac) :
+   m_cipher_name(cipher_name),
+   m_mac_name(mac_name),
+   m_cipher_keylen(cipher_keylen),
+   m_mac_keylen(mac_keylen),
+   m_use_encrypt_then_mac(use_encrypt_then_mac)
+   {
+   m_cipher = BlockCipher::create(m_cipher_name);
+   if(!m_cipher)
+      throw Algorithm_Not_Found(m_cipher_name);
+
+   m_mac = MessageAuthenticationCode::create("HMAC(" + m_mac_name + ")");
+   if(!m_mac)
+      throw Algorithm_Not_Found("HMAC(" + m_mac_name + ")");
+
+   m_tag_size = m_mac->output_length();
+   m_block_size = m_cipher->block_size();
+
+   m_iv_size = use_explicit_iv ? m_block_size : 0;
+   }
+
+void TLS_CBC_HMAC_AEAD_Mode::clear()
+   {
+   cipher().clear();
+   mac().clear();
+   cbc_state().clear();
+   }
+
+std::string TLS_CBC_HMAC_AEAD_Mode::name() const
+   {
+   return "TLS_CBC(" + m_cipher_name + "," + m_mac_name + ")";
+   }
+
+size_t TLS_CBC_HMAC_AEAD_Mode::update_granularity() const
+   {
+   return 1; // just buffers anyway
+   }
+
+bool TLS_CBC_HMAC_AEAD_Mode::valid_nonce_length(size_t nl) const
+   {
+   if(m_cbc_state.empty())
+      return nl == block_size();
+   return nl == iv_size();
+   }
+
+Key_Length_Specification TLS_CBC_HMAC_AEAD_Mode::key_spec() const
+   {
+   return Key_Length_Specification(m_cipher_keylen + m_mac_keylen);
+   }
+
+void TLS_CBC_HMAC_AEAD_Mode::key_schedule(const byte key[], size_t keylen)
+   {
+   // Both keys are of fixed length specified by the ciphersuite
+
+   if(keylen != m_cipher_keylen + m_mac_keylen)
+      throw Invalid_Key_Length(name(), keylen);
+
+   cipher().set_key(&key[0], m_cipher_keylen);
+   mac().set_key(&key[m_cipher_keylen], m_mac_keylen);
+   }
+
+void TLS_CBC_HMAC_AEAD_Mode::start_msg(const byte nonce[], size_t nonce_len)
+   {
+   if(!valid_nonce_length(nonce_len))
+      {
+      throw Invalid_IV_Length(name(), nonce_len);
+      }
+
+   m_msg.clear();
+
+   if(nonce_len > 0)
+      {
+      m_cbc_state.assign(nonce, nonce + nonce_len);
+      }
+   }
+
+size_t TLS_CBC_HMAC_AEAD_Mode::process(byte buf[], size_t sz)
+   {
+   m_msg.insert(m_msg.end(), buf, buf + sz);
+   return 0;
+   }
+
+std::vector<byte> TLS_CBC_HMAC_AEAD_Mode::assoc_data_with_len(uint16_t len)
+   {
+   std::vector<byte> ad = m_ad;
+   BOTAN_ASSERT(ad.size() == 13, "Expected AAD size");
+   ad[11] = get_byte(0, len);
+   ad[12] = get_byte(1, len);
+   return ad;
+   }
+
+void TLS_CBC_HMAC_AEAD_Mode::set_associated_data(const byte ad[], size_t ad_len)
+   {
+   if(ad_len != 13)
+      throw Exception("Invalid TLS AEAD associated data length");
+   m_ad.assign(ad, ad + ad_len);
+   }
+
+void TLS_CBC_HMAC_AEAD_Encryption::set_associated_data(const byte ad[], size_t ad_len)
+   {
+   TLS_CBC_HMAC_AEAD_Mode::set_associated_data(ad, ad_len);
+
+   if(use_encrypt_then_mac())
+      {
+      std::vector<byte>& ad = assoc_data();
+      // AAD hack for EtM
+      size_t pt_size = make_u16bit(ad[11], ad[12]);
+      size_t enc_size = round_up(iv_size() + pt_size + 1, block_size());
+      ad[11] = get_byte<uint16_t>(0, enc_size);
+      ad[12] = get_byte<uint16_t>(1, enc_size);
+      }
+   }
+
+void TLS_CBC_HMAC_AEAD_Encryption::cbc_encrypt_record(byte buf[], size_t buf_size)
+   {
+   const size_t blocks = buf_size / block_size();
+   BOTAN_ASSERT(buf_size % block_size() == 0, "Valid CBC input");
+
+   xor_buf(buf, cbc_state().data(), block_size());
+   cipher().encrypt(buf);
+
+   for(size_t i = 1; i < blocks; ++i)
+      {
+      xor_buf(&buf[block_size()*i], &buf[block_size()*(i-1)], block_size());
+      cipher().encrypt(&buf[block_size()*i]);
+      }
+
+   cbc_state().assign(&buf[block_size()*(blocks-1)],
+                      &buf[block_size()*blocks]);
+   }
+
+size_t TLS_CBC_HMAC_AEAD_Encryption::output_length(size_t input_length) const
+   {
+   return round_up(input_length + 1 + (use_encrypt_then_mac() ? 0 : tag_size()), block_size()) +
+      (use_encrypt_then_mac() ? tag_size() : 0);
+   }
+
+void TLS_CBC_HMAC_AEAD_Encryption::finish(secure_vector<byte>& buffer, size_t offset)
+   {
+   update(buffer, offset);
+   buffer.resize(offset); // truncate, leaving just header
+   const size_t header_size = offset;
+
+   buffer.insert(buffer.end(), msg().begin(), msg().end());
+
+   const size_t input_size = msg().size() + 1 + (use_encrypt_then_mac() ? 0 : tag_size());
+   const size_t enc_size = round_up(input_size, block_size());
+   const size_t pad_val = enc_size - input_size;
+   const size_t buf_size = enc_size + (use_encrypt_then_mac() ? tag_size() : 0);
+
+   BOTAN_ASSERT(enc_size % block_size() == 0,
+                "Buffer is an even multiple of block size");
+
+   mac().update(assoc_data());
+
+   if(use_encrypt_then_mac())
+      {
+      if(iv_size() > 0)
+         {
+         mac().update(cbc_state());
+         }
+
+      for(size_t i = 0; i != pad_val + 1; ++i)
+         buffer.push_back(static_cast<byte>(pad_val));
+      cbc_encrypt_record(&buffer[header_size], enc_size);
+      }
+
+   // EtM also uses ciphertext size instead of plaintext size for AEAD input
+   const byte* mac_input = (use_encrypt_then_mac() ? &buffer[header_size] : msg().data());
+   const size_t mac_input_len = (use_encrypt_then_mac() ? enc_size : msg().size());
+
+   mac().update(mac_input, mac_input_len);
+
+   buffer.resize(buffer.size() + tag_size());
+   mac().final(&buffer[buffer.size() - tag_size()]);
+
+   if(use_encrypt_then_mac() == false)
+      {
+      for(size_t i = 0; i != pad_val + 1; ++i)
+         buffer.push_back(static_cast<byte>(pad_val));
+      cbc_encrypt_record(&buffer[header_size], buf_size);
+      }
+   }
+
+namespace {
+
+
+/*
+* Checks the TLS padding. Returns 0 if the padding is invalid (we
+* count the padding_length field as part of the padding size so a
+* valid padding will always be at least one byte long), or the length
+* of the padding otherwise. This is actually padding_length + 1
+* because both the padding and padding_length fields are padding from
+* our perspective.
+*
+* Returning 0 in the error case should ensure the MAC check will fail.
+* This approach is suggested in section 6.2.3.2 of RFC 5246.
+*/
+u16bit check_tls_padding(const byte record[], size_t record_len)
+   {
+   /*
+   * TLS v1.0 and up require all the padding bytes be the same value
+   * and allows up to 255 bytes.
+   */
+
+   const byte pad_byte = record[(record_len-1)];
+
+   byte pad_invalid = 0;
+   for(size_t i = 0; i != record_len; ++i)
+      {
+      const size_t left = record_len - i - 2;
+      const byte delim_mask = CT::is_less<u16bit>(static_cast<u16bit>(left), pad_byte) & 0xFF;
+      pad_invalid |= (delim_mask & (record[i] ^ pad_byte));
+      }
+
+   u16bit pad_invalid_mask = CT::expand_mask<u16bit>(pad_invalid);
+   return CT::select<u16bit>(pad_invalid_mask, 0, pad_byte + 1);
+   }
+
+}
+
+void TLS_CBC_HMAC_AEAD_Decryption::cbc_decrypt_record(byte record_contents[], size_t record_len)
+   {
+   BOTAN_ASSERT(record_len % block_size() == 0,
+                "Buffer is an even multiple of block size");
+
+   const size_t blocks = record_len / block_size();
+
+   BOTAN_ASSERT(blocks >= 1, "At least one ciphertext block");
+
+   byte* buf = record_contents;
+
+   secure_vector<byte> last_ciphertext(block_size());
+   copy_mem(last_ciphertext.data(), buf, block_size());
+
+   cipher().decrypt(buf);
+   xor_buf(buf, cbc_state().data(), block_size());
+
+   secure_vector<byte> last_ciphertext2;
+
+   for(size_t i = 1; i < blocks; ++i)
+      {
+      last_ciphertext2.assign(&buf[block_size()*i], &buf[block_size()*(i+1)]);
+      cipher().decrypt(&buf[block_size()*i]);
+      xor_buf(&buf[block_size()*i], last_ciphertext.data(), block_size());
+      std::swap(last_ciphertext, last_ciphertext2);
+      }
+
+   cbc_state().assign(last_ciphertext.begin(), last_ciphertext.end());
+   }
+
+size_t TLS_CBC_HMAC_AEAD_Decryption::output_length(size_t) const
+   {
+   /*
+   * We don't know this because the padding is arbitrary
+   */
+   return 0;
+   }
+
+void TLS_CBC_HMAC_AEAD_Decryption::finish(secure_vector<byte>& buffer, size_t offset)
+   {
+   update(buffer, offset);
+   buffer.resize(offset);
+
+   const size_t record_len = msg().size();
+   byte* record_contents = msg().data();
+
+   // This early exit does not leak info because all the values compared are public
+   if(record_len < tag_size() ||
+      (record_len - (use_encrypt_then_mac() ? tag_size() : 0)) % block_size() != 0)
+      {
+      throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
+      }
+
+   if(use_encrypt_then_mac())
+      {
+      const size_t enc_size = record_len - tag_size();
+
+      mac().update(assoc_data_with_len(iv_size() + enc_size));
+      if(iv_size() > 0)
+         {
+         mac().update(cbc_state());
+         }
+      mac().update(record_contents, enc_size);
+
+      std::vector<byte> mac_buf(tag_size());
+      mac().final(mac_buf.data());
+
+      const size_t mac_offset = enc_size;
+
+      const bool mac_ok = same_mem(&record_contents[mac_offset], mac_buf.data(), tag_size());
+
+      if(!mac_ok)
+         {
+         throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
+         }
+
+      cbc_decrypt_record(record_contents, enc_size);
+
+      // 0 if padding was invalid, otherwise 1 + padding_bytes
+      u16bit pad_size = check_tls_padding(record_contents, enc_size);
+
+      // No oracle here, whoever sent us this had the key since MAC check passed
+      if(pad_size == 0)
+         {
+         throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
+         }
+
+      const byte* plaintext_block = &record_contents[0];
+      const u16bit plaintext_length = enc_size - pad_size;
+
+      buffer.insert(buffer.end(), plaintext_block, plaintext_block + plaintext_length);
+      }
+   else
+      {
+      uint8_t* record_contents = msg().data();
+      const size_t record_len = msg().size();
+
+      CT::poison(record_contents, record_len);
+
+      cbc_decrypt_record(record_contents, record_len);
+
+      // 0 if padding was invalid, otherwise 1 + padding_bytes
+      u16bit pad_size = check_tls_padding(record_contents, record_len);
+
+      /*
+      This mask is zero if there is not enough room in the packet to get a valid MAC.
+
+      We have to accept empty packets, since otherwise we are not compatible
+      with how OpenSSL's countermeasure for fixing BEAST in TLS 1.0 CBC works
+      (sending empty records, instead of 1/(n-1) splitting)
+      */
+
+      const u16bit size_ok_mask = CT::is_lte<u16bit>(static_cast<u16bit>(tag_size() + pad_size), static_cast<u16bit>(record_len + 1));
+      pad_size &= size_ok_mask;
+
+      CT::unpoison(record_contents, record_len);
+
+      /*
+      This is unpoisoned sooner than it should. The pad_size leaks to plaintext_length and
+      then to the timing channel in the MAC computation described in the Lucky 13 paper.
+      */
+      CT::unpoison(pad_size);
+
+      const byte* plaintext_block = &record_contents[0];
+      const u16bit plaintext_length = static_cast<u16bit>(record_len - tag_size() - pad_size);
+
+      mac().update(assoc_data_with_len(plaintext_length));
+      mac().update(plaintext_block, plaintext_length);
+
+      std::vector<byte> mac_buf(tag_size());
+      mac().final(mac_buf.data());
+
+      const size_t mac_offset = record_len - (tag_size() + pad_size);
+
+      const bool mac_ok = same_mem(&record_contents[mac_offset], mac_buf.data(), tag_size());
+
+      const u16bit ok_mask = size_ok_mask & CT::expand_mask<u16bit>(mac_ok) & CT::expand_mask<u16bit>(pad_size);
+
+      CT::unpoison(ok_mask);
+
+      if(ok_mask)
+         {
+         buffer.insert(buffer.end(), plaintext_block, plaintext_block + plaintext_length);
+         }
+      else
+         {
+         throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
+         }
+      }
+   }
+
+}
+
+}
diff --git a/src/lib/tls/tls_cbc.h b/src/lib/tls/tls_cbc.h
new file mode 100644
index 000000000..90b54bb5a
--- /dev/null
+++ b/src/lib/tls/tls_cbc.h
@@ -0,0 +1,168 @@
+/*
+* TLS CBC+HMAC AEAD
+* (C) 2016 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#ifndef BOTAN_TLS_CBC_HMAC_AEAD_H__
+#define BOTAN_TLS_CBC_HMAC_AEAD_H__
+
+#include <botan/aead.h>
+#include <botan/block_cipher.h>
+#include <botan/mac.h>
+
+namespace Botan {
+
+namespace TLS {
+
+/**
+* TLS CBC+HMAC AEAD base class (GenericBlockCipher in TLS spec)
+* This is the weird TLS-specific mode, not for general consumption.
+*/
+class TLS_CBC_HMAC_AEAD_Mode : public AEAD_Mode
+   {
+   public:
+      size_t process(uint8_t buf[], size_t sz) override final;
+
+      std::string name() const override final;
+
+      void set_associated_data(const byte ad[], size_t ad_len) override;
+
+      size_t update_granularity() const override final;
+
+      Key_Length_Specification key_spec() const override final;
+
+      bool valid_nonce_length(size_t nl) const override final;
+
+      size_t tag_size() const override final { return m_tag_size; }
+
+      size_t default_nonce_length() const override final { return m_iv_size; }
+
+      void clear() override final;
+
+ protected:
+      TLS_CBC_HMAC_AEAD_Mode(const std::string& cipher_name,
+                             size_t cipher_keylen,
+                             const std::string& mac_name,
+                             size_t mac_keylen,
+                             bool use_explicit_iv,
+                             bool use_encrypt_then_mac);
+
+      size_t cipher_keylen() const { return m_cipher_keylen; }
+      size_t mac_keylen() const { return m_mac_keylen; }
+      size_t iv_size() const { return m_iv_size; }
+      size_t block_size() const { return m_block_size; }
+
+      bool use_encrypt_then_mac() const { return m_use_encrypt_then_mac; }
+
+      BlockCipher& cipher() const
+         {
+         BOTAN_ASSERT_NONNULL(m_cipher);
+         return *m_cipher;
+         }
+
+      MessageAuthenticationCode& mac() const
+         {
+         BOTAN_ASSERT_NONNULL(m_mac);
+         return *m_mac;
+         }
+
+      secure_vector<byte>& cbc_state() { return m_cbc_state; }
+      std::vector<byte>& assoc_data() { return m_ad; }
+      secure_vector<byte>& msg() { return m_msg; }
+
+      std::vector<byte> assoc_data_with_len(uint16_t len);
+
+   private:
+      void start_msg(const byte nonce[], size_t nonce_len) override final;
+
+      void key_schedule(const byte key[], size_t length) override final;
+
+      const std::string m_cipher_name;
+      const std::string m_mac_name;
+      size_t m_cipher_keylen;
+      size_t m_mac_keylen;
+      size_t m_iv_size;
+      size_t m_tag_size;
+      size_t m_block_size;
+      bool m_use_encrypt_then_mac;
+
+      std::unique_ptr<BlockCipher> m_cipher;
+      std::unique_ptr<MessageAuthenticationCode> m_mac;
+
+      secure_vector<byte> m_cbc_state;
+      std::vector<byte> m_ad;
+      secure_vector<byte> m_msg;
+   };
+
+/**
+* TLS_CBC_HMAC_AEAD Encryption
+*/
+class BOTAN_DLL TLS_CBC_HMAC_AEAD_Encryption final : public TLS_CBC_HMAC_AEAD_Mode
+   {
+   public:
+      /**
+      */
+      TLS_CBC_HMAC_AEAD_Encryption(const std::string& cipher_algo,
+                                   const size_t cipher_keylen,
+                                   const std::string& mac_algo,
+                                   const size_t mac_keylen,
+                                   bool use_explicit_iv,
+                                   bool use_encrypt_then_mac) :
+         TLS_CBC_HMAC_AEAD_Mode(cipher_algo,
+                                cipher_keylen,
+                                mac_algo,
+                                mac_keylen,
+                                use_explicit_iv,
+                                use_encrypt_then_mac)
+         {}
+
+      void set_associated_data(const byte ad[], size_t ad_len) override;
+
+      size_t output_length(size_t input_length) const override;
+
+      size_t minimum_final_size() const override { return 0; }
+
+      void finish(secure_vector<byte>& final_block, size_t offset = 0) override;
+   private:
+      void cbc_encrypt_record(byte record_contents[], size_t record_len);
+   };
+
+/**
+* TLS_CBC_HMAC_AEAD Decryption
+*/
+class BOTAN_DLL TLS_CBC_HMAC_AEAD_Decryption final : public TLS_CBC_HMAC_AEAD_Mode
+   {
+   public:
+      /**
+      */
+      TLS_CBC_HMAC_AEAD_Decryption(const std::string& cipher_algo,
+                                   const size_t cipher_keylen,
+                                   const std::string& mac_algo,
+                                   const size_t mac_keylen,
+                                   bool use_explicit_iv,
+                                   bool use_encrypt_then_mac) :
+         TLS_CBC_HMAC_AEAD_Mode(cipher_algo,
+                                cipher_keylen,
+                                mac_algo,
+                                mac_keylen,
+                                use_explicit_iv,
+                                use_encrypt_then_mac)
+         {}
+
+      size_t output_length(size_t input_length) const override;
+
+      size_t minimum_final_size() const override { return tag_size(); }
+
+      void finish(secure_vector<byte>& final_block, size_t offset = 0) override;
+
+   private:
+      void cbc_decrypt_record(byte record_contents[], size_t record_len);
+   };
+
+}
+
+}
+
+#endif
diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index 5e9207da7..574be8a46 100644
--- a/src/lib/tls/tls_channel.cpp
+++ b/src/lib/tls/tls_channel.cpp
@@ -276,7 +276,7 @@ void Channel::activate_session()
    if(!m_active_state->version().is_datagram_protocol())
       {
       // TLS is easy just remove all but the current state
-      auto current_epoch = sequence_numbers().current_write_epoch();
+      const u16bit current_epoch = sequence_numbers().current_write_epoch();
 
       const auto not_current_epoch =
          [current_epoch](u16bit epoch) { return (epoch != current_epoch); };
@@ -502,10 +502,11 @@ void Channel::send_record_array(u16bit epoch, byte type, const byte input[], siz
       return;
 
    /*
-   * If using CBC mode without an explicit IV (SSL v3 or TLS v1.0),
-   * send a single byte of plaintext to randomize the (implicit) IV of
-   * the following main block. If using a stream cipher, or TLS v1.1
-   * or higher, this isn't necessary.
+   * In versions without an explicit IV field (only TLS v1.0 now that
+   * SSLv3 has been removed) send a single byte record first to randomize
+   * the following (implicit) IV of the following record.
+   *
+   * This isn't needed in TLS v1.1 or higher.
    *
    * An empty record also works but apparently some implementations do
    * not like this (https://bugzilla.mozilla.org/show_bug.cgi?id=665814)
@@ -515,7 +516,7 @@ void Channel::send_record_array(u16bit epoch, byte type, const byte input[], siz
 
    auto cipher_state = write_cipher_state_epoch(epoch);
 
-   if(type == APPLICATION_DATA && cipher_state->cbc_without_explicit_iv())
+   if(type == APPLICATION_DATA && m_active_state->version().supports_explicit_cbc_ivs() == false)
       {
       write_record(cipher_state.get(), epoch, type, input, 1);
       input += 1;
diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp
index 9646aa320..53ef8e46d 100644
--- a/src/lib/tls/tls_policy.cpp
+++ b/src/lib/tls/tls_policy.cpp
@@ -352,12 +352,19 @@ std::vector<u16bit> Policy::ciphersuite_list(Protocol_Version version,
 
    for(auto&& suite : Ciphersuite::all_known_ciphersuites())
       {
-      if(!acceptable_ciphersuite(suite))
+      // Can we use it?
+      if(suite.valid() == false)
          continue;
 
+      // Is it acceptable to the policy?
+      if(!this->acceptable_ciphersuite(suite))
+         continue;
+
+      // Are we doing SRP?
       if(!have_srp && suite.kex_algo() == "SRP_SHA")
          continue;
 
+      // Are we doing AEAD in a non-AEAD version
       if(!version.supports_aead_modes() && suite.mac_algo() == "AEAD")
          continue;
 
diff --git a/src/lib/tls/tls_record.cpp b/src/lib/tls/tls_record.cpp
index 88609ca26..0bee24e34 100644
--- a/src/lib/tls/tls_record.cpp
+++ b/src/lib/tls/tls_record.cpp
@@ -13,6 +13,7 @@
 #include <botan/loadstor.h>
 #include <botan/internal/tls_seq_numbers.h>
 #include <botan/internal/tls_session_key.h>
+#include <botan/internal/tls_cbc.h>
 #include <botan/internal/rounding.h>
 #include <botan/internal/ct_utils.h>
 #include <botan/rng.h>
@@ -29,8 +30,7 @@ Connection_Cipher_State::Connection_Cipher_State(Protocol_Version version,
                                                  bool uses_encrypt_then_mac) :
    m_start_time(std::chrono::system_clock::now()),
    m_nonce_bytes_from_handshake(suite.nonce_bytes_from_handshake()),
-   m_nonce_bytes_from_record(suite.nonce_bytes_from_record()),
-   m_uses_encrypt_then_mac(uses_encrypt_then_mac)
+   m_nonce_bytes_from_record(suite.nonce_bytes_from_record())
    {
    SymmetricKey mac_key, cipher_key;
    InitializationVector iv;
@@ -48,46 +48,75 @@ Connection_Cipher_State::Connection_Cipher_State(Protocol_Version version,
       mac_key = keys.server_mac_key();
       }
 
-   const std::string cipher_algo = suite.cipher_algo();
-   const std::string mac_algo = suite.mac_algo();
+   BOTAN_ASSERT_EQUAL(iv.length(), nonce_bytes_from_handshake(), "Matching nonce sizes");
 
-   if(AEAD_Mode* aead = get_aead(cipher_algo, our_side ? ENCRYPTION : DECRYPTION))
+   m_nonce = unlock(iv.bits_of());
+
+   if(suite.mac_algo() == "AEAD")
       {
-      m_aead.reset(aead);
-      m_aead->set_key(cipher_key + mac_key);
+      m_aead.reset(get_aead(suite.cipher_algo(), our_side ? ENCRYPTION : DECRYPTION));
+      BOTAN_ASSERT(m_aead, "Have AEAD");
 
-      BOTAN_ASSERT_EQUAL(iv.length(), nonce_bytes_from_handshake(), "Matching nonce sizes");
-      m_nonce = unlock(iv.bits_of());
+      m_aead->set_key(cipher_key + mac_key);
 
       BOTAN_ASSERT(nonce_bytes_from_record() == 0 || nonce_bytes_from_record() == 8,
                    "Ciphersuite uses implemented IV length");
 
+      m_cbc_nonce = false;
       if(m_nonce.size() != 12)
          {
          m_nonce.resize(m_nonce.size() + 8);
          }
-
-      return;
       }
+   else
+      {
+      // legacy CBC+HMAC mode
+      if(our_side)
+         {
+         m_aead.reset(new TLS_CBC_HMAC_AEAD_Encryption(
+                         suite.cipher_algo(),
+                         suite.cipher_keylen(),
+                         suite.mac_algo(),
+                         suite.mac_keylen(),
+                         version.supports_explicit_cbc_ivs(),
+                         uses_encrypt_then_mac));
+         }
+      else
+         {
+         m_aead.reset(new TLS_CBC_HMAC_AEAD_Decryption(
+                         suite.cipher_algo(),
+                         suite.cipher_keylen(),
+                         suite.mac_algo(),
+                         suite.mac_keylen(),
+                         version.supports_explicit_cbc_ivs(),
+                         uses_encrypt_then_mac));
+         }
 
-   m_block_cipher = BlockCipher::create(cipher_algo);
-   m_mac = MessageAuthenticationCode::create("HMAC(" + mac_algo + ")");
-   if(!m_block_cipher)
-      throw Invalid_Argument("Unknown TLS cipher " + cipher_algo);
-
-   m_block_cipher->set_key(cipher_key);
-   m_block_cipher_cbc_state = iv.bits_of();
-   m_block_size = m_block_cipher->block_size();
-
-   if(version.supports_explicit_cbc_ivs())
-      m_iv_size = m_block_size;
+      m_aead->set_key(cipher_key + mac_key);
 
-   m_mac->set_key(mac_key);
+      m_cbc_nonce = true;
+      if(version.supports_explicit_cbc_ivs())
+         m_nonce_bytes_from_record = m_nonce_bytes_from_handshake;
+      else if(our_side == false)
+         m_aead->start(iv.bits_of());
+      }
    }
 
-std::vector<byte> Connection_Cipher_State::aead_nonce(u64bit seq)
+std::vector<byte> Connection_Cipher_State::aead_nonce(u64bit seq, RandomNumberGenerator& rng)
    {
-   if(nonce_bytes_from_handshake() == 12)
+   if(m_cbc_nonce)
+      {
+      if(m_nonce.size())
+         {
+         std::vector<byte> nonce;
+         nonce.swap(m_nonce);
+         return nonce;
+         }
+      std::vector<byte> nonce(nonce_bytes_from_record());
+      rng.randomize(nonce.data(), nonce.size());
+      return nonce;
+      }
+   else if(nonce_bytes_from_handshake() == 12)
       {
       std::vector<byte> nonce(12);
       store_be(seq, nonce.data() + 4);
@@ -105,7 +134,14 @@ std::vector<byte> Connection_Cipher_State::aead_nonce(u64bit seq)
 std::vector<byte>
 Connection_Cipher_State::aead_nonce(const byte record[], size_t record_len, u64bit seq)
    {
-   if(nonce_bytes_from_handshake() == 12)
+   if(m_cbc_nonce)
+      {
+      if(record_len < nonce_bytes_from_record())
+         throw Decoding_Error("Invalid CBC packet too short to be valid");
+      std::vector<byte> nonce(record, record + nonce_bytes_from_record());
+      return nonce;
+      }
+   else if(nonce_bytes_from_handshake() == 12)
       {
       /*
       Assumes if the suite specifies 12 bytes come from the handshake then
@@ -157,29 +193,6 @@ Connection_Cipher_State::format_ad(u64bit msg_sequence,
 
 namespace {
 
-void cbc_encrypt_record(const BlockCipher& bc,
-                        secure_vector<byte>& cbc_state,
-                        byte buf[],
-                        size_t buf_size)
-   {
-   const size_t block_size = bc.block_size();
-   const size_t blocks = buf_size / block_size;
-   BOTAN_ASSERT(buf_size % block_size == 0, "CBC input");
-   BOTAN_ASSERT(blocks > 0, "Expected at least 1 block");
-
-   xor_buf(buf, cbc_state.data(), block_size);
-   bc.encrypt(buf);
-
-   for(size_t i = 1; i < blocks; ++i)
-      {
-      xor_buf(&buf[block_size*i], &buf[block_size*(i-1)], block_size);
-      bc.encrypt(&buf[block_size*i]);
-      }
-
-   cbc_state.assign(&buf[block_size*(blocks-1)],
-                    &buf[block_size*blocks]);
-   }
-
 inline void append_u16_len(secure_vector<byte>& output, size_t len_field)
    {
    const uint16_t len16 = len_field;
@@ -213,99 +226,38 @@ void write_record(secure_vector<byte>& output,
       {
       append_u16_len(output, msg.get_size());
       output.insert(output.end(), msg.get_data(), msg.get_data() + msg.get_size());
-
       return;
       }
 
+   AEAD_Mode* aead = cs->aead();
    std::vector<byte> aad = cs->format_ad(seq, msg.get_type(), version, static_cast<u16bit>(msg.get_size()));
 
-   if(AEAD_Mode* aead = cs->aead())
-      {
-      const size_t ctext_size = aead->output_length(msg.get_size());
-
-      const std::vector<byte> nonce = cs->aead_nonce(seq);
+   const size_t ctext_size = aead->output_length(msg.get_size());
 
-      const size_t rec_size = ctext_size + cs->nonce_bytes_from_record();
+   const size_t rec_size = ctext_size + cs->nonce_bytes_from_record();
 
-      BOTAN_ASSERT(rec_size <= 0xFFFF, "Ciphertext length fits in field");
-      append_u16_len(output, rec_size);
+   aead->set_ad(aad);
 
-      aead->set_ad(aad);
+   const std::vector<byte> nonce = cs->aead_nonce(seq, rng);
 
-      if(cs->nonce_bytes_from_record() > 0)
-         {
-         output += std::make_pair(&nonce[cs->nonce_bytes_from_handshake()], cs->nonce_bytes_from_record());
-         }
-      const size_t header_size = output.size();
-      output += std::make_pair(msg.get_data(), msg.get_size());
+   append_u16_len(output, rec_size);
 
-      aead->start(nonce);
-      aead->finish(output, header_size);
-
-      BOTAN_ASSERT(output.size() < MAX_CIPHERTEXT_SIZE,
-                   "Produced ciphertext larger than protocol allows");
-      return;
-      }
-
-   const size_t block_size = cs->block_size();
-   const size_t iv_size = cs->iv_size();
-   const size_t mac_size = cs->mac_size();
-
-   const size_t input_size =
-      iv_size + msg.get_size() + 1 + (cs->uses_encrypt_then_mac() ? 0 : mac_size);
-   const size_t enc_size = round_up(input_size, block_size);
-   const size_t pad_val = enc_size - input_size;
-   const size_t buf_size = enc_size + (cs->uses_encrypt_then_mac() ? mac_size : 0);
-
-   if(cs->uses_encrypt_then_mac())
+   if(cs->nonce_bytes_from_record() > 0)
       {
-      aad[11] = get_byte<uint16_t>(0, enc_size);
-      aad[12] = get_byte<uint16_t>(1, enc_size);
+      if(cs->cbc_nonce())
+         output += nonce;
+      else
+         output += std::make_pair(&nonce[cs->nonce_bytes_from_handshake()], cs->nonce_bytes_from_record());
       }
 
-   BOTAN_ASSERT(enc_size % block_size == 0,
-                "Buffer is an even multiple of block size");
-
-   append_u16_len(output, buf_size);
-
    const size_t header_size = output.size();
+   output += std::make_pair(msg.get_data(), msg.get_size());
 
-   if(iv_size)
-      {
-      output.resize(output.size() + iv_size);
-      rng.randomize(&output[output.size() - iv_size], iv_size);
-      }
-
-   output.insert(output.end(), msg.get_data(), msg.get_data() + msg.get_size());
-
-   // EtM also uses ciphertext size instead of plaintext size for AEAD input
-   const byte* mac_input = (cs->uses_encrypt_then_mac() ? &output[header_size] : msg.get_data());
-   const size_t mac_input_len = (cs->uses_encrypt_then_mac() ? enc_size : msg.get_size());
-
-   if(cs->uses_encrypt_then_mac())
-      {
-      for(size_t i = 0; i != pad_val + 1; ++i)
-         output.push_back(static_cast<byte>(pad_val));
-      cbc_encrypt_record(*cs->block_cipher(), cs->cbc_state(), &output[header_size], enc_size);
-      }
-
-   output.resize(output.size() + mac_size);
-   cs->mac()->update(aad);
-   cs->mac()->update(mac_input, mac_input_len);
-   cs->mac()->final(&output[output.size() - mac_size]);
-
-   if(cs->uses_encrypt_then_mac() == false)
-      {
-      for(size_t i = 0; i != pad_val + 1; ++i)
-         output.push_back(static_cast<byte>(pad_val));
-      cbc_encrypt_record(*cs->block_cipher(), cs->cbc_state(), &output[header_size], buf_size);
-      }
-
-   if(buf_size > MAX_CIPHERTEXT_SIZE)
-      throw Internal_Error("Output record is larger than allowed by protocol");
+   aead->start(nonce);
+   aead->finish(output, header_size);
 
-   BOTAN_ASSERT_EQUAL(buf_size + header_size, output.size(),
-                      "Output buffer is sized properly");
+   BOTAN_ASSERT(output.size() < MAX_CIPHERTEXT_SIZE,
+                "Produced ciphertext larger than protocol allows");
    }
 
 namespace {
@@ -329,72 +281,6 @@ size_t fill_buffer_to(secure_vector<byte>& readbuf,
    return (desired - readbuf.size()); // how many bytes do we still need?
    }
 
-/*
-* Checks the TLS padding. Returns 0 if the padding is invalid (we
-* count the padding_length field as part of the padding size so a
-* valid padding will always be at least one byte long), or the length
-* of the padding otherwise. This is actually padding_length + 1
-* because both the padding and padding_length fields are padding from
-* our perspective.
-*
-* Returning 0 in the error case should ensure the MAC check will fail.
-* This approach is suggested in section 6.2.3.2 of RFC 5246.
-*/
-u16bit tls_padding_check(const byte record[], size_t record_len)
-   {
-   /*
-   * TLS v1.0 and up require all the padding bytes be the same value
-   * and allows up to 255 bytes.
-   */
-
-   const byte pad_byte = record[(record_len-1)];
-
-   byte pad_invalid = 0;
-   for(size_t i = 0; i != record_len; ++i)
-      {
-      const size_t left = record_len - i - 2;
-      const byte delim_mask = CT::is_less<u16bit>(static_cast<u16bit>(left), pad_byte) & 0xFF;
-      pad_invalid |= (delim_mask & (record[i] ^ pad_byte));
-      }
-
-   u16bit pad_invalid_mask = CT::expand_mask<u16bit>(pad_invalid);
-   return CT::select<u16bit>(pad_invalid_mask, 0, pad_byte + 1);
-   }
-
-void cbc_decrypt_record(byte record_contents[], size_t record_len,
-                        Connection_Cipher_State& cs,
-                        const BlockCipher& bc)
-   {
-   const size_t block_size = cs.block_size();
-
-   BOTAN_ASSERT(record_len % block_size == 0,
-                "Buffer is an even multiple of block size");
-
-   const size_t blocks = record_len / block_size;
-
-   BOTAN_ASSERT(blocks >= 1, "At least one ciphertext block");
-
-   byte* buf = record_contents;
-
-   secure_vector<byte> last_ciphertext(block_size);
-   copy_mem(last_ciphertext.data(), buf, block_size);
-
-   bc.decrypt(buf);
-   xor_buf(buf, &cs.cbc_state()[0], block_size);
-
-   secure_vector<byte> last_ciphertext2;
-
-   for(size_t i = 1; i < blocks; ++i)
-      {
-      last_ciphertext2.assign(&buf[block_size*i], &buf[block_size*(i+1)]);
-      bc.decrypt(&buf[block_size*i]);
-      xor_buf(&buf[block_size*i], last_ciphertext.data(), block_size);
-      std::swap(last_ciphertext, last_ciphertext2);
-      }
-
-   cs.cbc_state() = last_ciphertext;
-   }
-
 void decrypt_record(secure_vector<byte>& output,
                     byte record_contents[], size_t record_len,
                     u64bit record_sequence,
@@ -402,121 +288,24 @@ void decrypt_record(secure_vector<byte>& output,
                     Record_Type record_type,
                     Connection_Cipher_State& cs)
    {
-   if(AEAD_Mode* aead = cs.aead())
-      {
-      const std::vector<byte> nonce = cs.aead_nonce(record_contents, record_len, record_sequence);
-      const byte* msg = &record_contents[cs.nonce_bytes_from_record()];
-      const size_t msg_length = record_len - cs.nonce_bytes_from_record();
-
-      const size_t ptext_size = aead->output_length(msg_length);
-
-      aead->set_associated_data_vec(
-         cs.format_ad(record_sequence, record_type, record_version, static_cast<u16bit>(ptext_size))
-         );
-
-      aead->start(nonce);
-
-      const size_t offset = output.size();
-      output += std::make_pair(msg, msg_length);
-      aead->finish(output, offset);
+   AEAD_Mode* aead = cs.aead();
+   BOTAN_ASSERT(aead, "Cannot decrypt without cipher");
 
-      BOTAN_ASSERT(output.size() == ptext_size + offset, "Produced expected size");
-      }
-   else
-      {
-      // GenericBlockCipher case
-      BlockCipher* bc = cs.block_cipher();
-      BOTAN_ASSERT(bc != nullptr, "No cipher state set but needed to decrypt");
-
-      const size_t mac_size = cs.mac_size();
-      const size_t iv_size = cs.iv_size();
-
-      if(!cs.uses_encrypt_then_mac())
-         {
-         // This early exit does not leak info because all the values are public
-         if((record_len < mac_size + iv_size) || (record_len % cs.block_size() != 0))
-            throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
-
-         CT::poison(record_contents, record_len);
-
-         cbc_decrypt_record(record_contents, record_len, cs, *bc);
-
-         // 0 if padding was invalid, otherwise 1 + padding_bytes
-         u16bit pad_size = tls_padding_check(record_contents, record_len);
-
-         // This mask is zero if there is not enough room in the packet to get
-         // a valid MAC. We have to accept empty packets, since otherwise we
-         // are not compatible with the BEAST countermeasure (thus record_len+1).
-         const u16bit size_ok_mask = CT::is_lte<u16bit>(static_cast<u16bit>(mac_size + pad_size + iv_size), static_cast<u16bit>(record_len + 1));
-         pad_size &= size_ok_mask;
-
-         CT::unpoison(record_contents, record_len);
-
-         /*
-         This is unpoisoned sooner than it should. The pad_size leaks to plaintext_length and
-         then to the timing channel in the MAC computation described in the Lucky 13 paper.
-         */
-         CT::unpoison(pad_size);
-
-         const byte* plaintext_block = &record_contents[iv_size];
-         const u16bit plaintext_length = static_cast<u16bit>(record_len - mac_size - iv_size - pad_size);
-
-         cs.mac()->update(cs.format_ad(record_sequence, record_type, record_version, plaintext_length));
-         cs.mac()->update(plaintext_block, plaintext_length);
-
-         std::vector<byte> mac_buf(mac_size);
-         cs.mac()->final(mac_buf.data());
+   const std::vector<byte> nonce = cs.aead_nonce(record_contents, record_len, record_sequence);
+   const byte* msg = &record_contents[cs.nonce_bytes_from_record()];
+   const size_t msg_length = record_len - cs.nonce_bytes_from_record();
 
-         const size_t mac_offset = record_len - (mac_size + pad_size);
+   const size_t ptext_size = aead->output_length(msg_length);
 
-         const bool mac_ok = same_mem(&record_contents[mac_offset], mac_buf.data(), mac_size);
+   aead->set_associated_data_vec(
+      cs.format_ad(record_sequence, record_type, record_version, static_cast<u16bit>(ptext_size))
+      );
 
-         const u16bit ok_mask = size_ok_mask & CT::expand_mask<u16bit>(mac_ok) & CT::expand_mask<u16bit>(pad_size);
+   aead->start(nonce);
 
-         CT::unpoison(ok_mask);
-
-         if(ok_mask)
-            {
-            output.assign(plaintext_block, plaintext_block + plaintext_length);
-            }
-         else
-            {
-            throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
-            }
-         }
-      else
-         {
-         const size_t enc_size = record_len - mac_size;
-         // This early exit does not leak info because all the values are public
-         if((record_len < mac_size + iv_size) || ( enc_size % cs.block_size() != 0))
-            throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
-
-         cs.mac()->update(cs.format_ad(record_sequence, record_type, record_version, enc_size));
-         cs.mac()->update(record_contents, enc_size);
-
-         std::vector<byte> mac_buf(mac_size);
-         cs.mac()->final(mac_buf.data());
-
-         const size_t mac_offset = enc_size;
-
-         const bool mac_ok = same_mem(&record_contents[mac_offset], mac_buf.data(), mac_size);
-
-         if(!mac_ok)
-            {
-            throw TLS_Exception(Alert::BAD_RECORD_MAC, "Message authentication failure");
-            }
-
-         cbc_decrypt_record(record_contents, enc_size, cs, *bc);
-
-         // 0 if padding was invalid, otherwise 1 + padding_bytes
-         u16bit pad_size = tls_padding_check(record_contents, enc_size);
-
-         const byte* plaintext_block = &record_contents[iv_size];
-         const u16bit plaintext_length = enc_size - iv_size - pad_size;
-
-         output.assign(plaintext_block, plaintext_block + plaintext_length);
-         }
-      }
+   const size_t offset = output.size();
+   output += std::make_pair(msg, msg_length);
+   aead->finish(output, offset);
    }
 
 size_t read_tls_record(secure_vector<byte>& readbuf,
diff --git a/src/lib/tls/tls_record.h b/src/lib/tls/tls_record.h
index 4420a9c66..b17d0a7b6 100644
--- a/src/lib/tls/tls_record.h
+++ b/src/lib/tls/tls_record.h
@@ -44,7 +44,7 @@ class Connection_Cipher_State
 
       AEAD_Mode* aead() { return m_aead.get(); }
 
-      std::vector<byte> aead_nonce(u64bit seq);
+      std::vector<byte> aead_nonce(u64bit seq, RandomNumberGenerator& rng);
 
       std::vector<byte> aead_nonce(const byte record[], size_t record_len, u64bit seq);
 
@@ -52,26 +52,9 @@ class Connection_Cipher_State
                                   Protocol_Version version,
                                   u16bit ptext_length);
 
-      BlockCipher* block_cipher() { return m_block_cipher.get(); }
-
-      MessageAuthenticationCode* mac() { return m_mac.get(); }
-
-      secure_vector<byte>& cbc_state() { return m_block_cipher_cbc_state; }
-
-      size_t block_size() const { return m_block_size; }
-
-      size_t mac_size() const { return m_mac->output_length(); }
-
-      size_t iv_size() const { return m_iv_size; }
-
-      size_t nonce_bytes_from_record() const { return m_nonce_bytes_from_record; }
-
       size_t nonce_bytes_from_handshake() const { return m_nonce_bytes_from_handshake; }
-
-      bool uses_encrypt_then_mac() const { return m_uses_encrypt_then_mac; }
-
-      bool cbc_without_explicit_iv() const
-         { return (m_block_size > 0) && (m_iv_size == 0); }
+      size_t nonce_bytes_from_record() const { return m_nonce_bytes_from_record; }
+      bool cbc_nonce() const { return m_cbc_nonce; }
 
       std::chrono::seconds age() const
          {
@@ -81,19 +64,12 @@ class Connection_Cipher_State
 
    private:
       std::chrono::system_clock::time_point m_start_time;
-      std::unique_ptr<BlockCipher> m_block_cipher;
-      secure_vector<byte> m_block_cipher_cbc_state;
-      std::unique_ptr<MessageAuthenticationCode> m_mac;
-
       std::unique_ptr<AEAD_Mode> m_aead;
-      std::vector<byte> m_nonce;
 
-      size_t m_block_size = 0;
+      std::vector<byte> m_nonce;
       size_t m_nonce_bytes_from_handshake;
       size_t m_nonce_bytes_from_record;
-      size_t m_iv_size = 0;
-      
-      bool m_uses_encrypt_then_mac;
+      bool m_cbc_nonce;
    };
 
 class Record
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index 5ae24ba81..f869f426b 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -118,9 +118,10 @@ class Credentials_Manager_Test : public Botan::Credentials_Manager
       bool m_provides_client_certs;
    };
 
-Botan::Credentials_Manager* create_creds(bool client_type)
+Botan::Credentials_Manager* create_creds(Botan::RandomNumberGenerator& rng,
+                                         bool with_client_certs = false)
    {
-   std::unique_ptr<Botan::Private_Key> ca_key(new Botan::RSA_PrivateKey(Test::rng(), 1024));
+   std::unique_ptr<Botan::Private_Key> ca_key(new Botan::RSA_PrivateKey(rng, 1024));
 
    Botan::X509_Cert_Options ca_opts;
    ca_opts.common_name = "Test CA";
@@ -131,9 +132,9 @@ Botan::Credentials_Manager* create_creds(bool client_type)
       Botan::X509::create_self_signed_cert(ca_opts,
                                            *ca_key,
                                            "SHA-256",
-                                           Test::rng());
+                                           rng);
 
-   Botan::Private_Key* server_key = new Botan::RSA_PrivateKey(Test::rng(), 1024);
+   Botan::Private_Key* server_key = new Botan::RSA_PrivateKey(rng, 1024);
 
    Botan::X509_Cert_Options server_opts;
    server_opts.common_name = "server.example.com";
@@ -142,7 +143,7 @@ Botan::Credentials_Manager* create_creds(bool client_type)
    Botan::PKCS10_Request req = Botan::X509::create_cert_req(server_opts,
                                                             *server_key,
                                                             "SHA-256",
-                                                            Test::rng());
+                                                            rng);
 
    Botan::X509_CA ca(ca_cert, *ca_key, "SHA-256");
 
@@ -152,12 +153,12 @@ Botan::Credentials_Manager* create_creds(bool client_type)
    Botan::X509_Time end_time(now + years(1));
 
    Botan::X509_Certificate server_cert = ca.sign_request(req,
-                                                         Test::rng(),
+                                                         rng,
                                                          start_time,
                                                          end_time);
 
    Credentials_Manager_Test* cmt (new Credentials_Manager_Test(server_cert, ca_cert, server_key));
-   cmt->m_provides_client_certs = client_type;
+   cmt->m_provides_client_certs = with_client_certs;
    return cmt;
    }
 
@@ -176,11 +177,10 @@ void alert_cb_with_data(Botan::TLS::Alert, const byte[], size_t)
 
 Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
                                 Botan::Credentials_Manager& creds,
-                                Botan::TLS::Policy& client_policy,
-                                Botan::TLS::Policy& server_policy )
+                                const Botan::TLS::Policy& client_policy,
+                                const Botan::TLS::Policy& server_policy,
+                                Botan::RandomNumberGenerator& rng)
    {
-   Botan::RandomNumberGenerator& rng = Test::rng();
-
    Botan::TLS::Session_Manager_In_Memory server_sessions(rng);
    Botan::TLS::Session_Manager_In_Memory client_sessions(rng);
 
@@ -197,9 +197,12 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
       auto handshake_complete = [&](const Botan::TLS::Session& session) -> bool {
          handshake_done = true;
 
-         result.test_note("Session established " + session.version().to_string() + " " +
-                          session.ciphersuite().to_string() + " " +
-                          Botan::hex_encode(session.session_id()));
+         const std::string session_report =
+            "Session established " + session.version().to_string() + " " +
+            session.ciphersuite().to_string() + " " +
+            Botan::hex_encode(session.session_id());
+
+         result.test_note(session_report);
 
          if(session.version() != offer_version)
             {
@@ -311,7 +314,9 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
                if(rounds > 25)
                   {
                   if(r <= 2)
+                     {
                      result.test_failure("Still here after many rounds, deadlock?");
+                     }
                   break;
                   }
 
@@ -320,21 +325,39 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
 
                if(client->is_active() && client_sent.empty())
                   {
-                  // Choose a len between 1 and 511
-                  const size_t c_len = 1 + rng.next_byte() + rng.next_byte();
+                  // Choose random application data to send
+                  const size_t c_len = 1 + (static_cast<size_t>(rng.next_byte()) << 4) ^ rng.next_byte();
                   client_sent = unlock(rng.random_vec(c_len));
 
-                  // TODO send in several records
-                  client->send(client_sent);
+                  size_t sent_so_far = 0;
+                  while(sent_so_far != client_sent.size())
+                     {
+                     const size_t left = client_sent.size() - sent_so_far;
+                     const size_t rnd12 = (rng.next_byte() << 4) ^ rng.next_byte();
+                     const size_t sending = std::min(left, rnd12);
+
+                     client->send(&client_sent[sent_so_far], sending);
+                     sent_so_far += sending;
+                     }
                   }
 
                if(server->is_active() && server_sent.empty())
                   {
                   result.test_eq("server->protocol", server->next_protocol(), "test/3");
 
-                  const size_t s_len = 1 + rng.next_byte() + rng.next_byte();
+                  const size_t s_len = 1 + (static_cast<size_t>(rng.next_byte()) << 4) ^ rng.next_byte();
                   server_sent = unlock(rng.random_vec(s_len));
-                  server->send(server_sent);
+
+                  size_t sent_so_far = 0;
+                  while(sent_so_far != server_sent.size())
+                     {
+                     const size_t left = server_sent.size() - sent_so_far;
+                     const size_t rnd12 = (rng.next_byte() << 4) ^ rng.next_byte();
+                     const size_t sending = std::min(left, rnd12);
+
+                     server->send(&server_sent[sent_so_far], sending);
+                     sent_so_far += sending;
+                     }
                   }
 
                const bool corrupt_client_data = (r == 3);
@@ -362,7 +385,7 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
                            result.test_lt("Total requested is readonable", total_consumed, 128*1024))
                         {
                         input.resize(needed);
-                        Test::rng().randomize(input.data(), input.size());
+                        rng.randomize(input.data(), input.size());
                         needed = server->received_data(input.data(), input.size());
                         total_consumed += needed;
                         }
@@ -391,7 +414,7 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
                      while(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024))
                         {
                         input.resize(needed);
-                        Test::rng().randomize(input.data(), input.size());
+                        rng.randomize(input.data(), input.size());
                         needed = client->received_data(input.data(), input.size());
                         total_consumed += needed;
                         }
@@ -461,20 +484,20 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
 
 Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version,
                                 Botan::Credentials_Manager& creds,
-                                Botan::TLS::Policy& policy )
+                                const Botan::TLS::Policy& policy,
+                                Botan::RandomNumberGenerator& rng)
    {
-   return test_tls_handshake(offer_version, creds, policy, policy);
+   return test_tls_handshake(offer_version, creds, policy, policy, rng);
    }
 
 Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
                                  Botan::Credentials_Manager& creds,
-                                 Botan::TLS::Policy& client_policy,
-                                 Botan::TLS::Policy& server_policy )
+                                 const Botan::TLS::Policy& client_policy,
+                                 const Botan::TLS::Policy& server_policy,
+                                 Botan::RandomNumberGenerator& rng)
    {
    BOTAN_ASSERT(offer_version.is_datagram_protocol(), "Test is for datagram version");
 
-   Botan::RandomNumberGenerator& rng = Test::rng();
-
    Botan::TLS::Session_Manager_In_Memory server_sessions(rng);
    Botan::TLS::Session_Manager_In_Memory client_sessions(rng);
 
@@ -592,7 +615,7 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
             while(true)
                {
                // TODO: client and server should be in different threads
-               std::this_thread::sleep_for(std::chrono::milliseconds(rng.next_byte() % 2));
+               std::this_thread::sleep_for(std::chrono::microseconds(rng.next_byte() % 128));
                ++rounds;
 
                if(rounds > 100)
@@ -644,7 +667,7 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
                         if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024))
                            {
                            input.resize(needed);
-                           Test::rng().randomize(input.data(), input.size());
+                           rng.randomize(input.data(), input.size());
                            client->received_data(input.data(), input.size());
                            }
                         }
@@ -684,7 +707,7 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
                         if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024))
                            {
                            input.resize(needed);
-                           Test::rng().randomize(input.data(), input.size());
+                           rng.randomize(input.data(), input.size());
                            client->received_data(input.data(), input.size());
                            }
                         }
@@ -761,10 +784,11 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
    }
 
 Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version,
-                                Botan::Credentials_Manager& creds,
-                                Botan::TLS::Policy& policy)
+                                 Botan::Credentials_Manager& creds,
+                                 const Botan::TLS::Policy& policy,
+                                 Botan::RandomNumberGenerator& rng)
    {
-   return test_dtls_handshake(offer_version, creds, policy, policy);
+   return test_dtls_handshake(offer_version, creds, policy, policy, rng);
    }
 
 class Test_Policy : public Botan::TLS::Text_Policy
@@ -781,104 +805,131 @@ class Test_Policy : public Botan::TLS::Text_Policy
    };
 
 
+
 class TLS_Unit_Tests : public Test
    {
+   private:
+      void test_with_policy(std::vector<Test::Result>& results,
+                            const std::vector<Botan::TLS::Protocol_Version>& versions,
+                            Botan::Credentials_Manager& creds,
+                            const Botan::TLS::Policy& policy)
+         {
+         Botan::RandomNumberGenerator& rng = Test::rng();
+
+         for(auto&& version : versions)
+            {
+            if(version.is_datagram_protocol())
+               results.push_back(test_dtls_handshake(version, creds, policy, rng));
+            else
+               results.push_back(test_tls_handshake(version, creds, policy, rng));
+            }
+         }
+
+      void test_all_versions(std::vector<Test::Result>& results,
+                             Botan::Credentials_Manager& creds,
+                             const std::string& kex_policy,
+                             const std::string& cipher_policy,
+                             const std::string& mac_policy,
+                             const std::string& etm_policy)
+         {
+         Test_Policy policy;
+         policy.set("ciphers", cipher_policy);
+         policy.set("macs", mac_policy);
+         policy.set("key_exchange_methods", kex_policy);
+         policy.set("negotiate_encrypt_then_mac", etm_policy);
+
+         std::vector<Botan::TLS::Protocol_Version> versions = {
+            Botan::TLS::Protocol_Version::TLS_V10,
+            Botan::TLS::Protocol_Version::TLS_V11,
+            Botan::TLS::Protocol_Version::TLS_V12,
+            Botan::TLS::Protocol_Version::DTLS_V10,
+            Botan::TLS::Protocol_Version::DTLS_V12
+         };
+
+         return test_with_policy(results, versions, creds, policy);
+         }
+
+      void test_modern_versions(std::vector<Test::Result>& results,
+                                Botan::Credentials_Manager& creds,
+                                const std::string& kex_policy,
+                                const std::string& cipher_policy,
+                                const std::string& mac_policy = "AEAD",
+                                const std::map<std::string, std::string>& extra_policies = {})
+         {
+         Test_Policy policy;
+         policy.set("ciphers", cipher_policy);
+         policy.set("macs", mac_policy);
+         policy.set("key_exchange_methods", kex_policy);
+
+         for(auto&& kv : extra_policies)
+            policy.set(kv.first, kv.second);
+
+         std::vector<Botan::TLS::Protocol_Version> versions = {
+            Botan::TLS::Protocol_Version::TLS_V12,
+            Botan::TLS::Protocol_Version::DTLS_V12
+         };
+
+         return test_with_policy(results, versions, creds, policy);
+         }
+
    public:
       std::vector<Test::Result> run() override
          {
-         std::unique_ptr<Botan::Credentials_Manager> basic_creds(create_creds(false));
-         std::unique_ptr<Botan::Credentials_Manager> basic_creds_with_client_cert(create_creds(true));
+         Botan::RandomNumberGenerator& rng = Test::rng();
+
+         std::unique_ptr<Botan::Credentials_Manager> creds(create_creds(rng));
          std::vector<Test::Result> results;
 
-         Test_Policy policy;
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy));
-
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         policy.set("key_exchange_methods", "RSA");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         policy.set("key_exchange_methods", "DH");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy));
-
-         policy.set("key_exchange_methods", "ECDH");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         policy.set("ciphers", "AES-128");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds_with_client_cert, policy));
+         for(std::string etm_setting : { "true", "false" })
+            {
+            test_all_versions(results, *creds, "RSA", "AES-128", "SHA-256 SHA-1", etm_setting);
+            test_all_versions(results, *creds, "ECDH", "AES-128", "SHA-256 SHA-1", etm_setting);
+
+            test_all_versions(results, *creds, "RSA", "AES-256", "SHA-1", etm_setting);
+            test_all_versions(results, *creds, "ECDH", "AES-256", "SHA-1", etm_setting);
+
+#if defined(BOTAN_HAS_CAMELLIA)
+            test_all_versions(results, *creds, "RSA", "Camellia-128", "SHA-256", etm_setting);
+            test_all_versions(results, *creds, "ECDH", "Camellia-256", "SHA-256 SHA-384", etm_setting);
+#endif
+
+#if defined(BOTAN_HAS_DES)
+            test_all_versions(results, *creds, "RSA", "3DES", "SHA-1", etm_setting);
+            test_all_versions(results, *creds, "ECDH", "3DES", "SHA-1", etm_setting);
+#endif
+
+#if defined(BOTAN_HAS_SEED)
+            test_all_versions(results, *creds, "RSA", "SEED", "SHA-1", etm_setting);
+#endif
+            }
+
+         test_modern_versions(results, *creds, "DH", "AES-128", "SHA-256");
+         test_modern_versions(results, *creds, "RSA", "AES-128/GCM");
+         test_modern_versions(results, *creds, "ECDH", "AES-128/GCM");
+         test_modern_versions(results, *creds, "ECDH", "AES-128/GCM", "AEAD",
+                              { { "use_ecc_point_compression", "true" } });
+
+         std::unique_ptr<Botan::Credentials_Manager> creds_with_client_cert(create_creds(rng, true));
+         test_modern_versions(results, *creds_with_client_cert, "ECDH", "AES-256/GCM");
 
 #if defined(BOTAN_HAS_AEAD_OCB)
-         policy.set("ciphers", "AES-128/OCB(12)");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
+         test_modern_versions(results, *creds, "ECDH", "AES-128/OCB(12)");
 #endif
 
 #if defined(BOTAN_HAS_AEAD_CHACHA20_POLY1305)
-         policy.set("ciphers", "ChaCha20Poly1305");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
+         test_modern_versions(results, *creds, "ECDH", "ChaCha20Poly1305");
 #endif
 
-         policy.set("ciphers", "AES-128/GCM");
-         policy.set("key_exchange_methods", "PSK");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
+         test_modern_versions(results, *creds, "PSK", "AES-128/GCM");
+
+#if defined(BOTAN_HAS_CCM)
+         test_modern_versions(results, *creds, "PSK", "AES-128/CCM");
+         test_modern_versions(results, *creds, "PSK", "AES-128/CCM(8)");
+#endif
 
          // For whatever reason no (EC)DHE_PSK GCM ciphersuites are defined
-         policy.set("ciphers", "AES-128");
-         policy.set("key_exchange_methods", "ECDHE_PSK");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         policy.set("key_exchange_methods", "DHE_PSK");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         policy.set("negotiate_encrypt_then_mac", "false");
-         policy.set("key_exchange_methods", "ECDH");
-         policy.set("ciphers", "AES-128");
-         Test_Policy server_policy;
-         server_policy.set("key_exchange_methods", "ECDH");
-         server_policy.set("ciphers", "AES-128");
-         server_policy.set("negotiate_encrypt_then_mac", "true");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy, server_policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy, server_policy));
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy, server_policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy, server_policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy, server_policy));
-         
-         policy.set("negotiate_encrypt_then_mac", "true");
-         policy.set("ciphers", "AES-128/GCM");
-         server_policy.set("ciphers", "AES-128/GCM");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy, server_policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy, server_policy));
-         
-         policy.set("use_ecc_point_compression", "true");
-         policy.set("key_exchange_methods", "ECDH");
-         policy.set("ciphers", "AES-128");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy));
-
-         server_policy.set("use_ecc_point_compression", "true");
-         server_policy.set("key_exchange_methods", "ECDH");
-         server_policy.set("ciphers", "AES-128");
-         results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy, server_policy));
-         results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy, server_policy));
+         test_modern_versions(results, *creds, "ECDHE_PSK", "AES-128", "SHA-256");
+         test_modern_versions(results, *creds, "DHE_PSK", "AES-128", "SHA-1");
 
          return results;
          }