fix: Fix ssl in new webserver (#1981)

Fixes #1980.

An SSL handshake was missing and WsConnection should be build from
stream not socket because in case SSL connection stream already
completed SSL handshake.
This commit is contained in:
Sergey Kuznetsov
2025-04-01 16:12:16 +01:00
committed by GitHub
parent 60df3a1914
commit d3df6d10e4
9 changed files with 57 additions and 156 deletions

View File

@@ -23,7 +23,6 @@
#include "etlng/Models.hpp"
#include "util/log/Logger.hpp"
#include <cstdint>
#include <memory>
#include <utility>

View File

@@ -10,7 +10,6 @@ target_sources(
ng/impl/ErrorHandling.cpp
ng/impl/ConnectionHandler.cpp
ng/impl/ServerSslContext.cpp
ng/impl/WsConnection.cpp
ng/Request.cpp
ng/Response.cpp
ng/Server.cpp

View File

@@ -46,6 +46,7 @@
#include <boost/system/system_error.hpp>
#include <fmt/core.h>
#include <chrono>
#include <cstddef>
#include <functional>
#include <memory>
@@ -136,13 +137,19 @@ makeConnection(
if (not sslContext.has_value())
return std::unexpected{"Error creating a connection: SSL is not supported by this server"};
connection = std::make_unique<impl::SslHttpConnection>(
auto sslConnection = std::make_unique<impl::SslHttpConnection>(
std::move(sslDetectionResult.socket),
std::move(ip),
std::move(sslDetectionResult.buffer),
*sslContext,
tagDecoratorFactory
);
sslConnection->setTimeout(std::chrono::seconds{10});
auto const maybeError = sslConnection->sslHandshake(yield);
if (maybeError.has_value())
return std::unexpected{fmt::format("SSL handshake error: {}", maybeError->message())};
connection = std::move(sslConnection);
} else {
connection = std::make_unique<impl::PlainHttpConnection>(
std::move(sslDetectionResult.socket),
@@ -164,7 +171,6 @@ makeConnection(
std::expected<ConnectionPtr, std::string>
tryUpgradeConnection(
impl::UpgradableConnectionPtr connection,
std::optional<boost::asio::ssl::context>& sslContext,
util::TagDecoratorFactory& tagDecoratorFactory,
boost::asio::yield_context yield
)
@@ -177,7 +183,7 @@ tryUpgradeConnection(
}
if (*expectedIsUpgrade) {
auto expectedUpgradedConnection = connection->upgrade(sslContext, tagDecoratorFactory, yield);
auto expectedUpgradedConnection = connection->upgrade(tagDecoratorFactory, yield);
if (expectedUpgradedConnection.has_value())
return std::move(expectedUpgradedConnection).value();
@@ -316,8 +322,7 @@ Server::handleConnection(boost::asio::ip::tcp::socket socket, boost::asio::yield
return;
}
auto connection =
tryUpgradeConnection(std::move(connectionExpected).value(), sslContext_, tagDecoratorFactory_, yield);
auto connection = tryUpgradeConnection(std::move(connectionExpected).value(), tagDecoratorFactory_, yield);
if (not connection.has_value()) {
LOG(log_.info()) << connection.error();
return;

View File

@@ -28,10 +28,12 @@
#include "web/ng/impl/Concepts.hpp"
#include "web/ng/impl/WsConnection.hpp"
#include <boost/asio/buffer.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/spawn.hpp>
#include <boost/asio/ssl/context.hpp>
#include <boost/asio/ssl/stream.hpp>
#include <boost/asio/ssl/stream_base.hpp>
#include <boost/beast/core/basic_stream.hpp>
#include <boost/beast/core/error.hpp>
#include <boost/beast/core/flat_buffer.hpp>
@@ -57,11 +59,7 @@ public:
isUpgradeRequested(boost::asio::yield_context yield) = 0;
virtual std::expected<ConnectionPtr, Error>
upgrade(
std::optional<boost::asio::ssl::context>& sslContext,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
) = 0;
upgrade(util::TagDecoratorFactory const& tagDecoratorFactory, boost::asio::yield_context yield) = 0;
virtual std::optional<Error>
sendRaw(
@@ -104,6 +102,22 @@ public:
{
}
std::optional<Error>
sslHandshake(boost::asio::yield_context yield)
requires IsSslTcpStream<StreamType>
{
boost::system::error_code error;
boost::beast::get_lowest_layer(stream_).expires_after(timeout_);
auto const bytesUsed =
stream_.async_handshake(boost::asio::ssl::stream_base::server, buffer_.cdata(), yield[error]);
if (error)
return error;
buffer_.consume(bytesUsed);
return std::nullopt;
}
bool
wasUpgraded() const override
{
@@ -183,35 +197,18 @@ public:
}
std::expected<ConnectionPtr, Error>
upgrade(
[[maybe_unused]] std::optional<boost::asio::ssl::context>& sslContext,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
) override
upgrade(util::TagDecoratorFactory const& tagDecoratorFactory, boost::asio::yield_context yield) override
{
ASSERT(request_.has_value(), "Request must be present to upgrade the connection");
if constexpr (IsSslTcpStream<StreamType>) {
ASSERT(sslContext.has_value(), "SSL context must be present to upgrade the connection");
return makeSslWsConnection(
boost::beast::get_lowest_layer(stream_).release_socket(),
std::move(ip_),
std::move(buffer_),
std::move(request_).value(),
sslContext.value(),
tagDecoratorFactory,
yield
);
} else {
return makePlainWsConnection(
stream_.release_socket(),
std::move(ip_),
std::move(buffer_),
std::move(request_).value(),
tagDecoratorFactory,
yield
);
}
return makeWsConnection(
std::move(stream_),
std::move(ip_),
std::move(buffer_),
std::move(request_).value(),
tagDecoratorFactory,
yield
);
}
private:

View File

@@ -1,77 +0,0 @@
//------------------------------------------------------------------------------
/*
This file is part of clio: https://github.com/XRPLF/clio
Copyright (c) 2024, the clio developers.
Permission to use, copy, modify, and distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include "web/ng/impl/WsConnection.hpp"
#include "util/Taggable.hpp"
#include "web/ng/Error.hpp"
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/spawn.hpp>
#include <boost/asio/ssl/context.hpp>
#include <boost/beast/core/flat_buffer.hpp>
#include <boost/beast/http/message.hpp>
#include <boost/beast/http/string_body.hpp>
#include <memory>
#include <string>
#include <utility>
namespace web::ng::impl {
std::expected<std::unique_ptr<PlainWsConnection>, Error>
makePlainWsConnection(
boost::asio::ip::tcp::socket socket,
std::string ip,
boost::beast::flat_buffer buffer,
boost::beast::http::request<boost::beast::http::string_body> request,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
)
{
auto connection = std::make_unique<PlainWsConnection>(
std::move(socket), std::move(ip), std::move(buffer), std::move(request), tagDecoratorFactory
);
auto maybeError = connection->performHandshake(yield);
if (maybeError.has_value())
return std::unexpected{maybeError.value()};
return connection;
}
std::expected<std::unique_ptr<SslWsConnection>, Error>
makeSslWsConnection(
boost::asio::ip::tcp::socket socket,
std::string ip,
boost::beast::flat_buffer buffer,
boost::beast::http::request<boost::beast::http::string_body> request,
boost::asio::ssl::context& sslContext,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
)
{
auto connection = std::make_unique<SslWsConnection>(
std::move(socket), std::move(ip), std::move(buffer), sslContext, std::move(request), tagDecoratorFactory
);
auto maybeError = connection->performHandshake(yield);
if (maybeError.has_value())
return std::unexpected{maybeError.value()};
return connection;
}
} // namespace web::ng::impl

View File

@@ -68,31 +68,14 @@ class WsConnection : public WsConnectionBase {
public:
WsConnection(
boost::asio::ip::tcp::socket socket,
StreamType&& stream,
std::string ip,
boost::beast::flat_buffer buffer,
boost::beast::http::request<boost::beast::http::string_body> initialRequest,
util::TagDecoratorFactory const& tagDecoratorFactory
)
requires IsTcpStream<StreamType>
: WsConnectionBase(std::move(ip), std::move(buffer), tagDecoratorFactory)
, stream_(std::move(socket))
, initialRequest_(std::move(initialRequest))
{
setupWsStream();
}
WsConnection(
boost::asio::ip::tcp::socket socket,
std::string ip,
boost::beast::flat_buffer buffer,
boost::asio::ssl::context& sslContext,
boost::beast::http::request<boost::beast::http::string_body> initialRequest,
util::TagDecoratorFactory const& tagDecoratorFactory
)
requires IsSslTcpStream<StreamType>
: WsConnectionBase(std::move(ip), std::move(buffer), tagDecoratorFactory)
, stream_(std::move(socket), sslContext)
, stream_(std::move(stream))
, initialRequest_(std::move(initialRequest))
{
setupWsStream();
@@ -189,25 +172,24 @@ private:
using PlainWsConnection = WsConnection<boost::beast::tcp_stream>;
using SslWsConnection = WsConnection<boost::asio::ssl::stream<boost::beast::tcp_stream>>;
std::expected<std::unique_ptr<PlainWsConnection>, Error>
makePlainWsConnection(
boost::asio::ip::tcp::socket socket,
template <typename StreamType>
std::expected<std::unique_ptr<WsConnection<StreamType>>, Error>
makeWsConnection(
StreamType&& stream,
std::string ip,
boost::beast::flat_buffer buffer,
boost::beast::http::request<boost::beast::http::string_body> request,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
);
std::expected<std::unique_ptr<SslWsConnection>, Error>
makeSslWsConnection(
boost::asio::ip::tcp::socket socket,
std::string ip,
boost::beast::flat_buffer buffer,
boost::beast::http::request<boost::beast::http::string_body> request,
boost::asio::ssl::context& sslContext,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield
);
)
{
auto connection = std::make_unique<WsConnection<StreamType>>(
std::forward<StreamType>(stream), std::move(ip), std::move(buffer), std::move(request), tagDecoratorFactory
);
auto maybeError = connection->performHandshake(yield);
if (maybeError.has_value())
return std::unexpected{maybeError.value()};
return connection;
}
} // namespace web::ng::impl

View File

@@ -67,9 +67,7 @@ struct MockHttpConnectionImpl : web::ng::impl::UpgradableConnection {
MOCK_METHOD(
UpgradeReturnType,
upgrade,
(OptionalSslContext & sslContext,
util::TagDecoratorFactory const& tagDecoratorFactory,
boost::asio::yield_context yield),
(util::TagDecoratorFactory const& tagDecoratorFactory, boost::asio::yield_context yield),
(override)
);
};

View File

@@ -300,8 +300,7 @@ TEST_F(HttpConnectionTests, Upgrade)
[&]() { ASSERT_TRUE(expectedResult.has_value()) << expectedResult.error().message(); }();
[&]() { ASSERT_TRUE(expectedResult.value()); }();
std::optional<boost::asio::ssl::context> sslContext;
auto expectedWsConnection = connection.upgrade(sslContext, tagDecoratorFactory_, yield);
auto expectedWsConnection = connection.upgrade(tagDecoratorFactory_, yield);
[&]() { ASSERT_TRUE(expectedWsConnection.has_value()) << expectedWsConnection.error().message(); }();
});
}

View File

@@ -74,8 +74,7 @@ struct WebWsConnectionTests : SyncAsioContextTest {
ASSERT_TRUE(expectedTrue.value()) << "Expected upgrade request";
}();
std::optional<boost::asio::ssl::context> sslContext;
auto expectedWsConnection = httpConnection.upgrade(sslContext, tagDecoratorFactory_, yield);
auto expectedWsConnection = httpConnection.upgrade(tagDecoratorFactory_, yield);
[&]() { ASSERT_TRUE(expectedWsConnection.has_value()) << expectedWsConnection.error().message(); }();
auto connection = std::move(expectedWsConnection).value();
auto wsConnectionPtr = dynamic_cast<PlainWsConnection*>(connection.release());