From: Thomas Markwalder Date: Fri, 23 May 2025 15:20:20 +0000 (-0400) Subject: [#3902] servers disable security on -X X-Git-Tag: Kea-3.0.0~182 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9adef66837f310487938b0d1b9423ca6638a3ee1;p=thirdparty%2Fkea.git [#3902] servers disable security on -X modified: doc/sphinx/arm/agent.rst modified: doc/sphinx/arm/ddns.rst modified: doc/sphinx/arm/dhcp4-srv.rst modified: doc/sphinx/arm/dhcp6-srv.rst modified: doc/sphinx/arm/security.rst modified: src/bin/agent/ca_messages.mes modified: src/bin/agent/ca_process.cc modified: src/bin/d2/d2_process.cc modified: src/bin/dhcp4/dhcp4_messages.mes modified: src/bin/dhcp4/main.cc modified: src/bin/dhcp6/dhcp6_messages.mes modified: src/bin/dhcp6/main.cc modified: src/lib/config/unix_command_config.cc modified: src/lib/config/unix_command_config.h modified: src/lib/d2srv/d2_messages.mes modified: src/lib/dhcpsrv/cfgmgr.cc modified: src/lib/dhcpsrv/cfgmgr.h modified: src/lib/dhcpsrv/legal_log_mgr.cc modified: src/lib/dhcpsrv/legal_log_mgr.h modified: src/lib/hooks/hooks_parser.cc modified: src/lib/hooks/hooks_parser.h modified: src/lib/hooks/tests/hooks_manager_unittest.cc modified: src/lib/process/d_controller.cc modified: src/lib/process/log_parser.cc modified: src/lib/process/log_parser.h modified: src/lib/util/filesystem.cc modified: src/lib/util/filesystem.h modified: src/lib/util/tests/filesystem_unittests.cc --- diff --git a/doc/sphinx/arm/agent.rst b/doc/sphinx/arm/agent.rst index b6c58e0265..a9cbcdc1ba 100644 --- a/doc/sphinx/arm/agent.rst +++ b/doc/sphinx/arm/agent.rst @@ -285,6 +285,13 @@ Starting and Stopping the Control Agent # from sources using libcfgrpt.a $ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' +- ``-X`` - As of Kea 3.0, disables path and permissions restrictions. + The server will emit a warning at startup that sercurity restrctions + have been disabled. Do not use this mode of operation without careful + consideration and takng any necessary precautions. Falure to do so can + expose deployments to security vulnerabilities. For more information + please read section :ref:`securing-a-kea-deployment`. + The CA is started by running its binary and specifying the configuration file it should use. For example: diff --git a/doc/sphinx/arm/ddns.rst b/doc/sphinx/arm/ddns.rst index 384a2dd44a..90c1d3e4de 100644 --- a/doc/sphinx/arm/ddns.rst +++ b/doc/sphinx/arm/ddns.rst @@ -130,14 +130,6 @@ directly. It accepts the following command-line switches: flag is convenient for temporarily switching the server into maximum verbosity, e.g. when debugging. -- ``-v`` - displays the Kea version and exits. - -- ``-V`` - displays the extended Kea version and exits. - -- ``-W`` - displays the Kea configuration report and exits. The report - is a copy of the ``config.report`` file produced by ``meson setup``; - it is embedded in the executable binary. - - ``-t file`` - specifies the configuration file to be tested. :iscman:`kea-dhcp-ddns` attempts to load it and conducts sanity checks. Certain checks are possible only while running the actual @@ -146,6 +138,14 @@ directly. It accepts the following command-line switches: messages to standard output and errors to standard error when testing the configuration. +- ``-v`` - displays the Kea version and exits. + +- ``-V`` - displays the extended Kea version and exits. + +- ``-W`` - displays the Kea configuration report and exits. The report + is a copy of the ``config.report`` file produced by ``meson setup``; + it is embedded in the executable binary. + The contents of the ``config.report`` file may also be accessed by examining certain libraries in the installation tree or in the source tree. @@ -163,6 +163,13 @@ directly. It accepts the following command-line switches: # from sources using libcfgrpt.a $ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' +- ``-X`` - As of Kea 3.0, disables path and permissions restrictions. + The server will emit a warning at startup that sercurity restrctions + have been disabled. Do not use this mode of operation without careful + consideration and takng any necessary precautions. Falure to do so can + expose deployments to security vulnerabilities. For more information + please read section :ref:`securing-a-kea-deployment`. + Upon startup, the module loads its configuration and begins listening for NCRs based on that configuration. diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index 31e66b9bc0..82612d6456 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -78,6 +78,13 @@ the following command-line switches: # from sources using libcfgrpt.a $ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' +- ``-X`` - As of Kea 3.0, disables path and permissions restrictions. + The server will emit a warning at startup that sercurity restrctions + have been disabled. Do not use this mode of operation without careful + consideration and takng any necessary precautions. Falure to do so can + expose deployments to security vulnerabilities. For more information + please read section :ref:`securing-a-kea-deployment`. + On startup, the server detects available network interfaces and attempts to open UDP sockets on all interfaces listed in the configuration file. Since the DHCPv4 server opens privileged ports, it diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index 410716258d..4f16f604f8 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/doc/sphinx/arm/dhcp6-srv.rst @@ -78,6 +78,13 @@ the following command-line switches: # from sources using libcfgrpt.a $ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' +- ``-X`` - As of Kea 3.0, disables path and permissions restrictions. + The server will emit a warning at startup that sercurity restrctions + have been disabled. Do not use this mode of operation without careful + consideration and takng any necessary precautions. Falure to do so can + expose deployments to security vulnerabilities. For more information + please read section :ref:`securing-a-kea-deployment`. + On startup, the server detects available network interfaces and attempts to open UDP sockets on all interfaces listed in the configuration file. Since the DHCPv6 server opens privileged ports, it diff --git a/doc/sphinx/arm/security.rst b/doc/sphinx/arm/security.rst index 783724f1c5..f7b8226d31 100644 --- a/doc/sphinx/arm/security.rst +++ b/doc/sphinx/arm/security.rst @@ -266,6 +266,8 @@ Configuring only one or two string parameters results in an error. The :iscman:`kea-shell` tool also supports TLS. +.. _securing-a-kea-deployment: + Securing a Kea Deployment ========================= @@ -425,7 +427,15 @@ the following table: | Unix Sockets | ``var/run/kea`` | ``KEA_CONTROL_SOCKET_DIR`` | +-------------------------------------+---------------------------------------+-------------------------------+ +.. note: + As of Kea 3.0, the path and permissions restrictions may be disabled by adding ``-X`` + to command line of the Kea servers. The server will emit a warning at startup that + sercurity restrctions have been disabled. Do not use this mode of operation without + careful consideration and takng any necessary precautions. Falure to do so may expose + deployments to security vulnerabilities. This command line option is supported by + all of the daemons: ``kea-dhcp4``, ``kea-dhcp6``, ``kea-dhcp-ddns``, and + ``kea-ctrl-agent``. Cryptography Components ----------------------- diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes index 756186d0e5..92671461c2 100644 --- a/src/bin/agent/ca_messages.mes +++ b/src/bin/agent/ca_messages.mes @@ -81,3 +81,11 @@ event loop. This informational message indicates that the Control Agent has processed all configuration information and is ready to begin processing. The version is also printed. + +% CTRL_AGENT_SECURITY_CHECKS_DISABLED Invoked with command line option -X, Security checks are disabled!! +This warning is emitted when internal security checks normally +performed by kea-ctrl-agent have been disabled via command line opion '-X'. +This means the server is not enforcing restrictions on resource +paths or permissions. This mode of operation may expose your +environment to ecurity vulnerabilities and should only be used +after consideration. diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 601d5ffb20..b3164ca244 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include using namespace isc::asiolink; @@ -22,7 +23,7 @@ using namespace isc::config; using namespace isc::data; using namespace isc::http; using namespace isc::process; - +using namespace isc::util::file; namespace isc { namespace agent { @@ -43,6 +44,10 @@ void CtrlAgentProcess::run() { LOG_INFO(agent_logger, CTRL_AGENT_STARTED).arg(VERSION); + if (!PathChecker::shouldEnforceSecurity()) { + LOG_WARN(agent_logger, CTRL_AGENT_SECURITY_CHECKS_DISABLED); + } + try { // Register commands. CtrlAgentControllerPtr controller = diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index 7d17961b8f..7db49a3474 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -19,12 +19,14 @@ #include #include #include +#include using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; using namespace isc::hooks; using namespace isc::process; +using namespace isc::util::file; namespace { @@ -93,6 +95,11 @@ D2Process::init() { void D2Process::run() { LOG_INFO(d2_logger, DHCP_DDNS_STARTED).arg(VERSION); + + if (!PathChecker::shouldEnforceSecurity()) { + LOG_WARN(d2_logger, DHCP_DDNS_SECURITY_CHECKS_DISABLED); + } + D2ControllerPtr controller = boost::dynamic_pointer_cast(D2Controller::instance()); try { diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index c7cdc546cb..332232ec25 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -1182,3 +1182,11 @@ expected: the erroneous response is dropped, the request query is displayed. An DHCPOFFER for the 0.0.0.0 address was generated for a client requesting the v6-only-preferred (108) option but the option is not in the response as expected: the erroneous response is dropped, the discover query is displayed. + +% DHCP4_SECURITY_CHECKS_DISABLED Invoked with command line option -X, Security checks are disabled!! +This warning is emitted when internal security checks normally +performed by kea-dhcp4 have been disabled via command line opion '-X'. +This means the server is not enforcing restrictions on resource +paths or permissions. This mode of operation may expose your +environment to security vulnerabilities and should only be used +after careful consideration. diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index 91dc013e26..4f88e294aa 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -27,6 +27,7 @@ using namespace isc::data; using namespace isc::dhcp; using namespace isc::process; +using namespace isc::util::file; using namespace std; /// This file contains entry point (main() function) for standard DHCPv4 server @@ -53,7 +54,7 @@ usage() { << endl; cerr << endl; cerr << "Usage: " << DHCP4_NAME - << " -[v|V|W] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; + << " -[v|V|W|X] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; cerr << " -v: print version number and exit" << endl; cerr << " -V: print extended version and exit" << endl; cerr << " -W: display the configuration report and exit" << endl; @@ -66,6 +67,7 @@ usage() { << "(useful for testing only)" << endl; cerr << " -P number: specify non-standard client port number 1-65535 " << "(useful for testing only)" << endl; + cerr << " -X: disables security restrictions" << endl; exit(EXIT_FAILURE); } } // namespace @@ -89,7 +91,7 @@ main(int argc, char* argv[]) { // This is the DHCPv4 server CfgMgr::instance().setFamily(AF_INET); - while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:")) != -1) { + while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:X")) != -1) { switch (ch) { case 'd': verbose_mode = true; @@ -152,6 +154,10 @@ main(int argc, char* argv[]) { } break; + case 'X': // relax security checks + PathChecker::enableEnforcement(false); + break; + default: usage(); } @@ -240,6 +246,10 @@ main(int argc, char* argv[]) { LOG_WARN(dhcp4_logger, DHCP4_DEVELOPMENT_VERSION); } + if (!PathChecker::shouldEnforceSecurity()) { + LOG_WARN(dhcp4_logger, DHCP4_SECURITY_CHECKS_DISABLED); + } + // Create the server instance. ControlledDhcpv4Srv server(server_port_number, client_port_number); diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index de4e4d2ea0..6f75fe6aba 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -1159,3 +1159,11 @@ such modification. The clients will remember previous server-id, and will use it to extend their leases. As a result, they will have to go through a rebinding phase to re-acquire their leases and associate them with a new server id. + +% DHCP6_SECURITY_CHECKS_DISABLED Invoked with command line option -X, Security checks are disabled!! +This warning is emitted when internal security checks normally +performed by kea-dhcp6 have been disabled via command line opion '-X'. +This means the server is not enforcing restrictions on resource +paths or permissions. This mode of operation may expose your +environment to security vulnerabilities and should only be used +after careful consideration. diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 9a9a34f621..7ab199981f 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -27,6 +27,7 @@ using namespace isc::data; using namespace isc::dhcp; using namespace isc::process; +using namespace isc::util::file; using namespace std; /// This file contains entry point (main() function) for standard DHCPv6 server @@ -53,7 +54,7 @@ usage() { << endl; cerr << endl; cerr << "Usage: " << DHCP6_NAME - << " -[v|V|W] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; + << " -[v|V|W|X] [-d] [-{c|t|T} cfgfile] [-p number] [-P number]" << endl; cerr << " -v: print version number and exit" << endl; cerr << " -V: print extended version and exit" << endl; cerr << " -W: display the configuration report and exit" << endl; @@ -66,6 +67,7 @@ usage() { << "(useful for testing only)" << endl; cerr << " -P number: specify non-standard client port number 1-65535 " << "(useful for testing only)" << endl; + cerr << " -X: disables security restrictions" << endl; exit(EXIT_FAILURE); } } // namespace @@ -89,7 +91,7 @@ main(int argc, char* argv[]) { // This is the DHCPv6 server CfgMgr::instance().setFamily(AF_INET6); - while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:")) != -1) { + while ((ch = getopt(argc, argv, "dvVWc:p:P:t:T:X")) != -1) { switch (ch) { case 'd': verbose_mode = true; @@ -152,6 +154,10 @@ main(int argc, char* argv[]) { } break; + case 'X': // relax security checks + PathChecker::enableEnforcement(false); + break; + default: usage(); } @@ -240,6 +246,10 @@ main(int argc, char* argv[]) { LOG_WARN(dhcp6_logger, DHCP6_DEVELOPMENT_VERSION); } + if (!PathChecker::shouldEnforceSecurity()) { + LOG_WARN(dhcp6_logger, DHCP6_SECURITY_CHECKS_DISABLED); + } + // Create the server instance. ControlledDhcpv6Srv server(server_port_number, client_port_number); diff --git a/src/lib/config/unix_command_config.cc b/src/lib/config/unix_command_config.cc index 9e2fa16e4b..c058510712 100644 --- a/src/lib/config/unix_command_config.cc +++ b/src/lib/config/unix_command_config.cc @@ -111,14 +111,13 @@ UnixCommandConfig::getSocketPath(bool reset /* = false */, } std::string -UnixCommandConfig::validatePath(const std::string socket_path, - bool enforce /* = true */) { +UnixCommandConfig::validatePath(const std::string socket_path) { if (!socket_path_checker_) { getSocketPath(); } - auto valid_path = socket_path_checker_->validatePath(socket_path, enforce); - if (enforce && !(socket_path_checker_->pathHasPermissions(socket_path_perms_))) { + auto valid_path = socket_path_checker_->validatePath(socket_path); + if (!socket_path_checker_->pathHasPermissions(socket_path_perms_)) { isc_throw (DhcpConfigError, "socket path:" << socket_path_checker_->getPath() << " does not exist or does not have permssions = " diff --git a/src/lib/config/unix_command_config.h b/src/lib/config/unix_command_config.h index b3700f3e92..41573cfb49 100644 --- a/src/lib/config/unix_command_config.h +++ b/src/lib/config/unix_command_config.h @@ -13,6 +13,7 @@ #include #include #include +#include namespace isc { namespace config { @@ -54,13 +55,9 @@ public: /// sockets. /// /// @param socket_path path to validate. - /// @param enforce enables validation against the supported path and - /// permissions. - /// If false simply returns the input path. /// /// @return validated path - static std::string validatePath(const std::string socket_path, - bool enforce = true); + static std::string validatePath(const std::string socket_path); /// @brief Fetches the required socket path permissions mask /// diff --git a/src/lib/d2srv/d2_messages.mes b/src/lib/d2srv/d2_messages.mes index 3e14e7cefc..c9ac59a7b4 100644 --- a/src/lib/d2srv/d2_messages.mes +++ b/src/lib/d2srv/d2_messages.mes @@ -447,3 +447,11 @@ server. Logged at debug log level 50. This is a debug message issued when DHCP_DDNS receives sends a DNS update response from a DNS server. + +% DHCP_DDNS_SECURITY_CHECKS_DISABLED Invoked with command line option -X, Security checks are disabled!! +This warning is emitted when internal security checks normally +performed by kea-dhcp-ddns have been disabled via command line opion '-X'. +This means the server is not enforcing restrictions on resource +paths or permissions. This mode of operation may expose your +environment to ecurity vulnerabilities and should only be used +after consideration. diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index e1937f7076..edd46be53f 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -37,8 +37,8 @@ CfgMgr::getDataDir(bool reset /* = false */, const std::string explicit_path /* } std::string -CfgMgr::validatePath(const std::string data_path, bool enforce_path /* = true */) const { - return (data_dir_checker_->validatePath(data_path, enforce_path)); +CfgMgr::validatePath(const std::string data_path) const { + return (data_dir_checker_->validatePath(data_path)); } void diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 56e71f4129..50313761f0 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -95,12 +95,9 @@ public: /// @brief Validates a file path against the supported directory for DHDP data. /// /// @param data_path data path to validate. - /// @param enforce_path enables validation against the supported path. - /// If false verifies only that the path contains a file name. /// /// @return validated path - std::string validatePath(const std::string data_path, - bool enforce_path = true) const; + std::string validatePath(const std::string data_path) const; /// @brief Updates the DHCP-DDNS client configuration to the given value. /// diff --git a/src/lib/dhcpsrv/legal_log_mgr.cc b/src/lib/dhcpsrv/legal_log_mgr.cc index e3f5f0cfdf..d654e1cc5a 100644 --- a/src/lib/dhcpsrv/legal_log_mgr.cc +++ b/src/lib/dhcpsrv/legal_log_mgr.cc @@ -392,13 +392,12 @@ LegalLogMgr::getLogPath(bool reset /* = false */, const std::string explicit_pat } std::string -LegalLogMgr::validatePath(const std::string logpath, - bool enforce_path /* = true */) { +LegalLogMgr::validatePath(const std::string logpath) { if (!legal_log_path_checker_) { getLogPath(); } - return (legal_log_path_checker_->validateDirectory(logpath, enforce_path)); + return (legal_log_path_checker_->validateDirectory(logpath)); } } // namespace dhcp diff --git a/src/lib/dhcpsrv/legal_log_mgr.h b/src/lib/dhcpsrv/legal_log_mgr.h index 07e21133aa..ad1975c097 100644 --- a/src/lib/dhcpsrv/legal_log_mgr.h +++ b/src/lib/dhcpsrv/legal_log_mgr.h @@ -71,12 +71,9 @@ public: /// log files. /// /// @param logpath path to validate. - /// @param enforce_path enables validation against the supported path. - /// If false simply returns the input path. /// /// @return validated path - static std::string validatePath(const std::string logpath, - bool enforce_path = true); + static std::string validatePath(const std::string logpath); /// @brief Parse database specification. /// diff --git a/src/lib/hooks/hooks_parser.cc b/src/lib/hooks/hooks_parser.cc index a79a255c18..6969823d9a 100644 --- a/src/lib/hooks/hooks_parser.cc +++ b/src/lib/hooks/hooks_parser.cc @@ -42,13 +42,12 @@ HooksLibrariesParser::getHooksPath(bool reset /* = false */, const std::string e } std::string -HooksLibrariesParser::validatePath(const std::string libpath, - bool enforce_path /* = true */) { +HooksLibrariesParser::validatePath(const std::string libpath) { if (!hooks_path_checker_) { getHooksPath(); } - return (hooks_path_checker_->validatePath(libpath, enforce_path)); + return (hooks_path_checker_->validatePath(libpath)); } // @todo use the flat style, split into list and item diff --git a/src/lib/hooks/hooks_parser.h b/src/lib/hooks/hooks_parser.h index 3ac675bd71..41e79b5eea 100644 --- a/src/lib/hooks/hooks_parser.h +++ b/src/lib/hooks/hooks_parser.h @@ -62,12 +62,9 @@ public: /// @brief Validates a library path against the supported path for hooks libraries. /// /// @param libpath library path to validate. - /// @param enforce_path enables validation against the supported path. - /// If false verifies only that the path contains a file name. /// /// @return validated path - static std::string validatePath(const std::string libpath, - bool enforce_path = true); + static std::string validatePath(const std::string libpath); /// @brief Fetches the supported Hooks path. /// diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc index 6fcf9889d2..eafa1f32f4 100644 --- a/src/lib/hooks/tests/hooks_manager_unittest.cc +++ b/src/lib/hooks/tests/hooks_manager_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -30,6 +31,7 @@ using namespace isc; using namespace isc::hooks; using namespace isc::data; +using namespace isc::util::file; using namespace std; namespace { @@ -1237,6 +1239,7 @@ TEST_F(HooksParserTest, validatePathEnforcePathFalse) { } }; + PathChecker::enableEnforcement(false); for (auto scenario : scenarios) { std::ostringstream oss; oss << " Scenario at line: " << scenario.line_; @@ -1244,11 +1247,11 @@ TEST_F(HooksParserTest, validatePathEnforcePathFalse) { std::string validated_path; if (scenario.exp_error_.empty()) { ASSERT_NO_THROW_LOG(validated_path = - HooksLibrariesParser::validatePath(scenario.lib_path_, false)); + HooksLibrariesParser::validatePath(scenario.lib_path_)); EXPECT_EQ(validated_path, scenario.exp_path_); } else { ASSERT_THROW_MSG(validated_path = - HooksLibrariesParser::validatePath(scenario.lib_path_, false), + HooksLibrariesParser::validatePath(scenario.lib_path_), BadValue, scenario.exp_error_); } } diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index 2f8c2e902c..0d0902b67d 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -254,7 +255,7 @@ DControllerBase::parseArgs(int argc, char* argv[]) { optarg = 0; opterr = 0; optind = 1; - std::string opts("dvVWc:t:" + getCustomOpts()); + std::string opts("dvVWc:t:X" + getCustomOpts()); // Defer exhausting of arguments to the end. ExhaustOptions e(argc, argv, opts); @@ -298,6 +299,10 @@ DControllerBase::parseArgs(int argc, char* argv[]) { } break; + case 'X': // relax security checks + file::PathChecker::enableEnforcement(false); + break; + case '?': { char const saved_optopt(optopt); std::string const saved_optarg(optarg ? optarg : std::string()); @@ -834,7 +839,8 @@ DControllerBase::usage(const std::string & text) { << " -c : mandatory," << " specify name of configuration file" << std::endl << " -t : check the" - << " configuration file and exit" << std::endl; + << " configuration file and exit" << std::endl + << " -X: disables security restrictions" << std::endl; // add any derivation specific usage std::cerr << getUsageText() << std::endl; diff --git a/src/lib/process/log_parser.cc b/src/lib/process/log_parser.cc index f052730cfc..fb59cfd44b 100644 --- a/src/lib/process/log_parser.cc +++ b/src/lib/process/log_parser.cc @@ -146,13 +146,12 @@ LogConfigParser::getLogPath(bool reset /* = false */, const std::string explicit } std::string -LogConfigParser::validatePath(const std::string logpath, - bool enforce_path /* = true */) { +LogConfigParser::validatePath(const std::string logpath) { if (!log_path_checker_) { getLogPath(); } - return (log_path_checker_->validatePath(logpath, enforce_path)); + return (log_path_checker_->validatePath(logpath)); } void LogConfigParser::parseOutputOptions(std::vector& destination, diff --git a/src/lib/process/log_parser.h b/src/lib/process/log_parser.h index 79ee1f6573..411eef597b 100644 --- a/src/lib/process/log_parser.h +++ b/src/lib/process/log_parser.h @@ -75,11 +75,9 @@ public: /// @brief Validates a library path against the supported path for log files. /// /// @param logpath path to validate. - /// @param enforce_path enables validation against the supported path. - /// If false verifies only that the path contains a file name. /// /// @return validated path - static std::string validatePath(const std::string logpath, bool enforce_path = true); + static std::string validatePath(const std::string logpath); private: diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index fd65e7bcb5..50c2088082 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -27,6 +27,7 @@ namespace isc { namespace util { namespace file { + string getContent(string const& file_name) { if (!exists(file_name)) { @@ -241,7 +242,8 @@ string TemporaryDirectory::dirName() { PathChecker::PathChecker(const std::string default_path, const std::string env_name /* = "" */) - : default_path_(default_path), env_name_(env_name) { + : default_path_(default_path), env_name_(env_name), + default_overridden_(false) { getPath(true); } @@ -263,6 +265,8 @@ PathChecker::getPath(bool reset /* = false */, while (!path_.empty() && path_.back() == '/') { path_.pop_back(); } + + default_overridden_ = (path_ != default_path_); } return (path_); @@ -270,7 +274,7 @@ PathChecker::getPath(bool reset /* = false */, std::string PathChecker::validatePath(const std::string input_path_str, - bool enforce_path /* = true */) const { + bool enforce_path /* = PathChecker::shouldEnforceSecurity() */) const { Path input_path(trim(input_path_str)); auto filename = input_path.filename(); if (filename.empty()) { @@ -284,7 +288,6 @@ PathChecker::validatePath(const std::string input_path_str, return (input_path_str); } - // We only allow absolute path equal to default. Catch an invalid path. if (parent_path != path_) { isc_throw(BadValue, "invalid path specified: '" @@ -299,7 +302,7 @@ PathChecker::validatePath(const std::string input_path_str, std::string PathChecker::validateDirectory(const std::string input_path_str, - bool enforce_path /* = true */) const { + bool enforce_path /* = PathChecker::shouldEnforceSecurity() */) const { std::string input_copy = trim(input_path_str); if (!enforce_path) { return(input_copy); @@ -323,10 +326,26 @@ PathChecker::validateDirectory(const std::string input_path_str, } bool -PathChecker::pathHasPermissions(mode_t permissions) { - return(hasPermissions(path_, permissions)); +PathChecker::pathHasPermissions(mode_t permissions, bool enforce_perms + /* = PathChecker::shouldEnforceSecurity() */) const { + return((!enforce_perms) || hasPermissions(path_, permissions)); +} + +bool +PathChecker::isDefaultOverridden() { + return (default_overridden_); } +bool PathChecker::shouldEnforceSecurity() { + return (enforce_security_); +} + +void PathChecker::enableEnforcement(bool enable) { + enforce_security_ = enable; +} + +bool PathChecker::enforce_security_ = true; + } // namespace file } // namespace util } // namespace isc diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index 8147d8c5b7..9c1d6896ba 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -224,7 +224,7 @@ public: /// @throw BadValue if the input path does not include a file name or if the /// it the parent path does not path the supported path. std::string validatePath(const std::string input_path_str, - bool enforce_path = true) const; + bool enforce_path = shouldEnforceSecurity()) const; /// @brief Validates a directory against a supported path. /// @@ -244,14 +244,14 @@ public: /// @throw BadValue if the input directory does not match the supported /// path. std::string validateDirectory(const std::string input_path_str, - bool enforce_path = true) const; + bool enforce_path = shouldEnforceSecurity()) const; - /// @brief Tests that the supported path has the given permissions. + /// @param enforce_perms Enables permsissions check. If false the function + /// simply returns true. /// - /// @param permissions mode_t mask of required permissions. - /// @return True if the path's permissions exactly match the permissions - /// parameter. - bool pathHasPermissions(mode_t permissions); + /// @return True if the path points to a file or a directory, false otherwise. + bool pathHasPermissions(mode_t permissions, + bool enforce_perms = shouldEnforceSecurity()) const; /// @brief Fetches the default path. std::string getDefaultPath() const { @@ -263,6 +263,17 @@ public: return (env_name_); } + /// @brief Indicates if the default path has been overridden. + bool isDefaultOverridden(); + + /// @brief Indicates security checks should be enforced. + static bool shouldEnforceSecurity(); + + /// @brief Enables or disables security enforcment checks. + /// + /// @param enable true to enable security checks, false to disable. + static void enableEnforcement(bool enable); + private: /// @brief Default supported path. std::string default_path_; @@ -272,6 +283,12 @@ private: /// @brief The supported path currently in effect. std::string path_; + + /// @brief Tracks if default has been overridden. + bool default_overridden_; + + /// @brief True if security checks should be enforced, false if not. + static bool enforce_security_; }; /// @brief Defines a pointer to a PathChecker. diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index 58e9d18529..3d82234163 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -237,6 +237,7 @@ public: // Clear the environment path. unsetenv(env_name_.c_str()); + PathChecker::enableEnforcement(true); } /// @brief Destructor @@ -247,6 +248,8 @@ public: } else { unsetenv(env_name_.c_str()); } + + PathChecker::enableEnforcement(true); } /// @brief Name of env variable for overriding default path. @@ -295,7 +298,7 @@ TEST_F(PathCheckerTest, getPathExplicit) { } // Verifies PathChecker::validatePath() when enforce_path is true. -TEST(PathChecker, validatePathEnforcePath) { +TEST_F(PathCheckerTest, validatePathEnforcePath) { std::string def_path(TEST_DATA_BUILDDIR); struct Scenario { int line_; @@ -370,7 +373,7 @@ TEST(PathChecker, validatePathEnforcePath) { } // Verifies PathChecker::validatePath() when enforce_path is false. -TEST(PathChecker, validatePathEnforcePathFalse) { +TEST_F(PathCheckerTest, validatePathEnforcePathFalse) { std::string def_path(TEST_DATA_BUILDDIR); struct Scenario { int line_; @@ -417,6 +420,10 @@ TEST(PathChecker, validatePathEnforcePathFalse) { } }; + ASSERT_TRUE(PathChecker::shouldEnforceSecurity()); + PathChecker::enableEnforcement(false); + ASSERT_FALSE(PathChecker::shouldEnforceSecurity()); + // Create a PathChecker with a supported path of def_path. PathChecker checker(def_path); for (auto scenario : scenarios) { @@ -426,18 +433,18 @@ TEST(PathChecker, validatePathEnforcePathFalse) { std::string validated_path; if (scenario.exp_error_.empty()) { ASSERT_NO_THROW_LOG(validated_path = - checker.validatePath(scenario.lib_path_, false)); + checker.validatePath(scenario.lib_path_)); EXPECT_EQ(validated_path, scenario.exp_path_); } else { ASSERT_THROW_MSG(validated_path = - checker.validatePath(scenario.lib_path_, false), + checker.validatePath(scenario.lib_path_), BadValue, scenario.exp_error_); } } } // Verifies PathChecker::validateDirectory() when enforce_path is true. -TEST(PathChecker, validateDirectoryEnforcePath) { +TEST_F(PathCheckerTest, validateDirectoryEnforcePath) { std::string def_path(TEST_DATA_BUILDDIR); struct Scenario { int line_; @@ -512,7 +519,7 @@ TEST(PathChecker, validateDirectoryEnforcePath) { } // Verifies PathChecker::validateDirectory() when enforce_path is false. -TEST(PathChecker, validateDirectoryEnforcePathFalse) { +TEST_F(PathCheckerTest, validateDirectoryEnforcePathFalse) { std::string def_path(TEST_DATA_BUILDDIR); struct Scenario { int line_; @@ -552,6 +559,10 @@ TEST(PathChecker, validateDirectoryEnforcePathFalse) { } }; + ASSERT_TRUE(PathChecker::shouldEnforceSecurity()); + PathChecker::enableEnforcement(false); + ASSERT_FALSE(PathChecker::shouldEnforceSecurity()); + // Create a PathChecker with a supported path of def_path. PathChecker checker(def_path); for (auto scenario : scenarios) { @@ -561,14 +572,30 @@ TEST(PathChecker, validateDirectoryEnforcePathFalse) { std::string validated_path; if (scenario.exp_error_.empty()) { ASSERT_NO_THROW_LOG(validated_path = - checker.validateDirectory(scenario.lib_path_, false)); + checker.validateDirectory(scenario.lib_path_)); EXPECT_EQ(validated_path, scenario.exp_path_); } else { ASSERT_THROW_MSG(validated_path = - checker.validateDirectory(scenario.lib_path_, false), + checker.validateDirectory(scenario.lib_path_), BadValue, scenario.exp_error_); } } } +/// @brief Check pathHasPermissions. +TEST_F(PathCheckerTest, pathHasPermissions) { + PathChecker checker(TEST_DATA_BUILDDIR); + + ASSERT_TRUE(PathChecker::shouldEnforceSecurity()); + mode_t current_permissions = getPermissions(TEST_DATA_BUILDDIR); + EXPECT_TRUE(checker.pathHasPermissions(current_permissions)); + + current_permissions = ~current_permissions; + EXPECT_FALSE(checker.pathHasPermissions(current_permissions)); + + PathChecker::enableEnforcement(false); + ASSERT_FALSE(PathChecker::shouldEnforceSecurity()); + EXPECT_TRUE(checker.pathHasPermissions(current_permissions)); +} + } // namespace