[#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

diff --git a/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname b/changelog_unreleased/CVE-2025-11232-catch-empty-sanitized-hostname
index 1af77ebc168a51b27b81ca15e8837f707ed129fa..79fb39677a4472ee0b917c8cb40ee3644d778193 100644 (file)
--- 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 8a84d648d882b5c77aa746419b84532546c91964..4ca6cbfa51b7bd910d4f1fce2be76dfd74961c1e 100644 (file)
--- 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 fd9e4d17f9bd2da81bac9c6cb2ac5ca160bbae38..19eb56d75da2508b8ac82dd869e8f7e858169551 100644 (file)
--- 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 9a85ec58dd914494d988991fde2403057dba7065..de1f25df6f0892dac82b45053cd2cc276f718b3e 100644 (file)
--- 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 cc239d0668dd812be234d08b1de5c3c127f739d6..82a6a011dbc59b037c6a907d577297d58bcefe22 100644 (file)
--- 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 10e44140bcbb8ef14560126ee5a8f8419041eaf3..54c815176e8f56c95cdb3e98bc2c512b3a722ea0 100644 (file)
--- 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 b160b47fe186405f5980253bee826cea4f048032..45437f4b7995b35c9fa2103e518a4c713cd84bd7 100644 (file)
--- 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 15dbed9eb599cc5a074fecc4d6957884025b3526..00375d0066c7abb495955125df074434a81adf94 100644 (file)
--- 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