From: Francis Dupont Date: Wed, 5 Aug 2026 16:34:53 +0000 (+0200) Subject: [#4599] Checkpoint X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8bc8a006a4d6e45de78ffbdabe8b5a54108cb24e;p=thirdparty%2Fkea.git [#4599] Checkpoint --- diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index 3cb9532e99..7cca885e75 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -122,19 +122,28 @@ HAService::HAService(const unsigned int id, const IOServicePtr& io_service, // Fetch the TLS context. auto tls_context = config_->getThisServerConfig()->getTlsContext(); - // Instantiate the listener. - listener_.reset(new CmdHttpListener(server_address, my_url.getPort(), - listener_threads, tls_context)); + // Set the HTTP basic authentication. + HttpAuthConfigPtr auth_config; + // Wrong type: BasicHttpAuthPtr vs BasicHttpAuthConfigPtr + // auth_config = config_->getThisServerConfig()->getBasicAuth(); + // Set the command filter when enabled. + std::unordered_set command_accept_list; if (config_->getRestrictCommands()) { if (server_type == HAServerType::DHCPv4) { - CmdResponseCreator::command_accept_list_ = - CommandCreator::ha_commands4_; + command_accept_list = CommandCreator::ha_commands4_; } else { - CmdResponseCreator::command_accept_list_ = - CommandCreator::ha_commands6_; + command_accept_list = CommandCreator::ha_commands6_; } } + + // Instantiate the listener. + listener_.reset(new CmdHttpListener(server_address, + my_url.getPort(), + listener_threads, + tls_context, + auth_config, + command_accept_list)); } } diff --git a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc index c003a80a56..aa2e05071b 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_mt_unittest.cc @@ -127,7 +127,6 @@ public: HAMtServiceTest() : HATest() { MultiThreadingMgr::instance().setMode(true); - CmdResponseCreator::command_accept_list_.clear(); } /// @brief Destructor. @@ -136,7 +135,6 @@ public: ~HAMtServiceTest() { io_service_->stopAndPoll(); MultiThreadingMgr::instance().setMode(false); - CmdResponseCreator::command_accept_list_.clear(); } /// @brief Callback function invoke upon test timeout. @@ -197,8 +195,12 @@ TEST_F(HAMtServiceTest, multiThreadingBasics) { // Multi-threading should be enabled. ASSERT_TRUE(ha_config->get()->getEnableMultiThreading()); + // Authentication is disabled. + ASSERT_TRUE(service->listener_); + EXPECT_FALSE(service->listener_->getAuthConfig()); + // Command filtering is enabled. - EXPECT_FALSE(CmdResponseCreator::command_accept_list_.empty()); + EXPECT_FALSE(service->listener_->getCommandAcceptList().empty()); // Now we'll start, pause, resume and stop a few times. for (int i = 0; i < 3; ++i) { diff --git a/src/lib/config/cmd_http_listener.cc b/src/lib/config/cmd_http_listener.cc index bc35a68dd0..8753da9f5b 100644 --- a/src/lib/config/cmd_http_listener.cc +++ b/src/lib/config/cmd_http_listener.cc @@ -22,16 +22,20 @@ using namespace isc::config; using namespace isc::data; using namespace isc::http; using namespace isc::util; +using namespace std; namespace isc { namespace config { CmdHttpListener::CmdHttpListener(const IOAddress& address, const uint16_t port, const uint16_t thread_pool_size /* = 1 */, - TlsContextPtr context /* = () */) + TlsContextPtr context /* = () */, + HttpAuthConfigPtr http_auth_config /* = () */, + unordered_set command_accept_list /* = {} */) : address_(address), port_(port), thread_io_service_(), http_listener_(), thread_pool_size_(thread_pool_size), thread_pool_(), - tls_context_(context) { + tls_context_(context), http_auth_config_(http_auth_config), + command_accept_list_(command_accept_list) { } CmdHttpListener::~CmdHttpListener() { @@ -61,7 +65,9 @@ CmdHttpListener::start() { // Create the response creator factory first. It will be used to // generate response creators. Each response creator will be // used to generate the answer to specific request. - HttpResponseCreatorFactoryPtr rcf(new CmdResponseCreatorFactory()); + HttpResponseCreatorFactoryPtr + rcf(new CmdResponseCreatorFactory(http_auth_config_, + command_accept_list_)); // Create the HTTP listener. It will open up a TCP socket and be // prepared to accept incoming connections. diff --git a/src/lib/config/cmd_http_listener.h b/src/lib/config/cmd_http_listener.h index 4a87210290..5d639892b2 100644 --- a/src/lib/config/cmd_http_listener.h +++ b/src/lib/config/cmd_http_listener.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -34,7 +35,9 @@ public: /// @brief Constructor CmdHttpListener(const asiolink::IOAddress& address, const uint16_t port, const uint16_t thread_pool_size = 1, - asiolink::TlsContextPtr context = asiolink::TlsContextPtr()); + asiolink::TlsContextPtr context = asiolink::TlsContextPtr(), + http::HttpAuthConfigPtr http_auth_config = http::HttpAuthConfigPtr(), + std::unordered_set command_accept_list = {}); /// @brief Destructor virtual ~CmdHttpListener(); @@ -128,6 +131,20 @@ public: return (thread_io_service_); } + /// @brief Fetches the authentication configuration. + /// + /// @return the authentication configuration. + http::HttpAuthConfigPtr getAuthConfig() const { + return (http_auth_config_); + } + + /// @brief Fetches the command accept list. + /// + /// @return the command accept list. + std::unordered_set getCommandAcceptList() const { + return (command_accept_list_); + } + private: /// @brief IP address on which to listen. isc::asiolink::IOAddress address_; @@ -149,6 +166,12 @@ private: /// @brief The TLS context. asiolink::TlsContextPtr tls_context_; + + /// @brief The server current authentication configuration. + http::HttpAuthConfigPtr http_auth_config_; + + /// @brief The server command accept list. + std::unordered_set command_accept_list_; }; /// @brief Defines a shared pointer to CmdHttpListener. diff --git a/src/lib/config/cmd_response_creator.cc b/src/lib/config/cmd_response_creator.cc index adf54aa9b3..c45fab60a2 100644 --- a/src/lib/config/cmd_response_creator.cc +++ b/src/lib/config/cmd_response_creator.cc @@ -23,10 +23,6 @@ using namespace std; namespace isc { namespace config { -HttpAuthConfigPtr CmdResponseCreator::http_auth_config_; - -unordered_set CmdResponseCreator::command_accept_list_; - bool CmdResponseCreator::EMULATE_AGENT_RESPONSE = true; HttpRequestPtr diff --git a/src/lib/config/cmd_response_creator.h b/src/lib/config/cmd_response_creator.h index 70703afd9d..3b80ef5fea 100644 --- a/src/lib/config/cmd_response_creator.h +++ b/src/lib/config/cmd_response_creator.h @@ -32,6 +32,19 @@ namespace config { class CmdResponseCreator : public http::HttpResponseCreator { public: + /// @brief Constructor. + /// + /// @param http_auth_config Authentication configuration. + /// @param command_accept_list Command accept list. + CmdResponseCreator(http::HttpAuthConfigPtr http_auth_config, + std::unordered_set command_accept_list) : + http::HttpResponseCreator(), http_auth_config_(http_auth_config), + command_accept_list_(command_accept_list) { + } + + /// @brief virtual destructor. + virtual ~CmdResponseCreator() = default; + /// @brief Create a new request. /// /// This method creates a bare instance of the @ref @@ -64,22 +77,22 @@ public: const data::ConstElementPtr& body, const std::unordered_set& accept); - /// @brief The server current authentication configuration. - /// - /// Default to the empty HttpAuthConfigPtr. - /// - /// @note: This is currently not used, except in unit-tests. For the time being, - /// we postponed writing the corresponding code in the HA, so http_auth_config_ - /// is left to its empty default value. - static http::HttpAuthConfigPtr http_auth_config_; + /// @brief The emulate agent response flag. + static bool EMULATE_AGENT_RESPONSE; - /// @brief The server command accept list. + /// @brief Fetches the authentication configuration. /// - /// Default to the empty list which means to accept everything. - static std::unordered_set command_accept_list_; + /// @return the authentication configuration. + http::HttpAuthConfigPtr getAuthConfig() const { + return (http_auth_config_); + } - /// @brief The emulate agent response flag. - static bool EMULATE_AGENT_RESPONSE; + /// @brief Fetches the command accept list. + /// + /// @return the command accept list. + std::unordered_set getCommandAcceptList() const { + return (command_accept_list_); + } private: @@ -103,6 +116,12 @@ private: /// @return Pointer to an object representing HTTP response. virtual http::HttpResponsePtr createDynamicHttpResponse(http::HttpRequestPtr request); + + /// @brief The server current authentication configuration. + http::HttpAuthConfigPtr http_auth_config_; + + /// @brief The server command accept list. + std::unordered_set command_accept_list_; }; /// @brief Pointer to the @ref CmdResponseCreator. diff --git a/src/lib/config/cmd_response_creator_factory.h b/src/lib/config/cmd_response_creator_factory.h index d55d172d5c..b2845d9c81 100644 --- a/src/lib/config/cmd_response_creator_factory.h +++ b/src/lib/config/cmd_response_creator_factory.h @@ -30,7 +30,13 @@ public: /// /// Creates sole instance of the @ref CmdResponseCreator object /// returned by the @ref CmdResponseCreatorFactory::create. - CmdResponseCreatorFactory() : sole_creator_(new CmdResponseCreator) { + /// + /// @param http_auth_config Authentication configuration. + /// @param command_accept_list Command accept list. + CmdResponseCreatorFactory(http::HttpAuthConfigPtr http_auth_config, + std::unordered_set command_accept_list) + : sole_creator_(new CmdResponseCreator(http_auth_config, + command_accept_list)) { } /// @brief Returns an instance of the @ref CmdResponseCreator which diff --git a/src/lib/config/tests/cmd_response_creator_factory_unittests.cc b/src/lib/config/tests/cmd_response_creator_factory_unittests.cc index 38f7866609..3a656fa84d 100644 --- a/src/lib/config/tests/cmd_response_creator_factory_unittests.cc +++ b/src/lib/config/tests/cmd_response_creator_factory_unittests.cc @@ -12,6 +12,8 @@ #include using namespace isc::config; +using namespace isc::http; +using namespace std; namespace { @@ -19,7 +21,9 @@ namespace { // the create() method. TEST(CmdResponseCreatorFactory, createDefault) { // Create the factory. - CmdResponseCreatorFactory factory; + HttpAuthConfigPtr http_auth_config; + unordered_set command_accept_list; + CmdResponseCreatorFactory factory(http_auth_config, command_accept_list); // Create a response creator. CmdResponseCreatorPtr response1; @@ -31,10 +35,10 @@ TEST(CmdResponseCreatorFactory, createDefault) { EXPECT_TRUE(CmdResponseCreator::EMULATE_AGENT_RESPONSE); // Authorization configuration should be an empty pointer. - EXPECT_FALSE(CmdResponseCreator::http_auth_config_); + EXPECT_FALSE(response1->getAuthConfig()); // By default all commands are accepted. - EXPECT_TRUE(CmdResponseCreator::command_accept_list_.empty()); + EXPECT_TRUE(response1->getCommandAcceptList().empty()); // Invoke create() again. CmdResponseCreatorPtr response2; @@ -50,7 +54,9 @@ TEST(CmdResponseCreatorFactory, createDefault) { // be turned off. TEST(CmdResponseCreatorFactory, createAgentEmulationDisabled) { // Instantiate the factory. - CmdResponseCreatorFactory factory; + HttpAuthConfigPtr http_auth_config; + unordered_set command_accept_list; + CmdResponseCreatorFactory factory(http_auth_config, command_accept_list); // Disable agent emulation. CmdResponseCreator::EMULATE_AGENT_RESPONSE = false; @@ -65,10 +71,10 @@ TEST(CmdResponseCreatorFactory, createAgentEmulationDisabled) { EXPECT_FALSE(CmdResponseCreator::EMULATE_AGENT_RESPONSE); // Authorization configuration should be an empty pointer. - EXPECT_FALSE(CmdResponseCreator::http_auth_config_); + EXPECT_FALSE(response->getAuthConfig()); // By default all commands are accepted. - EXPECT_TRUE(CmdResponseCreator::command_accept_list_.empty()); + EXPECT_TRUE(response->getCommandAcceptList().empty()); } } // end of anonymous namespace diff --git a/src/lib/config/tests/cmd_response_creator_unittests.cc b/src/lib/config/tests/cmd_response_creator_unittests.cc index 80c7cf1d4a..6993b62bec 100644 --- a/src/lib/config/tests/cmd_response_creator_unittests.cc +++ b/src/lib/config/tests/cmd_response_creator_unittests.cc @@ -42,9 +42,6 @@ public: config::CommandMgr::instance(). registerCommand("foo", std::bind(&CmdResponseCreatorTest::fooCommandHandler, this, ph::_1, ph::_2)); - // Clear class variables. - CmdResponseCreator::http_auth_config_.reset(); - CmdResponseCreator::command_accept_list_.clear(); } /// @brief Destructor. @@ -52,8 +49,6 @@ public: /// Removes registered commands from the command manager. virtual ~CmdResponseCreatorTest() { config::CommandMgr::instance().deregisterAll(); - CmdResponseCreator::http_auth_config_.reset(); - CmdResponseCreator::command_accept_list_.clear(); } /// @brief SetUp function that wraps call to initCreator. @@ -66,10 +61,15 @@ public: /// @brief Creates a new CmdResponseCreator and new HttpRequest. /// /// @param emulate_agent_flag enables/disables agent response emulation + /// @param http_auth_config authentication configuration. + /// @param command_accept_list command accept list. /// in the CmdResponsCreator. - void initCreator(bool emulate_agent_flag = true) { - response_creator_.reset(new CmdResponseCreator); + void initCreator(bool emulate_agent_flag = true, + HttpAuthConfigPtr http_auth_config = HttpAuthConfigPtr(), + unordered_set command_accept_list = {}) { CmdResponseCreator::EMULATE_AGENT_RESPONSE = emulate_agent_flag; + response_creator_.reset(new CmdResponseCreator(http_auth_config, + command_accept_list)); request_ = response_creator_->createNewHttpRequest(); ASSERT_TRUE(request_) << "initCreator failed to create request"; } @@ -307,7 +307,12 @@ TEST_F(CmdResponseCreatorTest, filterCommand) { // This test verifies basic HTTP authentication - reject case. // Empty case was handled in createDynamicHttpResponseNoEmulation. TEST_F(CmdResponseCreatorTest, basicAuthReject) { - initCreator(false); + // Create basic HTTP authentication configuration. + BasicHttpAuthConfigPtr basic(new BasicHttpAuthConfig); + ASSERT_TRUE(basic); + EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); + + initCreator(false, basic); setBasicContext(request_); // Body: "foo" command has been registered in the test fixture constructor. @@ -318,14 +323,6 @@ TEST_F(CmdResponseCreatorTest, basicAuthReject) { // All requests must be finalized before they can be processed. ASSERT_NO_THROW(request_->finalize()); - // Create basic HTTP authentication configuration. - CmdResponseCreator::http_auth_config_.reset(new BasicHttpAuthConfig()); - BasicHttpAuthConfigPtr basic = - boost::dynamic_pointer_cast( - CmdResponseCreator::http_auth_config_); - ASSERT_TRUE(basic); - EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); - // Create response from the request. HttpResponsePtr response; ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); @@ -338,7 +335,12 @@ TEST_F(CmdResponseCreatorTest, basicAuthReject) { // This test verifies basic HTTP authentication - accept case. // Empty case was handled in createDynamicHttpResponseNoEmulation. TEST_F(CmdResponseCreatorTest, basicAuthAccept) { - initCreator(false); + // Create basic HTTP authentication configuration. + BasicHttpAuthConfigPtr basic(new BasicHttpAuthConfig); + ASSERT_TRUE(basic); + EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); + + initCreator(false, basic); setBasicContext(request_); // Body: "foo" command has been registered in the test fixture constructor. @@ -351,14 +353,6 @@ TEST_F(CmdResponseCreatorTest, basicAuthAccept) { // All requests must be finalized before they can be processed. ASSERT_NO_THROW(request_->finalize()); - // Create basic HTTP authentication configuration. - CmdResponseCreator::http_auth_config_.reset(new BasicHttpAuthConfig()); - BasicHttpAuthConfigPtr basic = - boost::dynamic_pointer_cast( - CmdResponseCreator::http_auth_config_); - ASSERT_TRUE(basic); - EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); - // Create response from the request. HttpResponsePtr response; ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); @@ -380,7 +374,10 @@ TEST_F(CmdResponseCreatorTest, basicAuthAccept) { // This test verifies command filtering at the HTTP level - reject case. TEST_F(CmdResponseCreatorTest, filterCommandReject) { - initCreator(false); + // Add foo in the access list. + unordered_set accept; + accept.insert("foo"); + initCreator(false, 0, accept); setBasicContext(request_); // For the log message... request_->setRemote("127.0.0.1"); @@ -391,9 +388,6 @@ TEST_F(CmdResponseCreatorTest, filterCommandReject) { // All requests must be finalized before they can be processed. ASSERT_NO_THROW(request_->finalize()); - // Add foo in the access list. - CmdResponseCreator::command_accept_list_.insert("foo"); - // Create response from the request. HttpResponsePtr response; ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); @@ -405,7 +399,10 @@ TEST_F(CmdResponseCreatorTest, filterCommandReject) { // This test verifies command filtering at the HTTP level - accept case. TEST_F(CmdResponseCreatorTest, filterCommandAccept) { - initCreator(false); + // Add foo in the access list. + unordered_set accept; + accept.insert("foo"); + initCreator(false, 0, accept); setBasicContext(request_); // Body: "foo" command has been registered in the test fixture constructor. @@ -414,9 +411,6 @@ TEST_F(CmdResponseCreatorTest, filterCommandAccept) { // All requests must be finalized before they can be processed. ASSERT_NO_THROW(request_->finalize()); - // Add foo in the access list. - CmdResponseCreator::command_accept_list_.insert("foo"); - // Create response from the request. HttpResponsePtr response; ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); diff --git a/src/lib/config/tests/http_command_mgr_unittests.cc b/src/lib/config/tests/http_command_mgr_unittests.cc index 691483061c..05a5a3d6c4 100644 --- a/src/lib/config/tests/http_command_mgr_unittests.cc +++ b/src/lib/config/tests/http_command_mgr_unittests.cc @@ -47,7 +47,7 @@ const unsigned short SERVER_PORT = 18123; /// @brief Test timeout (ms). const long TEST_TIMEOUT = 10000; -/// @brief Test fixture class for @ref CmdHttpListener. +/// @brief Test fixture class for @ref HttpCommandMgr. class HttpCommandMgrTest : public ::testing::Test { public: