[#936] kea-dhcpX now calculates DDNS TTL

DDNS TTL is now calculated based on RFC 4702 Sec 5 guidelines.

src/lib/dhcpsrv/ncr_generator.*
    calculateDdnsTtl() - new function to calculate DNS RR TTL from a lease
    life time

    queueNCRCommon() - modified to use calculateDdnsTtl()

src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
    Updated unit tests

src/bin/dhcp6/dhcp6_srv.cc
    Dhcpv6Srv::createNameChangeRequests() - modified to use calculateDdnsTtl()

src/bin/dhcp4/tests/fqdn_unittest.cc
src/bin/dhcp6/tests/fqdn_unittest.cc
    Updated unit tests

diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 7445bdd21586c97c0c9f3168f0d27c592f272064..db17c04e030c42b950853b0e0e8448018d3b9d76 100644 (file)
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -15,6 +15,7 @@
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/ncr_generator.h>
 #include <testutils/gtest_utils.h>
 
 #include <gtest/gtest.h>
@@ -746,10 +747,11 @@ public:
     /// if the value supplied is not empty):w
     /// @param cltt - cltt value from the lease the NCR for which the NCR
     /// was generated expected value for
-    /// @param len - expected lease length in the NCR
+    /// @param lifetime - lease's valid lifetime from which NCR ttl was
+    /// generated
     /// @param not_strict_expire_check - when true the comparison of the NCR
     /// lease expiration time is conducted as greater than or equal to rather
-    /// equal to CLTT plus lease length.
+    /// equal to CLTT plus lease ttl .
     /// @param exp_use_cr expected value of conflict resolution flag
     void verifyNameChangeRequest(const isc::dhcp_ddns::NameChangeType type,
                                  const bool reverse, const bool forward,
@@ -757,7 +759,7 @@ public:
                                  const std::string& fqdn,
                                  const std::string& dhcid,
                                  const time_t cltt,
-                                 const uint16_t len,
+                                 const uint16_t valid_lft,
                                  const bool not_strict_expire_check = false,
                                  const bool exp_use_cr = true) {
         NameChangeRequestPtr ncr;
@@ -774,17 +776,20 @@ public:
         if (!dhcid.empty()) {
             EXPECT_EQ(dhcid, ncr->getDhcid().toStr());
         }
+
         // In some cases, the test doesn't have access to the last transmission
         // time for the particular client. In such cases, the test can use the
         // current time as cltt but the it may not check the lease expiration
         // time for equality but rather check that the lease expiration time
         // is not greater than the current time + lease lifetime.
+        uint32_t ttl = calculateDdnsTtl(valid_lft);
         if (not_strict_expire_check) {
-            EXPECT_GE(cltt + len, ncr->getLeaseExpiresOn());
+            EXPECT_GE(cltt + ttl, ncr->getLeaseExpiresOn());
         } else {
-            EXPECT_EQ(cltt + len, ncr->getLeaseExpiresOn());
+            EXPECT_EQ(cltt + ttl, ncr->getLeaseExpiresOn());
         }
-        EXPECT_EQ(len, ncr->getLeaseLength());
+
+        EXPECT_EQ(ttl, ncr->getLeaseLength());
         EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr->getStatus());
         EXPECT_EQ(exp_use_cr, ncr->useConflictResolution());
 

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index dd98c805215c36f12d221509c5bcabe72b75c6d4..38f4687e16d18eb9fd1a46b16a49d1636f5e5049 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1940,7 +1940,7 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
                                         do_fwd, do_rev,
                                         opt_fqdn->getDomainName(),
                                         iaaddr->getAddress().toText(),
-                                        dhcid, 0, iaaddr->getValid(),
+                                        dhcid, 0, calculateDdnsTtl(iaaddr->getValid()),
                                         ctx.getDdnsParams()->getUseConflictResolution()));
         LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL,
                   DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST).arg(ncr->toText());

diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index f5b7076c844b1df24975b1bbcf8c843525c515cd..9c0e83e9a5a88ecbab53d71822a49286a0127ebb 100644 (file)
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -590,9 +590,9 @@ public:
     /// these values programmatically and place them here. Should the
     /// underlying implementation of createDigest() change these test values
     /// will likely need to be updated as well.
-    /// @param expires A timestamp when the lease associated with the
-    /// NameChangeRequest expires.
-    /// @param len A valid lifetime of the lease associated with the
+    /// @param expires The cltt of the lease associated with the
+    /// NameChangeRequest, and used to calculate NCR expires value.
+    /// @param valid_lft the valid lifetime of the lease associated with the
     /// NameChangeRequest.
     /// @param fqdn The expected string value of the FQDN, if blank the
     /// check is skipped
@@ -602,7 +602,7 @@ public:
                                  const std::string& addr,
                                  const std::string& dhcid,
                                  const uint64_t expires,
