2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-29 21:18:02 +00:00

[#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
This commit is contained in:
Thomas Markwalder 2025-06-17 09:39:28 -04:00
parent 8ba41dcfaf
commit b3ded306f3
19 changed files with 24 additions and 27 deletions

View File

@ -285,7 +285,7 @@ Starting and Stopping the Control Agent
# from sources using libcfgrpt.a # from sources using libcfgrpt.a
$ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' $ 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 still check for violations but will emit warning logs when they are found
rather than fail with an error. Please see rather than fail with an error. Please see
:ref:`sec-kea-runtime-security-risk-checking` for details. :ref:`sec-kea-runtime-security-risk-checking` for details.

View File

@ -163,7 +163,7 @@ directly. It accepts the following command-line switches:
# from sources using libcfgrpt.a # from sources using libcfgrpt.a
$ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' $ 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 still check for violations but will emit warning logs when they are found
rather than fail with an error. Please see rather than fail with an error. Please see
:ref:`sec-kea-runtime-security-risk-checking` for details. :ref:`sec-kea-runtime-security-risk-checking` for details.

View File

@ -78,7 +78,7 @@ the following command-line switches:
# from sources using libcfgrpt.a # from sources using libcfgrpt.a
$ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' $ 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 still check for violations but will emit warning logs when they are found
rather than fail with an error. Please see rather than fail with an error. Please see
:ref:`sec-kea-runtime-security-risk-checking` for details. :ref:`sec-kea-runtime-security-risk-checking` for details.

View File

@ -78,7 +78,7 @@ the following command-line switches:
# from sources using libcfgrpt.a # from sources using libcfgrpt.a
$ strings src/lib/process/cfgrpt/.libs/libcfgrpt.a | sed -n 's/;;;; //p' $ 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 still check for violations but will emit warning logs when they are found
rather than fail with an error. Please see rather than fail with an error. Please see
:ref:`sec-kea-runtime-security-risk-checking` for details. :ref:`sec-kea-runtime-security-risk-checking` for details.

View File

@ -554,7 +554,7 @@ and DDNS servers since Kea version 2.7.2.
components. components.
The three primary Kea daemons (:iscman:`kea-dhcp4`, :iscman:`kea-dhcp6` and :iscman:`kea-dhcp-ddns`) all support a control 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: .. _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`, 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. :iscman:`kea-dhcp6`, :iscman:`kea-dhcp-ddns`, :iscman:`kea-ctrl-agent`. in Kea 2.7.9
In Kea 3.0 additional checks were added. By default, when a daemon detects a security release. In Kea 3.0 additional checks were added. By default, when a daemon detects
risk it emits an error log and exits. The following checks are performed: 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` - Use of unsupported file paths or permissions as detailed in :ref:`sec-summary-of-path-restrictions`

View File

@ -246,7 +246,7 @@ main(int argc, char* argv[]) {
LOG_WARN(dhcp4_logger, DHCP4_DEVELOPMENT_VERSION); LOG_WARN(dhcp4_logger, DHCP4_DEVELOPMENT_VERSION);
} }
if (amRoot()) { if (amRunningAsRoot()) {
LOG_WARN(dhcp4_logger, DHCP4_ROOT_USER_SECURITY_WARN); LOG_WARN(dhcp4_logger, DHCP4_ROOT_USER_SECURITY_WARN);
} }

View File

@ -246,7 +246,7 @@ main(int argc, char* argv[]) {
LOG_WARN(dhcp6_logger, DHCP6_DEVELOPMENT_VERSION); LOG_WARN(dhcp6_logger, DHCP6_DEVELOPMENT_VERSION);
} }
if (amRoot()) { if (amRunningAsRoot()) {
LOG_WARN(dhcp6_logger, DHCP6_ROOT_USER_SECURITY_WARN); LOG_WARN(dhcp6_logger, DHCP6_ROOT_USER_SECURITY_WARN);
} }

View File

