TLS::Context holds references rather than pointers

Parameters passed from TLS::Context to TLS::Client for initialization
are now held as references in the context. Ownership of these members is
thereby explicitly left with the user.

Co-authored-by: Tim Oesterreich <[email protected]>

3 files changed, 94 insertions, 84 deletions

diff --git a/src/lib/tls/asio/asio_context.h b/src/lib/tls/asio/asio_context.h
index c61086497..e5e99e83a 100644
--- a/src/lib/tls/asio/asio_context.h
+++ b/src/lib/tls/asio/asio_context.h
@@ -53,22 +53,24 @@ class Context
       using Verify_Callback =
          detail::fn_signature_helper<decltype(&Callbacks::tls_verify_cert_chain)>::type;
 
-      Context(Credentials_Manager*   credentialsManager,
-              RandomNumberGenerator* randomNumberGenerator,
-              Session_Manager*       sessionManager,
-              Policy*                policy,
-              Server_Information     serverInfo = Server_Information()) :
-         credentialsManager(credentialsManager),
-         randomNumberGenerator(randomNumberGenerator),
-         sessionManager(sessionManager),
-         policy(policy),
-         serverInfo(serverInfo)
+      Context(Credentials_Manager&   credentials_manager,
+              RandomNumberGenerator& rng,
+              Session_Manager&       session_manager,
+              Policy&                policy,
+              Server_Information     server_info = Server_Information()) :
+         m_credentials_manager(credentials_manager),
+         m_rng(rng),
+         m_session_manager(session_manager),
+         m_policy(policy),
+         m_server_info(server_info)
          {}
 
-      Context(const Context& other) = delete;
-      Context& operator=(const Context& other) = delete;
-      Context(Context&& other) = default;
-      Context& operator=(Context&& other) = default;
+      virtual ~Context() = default;
+
+      Context(Context&&)                 = default;
+      Context(const Context&)            = delete;
+      Context& operator=(const Context&) = delete;
+      Context& operator=(Context&&)      = delete;
 
       /**
        * @brief Override the tls_verify_cert_chain callback
@@ -81,24 +83,34 @@ class Context
        */
       void set_verify_callback(Verify_Callback callback)
          {
-         verifyCallback = std::move(callback);
+         m_verify_callback = std::move(callback);
          }
 
       bool has_verify_callback() const
          {
-         return static_cast<bool>(verifyCallback);
+         return static_cast<bool>(m_verify_callback);
+         }
+
+      const Verify_Callback& get_verify_callback() const
+         {
+         return m_verify_callback;
+         }
+
+      void set_server_info(const Server_Information& server_info)
+         {
+         m_server_info = server_info;
          }
 
    protected:
       template <class S, class C> friend class Stream;
 
-      Credentials_Manager*   credentialsManager;
-      RandomNumberGenerator* randomNumberGenerator;
-      Session_Manager*       sessionManager;
-      Policy*                policy;
+      Credentials_Manager&   m_credentials_manager;
+      RandomNumberGenerator& m_rng;
+      Session_Manager&       m_session_manager;
+      Policy&                m_policy;
 
-      Server_Information     serverInfo;
-      Verify_Callback        verifyCallback;
+      Server_Information     m_server_info;
+      Verify_Callback        m_verify_callback;
    };
 
 }  // namespace TLS
diff --git a/src/lib/tls/asio/asio_stream.h b/src/lib/tls/asio/asio_stream.h
index 8fd309bc7..e8d9c2930 100644
--- a/src/lib/tls/asio/asio_stream.h
+++ b/src/lib/tls/asio/asio_stream.h
@@ -52,14 +52,14 @@ class Stream
       //! \name construction
       //! @{
 
