FIX: Detect stream truncation errors

TLS::Stream now detects connections that have been improperly closed by
the peer without sending a close_notify alert first. This is indicated
by a StreamTruncated error code.

4 files changed, 205 insertions, 135 deletions

diff --git a/src/lib/tls/asio/asio_async_ops.h b/src/lib/tls/asio/asio_async_ops.h
index 3520db770..47267a569 100644
--- a/src/lib/tls/asio/asio_async_ops.h
+++ b/src/lib/tls/asio/asio_async_ops.h
@@ -1,7 +1,7 @@
 /*
 * Helpers for TLS ASIO Stream
-* (C) 2018-2019 Jack Lloyd
-*     2018-2019 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
+* (C) 2018-2020 Jack Lloyd
+*     2018-2020 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -144,24 +144,18 @@ class AsyncReadOperation : public AsyncBase<Handler, typename Stream::executor_t
                {
                // We have received encrypted data from the network, now hand it to TLS::Channel for decryption.
                boost::asio::const_buffer read_buffer{m_stream.input_buffer().data(), bytes_transferred};
-               try
-                  {
-                  m_stream.native_handle()->received_data(
-                     static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size()
-                  );
-                  }
-               catch(const TLS_Exception& e)
-                  {
-                  ec = e.type();
-                  }
-               catch(const Botan::Exception& e)
-                  {
-                  ec = e.error_type();
-                  }
-               catch(...)
-                  {
-                  ec = Botan::ErrorType::Unknown;
-                  }
+               m_stream.process_encrypted_data(read_buffer, ec);
+               }
+
+            if (m_stream.shutdown_received())
+               {
+               // we just received a 'close_notify' from the peer and don't expect any more data
+               ec = boost::asio::error::eof;
+               }
+            else if (ec == boost::asio::error::eof)
+               {
+               // we did not expect this disconnection from the peer
+               ec = StreamError::StreamTruncated;
                }
 
             if(!m_stream.has_received_data() && !ec && boost::asio::buffer_size(m_buffers) > 0)
@@ -242,6 +236,12 @@ class AsyncWriteOperation : public AsyncBase<Handler, typename Stream::executor_
                return;
                }
 
+            if (ec == boost::asio::error::eof && !m_stream.shutdown_received())
+               {
+               // transport layer was closed by peer without receiving 'close_notify'
+               ec = StreamError::StreamTruncated;
+               }
+
             if(!isContinuation)
                {
                // Make sure the handler is not called without an intermediate initiating function.
@@ -293,28 +293,16 @@ class AsyncHandshakeOperation : public AsyncBase<Handler, typename Stream::execu
          {
          reenter(this)
             {
+            if(ec == boost::asio::error::eof)
+               {
+               ec = StreamError::StreamTruncated;
+               }
+
             if(bytesTransferred > 0 && !ec)
                {
                // Provide encrypted TLS data received from the network to TLS::Channel for decryption
                boost::asio::const_buffer read_buffer {m_stream.input_buffer().data(), bytesTransferred};
-               try
-                  {
-                  m_stream.native_handle()->received_data(
-                     static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size()
-                  );
-                  }
-               catch(const TLS_Exception& e)
-                  {
-                  ec = e.type();
-                  }
-               catch(const Botan::Exception& e)
-                  {
-                  ec = e.error_type();
-                  }
-               catch(...)
-                  {
-                  ec = Botan::ErrorType::Unknown;
-                  }
+               m_stream.process_encrypted_data(read_buffer, ec);
                }
 
             if(m_stream.has_data_to_send() && !ec)
@@ -325,11 +313,10 @@ class AsyncHandshakeOperation : public AsyncBase<Handler, typename Stream::execu
                // operation will eventually call `*this` as its own handler, passing the 0 back to this call operator.
                // This is necessary because the check of `bytesTransferred > 0` assumes that `bytesTransferred` bytes
                // were just read and are available in input_buffer for further processing.
-               AsyncWriteOperation<
-               AsyncHandshakeOperation<typename std::decay<Handler>::type, Stream, Allocator>,
-                                       Stream,
-                                       Allocator>
-                                       op{std::move(*this), m_stream, 0};
+               AsyncWriteOperation<AsyncHandshakeOperation<typename std::decay<Handler>::type, Stream, Allocator>,
+                                   Stream,
+                                   Allocator>
+                                   op{std::move(*this), m_stream, 0};
                return;
                }
 
diff --git a/src/lib/tls/asio/asio_error.h b/src/lib/tls/asio/asio_error.h
index 7356ab3d7..2ee5ee9dd 100644
--- a/src/lib/tls/asio/asio_error.h
+++ b/src/lib/tls/asio/asio_error.h
@@ -1,7 +1,7 @@
 /*
 * TLS Stream Errors
-* (C) 2018-2019 Jack Lloyd
-*     2018-2019 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
+* (C) 2018-2020 Jack Lloyd
+*     2018-2020 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -23,6 +23,42 @@
 namespace Botan {
 namespace TLS {
 
+enum StreamError
+   {
+   StreamTruncated = 1
+   };
+
+class StreamCategory : public boost::system::error_category
+   {
+   public:
+      const char* name() const noexcept override
+         {
+         return "asio.ssl.stream";
+         }
+
+      std::string message(int value) const override
+         {
+         switch(value)
+            {
+            case StreamTruncated:
+               return "stream truncated";
+            default:
+               return "asio.botan.tls.stream error";
+            }
+         }
+   };
+
+inline const StreamCategory& botan_stream_category()
+   {
+   static StreamCategory category;
+   return category;
+   }
+
+inline boost::system::error_code make_error_code(Botan::TLS::StreamError e)
+   {
+   return boost::system::error_code(static_cast<int>(e), Botan::TLS::botan_stream_category());
+   }
+
 //! @brief An error category for TLS alerts
 struct BotanAlertCategory : boost::system::error_category
    {
@@ -86,6 +122,11 @@ template<> struct is_error_code_enum<Botan::TLS::Alert::Type>
    static const bool value = true;
    };
 
+template<> struct is_error_code_enum<Botan::TLS::StreamError>
+   {
+   static const bool value = true;
+   };
+
 template<> struct is_error_code_enum<Botan::ErrorType>
    {
    static const bool value = true;
diff --git a/src/lib/tls/asio/asio_stream.h b/src/lib/tls/asio/asio_stream.h
index 37f031571..fbff8db9d 100644
--- a/src/lib/tls/asio/asio_stream.h
+++ b/src/lib/tls/asio/asio_stream.h
@@ -1,7 +1,7 @@
 /*
 * TLS ASIO Stream
-* (C) 2018-2019 Jack Lloyd
-*     2018-2019 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
+* (C) 2018-2020 Jack Lloyd
+*     2018-2020 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -64,7 +64,8 @@ class Stream
       explicit Stream(Context& context, Args&& ... args)
          : m_context(context)
          , m_nextLayer(std::forward<Args>(args)...)
-         , m_core(m_receive_buffer, m_send_buffer, m_context)
+         , m_core(*this)
+         , m_shutdown_received(false)
          , m_input_buffer_space(MAX_CIPHERTEXT_SIZE, '\0')
          , m_input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size())
          {}
@@ -83,7 +84,8 @@ class Stream
       explicit Stream(Arg&& arg, Context& context)
          : m_context(context)
          , m_nextLayer(std::forward<Arg>(arg))
-         , m_core(m_receive_buffer, m_send_buffer, m_context)
+         , m_core(*this)
+         , m_shutdown_received(false)
          , m_input_buffer_space(MAX_CIPHERTEXT_SIZE, '\0')
          , m_input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size())
          {}
@@ -228,22 +230,7 @@ class Stream
             if(ec)
                { return; }
 
-            try
-               {
-               native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size());
-               }
-            catch(const TLS_Exception& e)
-               {
-               ec = e.type();
-               }
-            catch(const Botan::Exception& e)
-               {
-               ec = e.error_type();
-               }
-            catch(const std::exception&)
-               {
-               ec = Botan::ErrorType::Unknown;
-               }
+            process_encrypted_data(read_buffer, ec);
 
             send_pending_encrypted_data(ec);
             }
@@ -300,29 +287,18 @@ class Stream
        * This function is used to shut down SSL on the stream. The function call will block until SSL has been shut down
        * or an error occurs. Note that this will not close the lowest layer.
        *
+       * Note that this can be used in reaction of a received shutdown alert from the peer.
+       *
        * @param ec Set to indicate what error occured, if any.
        */
       void shutdown(boost::system::error_code& ec)
          {
-         try
+         try_with_error_code([&]
             {
             native_handle()->close();
-            }
-         catch(const TLS_Exception& e)
-            {
-            ec = e.type();
-            }
-         catch(const Botan::Exception& e)
-            {
-            ec = e.error_type();
-            }
-         catch(const std::exception&)
-            {
-            ec = Botan::ErrorType::Unknown;
-            }
+            }, ec);
 
