[#3356] Avoid FQDN update when lifetime is zero

/src/bin/dhcp6/dhcp6_srv.cc
    Dhcpv6Srv::generateFqdn() - modified to return early if
    the iaaddr valid lifetime is zero.

/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
    TEST_F(Dhcpv6SrvTest, generateFqdnUpdate)
    TEST_F(Dhcpv6SrvTest, generateFqdnNoUpdate) - new tests

/src/bin/dhcp6/tests/dhcp6_test_utils.h
    class BaseServerTest - now derives from LogContentTest

Added ChangeLog entry

diff --git a/ChangeLog b/ChangeLog
index c3e74b22333a34fdb569298b526a75c721ed24dd..e316ef42e6cd56570b8338d0113b6c968a21840a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2248.  [bug]           tmark
+       Fixed a corner-case isssue in kea-dhcp6 that was 
+       causing it to attempt to update a lease for an address
+       with a generated FQDN even though the address was not
+       available to be leased.
+       (Gitlab #3356)
+
 Kea 2.6.0 (stable) released on May 29, 2024
 
 2247.  [build]         razvan

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index c4783d83c6b028bc4dbf223fe8cd59d1fe48040a..6ac3b6aa4de0650b42be708db955212b128b55c8 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -4580,12 +4580,14 @@ Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer,
         return;
     }
 
-    // If it has any IAAddr, use the first one to generate unique FQDN.
+    // If it has any IAAddr with a valid lifetime > 0, use it to
+    // generate unique FQDN.
     Option6IAAddrPtr iaaddr = boost::dynamic_pointer_cast<
         Option6IAAddr>(ia->getOption(D6O_IAADDR));
-    if (!iaaddr) {
+    if (!iaaddr || iaaddr->getValid() == 0) {
         return;
     }
+
     // Get the IPv6 address acquired by the client.
     IOAddress addr = iaaddr->getAddress();
     std::string generated_name =

diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 78cc98dea3f0596179a7d66b6668a938f21f4e2a..9c813bf5e832ff25751f326c743bd86080faee7e 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -194,7 +194,25 @@ const char* CONFIGS[] = {
     "       \"subnet\": \"2001:db8:1::/48\","
     "       \"user-context\": { \"secure\": false }"
     "    } ] "
-    "}"
+    "}",
+
+    // Configuration 5:
+    // - used in advertiseOptions
+    "{ \"interfaces-config\": { \n"
+    "  \"interfaces\": [ \"*\" ] \n"
+    "}, \n"
+    "\"preferred-lifetime\": 3000, \n"
+    "\"rebind-timer\": 2000,  \n"
+    "\"renew-timer\": 1000,  \n"
+    "\"ddns-replace-client-name\": \"always\", "
+    "\"ddns-qualifying-suffix\": \"example.com\", "
+    "\"subnet6\": [ {  \n"
+    "    \"id\": 1,  \n"
+    "    \"pools\": [ { \"pool\": \"2001:db8:1::1-2001:db8:1::1\" } ], \n"
+    "    \"subnet\": \"2001:db8:1::/64\",  \n"
+    "    \"interface\": \"eth0\"  \n"
+    "    } ], \n"
+    "\"valid-lifetime\": 4000 }",
 };
 
 } // namespace
@@ -3931,6 +3949,137 @@ TEST_F(Dhcpv6SrvTest, calculateTeeTimers) {
     }
 }
 
