[#3831] Checkpoint: code done, UTs to be updated

diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst
index a9757c21afd3f058bcd3231da859bf30c4b177b8..652fa525afa7bfb405d7dbbc522a08681ac0eba7 100644 (file)
--- a/doc/sphinx/arm/ctrl-channel.rst
+++ b/doc/sphinx/arm/ctrl-channel.rst
@@ -532,6 +532,12 @@ An example command invocation looks like this:
        }
    }
 
+.. note::
+
+    As of Kea 2.7.9, the config file file may only be written to the same
+    directory as the config file used when starting Kea (passed as a ``-c``
+    argument).
+
 .. isccmd:: leases-reclaim
 .. _command-leases-reclaim:
 

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index a5c9371fe8bb020170e9b1e31e44ae2a726ccd0d..9cf49e47cb75334eeced66f5fbe47358af50536b 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -290,11 +290,18 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&,
         // filename parameter was not specified, so let's use whatever we remember
         // from the command-line
         filename = getConfigFile();
-    }
-
-    if (filename.empty()) {
-        return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
-                             "Please specify filename explicitly."));
+        if (filename.empty()) {
+            return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
+                                 "Please specify filename explicitly."));
+        }
+    } else {
+        try {
+            checkWriteConfigFile(filename);
+        } catch (const isc::Exception& ex) {
+            return (createAnswer(CONTROL_RESULT_ERROR,
+                                 string("not allowed to write config into ") +
+                                 filename));
+        }
     }
 
     // Ok, it's time to write the file.

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h
index 31e78bed49135cb6c880938434168b9bf03733f6..4e56c0ca900e9794ed275c01a149d500cbb917d8 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.h
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h
@@ -179,9 +179,9 @@ private:
     /// current configuration to disk. This command takes one optional
     /// parameter called filename. If specified, the current configuration
     /// will be written to that file. If not specified, the file used during
-    /// Kea start-up will be used. To avoid any exploits, the path is
-    /// always relative and .. is not allowed in the filename. This is
-    /// a security measure against exploiting file writes remotely.
+    /// Kea start-up will be used. To avoid any exploits, the target
+    /// directory must be the same as a security measure against
+    /// exploiting file writes remotely.
     ///
     /// @param command (ignored)
     /// @param args may contain optional string argument filename

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index cf5a0d3bf0c691279a926ed0754a31605a864cc4..304c083b8fd0ca7b8786ce87a41f142113929dd2 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -293,11 +293,18 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&,
         // filename parameter was not specified, so let's use whatever we remember
         // from the command-line
         filename = getConfigFile();
-    }
-
-    if (filename.empty()) {
-        return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
-                             "Please specify filename explicitly."));
+        if (filename.empty()) {
+            return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
+                                 "Please specify filename explicitly."));
+        }
+    } else {
+        try {
+            checkWriteConfigFile(filename);
+        } catch (const isc::Exception& ex) {
+            return (createAnswer(CONTROL_RESULT_ERROR,
+                                 string("not allowed to write config into ") +
+                                 filename));
+        }
     }
 
     // Ok, it's time to write the file.

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h
index d2b5b734843c88d4883552338a83bd14e2c60734..764a7c5834858e3f09564881f0dbc6615e234211 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.h
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h
@@ -179,9 +179,9 @@ private:
     /// current configuration to disk. This command takes one optional
     /// parameter called filename. If specified, the current configuration
     /// will be written to that file. If not specified, the file used during
-    /// Kea start-up will be used. To avoid any exploits, the path is
-    /// always relative and .. is not allowed in the filename. This is
-    /// a security measure against exploiting file writes remotely.
+    /// Kea start-up will be used. To avoid any exploits, the target
+    /// directory must be the same as a security measure against
+    /// exploiting file writes remotely.
     ///
     /// @param command (ignored)
     /// @param args may contain optional string argument filename

diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc
index b4f5781a35aad6b5d269b5109a18012b2976c9ff..a535ffb65bed76394f1208309632a1f665775dc0 100644 (file)
--- a/src/lib/process/d_controller.cc
+++ b/src/lib/process/d_controller.cc
@@ -508,13 +508,21 @@ DControllerBase::configWriteHandler(const std::string&,
                                  "Unable to determine filename."
                                  "Please specify filename explicitly."));
         }
+    } else {
+        try {
+            checkWriteConfigFile(filename);
+        } catch (const isc::Exception& ex) {
+            return (createAnswer(CONTROL_RESULT_ERROR,
+                                 std::string("not allowed to write config into ") +
+                                 filename));
+        }
     }
 
     // Ok, it's time to write the file.
     size_t size = 0;
-    ElementPtr cfg = process_->getCfgMgr()->getContext()->toElement();
 
     try {
+        ElementPtr cfg = process_->getCfgMgr()->getContext()->toElement();
         size = writeConfigFile(filename, cfg);
     } catch (const isc::Exception& ex) {
         return (createAnswer(CONTROL_RESULT_ERROR,

diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h
index cb55b667a6b0d0553f601814c84701603974c01c..3ea9558be43d5c3ce5619905558d6d6ca0030ba4 100644 (file)
--- a/src/lib/process/d_controller.h
+++ b/src/lib/process/d_controller.h
@@ -283,9 +283,9 @@ public:
     /// current configuration to disk. This command takes one optional
     /// parameter called filename. If specified, the current configuration
     /// will be written to that file. If not specified, the file used during
-    /// Kea start-up will be used. To avoid any exploits, the path is
-    /// always relative and .. is not allowed in the filename. This is
-    /// a security measure against exploiting file writes remotely.
+    /// Kea start-up will be used. To avoid any exploits, the target
+    /// directory must be the same as a security measure against
+    /// exploiting file writes remotely.
     ///
     /// @param command (ignored)
     /// @param args may contain optional string argument filename

diff --git a/src/lib/process/daemon.cc b/src/lib/process/daemon.cc
index 7069e1619eca3d64cb2bd67f4ecdd1a8fd9ccd4d..fdd66128df81c8be31b8dfc3c2884977eb7cca52 100644 (file)
--- a/src/lib/process/daemon.cc
+++ b/src/lib/process/daemon.cc
@@ -125,6 +125,28 @@ Daemon::checkConfigFile() const {
     }
 }
 
+void
+Daemon::checkWriteConfigFile(std::string& file) {
+    Path path(file);
+    // from checkConfigFile().
+    if (path.stem().empty()) {
+        isc_throw(isc::BadValue, "config file:" << file
+                  << " is missing file name");
+    }
+    Path current(config_file_);
+    if (current.parentPath() == path.parentPath()) {
+        // Same paths!
+        return;
+    }
+    if (path.parentPath().empty()) {
+        // Note the current path can't be empty here.
+        file = current.parentPath() + "/" + file;
+        return;
+    }
+    isc_throw(isc::BadValue, "file " << file << " must be in the same "
+              << "directory as the config file (" << config_file_ << "'");
+}
+
 std::string
 Daemon::getProcName() {
     return (proc_name_);

diff --git a/src/lib/process/daemon.h b/src/lib/process/daemon.h
index 99b841c9d9fcad7c6aa9d32e095535c72d68557d..37c3fc1422fdd0989626d872975a9c48bc1c2ce4 100644 (file)
--- a/src/lib/process/daemon.h
+++ b/src/lib/process/daemon.h
@@ -139,6 +139,15 @@ public:
     /// @throw BadValue when the configuration file name is bad.
     void checkConfigFile() const;
 
+    /// @brief Checks the to-be-written configuration file name.
+    ///
+    /// @note As a side effect prepend the current config file path
+    /// when the name does not contain a slash.
+    ///
+    /// @param[in][out] file Reference to the TBW configuration file name.
+    /// @throw BadValue when not in the same directory.
+    void checkWriteConfigFile(std::string& file);
+
     /// @brief Writes current configuration to specified file
     ///
     /// This method writes the current configuration to specified file.