-         if(!ec)
-            { send_pending_encrypted_data(ec); }
+         send_pending_encrypted_data(ec);
          }
 
       /**
@@ -331,6 +307,8 @@ class Stream
        * This function is used to shut down SSL on the stream. The function call will block until SSL has been shut down
        * or an error occurs. Note that this will not close the lowest layer.
        *
+       * Note that this can be used in reaction of a received shutdown alert from the peer.
+       *
        * @throws boost::system::system_error if error occured
        */
       void shutdown()
@@ -345,7 +323,9 @@ class Stream
        *
        * This function call always returns immediately.
        *
-       * @param handler The handler to be called when the handshake operation completes.
+       * Note that this can be used in reaction of a received shutdown alert from the peer.
+       *
+       * @param handler The handler to be called when the shutdown operation completes.
        *                The equivalent function signature of the handler must be: void(boost::system::error_code)
        */
       template <typename ShutdownHandler>
@@ -369,7 +349,8 @@ class Stream
        * occurs.
        *
        * @param buffers The buffers into which the data will be read.
-       * @param ec Set to indicate what error occured, if any.
+       * @param ec Set to indicate what error occurred, if any. Specifically, StreamTruncated will be set if the peer
+       *           has closed the connection but did not properly shut down the SSL connection.
        * @return The number of bytes read. Returns 0 if an error occurred.
        */
       template <typename MutableBufferSequence>
