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>
diff --git a/http/app.hpp b/http/app.hpp
index d9c88b9..1a7af83 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 @@
return *this;
}
- App& bindaddr(std::string bindaddr)
- {
- bindaddrStr = std::move(bindaddr);
- return *this;
- }
-
void validate()
{
router.validate();
@@ -111,8 +97,8 @@
#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 @@
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 @@
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 @@
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 @@
#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 1ec80ae..c64d511 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 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 @@
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";
- adaptor.set_verify_callback(
- std::bind_front(&self_type::tlsVerifyCallback, this));
+ 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& socket()
@@ -157,9 +169,7 @@
// 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 @@
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 @@
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 @@
}
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 @@
{
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 @@
{
BMCWEB_LOG_DEBUG("Unable to get client IP");
}
-#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
- 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)
+ if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
{
- const boost::optional<uint64_t> contentLength =
- parser->content_length();
- if (contentLength && *contentLength > loggedOutPostBodyLimit)
- {
- BMCWEB_LOG_DEBUG("Content length greater than limit {}",
- *contentLength);
- close();
- return;
- }
+#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
+ boost::beast::http::verb method = parser->get().method();
+ userSession = crow::authentication::authenticate(
+ ip, res, method, parser->get().base(), mtlsSession);
- BMCWEB_LOG_DEBUG("Starting quick deadline");
- }
+ bool loggedIn = userSession != nullptr;
+ if (!loggedIn)
+ {
+ 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");
+ }
#endif // BMCWEB_INSECURE_DISABLE_AUTHX
+ }
if (parser->is_done())
{
diff --git a/http/http_server.hpp b/http/http_server.hpp
index b290ad7..2a6bd9f 100644
--- a/http/http_server.hpp
+++ b/http/http_server.hpp
@@ -38,14 +38,14 @@
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 1b9e984..236b367 100644
--- a/include/security_headers.hpp
+++ b/include/security_headers.hpp
@@ -56,13 +56,10 @@
"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 e1ee9c3..541fa5e 100644
--- a/meson.build
+++ b/meson.build
@@ -412,6 +412,7 @@
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 0000000..c4252e1
--- /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