Don't reject TLS packets with zero plaintext bytes

OpenSSL sends an empty record before each new data record in TLS v1.0
to randomize the IV, as a countermeasure to the BEAST attack. Most
implementations use 1/(n-1) splitting for this instead.

Bug introduced with the const time changes in 1.11.23

3 files changed, 24 insertions, 13 deletions

diff --git a/doc/news.rst b/doc/news.rst
index 7a5b3b115..b1a04302a 100644
--- a/doc/news.rst
+++ b/doc/news.rst
@@ -4,6 +4,11 @@ Release Notes
 Version 1.11.30, Not Yet Released
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+* In 1.11.23 a bug was introduced such that CBC-encrypted TLS packets
+  containing no plaintext bytes at all were incorrectly rejected with
+  a MAC failure. Records like this are used by OpenSSL in TLS 1.0
+  connections in order to randomize the IV.
+
 * Add IETF versions of the ChaCha20Poly1305 TLS ciphersuites from
   draft-ietf-tls-chacha20-poly1305-04. The previously implemented
   (non-standard) ChaCha20Poly1305 ciphersuites from
diff --git a/src/lib/tls/tls_record.cpp b/src/lib/tls/tls_record.cpp
index efa6e2d18..8af6587e3 100644
--- a/src/lib/tls/tls_record.cpp
+++ b/src/lib/tls/tls_record.cpp
@@ -139,20 +139,16 @@ Connection_Cipher_State::format_ad(u64bit msg_sequence,
                                    Protocol_Version version,
                                    u16bit msg_length)
    {
-   std::vector<byte> m_ad;
-   m_ad.reserve(13);
+   std::vector<byte> ad(13);
 
-   for(size_t i = 0; i != 8; ++i)
-      m_ad.push_back(get_byte(i, msg_sequence));
-   m_ad.push_back(msg_type);
+   store_be(msg_sequence, &ad[0]);
+   ad[8] = msg_type;
+   ad[9] = version.major_version();
+   ad[10] = version.minor_version();
+   ad[11] = get_byte(0, msg_length);
+   ad[12] = get_byte(1, msg_length);
 
-   m_ad.push_back(version.major_version());
-   m_ad.push_back(version.minor_version());
-
-   m_ad.push_back(get_byte(0, msg_length));
-   m_ad.push_back(get_byte(1, msg_length));
-
-   return m_ad;
+   return ad;
    }
 
 void write_record(secure_vector<byte>& output,
@@ -425,7 +421,7 @@ void decrypt_record(secure_vector<byte>& output,
       u16bit pad_size = tls_padding_check(record_contents, record_len);
 
       // This mask is zero if there is not enough room in the packet
-      const u16bit size_ok_mask = CT::is_less<u16bit>(mac_size + pad_size + iv_size, record_len);
+      const u16bit size_ok_mask = CT::is_lte<u16bit>(mac_size + pad_size + iv_size, record_len);
       pad_size &= size_ok_mask;
 
       CT::unpoison(record_contents, record_len);
@@ -454,9 +450,13 @@ void decrypt_record(secure_vector<byte>& output,
       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");
+         }
       }
    }
 
diff --git a/src/lib/utils/ct_utils.h b/src/lib/utils/ct_utils.h
index 5a1d03d4f..1f095ba88 100644
--- a/src/lib/utils/ct_utils.h
+++ b/src/lib/utils/ct_utils.h
@@ -129,6 +129,12 @@ inline T is_less(T x, T y)
    }
 
 template<typename T>
+inline T is_lte(T x, T y)
+   {
+   return expand_mask<T>(x <= y);
+   }
+
+template<typename T>
 inline void conditional_copy_mem(T value,
                                  T* to,
                                  const T* from0,