From: Francis Dupont Date: Thu, 15 May 2025 16:23:49 +0000 (+0200) Subject: [#3831] Restricted cache-write X-Git-Tag: Kea-2.7.9~33 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0e25831cf883256b113b1d36056080b3090d3f46;p=thirdparty%2Fkea.git [#3831] Restricted cache-write --- diff --git a/doc/sphinx/arm/hooks-host-cache.rst b/doc/sphinx/arm/hooks-host-cache.rst index bd98057213..b903098610 100644 --- a/doc/sphinx/arm/hooks-host-cache.rst +++ b/doc/sphinx/arm/hooks-host-cache.rst @@ -137,13 +137,21 @@ example usage looks as follows: { "command": "cache-write", - "arguments": "/tmp/kea-host-cache.json" + "arguments": "/usr/local/var/lib/kea/kea-host-cache.json" } -This causes the contents to be stored in the ``/tmp/kea-host-cache.json`` +This causes the contents to be stored in the ``/usr/local/var/lib/kea/kea-host-cache.json`` file. That file can then be loaded with the :isccmd:`cache-load` command or processed by any other tool that is able to understand JSON format. +.. note:: + + As of Kea 2.7.9, the cache file may only be written to the data directory + determined during compilation: ``"[kea-install-dir]/var/lib/kea"``. This + path may be overridden at startup by setting the environment variable + ``KEA_DHCP_DATA_DIRECTORY`` to the desired path. For ease of use in + specifying a custom file name simply omit the path portion from ``filename``. + .. isccmd:: cache-load .. _command-cache-load: diff --git a/src/hooks/dhcp/host_cache/host_cache.cc b/src/hooks/dhcp/host_cache/host_cache.cc index edde0829d7..3ceb9abbf9 100644 --- a/src/hooks/dhcp/host_cache/host_cache.cc +++ b/src/hooks/dhcp/host_cache/host_cache.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -750,9 +751,10 @@ HostCache::cacheWriteHandler(hooks::CalloutHandle& handle) { isc_throw(BadValue, "invalid (not a string) parameter"); } - filename = cmd_args_->stringValue(); - if (filename.empty()) { - isc_throw(BadValue, "invalid (empty string) parameter"); + try { + filename = CfgMgr::instance().validatePath(cmd_args_->stringValue()); + } catch (const std::exception& ex) { + isc_throw(BadValue, "parameter is invalid: " << ex.what()); } ofstream out(filename, ios::trunc); diff --git a/src/hooks/dhcp/host_cache/tests/Makefile.am b/src/hooks/dhcp/host_cache/tests/Makefile.am index e8e2b36f3c..d48800a4cf 100644 --- a/src/hooks/dhcp/host_cache/tests/Makefile.am +++ b/src/hooks/dhcp/host_cache/tests/Makefile.am @@ -47,6 +47,7 @@ host_cache_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/database/testutils/libdatabasetest.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/database/libkea-database.la +host_cache_unittests_LDADD += $(top_builddir)/src/lib/testutils/libkea-testutils.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la host_cache_unittests_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/hooks/dhcp/host_cache/tests/command_unittests.cc b/src/hooks/dhcp/host_cache/tests/command_unittests.cc index 4de32093af..8c85414053 100644 --- a/src/hooks/dhcp/host_cache/tests/command_unittests.cc +++ b/src/hooks/dhcp/host_cache/tests/command_unittests.cc @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include #include #include @@ -36,6 +38,11 @@ class CommandTest : public ::testing::Test { public: /// @brief Constructor CommandTest() { + // Save the pre-test data dir and set it to the test directory. + CfgMgr::instance().clear(); + original_datadir_ = CfgMgr::instance().getDataDir(); + CfgMgr::instance().getDataDir(true, TEST_DATA_BUILDDIR); + hcptr_.reset(new HostCache()); MultiThreadingMgr::instance().setMode(false); } @@ -44,6 +51,9 @@ public: virtual ~CommandTest() { ::remove("dump.json"); ::remove("source.json"); + + // Revert to original data directory. + CfgMgr::instance().getDataDir(true, original_datadir_); } /// @brief Creates a sample IPv4 host @@ -290,6 +300,9 @@ public: /// @brief Host Cache HostCachePtr hcptr_; + + /// @brief Stores the pre-test DHCP data directory. + std::string original_datadir_; }; /// @brief Size handler function @@ -686,6 +699,7 @@ CommandTest::testWriteCommand() { // Prepare handlerType write_handler = std::bind(&writeHandler, hcptr_, ph::_1); string file_txt = "dump.json"; + string full_file_txt = CfgMgr::instance().getDataDir() + "/" + file_txt; string write_cmd = "{ \"command\": \"cache-write\", \"arguments\": \"" + file_txt + "\" }"; @@ -696,19 +710,31 @@ CommandTest::testWriteCommand() { // Write file checkCommand(write_handler, write_cmd, 0, 0, - "2 entries dumped to '" + file_txt + "'."); + "2 entries dumped to '" + full_file_txt + "'."); // Check file - ifstream f(file_txt, ios::binary | ios::ate); + ifstream f(full_file_txt, ios::binary | ios::ate); ASSERT_TRUE(f.good()); EXPECT_LE(host4_txt.size(), static_cast(f.tellg())); - ElementPtr from_file = Element::fromJSONFile(file_txt); + ElementPtr from_file = Element::fromJSONFile(full_file_txt); ASSERT_TRUE(from_file); ElementPtr expected = Element::createList(); expected->add(Element::fromJSON(host4_txt)); expected->add(Element::fromJSON(host6_txt)); EXPECT_EQ(*from_file, *expected); + // Check with full path. + write_cmd = "{ \"command\": \"cache-write\", \"arguments\": \""; + write_cmd += full_file_txt + "\" }"; + checkCommand(write_handler, write_cmd, 0, 0, + "2 entries dumped to '" + full_file_txt + "'."); + ifstream ff(full_file_txt, ios::binary | ios::ate); + ASSERT_TRUE(ff.good()); + EXPECT_LE(host4_txt.size(), static_cast(ff.tellg())); + from_file = Element::fromJSONFile(full_file_txt); + ASSERT_TRUE(from_file); + EXPECT_EQ(*from_file, *expected); + // Check errors string noarg_cmd = "{ \"command\": \"cache-write\" }"; checkCommand(write_handler, noarg_cmd, 1, 1, @@ -720,12 +746,16 @@ CommandTest::testWriteCommand() { string emptyarg_cmd = "{ \"command\": \"cache-write\", \"arguments\": \"\" }"; checkCommand(write_handler, emptyarg_cmd, 1, 1, - "invalid (empty string) parameter"); - string notexists = "/this/does/not/exit"; - string notexists_cmd = - "{ \"command\": \"cache-write\", \"arguments\": \"" + notexists + "\" }"; - checkCommand(write_handler, notexists_cmd, 1, 1, - "Unable to open file '" + notexists + "' for writing."); + "parameter is invalid: path: '' has no filename"); + string badpath = "/foo/bar"; + string badpath_cmd = + "{ \"command\": \"cache-write\", \"arguments\": \"" + badpath + "\" }"; + string exp_error = "parameter is invalid: invalid path specified: "; + exp_error += "'/foo', "; + exp_error += "supported path is '"; + exp_error += CfgMgr::instance().getDataDir(); + exp_error += "'"; + checkCommand(write_handler, badpath_cmd, 1, 1, exp_error); } // Verifies that cache-load can load a dump file. @@ -1117,6 +1147,14 @@ TEST_F(CommandTest, writeMultiThreading) { testWriteCommand(); } +TEST_F(CommandTest, writeEnvVarOverride) { + EnvVarWrapper data_dir_env_var_("KEA_DHCP_DATA_DIR"); + data_dir_env_var_.setValue("/tmp"); + auto valid_path = CfgMgr::instance().getDataDir(true); + EXPECT_EQ(valid_path, "/tmp"); + testWriteCommand(); +} + TEST_F(CommandTest, load) { testLoadCommand(); } diff --git a/src/hooks/dhcp/host_cache/tests/meson.build b/src/hooks/dhcp/host_cache/tests/meson.build index 9a2ab56e38..7e62717972 100644 --- a/src/hooks/dhcp/host_cache/tests/meson.build +++ b/src/hooks/dhcp/host_cache/tests/meson.build @@ -7,6 +7,7 @@ dhcp_host_cache_tests_libs = [ dhcp_host_cache_archive, kea_dhcpsrv_testutils_lib, kea_database_testutils_lib, + kea_testutils_lib, ] dhcp_host_cache_tests = executable( 'dhcp-host-cache-tests',