-                                 const uint16_t len,
+                                 const uint16_t valid_lft,
                                  const std::string& fqdn = "",
                                  const bool exp_use_cr = true) {
         NameChangeRequestPtr ncr;
@@ -617,11 +617,13 @@ public:
             EXPECT_EQ(dhcid, ncr->getDhcid().toStr());
         }
 
+        uint32_t ttl = calculateDdnsTtl(valid_lft);
         if (expires != 0) {
-            EXPECT_EQ(expires, ncr->getLeaseExpiresOn());
+            EXPECT_EQ(expires + ttl, ncr->getLeaseExpiresOn());
         }
 
-        EXPECT_EQ(len, ncr->getLeaseLength());
+        EXPECT_EQ(ttl, ncr->getLeaseLength());
+
         EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr->getStatus());
 
         if (! fqdn.empty()) {
@@ -916,8 +918,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestFwdRev) {
                             "2001:db8:1::1",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 502);
-
+                            lease_->cltt_, lease_->valid_lft_);
 }
 
 // Checks that calling queueNCR would not result in error if DDNS updates are
@@ -956,7 +957,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestRev) {
                             "2001:db8:1::1",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 502);
+                            lease_->cltt_, lease_->valid_lft_);
 
 }
 
@@ -1049,12 +1050,12 @@ TEST_F(FqdnDhcpv6SrvTest, processTwoRequestsDiffFqdn) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 4000);
+                            lease_->cltt_, lease_->valid_lft_);
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
                             "000201D422AA463306223D269B6CB7AFE7AAD265FC"
                             "EA97F93623019B2E0D14E5323D5A",
-                            0, 4000);
+                            0, lease_->valid_lft_);
 
 }
 
@@ -1159,12 +1160,12 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenewDiffFqdn) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 4000);
+                            lease_->cltt_, lease_->valid_lft_);
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
                             "000201D422AA463306223D269B6CB7AFE7AAD265FC"
                             "EA97F93623019B2E0D14E5323D5A",
-                            0, 4000);
+                            0, lease_->valid_lft_);
 
 }
 
@@ -1251,13 +1252,13 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenewFqdnFlags) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 4000);
+                            lease_->cltt_, lease_->valid_lft_);
 
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4000);
+                            0, lease_->valid_lft_);
 
     // Lastly, we renew with the N flag = 1 (which means no updates) so we
     // should have a dual direction remove NCR but NO add NCR.
@@ -1269,8 +1270,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenewFqdnFlags) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 4000);
-
+                            lease_->cltt_, lease_->valid_lft_);
 }
 
 
@@ -1292,7 +1292,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            0, 4000);
+                            0, lease_->valid_lft_);
 
     // Client may send Release message. In this case the lease should be
     // removed and all existing DNS entries for this lease should also
@@ -1305,8 +1305,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
                             "FAAAA3EBD29826B5C907B2C9268A6F52",
-                            lease_->cltt_ + lease_->valid_lft_, 4000);
-
+                            lease_->cltt_, lease_->valid_lft_);
 }
 
 // Checks that the server include DHCPv6 Client FQDN option in its
@@ -1379,7 +1378,8 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) {
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
                             "000201415AA33D1187D148275136FA30300478"
-                            "FAAAA3EBD29826B5C907B2C9268A6F52", 0, 4);
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            0, lease_->valid_lft_);
     // One of the following: IAID, DUID or subnet identifier has to be changed
     // because otherwise the allocation engine will treat the lease as
     // being renewed by the same client. If we at least change subnet identifier
@@ -1409,7 +1409,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) {
                             "2001:db8:1:1::dead:beef",
                             "000201D422AA463306223D269B6CB7AFE7AAD2"
                             "65FCEA97F93623019B2E0D14E5323D5A",
-                            lease->cltt_ + lease->valid_lft_, 5);
+                            lease->cltt_, lease->valid_lft_);
     // The second name change request should add a DNS mapping for
     // a new lease.
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,

diff --git a/src/lib/dhcpsrv/ncr_generator.cc b/src/lib/dhcpsrv/ncr_generator.cc
index 2fb2cdab163fd1373669876a4c26adc9c55614ba..bcb8011e682e11edeb38886122d576789a50f93a 100644 (file)
--- a/src/lib/dhcpsrv/ncr_generator.cc
+++ b/src/lib/dhcpsrv/ncr_generator.cc
@@ -56,12 +56,16 @@ void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease,
         std::vector<uint8_t> hostname_wire;
         OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
         D2Dhcid dhcid = D2Dhcid(identifier, hostname_wire);
