[#3831] Restricted cache-write

diff --git a/doc/sphinx/arm/hooks-host-cache.rst b/doc/sphinx/arm/hooks-host-cache.rst
index bd980572136a7edd1d7af5198ecd937a0f3b0784..b903098610c005d4c70bdeb02e075a164724a3a8 100644 (file)
--- 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 edde0829d7bcf8f9335c6ebfa1ae90f2e4a96354..3ceb9abbf9beb6c6139336acce9e6f072a5548f5 100644 (file)
--- a/src/hooks/dhcp/host_cache/host_cache.cc
+++ b/src/hooks/dhcp/host_cache/host_cache.cc
@@ -11,6 +11,7 @@
 #include <host_cache_parsers.h>
 #include <host_cache_log.h>
 #include <database/db_exceptions.h>
+#include <dhcpsrv/cfgmgr.h>
 #include <util/encode/encode.h>
 #include <util/multi_threading_mgr.h>
 #include <util/str.h>
@@ -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 e8e2b36f3cef13b2fb0dd90544daf63764990b3c..d48800a4cfc4c8ac1715a62c733647b151dd3649 100644 (file)
--- 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 4de32093af5a6798d2ffd77ad538869090f38b96..8c85414053cb30d652beeec7d9be856d5e6acd1b 100644 (file)
--- 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 <host_cache.h>
 #include <host_cache_parsers.h>
 #include <hooks/hooks_manager.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <testutils/env_var_wrapper.h>
 #include <testutils/multi_threading_utils.h>
 #include <gtest/gtest.h>
 #include <fstream>
@@ -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<size_t>(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<size_t>(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 9a2ab56e38e78336001cc2a2b4ff12aeadbb2107..7e627179727c31c63a2862655cc4b5d521d9b294 100644 (file)
--- 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',