From 9e8d3e5cf2668386b13a92e3dbd0fecdc60a7252 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 7 Oct 2025 09:32:41 -0400 Subject: [PATCH] [#4142] Addressed review comments Mostly nits. D2ClientMgr::qualifyName() now throws if partial name is emtpy. modified: changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname modified: src/bin/dhcp4/dhcp4_messages.cc modified: src/bin/dhcp4/dhcp4_messages.mes modified: src/bin/dhcp6/dhcp6_messages.cc modified: src/bin/dhcp6/dhcp6_messages.mes modified: src/lib/dhcpsrv/d2_client_mgr.cc modified: src/lib/dhcpsrv/d2_client_mgr.h modified: src/lib/dhcpsrv/tests/d2_client_unittest.cc --- .../CVE-2025-11232-catch-empty-sanitized-hostname | 4 ++-- src/bin/dhcp4/dhcp4_messages.cc | 4 ++++ src/bin/dhcp4/dhcp4_messages.mes | 4 ++-- src/bin/dhcp6/dhcp6_messages.cc | 2 ++ src/bin/dhcp6/dhcp6_messages.mes | 4 ++-- src/lib/dhcpsrv/d2_client_mgr.cc | 10 +++++++--- src/lib/dhcpsrv/d2_client_mgr.h | 5 +++-- src/lib/dhcpsrv/tests/d2_client_unittest.cc | 12 ++---------- 8 files changed, 24 insertions(+), 21 deletions(-) diff --git a/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname b/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname index 1af77ebc16..79fb39677a 100644 --- a/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname +++ b/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname @@ -1,6 +1,6 @@ [sec] tmark When a hostname or FQDN received from a client is - reduced to an empty string by hostname sanitiziing, + reduced to an empty string by hostname sanitizing, kea-dhcp4 and kea-dhcp6 will now drop the option. - CVE:2025-11232 + CVE:2025-11232 (Gitlab #4142) diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc index 8a84d648d8..4ca6cbfa51 100644 --- a/src/bin/dhcp4/dhcp4_messages.cc +++ b/src/bin/dhcp4/dhcp4_messages.cc @@ -26,9 +26,11 @@ extern const isc::log::MessageID DHCP4_CLASS_UNCONFIGURED = "DHCP4_CLASS_UNCONFI extern const isc::log::MessageID DHCP4_CLIENTID_IGNORED_FOR_LEASES = "DHCP4_CLIENTID_IGNORED_FOR_LEASES"; extern const isc::log::MessageID DHCP4_CLIENT_FQDN_DATA = "DHCP4_CLIENT_FQDN_DATA"; extern const isc::log::MessageID DHCP4_CLIENT_FQDN_PROCESS = "DHCP4_CLIENT_FQDN_PROCESS"; +extern const isc::log::MessageID DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY = "DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY"; extern const isc::log::MessageID DHCP4_CLIENT_HOSTNAME_DATA = "DHCP4_CLIENT_HOSTNAME_DATA"; extern const isc::log::MessageID DHCP4_CLIENT_HOSTNAME_MALFORMED = "DHCP4_CLIENT_HOSTNAME_MALFORMED"; extern const isc::log::MessageID DHCP4_CLIENT_HOSTNAME_PROCESS = "DHCP4_CLIENT_HOSTNAME_PROCESS"; +extern const isc::log::MessageID DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY = "DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY"; extern const isc::log::MessageID DHCP4_CLIENT_NAME_PROC_FAIL = "DHCP4_CLIENT_NAME_PROC_FAIL"; extern const isc::log::MessageID DHCP4_CONFIG_COMPLETE = "DHCP4_CONFIG_COMPLETE"; extern const isc::log::MessageID DHCP4_CONFIG_LOAD_FAIL = "DHCP4_CONFIG_LOAD_FAIL"; @@ -207,9 +209,11 @@ const char* values[] = { "DHCP4_CLIENTID_IGNORED_FOR_LEASES", "%1: not using client identifier for lease allocation for subnet %2", "DHCP4_CLIENT_FQDN_DATA", "%1: Client sent FQDN option: %2", "DHCP4_CLIENT_FQDN_PROCESS", "%1: processing Client FQDN option", + "DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY", "%1: sanitizing client's FQDN option '%2' yielded an empty string", "DHCP4_CLIENT_HOSTNAME_DATA", "%1: client sent Hostname option: %2", "DHCP4_CLIENT_HOSTNAME_MALFORMED", "%1: client hostname option malformed: %2", "DHCP4_CLIENT_HOSTNAME_PROCESS", "%1: processing client's Hostname option", + "DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY", "%1: sanitizing client's Hostname option '%2' yielded an empty string", "DHCP4_CLIENT_NAME_PROC_FAIL", "%1: failed to process the fqdn or hostname sent by a client: %2", "DHCP4_CONFIG_COMPLETE", "DHCPv4 server has completed configuration: %1", "DHCP4_CONFIG_LOAD_FAIL", "configuration error using file: %1, reason: %2", diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index fd9e4d17f9..19eb56d75d 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -164,14 +164,14 @@ This debug message is issued when the server starts processing the Hostname option sent in the client's query. The argument includes the client and transaction identification information. -% DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY %1: sanitiziing client's Hostname option '%2' yielded an empty string +% DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY %1: sanitizing client's Hostname option '%2' yielded an empty string Logged at debug log level 50. This debug message is issued when the result of sanitizing the hostname option(12) sent by the client is an empty string. When this occurs the server will ignore the hostname option. The arguments include the client and the hostname option it sent. -% DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY %1: sanitiziing client's FQDN option '%2' yielded an empty string +% DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY %1: sanitizing client's FQDN option '%2' yielded an empty string Logged at debug log level 50. This debug message is issued when the result of sanitizing the FQDN option(81) sent by the client is an empty string. When this occurs diff --git a/src/bin/dhcp6/dhcp6_messages.cc b/src/bin/dhcp6/dhcp6_messages.cc index 9a85ec58dd..de1f25df6f 100644 --- a/src/bin/dhcp6/dhcp6_messages.cc +++ b/src/bin/dhcp6/dhcp6_messages.cc @@ -27,6 +27,7 @@ extern const isc::log::MessageID DHCP6_CLASSES_ASSIGNED = "DHCP6_CLASSES_ASSIGNE extern const isc::log::MessageID DHCP6_CLASSES_ASSIGNED_AFTER_SUBNET_SELECTION = "DHCP6_CLASSES_ASSIGNED_AFTER_SUBNET_SELECTION"; extern const isc::log::MessageID DHCP6_CLASS_ASSIGNED = "DHCP6_CLASS_ASSIGNED"; extern const isc::log::MessageID DHCP6_CLASS_UNCONFIGURED = "DHCP6_CLASS_UNCONFIGURED"; +extern const isc::log::MessageID DHCP6_CLIENT_FQDN_SCRUBBED_EMPTY = "DHCP6_CLIENT_FQDN_SCRUBBED_EMPTY"; extern const isc::log::MessageID DHCP6_CONFIG_COMPLETE = "DHCP6_CONFIG_COMPLETE"; extern const isc::log::MessageID DHCP6_CONFIG_LOAD_FAIL = "DHCP6_CONFIG_LOAD_FAIL"; extern const isc::log::MessageID DHCP6_CONFIG_PACKET_QUEUE = "DHCP6_CONFIG_PACKET_QUEUE"; @@ -204,6 +205,7 @@ const char* values[] = { "DHCP6_CLASSES_ASSIGNED_AFTER_SUBNET_SELECTION", "%1: client packet has been assigned to the following classes: %2", "DHCP6_CLASS_ASSIGNED", "%1: client packet has been assigned to the following class: %2", "DHCP6_CLASS_UNCONFIGURED", "%1: client packet belongs to an unconfigured class: %2", + "DHCP6_CLIENT_FQDN_SCRUBBED_EMPTY", "%1: sanitizing client's FQDN option '%2' yielded an empty string", "DHCP6_CONFIG_COMPLETE", "DHCPv6 server has completed configuration: %1", "DHCP6_CONFIG_LOAD_FAIL", "configuration error using file: %1, reason: %2", "DHCP6_CONFIG_PACKET_QUEUE", "DHCPv6 packet queue info after configuration: %1", diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index cc239d0668..82a6a011db 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -1174,9 +1174,9 @@ 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_CLIENT_FQDN_SCRUBBED_EMPTY %1: sanitiziing client's FQDN option '%2' yielded an empty string +% DHCP6_CLIENT_FQDN_SCRUBBED_EMPTY %1: sanitizing client's FQDN option '%2' yielded an empty string Logged at debug log level 50. This debug message is issued when the result of sanitizing the -FQDN option(81) sent by the client is an empty string. When this occurs +FQDN option(39) sent by the client is an empty string. When this occurs the server will ignore the FQDN option. The arguments include the client and the FQDN option it sent. diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc index 10e44140bc..54c815176e 100644 --- a/src/lib/dhcpsrv/d2_client_mgr.cc +++ b/src/lib/dhcpsrv/d2_client_mgr.cc @@ -186,11 +186,15 @@ std::string D2ClientMgr::qualifyName(const std::string& partial_name, const DdnsParams& ddns_params, const bool trailing_dot) const { + if (partial_name.empty()) { + isc_throw(BadValue, "D2ClientMgr::qualifyName" + " - partial_name cannot be an empty string"); + } + std::ostringstream gen_name; gen_name << partial_name; std::string suffix = ddns_params.getQualifyingSuffix(); - if ((!suffix.empty()) && (!partial_name.empty()) - && (partial_name.back() != '.')) { + if (!suffix.empty() && (partial_name.back() != '.')) { bool suffix_present = true; std::string str = gen_name.str(); auto suffix_rit = suffix.rbegin(); @@ -242,7 +246,7 @@ D2ClientMgr::qualifyName(const std::string& partial_name, // If the trailing dot should not be appended but it is present, // remove it. if ((len > 0) && (str[len - 1] == '.')) { - gen_name.str(str.substr(0,len-1)); + gen_name.str(str.substr(0, len-1)); } } diff --git a/src/lib/dhcpsrv/d2_client_mgr.h b/src/lib/dhcpsrv/d2_client_mgr.h index b160b47fe1..45437f4b79 100644 --- a/src/lib/dhcpsrv/d2_client_mgr.h +++ b/src/lib/dhcpsrv/d2_client_mgr.h @@ -30,8 +30,8 @@ namespace isc { namespace dhcp { -/// @brief Exception thrown upon attempt to add subnet with an ID that belongs -/// to the subnet that already exists. +/// @brief Exception thrown when host name sanitizing reduces +/// the domain name to an empty string. class FQDNScrubbedEmpty : public Exception { public: FQDNScrubbedEmpty(const char* file, size_t line, const char* what) : @@ -205,6 +205,7 @@ public: /// suffix itself is empty (i.e. ""). /// /// @return std::string containing the qualified name. + /// @throw BadValue if partial_name is empty. std::string qualifyName(const std::string& partial_name, const DdnsParams& ddns_params, const bool trailing_dot) const; diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc index 15dbed9eb5..00375d0066 100644 --- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc +++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc @@ -628,10 +628,9 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) { qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot); EXPECT_EQ("somehost.suffix.com", qualified_name); - // Verify that an empty name does not crash. + // Verify that an empty name throws. partial_name = ""; - qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot); - EXPECT_EQ("", qualified_name); + ASSERT_THROW(mgr.qualifyName(partial_name, *ddns_params_, do_not_dot), BadValue); // Verify that an empty suffix and false flag, does not change the name subnet_->setDdnsQualifyingSuffix(""); @@ -665,11 +664,6 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) { // suffix is blank and trailing dot is false qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_not_dot); EXPECT_EQ("somehost", qualified_name); - - // Verify that an empty name and an empty suffix does not crash. - partial_name = ""; - qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot); - EXPECT_EQ("", qualified_name); } /// @brief Tests the qualifyName method's ability to avoid duplicating @@ -1305,6 +1299,4 @@ TEST_F(D2ClientMgrParamsTest, adjustDomainNameV6ScrubbedEmpty) { FQDNScrubbedEmpty, "___."); } - - } // end of anonymous namespace -- 2.47.3