+
+        // Calculate the TTL based on lease life time.
+        uint32_t ttl = calculateDdnsTtl(lease->valid_lft_);
+
         // Create name change request.
         NameChangeRequestPtr ncr
             (new NameChangeRequest(chg_type, lease->fqdn_fwd_, lease->fqdn_rev_,
                                    lease->hostname_, lease->addr_.toText(),
-                                   dhcid, lease->cltt_ + lease->valid_lft_,
-                                   lease->valid_lft_, use_conflict_resolution));
+                                   dhcid, lease->cltt_ + ttl,
+                                   ttl, use_conflict_resolution));
 
         LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL_DATA, DHCPSRV_QUEUE_NCR)
             .arg(label)
@@ -129,5 +133,15 @@ void queueNCR(const NameChangeType& chg_type, const Lease6Ptr& lease) {
     }
 }
 
+uint32_t calculateDdnsTtl(uint32_t lease_lft) {
+    // Per RFC 4702 DDNS RR TTL should be given by:
+    // ((lease life time / 3) < 10 minutes) ? 10 minutes : (lease life time / 3)
+    if (lease_lft < 1800) {
+        return (600);
+    }
+
+    return (lease_lft / 3);
+}
+
 }
 }

diff --git a/src/lib/dhcpsrv/ncr_generator.h b/src/lib/dhcpsrv/ncr_generator.h
index 15c928fbc07a510415498a865cbff62da8f819e8..5f079399923301dc83ea678ee326968f0344a366 100644 (file)
--- a/src/lib/dhcpsrv/ncr_generator.h
+++ b/src/lib/dhcpsrv/ncr_generator.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2020 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -41,6 +41,19 @@ void queueNCR(const dhcp_ddns::NameChangeType& chg_type, const Lease4Ptr& lease)
 /// @param lease Pointer to the lease.
 void queueNCR(const dhcp_ddns::NameChangeType& chg_type, const Lease6Ptr& lease);
 
+/// @brief Calculates TTL for a DNS resource record based on lease life time.
+///
+/// Per RFC 4702 Section 5, the RR TTL should be calculated as:
+/// TTL = ((lease life time / 3)  < 10 minutes) ? 10 minutes : (lease life time / 3) 
+///
+/// This function may be expanded in the future to support configurable
+/// parameters.
+///
+/// @param valid_lft valid life time of the lease
+///
+/// @return the calculated TTL.
+uint32_t calculateDdnsTtl(uint32_t lease_life_time);
+
 } // end of isc::dhcp namespace
 } // end of isc namespace
 

diff --git a/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc b/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
index 34ebee4838f130e5277f40da6d9e32c914fef979..905df55479b89561a6b10e4750ccc7b30ae4df6b 100644 (file)
--- a/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
+++ b/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
@@ -228,10 +228,12 @@ public:
         ASSERT_NO_FATAL_FAILURE(sendNCR(chg_type, fwd, rev, fqdn));
         // Expecting one NCR be generated.
         ASSERT_EQ(1, d2_mgr_.getQueueSize());
+
+        uint32_t ttl = calculateDdnsTtl(lease_->valid_lft_);
+
         // Check the details of the NCR.
         verifyNameChangeRequest(chg_type, rev, fwd, lease_->addr_.toText(), exp_dhcid,
-                                lease_->cltt_ + lease_->valid_lft_,
-                                lease_->valid_lft_, fqdn, exp_use_cr);
+                                lease_->cltt_ + ttl, ttl, fqdn, exp_use_cr);
     }
 
     /// @brief Test that calling queueNCR for NULL lease doesn't cause
@@ -621,11 +623,12 @@ TEST_F(NCRGenerator4Test, useClientId) {
     ASSERT_NO_FATAL_FAILURE(queueRemovalNCR(true, true, "myhost.example.com."));
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
+    uint32_t ttl = calculateDdnsTtl(lease_->valid_lft_);
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
                             "192.0.2.1",
                             "000101C7AA5420483BDA99C437636EA7DA2FE18"
                             "31C9679FEB031C360CA571298F3D1FA",
-                            lease_->cltt_ + lease_->valid_lft_, 100);
+                            lease_->cltt_ + ttl, ttl);
     {
         SCOPED_TRACE("case CHG_REMOVE");
         testNCR(CHG_REMOVE, true, true, "myhost.example.com.",
@@ -672,4 +675,14 @@ TEST_F(NCRGenerator4Test, useConflictResolution) {
     }
 }
 
+// Verify that calculateDdnsTtl() produces the expected values.
+TEST_F(NCRGenerator4Test, calculateDdnsTtl) {
+
+    // A life time less than or equal to 1800 should yield a TTL of 600 seconds.
+    EXPECT_EQ(600, calculateDdnsTtl(100));
+
+    // A life time > 1800 should be 1/3 of the value.
+    EXPECT_EQ(601, calculateDdnsTtl(1803));
+}
+
 } // end of anonymous namespace