Support setting the number of pad bytes in a heartbeat message. Use

random instead of all-zero padding. Check on sanity of received pads
to the extent possible. Bugzilla 269.

5 files changed, 86 insertions, 70 deletions

diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index 76332f7d2..4b42016fa 100644
--- a/src/lib/tls/tls_channel.cpp
+++ b/src/lib/tls/tls_channel.cpp
@@ -1,6 +1,6 @@
 /*
 * TLS Channels
-* (C) 2011,2012,2014 Jack Lloyd
+* (C) 2011,2012,2014,2015 Jack Lloyd
 *
 * Released under the terms of the Botan license
 */
@@ -405,8 +405,9 @@ size_t Channel::received_data(const byte input[], size_t input_size)
                {
                if(!pending_state())
                   {
+                  const std::vector<byte> padding = unlock(rng().random_vec(16));
                   Heartbeat_Message response(Heartbeat_Message::RESPONSE,
-                                             &payload[0], payload.size());
+                                             &payload[0], payload.size(), padding);
 
                   send_record(HEARTBEAT, response.contents());
                   }
@@ -483,12 +484,13 @@ size_t Channel::received_data(const byte input[], size_t input_size)
       }
    }
 
-void Channel::heartbeat(const byte payload[], size_t payload_size)
+void Channel::heartbeat(const byte payload[], size_t payload_size, size_t pad_size)
    {
    if(heartbeat_sending_allowed())
       {
+      const std::vector<byte> padding = unlock(rng().random_vec(pad_size + 16));
       Heartbeat_Message heartbeat(Heartbeat_Message::REQUEST,
-                                  payload, payload_size);
+                                  payload, payload_size, padding);
 
       send_record(HEARTBEAT, heartbeat.contents());
       }
diff --git a/src/lib/tls/tls_channel.h b/src/lib/tls/tls_channel.h
index 8aea2dab0..c5da583a1 100644
--- a/src/lib/tls/tls_channel.h
+++ b/src/lib/tls/tls_channel.h
@@ -1,6 +1,6 @@
 /*
 * TLS Channel
-* (C) 2011,2012,2014 Jack Lloyd
+* (C) 2011,2012,2014,2015 Jack Lloyd
 *
 * Released under the terms of the Botan license
 */