@@ -383,21 +364,20 @@ class Stream
          if(ec)
             { return 0; }
 
-         try
-            {
-            native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size());
-            }
-         catch(const TLS_Exception& e)
-            {
-            ec = e.type();
-            }
-         catch(const Botan::Exception& e)
+         process_encrypted_data(read_buffer, ec);
+
+         if(ec)  // something went wrong in process_encrypted_data()
+            { return 0; }
+
+         if(shutdown_received())
             {
-            ec = e.error_type();
+            // we just received a 'close_notify' from the peer and don't expect any more data
+            ec = boost::asio::error::eof;
             }
-         catch(const std::exception&)
+         else if(ec == boost::asio::error::eof)
             {
-            ec = Botan::ErrorType::Unknown;
+            // we did not expect this disconnection from the peer
+            ec = StreamError::StreamTruncated;
             }
 
          return !ec ? copy_received_data(buffers) : 0;
@@ -521,6 +501,12 @@ class Stream
 
       //! @}
 
+      //! @brief Indicates whether a close_notify alert has been received from the peer.
+      bool shutdown_received() const
+         {
+         return m_shutdown_received;
+         }
+
    protected:
       template <class H, class S, class M, class A> friend class detail::AsyncReadOperation;
       template <class H, class S, class A> friend class detail::AsyncWriteOperation;
@@ -538,28 +524,32 @@ class Stream
       class StreamCore : public Botan::TLS::Callbacks
          {
          public:
-            StreamCore(boost::beast::flat_buffer& receive_buffer, boost::beast::flat_buffer& send_buffer, Context& context)
-               : m_receive_buffer(receive_buffer), m_send_buffer(send_buffer), m_tls_context(context) {}
+            StreamCore(Stream& stream) : m_stream(stream) {}
 
             virtual ~StreamCore() = default;
 
             void tls_emit_data(const uint8_t data[], std::size_t size) override
                {
-               m_send_buffer.commit(
-                  boost::asio::buffer_copy(m_send_buffer.prepare(size), boost::asio::buffer(data, size))
+               m_stream.m_send_buffer.commit(
+                  boost::asio::buffer_copy(m_stream.m_send_buffer.prepare(size), boost::asio::buffer(data, size))
                );
                }
 
             void tls_record_received(uint64_t, const uint8_t data[], std::size_t size) override
                {
-               m_receive_buffer.commit(
-                  boost::asio::buffer_copy(m_receive_buffer.prepare(size), boost::asio::const_buffer(data, size))
+               m_stream.m_receive_buffer.commit(
+                  boost::asio::buffer_copy(m_stream.m_receive_buffer.prepare(size), boost::asio::const_buffer(data, size))
                );
                }
 
             void tls_alert(Botan::TLS::Alert alert) override
                {
-               BOTAN_UNUSED(alert);
+               if(alert.type() == Botan::TLS::Alert::CLOSE_NOTIFY)
+                  {
+                  m_stream.set_shutdown_received();
+                  // Channel::process_alert will automatically write the corresponding close_notify response to the
+                  // send_buffer and close the native_handle after this function returns.
+                  }
                }
 
             std::chrono::milliseconds tls_verify_cert_chain_ocsp_timeout() const override
@@ -581,9 +571,9 @@ class Stream
                const std::string& hostname,
                const TLS::Policy& policy) override
                {
-               if(m_tls_context.has_verify_callback())
+               if(m_stream.m_context.has_verify_callback())
                   {
-                  m_tls_context.get_verify_callback()(cert_chain, ocsp_responses, trusted_roots, usage, hostname, policy);
+                  m_stream.m_context.get_verify_callback()(cert_chain, ocsp_responses, trusted_roots, usage, hostname, policy);
                   }
                else
                   {
@@ -591,9 +581,8 @@ class Stream
                   }
                }
 
-            boost::beast::flat_buffer& m_receive_buffer;
-            boost::beast::flat_buffer& m_send_buffer;
-            Context& m_tls_context;
+         private:
+            Stream& m_stream;
          };
 
       const boost::asio::mutable_buffer& input_buffer() { return m_input_buffer; }
@@ -660,6 +649,14 @@ class Stream
             }
          }
 