-       /**
-        * @brief Construct a new Stream
-        *
-        * @param context The context parameter is used to set up the underlying native handle. Using code is
-        *                responsible for lifetime management of the context and must ensure that it is available for the
-        *                lifetime of the stream.
-        * @param args Arguments to be forwarded to the construction of the next layer.
-        */
+      /**
+       * @brief Construct a new Stream
+       *
+       * @param context The context parameter is used to set up the underlying native handle. Using code is
+       *                responsible for lifetime management of the context and must ensure that it is available for the
+       *                lifetime of the stream.
+       * @param args Arguments to be forwarded to the construction of the next layer.
+       */
       template <typename... Args>
       explicit Stream(Context& context, Args&& ... args)
          : m_context(context)
@@ -69,16 +69,16 @@ class Stream
          , m_input_buffer(m_input_buffer_space.data(), m_input_buffer_space.size())
          {}
 
-       /**
-        * @brief Construct a new Stream
-        *
-        * Convenience overload for boost::asio::ssl::stream compatibility.
-        *
-        * @param arg This argument is forwarded to the construction of the next layer.
-        * @param context The context parameter is used to set up the underlying native handle. Using code is
-        *                responsible for lifetime management of the context and must ensure that is available for the
-        *                lifetime of the stream.
-        */
+      /**
+       * @brief Construct a new Stream
+       *
+       * Convenience overload for boost::asio::ssl::stream compatibility.
+       *
+       * @param arg This argument is forwarded to the construction of the next layer.
+       * @param context The context parameter is used to set up the underlying native handle. Using code is
+       *                responsible for lifetime management of the context and must ensure that is available for the
+       *                lifetime of the stream.
+       */
       template <typename Arg>
       explicit Stream(Arg&& arg, Context& context)
          : m_context(context)
