Unit test Connection

Boost asio provides a test stream object that we can use to begin unit
testing the connection object.  This patchset uses it to re-enable
some simple http1.1 tests.  There's some features that have snuck into
the connection class that aren't compatible with a stream (like ip
address getting), so unfortunately we do need the connection class to
be aware if it's in test mode, but that tradeoff seems worthwhile.

Tested: Unit test pass.

Change-Id: Id8b1f8866582b58502dbafe6139f841bf64b8ef3
Signed-off-by: Ed Tanous <edtanous@google.com>

6 files changed, 180 insertions, 107 deletions

diff --git a/http/app.hpp b/http/app.hpp
index d9c88b9563..1a7af83241 100644
--- a/http/app.hpp
+++ b/http/app.hpp
@@ -10,10 +10,8 @@
 
 #include <boost/asio/io_context.hpp>
 #include <boost/asio/ip/tcp.hpp>
-#ifdef BMCWEB_ENABLE_SSL
 #include <boost/asio/ssl/context.hpp>
 #include <boost/beast/ssl/ssl_stream.hpp>
-#endif
 
 #include <chrono>
 #include <cstdint>
@@ -29,19 +27,13 @@
 
 namespace crow
 {
-#ifdef BMCWEB_ENABLE_SSL
-using ssl_context_t = boost::asio::ssl::context;
-#endif
 class App
 {
   public:
-#ifdef BMCWEB_ENABLE_SSL
     using ssl_socket_t = boost::beast::ssl_stream<boost::asio::ip::tcp::socket>;
     using ssl_server_t = Server<App, ssl_socket_t>;
-#else
     using socket_t = boost::asio::ip::tcp::socket;
     using server_t = Server<App, socket_t>;
-#endif
 
     explicit App(std::shared_ptr<boost::asio::io_context> ioIn =
                      std::make_shared<boost::asio::io_context>()) :
@@ -94,12 +86,6 @@ class App
         return *this;
     }
 
-    App& bindaddr(std::string bindaddr)
-    {
-        bindaddrStr = std::move(bindaddr);
-        return *this;
-    }
-
     void validate()
     {
         router.validate();
@@ -111,8 +97,8 @@ class App
 #ifdef BMCWEB_ENABLE_SSL
         if (-1 == socketFd)
         {
-            sslServer = std::make_unique<ssl_server_t>(
-                this, bindaddrStr, portUint, sslContext, io);
+            sslServer = std::make_unique<ssl_server_t>(this, portUint,
+                                                       sslContext, io);
         }
         else
         {
@@ -125,8 +111,7 @@ class App
 
         if (-1 == socketFd)
         {
-            server = std::make_unique<server_t>(this, bindaddrStr, portUint,
-                                                nullptr, io);
+            server = std::make_unique<server_t>(this, portUint, nullptr, io);
         }
         else
         {
@@ -158,7 +143,6 @@ class App
         return router.getRoutes(parent);
     }
 
-#ifdef BMCWEB_ENABLE_SSL
     App& ssl(std::shared_ptr<boost::asio::ssl::context>&& ctx)
     {
         sslContext = std::move(ctx);
@@ -167,21 +151,7 @@ class App
         return *this;
     }
 
-    std::shared_ptr<ssl_context_t> sslContext = nullptr;
-
-#else
-    template <typename T>
-    App& ssl(T&&)
-    {
-        // We can't call .ssl() member function unless BMCWEB_ENABLE_SSL is
-        // defined.
-        static_assert(
-            // make static_assert dependent to T; always false
-            std::is_base_of<T, void>::value,
-            "Define BMCWEB_ENABLE_SSL to enable ssl support.");
-        return *this;
-    }
-#endif
+    std::shared_ptr<boost::asio::ssl::context> sslContext = nullptr;
 
     boost::asio::io_context& ioContext()
     {
@@ -195,7 +165,6 @@ class App
 #else
     uint16_t portUint = 80;
 #endif
-    std::string bindaddrStr = "0.0.0.0";
     int socketFd = -1;
     Router router;
 
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 1ec80ae77a..c64d511925 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -17,6 +17,7 @@
 #include <boost/asio/ip/tcp.hpp>
 #include <boost/asio/ssl/stream.hpp>
 #include <boost/asio/steady_timer.hpp>
+#include <boost/beast/_experimental/test/stream.hpp>
 #include <boost/beast/core/buffers_generator.hpp>
 #include <boost/beast/core/flat_static_buffer.hpp>
 #include <boost/beast/http/error.hpp>
@@ -44,6 +45,14 @@ constexpr uint64_t loggedOutPostBodyLimit = 4096;
 
 constexpr uint32_t httpHeaderLimit = 8192;
 
+template <typename>
+struct IsTls : std::false_type
+{};
+
+template <typename T>
+struct IsTls<boost::beast::ssl_stream<T>> : std::true_type
+{};
+
 template <typename Adaptor, typename Handler>
 class Connection :
     public std::enable_shared_from_this<Connection<Adaptor, Handler>>
@@ -112,31 +121,34 @@ class Connection :
 
     void prepareMutualTls()
     {
-        std::error_code error;
-        std::filesystem::path caPath(ensuressl::trustStorePath);
-        auto caAvailable = !std::filesystem::is_empty(caPath, error);
-        caAvailable = caAvailable && !error;
-        if (caAvailable && persistent_data::SessionStore::getInstance()
-                               .getAuthMethodsConfig()
-                               .tls)
+        if constexpr (IsTls<Adaptor>::value)
         {
-            adaptor.set_verify_mode(boost::asio::ssl::verify_peer);
-            std::string id = "bmcweb";
-
-            const char* cStr = id.c_str();
-            // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-            const auto* idC = reinterpret_cast<const unsigned char*>(cStr);
-            int ret = SSL_set_session_id_context(
-                adaptor.native_handle(), idC,
-                static_cast<unsigned int>(id.length()));
-            if (ret == 0)
+            std::error_code error;
+            std::filesystem::path caPath(ensuressl::trustStorePath);
+            auto caAvailable = !std::filesystem::is_empty(caPath, error);
+            caAvailable = caAvailable && !error;
+            if (caAvailable && persistent_data::SessionStore::getInstance()
+                                   .getAuthMethodsConfig()
+                                   .tls)
             {
-                BMCWEB_LOG_ERROR("{} failed to set SSL id", logPtr(this));
+                adaptor.set_verify_mode(boost::asio::ssl::verify_peer);
+                std::string id = "bmcweb";
+
+                const char* cStr = id.c_str();
+                // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+                const auto* idC = reinterpret_cast<const unsigned char*>(cStr);
+                int ret = SSL_set_session_id_context(
+                    adaptor.native_handle(), idC,
+                    static_cast<unsigned int>(id.length()));
+                if (ret == 0)
+                {
+                    BMCWEB_LOG_ERROR("{} failed to set SSL id", logPtr(this));
+                }
             }
-        }
 
-        adaptor.set_verify_callback(
-            std::bind_front(&self_type::tlsVerifyCallback, this));
+            adaptor.set_verify_callback(
+                std::bind_front(&self_type::tlsVerifyCallback, this));
+        }
     }
 
     Adaptor& socket()
@@ -157,9 +169,7 @@ class Connection :
 
         // TODO(ed) Abstract this to a more clever class with the idea of an
         // asynchronous "start"
-        if constexpr (std::is_same_v<Adaptor,
-                                     boost::beast::ssl_stream<
-                                         boost::asio::ip::tcp::socket>>)
+        if constexpr (IsTls<Adaptor>::value)
         {
             adaptor.async_handshake(boost::asio::ssl::stream_base::server,
                                     [this, self(shared_from_this())](
@@ -252,20 +262,23 @@ class Connection :
             return;
         }
         keepAlive = thisReq.keepAlive();
-#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
-        if (!crow::authentication::isOnAllowlist(req->url().path(),
-                                                 req->method()) &&
-            thisReq.session == nullptr)
+        if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
         {
-            BMCWEB_LOG_WARNING("Authentication failed");
-            forward_unauthorized::sendUnauthorized(
-                req->url().encoded_path(),
-                req->getHeaderValue("X-Requested-With"),
-                req->getHeaderValue("Accept"), res);
-            completeRequest(res);
-            return;
-        }
+#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
+            if (!crow::authentication::isOnAllowlist(req->url().path(),
+                                                     req->method()) &&
+                thisReq.session == nullptr)
+            {
+                BMCWEB_LOG_WARNING("Authentication failed");
+                forward_unauthorized::sendUnauthorized(
+                    req->url().encoded_path(),
+                    req->getHeaderValue("X-Requested-With"),
+                    req->getHeaderValue("Accept"), res);
+                completeRequest(res);
+                return;
+            }
 #endif // BMCWEB_INSECURE_DISABLE_AUTHX
+        }
         auto asyncResp = std::make_shared<bmcweb::AsyncResp>();
         BMCWEB_LOG_DEBUG("Setting completion handler");
         asyncResp->res.setCompleteRequestHandler(
@@ -309,12 +322,14 @@ class Connection :
 
     bool isAlive()
     {
-        if constexpr (std::is_same_v<Adaptor,
-                                     boost::beast::ssl_stream<
-                                         boost::asio::ip::tcp::socket>>)
+        if constexpr (IsTls<Adaptor>::value)
         {
             return adaptor.next_layer().is_open();
         }
+        else if constexpr (std::is_same_v<Adaptor, boost::beast::test::stream>)
+        {
+            return true;
+        }
         else
         {
             return adaptor.is_open();
@@ -322,9 +337,7 @@ class Connection :
     }
     void close()
     {
-        if constexpr (std::is_same_v<Adaptor,
-                                     boost::beast::ssl_stream<
-                                         boost::asio::ip::tcp::socket>>)
+        if constexpr (IsTls<Adaptor>::value)
         {
             adaptor.next_layer().close();
             if (mtlsSession != nullptr)
@@ -384,18 +397,22 @@ class Connection :
     {
         boost::system::error_code ec;
         BMCWEB_LOG_DEBUG("Fetch the client IP address");
-        boost::asio::ip::tcp::endpoint endpoint =
-            boost::beast::get_lowest_layer(adaptor).remote_endpoint(ec);
 
-        if (ec)
+        if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
         {
-            // If remote endpoint fails keep going. "ClientOriginIPAddress"
-            // will be empty.
-            BMCWEB_LOG_ERROR("Failed to get the client's IP Address. ec : {}",
-                             ec);
-            return ec;
+            boost::asio::ip::tcp::endpoint endpoint =
+                boost::beast::get_lowest_layer(adaptor).remote_endpoint(ec);
+
+            if (ec)
+            {
+                // If remote endpoint fails keep going. "ClientOriginIPAddress"
+                // will be empty.
+                BMCWEB_LOG_ERROR(
+                    "Failed to get the client's IP Address. ec : {}", ec);
+                return ec;
+            }
+            ip = endpoint.address();
         }
-        ip = endpoint.address();
         return ec;
     }
 
@@ -457,27 +474,31 @@ class Connection :
             {
                 BMCWEB_LOG_DEBUG("Unable to get client IP");
             }
+            if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
+            {
 #ifndef BMCWEB_INSECURE_DISABLE_AUTHX
-            boost::beast::http::verb method = parser->get().method();
-            userSession = crow::authentication::authenticate(
-                ip, res, method, parser->get().base(), mtlsSession);
+                boost::beast::http::verb method = parser->get().method();
+                userSession = crow::authentication::authenticate(
+                    ip, res, method, parser->get().base(), mtlsSession);
 
-            bool loggedIn = userSession != nullptr;
-            if (!loggedIn)
-            {
-                const boost::optional<uint64_t> contentLength =
-                    parser->content_length();
-                if (contentLength && *contentLength > loggedOutPostBodyLimit)
+                bool loggedIn = userSession != nullptr;
+                if (!loggedIn)
                 {
-                    BMCWEB_LOG_DEBUG("Content length greater than limit {}",
-                                     *contentLength);
-                    close();
-                    return;
+                    const boost::optional<uint64_t> contentLength =
+                        parser->content_length();
+                    if (contentLength &&
+                        *contentLength > loggedOutPostBodyLimit)
+                    {
+                        BMCWEB_LOG_DEBUG("Content length greater than limit {}",
+                                         *contentLength);
+                        close();
+                        return;
+                    }
+
+                    BMCWEB_LOG_DEBUG("Starting quick deadline");
                 }
-
-                BMCWEB_LOG_DEBUG("Starting quick deadline");
-            }
 #endif // BMCWEB_INSECURE_DISABLE_AUTHX
+            }
 
             if (parser->is_done())
             {
diff --git a/http/http_server.hpp b/http/http_server.hpp
index b290ad7902..2a6bd9f4aa 100644
--- a/http/http_server.hpp
+++ b/http/http_server.hpp
@@ -38,14 +38,14 @@ class Server
         adaptorCtx(std::move(adaptorCtxIn))
     {}
 
-    Server(Handler* handlerIn, const std::string& bindaddr, uint16_t port,
+    Server(Handler* handlerIn, uint16_t port,
            const std::shared_ptr<boost::asio::ssl::context>& adaptorCtxIn,
            const std::shared_ptr<boost::asio::io_context>& io =
                std::make_shared<boost::asio::io_context>()) :
         Server(handlerIn,
                std::make_unique<boost::asio::ip::tcp::acceptor>(
                    *io, boost::asio::ip::tcp::endpoint(
-                            boost::asio::ip::make_address(bindaddr), port)),
+                            boost::asio::ip::make_address("0.0.0.0"), port)),
                adaptorCtxIn, io)
     {}
 
diff --git a/include/security_headers.hpp b/include/security_headers.hpp
index 1b9e984dd2..236b367fac 100644
--- a/include/security_headers.hpp
+++ b/include/security_headers.hpp
@@ -56,13 +56,10 @@ inline void addSecurityHeaders(const crow::Request& req [[maybe_unused]],
                                         "screen-wak-lock=(),"
                                         "web-share=(),"
                                         "xr-spatial-tracking=()");
-
     res.addHeader("X-Permitted-Cross-Domain-Policies", "none");
-
     res.addHeader("Cross-Origin-Embedder-Policy", "require-corp");
     res.addHeader("Cross-Origin-Opener-Policy", "same-origin");
     res.addHeader("Cross-Origin-Resource-Policy", "same-origin");
-
     if (bmcwebInsecureDisableXssPrevention == 0)
     {
         res.addHeader("Content-Security-Policy", "default-src 'none'; "
diff --git a/meson.build b/meson.build
index e1ee9c35d5..541fa5e820 100644
--- a/meson.build
+++ b/meson.build
@@ -412,6 +412,7 @@ executable(
 
 srcfiles_unittest = files(
   'test/http/crow_getroutes_test.cpp',
+  'test/http/http_connection_test.cpp',
   'test/http/router_test.cpp',
   'test/http/utility_test.cpp',
   'test/http/verb_test.cpp',
diff --git a/test/http/http_connection_test.cpp b/test/http/http_connection_test.cpp
new file mode 100644
index 0000000000..c4252e1ee7
--- /dev/null
+++ b/test/http/http_connection_test.cpp
@@ -0,0 +1,85 @@
+#include "http/http_connection.hpp"
+#include "http/http_request.hpp"
+#include "http/http_response.hpp"
+
+#include <boost/asio/steady_timer.hpp>
+#include <boost/beast/_experimental/test/stream.hpp>
+
+#include <filesystem>
+#include <fstream>
+#include <functional>
+#include <memory>
+#include <string>
+
+#include "gtest/gtest.h"
+namespace crow
+{
+
+struct FakeHandler
+{
+    static void
+        handleUpgrade(Request& /*req*/,
+                      const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/,
+                      boost::beast::test::stream&& /*adaptor*/)
+    {
+        // Handle Upgrade should never be called
+        EXPECT_FALSE(true);
+    }
+
+    void handle(Request& req,
+                const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/)
+    {
+        EXPECT_EQ(req.method(), boost::beast::http::verb::get);
+        EXPECT_EQ(req.target(), "/");
+        called = true;
+    }
+    bool called = false;
+};
+
+static std::string getDateStr()
+{
+    return "TestTime";
+}
+
+TEST(http_connection, RequestPropogates)
+{
+    boost::asio::io_context io;
+    boost::beast::test::stream stream(io);
+    boost::beast::test::stream out(io);
+    stream.connect(out);
+
+    out.write_some(boost::asio::buffer(
+        "GET / HTTP/1.1\r\nHost: openbmc_project.xyz\r\nConnection: close\r\n\r\n"));
+    FakeHandler handler;
+    boost::asio::steady_timer timer(io);
+    std::function<std::string()> date(&getDateStr);
+    std::shared_ptr<crow::Connection<boost::beast::test::stream, FakeHandler>>
+        conn = std::make_shared<
+            crow::Connection<boost::beast::test::stream, FakeHandler>>(
+            &handler, std::move(timer), date, std::move(stream));
+    conn->start();
+    io.run_for(std::chrono::seconds(1000));
+    EXPECT_TRUE(handler.called);
+    std::string outStr = out.str();
+
+    std::string expected =
+        "HTTP/1.1 200 OK\r\n"
+        "Connection: close\r\n"
+        "Strict-Transport-Security: max-age=31536000; includeSubdomains\r\n"
+        "X-Frame-Options: DENY\r\n"
+        "Pragma: no-cache\r\n"
+        "Cache-Control: no-store, max-age=0\r\n"
+        "X-Content-Type-Options: nosniff\r\n"
+        "Referrer-Policy: no-referrer\r\n"
+        "Permissions-Policy: accelerometer=(),ambient-light-sensor=(),autoplay=(),battery=(),camera=(),display-capture=(),document-domain=(),encrypted-media=(),fullscreen=(),gamepad=(),geolocation=(),gyroscope=(),layout-animations=(self),legacy-image-formats=(self),magnetometer=(),microphone=(),midi=(),oversized-images=(self),payment=(),picture-in-picture=(),publickey-credentials-get=(),speaker-selection=(),sync-xhr=(self),unoptimized-images=(self),unsized-media=(self),usb=(),screen-wak-lock=(),web-share=(),xr-spatial-tracking=()\r\n"
+        "X-Permitted-Cross-Domain-Policies: none\r\n"
+        "Cross-Origin-Embedder-Policy: require-corp\r\n"
+        "Cross-Origin-Opener-Policy: same-origin\r\n"
+        "Cross-Origin-Resource-Policy: same-origin\r\n"
+        "Content-Security-Policy: default-src 'none'; img-src 'self' data:; font-src 'self'; style-src 'self'; script-src 'self'; connect-src 'self' wss:; form-action 'none'; frame-ancestors 'none'; object-src 'none'; base-uri 'none'\r\n"
+        "Content-Length: 0\r\n"
+        "\r\n";
+    EXPECT_EQ(outStr, expected);
+}
+
+} // namespace crow