Changes to TLS::Callbacks for GH PR #457

Make TLS::Channel::m_callbacks a reference, so deriving from TLS::Callbacks works

Split out the compat (std::function) based interface to Compat_Callbacks.
This avoids the overhead of empty std::functions when using the virtual
interface, and ensures the virtual interface works since there is no
callback path that does not involve a vtable lookup.

Rename the TLS::Callback functions. Since the idea is that often an owning
class will pass *this as the callbacks argument, it is good to namespace
the virtual functions so as not to conflict with other names chosen by
the class. Specifically, prefixes all cb functions with tls_

Revert changes to use the old style alert callback (with no longer used data/len
params) so no API changes are required for old code. The new Callbacks interface
continues to just receive the alert code itself.

Switch to virtual function interface in CLI tls_client for testing.

Inline tls_server_handshake_state.h - only used in tls_server.cpp

Fix tests - test looked like it was creating a new client object but it
was not actually being used. And when enabled, it failed because the queues
were not being emptied in between. So, fix that.

14 files changed, 221 insertions, 151 deletions

diff --git a/src/lib/tls/info.txt b/src/lib/tls/info.txt
index b62adc17f..208c4ebb2 100644
--- a/src/lib/tls/info.txt
+++ b/src/lib/tls/info.txt
@@ -30,7 +30,6 @@ tls_messages.h
 tls_reader.h
 tls_record.h
 tls_seq_numbers.h
-tls_server_handshake_state.h
 tls_session_key.h
 </header:internal>
 
diff --git a/src/lib/tls/tls_blocking.cpp b/src/lib/tls/tls_blocking.cpp
index 3910d242c..34ebc6c91 100644
--- a/src/lib/tls/tls_blocking.cpp
+++ b/src/lib/tls/tls_blocking.cpp
@@ -24,12 +24,13 @@ Blocking_Client::Blocking_Client(read_fn reader,
                                  const Protocol_Version& offer_version,
                                  const std::vector<std::string>& next) :
    m_read(reader),