@@ -31,6 +31,21 @@ class Handshake_State;
 class BOTAN_DLL Channel
    {
    public:
+      Channel(std::function<void (const byte[], size_t)> socket_output_fn,
+              std::function<void (const byte[], size_t)> data_cb,
+              std::function<void (Alert, const byte[], size_t)> alert_cb,
+              std::function<bool (const Session&)> handshake_cb,
+              Session_Manager& session_manager,
+              RandomNumberGenerator& rng,
+              bool is_datagram,
+              size_t reserved_io_buffer_size);
+
+      Channel(const Channel&) = delete;
+
+      Channel& operator=(const Channel&) = delete;
+
+      virtual ~Channel();
+
       /**
       * Inject TLS traffic received from counterparty
       * @return a hint as the how many more bytes we need to process the
@@ -46,14 +61,6 @@ class BOTAN_DLL Channel
       size_t received_data(const std::vector<byte>& buf);
 
       /**
-      * Perform a handshake timeout check. This does nothing unless
-      * this is a DTLS channel with a pending handshake state, in
-      * which case we check for timeout and potentially retransmit
-      * handshake packets.
-      */
-      bool timeout_check();
-
-      /**
       * Inject plaintext intended for counterparty
       * Throws an exception if is_active() is false
       */
@@ -107,22 +114,29 @@ class BOTAN_DLL Channel
       */
       bool is_closed() const;
 
+
       /**
-      * Attempt to renegotiate the session
-      * @param force_full_renegotiation if true, require a full renegotiation,
-      *                                 otherwise allow session resumption
+      * @return certificate chain of the peer (may be empty)
       */
-      void renegotiate(bool force_full_renegotiation = false);
+      std::vector<X509_Certificate> peer_cert_chain() const;
 
       /**
-      * @return true iff the peer supports heartbeat messages
+      * Key material export (RFC 5705)
+      * @param label a disambiguating label string
+      * @param context a per-association context value
+      * @param length the length of the desired key in bytes
+      * @return key of length bytes
       */
-      bool peer_supports_heartbeats() const;
+      SymmetricKey key_material_export(const std::string& label,
+                                       const std::string& context,
+                                       size_t length) const;
 
       /**
-      * @return true iff we are allowed to send heartbeat messages
+      * Attempt to renegotiate the session
+      * @param force_full_renegotiation if true, require a full renegotiation,
+      * otherwise allow session resumption
       */
-      bool heartbeat_sending_allowed() const;
+      void renegotiate(bool force_full_renegotiation = false);
 
       /**
       * @return true iff the counterparty supports the secure
@@ -131,47 +145,35 @@ class BOTAN_DLL Channel
       bool secure_renegotiation_supported() const;
 
       /**
-      * Attempt to send a heartbeat message (if negotiated with counterparty)
-      * @param payload will be echoed back
-      * @param payload_size size of payload in bytes
+      * Perform a handshake timeout check. This does nothing unless
+      * this is a DTLS channel with a pending handshake state, in
+      * which case we check for timeout and potentially retransmit
+      * handshake packets.
       */
-      void heartbeat(const byte payload[], size_t payload_size);
+      bool timeout_check();
 
       /**
-      * Attempt to send a heartbeat message (if negotiated with counterparty)
+      * @return true iff the peer supports heartbeat messages
       */
-      void heartbeat() { heartbeat(nullptr, 0); }
+      bool peer_supports_heartbeats() const;
 
       /**
-      * @return certificate chain of the peer (may be empty)
+      * @return true iff we are allowed to send heartbeat messages
       */
-      std::vector<X509_Certificate> peer_cert_chain() const;
+      bool heartbeat_sending_allowed() const;
 
       /**
-      * Key material export (RFC 5705)
-      * @param label a disambiguating label string
-      * @param context a per-association context value
-      * @param length the length of the desired key in bytes
-      * @return key of length bytes
+      * Attempt to send a heartbeat message (if negotiated with counterparty)
+      * @param payload will be echoed back
+      * @param payload_size size of payload in bytes
+      * @param pad_bytes include 16 + pad_bytes extra bytes in the message (not echoed)
       */
-      SymmetricKey key_material_export(const std::string& label,
-                                       const std::string& context,
-                                       size_t length) const;
-
-      Channel(std::function<void (const byte[], size_t)> socket_output_fn,
-              std::function<void (const byte[], size_t)> data_cb,
-              std::function<void (Alert, const byte[], size_t)> alert_cb,
-              std::function<bool (const Session&)> handshake_cb,
-              Session_Manager& session_manager,
-              RandomNumberGenerator& rng,
-              bool is_datagram,
-              size_t reserved_io_buffer_size);
-
-      Channel(const Channel&) = delete;
-
-      Channel& operator=(const Channel&) = delete;
+      void heartbeat(const byte payload[], size_t payload_size, size_t pad_bytes = 0);
 
-      virtual ~Channel();
+      /**
+      * Attempt to send a heartbeat message (if negotiated with counterparty)
+      */
+      void heartbeat() { heartbeat(nullptr, 0); }
    protected:
 
       virtual void process_handshake_msg(const Handshake_State* active_state,
diff --git a/src/lib/tls/tls_heartbeats.cpp b/src/lib/tls/tls_heartbeats.cpp
index 1b4f099c1..a8bcc0189 100644
--- a/src/lib/tls/tls_heartbeats.cpp
+++ b/src/lib/tls/tls_heartbeats.cpp
@@ -1,6 +1,6 @@
 /*
 * TLS Heartbeats
-* (C) 2012 Jack Lloyd
+* (C) 2012,2015 Jack Lloyd
 *
 * Released under the terms of the Botan license
 */
@@ -28,25 +28,37 @@ Heartbeat_Message::Heartbeat_Message(const std::vector<byte>& buf)
 
    m_payload = reader.get_range<byte>(2, 0, 16*1024);
 
-   // padding follows and is ignored
+   m_padding = reader.get_remaining();
+
+   if(m_padding.size() < 16)
+      throw Decoding_Error("Invalid heartbeat padding");
    }
 
 Heartbeat_Message::Heartbeat_Message(Type type,
                                      const byte payload[],
-                                     size_t payload_len) :
+                                     size_t payload_len,
+                                     const std::vector<byte>& padding) :
    m_type(type),
