From: Tomek Mrugalski Date: Tue, 14 Mar 2017 20:06:56 +0000 (+0100) Subject: [5151] get-config, write-config implemented X-Git-Tag: trac102b_base~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=16deb5e4db9dc2dfb1317c7eefa59d08824e473e;p=thirdparty%2Fkea.git [5151] get-config, write-config implemented --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index c6c902936b..055b6493b0 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -74,6 +74,77 @@ ControlledDhcpv4Srv::commandGetConfigHandler(const string&, return (createAnswer(0, config)); } +ConstElementPtr +ControlledDhcpv4Srv::commandWriteConfigHandler(const string&, + ConstElementPtr args) { + ConstElementPtr config = CfgMgr::instance().getCurrentCfg()->toElement(); + + string filename; + + if (args) { + if (args->getType() != Element::map) { + return (createAnswer(CONTROL_RESULT_ERROR, "Argument must be a map")); + } + ConstElementPtr filename_param = args->get("filename"); + if (filename_param) { + if (filename_param->getType() != Element::string) { + return (createAnswer(CONTROL_RESULT_ERROR, + "passed parameter 'filename' is not a string")); + } + filename = filename_param->stringValue(); + } + } + + 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.")); + } + + size_t size = 0; + try { + size = writeConfigFile(filename); + } catch (const isc::Exception& ex) { + return (createAnswer(CONTROL_RESULT_ERROR, string("Error during write-config:") + + ex.what())); + } + if (size == 0) { + return (createAnswer(CONTROL_RESULT_ERROR, "Error writing configuration to " + + filename)); + } + + // Ok, it's time to return the successful response + ElementPtr params = Element::createMap(); + params->set("size", Element::create(static_cast(size))); + params->set("filename", Element::create(filename)); + + return (createAnswer(CONTROL_RESULT_SUCCESS, "Configuration written to " + + filename + " successful", params)); +} + ConstElementPtr ControlledDhcpv4Srv::commandSetConfigHandler(const string&, ConstElementPtr args) { @@ -194,6 +265,8 @@ ControlledDhcpv4Srv::processCommand(const string& command, } else if (command == "leases-reclaim") { return (srv->commandLeasesReclaimHandler(command, args)); + } else if (command == "write-config") { + return (srv->commandWriteConfigHandler(command, args)); } ConstElementPtr answer = isc::config::createAnswer(1, "Unrecognized command:" + command); @@ -358,6 +431,9 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) CommandMgr::instance().registerCommand("statistic-remove-all", boost::bind(&StatsMgr::statisticRemoveAllHandler, _1, _2)); + + CommandMgr::instance().registerCommand("write-config", + boost::bind(&ControlledDhcpv4Srv::commandWriteConfigHandler, this, _1, _2)); } void ControlledDhcpv4Srv::shutdown() { @@ -389,6 +465,7 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() { CommandMgr::instance().deregisterCommand("statistic-get-all"); CommandMgr::instance().deregisterCommand("statistic-reset-all"); CommandMgr::instance().deregisterCommand("statistic-remove-all"); + CommandMgr::instance().deregisterCommand("write-config"); } catch (...) { // Don't want to throw exceptions from the destructor. The server diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 719b7a1359..db1e4379ec 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -144,10 +144,35 @@ private: commandConfigReloadHandler(const std::string& command, isc::data::ConstElementPtr args); + /// @brief handler for processing 'get-config' command + /// + /// This handler processes get-config command, which retrieves + /// the current configuration and returns it in response. + /// + /// @param command (ignored) + /// @param args (ignored) + /// @return current configuration wrapped in a response isc::data::ConstElementPtr commandGetConfigHandler(const std::string& command, isc::data::ConstElementPtr args); + /// @brief handler for processing 'write-config' command + /// + /// This handle processes write-config comamnd, which writes the + /// 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. The filename must be within the + /// {prefix} directory specified during Kea compilation. This is + /// a security measure against exploiting file writes remotely. + /// + /// @param command (ignored) + /// @param args may contain optional string argument filename + /// @return status of the configuration file write + isc::data::ConstElementPtr + commandWriteConfigHandler(const std::string& command, + isc::data::ConstElementPtr args); + /// @brief handler for processing 'set-config' command /// /// This handler processes set-config command, which processes diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 0954110eec..06780c6949 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -198,6 +198,93 @@ public: client->disconnectFromServer(); ASSERT_NO_THROW(server_->receivePacket(0)); } + + /// @brief Checks response for list-commands + /// + /// This method checks if the list-commands response is generally sane + /// and whether specified command is mentioned in the response. + /// + /// @param rsp response sent back by the server + /// @param command command expected to be on the list. + void checkListCommands(const ConstElementPtr& rsp, const std::string& command) { + ConstElementPtr params; + int status_code; + EXPECT_NO_THROW(params = parseAnswer(status_code, rsp)); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status_code); + ASSERT_TRUE(params); + ASSERT_EQ(Element::list, params->getType()); + + int cnt = 0; + for (int i=0; i < params->size(); ++i) { + string tmp = params->get(i)->stringValue(); + if (tmp == command) { + // Command found, but that's not enough. Need to continue working + // through the list to see if there are no duplicates. + cnt++; + } + } + + // Exactly one command on the list is expected. + EXPECT_EQ(1, cnt) << "Command " << command << " not found"; + } + + /// @brief Check if the answer for write-config command is correct + /// + /// @param response_txt response in text form (as read from the control socket) + /// @param exp_status expected status (0 success, 1 failure) + /// @param exp_txt for success cases this defines the expected filename, + /// for failure cases this defines the expected error message + void checkWriteConfig(const std::string& response_txt, int exp_status, + const std::string& exp_txt = "") { + + cout << "#### response=" << response_txt << endl; + + ConstElementPtr rsp; + EXPECT_NO_THROW(rsp = Element::fromJSON(response_txt)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr params = parseAnswer(status, rsp); + EXPECT_EQ(exp_status, status); + + if (exp_status == CONTROL_RESULT_SUCCESS) { + // Let's check couple things... + + // The parameters must include filename + ASSERT_TRUE(params); + ASSERT_TRUE(params->get("filename")); + EXPECT_EQ(Element::string, params->get("filename")->getType()); + EXPECT_EQ(exp_txt, params->get("filename")->stringValue()); + + // The parameters must include size. And the size + // must indicate some content. + ASSERT_TRUE(params->get("size")); + EXPECT_EQ(Element::integer, params->get("size")->getType()); + int64_t size = params->get("size")->intValue(); + EXPECT_LE(1, size); + + // Now check if the file is really there and suitable for + // opening. + ifstream f(exp_txt, ios::binary | ios::ate); + ASSERT_TRUE(f.good()); + + // Now check that it is the correct size as reported. + EXPECT_EQ(size, static_cast(f.tellg())); + + // Finally, check that it's really a JSON. + ElementPtr from_file = Element::fromJSONFile(exp_txt); + ASSERT_TRUE(from_file); + } else if (exp_status == CONTROL_RESULT_ERROR) { + + // Let's check if the reason for failure was given. + ConstElementPtr text = rsp->get("text"); + ASSERT_TRUE(text); + ASSERT_EQ(Element::string, text->getType()); + EXPECT_EQ(exp_txt, text->stringValue()); + } else { + ADD_FAILURE() << "Invalid expected status: " << exp_status; + } + } }; TEST_F(CtrlChannelDhcpv4SrvTest, commands) { @@ -625,4 +712,112 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) { CfgMgr::instance().clear(); } +// Tests that the server properly responds to shtudown command sent +// via ControlChannel +TEST_F(CtrlChannelDhcpv4SrvTest, listCommands) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"list-commands\" }", response); + + ConstElementPtr rsp; + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + + // We expect the server to report at least the following commands: + checkListCommands(rsp, "get-config"); + checkListCommands(rsp, "list-commands"); + checkListCommands(rsp, "leases-reclaim"); + checkListCommands(rsp, "libreload"); + checkListCommands(rsp, "set-config"); + checkListCommands(rsp, "shutdown"); + checkListCommands(rsp, "statistic-get"); + checkListCommands(rsp, "statistic-get-all"); + checkListCommands(rsp, "statistic-remove"); + checkListCommands(rsp, "statistic-remove-all"); + checkListCommands(rsp, "statistic-reset"); + checkListCommands(rsp, "statistic-reset-all"); + checkListCommands(rsp, "write-config"); +} + +// Tests if the server returns its configuration using get-config. +// Note there are separate tests that verify if toElement() called by the +// get-config handler are actually converting the configuration correctly. +TEST_F(CtrlChannelDhcpv4SrvTest, getConfig) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"get-config\" }", response); + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // Ok, now roughly check if the response seems legit. + ASSERT_TRUE(cfg); + EXPECT_EQ(Element::map, cfg->getType()); + EXPECT_TRUE(cfg->get("Dhcp4")); +} + + +TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigNoFilename) { + createUnixChannelServer(); + std::string response; + + // This is normally set by the command line -c parameter. + server_->setConfigFile("test1.json"); + + // If the filename is not explicitly specified, the name used + // in -c command line switch is used. + sendUnixCommand("{ \"command\": \"write-config\" }", response); + + checkWriteConfig(response, CONTROL_RESULT_SUCCESS, "test1.json"); + ::remove("test1.json"); +} + +TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigFilename) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"write-config\", " + "\"arguments\": { \"filename\": \"test2.json\" } }", response); + checkWriteConfig(response, CONTROL_RESULT_SUCCESS, "test2.json"); + ::remove("test2.json"); +} + +TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidJailEscape) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"write-config\", \"arguments\": " + "{ \"filename\": \"../test3.json\" } }", response); + checkWriteConfig(response, CONTROL_RESULT_ERROR, + "Using '..' in filename is not allowed."); +} + +TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidAbsPath) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"write-config\", \"arguments\": " + "{ \"filename\": \"/tmp/test4.json\" } }", response); + checkWriteConfig(response, CONTROL_RESULT_ERROR, + "Absolute path in filename is not allowed."); +} + +TEST_F(CtrlChannelDhcpv4SrvTest, writeConfigInvalidEscape) { + createUnixChannelServer(); + std::string response; + + // This will be converted to foo(single backslash)test5.json + sendUnixCommand("{ \"command\": \"write-config\", \"arguments\": " + "{ \"filename\": \"foo\\\\test5.json\" } }", response); + checkWriteConfig(response, CONTROL_RESULT_ERROR, + "Using \\ in filename is not allowed."); +} + } // End of anonymous namespace diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 3a9a0b52d5..a16e0242ed 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -17,6 +17,7 @@ #include #include +#include #include /// @brief provides default implementation for basic daemon operations @@ -205,5 +206,26 @@ Daemon::createPIDFile(int pid) { am_file_author_ = true; } +size_t +Daemon::writeConfigFile(const std::string& config_file) const { + isc::data::ConstElementPtr cfg = CfgMgr::instance().getCurrentCfg()->toElement(); + if (!cfg) { + isc_throw(Unexpected, "Can't write configuration: conversion to JSON failed"); + } + + std::ofstream out(config_file, std::ios::trunc); + if (!out.good()) { + isc_throw(Unexpected, "Unable to open file " + config_file + " for writing"); + } + + out << cfg->str(); + + size_t bytes = static_cast(out.tellp()); + + out.close(); + + return (bytes); +} + }; }; diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index b52014f114..7ff12bbe3f 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -136,6 +136,18 @@ public: /// @param config_file pathname of the configuration file void setConfigFile(const std::string& config_file); + /// @brief Writes current configuration to specified file + /// + /// This method writes the current configuration to specified file. + /// @todo: this logically more belongs to CPL process file. Once + /// Daemon is merged with CPL architecture, it will be a better + /// fit. + /// + /// @param config_file name of the file to write the configuration to + /// @return number of files written + virtual size_t + writeConfigFile(const std::string& config_file) const; + /// @brief returns the process name /// This value is used as when forming the default PID file name /// @return text string