From d0dc88fe5ee2b1cee37a2f3babcdbfee81df472f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Wed, 15 Mar 2017 17:04:15 +0100 Subject: [PATCH] [5151] config-get, config-write implemented for DHCPv6 --- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 133 +++++++++--- src/bin/dhcp6/ctrl_dhcp6_srv.h | 27 ++- .../dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 199 ++++++++++++++++++ src/lib/dhcpsrv/daemon.cc | 2 +- src/lib/dhcpsrv/daemon.h | 4 +- 5 files changed, 336 insertions(+), 29 deletions(-) diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 9d169fa8b6..2625529ed4 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -72,13 +72,85 @@ ControlledDhcpv6Srv::commandConfigReloadHandler(const string&, ConstElementPtr a } ConstElementPtr -ControlledDhcpv6Srv::commandGetConfigHandler(const string&, +ControlledDhcpv6Srv::commandConfigGetHandler(const string&, ConstElementPtr /*args*/) { ConstElementPtr config = CfgMgr::instance().getCurrentCfg()->toElement(); return (createAnswer(0, config)); } +ConstElementPtr +ControlledDhcpv6Srv::commandConfigWriteHandler(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 + // 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.")); + } + + // Ok, it's time to write the file. + 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 ControlledDhcpv6Srv::commandSetConfigHandler(const string&, ConstElementPtr args) { @@ -186,8 +258,6 @@ ControlledDhcpv6Srv::processCommand(const std::string& command, if (command == "shutdown") { return (srv->commandShutdownHandler(command, args)); - /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler) - } else if (command == "libreload") { return (srv->commandLibReloadHandler(command, args)); @@ -197,11 +267,15 @@ ControlledDhcpv6Srv::processCommand(const std::string& command, } else if (command == "set-config") { return (srv->commandSetConfigHandler(command, args)); - } else if (command == "get-config") { - return (srv->commandGetConfigHandler(command, args)); + } else if (command == "config-get") { + return (srv->commandConfigGetHandler(command, args)); } else if (command == "leases-reclaim") { return (srv->commandLeasesReclaimHandler(command, args)); + + } else if (command == "config-write") { + return (srv->commandConfigWriteHandler(command, args)); + } return (isc::config::createAnswer(1, "Unrecognized command:" @@ -353,9 +427,18 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port) } server_ = this; // remember this instance for use in callback - // Register supported commands in CommandMgr - CommandMgr::instance().registerCommand("shutdown", - boost::bind(&ControlledDhcpv6Srv::commandShutdownHandler, this, _1, _2)); + // These are the commands always supported by the DHCPv6 server. + // Please keep the list in alphabetic order. + CommandMgr::instance().registerCommand("config-get", + boost::bind(&ControlledDhcpv6Srv::commandConfigGetHandler, this, _1, _2)); + + /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler) + + CommandMgr::instance().registerCommand("config-write", + boost::bind(&ControlledDhcpv6Srv::commandConfigWriteHandler, this, _1, _2)); + + CommandMgr::instance().registerCommand("leases-reclaim", + boost::bind(&ControlledDhcpv6Srv::commandLeasesReclaimHandler, this, _1, _2)); CommandMgr::instance().registerCommand("libreload", boost::bind(&ControlledDhcpv6Srv::commandLibReloadHandler, this, _1, _2)); @@ -363,28 +446,25 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port) CommandMgr::instance().registerCommand("set-config", boost::bind(&ControlledDhcpv6Srv::commandSetConfigHandler, this, _1, _2)); - CommandMgr::instance().registerCommand("get-config", - boost::bind(&ControlledDhcpv6Srv::commandGetConfigHandler, this, _1, _2)); - - CommandMgr::instance().registerCommand("leases-reclaim", - boost::bind(&ControlledDhcpv6Srv::commandLeasesReclaimHandler, this, _1, _2)); + CommandMgr::instance().registerCommand("shutdown", + boost::bind(&ControlledDhcpv6Srv::commandShutdownHandler, this, _1, _2)); // Register statistic related commands CommandMgr::instance().registerCommand("statistic-get", boost::bind(&StatsMgr::statisticGetHandler, _1, _2)); - CommandMgr::instance().registerCommand("statistic-reset", - boost::bind(&StatsMgr::statisticResetHandler, _1, _2)); - - CommandMgr::instance().registerCommand("statistic-remove", - boost::bind(&StatsMgr::statisticRemoveHandler, _1, _2)); - CommandMgr::instance().registerCommand("statistic-get-all", boost::bind(&StatsMgr::statisticGetAllHandler, _1, _2)); + CommandMgr::instance().registerCommand("statistic-reset", + boost::bind(&StatsMgr::statisticResetHandler, _1, _2)); + CommandMgr::instance().registerCommand("statistic-reset-all", boost::bind(&StatsMgr::statisticResetAllHandler, _1, _2)); + CommandMgr::instance().registerCommand("statistic-remove", + boost::bind(&StatsMgr::statisticRemoveHandler, _1, _2)); + CommandMgr::instance().registerCommand("statistic-remove-all", boost::bind(&StatsMgr::statisticRemoveAllHandler, _1, _2)); } @@ -406,18 +486,19 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() { // Close the command socket (if it exists). CommandMgr::instance().closeCommandSocket(); - // Deregister any registered commands - CommandMgr::instance().deregisterCommand("get-config"); - CommandMgr::instance().deregisterCommand("shutdown"); + // Deregister any registered commands (please keep in alphabetic order) + CommandMgr::instance().deregisterCommand("config-get"); + CommandMgr::instance().deregisterCommand("config-write"); CommandMgr::instance().deregisterCommand("libreload"); - CommandMgr::instance().deregisterCommand("set-config"); CommandMgr::instance().deregisterCommand("leases-reclaim"); + CommandMgr::instance().deregisterCommand("set-config"); + CommandMgr::instance().deregisterCommand("shutdown"); CommandMgr::instance().deregisterCommand("statistic-get"); - CommandMgr::instance().deregisterCommand("statistic-reset"); - CommandMgr::instance().deregisterCommand("statistic-remove"); CommandMgr::instance().deregisterCommand("statistic-get-all"); - CommandMgr::instance().deregisterCommand("statistic-reset-all"); + CommandMgr::instance().deregisterCommand("statistic-remove"); CommandMgr::instance().deregisterCommand("statistic-remove-all"); + CommandMgr::instance().deregisterCommand("statistic-reset"); + CommandMgr::instance().deregisterCommand("statistic-reset-all"); } catch (...) { // Don't want to throw exceptions from the destructor. The server diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index d05383e208..214c114c2c 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_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, + commandConfigGetHandler(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. 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. + /// + /// @param command (ignored) + /// @param args may contain optional string argument filename + /// @return status of the configuration file write + isc::data::ConstElementPtr + commandConfigWriteHandler(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/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index addc1ab8d8..f28a60679c 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -211,6 +211,91 @@ 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 checkConfigWrite(const std::string& response_txt, int exp_status, + const std::string& exp_txt = "") { + + 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; + } + } }; @@ -711,4 +796,118 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlChannelStats) { response); } +// Tests that the server properly responds to shtudown command sent +// via ControlChannel +TEST_F(CtrlChannelDhcpv6SrvTest, commandsList) { + 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, "config-get"); + checkListCommands(rsp, "config-write"); + 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"); +} + +// Tests if the server returns its configuration using get-config. +// Note there are separate tests that verify if toElement() called by the +// config-get handler are actually converting the configuration correctly. +TEST_F(CtrlChannelDhcpv6SrvTest, configGet) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"config-get\" }", 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); + ASSERT_EQ(Element::map, cfg->getType()); + EXPECT_TRUE(cfg->get("Dhcp6")); +} + +// Tests if config-write can be called without any parameters. +TEST_F(CtrlChannelDhcpv6SrvTest, configWriteNoFilename) { + 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\": \"config-write\" }", response); + + checkConfigWrite(response, CONTROL_RESULT_SUCCESS, "test1.json"); + ::remove("test1.json"); +} + +// Tests if config-write can be called with a valid filename as parameter. +TEST_F(CtrlChannelDhcpv6SrvTest, configWriteFilename) { + createUnixChannelServer(); + std::string response; + + sendUnixCommand("{ \"command\": \"config-write\", " + "\"arguments\": { \"filename\": \"test2.json\" } }", response); + checkConfigWrite(response, CONTROL_RESULT_SUCCESS, "test2.json"); + ::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."); +} + } // End of anonymous namespace diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index a16e0242ed..cbb8b7adf3 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2015,2017 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 7ff12bbe3f..c5f8beae8a 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2015,2017 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -145,6 +145,8 @@ public: /// /// @param config_file name of the file to write the configuration to /// @return number of files written + /// @throw Unexpected if CfgMgr can't retrieve configuation or file cannot + /// be written virtual size_t writeConfigFile(const std::string& config_file) const;