]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#3902] servers disable security on -X
authorThomas Markwalder <tmark@isc.org>
Fri, 23 May 2025 15:20:20 +0000 (11:20 -0400)
committerThomas Markwalder <tmark@isc.org>
Fri, 23 May 2025 15:20:20 +0000 (11:20 -0400)
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

28 files changed:
doc/sphinx/arm/agent.rst
doc/sphinx/arm/ddns.rst
doc/sphinx/arm/dhcp4-srv.rst
doc/sphinx/arm/dhcp6-srv.rst
doc/sphinx/arm/security.rst
src/bin/agent/ca_messages.mes
src/bin/agent/ca_process.cc
src/bin/d2/d2_process.cc
src/bin/dhcp4/dhcp4_messages.mes
src/bin/dhcp4/main.cc
src/bin/dhcp6/dhcp6_messages.mes
src/bin/dhcp6/main.cc
src/lib/config/unix_command_config.cc
src/lib/config/unix_command_config.h
src/lib/d2srv/d2_messages.mes
src/lib/dhcpsrv/cfgmgr.cc
src/lib/dhcpsrv/cfgmgr.h
src/lib/dhcpsrv/legal_log_mgr.cc
src/lib/dhcpsrv/legal_log_mgr.h
src/lib/hooks/hooks_parser.cc
src/lib/hooks/hooks_parser.h
src/lib/hooks/tests/hooks_manager_unittest.cc
src/lib/process/d_controller.cc
src/lib/process/log_parser.cc
src/lib/process/log_parser.h
src/lib/util/filesystem.cc
src/lib/util/filesystem.h
src/lib/util/tests/filesystem_unittests.cc

index b6c58e026579a0587d45a203795f500b18ad15cb..a9cbcdc1baa2ac9f1a4018673c65397fe5bfdc65 100644 (file)
@@ -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:
 
index 384a2dd44ac791f9d3d92570446c781c1ff5d2c2..90c1d3e4de03cf6e556a682665a5297deffc47e0 100644 (file)
@@ -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.
 
index 31e66b9bc04f0e72db384989d5177971ce5b1943..82612d64563cf65a94020330b2d0b59a56e82063 100644 (file)
@@ -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
index 410716258d6c2fedc2e4e6bffdae64e98091e32d..4f16f604f88e5f3393760bb43950336c8ae5f04b 100644 (file)
@@ -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
index 783724f1c53af3e8de0a9de4ab8e4379a419d88e..f7b8226d31d378d543db0163573266a884bb0830 100644 (file)
@@ -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
 -----------------------
index 756186d0e57d43dd5804c4f15d6f5880aa6166d4..92671461c2740031020a56129701947e008b99de 100644 (file)
@@ -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.
index 601d5ffb20c023df28ff0f8178dac37720444b4c..b3164ca24426e84bf4e2b0ee36d8afec569fc066 100644 (file)
@@ -15,6 +15,7 @@
 #include <asiolink/io_error.h>
 #include <cc/command_interpreter.h>
 #include <config/timeouts.h>
+#include <util/filesystem.h>
 #include <boost/pointer_cast.hpp>
 
 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 =
index 7d17961b8fe05e647d8ffd2a821a89d94558bab1..7db49a34741e664bfe72c3bc010e6f5b386635c6 100644 (file)
 #include <d2srv/d2_tsig_key.h>
 #include <hooks/hooks.h>
 #include <hooks/hooks_manager.h>
+#include <util/filesystem.h>
 
 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>(D2Controller::instance());
     try {
index c7cdc546cb55ad09d1368e3ef55d80a2e01953f0..332232ec2508a35dd2ce78fcc6382fc047bb909d 100644 (file)
@@ -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.
index 91dc013e265898851662dc553d8e3cd03dbf7f69..4f88e294aa09a3084778b096c47d9c213e8b49f8 100644 (file)
@@ -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);
 