+      /** @brief Synchronously write encrypted data from the send buffer to the next layer.
+       *
+       * If this function is called with an error code other than 'Success', it will do nothing and return 0.
+       *
+       * @param ec Set to indicate what error occurred, if any. Specifically, StreamTruncated will be set if the peer
+       *           has closed the connection but did not properly shut down the SSL connection.
+       * @return The number of bytes written.
+       */
       size_t send_pending_encrypted_data(boost::system::error_code& ec)
          {
          if(ec)
@@ -667,9 +664,21 @@ class Stream
 
          auto writtenBytes = boost::asio::write(m_nextLayer, send_buffer(), ec);
          consume_send_buffer(writtenBytes);
+
+         if(ec == boost::asio::error::eof && !shutdown_received())
+            {
+            // transport layer was closed by peer without receiving 'close_notify'
+            ec = StreamError::StreamTruncated;
+            }
+
          return writtenBytes;
          }
 
+      /**
+       * @brief Pass plaintext data to the native handle for processing.
+       *
+       * The native handle will then create TLS records and hand them back to the Stream via the tls_emit_data callback.
+       */
       template <typename ConstBufferSequence>
       void tls_encrypt(const ConstBufferSequence& buffers, boost::system::error_code& ec)
          {
@@ -681,25 +690,56 @@ class Stream
                it++)
             {
             const boost::asio::const_buffer buffer = *it;
-            try
+            try_with_error_code([&]
                {
                native_handle()->send(static_cast<const uint8_t*>(buffer.data()), buffer.size());
-               }
-            catch(const TLS_Exception& e)
-               {
-               ec = e.type();
-               }
-            catch(const Botan::Exception& e)
-               {
-               ec = e.error_type();
-               }
-            catch(const std::exception&)
-               {
-               ec = Botan::ErrorType::Unknown;
-               }
+               }, ec);
             }
          }
 
+      /**
+       * @brief Pass encrypted data to the native handle for processing.
+       *
+       * If an exception occurs while processing the data, an error code will be set.
+       *
+       * @param read_buffer Input buffer containing the encrypted data.
+       * @param ec Set to indicate what error occurred, if any.
+       */
+      void process_encrypted_data(const boost::asio::const_buffer& read_buffer, boost::system::error_code& ec)
+         {
+         try_with_error_code([&]
+            {
+            native_handle()->received_data(static_cast<const uint8_t*>(read_buffer.data()), read_buffer.size());
+            }, ec);
+         }
+
+      //! @brief Catch exceptions and set an error_code
+      template <typename Fun>
+      void try_with_error_code(Fun f, boost::system::error_code& ec)
+         {
+         try
+            {
+            f();
+            }
+         catch(const TLS_Exception& e)
+            {
+            ec = e.type();
+            }
+         catch(const Botan::Exception& e)
+            {
+            ec = e.error_type();
+            }
+         catch(const std::exception&)
+            {
+            ec = Botan::ErrorType::Unknown;
+            }
+         }
+
+      void set_shutdown_received()
+         {
+         m_shutdown_received = true;
+         }
+
       Context&                  m_context;
       StreamLayer               m_nextLayer;
 
