From: Thomas Markwalder Date: Wed, 30 Jul 2025 13:35:16 +0000 (-0400) Subject: [#3949] Do not add suffix to qualifed host names X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=05159da1080ff57f0d09c7a4987a875aa394c6de;p=thirdparty%2Fkea.git [#3949] Do not add suffix to qualifed host names /src/bin/dhcp4/tests/host_unittest.cc /src/bin/dhcp6/tests/host_unittest.cc Updated tests /src/lib/dhcpsrv/d2_client_mgr.cc D2ClientMgr::qualifyName() - don't add suffix to names that end with a dot. /src/lib/dhcpsrv/d2_client_mgr.h D2ClientMgr::adjustDomainName() - strip trailing dots from T::PARTIAL FQDNs before calling qualifyName() /src/lib/dhcpsrv/tests/d2_client_unittest.cc TEST_F(D2ClientMgrParamsTest, qualifyName) - updated --- diff --git a/changelog_unreleased/3949-ddns-qualifying-suffix-added-to-fqdn-reservations-with-different-suffixes b/changelog_unreleased/3949-ddns-qualifying-suffix-added-to-fqdn-reservations-with-different-suffixes new file mode 100644 index 0000000000..b305d946ad --- /dev/null +++ b/changelog_unreleased/3949-ddns-qualifying-suffix-added-to-fqdn-reservations-with-different-suffixes @@ -0,0 +1,4 @@ +[bug] tmark + Avoid adding the qualifying-suffix to fully qualified + host names specified in host reservations. + (Gitlab #3949) diff --git a/src/bin/dhcp4/tests/host_unittest.cc b/src/bin/dhcp4/tests/host_unittest.cc index efa0672ff8..20f8ae6727 100644 --- a/src/bin/dhcp4/tests/host_unittest.cc +++ b/src/bin/dhcp4/tests/host_unittest.cc @@ -41,12 +41,13 @@ const char* CONFIGS[] = { "{ \"interfaces-config\": {\n" " \"interfaces\": [ \"*\" ]\n" "},\n" + "\"ddns-qualifying-suffix\": \"example.org.\",\n" "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\",\n" " \"duid\", \"client-id\" ],\n" "\"reservations\": [ \n" "{\n" " \"hw-address\": \"aa:bb:cc:dd:ee:ff\",\n" - " \"hostname\": \"hw-host-dynamic\"\n" + " \"hostname\": \"hw-host-dynamic.already.com.\"\n" "},\n" "{\n" " \"hw-address\": \"01:02:03:04:05:06\",\n" @@ -55,12 +56,12 @@ const char* CONFIGS[] = { "},\n" "{\n" " \"hw-address\": \"02:02:03:04:05:06\",\n" - " \"hostname\": \"hw-host-fixed-in-range\",\n" + " \"hostname\": \"hw-host-fixed-in-range.example.org.\",\n" " \"ip-address\": \"10.0.0.77\"\n" "},\n" "{\n" " \"duid\": \"01:02:03:04:05\",\n" - " \"hostname\": \"duid-host\"\n" + " \"hostname\": \"duid-host.\"\n" "},\n" "{\n" " \"circuit-id\": \"'charter950'\",\n" @@ -590,7 +591,7 @@ TEST_F(HostTest, globalHardwareDynamicAddress) { Dhcp4Client client(srv_, Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); - runDoraTest(CONFIGS[0], client, "hw-host-dynamic", "10.0.0.10"); + runDoraTest(CONFIGS[0], client, "hw-host-dynamic.already.com", "10.0.0.10"); } // Verifies that a client matched to a global in-subnet address reservation @@ -600,7 +601,7 @@ TEST_F(HostTest, globalHardwareFixedAddressInRange) { Dhcp4Client client(srv_, Dhcp4Client::SELECTING); client.setHWAddress("02:02:03:04:05:06"); - runDoraTest(CONFIGS[0], client, "hw-host-fixed-in-range", "10.0.0.77"); + runDoraTest(CONFIGS[0], client, "hw-host-fixed-in-range.example.org", "10.0.0.77"); } // Verifies that a client matched to a global out-of-range address reservation @@ -610,7 +611,7 @@ TEST_F(HostTest, globalHardwareFixedAddressOutOfRange) { Dhcp4Client client(srv_, Dhcp4Client::SELECTING); client.setHWAddress("01:02:03:04:05:06"); - runDoraTest(CONFIGS[0], client, "hw-host-fixed-out-of-range", "10.0.0.10"); + runDoraTest(CONFIGS[0], client, "hw-host-fixed-out-of-range.example.org", "10.0.0.10"); } // Verifies that a client can be matched to a global reservation by DUID @@ -641,7 +642,7 @@ TEST_F(HostTest, globalCircuitId) { // Set the circuit id client.setCircuitId("charter950"); - runDoraTest(CONFIGS[0], client, "circuit-id-host", "10.0.0.10"); + runDoraTest(CONFIGS[0], client, "circuit-id-host.example.org", "10.0.0.10"); } // Verifies that a client can be matched to a global reservation by client-id @@ -655,7 +656,7 @@ TEST_F(HostTest, globalClientID) { // - 11:22:33:44:55:66 - is an actual DUID for which there is a client.includeClientId("01:11:22:33:44:55:66"); - runDoraTest(CONFIGS[0], client, "client-id-host", "10.0.0.10"); + runDoraTest(CONFIGS[0], client, "client-id-host.example.org", "10.0.0.10"); } // Verifies that even when a matching global reservation exists, diff --git a/src/bin/dhcp6/tests/host_unittest.cc b/src/bin/dhcp6/tests/host_unittest.cc index c030d8622d..380c0befb9 100644 --- a/src/bin/dhcp6/tests/host_unittest.cc +++ b/src/bin/dhcp6/tests/host_unittest.cc @@ -68,6 +68,7 @@ const char* CONFIGS[] = { "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " + "\"ddns-qualifying-suffix\":\"example.org.\", " "\"subnet6\": [ " " { " " \"id\": 1, " @@ -82,7 +83,8 @@ const char* CONFIGS[] = { " }," " {" " \"duid\": \"01:02:03:05\"," - " \"ip-addresses\": [ \"2001:db8:1:1::babf\" ]" + " \"ip-addresses\": [ \"2001:db8:1:1::babf\" ]," + " \"hostname\": \"hare.mad-hatter.com.\"" " } ]" " } ]" "}", @@ -1551,7 +1553,7 @@ TEST_F(HostTest, basicSarrs) { // and lease has reserved hostname Lease6Ptr lease_server = checkLease(lease_client); ASSERT_TRUE(lease_server); - EXPECT_EQ("alice", lease_server->hostname_); + EXPECT_EQ("alice.example.org", lease_server->hostname_); // Now redo the client, adding one to the DUID client.clearConfig(); @@ -1569,7 +1571,7 @@ TEST_F(HostTest, basicSarrs) { // and that the server lease has NO hostname lease_server = checkLease(lease_client); ASSERT_TRUE(lease_server); - EXPECT_EQ("", lease_server->hostname_); + EXPECT_EQ("hare.mad-hatter.com", lease_server->hostname_); // Now redo the client with yet another DUID and verify that // we get a dynamic address. diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc index 4397d4f51d..84ee11d9fb 100644 --- a/src/lib/dhcpsrv/d2_client_mgr.cc +++ b/src/lib/dhcpsrv/d2_client_mgr.cc @@ -187,11 +187,10 @@ D2ClientMgr::qualifyName(const std::string& partial_name, const DdnsParams& ddns_params, const bool trailing_dot) const { std::ostringstream gen_name; - gen_name << partial_name; std::string suffix = ddns_params.getQualifyingSuffix(); - bool suffix_present = true; - if (!suffix.empty()) { + if (!suffix.empty() && partial_name.back() != '.') { + bool suffix_present = true; std::string str = gen_name.str(); auto suffix_rit = suffix.rbegin(); if (*suffix_rit == '.') { @@ -199,10 +198,6 @@ D2ClientMgr::qualifyName(const std::string& partial_name, } auto gen_rit = str.rbegin(); - if (*gen_rit == '.') { - ++gen_rit; - } - while (suffix_rit != suffix.rend()) { if ((gen_rit == str.rend()) || (*suffix_rit != *gen_rit)) { // They don't match. diff --git a/src/lib/dhcpsrv/d2_client_mgr.h b/src/lib/dhcpsrv/d2_client_mgr.h index c9f44c98c7..7454c5f223 100644 --- a/src/lib/dhcpsrv/d2_client_mgr.h +++ b/src/lib/dhcpsrv/d2_client_mgr.h @@ -491,7 +491,6 @@ D2ClientMgr::adjustDomainName(const T& fqdn, T& fqdn_resp, const DdnsParams& ddn } else { // Sanitize the name the client sent us, if we're configured to do so. std::string client_name = fqdn.getDomainName(); - isc::util::str::StringSanitizerPtr sanitizer = ddns_params.getHostnameSanitizer(); if (sanitizer) { // We need the raw text form, so we can replace escaped chars @@ -521,6 +520,12 @@ D2ClientMgr::adjustDomainName(const T& fqdn, T& fqdn_resp, const DdnsParams& ddn // If the supplied name is partial, qualify it by adding the suffix. if (fqdn.getDomainNameType() == T::PARTIAL) { + if (client_name.back() == '.') { + // By definition a partial cannot end in a dot, sanitizing above + // may have added one. Strip it, so we'll add the suffix (if one). + client_name.pop_back(); + } + fqdn_resp.setDomainName(qualifyName(client_name, ddns_params, true), T::FULL); } else { fqdn_resp.setDomainName(client_name, T::FULL); diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc index 7de98aee4b..ebdff514d1 100644 --- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc +++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc @@ -639,13 +639,8 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) { qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_dot); EXPECT_EQ("somehost.hasdot.com.", qualified_name); - // Verify that the qualifying suffix gets appended without an - // extraneous dot when partial_name ends with a "." - qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_dot); - EXPECT_EQ("somehost.hasdot.com.", qualified_name); - - // Verify that a name with a trailing dot does not get an extraneous - // dot when the suffix is blank + // Verify that the qualifying suffix does not get appended nor an + // extraneous dot added to a name with a trailing dot. subnet_->setDdnsQualifyingSuffix(""); qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_dot); EXPECT_EQ("somehost.", qualified_name); @@ -664,7 +659,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); - } /// @brief Tests the qualifyName method's ability to avoid duplicating @@ -802,6 +796,12 @@ TEST_F(D2ClientMgrParamsTest, adjustDomainNameV4) { "myhost.example.com.", Option4ClientFqdn::FULL, "myhost.example.com.", Option4ClientFqdn::FULL }, + { + "RCM_NEVER #4, partial client name with traling .", + D2ClientConfig::RCM_NEVER, + "myhost.", Option4ClientFqdn::PARTIAL, + "myhost.suffix.com.", Option4ClientFqdn::FULL + }, { "RCM_ALWAYS #1, empty client name", D2ClientConfig::RCM_ALWAYS, @@ -917,6 +917,12 @@ TEST_F(D2ClientMgrParamsTest, adjustDomainNameV6) { "myhost.example.com.", Option6ClientFqdn::FULL, "myhost.example.com.", Option6ClientFqdn::FULL }, + { + "RCM_NEVER #4, partial client name with trailing .", + D2ClientConfig::RCM_NEVER, + "myhost.", Option6ClientFqdn::PARTIAL, + "myhost.suffix.com.", Option6ClientFqdn::FULL + }, { "RCM_ALWAYS #1, empty client name", D2ClientConfig::RCM_ALWAYS,