Browse Source

[5187] Kea is less strict with filename argument to config-write

Tomek Mrugalski 8 years ago
parent
commit
002e269071

+ 3 - 4
doc/guide/ctrl-channel.xml

@@ -363,10 +363,9 @@ $ curl -X POST -H "Content-Type: application/json" -d '{ "command": "config-get"
         to write its current configuration to a file on disk. It takes one
         optional argument called <emphasis>filename</emphasis> that specifies
         the name of the file to write configuration to. If not specified, the
-        name used when starting Kea (passed as -c argument) will be used. Note
-        that the filename specified must not contain .. or backslashes. Kea
-        should be able to write its files only in the directory it is running
-        and any attempts to step out of that directory will be rejected.</para>
+        name used when starting Kea (passed as -c argument) will be
+        used. If relative path is specified, Kea will write its files
+        only in the directory it is running.</para>
         <para>
           An example command invocation looks like this:
 <screen>

+ 3 - 22
src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -246,30 +246,11 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&,
     if (filename.empty()) {
         // filename parameter was not specified, so let's use whatever we remember
         filename = getConfigFile();
-        if (filename.empty()) {
-            return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
-                                 "Please specify filename explicitly."));
-        }
     }
 
-    // Now do the sanity checks on the filename
-    if (filename.find("..") != string::npos) {
-        // Trying to escape the directory.. nope.
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Using '..' in filename is not allowed."));
-    }
-
-    if (filename.find("\\") != string::npos) {
-        // Trying to inject escapes (possibly to inject quotes and something
-        // nasty afterward)
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Using \\ in filename is not allowed."));
-    }
-
-    if (filename[0] == '/') {
-        // Absolute paths are not allowed.
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Absolute path in filename is not allowed."));
+    if (filename.empty()) {
+        return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
+                             "Please specify filename explicitly."));
     }
 
     // Ok, it's time to write the file.

+ 0 - 36
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -965,42 +965,6 @@ TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigFilename) {
     ::remove("test2.json");
 }
 
-// Tests if config-write rejects invalid filename (a one that tries to escape
-// the current directory).
-TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidJailEscape) {
-    createUnixChannelServer();
-    std::string response;
-
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"../test3.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Using '..' in filename is not allowed.");
-}
-
-// Tests if config-write rejects invalid filename (absolute paths are not allowed)
-TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidAbsPath) {
-    createUnixChannelServer();
-    std::string response;
-
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"/tmp/test4.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Absolute path in filename is not allowed.");
-}
-
-// Tests if config-write rejects invalid filename (one with backslashes, which may
-// lead to some other tricks)
-TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidEscape) {
-    createUnixChannelServer();
-    std::string response;
-
-    // This will be converted to foo(single backslash)test5.json
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"foo\\\\test5.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Using \\ in filename is not allowed.");
-}
-
 // Tests if config-reload attempts to reload a file and reports that the
 // file is missing.
 TEST_F(CtrlChannelDhcpv4SrvTest, configReloadMissingFile) {

+ 3 - 22
src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -249,30 +249,11 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&, ConstElementPtr ar
         // 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."));
-        }
     }
 
-    // Now do the sanity checks on the filename
-    if (filename.find("..") != string::npos) {
-        // Trying to escape the directory with .. nope.
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Using '..' in filename is not allowed."));
-    }
-
-    if (filename.find("\\") != string::npos) {
-        // Trying to inject escapes (possibly to inject quotes and something
-        // nasty afterward)
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Using \\ in filename is not allowed."));
-    }
-
-    if (filename[0] == '/') {
-        // Absolute paths are not allowed.
-        return (createAnswer(CONTROL_RESULT_ERROR,
-                             "Absolute path in filename is not allowed."));
+    if (filename.empty()) {
+        return (createAnswer(CONTROL_RESULT_ERROR, "Unable to determine filename."
+                             "Please specify filename explicitly."));
     }
 
     // Ok, it's time to write the file.

+ 0 - 36
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -991,42 +991,6 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configWriteFilename) {
     ::remove("test2.json");
 }
 
-// Tests if config-write rejects invalid filename (a one that tries to escape
-// the current directory).
-TEST_F(CtrlChannelDhcpv6SrvTest, configWriteInvalidJailEscape) {
-    createUnixChannelServer();
-    std::string response;
-
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"../test3.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Using '..' in filename is not allowed.");
-}
-
-// Tests if config-write rejects invalid filename (absolute paths are not allowed)
-TEST_F(CtrlChannelDhcpv6SrvTest, configWriteInvalidAbsPath) {
-    createUnixChannelServer();
-    std::string response;
-
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"/tmp/test4.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Absolute path in filename is not allowed.");
-}
-
-// Tests if config-write rejects invalid filename (one with backslashes, which may
-// lead to some other tricks)
-TEST_F(CtrlChannelDhcpv6SrvTest, configWriteInvalidEscape) {
-    createUnixChannelServer();
-    std::string response;
-
-    // This will be converted to foo(single backslash)test5.json
-    sendUnixCommand("{ \"command\": \"config-write\", \"arguments\": "
-                    "{ \"filename\": \"foo\\\\test5.json\" } }", response);
-    checkConfigWrite(response, CONTROL_RESULT_ERROR,
-                     "Using \\ in filename is not allowed.");
-}
-
 // Tests if config-reload attempts to reload a file and reports that the
 // file is missing.
 TEST_F(CtrlChannelDhcpv6SrvTest, configReloadMissingFile) {