-   m_payload(payload, payload + payload_len)
+   m_payload(payload, payload + payload_len),
+   m_padding(padding)
    {
+   if(payload_len >= 64*1024)
+      throw std::runtime_error("Heartbeat payload too long");
+   if(m_padding.size() < 16)
+      throw std::runtime_error("Invalid heartbeat padding length");
    }
 
 std::vector<byte> Heartbeat_Message::contents() const
    {
-   std::vector<byte> send_buf(3 + m_payload.size() + 16);
-   send_buf[0] = m_type;
-   send_buf[1] = get_byte<u16bit>(0, m_payload.size());
-   send_buf[2] = get_byte<u16bit>(1, m_payload.size());
-   copy_mem(&send_buf[3], &m_payload[0], m_payload.size());
-   // leave padding as all zeros
+   //std::vector<byte> send_buf(3 + m_payload.size() + 16);
+   std::vector<byte> send_buf;
+   send_buf.reserve(3 + m_payload.size() + m_padding.size());
+
+   send_buf.push_back(m_type);
+   send_buf.push_back(get_byte<u16bit>(0, m_payload.size()));
+   send_buf.push_back(get_byte<u16bit>(1, m_payload.size()));
+   send_buf += m_payload;
+   send_buf += m_padding;
 
    return send_buf;
    }
diff --git a/src/lib/tls/tls_heartbeats.h b/src/lib/tls/tls_heartbeats.h
index 00b197089..c17c5606f 100644
--- a/src/lib/tls/tls_heartbeats.h
+++ b/src/lib/tls/tls_heartbeats.h
@@ -1,6 +1,6 @@
 /*
 * TLS Heartbeats
-* (C) 2012 Jack Lloyd
+* (C) 2012,2015 Jack Lloyd
 *
 * Released under the terms of the Botan license
 */
@@ -30,10 +30,11 @@ class Heartbeat_Message
 
       Heartbeat_Message(const std::vector<byte>& buf);
 
-      Heartbeat_Message(Type type, const byte payload[], size_t payload_len);
+      Heartbeat_Message(Type type, const byte payload[], size_t payload_len,
+                        const std::vector<byte>& padding);
    private:
       Type m_type;
-      std::vector<byte> m_payload;
+      std::vector<byte> m_payload, m_padding;
    };
 
 }
diff --git a/src/lib/tls/tls_reader.h b/src/lib/tls/tls_reader.h
index 028893cc1..f80e494f4 100644
--- a/src/lib/tls/tls_reader.h
+++ b/src/lib/tls/tls_reader.h
@@ -34,14 +34,13 @@ class TLS_Data_Reader
             throw decode_error("Extra bytes at end of message");
          }
 
-      size_t remaining_bytes() const
-         {
-         return m_buf.size() - m_offset;
-         }
+      size_t remaining_bytes() const { return m_buf.size() - m_offset; }
+
+      bool has_remaining() const { return (remaining_bytes() > 0); }
 
-      bool has_remaining() const
+      std::vector<byte> get_remaining()
          {
-         return (remaining_bytes() > 0);
+         return std::vector<byte>(m_buf.begin() + m_offset, m_buf.end());
          }
 
       void discard_next(size_t bytes)