@@ -709,6 +749,8 @@ class Stream
       StreamCore                m_core;
       std::unique_ptr<ChannelT> m_native_handle;
 
+      bool m_shutdown_received;
+
       // Buffer space used to read input intended for the core
       std::vector<uint8_t>              m_input_buffer_space;
       const boost::asio::mutable_buffer m_input_buffer;
diff --git a/src/tests/unit_asio_stream.cpp b/src/tests/unit_asio_stream.cpp
index aa65b145f..8691cb211 100644
--- a/src/tests/unit_asio_stream.cpp
+++ b/src/tests/unit_asio_stream.cpp
@@ -1,7 +1,7 @@
 /*
 * TLS ASIO Stream Unit Tests
-* (C) 2018-2019 Jack Lloyd
-*     2018-2019 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
+* (C) 2018-2020 Jack Lloyd
+*     2018-2020 Hannes Rantzsch, Tim Oesterreich, Rene Meusel
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -177,7 +177,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          TestStream remote{ioc};
 
          auto ctx = get_context();
@@ -192,7 +192,7 @@ class Asio_Stream_Tests final : public Test
 
          Test::Result result("sync TLS handshake error");
          result.test_eq("does not activate channel", ssl.native_handle()->is_active(), false);
-         result.confirm("propagates error code", ec == net::error::eof);
+         result.confirm("propagates error code", ec == net::error::no_recovery);
          results.push_back(result);
          }
 
@@ -246,7 +246,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          TestStream remote{ioc};
 
          auto ctx = get_context();
@@ -261,7 +261,7 @@ class Asio_Stream_Tests final : public Test
          auto handler = [&](const error_code &ec)
             {
             result.test_eq("does not activate channel", ssl.native_handle()->is_active(), false);
-            result.confirm("propagates error code", ec == net::error::eof);
+            result.confirm("propagates error code", ec == net::error::no_recovery);
             };
 
          ssl.async_handshake(Botan::TLS::CLIENT, handler);
@@ -345,7 +345,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          TestStream remote{ioc};
 
          auto ctx = get_context();
@@ -359,7 +359,7 @@ class Asio_Stream_Tests final : public Test
 
          Test::Result result("sync read_some error");
          result.test_eq("didn't transfer anything", bytes_transferred, 0);
-         result.confirm("propagates error code", ec == net::error::eof);
+         result.confirm("propagates error code", ec == net::error::no_recovery);
 
          results.push_back(result);
          }
@@ -467,7 +467,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          uint8_t    data[TEST_DATA_SIZE];
@@ -477,7 +477,7 @@ class Asio_Stream_Tests final : public Test
          auto read_handler = [&](const error_code &ec, std::size_t bytes_transferred)
             {
             result.test_eq("didn't transfer anything", bytes_transferred, 0);
-            result.confirm("propagates error code", ec == net::error::eof);
+            result.confirm("propagates error code", ec == net::error::no_recovery);
             };
 
          net::mutable_buffer buf {data, TEST_DATA_SIZE};
@@ -618,7 +618,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          TestStream remote{ioc};
 
          auto ctx = get_context();
@@ -631,7 +631,7 @@ class Asio_Stream_Tests final : public Test
 
          Test::Result result("sync write_some error");
          result.test_eq("didn't transfer anything", bytes_transferred, 0);
-         result.confirm("propagates error code", ec == net::error::eof);
+         result.confirm("propagates error code", ec == net::error::no_recovery);
 
          results.push_back(result);
          }
@@ -724,7 +724,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
          // fail right away
-         FailCount  fc{0, net::error::eof};
+         FailCount  fc{0, net::error::no_recovery};
          TestStream remote{ioc};
 
          auto ctx = get_context();
@@ -736,7 +736,7 @@ class Asio_Stream_Tests final : public Test
          auto write_handler = [&](const error_code &ec, std::size_t bytes_transferred)
             {
             result.test_eq("committed some bytes to the core", bytes_transferred, TEST_DATA_SIZE);
-            result.confirm("propagates error code", ec == net::error::eof);
+            result.confirm("propagates error code", ec == net::error::no_recovery);
             };
 
          net::async_write(ssl, net::const_buffer(TEST_DATA, TEST_DATA_SIZE), write_handler);