@@ -576,7 +576,7 @@ class Stream
                {
                if(m_tls_context.has_verify_callback())
                   {
-                  m_tls_context.verifyCallback(cert_chain, ocsp_responses, trusted_roots, usage, hostname, policy);
+                  m_tls_context.get_verify_callback()(cert_chain, ocsp_responses, trusted_roots, usage, hostname, policy);
                   }
                else
                   {
@@ -637,12 +637,13 @@ class Stream
          {
          if(side == CLIENT)
             {
-            m_native_handle = std::unique_ptr<Client>(new Client(m_core,
-                              *m_context.sessionManager,
-                              *m_context.credentialsManager,
-                              *m_context.policy,
-                              *m_context.randomNumberGenerator,
-                              m_context.serverInfo));
+            m_native_handle = std::unique_ptr<Client>(
+                                 new Client(m_core,
+                                            m_context.m_session_manager,
+                                            m_context.m_credentials_manager,
+                                            m_context.m_policy,
+                                            m_context.m_rng,
+                                            m_context.m_server_info));
             }
          else
             {
diff --git a/src/tests/unit_asio_stream.cpp b/src/tests/unit_asio_stream.cpp
index 510333bba..5fd67cbd4 100644
--- a/src/tests/unit_asio_stream.cpp
+++ b/src/tests/unit_asio_stream.cpp
@@ -62,10 +62,6 @@ class MockChannel
       Botan::TLS::Callbacks& m_callbacks;
       std::size_t m_bytes_till_complete_record;  // number of bytes still to read before tls record is completed
       bool        m_active;
-
-      Botan::TLS::Session_Manager_Noop m_session_manager;
-      Botan::Null_RNG m_rng;
-      Botan::TLS::Default_Policy m_policy;
    };
 
 class ThrowingMockChannel : public MockChannel
@@ -121,15 +117,6 @@ class ThrowingAsioStream : public Botan::TLS::Stream<TestStream, ThrowingMockCha
    };
 
 /**
- * Mocked Botan::TLS::Context. It is broken, but never used since the Channel is mocked as well.
- */
-class MockContext : public Botan::TLS::Context
-   {
-   public:
-      MockContext() : Botan::TLS::Context(nullptr, nullptr, nullptr, nullptr) {}
-   };
-
-/**
  * Synchronous tests for Botan::Stream.
  *
  * This test validates the asynchronous behavior Botan::Stream, including its utility classes StreamCore and Async_*_Op.
@@ -139,6 +126,16 @@ class MockContext : public Botan::TLS::Context
  */
 class Asio_Stream_Tests final : public Test
    {
+      Botan::Credentials_Manager m_credentials_manager;
+      Botan::Null_RNG m_rng;
+      Botan::TLS::Session_Manager_Noop m_session_manager;
+      Botan::TLS::Default_Policy m_policy;
+
+      Botan::TLS::Context get_context()
+         {
+         return Botan::TLS::Context(m_credentials_manager, m_rng, m_session_manager, m_policy);
+         }
+
       // use memcmp to check if the data in a is a prefix of the data in b
       bool contains(const void* a, const void* b, const std::size_t size) { return memcmp(a, b, size) == 0; }
 
@@ -150,7 +147,7 @@ class Asio_Stream_Tests final : public Test
       void test_sync_handshake(std::vector<Test::Result>& results)
          {
          net::io_context ioc;
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
 
          ssl.handshake(Botan::TLS::CLIENT);
@@ -167,7 +164,7 @@ class Asio_Stream_Tests final : public Test
          FailCount  fc{0, net::error::eof};
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          ssl.next_layer().connect(remote);
 
@@ -188,7 +185,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc, test_data());
          ssl.next_layer().connect(remote);
 
@@ -206,7 +203,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
          ssl.next_layer().connect(remote);
 
@@ -236,7 +233,7 @@ class Asio_Stream_Tests final : public Test
          FailCount  fc{0, net::error::eof};
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          ssl.next_layer().connect(remote);
 
@@ -262,7 +259,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc, test_data());
          ssl.next_layer().connect(remote);
 
@@ -284,7 +281,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
 
          const std::size_t buf_size = 128;
@@ -305,7 +302,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
          error_code ec;
 
@@ -335,7 +332,7 @@ class Asio_Stream_Tests final : public Test
          FailCount  fc{0, net::error::eof};
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          ssl.next_layer().connect(remote);
 
@@ -356,7 +353,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc, test_data());
          ssl.next_layer().connect(remote);
 
@@ -376,7 +373,7 @@ class Asio_Stream_Tests final : public Test
          {
          net::io_context ioc;
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
 
          const std::size_t buf_size = 128;
@@ -399,7 +396,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
          uint8_t    data[TEST_DATA_SIZE];
 
@@ -423,7 +420,7 @@ class Asio_Stream_Tests final : public Test
       void test_async_read_some_buffer_sequence(std::vector<Test::Result>& results)
          {
          net::io_context ioc;
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, test_data());
 
          std::vector<net::mutable_buffer> data;
@@ -455,7 +452,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          // fail right away
          FailCount  fc{0, net::error::eof};
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          uint8_t    data[TEST_DATA_SIZE];
 
@@ -478,7 +475,7 @@ class Asio_Stream_Tests final : public Test
       void test_async_read_some_throw(std::vector<Test::Result>& results)
          {
          net::io_context ioc;
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc, test_data());
          uint8_t    data[TEST_DATA_SIZE];
 
@@ -503,7 +500,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
          uint8_t    data[TEST_DATA_SIZE];
 
@@ -530,7 +527,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);
          error_code ec;
@@ -550,7 +547,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);
          error_code ec;
@@ -590,7 +587,7 @@ class Asio_Stream_Tests final : public Test
          FailCount  fc{0, net::error::eof};
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          ssl.next_layer().connect(remote);
 
@@ -610,7 +607,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);
          error_code ec;
@@ -629,7 +626,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);
 
@@ -653,7 +650,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream      remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);
 
@@ -696,7 +693,7 @@ class Asio_Stream_Tests final : public Test
          FailCount  fc{0, net::error::eof};
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          AsioStream ssl(ctx, ioc, fc);
          ssl.next_layer().connect(remote);
 
@@ -719,7 +716,7 @@ class Asio_Stream_Tests final : public Test
          net::io_context ioc;
          TestStream remote{ioc};
 
-         Botan_Tests::MockContext ctx;
+         auto ctx = get_context();
          ThrowingAsioStream ssl(ctx, ioc);
          ssl.next_layer().connect(remote);