+// This test verifies that generateFdqn() updates the lease's FQDN when
+// the lease is active.
+TEST_F(Dhcpv6SrvTest, generateFqdnUpdate) {
+    IfaceMgrTestConfig test_config(true);
+
+    NakedDhcpv6Srv srv(0);
+
+    // Configure with a pool of 1 and ddns-replace-client-name = "always"
+    ASSERT_NO_THROW(configure(CONFIGS[5], srv));
+
+    // Lease the only address in the pool to a client 1.
+    OptionPtr clientid = generateClientId();
+    const IOAddress addr("2001:db8:1::1");
+    const uint32_t iaid = 234;
+    Lease6Ptr used(new Lease6(Lease::TYPE_NA, addr, duid_, iaid, 3000, 4000, 1));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Let's create a RENEW of the leased address for client 2.
+    Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_RENEW, 1234));
+    req->setRemoteAddr(IOAddress("fe80::abcd"));
+    req->setIface("eth0");
+    req->setIndex(ETH0_INDEX);
+    req->addOption(createIA(Lease::TYPE_NA, addr, 128, iaid));
+    req->addOption(
+        Option6ClientFqdnPtr(
+            new Option6ClientFqdn(Option6ClientFqdn::FLAG_S,
+                                  "replace_me.com",
+                                  Option6ClientFqdn::FULL))
+    );
+
+    req->addOption(clientid);
+    req->addOption(srv.getServerID());
+
+    // Pass it to the server and get a reply
+    AllocEngine::ClientContext6 ctx;
+    bool drop = !srv.earlyGHRLookup(req, ctx);
+    ASSERT_FALSE(drop);
+    ctx.subnet_ = srv.selectSubnet(req, drop);
+    ASSERT_FALSE(drop);
+    srv.initContext(ctx, drop);
+    ASSERT_FALSE(drop);
+    Pkt6Ptr reply = srv.processRenew(ctx);
+
+    // Check if we get response at all
+    checkResponse(reply, DHCPV6_REPLY, 1234);
+
+    // check that an IA_NA with an iaaddr was returned for the requeted
+    // address with lifetime so 0.
+    boost::shared_ptr<Option6IAAddr> iaaddr = checkIA_NA(reply, 234, 1000, 2000); 
+    ASSERT_TRUE(iaaddr);
+    EXPECT_EQ(addr, iaaddr->getAddress());
+    EXPECT_EQ(3000, iaaddr->getPreferred());
+    EXPECT_EQ(4000, iaaddr->getValid());
+
+    // Fetch the lease in the database and verify hostname has been updated
+    // with generated FQDN.
+    Lease6Ptr l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
+    ASSERT_TRUE(l);
+    EXPECT_EQ("myhost-2001-db8-1--1.example.com.", l->hostname_);
+
+    // Should see follow log message ids in the log file.
+    EXPECT_EQ(1, countFile("DHCP6_DDNS_FQDN_GENERATED")); 
+    EXPECT_EQ(0, countFile("DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL"));
+}
+
+
+// This test verifies that generateFqdn() does not attempt to
+// update the lease's FQDN if lease lifetime is zero.
+TEST_F(Dhcpv6SrvTest, generateFqdnNoUpdate) {
+    IfaceMgrTestConfig test_config(true);
+
+    NakedDhcpv6Srv srv(0);
+
+    // Configure with a pool of 1 and ddns-replace-client-name = "always"
+    ASSERT_NO_THROW(configure(CONFIGS[5], srv));
+
+    // Lease the only address in the pool to client 1.
+    OptionPtr clientid = generateClientId();
+    const IOAddress addr("2001:db8:1::1");
+    const uint32_t iaid = 234;
+    Lease6Ptr used(new Lease6(Lease::TYPE_NA, addr, duid_, iaid, 3000, 4000, 1));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
+
+    // Create a RENEW of the leased address for client 2.
+    Pkt6Ptr req = Pkt6Ptr(new Pkt6(DHCPV6_RENEW, 1234));
+    req->setRemoteAddr(IOAddress("fe80::abcd"));
+    req->setIface("eth0");
+    req->setIndex(ETH0_INDEX);
+    req->addOption(createIA(Lease::TYPE_NA, addr, 128, iaid));
+    req->addOption(
+        Option6ClientFqdnPtr(
+            new Option6ClientFqdn(Option6ClientFqdn::FLAG_S,
+                                  "replace_me.com",
+                                  Option6ClientFqdn::FULL))
+    );
+
+    OptionPtr clientid2 = generateClientId(40);
+    req->addOption(clientid2);
+    req->addOption(srv.getServerID());
+
+    // Pass it to the server and get a reply
+    AllocEngine::ClientContext6 ctx;
+    bool drop = !srv.earlyGHRLookup(req, ctx);
+    ASSERT_FALSE(drop);
+    ctx.subnet_ = srv.selectSubnet(req, drop);
+    ASSERT_FALSE(drop);
+    srv.initContext(ctx, drop);
+    ASSERT_FALSE(drop);
+    Pkt6Ptr reply = srv.processRenew(ctx);
+
+    // Check if we get response at all
+    checkResponse(reply, DHCPV6_REPLY, 1234);
+
+    // Check that an IA_NA with an iaaddr was returned for the requeted
+    // address with lifetimes of 0.
+    boost::shared_ptr<Option6IAAddr> iaaddr = checkIA_NA(reply, 234, 0, 0); 
+    ASSERT_TRUE(iaaddr);
+    EXPECT_EQ(addr, iaaddr->getAddress());
+    EXPECT_EQ(0, iaaddr->getPreferred());
+    EXPECT_EQ(0, iaaddr->getValid());
+
+    // Should not see either of the follow log message ids in the log file.
+    EXPECT_EQ(0, countFile("DHCP6_DDNS_FQDN_GENERATED"));
+    EXPECT_EQ(0, countFile("DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL"));
+
+    // Fetch the lease in the database and verify hostname has not been updated.
+    Lease6Ptr l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
+    ASSERT_TRUE(l);
+    EXPECT_TRUE(l->hostname_.empty());
+}
+
 /// @brief Check that example files from documentation are valid (can be parsed
 /// and loaded).
 TEST_F(Dhcpv6SrvTest, checkConfigFiles) {

diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h
index ddcd64a8a2afc9fabe156f68c5a84add49bf4636..d4928cd0fd0ed16a98b5b064d2d4b1286e169595 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_test_utils.h
+++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h
@@ -32,6 +32,7 @@
 #include <hooks/hooks_manager.h>
 #include <config/command_mgr.h>
 #include <util/multi_threading_mgr.h>
+#include <testutils/log_utils.h>
 
 #include <list>
 
@@ -112,7 +113,7 @@ struct StrictIAIDChecking : public SpecializedTypeWrapper<bool> {
 /// Currently it configures the test data path directory in
 /// the @c CfgMgr. When the object is destroyed, the original
 /// path is reverted.
-class BaseServerTest : public ::testing::Test {
+class BaseServerTest : public LogContentTest {
 public:
 
     /// @brief Location of a test DUID file
@@ -637,7 +638,7 @@ public:
     /// @brief Destructor
     ///
     /// Removes existing configuration.
-    ~Dhcpv6SrvTest();
+    virtual ~Dhcpv6SrvTest();
 
     /// @brief Used to configure a server for tests.
     ///