[#3848] Addressed review comments

 Fixed minor nits

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/dhcp4/main.cc
modified:   src/bin/dhcp6/main.cc
modified:   src/hooks/dhcp/host_cache/tests/command_unittests.cc
modified:   src/hooks/dhcp/lease_cmds/lease_cmds.cc
modified:   src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc
modified:   src/hooks/dhcp/lease_cmds/lease_cmds_messages.h
modified:   src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes
modified:   src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
modified:   src/lib/d2srv/d2_config.cc
modified:   src/lib/hooks/tests/hooks_manager_unittest.cc
modified:   src/lib/http/tests/basic_auth_config_unittests.cc
modified:   src/lib/process/d_controller.cc
modified:   src/lib/util/filesystem.cc
modified:   src/lib/util/filesystem.h

diff --git a/doc/sphinx/arm/agent.rst b/doc/sphinx/arm/agent.rst
index e331f41958bf68c909d255a54c092b0c6e009cd8..e12dab5d84eecfa0b78c42fbd4f12ab6f4d4fd47 100644 (file)
--- a/doc/sphinx/arm/agent.rst
+++ b/doc/sphinx/arm/agent.rst
@@ -285,7 +285,7 @@ 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 secruity restrictions.  The server will 
+-  ``-X`` - As of Kea 3.0, disables security restrictions. The server will 
    still check for violations but will emit warning logs when they are found
    rather than fail with an error. Please see 
    :ref:`sec-kea-runtime-security-risk-checking` for details.

diff --git a/doc/sphinx/arm/ddns.rst b/doc/sphinx/arm/ddns.rst
index 9e39a4b775183d556c6d39810cceb4d55047838a..67ecbd95ba01b7f98dca93882453b69f84b7542a 100644 (file)
--- a/doc/sphinx/arm/ddns.rst
+++ b/doc/sphinx/arm/ddns.rst
@@ -163,7 +163,7 @@ 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 secruity restrictions.  The server will 
+-  ``-X`` - As of Kea 3.0, disables security restrictions. The server will 
    still check for violations but will emit warning logs when they are found
    rather than fail with an error. Please see 
    :ref:`sec-kea-runtime-security-risk-checking` for details.

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index 95413590d012efe37b40a4e8bd113a1e28f45683..2ae635310415970bf78226c28dea7c43c9594f5e 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -78,7 +78,7 @@ 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 secruity restrictions.  The server will 
+-  ``-X`` - As of Kea 3.0, disables security restrictions. The server will 
    still check for violations but will emit warning logs when they are found
    rather than fail with an error. Please see
    :ref:`sec-kea-runtime-security-risk-checking` for details.

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index 06c0e3d0b098cdd65191a9c3a0e81b82238dda62..2f0cbe41b3a525496a5d5cc4853873e671a170dc 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -78,7 +78,7 @@ 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 secruity restrictions.  The server will 
+-  ``-X`` - As of Kea 3.0, disables security restrictions. The server will 
    still check for violations but will emit warning logs when they are found
    rather than fail with an error. Please see 
    :ref:`sec-kea-runtime-security-risk-checking` for details.

diff --git a/doc/sphinx/arm/security.rst b/doc/sphinx/arm/security.rst
index c9cdce582974a5aa1e308e7d56620b283c96ae52..ea46b0e023debdd4f8898fc32b94679673d7a81b 100644 (file)
--- a/doc/sphinx/arm/security.rst
+++ b/doc/sphinx/arm/security.rst
@@ -554,7 +554,7 @@ and DDNS servers since Kea version 2.7.2.
     components.
 
 The three primary Kea daemons (:iscman:`kea-dhcp4`, :iscman:`kea-dhcp6` and :iscman:`kea-dhcp-ddns`) all support a control
-channel, which is implemented as a UNIX socket. The control channel, which opens a UNIX socket, is disabled by default;
+channel, which is implemented as a UNIX socket. The control channel, which opens a UNIX socket, is disabled by default.
 
 .. _sec-kea-runtime-security-risk-checking:
 
@@ -562,9 +562,9 @@ Kea Runtime Security Risk Checking
 ==================================
 
 Runtime security risk checking was initially added to Kea daemons :iscman:`kea-dhcp4`,
-:iscman:`kea-dhcp6`, :iscman:`kea-dhcp-ddns`, :iscman:`kea-ctrl-agent`. in 2.7.9.
-In Kea 3.0 additional checks were added. By default, when a daemon detects a security
-risk it emits an error log and exits. The following checks are performed:
+:iscman:`kea-dhcp6`, :iscman:`kea-dhcp-ddns`, :iscman:`kea-ctrl-agent`. in Kea 2.7.9
+release.  In Kea 3.0 additional checks were added. By default, when a daemon detects
+a security risk it emits an error log and exits. The following checks are performed:
 
 - Use of unsupported file paths or permissions as detailed in :ref:`sec-summary-of-path-restrictions`
 

diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc
index 32b62f6c5302cdf102f022861f7d4360dc4a5fba..cb054b1f506b97e40024abad7659fbbec267d4ec 100644 (file)
--- a/src/bin/dhcp4/main.cc
+++ b/src/bin/dhcp4/main.cc
@@ -246,7 +246,7 @@ main(int argc, char* argv[]) {
             LOG_WARN(dhcp4_logger, DHCP4_DEVELOPMENT_VERSION);
         }
 
-        if (amRoot()) {
+        if (amRunningAsRoot()) {
             LOG_WARN(dhcp4_logger, DHCP4_ROOT_USER_SECURITY_WARN);
         }
 

diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc
index faff52da50fded556cb551a4f28c6a77049041cd..de519ba46a360883616579cc0817af046b6998f3 100644 (file)
--- a/src/bin/dhcp6/main.cc
+++ b/src/bin/dhcp6/main.cc
@@ -246,7 +246,7 @@ main(int argc, char* argv[]) {
             LOG_WARN(dhcp6_logger, DHCP6_DEVELOPMENT_VERSION);
         }
 
-        if (amRoot()) {
+        if (amRunningAsRoot()) {
             LOG_WARN(dhcp6_logger, DHCP6_ROOT_USER_SECURITY_WARN);
         }
 

diff --git a/src/hooks/dhcp/host_cache/tests/command_unittests.cc b/src/hooks/dhcp/host_cache/tests/command_unittests.cc
index b9bb5513e2ea846747b53b6f7dffdd34a91c0d89..359197197d08d349a10f10a523f042ec590deb9c 100644 (file)
--- a/src/hooks/dhcp/host_cache/tests/command_unittests.cc
+++ b/src/hooks/dhcp/host_cache/tests/command_unittests.cc
@@ -36,7 +36,6 @@ namespace ph = std::placeholders;
 namespace {
 
 /// @brief Test fixture for testing commands for the host-cache library
-//class CommandTest : public ::testing::Test {
 class CommandTest : public LogContentTest {
 public:
     /// @brief Constructor

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
index 389cb4097840d362357879bdfd09ada72a2b1f65..5d0b546c70fc788bd88081719f03c94dc7736b02 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
@@ -2758,7 +2758,7 @@ LeaseCmdsImpl::leaseWriteHandler(CalloutHandle& handle) {
         try {
           filename = CfgMgr::instance().validatePath(file->stringValue());
         } catch (const SecurityWarn& ex) {
-            LOG_WARN(lease_cmds_logger, LEASE_CMDS_PATH_SECURITY_WARNING)
+            LOG_WARN(lease_cmds_logger, LEASE_CMDS_PATH_SECURITY_WARN)
                     .arg(ex.what());
             filename = file->stringValue();
         } catch (const std::exception& ex) {

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc
index 9deb7f3d61a7ce452306397f40caab8cc2aeece9..e2105904811f6c2fdc5bfd9ff81eece49923b337 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc
@@ -26,7 +26,7 @@ extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_CONFLICT = "LEASE_
 extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_FAILED = "LEASE_CMDS_LEASES6_COMMITTED_FAILED";
 extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR = "LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR";
 extern const isc::log::MessageID LEASE_CMDS_LOAD_ERROR = "LEASE_CMDS_LOAD_ERROR";
-extern const isc::log::MessageID LEASE_CMDS_PATH_SECURITY_WARNING = "LEASE_CMDS_PATH_SECURITY_WARNING";
+extern const isc::log::MessageID LEASE_CMDS_PATH_SECURITY_WARN = "LEASE_CMDS_PATH_SECURITY_WARN";
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4 = "LEASE_CMDS_RESEND_DDNS4";
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED = "LEASE_CMDS_RESEND_DDNS4_FAILED";
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6 = "LEASE_CMDS_RESEND_DDNS6";
@@ -67,7 +67,7 @@ const char* values[] = {
     "LEASE_CMDS_LEASES6_COMMITTED_FAILED", "reason: %1",
     "LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR", "evaluating binding-variables for lease: %1 for: %2, reason: %3",
     "LEASE_CMDS_LOAD_ERROR", "loading Lease Commands hooks library failed: %1",
-    "LEASE_CMDS_PATH_SECURITY_WARNING", "lease file path specified is NOT SECURE: %1",
+    "LEASE_CMDS_PATH_SECURITY_WARN", "lease file path specified is NOT SECURE: %1",
     "LEASE_CMDS_RESEND_DDNS4", "lease4-resend-ddns command successful: %1",
     "LEASE_CMDS_RESEND_DDNS4_FAILED", "lease4-resend-ddns command failed: %1",
     "LEASE_CMDS_RESEND_DDNS6", "lease6-resend-ddns command successful: %1",

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h
index c2baa81c8ca985cff212878aff6438754f0ad478..96f41052812c76cd647e643aee256b3b63f3ce92 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h
@@ -27,7 +27,7 @@ extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_CONFLICT;
 extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_FAILED;
 extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR;
 extern const isc::log::MessageID LEASE_CMDS_LOAD_ERROR;
-extern const isc::log::MessageID LEASE_CMDS_PATH_SECURITY_WARNING;
+extern const isc::log::MessageID LEASE_CMDS_PATH_SECURITY_WARN;
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4;
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED;
 extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6;

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes
index 34e023454c35bbc6f92333b770bd8d07168fcb5a..35966219b623582a1f04eafc3603aa3b38f89641 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes
@@ -167,7 +167,7 @@ are logged.
 The lease6-wipe command has failed. Both the reason as well as the
 parameters passed are logged.
 
-% LEASE_CMDS_PATH_SECURITY_WARNING lease file path specified is NOT SECURE: %1
+% LEASE_CMDS_PATH_SECURITY_WARN lease file path specified is NOT SECURE: %1
 This warning message is issued when security enforcement is disabled
 and the path portion of the `filename` parameter of the lease4-write
 or lease6-write command does not comply with the supported path. The

diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
index a4fb0a61d3cd34141b24343bb33b9c078c1381e3..90add865335309c2804cf4a5dc8471e873e442b1 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
@@ -3520,7 +3520,7 @@ void Lease4CmdsTest::testLease4WriteSecurityWarn() {
         "}";
 
     std::ostringstream os;
-    os << "LEASE_CMDS_PATH_SECURITY_WARNING lease file path specified is NOT SECURE:"
+    os << "LEASE_CMDS_PATH_SECURITY_WARN lease file path specified is NOT SECURE:"
        << " invalid path specified: '/tmp', supported path is '"
        << CfgMgr::instance().getDataDir() << "'";
 

diff --git a/src/lib/d2srv/d2_config.cc b/src/lib/d2srv/d2_config.cc
index 1400dc3e058e688634a78bde85cdaaa95202e9d2..27804bd35d8bb64e47bb4fca30c25904e6b2af52 100644 (file)
--- a/src/lib/d2srv/d2_config.cc
+++ b/src/lib/d2srv/d2_config.cc
@@ -425,7 +425,7 @@ TSIGKeyInfoParser::parse(ConstElementPtr key_config) {
     } else {
         secret = getString(key_config, "secret");
         if (file::PathChecker::shouldEnforceSecurity()) {
-            isc_throw(D2CfgError, "use of clear text TSIG 'secret' is NOT SECURE ("
+            isc_throw(D2CfgError, "use of clear text TSIG 'secret' is NOT SECURE"
                       << " (" << getPosition("secret", key_config)
                       << ")");
         } else {

diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc
index 7562a1bddf273ee89448b1ef16b3adfcadbe3b1d..776103e2c550618e4782783d24533124fc4c863f 100644 (file)
--- a/src/lib/hooks/tests/hooks_manager_unittest.cc
+++ b/src/lib/hooks/tests/hooks_manager_unittest.cc
@@ -1308,4 +1308,4 @@ TEST(HooksConfig, toElementTest) {
     EXPECT_EQ(data::prettyPrint(cfg.toElement()), exp_cfg);
 }
 
-} // Anonymous namespae
+} // Anonymous namespace

diff --git a/src/lib/http/tests/basic_auth_config_unittests.cc b/src/lib/http/tests/basic_auth_config_unittests.cc
index b13cc1ee5b01706b926200c05db28ca1a00dcf25..5eec9e687a92d15c66333c49e02d3e812354c3de 100644 (file)
--- a/src/lib/http/tests/basic_auth_config_unittests.cc
+++ b/src/lib/http/tests/basic_auth_config_unittests.cc
@@ -100,7 +100,7 @@ public:
         file::PathChecker::enableEnforcement(true);
     }
 
-    /// @brief Desstructor.
+    /// @brief Destructor.
     virtual ~BasicHttpAuthConfigTest() {
         file::PathChecker::enableEnforcement(true);
     }

diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc
index 0d330943c9b9c285f7ec62b028b23f69f2738e0b..8f35366ca8ed7da459d3f9434062f3e7cd299c6d 100644 (file)
--- a/src/lib/process/d_controller.cc
+++ b/src/lib/process/d_controller.cc
@@ -133,7 +133,7 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
         LOG_WARN(dctl_logger, DCTL_DEVELOPMENT_VERSION);
     }
 
-    if (file::amRoot()) {
+    if (file::amRunningAsRoot()) {
         LOG_WARN(dctl_logger, DCTL_ROOT_USER_SECURITY_WARN)
                 .arg(app_name_);
     }

diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc
index 92f6f0df511fb2b684cacf0644cd5d2445e674e1..d2ab9f576190002393f3d251bac90e69b092a3d0 100644 (file)
--- a/src/lib/util/filesystem.cc
+++ b/src/lib/util/filesystem.cc
@@ -18,6 +18,7 @@
 
 #include <dirent.h>
 #include <fcntl.h>
+#include <unistd.h>
 
 using namespace isc;
 using namespace isc::util::str;
@@ -104,7 +105,7 @@ setUmask() {
     }
 }
 
-bool amRoot() {
+bool amRunningAsRoot() {
     return (getuid() == 0 || geteuid() == 0);
 }
 

diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h
index efc9a21ecafbe3db2eace49f633121f36d66850d..3093bd40bd9327bef42299a64e3d4cf23db373b1 100644 (file)
--- a/src/lib/util/filesystem.h
+++ b/src/lib/util/filesystem.h
@@ -32,9 +32,6 @@ public:
         isc::Exception(file, line, what) {}
 };
 
-/// @brief A generic exception that is thrown if a parameter given
-/// violates security check but enfordement is lax.
-
 /// @brief Get the content of a regular file.
 ///
 /// @param file_name The file name.
@@ -104,7 +101,7 @@ setUmask();
 /// @return True if either the uid or the effective
 /// uid is root.
 bool
-amRoot();
+amRunningAsRoot();
 
 /// @brief Paths on a filesystem
 struct Path {
@@ -249,7 +246,7 @@ public:
     /// @return validated path as a string (supported path + input file name)
     ///
     /// @throw BadValue if the input path does not include a file name.
-    /// SecurityError if the parent path does not path the supported path and
+    /// @trhow SecurityError if the parent path does not path the supported path and
     /// security is being enforced, SecurityWarn if it is not being enforced.
     std::string validatePath(const std::string input_path_str,
                              bool enforce_path = shouldEnforceSecurity()) const;