@ -36,7 +36,6 @@ namespace ph = std::placeholders;
namespace { namespace {
/// @brief Test fixture for testing commands for the host-cache library /// @brief Test fixture for testing commands for the host-cache library
//class CommandTest : public ::testing::Test {
class CommandTest : public LogContentTest { class CommandTest : public LogContentTest {
public: public:
/// @brief Constructor /// @brief Constructor

View File

@ -2758,7 +2758,7 @@ LeaseCmdsImpl::leaseWriteHandler(CalloutHandle& handle) {
try { try {
filename = CfgMgr::instance().validatePath(file->stringValue()); filename = CfgMgr::instance().validatePath(file->stringValue());
} catch (const SecurityWarn& ex) { } 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()); .arg(ex.what());
filename = file->stringValue(); filename = file->stringValue();
} catch (const std::exception& ex) { } catch (const std::exception& ex) {

View File

@ -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_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_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_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 = "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_DDNS4_FAILED = "LEASE_CMDS_RESEND_DDNS4_FAILED";
extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6 = "LEASE_CMDS_RESEND_DDNS6"; 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_FAILED", "reason: %1",
"LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR", "evaluating binding-variables for lease: %1 for: %2, reason: %3", "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_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", "lease4-resend-ddns command successful: %1",
"LEASE_CMDS_RESEND_DDNS4_FAILED", "lease4-resend-ddns command failed: %1", "LEASE_CMDS_RESEND_DDNS4_FAILED", "lease4-resend-ddns command failed: %1",
"LEASE_CMDS_RESEND_DDNS6", "lease6-resend-ddns command successful: %1", "LEASE_CMDS_RESEND_DDNS6", "lease6-resend-ddns command successful: %1",

View File

@ -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_FAILED;
extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR; 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_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;
extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED;
extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6;

View File

@ -167,7 +167,7 @@ are logged.
The lease6-wipe command has failed. Both the reason as well as the The lease6-wipe command has failed. Both the reason as well as the
parameters passed are logged. 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 This warning message is issued when security enforcement is disabled
and the path portion of the `filename` parameter of the lease4-write and the path portion of the `filename` parameter of the lease4-write
or lease6-write command does not comply with the supported path. The or lease6-write command does not comply with the supported path. The

View File

@ -3520,7 +3520,7 @@ void Lease4CmdsTest::testLease4WriteSecurityWarn() {
"}"; "}";
std::ostringstream os; 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 '" << " invalid path specified: '/tmp', supported path is '"
<< CfgMgr::instance().getDataDir() << "'"; << CfgMgr::instance().getDataDir() << "'";

View File

@ -425,7 +425,7 @@ TSIGKeyInfoParser::parse(ConstElementPtr key_config) {
} else { } else {
secret = getString(key_config, "secret"); secret = getString(key_config, "secret");
if (file::PathChecker::shouldEnforceSecurity()) { 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) << " (" << getPosition("secret", key_config)
<< ")"); << ")");
} else { } else {

View File

@ -1308,4 +1308,4 @@ TEST(HooksConfig, toElementTest) {
EXPECT_EQ(data::prettyPrint(cfg.toElement()), exp_cfg); EXPECT_EQ(data::prettyPrint(cfg.toElement()), exp_cfg);
} }
} // Anonymous namespae } // Anonymous namespace

View File

@ -100,7 +100,7 @@ public:
file::PathChecker::enableEnforcement(true); file::PathChecker::enableEnforcement(true);
} }
/// @brief Desstructor. /// @brief Destructor.
virtual ~BasicHttpAuthConfigTest() { virtual ~BasicHttpAuthConfigTest() {
file::PathChecker::enableEnforcement(true); file::PathChecker::enableEnforcement(true);
} }

View File

@ -133,7 +133,7 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
LOG_WARN(dctl_logger, DCTL_DEVELOPMENT_VERSION); LOG_WARN(dctl_logger, DCTL_DEVELOPMENT_VERSION);
} }
if (file::amRoot()) { if (file::amRunningAsRoot()) {
LOG_WARN(dctl_logger, DCTL_ROOT_USER_SECURITY_WARN) LOG_WARN(dctl_logger, DCTL_ROOT_USER_SECURITY_WARN)
.arg(app_name_); .arg(app_name_);
} }

View File

@ -18,6 +18,7 @@
#include <dirent.h> #include <dirent.h>
#include <fcntl.h> #include <fcntl.h>
#include <unistd.h>
using namespace isc; using namespace isc;
using namespace isc::util::str; using namespace isc::util::str;
@ -104,7 +105,7 @@ setUmask() {
} }
} }
bool amRoot() { bool amRunningAsRoot() {
return (getuid() == 0 || geteuid() == 0); return (getuid() == 0 || geteuid() == 0);
} }

View File

@ -32,9 +32,6 @@ public:
isc::Exception(file, line, what) {} 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. /// @brief Get the content of a regular file.
/// ///
/// @param file_name The file name. /// @param file_name The file name.
@ -104,7 +101,7 @@ setUmask();
/// @return True if either the uid or the effective /// @return True if either the uid or the effective
/// uid is root. /// uid is root.
bool bool
amRoot(); amRunningAsRoot();
/// @brief Paths on a filesystem /// @brief Paths on a filesystem
struct Path { struct Path {
@ -249,7 +246,7 @@ public:
/// @return validated path as a string (supported path + input file name) /// @return validated path as a string (supported path + input file name)
/// ///
/// @throw BadValue if the input path does not include a 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. /// security is being enforced, SecurityWarn if it is not being enforced.
std::string validatePath(const std::string input_path_str, std::string validatePath(const std::string input_path_str,
bool enforce_path = shouldEnforceSecurity()) const; bool enforce_path = shouldEnforceSecurity()) const;