-   m_channel(TLS::Callbacks(
+   m_callbacks(new TLS::Compat_Callbacks(
                writer,
                std::bind(&Blocking_Client::data_cb, this, _1, _2),
-               std::bind(&Blocking_Client::alert_cb, this, _1),
+               std::function<void (Alert)>(std::bind(&Blocking_Client::alert_cb, this, _1)),
                std::bind(&Blocking_Client::handshake_cb, this, _1)
-             ),
+             )),
+   m_channel(*m_callbacks.get(),
              session_manager,
              creds,
              policy,
@@ -38,6 +39,7 @@ Blocking_Client::Blocking_Client(read_fn reader,
              offer_version,
              next)
    {
+   printf("hi\n");
    }
 
 bool Blocking_Client::handshake_cb(const Session& session)
diff --git a/src/lib/tls/tls_blocking.h b/src/lib/tls/tls_blocking.h
index cba44b524..0f2986710 100644
--- a/src/lib/tls/tls_blocking.h
+++ b/src/lib/tls/tls_blocking.h
@@ -33,6 +33,7 @@ class BOTAN_DLL Blocking_Client
       typedef std::function<size_t (byte[], size_t)> read_fn;
       typedef std::function<void (const byte[], size_t)> write_fn;
 
+      BOTAN_DEPRECATED("Use the regular TLS::Client interface")
       Blocking_Client(read_fn reader,
                       write_fn writer,
                       Session_Manager& session_manager,
@@ -93,6 +94,7 @@ class BOTAN_DLL Blocking_Client
       void alert_cb(const Alert& alert);
 
       read_fn m_read;
+      std::unique_ptr<Compat_Callbacks> m_callbacks;
       TLS::Client m_channel;
       secure_vector<byte> m_plaintext;
    };
diff --git a/src/lib/tls/tls_callbacks.h b/src/lib/tls/tls_callbacks.h
index 216c58ce2..db1b70693 100644
--- a/src/lib/tls/tls_callbacks.h
+++ b/src/lib/tls/tls_callbacks.h
@@ -1,6 +1,7 @@
 /*
 * TLS Callbacks
 * (C) 2016 Matthias Gierlings
+*     2016 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -14,33 +15,76 @@ namespace Botan {
 
 namespace TLS {
 
-class Handshake_State;
 class Handshake_Message;
 
 /**
-* Virtual Interface for TLS-Channel related callback handling. The default
-* implementations involving std::function are only provided for compatibility
-* purposes. New implementations should override the virtual member methods
-* out_fn(), app_data(), alert(), handshake() and handshake_msg() instead.
-*
+* Encapsulates the callbacks that a TLS channel will make which are due to
+* channel specific operations.
 */
 class BOTAN_DLL Callbacks
    {
    public:
+       virtual ~Callbacks() {}
+
+       /**
+       * Mandatory callback: output function
+       * The channel will call this with data which needs to be sent to the peer
+       * (eg, over a socket or some other form of IPC). The array will be overwritten
+       * when the function returns so a copy must be made if the data cannot be
+       * sent immediately.
+       */
+       virtual void tls_emit_data(const uint8_t data[], size_t size) = 0;
+
+       /**
+       * Mandatory callback: process application data
+       * Called when application data record is received from the peer.
+       * Again the array is overwritten immediately after the function returns.
+       * seq_no is the underlying TLS/DTLS record sequence number.
+       */
+       virtual void tls_record_received(u64bit seq_no, const uint8_t data[], size_t size) = 0;
+     
+       /**
+       * Mandary callback: alert received
+       * Called when an alert is received from the peer
+       * If fatal, the connection is closing. If not fatal, the connection may
+       * still be closing (depending on the error and the peer).
+       */ 
+       virtual void tls_alert(Alert alert) = 0;
+
+       /**
+       * Mandatory callback: session established
+       * Called when a session is established. Throw an exception to abort
+       * the connection. Return false to prevent the session from being cached.
+       * Return true to cache the session in the configured session manager.
+       */ 
+       virtual bool tls_session_established(const Session& session) = 0;
+
+       /**
+       * Optional callback: inspect handshake message
+       */       
+       virtual void tls_inspect_handshake_msg(const Handshake_Message&) {}
+
+       /**
+       * Optional callback: debug logging. (not currently used)
+       */       
+       virtual bool tls_log_debug(const char*) { return false; }
+   };
+
+/**
+* TLS::Callbacks using std::function for compatability with the old API signatures.
+* This type is only provided for backward compatibility.
+* New implementations should derive from TLS::Callbacks instead.
+*/
+class BOTAN_DLL Compat_Callbacks final : public Callbacks
+   {
+   public:
       typedef std::function<void (const byte[], size_t)> output_fn;
       typedef std::function<void (const byte[], size_t)> data_cb;
-      typedef std::function<void (Alert)> alert_cb;
+      typedef std::function<void (Alert, const byte[], size_t)> alert_cb;
       typedef std::function<bool (const Session&)> handshake_cb;
       typedef std::function<void (const Handshake_Message&)> handshake_msg_cb;
 
       /**
-       * DEPRECATED: This constructor is only provided for backward
-       * compatibility. New implementations should override the
-       * virtual member methods out_fn(), app_data(), alert(),
-       * handshake() and handshake_msg() and use the default constructor
-       * Callbacks().
-       *
-       * Encapsulates a set of callback functions required by a TLS Channel.
        * @param output_fn is called with data for the outbound socket
        *
        * @param app_data_cb is called when new application data is received
@@ -49,48 +93,50 @@ class BOTAN_DLL Callbacks
        *
        * @param handshake_cb is called when a handshake is completed
        */
-       BOTAN_DEPRECATED("Use TLS::Callbacks() (virtual interface).")
-       Callbacks(output_fn out, data_cb app_data_cb, alert_cb alert_cb,
-                 handshake_cb hs_cb, handshake_msg_cb hs_msg_cb = nullptr)
+       BOTAN_DEPRECATED("Use TLS::Callbacks (virtual interface).")
+       Compat_Callbacks(output_fn out, data_cb app_data_cb, alert_cb alert_cb,
+                        handshake_cb hs_cb, handshake_msg_cb hs_msg_cb = nullptr)
           : m_output_function(out), m_app_data_cb(app_data_cb),
-            m_alert_cb(alert_cb), m_hs_cb(hs_cb), m_hs_msg_cb(hs_msg_cb) {}
+            m_alert_cb(std::bind(alert_cb, std::placeholders::_1, nullptr, 0)),
+            m_hs_cb(hs_cb), m_hs_msg_cb(hs_msg_cb) {}
 
-       Callbacks()
-          : m_output_function(nullptr), m_app_data_cb(nullptr),
-            m_alert_cb(nullptr), m_hs_cb(nullptr), m_hs_msg_cb(nullptr) {}
-
-
-       virtual ~Callbacks() {}
+       BOTAN_DEPRECATED("Use TLS::Callbacks (virtual interface).")
+       Compat_Callbacks(output_fn out, data_cb app_data_cb,
+                        std::function<void (Alert)> alert_cb,
+                        handshake_cb hs_cb, handshake_msg_cb hs_msg_cb = nullptr)
+          : m_output_function(out), m_app_data_cb(app_data_cb),
+            m_alert_cb(alert_cb),
+            m_hs_cb(hs_cb), m_hs_msg_cb(hs_msg_cb) {}
 
-       virtual void out_fn(const byte data[], size_t size) const
+       void tls_emit_data(const byte data[], size_t size) override
           {
           BOTAN_ASSERT(m_output_function != nullptr,
                        "Invalid TLS output function callback.");
           m_output_function(data, size);
           }
 
-       virtual void app_data(const byte data[], size_t size) const
+       void tls_record_received(u64bit /*seq_no*/, const byte data[], size_t size) override
           {
           BOTAN_ASSERT(m_app_data_cb != nullptr,
                        "Invalid TLS app data callback.");
           m_app_data_cb(data, size);
           }
 
-       virtual void alert(Alert alert) const
+       void tls_alert(Alert alert) override
           {
           BOTAN_ASSERT(m_alert_cb != nullptr,
                        "Invalid TLS alert callback.");
           m_alert_cb(alert);
           }
 
-       virtual bool handshake(const Session& session) const
+       bool tls_session_established(const Session& session) override
           {
           BOTAN_ASSERT(m_hs_cb != nullptr,
                        "Invalid TLS handshake callback.");
           return m_hs_cb(session);
           }
 
-       virtual void handshake_msg(const Handshake_Message& hmsg) const
+       void tls_inspect_handshake_msg(const Handshake_Message& hmsg) override
           {
           // The handshake message callback is optional so we can
           // not assume it has been set.
@@ -100,7 +146,7 @@ class BOTAN_DLL Callbacks
     private:
          const output_fn m_output_function;
          const data_cb m_app_data_cb;
-         const alert_cb m_alert_cb;
+         const std::function<void (Alert)> m_alert_cb;
          const handshake_cb m_hs_cb;
          const handshake_msg_cb m_hs_msg_cb;
    };
diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index b78796536..5f882af64 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,2015 Jack Lloyd
+* (C) 2011,2012,2014,2015,2016 Jack Lloyd
 *     2016 Matthias Gierlings
 *
 * Botan is released under the Simplified BSD License (see license.txt)
@@ -21,7 +21,7 @@ namespace TLS {
 
 size_t TLS::Channel::IO_BUF_DEFAULT_SIZE = 10*1024;
 
-Channel::Channel(const Callbacks& callbacks,
+Channel::Channel(Callbacks& callbacks,
                  Session_Manager& session_manager,
                  RandomNumberGenerator& rng,
                  const Policy& policy,
@@ -47,7 +47,8 @@ Channel::Channel(output_fn out,
                  bool is_datagram,
                  size_t io_buf_sz) :
     m_is_datagram(is_datagram),
-    m_callbacks(Callbacks(out, app_data_cb, alert_cb, hs_cb, hs_msg_cb)),
+    m_compat_callbacks(new Compat_Callbacks(out, app_data_cb, alert_cb, hs_cb, hs_msg_cb)),
+    m_callbacks(*m_compat_callbacks.get()),
     m_session_manager(session_manager),
     m_policy(policy),
     m_rng(rng)
@@ -319,15 +320,15 @@ size_t Channel::received_data(const byte input[], size_t input_size)
 
          if(record_type == HANDSHAKE || record_type == CHANGE_CIPHER_SPEC)
             {
-               process_handshake_ccs(record_data, record_sequence, record_type, record_version);
+            process_handshake_ccs(record_data, record_sequence, record_type, record_version);
             }
          else if(record_type == APPLICATION_DATA)
             {
-               process_application_data(record_data);
+            process_application_data(record_sequence, record_data);
             }
          else if(record_type == ALERT)
             {
-                process_alert(record_data);
+            process_alert(record_data);
             }
          else if(record_type != NO_RECORD)
             throw Unexpected_Message("Unexpected record type " +
@@ -359,10 +360,10 @@ size_t Channel::received_data(const byte input[], size_t input_size)
       }
    }
 
-void Channel::process_handshake_ccs(secure_vector<byte>& record,
-                                    u64bit& record_sequence,
-                                    Record_Type& record_type,
-                                    Protocol_Version& record_version)
+void Channel::process_handshake_ccs(const secure_vector<byte>& record,
+                                    u64bit record_sequence,
+                                    Record_Type record_type,
+                                    Protocol_Version record_version)
    {
    if(!m_pending_state)
       {
@@ -422,7 +423,7 @@ void Channel::process_handshake_ccs(secure_vector<byte>& record,
       }
    }
 
-void Channel::process_application_data(secure_vector<byte>& record)
+void Channel::process_application_data(u64bit seq_no, const secure_vector<byte>& record)
    {
    if(!active_state())
       throw Unexpected_Message("Application data before handshake done");
@@ -433,17 +434,17 @@ void Channel::process_application_data(secure_vector<byte>& record)
    * following record. Avoid spurious callbacks.
    */
    if(record.size() > 0)
-      m_callbacks.app_data(record.data(), record.size());
+      callbacks().tls_record_received(seq_no, record.data(), record.size());
    }
 
-void Channel::process_alert(secure_vector<byte>& record)
+void Channel::process_alert(const secure_vector<byte>& record)
     {
     Alert alert_msg(record);
 
     if(alert_msg.type() == Alert::NO_RENEGOTIATION)
        m_pending_state.reset();
 
-    m_callbacks.alert(alert_msg);
+    callbacks().tls_alert(alert_msg);
 
     if(alert_msg.is_fatal())
        {
@@ -478,7 +479,7 @@ void Channel::write_record(Connection_Cipher_State* cipher_state, u16bit epoch,
                      cipher_state,
                      m_rng);
 
-   m_callbacks.out_fn(m_writebuf.data(), m_writebuf.size());
+   callbacks().tls_emit_data(m_writebuf.data(), m_writebuf.size());
    }
 
 void Channel::send_record_array(u16bit epoch, byte type, const byte input[], size_t length)
diff --git a/src/lib/tls/tls_channel.h b/src/lib/tls/tls_channel.h
index 7c59e1d6f..073af760f 100644
--- a/src/lib/tls/tls_channel.h
+++ b/src/lib/tls/tls_channel.h
@@ -36,12 +36,12 @@ class BOTAN_DLL Channel
    public:
       typedef std::function<void (const byte[], size_t)> output_fn;
       typedef std::function<void (const byte[], size_t)> data_cb;
-      typedef std::function<void (Alert)> alert_cb;
+      typedef std::function<void (Alert, const byte[], size_t)> alert_cb;
       typedef std::function<bool (const Session&)> handshake_cb;
       typedef std::function<void (const Handshake_Message&)> handshake_msg_cb;
       static size_t IO_BUF_DEFAULT_SIZE;
 
-      Channel(const Callbacks& callbacks,
+      Channel(Callbacks& callbacks,
               Session_Manager& session_manager,
               RandomNumberGenerator& rng,
               const Policy& policy,
@@ -215,9 +215,9 @@ class BOTAN_DLL Channel
 
       const Policy& policy() const { return m_policy; }
 
-      bool save_session(const Session& session) const { return m_callbacks.handshake(session); }
+      bool save_session(const Session& session) const { return callbacks().tls_session_established(session); }
 
-      Callbacks get_callbacks() const { return m_callbacks; }
+      Callbacks& callbacks() const { return m_callbacks; }
    private:
       void init(size_t io_buf_sze);
 
@@ -245,19 +245,20 @@ class BOTAN_DLL Channel
       const Handshake_State* pending_state() const { return m_pending_state.get(); }
 
       /* methods to handle incoming traffic through Channel::receive_data. */
-      void process_handshake_ccs(secure_vector<byte>& record,
-                                 u64bit& record_sequence,
-                                 Record_Type& record_type,
-                                 Protocol_Version& record_version);
+      void process_handshake_ccs(const secure_vector<byte>& record,
+                                 u64bit record_sequence,
+                                 Record_Type record_type,
+                                 Protocol_Version record_version);
 
-      void process_application_data(secure_vector<byte>& record);
+      void process_application_data(u64bit req_no, const secure_vector<byte>& record);
 
-      void process_alert(secure_vector<byte>& record);
+      void process_alert(const secure_vector<byte>& record);
 
       bool m_is_datagram;
 
       /* callbacks */
-      Callbacks m_callbacks;
+      std::unique_ptr<Compat_Callbacks> m_compat_callbacks;
+      Callbacks& m_callbacks;
 
       /* external state */
       Session_Manager& m_session_manager;
diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp
index ab7beddbd..f77521c03 100644
--- a/src/lib/tls/tls_client.cpp
+++ b/src/lib/tls/tls_client.cpp
@@ -24,7 +24,7 @@ class Client_Handshake_State : public Handshake_State
    public:
       // using Handshake_State::Handshake_State;
 
-      Client_Handshake_State(Handshake_IO* io, handshake_msg_cb cb) : Handshake_State(io, cb) {}
+      Client_Handshake_State(Handshake_IO* io, Callbacks& cb) : Handshake_State(io, cb) {}
 
       const Public_Key& get_server_public_Key() const
          {
@@ -43,7 +43,7 @@ class Client_Handshake_State : public Handshake_State
 /*
 * TLS Client Constructor
 */
-Client::Client(const Callbacks& callbacks,
+Client::Client(Callbacks& callbacks,
                Session_Manager& session_manager,
                Credentials_Manager& creds,
                const Policy& policy,
@@ -112,10 +112,7 @@ void Client::init(const Protocol_Version& protocol_version,
 
 Handshake_State* Client::new_handshake_state(Handshake_IO* io)
    {
-   return new Client_Handshake_State(io,
-                                     std::bind(&TLS::Callbacks::handshake_msg,
-                                               get_callbacks(),
-                                               std::placeholders::_1));
+   return new Client_Handshake_State(io, callbacks());
    }
 
 std::vector<X509_Certificate>
diff --git a/src/lib/tls/tls_client.h b/src/lib/tls/tls_client.h
index 6452294cd..09af053af 100644
--- a/src/lib/tls/tls_client.h
+++ b/src/lib/tls/tls_client.h
@@ -23,6 +23,7 @@ namespace TLS {
 class BOTAN_DLL Client final : public Channel
    {
    public:
+
       /**
       * Set up a new TLS client session
       *
@@ -48,7 +49,51 @@ class BOTAN_DLL Client final : public Channel
       *        be preallocated for the read and write buffers. Smaller
       *        values just mean reallocations and copies are more likely.
       */
+     Client(Callbacks& callbacks,
+            Session_Manager& session_manager,
+            Credentials_Manager& creds,
+            const Policy& policy,
+            RandomNumberGenerator& rng,
+            const Server_Information& server_info = Server_Information(),
+            const Protocol_Version& offer_version = Protocol_Version::latest_tls_version(),
+            const std::vector<std::string>& next_protocols = {},
+            size_t reserved_io_buffer_size = TLS::Client::IO_BUF_DEFAULT_SIZE
+         );
 
+      /**
+      * DEPRECATED. This constructor is only provided for backward
+      * compatibility and should not be used in new code.
+      * 
+      * Set up a new TLS client session
+      *
+      * @param output_fn is called with data for the outbound socket
+      *
+      * @param app_data_cb is called when new application data is received
+      *
+      * @param alert_cb is called when a TLS alert is received
+      *
+      * @param handshake_cb is called when a handshake is completed
+      *
+      * @param session_manager manages session state
+      *
+      * @param creds manages application/user credentials
+      *
+      * @param policy specifies other connection policy information
+      *
+      * @param rng a random number generator
+      *
+      * @param server_info is identifying information about the TLS server
+      *
+      * @param offer_version specifies which version we will offer
+      *        to the TLS server.
+      *
+      * @param next_protocols specifies protocols to advertise with ALPN
+      *
+      * @param reserved_io_buffer_size This many bytes of memory will
+      *        be preallocated for the read and write buffers. Smaller
+      *        values just mean reallocations and copies are more likely.
+      */
+      BOTAN_DEPRECATED("Use TLS::Client(TLS::Callbacks ...)")
       Client(output_fn out,
              data_cb app_data_cb,
              alert_cb alert_cb,
@@ -82,18 +127,6 @@ class BOTAN_DLL Client final : public Channel
              const std::vector<std::string>& next_protocols = {}
          );
 
-
-     Client(const Callbacks& callbacks,
-            Session_Manager& session_manager,
-            Credentials_Manager& creds,
-            const Policy& policy,
-            RandomNumberGenerator& rng,
-            const Server_Information& server_info = Server_Information(),
-            const Protocol_Version& offer_version = Protocol_Version::latest_tls_version(),
-            const std::vector<std::string>& next_protocols = {},
-            size_t reserved_io_buffer_size = TLS::Client::IO_BUF_DEFAULT_SIZE
-         );
-
       const std::string& application_protocol() const { return m_application_protocol; }
    private:
       void init(const Protocol_Version& protocol_version,
diff --git a/src/lib/tls/tls_handshake_state.cpp b/src/lib/tls/tls_handshake_state.cpp
index afc32ba87..71cacdabd 100644
--- a/src/lib/tls/tls_handshake_state.cpp
+++ b/src/lib/tls/tls_handshake_state.cpp
@@ -8,6 +8,7 @@
 #include <botan/internal/tls_handshake_state.h>
 #include <botan/internal/tls_messages.h>
 #include <botan/internal/tls_record.h>
+#include <botan/tls_callbacks.h>
 
 namespace Botan {
 
@@ -174,8 +175,8 @@ std::string handshake_mask_to_string(u32bit mask)
 /*
 * Initialize the SSL/TLS Handshake State
 */
-Handshake_State::Handshake_State(Handshake_IO* io, handshake_msg_cb cb) :
-   m_msg_callback(cb),
+Handshake_State::Handshake_State(Handshake_IO* io, Callbacks& cb) :
+   m_callbacks(cb),
    m_handshake_io(io),
    m_version(m_handshake_io->initial_record_version())
    {
@@ -183,6 +184,11 @@ Handshake_State::Handshake_State(Handshake_IO* io, handshake_msg_cb cb) :
 
 Handshake_State::~Handshake_State() {}
 
+void Handshake_State::note_message(const Handshake_Message& msg)
+   {
+   m_callbacks.tls_inspect_handshake_msg(msg);
+   }
+
 void Handshake_State::hello_verify_request(const Hello_Verify_Request& hello_verify)
    {
    note_message(hello_verify);
diff --git a/src/lib/tls/tls_handshake_state.h b/src/lib/tls/tls_handshake_state.h
index 2943a8637..bdec10d14 100644
--- a/src/lib/tls/tls_handshake_state.h
+++ b/src/lib/tls/tls_handshake_state.h
@@ -24,6 +24,7 @@ class KDF;
 
 namespace TLS {
 
+class Callbacks;
 class Policy;
 
 class Hello_Verify_Request;
@@ -45,9 +46,7 @@ class Finished;
 class Handshake_State
    {
    public:
-      typedef std::function<void (const Handshake_Message&)> handshake_msg_cb;
-
-      Handshake_State(Handshake_IO* io, handshake_msg_cb cb);
+      Handshake_State(Handshake_IO* io, Callbacks& callbacks);
 
       virtual ~Handshake_State();
 
@@ -164,15 +163,10 @@ class Handshake_State
 
       const Handshake_Hash& hash() const { return m_handshake_hash; }
 
-      void note_message(const Handshake_Message& msg)
-         {
-         if(m_msg_callback)
-            m_msg_callback(msg);
-         }
-
+      void note_message(const Handshake_Message& msg);
    private:
 
-      handshake_msg_cb m_msg_callback;
+      Callbacks& m_callbacks;
 
       std::unique_ptr<Handshake_IO> m_handshake_io;
 
diff --git a/src/lib/tls/tls_record.h b/src/lib/tls/tls_record.h
index 53e447af6..c16b919b5 100644
--- a/src/lib/tls/tls_record.h
+++ b/src/lib/tls/tls_record.h
@@ -102,10 +102,6 @@ class Record
            m_type(type), m_size(data.size()) {};
 
       secure_vector<byte>& get_data() { return m_data; }
-      void set_data(secure_vector<byte> data)
-         {
-         m_data.swap(data);
-         }
 
       Protocol_Version* get_protocol_version() { return m_protocol_version; }
 
diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp
index 96af3cc90..5eccec2a7 100644
--- a/src/lib/tls/tls_server.cpp
+++ b/src/lib/tls/tls_server.cpp
@@ -8,7 +8,6 @@
 
 #include <botan/tls_server.h>
 #include <botan/internal/tls_handshake_state.h>
-#include <botan/internal/tls_server_handshake_state.h>
 #include <botan/internal/tls_messages.h>
 #include <botan/internal/stl_util.h>
 #include <botan/tls_magic.h>
@@ -17,9 +16,34 @@ namespace Botan {
 
 namespace TLS {
 
-namespace {
+class Server_Handshake_State : public Handshake_State
+   {
+   public:
+      Server_Handshake_State(Handshake_IO* io, Callbacks& cb)
+         : Handshake_State(io, cb) {}
+
+      Private_Key* server_rsa_kex_key() { return m_server_rsa_kex_key; }
+      void set_server_rsa_kex_key(Private_Key* key)
+         { m_server_rsa_kex_key = key; }
+
+      bool allow_session_resumption() const
+         { return m_allow_session_resumption; }
+      void set_allow_session_resumption(bool allow_session_resumption)
+         { m_allow_session_resumption = allow_session_resumption; }
+
 
+   private:
+      // Used by the server only, in case of RSA key exchange. Not owned
+      Private_Key* m_server_rsa_kex_key = nullptr;
 
+      /*
+      * Used by the server to know if resumption should be allowed on
+      * a server-initiated renegotiation
+      */
+      bool m_allow_session_resumption = true;
+   };
+
+namespace {
 
 bool check_for_resume(Session& session_info,
                       Session_Manager& session_manager,
@@ -213,7 +237,7 @@ get_server_certs(const std::string& hostname,
 /*
 * TLS Server Constructor
 */
-Server::Server(const Callbacks& callbacks,
+Server::Server(Callbacks& callbacks,
                Session_Manager& session_manager,
                Credentials_Manager& creds,
                const Policy& policy,
@@ -268,11 +292,7 @@ Server::Server(output_fn output,
 
 Handshake_State* Server::new_handshake_state(Handshake_IO* io)
    {
-   std::unique_ptr<Handshake_State> state(
-      new Server_Handshake_State(io,
-                                 std::bind(&TLS::Callbacks::handshake_msg,
-                                           get_callbacks(),
-                                           std::placeholders::_1)));
+   std::unique_ptr<Handshake_State> state(new Server_Handshake_State(io, callbacks()));
 
    state->set_expected_next(CLIENT_HELLO);
    return state.release();
diff --git a/src/lib/tls/tls_server.h b/src/lib/tls/tls_server.h
index c0960a66e..508dde440 100644
--- a/src/lib/tls/tls_server.h
+++ b/src/lib/tls/tls_server.h
@@ -30,8 +30,29 @@ class BOTAN_DLL Server final : public Channel
 
       /**
       * Server initialization
+      *
+      * @param callbacks contains a set of callback function references
+      *        required by the TLS client.
+      *
+      * @param session_manager manages session state
+      *
+      * @param creds manages application/user credentials
+      *
+      * @param policy specifies other connection policy information
+      *
+      * @param rng a random number generator
+      *
+      * @param next_proto is called with client's ALPN protocol list
+      *        and returns chosen protocol.
+      *
+      * @param is_datagram set to true if this server should expect DTLS
+      *        connections. Otherwise TLS connections are expected.
+      *
+      * @param reserved_io_buffer_size This many bytes of memory will
+      *        be preallocated for the read and write buffers. Smaller
+      *        values just mean reallocations and copies are more likely.
       */
-      Server(const Callbacks& callbacks,
+      Server(Callbacks& callbacks,
              Session_Manager& session_manager,
              Credentials_Manager& creds,
              const Policy& policy,
diff --git a/src/lib/tls/tls_server_handshake_state.h b/src/lib/tls/tls_server_handshake_state.h
deleted file mode 100644
index c65cade96..000000000
--- a/src/lib/tls/tls_server_handshake_state.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
-* TLS Server
-* (C) 2004-2011,2012,2016 Jack Lloyd
-*     2016 Matthias Gierlings
-*
-* Botan is released under the Simplified BSD License (see license.txt)
-*/
-
-#ifndef BOTAN_TLS_SERVER_HANDSHAKE_STATE_H__
-#define BOTAN_TLS_SERVER_HANDSHAKE_STATE_H__
-
-#include <botan/internal/tls_handshake_state.h>
-namespace Botan {
-
-namespace TLS {
-
-class Server_Handshake_State : public Handshake_State
-   {
-   public:
-      Server_Handshake_State(Handshake_IO* io, handshake_msg_cb cb)
-         : Handshake_State(io, cb) {}
-
-      Private_Key* server_rsa_kex_key() { return m_server_rsa_kex_key; }
-      void set_server_rsa_kex_key(Private_Key* key)
-         { m_server_rsa_kex_key = key; }
-
-      bool allow_session_resumption() const
-         { return m_allow_session_resumption; }
-      void set_allow_session_resumption(bool allow_session_resumption)
-         { m_allow_session_resumption = allow_session_resumption; }
-
-
-   private:
-      // Used by the server only, in case of RSA key exchange. Not owned
-      Private_Key* m_server_rsa_kex_key = nullptr;
-
-      /*
-      * Used by the server to know if resumption should be allowed on
-      * a server-initiated renegotiation
-      */
-      bool m_allow_session_resumption = true;
-   };
-
-}
-
-}
-
-#endif