From 3e40ed58fcf7bbd3bdec68dbfee708a5480b8002 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 30 Apr 2025 15:23:11 -0400 Subject: [PATCH] [#3838] Backport of 3830 to v2_6 On branch 3838-CVE-2025-32801-backport-to-v2_6 modified: doc/sphinx/arm/hooks.rst modified: src/bin/agent/tests/ca_cfg_mgr_unittests.cc modified: src/bin/agent/tests/ca_process_tests.sh.in modified: src/bin/agent/tests/get_config_unittest.cc modified: src/bin/agent/tests/test_callout_libraries.h.in modified: src/bin/d2/tests/d2_cfg_mgr_unittests.cc modified: src/bin/d2/tests/d2_process_tests.sh.in modified: src/bin/d2/tests/d2_process_unittests.cc modified: src/bin/d2/tests/get_config_unittest.cc modified: src/bin/d2/tests/test_callout_libraries.h.in modified: src/bin/d2/tests/test_configured_libraries.h.in modified: src/bin/dhcp4/tests/config_parser_unittest.cc modified: src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc modified: src/bin/dhcp4/tests/dhcp4_process_tests.sh.in modified: src/bin/dhcp4/tests/hooks_unittest.cc modified: src/bin/dhcp4/tests/test_libraries.h.in modified: src/bin/dhcp6/tests/config_parser_unittest.cc modified: src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc modified: src/bin/dhcp6/tests/dhcp6_process_tests.sh.in modified: src/bin/dhcp6/tests/hooks_unittest.cc modified: src/bin/dhcp6/tests/test_libraries.h.in modified: src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc modified: src/lib/dhcpsrv/tests/test_libraries.h.in modified: src/lib/hooks/Makefile.am modified: src/lib/hooks/hooks_parser.cc modified: src/lib/hooks/hooks_parser.h modified: src/lib/hooks/tests/Makefile.am modified: src/lib/hooks/tests/hooks_manager_unittest.cc modified: src/lib/util/filesystem.cc modified: src/lib/util/filesystem.h modified: src/lib/util/tests/filesystem_unittests.cc --- doc/sphinx/arm/hooks.rst | 40 +++++ src/bin/agent/tests/ca_cfg_mgr_unittests.cc | 27 ++- src/bin/agent/tests/ca_process_tests.sh.in | 2 + src/bin/agent/tests/get_config_unittest.cc | 18 ++ .../agent/tests/test_callout_libraries.h.in | 3 + src/bin/d2/tests/d2_cfg_mgr_unittests.cc | 18 ++ src/bin/d2/tests/d2_process_tests.sh.in | 2 + src/bin/d2/tests/d2_process_unittests.cc | 20 ++- src/bin/d2/tests/get_config_unittest.cc | 18 ++ src/bin/d2/tests/test_callout_libraries.h.in | 3 + .../d2/tests/test_configured_libraries.h.in | 3 + src/bin/dhcp4/tests/config_parser_unittest.cc | 22 ++- .../dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 1 - src/bin/dhcp4/tests/dhcp4_process_tests.sh.in | 4 + src/bin/dhcp4/tests/hooks_unittest.cc | 19 ++ src/bin/dhcp4/tests/test_libraries.h.in | 3 + src/bin/dhcp6/tests/config_parser_unittest.cc | 20 +++ .../dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 1 - src/bin/dhcp6/tests/dhcp6_process_tests.sh.in | 3 + src/bin/dhcp6/tests/hooks_unittest.cc | 20 ++- src/bin/dhcp6/tests/test_libraries.h.in | 3 + .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 33 +++- src/lib/dhcpsrv/tests/test_libraries.h.in | 4 + src/lib/hooks/Makefile.am | 1 + src/lib/hooks/hooks_parser.cc | 42 +++-- src/lib/hooks/hooks_parser.h | 27 +++ src/lib/hooks/tests/Makefile.am | 1 + src/lib/hooks/tests/hooks_manager_unittest.cc | 169 +++++++++++++++++- src/lib/util/filesystem.cc | 113 ++++++++++-- src/lib/util/filesystem.h | 124 +++++++++---- src/lib/util/tests/filesystem_unittests.cc | 157 +++++++++++++++- 31 files changed, 847 insertions(+), 74 deletions(-) diff --git a/doc/sphinx/arm/hooks.rst b/doc/sphinx/arm/hooks.rst index 43ca9c12c3..18c757fc25 100644 --- a/doc/sphinx/arm/hooks.rst +++ b/doc/sphinx/arm/hooks.rst @@ -213,6 +213,46 @@ configuration would be: because the parameters specified for the library (or the files those parameters point to) may have changed. +As of Kea 2.6.2, hook libraries may only be loaded from the default installation +directory determined during compilation and shown in the config report as +"Hooks directory". This value may be overridden at startup by setting the +environment variable ``KEA_HOOKS_PATH`` to the desired path. If a path other +than this value is used in a ``library`` element Kea will emit an error and refuse +to load the library. For ease of use ``library`` elements may simply omit path +components, specifying the file name only as shown below: + +.. code-block:: json + + { + "Dhcp6": { + "hooks-libraries": [ + { + "library": "first_custom_hooks_example.so" + }, + { + "library": "second_custom_hooks_example.so" + } + ] + } + } + +This snippet (on Debian 12) is equivalent to: + +.. code-block:: json + + { + "Dhcp6": { + "hooks-libraries": [ + { + "library": "/usr/lib/x86_64-linux-gnu/kea/hooks/first_custom_hooks_example.so" + }, + { + "library": "/usr/lib/x86_64-linux-gnu/kea/hooks/second_custom_hooks_example.so" + } + ] + } + } + Libraries may have additional parameters that are not mandatory, in the sense that there may be libraries that do not require them. However, for any given library there is often a requirement to specify a certain diff --git a/src/bin/agent/tests/ca_cfg_mgr_unittests.cc b/src/bin/agent/tests/ca_cfg_mgr_unittests.cc index a7b40c5567..20263d74b4 100644 --- a/src/bin/agent/tests/ca_cfg_mgr_unittests.cc +++ b/src/bin/agent/tests/ca_cfg_mgr_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2022 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2025 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -423,6 +424,28 @@ const char* AGENT_CONFIGS[] = { /// @brief Class used for testing CfgMgr class AgentParserTest : public isc::process::ConfigParseTest { public: + /// @brief Constructor. + AgentParserTest() { + resetHooksPath(); + } + + /// @brief Destructor. + virtual ~AgentParserTest() { + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : CA_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); + } /// @brief Tries to load input text as a configuration /// @@ -544,6 +567,8 @@ TEST_F(AgentParserTest, configParse3Sockets) { // name. In particular, it checks if such a library exists. Therefore we // can't use AGENT_CONFIGS[4] as is, but need to run it through path replacer. TEST_F(AgentParserTest, configParseHooks) { + setHooksTestPath(); + // Create the configuration with proper lib path. std::string cfg = pathReplacer(AGENT_CONFIGS[4], CALLOUT_LIBRARY); // The configuration should be successful. diff --git a/src/bin/agent/tests/ca_process_tests.sh.in b/src/bin/agent/tests/ca_process_tests.sh.in index 23751ed860..ab7e47b23d 100644 --- a/src/bin/agent/tests/ca_process_tests.sh.in +++ b/src/bin/agent/tests/ca_process_tests.sh.in @@ -18,6 +18,8 @@ set -eu CFG_FILE="@abs_top_builddir@/src/bin/agent/tests/test_config.json" # Path to the Control Agent log file. LOG_FILE="@abs_top_builddir@/src/bin/agent/tests/test.log" +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH +export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/agent/tests/.libs" # Control Agent configuration to be stored in the configuration file. CONFIG="{ diff --git a/src/bin/agent/tests/get_config_unittest.cc b/src/bin/agent/tests/get_config_unittest.cc index 40d9c502f9..6f8cb93764 100644 --- a/src/bin/agent/tests/get_config_unittest.cc +++ b/src/bin/agent/tests/get_config_unittest.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -27,6 +28,7 @@ using namespace isc::agent; using namespace isc::config; using namespace isc::data; using namespace isc::process; +using namespace isc::hooks; using namespace isc::test; namespace { @@ -134,6 +136,7 @@ class CtrlAgentGetCfgTest : public ConfigParseTest { public: CtrlAgentGetCfgTest() : rcode_(-1) { + resetHooksPath(); srv_.reset(new NakedAgentCfgMgr()); // Create fresh context. resetConfiguration(); @@ -141,6 +144,20 @@ public: ~CtrlAgentGetCfgTest() { resetConfiguration(); + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : CA_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); } /// @brief Parse and Execute configuration @@ -245,6 +262,7 @@ public: /// Test a configuration TEST_F(CtrlAgentGetCfgTest, simple) { + setHooksTestPath(); // get the simple configuration std::string simple_file = string(CFG_EXAMPLES) + "/" + "simple.json"; diff --git a/src/bin/agent/tests/test_callout_libraries.h.in b/src/bin/agent/tests/test_callout_libraries.h.in index 78f51c8815..7e888ba732 100644 --- a/src/bin/agent/tests/test_callout_libraries.h.in +++ b/src/bin/agent/tests/test_callout_libraries.h.in @@ -19,6 +19,9 @@ namespace { // Basic callout library with context_create and three "standard" callouts. static const char* CALLOUT_LIBRARY = "@abs_builddir@/.libs/libcallout.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* CA_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + } // anonymous namespace #endif // TEST_LIBRARIES_H diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index 12c95a7f5b..8d9fe15c1f 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -47,10 +48,25 @@ public: /// @brief Constructor D2CfgMgrTest():cfg_mgr_(new D2CfgMgr()), d2_params_() { + resetHooksPath(); } /// @brief Destructor ~D2CfgMgrTest() { + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : D2_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); } /// @brief Configuration manager instance. @@ -460,6 +476,8 @@ TEST(D2CfgMgr, construction) { /// as it would be done by d2_process in response to a configuration update /// event. TEST_F(D2CfgMgrTest, fullConfig) { + setHooksTestPath(); + // Create a configuration with all of application level parameters, plus // both the forward and reverse ddns managers. Both managers have two // domains with three servers per domain. diff --git a/src/bin/d2/tests/d2_process_tests.sh.in b/src/bin/d2/tests/d2_process_tests.sh.in index 65e321f487..cf9de463a3 100644 --- a/src/bin/d2/tests/d2_process_tests.sh.in +++ b/src/bin/d2/tests/d2_process_tests.sh.in @@ -15,6 +15,8 @@ CFG_FILE="@abs_top_builddir@/src/bin/d2/tests/test_config.json" # Path to the D2 log file. LOG_FILE="@abs_top_builddir@/src/bin/d2/tests/test.log" # D2 configuration to be stored in the configuration file. +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH +export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/d2/tests/.libs" CONFIG="{ \"DhcpDdns\": { diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index b4fb7ae907..c5e3c95810 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2024 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2025 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -67,12 +68,27 @@ public: D2ProcessTest() : D2Process("d2test", asiolink::IOServicePtr(new isc::asiolink::IOService())) { + resetHooksPath(); HooksManager::setTestMode(false); D2Controller::instance(); } /// @brief Destructor virtual ~D2ProcessTest() { + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : D2_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); } /// @brief Callback that will invoke shutdown method. @@ -661,6 +677,7 @@ TEST_F(D2ProcessTest, v6LoopbackTest) { /// @brief Check the configured callout (positive case). TEST_F(D2ProcessTest, configuredNoFail) { + setHooksTestPath(); const char* config = "{\n" "\"hooks-libraries\": [ {\n" " \"library\": \"%LIBRARY%\",\n" @@ -680,6 +697,7 @@ TEST_F(D2ProcessTest, configuredNoFail) { /// @brief Check the configured callout (negative case). TEST_F(D2ProcessTest, configuredFail) { + setHooksTestPath(); const char* config = "{\n" "\"user-context\": { \"error\": \"Fail!\" },\n" "\"hooks-libraries\": [ {\n" diff --git a/src/bin/d2/tests/get_config_unittest.cc b/src/bin/d2/tests/get_config_unittest.cc index 223aebdb69..6eee45d066 100644 --- a/src/bin/d2/tests/get_config_unittest.cc +++ b/src/bin/d2/tests/get_config_unittest.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +29,7 @@ using namespace isc::d2; using namespace isc::data; using namespace isc::process; using namespace isc::test; +using namespace isc::hooks; namespace { @@ -139,6 +141,7 @@ class D2GetConfigTest : public ConfigParseTest { public: D2GetConfigTest() : rcode_(-1) { + resetHooksPath(); srv_.reset(new D2CfgMgr()); // Enforce not verbose mode. Daemon::setVerbose(false); @@ -151,6 +154,20 @@ public: ~D2GetConfigTest() { static_cast(remove(test_file_name.c_str())); resetConfiguration(); + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : D2_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); } /// @brief Parse and Execute configuration @@ -261,6 +278,7 @@ public: /// Test a configuration TEST_F(D2GetConfigTest, sample1) { + setHooksTestPath(); // get the sample1 configuration std::string sample1_file = string(CFG_EXAMPLES) + "/" + "sample1.json"; diff --git a/src/bin/d2/tests/test_callout_libraries.h.in b/src/bin/d2/tests/test_callout_libraries.h.in index a88d131011..82602dfcc9 100644 --- a/src/bin/d2/tests/test_callout_libraries.h.in +++ b/src/bin/d2/tests/test_callout_libraries.h.in @@ -19,6 +19,9 @@ namespace { // Basic callout library with context_create and three "standard" callouts. static const char* CALLOUT_LIBRARY = "@abs_builddir@/.libs/libcallout.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* D2_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + } // anonymous namespace #endif // D2_TEST_CALLOUT_LIBRARIES_H diff --git a/src/bin/d2/tests/test_configured_libraries.h.in b/src/bin/d2/tests/test_configured_libraries.h.in index 2b235ae68f..9feb0bb341 100644 --- a/src/bin/d2/tests/test_configured_libraries.h.in +++ b/src/bin/d2/tests/test_configured_libraries.h.in @@ -21,6 +21,9 @@ namespace { // content of the error entry. static const char* CONFIGURED_LIBRARY = "@abs_builddir@/.libs/libconfigured.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* D2_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + } // anonymous namespace #endif // D2_TEST_CONFIGURED_LIBRARIES_H diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index b8cd32aabb..800fd4cd4a 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -311,6 +312,8 @@ public: } // Reset configuration for each test. resetConfiguration(); + + resetHooksPath(); } ~Dhcp4ParserTest() { @@ -320,7 +323,22 @@ public: // ... and delete the hooks library marker files if present static_cast(remove(LOAD_MARKER_FILE)); static_cast(remove(UNLOAD_MARKER_FILE)); - }; + + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : DHCP4_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); + } // Checks if the result of DHCP server configuration has // expected code (0 for success, other for failures). @@ -4440,6 +4458,8 @@ TEST_F(Dhcp4ParserTest, InvalidLibrary) { // Verify the configuration of hooks libraries with two being specified. TEST_F(Dhcp4ParserTest, LibrariesSpecified) { + setHooksTestPath(); + // Marker files should not be present. EXPECT_FALSE(checkMarkerFileExists(LOAD_MARKER_FILE)); EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE)); diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 64769cc054..187afde3c8 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -30,7 +30,6 @@ #include #include "marker_file.h" -#include "test_libraries.h" #include #include diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 4680da8476..8d7cb45df9 100644 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -23,6 +23,10 @@ HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" # Path to test hooks library HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco4.so" # Kea configuration to be stored in the configuration file. + +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH +export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs" + CONFIG="{ \"Dhcp4\": { diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index d0f7a4a616..ec512b01c2 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -1200,6 +1201,7 @@ public: LoadUnloadDhcpv4SrvTest() { reset(); MultiThreadingMgr::instance().setMode(false); + resetHooksPath(); } /// @brief Destructor @@ -1207,6 +1209,7 @@ public: server_.reset(); reset(); MultiThreadingMgr::instance().setMode(false); + resetHooksPath(); }; /// @brief Reset hooks data @@ -1224,6 +1227,19 @@ public: CfgMgr::instance().clear(); } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : DHCP4_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); + } }; // Checks if callouts installed on buffer4_receive are indeed called and the @@ -3435,6 +3451,7 @@ TEST_F(LoadUnloadDhcpv4SrvTest, failLoadIncompatibleLibraries) { // Checks if callouts installed on the dhcp4_srv_configured ared indeed called // and all the necessary parameters are passed. TEST_F(LoadUnloadDhcpv4SrvTest, Dhcpv4SrvConfigured) { + setHooksTestPath(); for (auto const& parameters : vector{ "", R"(, "parameters": { "mode": "fail-without-error" } )", @@ -4094,6 +4111,8 @@ TEST_F(HooksDhcpv4SrvTest, lease4OfferDiscoverDecline) { // Checks that postponed hook start service can fail. TEST_F(LoadUnloadDhcpv4SrvTest, startServiceFail) { + setHooksTestPath(); + boost::shared_ptr srv(new ControlledDhcpv4Srv(0)); // Ensure no marker files to start with. diff --git a/src/bin/dhcp4/tests/test_libraries.h.in b/src/bin/dhcp4/tests/test_libraries.h.in index af659f7e89..4c0f15c79a 100644 --- a/src/bin/dhcp4/tests/test_libraries.h.in +++ b/src/bin/dhcp4/tests/test_libraries.h.in @@ -24,6 +24,9 @@ const char* const CALLOUT_LIBRARY_2 = "@abs_builddir@/.libs/libco2.so"; const char* const CALLOUT_LIBRARY_3 = "@abs_builddir@/.libs/libco3.so"; const char* const CALLOUT_LIBRARY_4 = "@abs_builddir@/.libs/libco4.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* DHCP4_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + // Name of a library which is not present. const char* const NOT_PRESENT_LIBRARY = "@abs_builddir@/.libs/libnothere.so"; diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 17f54f5d9e..dfbe6175cf 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -399,6 +400,8 @@ public: } // Reset configuration for each test. resetConfiguration(); + + resetHooksPath(); } ~Dhcp6ParserTest() { @@ -408,8 +411,23 @@ public: // ... and delete the hooks library marker files if present static_cast(remove(LOAD_MARKER_FILE)); static_cast(remove(UNLOAD_MARKER_FILE)); + + resetHooksPath(); }; + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : DHCP6_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); + } + // Checks if the result of DHCP server configuration has // expected code (0 for success, other for failures). // Also stores result in rcode_ and comment_. @@ -4838,6 +4856,8 @@ TEST_F(Dhcp6ParserTest, InvalidLibrary) { // Verify the configuration of hooks libraries with two being specified. TEST_F(Dhcp6ParserTest, LibrariesSpecified) { + setHooksTestPath(); + // Marker files should not be present. EXPECT_FALSE(checkMarkerFileExists(LOAD_MARKER_FILE)); EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE)); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 7d28a6e9a1..6e7103bee8 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -28,7 +28,6 @@ #include #include "marker_file.h" -#include "test_libraries.h" #include #include diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index 2896804117..41dfef6332 100644 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -23,6 +23,9 @@ HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" # Path to test hooks library HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco4.so" # Kea configuration to be stored in the configuration file. +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH +export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs" + CONFIG="{ \"Dhcp6\": { diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 6b6170a555..b262409140 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1100,6 +1101,7 @@ public: LoadUnloadDhcpv6SrvTest() : Dhcpv6SrvTest() { reset(); MultiThreadingMgr::instance().setMode(false); + resetHooksPath(); } /// @brief Destructor @@ -1107,7 +1109,8 @@ public: server_.reset(); reset(); MultiThreadingMgr::instance().setMode(false); - }; + resetHooksPath(); + } /// @brief Reset hooks data /// @@ -1124,6 +1127,19 @@ public: CfgMgr::instance().clear(); } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : DHCP6_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); + } }; // Checks if callouts installed on buffer6_receive are indeed called and the @@ -5660,6 +5676,7 @@ TEST_F(LoadUnloadDhcpv6SrvTest, failLoadIncompatibleLibraries) { // Checks if callouts installed on the dhcp6_srv_configured ared indeed called // and all the necessary parameters are passed. TEST_F(LoadUnloadDhcpv6SrvTest, Dhcpv6SrvConfigured) { + setHooksTestPath(); for (auto const& parameters : vector{ "", R"(, "parameters": { "mode": "fail-without-error" } )", @@ -5887,6 +5904,7 @@ TEST_F(HooksDhcpv6SrvTest, leases6ParkedPacketLimit) { // Checks that postponed hook start service can fail. TEST_F(LoadUnloadDhcpv6SrvTest, startServiceFail) { + setHooksTestPath(); boost::shared_ptr srv(new ControlledDhcpv6Srv(0)); // Ensure no marker files to start with. diff --git a/src/bin/dhcp6/tests/test_libraries.h.in b/src/bin/dhcp6/tests/test_libraries.h.in index 09e8a62820..c18f3606b9 100644 --- a/src/bin/dhcp6/tests/test_libraries.h.in +++ b/src/bin/dhcp6/tests/test_libraries.h.in @@ -25,6 +25,9 @@ const char* const CALLOUT_LIBRARY_4 = "@abs_builddir@/.libs/libco4.so"; // Name of a library which is not present. const char* const NOT_PRESENT_LIBRARY = "@abs_builddir@/.libs/libnothere.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* DHCP6_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + } // anonymous namespace diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index c11891bb23..8e0bdb3e0c 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -170,11 +170,26 @@ public: ParseConfigTest() :family_(AF_INET6) { reset_context(); + resetHooksPath(); } ~ParseConfigTest() { reset_context(); CfgMgr::instance().clear(); + resetHooksPath(); + } + + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param custom_path path to use as the hooks path. + void setHooksTestPath(const std::string explicit_path = "") { + HooksLibrariesParser::getHooksPath(true, + (!explicit_path.empty() ? + explicit_path : DHCPSRV_HOOKS_TEST_PATH)); + } + + /// @brief Resets the hooks path to DEFAULT_HOOKS_PATH. + void resetHooksPath() { + HooksLibrariesParser::getHooksPath(true); } /// @brief Parses a configuration. @@ -1914,6 +1929,7 @@ TEST_F(ParseConfigTest, noHooksLibraries) { // hooks-libraries element that contains a single library. TEST_F(ParseConfigTest, oneHooksLibrary) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -1947,6 +1963,7 @@ TEST_F(ParseConfigTest, oneHooksLibrary) { // hooks-libraries element that contains two libraries TEST_F(ParseConfigTest, twoHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -1983,6 +2000,7 @@ TEST_F(ParseConfigTest, twoHooksLibraries) { // Configure with two libraries, then reconfigure with the same libraries. TEST_F(ParseConfigTest, reconfigureSameHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -2034,6 +2052,7 @@ TEST_F(ParseConfigTest, reconfigureSameHooksLibraries) { // Configure the hooks with two libraries, then reconfigure with the same // libraries, but in reverse order. TEST_F(ParseConfigTest, reconfigureReverseHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -2071,6 +2090,7 @@ TEST_F(ParseConfigTest, reconfigureReverseHooksLibraries) { // Configure the hooks with two libraries, then reconfigure with // no libraries. TEST_F(ParseConfigTest, reconfigureZeroHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -2111,6 +2131,7 @@ TEST_F(ParseConfigTest, reconfigureZeroHooksLibraries) { // Check with a set of libraries, some of which are invalid. TEST_F(ParseConfigTest, invalidHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -2145,6 +2166,7 @@ TEST_F(ParseConfigTest, invalidHooksLibraries) { // Check that trying to reconfigure with an invalid set of libraries fails. TEST_F(ParseConfigTest, reconfigureInvalidHooksLibraries) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); @@ -2189,7 +2211,7 @@ TEST_F(ParseConfigTest, reconfigureInvalidHooksLibraries) { // Check that if hooks-libraries contains invalid syntax, it is detected. TEST_F(ParseConfigTest, invalidSyntaxHooksLibraries) { - + setHooksTestPath("/opt/lib"); // Element holds a mixture of (valid) maps and non-maps. string config1 = "{ \"hooks-libraries\": [ " "{ \"library\": \"/opt/lib/lib1\" }, " @@ -2223,7 +2245,8 @@ TEST_F(ParseConfigTest, invalidSyntaxHooksLibraries) { "{ \"library\": \"/opt/lib/lib1\" }, " "{ \"library\": \"\" } " "] }"; - string error3 = "value of 'library' element must not be blank"; + string error3 = "Configuration parsing failed: hooks library configuration error:" + " path: '' has no filename (:1:69)"; rcode = parseConfiguration(config3); ASSERT_NE(0, rcode); @@ -2236,11 +2259,12 @@ TEST_F(ParseConfigTest, invalidSyntaxHooksLibraries) { "{ \"library\": \"/opt/lib/lib1\" }, " "{ \"library\": \" \" } " "] }"; - string error4 = "value of 'library' element must not be blank"; + string error4 = "Configuration parsing failed: hooks library configuration error:" + " path: '' has no filename (:1:69)"; rcode = parseConfiguration(config4); ASSERT_NE(0, rcode); - EXPECT_TRUE(error_text_.find(error3) != string::npos) << + EXPECT_TRUE(error_text_.find(error4) != string::npos) << "Error text returned from parse failure is " << error_text_; // Element holds valid maps, except one that does not contain a @@ -2261,6 +2285,7 @@ TEST_F(ParseConfigTest, invalidSyntaxHooksLibraries) { // Check that some parameters may have configuration parameters configured. TEST_F(ParseConfigTest, HooksLibrariesParameters) { + setHooksTestPath(); // Check that no libraries are currently loaded vector hooks_libraries = HooksManager::getLibraryNames(); EXPECT_TRUE(hooks_libraries.empty()); diff --git a/src/lib/dhcpsrv/tests/test_libraries.h.in b/src/lib/dhcpsrv/tests/test_libraries.h.in index 5a5545eae6..ef1b594351 100644 --- a/src/lib/dhcpsrv/tests/test_libraries.h.in +++ b/src/lib/dhcpsrv/tests/test_libraries.h.in @@ -29,6 +29,10 @@ static const char* CALLOUT_PARAMS_LIBRARY = "@abs_builddir@/.libs/libco3.so"; // Name of a library which is not present. static const char* NOT_PRESENT_LIBRARY = "@abs_builddir@/.libs/libnothere.so"; +// Test path to use for in place of DEFAULT_HOOKS_PATH +static const char* DHCPSRV_HOOKS_TEST_PATH = "@abs_builddir@/.libs"; + + } // anonymous namespace diff --git a/src/lib/hooks/Makefile.am b/src/lib/hooks/Makefile.am index 5782e5fe70..9d2d8cb594 100644 --- a/src/lib/hooks/Makefile.am +++ b/src/lib/hooks/Makefile.am @@ -1,6 +1,7 @@ SUBDIRS = . tests AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib +AM_CPPFLAGS += -DDEFAULT_HOOKS_PATH=\"$(libdir)/kea/hooks\" AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/lib/hooks/hooks_parser.cc b/src/lib/hooks/hooks_parser.cc index 56029896fe..2a65ce45e9 100644 --- a/src/lib/hooks/hooks_parser.cc +++ b/src/lib/hooks/hooks_parser.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -17,10 +18,27 @@ using namespace std; using namespace isc::data; using namespace isc::hooks; using namespace isc::dhcp; +using namespace isc::util::file; namespace isc { namespace hooks { +std::string +HooksLibrariesParser::getHooksPath(bool reset /* = false */, const std::string explicit_path /* = "" */) { + static std::string default_hooks_path = ""; + if (default_hooks_path.empty() || reset) { + if (explicit_path.empty()) { + default_hooks_path = std::string(std::getenv("KEA_HOOKS_PATH") ? + std::getenv("KEA_HOOKS_PATH") + : DEFAULT_HOOKS_PATH); + } else { + default_hooks_path = explicit_path; + } + } + + return (default_hooks_path); +} + // @todo use the flat style, split into list and item void @@ -63,18 +81,13 @@ HooksLibrariesParser::parse(HooksConfig& libraries, ConstElementPtr value) { } // Get the name of the library and add it to the list after - // removing quotes. - libname = (entry_item.second)->stringValue(); - - // Remove leading/trailing quotes and any leading/trailing - // spaces. - boost::erase_all(libname, "\""); - libname = isc::util::str::trim(libname); - if (libname.empty()) { + // validating it. + try { + libname = validatePath((entry_item.second)->stringValue()); + } catch (const std::exception& ex) { isc_throw(DhcpConfigError, "hooks library configuration" - " error: value of 'library' element must not be" - " blank (" << - entry_item.second->getPosition() << ")"); + " error: " << ex.what() << " (" + << entry_item.second->getPosition() << ")"); } // Note we have found the library name. @@ -105,5 +118,12 @@ HooksLibrariesParser::parse(HooksConfig& libraries, ConstElementPtr value) { } } +std::string +HooksLibrariesParser::validatePath(const std::string libpath, + bool enforce_path /* = true */) { + return (FileManager::validatePath(HooksLibrariesParser::getHooksPath(), + libpath, enforce_path)); +} + } } diff --git a/src/lib/hooks/hooks_parser.h b/src/lib/hooks/hooks_parser.h index 4bec9a6aff..6ea0173bf2 100644 --- a/src/lib/hooks/hooks_parser.h +++ b/src/lib/hooks/hooks_parser.h @@ -57,6 +57,33 @@ public: /// @param libraries parsed libraries information will be stored here /// @param value pointer to the content to be parsed void parse(HooksConfig& libraries, isc::data::ConstElementPtr value); + + /// @brief Validates a library path against the supported path for + /// hooks libraries. + /// + /// @param libpath library path to validate. + /// @param enforce_path enables validation against the supported path. + /// If false verifies only that the path contains a file name. + /// + /// @return validated path + static std::string validatePath(const std::string libpath, + bool enforce_path = true); + + /// @brief Fetches the supported Hooks path. + /// + /// The first call to this function with no arguments will set the default + /// hooks path to either the value of DEFAULT_HOOKS_PATH or the environment + /// variable KEA_HOOKS_PATH if it is defined. Subsequent calls with no + /// arguments will simply return this value. + /// + /// @param reset recalculate when true, defaults to false. This is for + /// testing purposes only. + /// @param explicit_path set default hooks path to this value. This is + /// for testing purposes only. + /// + /// @return String containing the default hooks path. + static std::string getHooksPath(bool reset = false, + const std::string explicit_path = ""); }; }; // namespace isc::hooks diff --git a/src/lib/hooks/tests/Makefile.am b/src/lib/hooks/tests/Makefile.am index 8cbe4138fa..803f53876b 100644 --- a/src/lib/hooks/tests/Makefile.am +++ b/src/lib/hooks/tests/Makefile.am @@ -1,6 +1,7 @@ SUBDIRS = . AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib +AM_CPPFLAGS += -DDEFAULT_HOOKS_PATH=\"$(libdir)/kea/hooks\" AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc index a36d42ce0d..b391b85415 100644 --- a/src/lib/hooks/tests/hooks_manager_unittest.cc +++ b/src/lib/hooks/tests/hooks_manager_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2025 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -8,7 +8,9 @@ #include #include +#include #include +#include #include #define TEST_ASYNC_CALLOUT @@ -1077,5 +1079,170 @@ TEST_F(HooksManagerTest, UnloadBeforeUnpark) { EXPECT_FALSE(unparked); } +/// @brief Test fixture for hooks parsing. +class HooksParserTest : public ::testing::Test { +public: + /// @brief Constructor + HooksParserTest() { + // Save current value of the environment path. + char* env_path = std::getenv("KEA_HOOKS_PATH"); + if (env_path) { + original_path_ = std::string(env_path); + } + + // Clear the environment path. + unsetenv("KEA_HOOKS_PATH"); + } + + /// @brief Destructor + ~HooksParserTest() { + // Restore the original environment path. + if (!original_path_.empty()) { + setenv("KEA_HOOKS_PATH", original_path_.c_str(), 1); + } else { + unsetenv("KEA_HOOKS_PATH"); + } + } + + /// @brief Retains the environment variable's original value. + std::string original_path_; +}; + +TEST_F(HooksParserTest, getHooksPath) { + ASSERT_FALSE(std::getenv("KEA_HOOKS_PATH")); + auto hooks_path = HooksLibrariesParser::getHooksPath(true); + EXPECT_EQ(hooks_path, DEFAULT_HOOKS_PATH); +} + +TEST_F(HooksParserTest, getHooksPathWithEnv) { + std::string evar("KEA_HOOKS_PATH=/tmp"); + putenv(const_cast(evar.c_str())); + ASSERT_TRUE(std::getenv("KEA_HOOKS_PATH")); + auto hooks_path = HooksLibrariesParser::getHooksPath(true); + EXPECT_EQ(hooks_path, "/tmp"); +} + +TEST_F(HooksParserTest, getHooksPathExplicit) { + auto hooks_path = HooksLibrariesParser::getHooksPath(true, "/explicit/path"); + EXPECT_EQ(hooks_path, "/explicit/path"); +} + +// Verifies HooksParser::validatePath() when enforce_path is true. +TEST_F(HooksParserTest, validatePathEnforcePath) { + HooksLibrariesParser::getHooksPath(true); + std::string def_path(HooksLibrariesParser::getHooksPath()); + struct Scenario { + int line_; + std::string lib_path_; + std::string exp_path_; + std::string exp_error_; + }; + + std::list scenarios = { + { + // Invalid parent path. + __LINE__, + "/var/lib/bs/mylib.so", + "", + string("invalid path specified: '/var/lib/bs', supported path is '" + def_path + "'") + }, + { + // No file name. + __LINE__, + def_path + "/", + "", + string ("path: '" + def_path + "/' has no filename") + }, + { + // File name only is valid. + __LINE__, + "mylib.so", + def_path + "/mylib.so", + "" + }, + { + // Valid full path. + __LINE__, + def_path + "/mylib.so", + def_path + "/mylib.so", + "" + } + }; + + for (auto scenario : scenarios) { + std::ostringstream oss; + oss << " Scenario at line: " << scenario.line_; + SCOPED_TRACE(oss.str()); + std::string validated_path; + if (scenario.exp_error_.empty()) { + ASSERT_NO_THROW_LOG(validated_path = + HooksLibrariesParser::validatePath(scenario.lib_path_)); + EXPECT_EQ(validated_path, scenario.exp_path_); + } else { + ASSERT_THROW_MSG(validated_path = + HooksLibrariesParser::validatePath(scenario.lib_path_), + BadValue, scenario.exp_error_); + } + } +} + +// Verifies HooksParser::validatePath() when enforce_path is false. +TEST_F(HooksParserTest, validatePathEnforcePathFalse) { + HooksLibrariesParser::getHooksPath(true); + std::string def_path(HooksLibrariesParser::getHooksPath()); + struct Scenario { + int line_; + std::string lib_path_; + std::string exp_path_; + std::string exp_error_; + }; + + std::list scenarios = { + { + // Invalid parent path will fly. + __LINE__, + "/var/lib/bs/mylib.so", + "/var/lib/bs/mylib.so", + "", + }, + { + // No file name. + __LINE__, + def_path + "/", + "", + string ("path: '" + def_path + "/' has no filename") + }, + { + // File name only is valid. + __LINE__, + "mylib.so", + def_path + "/mylib.so", + "" + }, + { + // Valid full path. + __LINE__, + def_path + "/mylib.so", + def_path + "/mylib.so", + "" + } + }; + + for (auto scenario : scenarios) { + std::ostringstream oss; + oss << " Scenario at line: " << scenario.line_; + SCOPED_TRACE(oss.str()); + std::string validated_path; + if (scenario.exp_error_.empty()) { + ASSERT_NO_THROW_LOG(validated_path = + HooksLibrariesParser::validatePath(scenario.lib_path_, false)); + EXPECT_EQ(validated_path, scenario.exp_path_); + } else { + ASSERT_THROW_MSG(validated_path = + HooksLibrariesParser::validatePath(scenario.lib_path_, false), + BadValue, scenario.exp_error_); + } + } +} } // Anonymous namespace diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index 5571231562..46946a228c 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2021-2025 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -10,17 +10,17 @@ #include #include -#include -#include -#include -#include +#include +#include #include -#include #include +#include +#include +#include #include -#include +using namespace isc; using namespace isc::util::str; using namespace std; @@ -69,6 +69,23 @@ isFile(string const& path) { return ((statbuf.st_mode & S_IFMT) == S_IFREG); } +Umask::Umask(mode_t mask) : orig_umask_(umask(S_IWGRP | S_IWOTH)) { + umask(orig_umask_ | mask); +} + +Umask::~Umask() { + umask(orig_umask_); +} + +bool +isSocket(string const& path) { + struct stat statbuf; + if (::stat(path.c_str(), &statbuf) < 0) { + return (false); + } + return ((statbuf.st_mode & S_IFMT) == S_IFSOCK); +} + Path::Path(string const& full_name) { if (!full_name.empty()) { bool dir_present = false; @@ -77,9 +94,9 @@ Path::Path(string const& full_name) { if (last_slash != string::npos) { // Found the last slash, so extract directory component and // set where the scan for the last_dot should terminate. - parent_path_ = full_name.substr(0, last_slash + 1); + parent_path_ = full_name.substr(0, last_slash); if (last_slash == full_name.size()) { - // The entire string was a directory, so exit not and don't + // The entire string was a directory, so exit and don't // do any more searching. return; } @@ -112,7 +129,7 @@ Path::Path(string const& full_name) { string Path::str() const { - return (parent_path_ + stem_ + extension_); + return (parent_path_ + ((parent_path_.empty() || parent_path_ == "/") ? string() : "/") + stem_ + extension_); } string @@ -156,14 +173,86 @@ Path::replaceParentPath(string const& replacement) { string const trimmed_replacement(trim(replacement)); if (trimmed_replacement.empty()) { parent_path_ = string(); - } else if (trimmed_replacement.at(trimmed_replacement.size() - 1) == '/') { + } else if (trimmed_replacement == "/") { parent_path_ = trimmed_replacement; + } else if (trimmed_replacement.at(trimmed_replacement.size() - 1) == '/') { + parent_path_ = trimmed_replacement.substr(0, trimmed_replacement.size() - 1); } else { - parent_path_ = trimmed_replacement + '/'; + parent_path_ = trimmed_replacement; } return (*this); } +TemporaryDirectory::TemporaryDirectory() { + char dir[]("/tmp/kea-tmpdir-XXXXXX"); + char const* dir_name = mkdtemp(dir); + if (!dir_name) { + isc_throw(Unexpected, "mkdtemp failed " << dir << ": " << strerror(errno)); + } + dir_name_ = string(dir_name); +} + +TemporaryDirectory::~TemporaryDirectory() { + DIR *dir(opendir(dir_name_.c_str())); + if (!dir) { + return; + } + + std::unique_ptr defer(dir, [](DIR* d) { closedir(d); }); + + struct dirent *i; + string filepath; + while ((i = readdir(dir))) { + if (strcmp(i->d_name, ".") == 0 || strcmp(i->d_name, "..") == 0) { + continue; + } + + filepath = dir_name_ + '/' + i->d_name; + remove(filepath.c_str()); + } + + rmdir(dir_name_.c_str()); +} + +string TemporaryDirectory::dirName() { + return dir_name_; +} + +std::string +FileManager::validatePath(const std::string supported_path_str, const std::string input_path_str, + bool enforce_path /* = true */) { + // Remove the trailing "/" if it present so comparison to + // input's parent path functions. + auto supported_path_copy(supported_path_str); + if (supported_path_copy.back() == '/') { + supported_path_copy.pop_back(); + } + + Path input_path(trim(input_path_str)); + auto filename = input_path.filename(); + if (filename.empty()) { + isc_throw(BadValue, "path: '" << input_path.str() << "' has no filename"); + } + + auto parent_path = input_path.parentPath(); + if (!parent_path.empty()) { + if (!enforce_path) { + // Security set to lax, let it fly. + return (input_path_str); + } + + // We only allow absolute path equal to default. Catch an invalid path. + if (parent_path != supported_path_copy) { + isc_throw(BadValue, "invalid path specified: '" + << parent_path << "', supported path is '" + << supported_path_copy << "'"); + } + } + + std::string valid_path(supported_path_copy + "/" + filename); + return (valid_path); +} + } // namespace file } // namespace util } // namespace isc diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index 1f20003f39..d726c3178c 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2024 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2021-2025 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,90 +7,111 @@ #ifndef KEA_UTIL_FILESYSTEM_H #define KEA_UTIL_FILESYSTEM_H +#include #include namespace isc { namespace util { namespace file { -/// \brief Get the content of a regular file. +/// @brief Get the content of a regular file. /// -/// \param file_name The file name. +/// @param file_name The file name. /// -/// \return The content of the file. -/// \throw BadValue when the file can't be opened or is not a regular one. +/// @return The content of the file. +/// @throw BadValue when the file can't be opened or is not a regular one. std::string getContent(const std::string& file_name); -/// \brief Check if there is a file or directory at the given path. +/// @brief Check if there is a file or directory at the given path. /// -/// \param path The path being checked. +/// @param path The path being checked. /// -/// \return True if the path points to a file or a directory, false otherwise. +/// @return True if the path points to a file or a directory, false otherwise. bool exists(const std::string& path); -/// \brief Check if there is a directory at the given path. +/// @brief Check if there is a directory at the given path. /// -/// \param path The path being checked. +/// @param path The path being checked. /// -/// \return True if the path points to a directory, false otherwise including +/// @return True if the path points to a directory, false otherwise including /// if the pointed location does not exist. bool isDir(const std::string& path); -/// \brief Check if there is a file at the given path. +/// @brief Check if there is a file at the given path. /// -/// \param path The path being checked. +/// @param path The path being checked. /// -/// \return True if the path points to a file, false otherwise including +/// @return True if the path points to a file, false otherwise including /// if the pointed location does not exist. bool isFile(const std::string& path); -/// \brief Paths on a filesystem +/// @brief RAII device to limit access of created files. +struct Umask { + /// @brief Constructor + /// + /// Set wanted bits in umask. + Umask(mode_t mask); + + /// @brief Destructor. + /// + /// Restore umask. + ~Umask(); + +private: + /// @brief Original umask. + mode_t orig_umask_; +}; + +bool +isSocket(const std::string& path); + +/// @brief Paths on a filesystem struct Path { - /// \brief Constructor + /// @brief Constructor /// /// Splits the full name into components. Path(std::string const& path); - /// \brief Get the path in textual format. + /// @brief Get the path in textual format. /// /// Counterpart for std::filesystem::path::string. /// - /// \return stored filename. + /// @return stored filename. std::string str() const; - /// \brief Get the parent path. + /// @brief Get the parent path. /// /// Counterpart for std::filesystem::path::parent_path. /// - /// \return parent path of current path. + /// @return parent path of current path. std::string parentPath() const; - /// \brief Get the base name of the file without the extension. + /// @brief Get the base name of the file without the extension. /// /// Counterpart for std::filesystem::path::stem. /// - /// \return the base name of current path without the extension. + /// @return the base name of current path without the extension. std::string stem() const; - /// \brief Get the extension of the file. + /// @brief Get the extension of the file. /// /// Counterpart for std::filesystem::path::extension. /// - /// \return extension of current path. + /// @return extension of current path. std::string extension() const; - /// \brief Get the name of the file, extension included. + /// @brief Get the name of the file, extension included. /// /// Counterpart for std::filesystem::path::filename. /// - /// \return name + extension of current path. + /// @return name + extension of current path. std::string filename() const; - /// \brief Identifies the extension in {replacement}, trims it, and + /// @brief Identifies the extension in {replacement}, trims it, and /// replaces this instance's extension with it. /// /// Counterpart for std::filesystem::path::replace_extension. @@ -98,33 +119,66 @@ struct Path { /// The change is done in the members and {this} is returned to allow call /// chaining. /// - /// \param replacement The extension to replace with. + /// @param replacement The extension to replace with. /// - /// \return The current instance after the replacement was done. + /// @return The current instance after the replacement was done. Path& replaceExtension(std::string const& replacement = std::string()); - /// \brief Trims {replacement} and replaces this instance's parent path with + /// @brief Trims {replacement} and replaces this instance's parent path with /// it. /// /// The change is done in the members and {this} is returned to allow call /// chaining. /// - /// \param replacement The parent path to replace with. + /// @param replacement The parent path to replace with. /// - /// \return The current instance after the replacement was done. + /// @return The current instance after the replacement was done. Path& replaceParentPath(std::string const& replacement = std::string()); private: - /// \brief Parent path. + /// @brief Parent path. std::string parent_path_; - /// \brief Stem. + /// @brief Stem. std::string stem_; - /// \brief File name extension. + /// @brief File name extension. std::string extension_; }; +struct TemporaryDirectory { + TemporaryDirectory(); + ~TemporaryDirectory(); + std::string dirName(); +private: + std::string dir_name_; +}; + +/// @brief Class that provides basic file related tasks. +class FileManager { +public: + /// @brief Validates a file path against a supported path. + /// + /// If the input path specifies a parent path and file name, the parent path + /// is validated against the supported path. If they match, the function returns + /// the validated path. If the input path contains only a file name the function + /// returns valid path using the supported path and the input path name. + /// + /// @param supported_path_str absolute path specifying the supported path + /// of the file against which the input path is validated. + /// @param input_path_str file path to validate. + /// @param enforce_path enables validation against the supported path. If false + /// verifies only that the path contains a file name. + /// + /// @return validated path as a string (supported path + input file name) + /// + /// @throw BadValue if the input path does not include a file name or if the + /// it the parent path does not path the supported path. + static std::string validatePath(const std::string supported_path_str, + const std::string input_path_str, + bool enforce_path = true); +}; + } // namespace file } // namespace util } // namespace isc diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index 548070a35a..8a83397a55 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -69,11 +70,24 @@ TEST_F(FileUtilTest, isFile) { EXPECT_FALSE(isFile(TEST_DATA_BUILDDIR)); } +/// @brief Check Umask. +TEST_F(FileUtilTest, umask) { + // Protect the test itself assuming that Umask does what we expect... + Umask m0(0); + mode_t orig = umask(0); + { + Umask m(S_IROTH); + EXPECT_EQ(S_IROTH, umask(S_IRWXO)); + } + EXPECT_EQ(0, umask(orig)); +} + /// @brief Check that the components are split correctly. TEST(PathTest, components) { // Complete name Path fname("/alpha/beta/gamma.delta"); - EXPECT_EQ("/alpha/beta/", fname.parentPath()); + EXPECT_EQ("/alpha/beta/gamma.delta", fname.str()); + EXPECT_EQ("/alpha/beta", fname.parentPath()); EXPECT_EQ("gamma", fname.stem()); EXPECT_EQ(".delta", fname.extension()); EXPECT_EQ("gamma.delta", fname.filename()); @@ -82,6 +96,7 @@ TEST(PathTest, components) { /// @brief Check replaceExtension. TEST(PathTest, replaceExtension) { Path fname("a.b"); + EXPECT_EQ("a.b", fname.str()); EXPECT_EQ("a", fname.replaceExtension("").str()); EXPECT_EQ("a.f", fname.replaceExtension(".f").str()); @@ -98,11 +113,11 @@ TEST(PathTest, replaceParentPath) { EXPECT_EQ("a.b", fname.str()); fname.replaceParentPath("/just/some/dir/"); - EXPECT_EQ("/just/some/dir/", fname.parentPath()); + EXPECT_EQ("/just/some/dir", fname.parentPath()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); fname.replaceParentPath("/just/some/dir"); - EXPECT_EQ("/just/some/dir/", fname.parentPath()); + EXPECT_EQ("/just/some/dir", fname.parentPath()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); fname.replaceParentPath("/"); @@ -114,12 +129,144 @@ TEST(PathTest, replaceParentPath) { EXPECT_EQ("a.b", fname.str()); fname = Path("/first/a.b"); - EXPECT_EQ("/first/", fname.parentPath()); + EXPECT_EQ("/first", fname.parentPath()); EXPECT_EQ("/first/a.b", fname.str()); fname.replaceParentPath("/just/some/dir"); - EXPECT_EQ("/just/some/dir/", fname.parentPath()); + EXPECT_EQ("/just/some/dir", fname.parentPath()); EXPECT_EQ("/just/some/dir/a.b", fname.str()); } + + +// Verifies FileManager::validatePath() when enforce_path is true. +TEST(FileManager, validatePathEnforcePath) { + std::string def_path(TEST_DATA_BUILDDIR + '/'); + struct Scenario { + int line_; + std::string lib_path_; + std::string exp_path_; + std::string exp_error_; + }; + + std::list scenarios = { + { + // Invalid parent path. + __LINE__, + "/var/lib/bs/mylib.so", + "", + string("invalid path specified: '/var/lib/bs', supported path is '" + def_path + "'") + }, + { + // No file name. + __LINE__, + def_path + "/", + "", + string ("path: '" + def_path + "/' has no filename") + }, + { + // File name only is valid. + __LINE__, + "mylib.so", + def_path + "/mylib.so", + "" + }, + { + // Valid full path. + __LINE__, + def_path + "/mylib.so", + def_path + "/mylib.so", + "" + }, + { + // White space for file name. + __LINE__, + " ", + "", + string("path: '' has no filename") + } + }; + + for (auto scenario : scenarios) { + std::ostringstream oss; + oss << " Scenario at line: " << scenario.line_; + SCOPED_TRACE(oss.str()); + std::string validated_path; + if (scenario.exp_error_.empty()) { + ASSERT_NO_THROW_LOG(validated_path = + FileManager::validatePath(def_path, scenario.lib_path_)); + EXPECT_EQ(validated_path, scenario.exp_path_); + } else { + ASSERT_THROW_MSG(validated_path = + FileManager::validatePath(def_path, scenario.lib_path_), + BadValue, scenario.exp_error_); + } + } +} + +// Verifies FileManager::validatePath() when enforce_path is false. +TEST(FileManager, validatePathEnforcePathFalse) { + std::string def_path(TEST_DATA_BUILDDIR); + struct Scenario { + int line_; + std::string lib_path_; + std::string exp_path_; + std::string exp_error_; + }; + + std::list scenarios = { + { + // Invalid parent path but shouldn't care. + __LINE__, + "/var/lib/bs/mylib.so", + "/var/lib/bs/mylib.so", + "" + }, + { + // No file name. + __LINE__, + def_path + "/", + "", + string ("path: '" + def_path + "/' has no filename") + }, + { + // File name only is valid. + __LINE__, + "mylib.so", + def_path + "/mylib.so", + "" + }, + { + // Valid full path. + __LINE__, + def_path + "/mylib.so", + def_path + "/mylib.so", + "" + }, + { + // White space for file name. + __LINE__, + " ", + "", + string("path: '' has no filename") + } + }; + + for (auto scenario : scenarios) { + std::ostringstream oss; + oss << " Scenario at line: " << scenario.line_; + SCOPED_TRACE(oss.str()); + std::string validated_path; + if (scenario.exp_error_.empty()) { + ASSERT_NO_THROW_LOG(validated_path = + FileManager::validatePath(def_path, scenario.lib_path_, false)); + EXPECT_EQ(validated_path, scenario.exp_path_); + } else { + ASSERT_THROW_MSG(validated_path = + FileManager::validatePath(def_path, scenario.lib_path_, false), + BadValue, scenario.exp_error_); + } + } +} + } // namespace -- 2.47.2