index de4e4d2ea0d5ca56407378085a81cc3e1f5077f2..6f75fe6aba36613399e5834f2404d64890f5d572 100644 (file)
@@ -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.
index 9a9a34f62152acc5e5c84c71a8078a8a1afa04e4..7ab199981f379bdfe3969b0ead088b5764c365b4 100644 (file)
@@ -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);
 
index 9e2fa16e4bcfc4343f1357e973fb9743a029300b..c05851071288d516ec2ed2f4936020a408040a5e 100644 (file)
@@ -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 = "
index b3700f3e9270aaee971aff3f10b23ca9313d6c16..41573cfb4995e70dc846fa3dcc631463d8aabb47 100644 (file)
@@ -13,6 +13,7 @@
 #include <asiolink/unix_domain_socket_acceptor.h>
 #include <cc/cfg_to_element.h>
 #include <cc/user_context.h>
+#include <util/filesystem.h>
 
 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
     ///
index 3e14e7cefca32ebc7c6026c531c34010a80add9c..c9ac59a7b4893a307ba64398ab73497ee6840af3 100644 (file)
@@ -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.
index e1937f70765eadc2c9b256ceca300fcc1dc488a7..edd46be53f9c73926368e944b82e051d15981fdf 100644 (file)
@@ -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
index 56e71f4129294071dd68301a00572fbe4ba0ee1b..50313761f02ccdaabeeda0416770b52c5ae754b1 100644 (file)
@@ -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.
     ///
index e3f5f0cfdf2f323d52b2ba22a5d45cad57e7f07b..d654e1cc5a20806bcfb5c61c5c733cf486482603 100644 (file)
@@ -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
index 07e21133aaca5dcc3f655fd26d4a24c5ee6d955a..ad1975c09705827d0226a28d95c623c6972dbc2d 100644 (file)
@@ -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.
     ///
index a79a255c18da8495453c0cabbfbf62a4799c4974..6969823d9a6c929b098411ed156ff7ce6493b199 100644 (file)
@@ -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
index 3ac675bd71b01b81a350cffe30f93b37c6d7b8fc..41e79b5eea36a085c4e48e8bc45718c49c6eee22 100644 (file)
@@ -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.
     ///
index 6fcf9889d2751ddf16c08249236ded4a425b47f8..eafa1f32f4265bf62edc0cb8060fd6490631f768 100644 (file)
@@ -10,6 +10,7 @@
 #include <hooks/hooks_manager.h>
 #include <hooks/hooks_parser.h>
 #include <hooks/server_hooks.h>
+#include <util/filesystem.h>
 #include <testutils/gtest_utils.h>
 
 #include <hooks/tests/common_test_class.h>
@@ -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_);
         }
     }
index 2f8c2e902c0ec669d73d6e2601193326309f5cd3..0d0902b67d8a031fb13f8ddf7735f75ba93d7a1b 100644 (file)
@@ -16,6 +16,7 @@
 #include <log/logger.h>
 #include <log/logger_support.h>
 #include <util/encode/encode.h>
+#include <util/filesystem.h>
 #include <process/daemon.h>
 #include <process/d_log.h>
 #include <process/d_controller.h>
@@ -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 <config file name> : mandatory,"
               << " specify name of configuration file" << std::endl
               << "  -t <config file name> : 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;
index f052730cfc0a93b82b81dc0319809cb9282e5242..fb59cfd44b0fc7b0d382f40bdb88b6c0d2de9460 100644 (file)
@@ -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<LoggingDestination>& destination,
index 79ee1f6573fb372e18f44e57d7f1848ce42a288c..411eef597b34822396b102f6146c90e27eb7761e 100644 (file)
@@ -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:
 
index fd65e7bcb58b117edb147ed8be8d796f2783b7a4..50c2088082160c0f4cc9850e8e6e5df6caf5c6c7 100644 (file)
@@ -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
index 8147d8c5b708525ce63816e8df4ab313b3850651..9c1d6896ba82c6fe9bf0a533b3f099f32fa217a7 100644 (file)
@@ -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.
index 58e9d18529c70f795913838815e76b2e765f1bc9..3d82234163e95964814e0e4f0affa2ae5d7dc921 100644 (file)
@@ -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