]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#557,!310] Fixed kea-dhcp6 DNS updates on renewals with generated FQDNs
authorThomas Markwalder <tmark@isc.org>
Tue, 30 Apr 2019 14:48:39 +0000 (10:48 -0400)
committerThomas Markwalder <tmark@isc.org>
Wed, 15 May 2019 15:21:56 +0000 (11:21 -0400)
Moved logic for detecting FQDN changes from the alloc engine to the
kea-dhcp6 server proper.  This allows the change detection to be done after
FQDN generation.

src/bin/dhcp6/dhcp6_srv.*
    Dhcpv6Srv::createNameChangeRequests() - modified to detect when changes
    to a valid lease require a DNS remove

    Dhcpv6Srv::generateFqdn() - updated to store the generated name in
    the current context

src/lib/dhcpsrv/alloc_engine.cc
    AllocEngine::extendLease6()
    AllocEngine::updateLeaseData() - removed logic that checks for FQDN changes

src/bin/dhcp6/dhcp6_messages.*
    Revamped a message

src/bin/dhcp6/dhcp6_messages.cc
src/bin/dhcp6/dhcp6_messages.h
src/bin/dhcp6/dhcp6_messages.mes
src/bin/dhcp6/dhcp6_srv.cc
src/bin/dhcp6/dhcp6_srv.h
src/lib/dhcpsrv/alloc_engine.cc

index 1bd1c732721ba994c12ab0e28816a9f7296eb0c3..65004a8db21f1d1dddf92b1b8ed695d8539a167d 100644 (file)
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp6/dhcp6_messages.mes on Tue Mar 26 2019 13:41
+// File created from ../../../src/bin/dhcp6/dhcp6_messages.mes on Tue Apr 30 2019 10:41
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -37,8 +37,8 @@ extern const isc::log::MessageID DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST = "DH
 extern const isc::log::MessageID DHCP6_DDNS_FQDN_GENERATED = "DHCP6_DDNS_FQDN_GENERATED";
 extern const isc::log::MessageID DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL = "DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL";
 extern const isc::log::MessageID DHCP6_DDNS_GENERATE_FQDN = "DHCP6_DDNS_GENERATE_FQDN";
-extern const isc::log::MessageID DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE = "DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE";
 extern const isc::log::MessageID DHCP6_DDNS_RECEIVE_FQDN = "DHCP6_DDNS_RECEIVE_FQDN";
+extern const isc::log::MessageID DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN = "DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN";
 extern const isc::log::MessageID DHCP6_DDNS_REQUEST_SEND_FAILED = "DHCP6_DDNS_REQUEST_SEND_FAILED";
 extern const isc::log::MessageID DHCP6_DDNS_RESPONSE_FQDN_DATA = "DHCP6_DDNS_RESPONSE_FQDN_DATA";
 extern const isc::log::MessageID DHCP6_DDNS_SEND_FQDN = "DHCP6_DDNS_SEND_FQDN";
@@ -177,8 +177,8 @@ const char* values[] = {
     "DHCP6_DDNS_FQDN_GENERATED", "%1: generated FQDN for the client: %2",
     "DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL", "%1: failed to update the lease using address %2, after generating FQDN for a client, reason: %3",
     "DHCP6_DDNS_GENERATE_FQDN", "%1: client did not send a FQDN option; FQDN will be",
-    "DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE", "FQDN %1: FQDN for the renewed lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5",
     "DHCP6_DDNS_RECEIVE_FQDN", "%1: received DHCPv6 Client FQDN option: %2",
+    "DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN", "%1: FQDN for a lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5",
     "DHCP6_DDNS_REQUEST_SEND_FAILED", "failed sending a request to kea-dhcp-ddns, error: %1,  ncr: %2",
     "DHCP6_DDNS_RESPONSE_FQDN_DATA", "%1: including FQDN option in the server's response: %2",
     "DHCP6_DDNS_SEND_FQDN", "sending DHCPv6 Client FQDN Option to the client: %1",
index 19f22199fcd5736d399af1ac70b547a167dad557..298d7d0bfea65b8191c1d71126db42351dea9ee7 100644 (file)
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp6/dhcp6_messages.mes on Tue Mar 26 2019 13:41
+// File created from ../../../src/bin/dhcp6/dhcp6_messages.mes on Tue Apr 30 2019 10:41
 
 #ifndef DHCP6_MESSAGES_H
 #define DHCP6_MESSAGES_H
@@ -38,8 +38,8 @@ extern const isc::log::MessageID DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST;
 extern const isc::log::MessageID DHCP6_DDNS_FQDN_GENERATED;
 extern const isc::log::MessageID DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL;
 extern const isc::log::MessageID DHCP6_DDNS_GENERATE_FQDN;
-extern const isc::log::MessageID DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE;
 extern const isc::log::MessageID DHCP6_DDNS_RECEIVE_FQDN;
+extern const isc::log::MessageID DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN;
 extern const isc::log::MessageID DHCP6_DDNS_REQUEST_SEND_FAILED;
 extern const isc::log::MessageID DHCP6_DDNS_RESPONSE_FQDN_DATA;
 extern const isc::log::MessageID DHCP6_DDNS_SEND_FQDN;
index 964f4bfff4f63e4015fb7792b3092c5a9c0dd26a..53bb696a7705275dc5afc0fef7f9f0d62591c27e 100644 (file)
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2019 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
@@ -180,13 +180,14 @@ This debug message is issued when the server did not receive a FQDN option
 from the client and client name replacement is enabled.  This provides a means
 to create DNS entries for unsophisticated clients.
 
-% DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE FQDN %1: FQDN for the renewed lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5
-This debug message is logged when FQDN mapping for a particular lease has been
-changed by the recent Renew message. This mapping will be changed in DNS.
-The first argument includes the client and the transaction identification
-information. The second argument holds the details about the lease for which
-the FQDN information and/or mappings have changed. The remaining arguments
-hold the new FQDN information and flags for mappings.
+% DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN %1: FQDN for a lease: %2 has changed. New values: hostname = %3, reverse mapping = %4, forward mapping = %5
+This debug message is logged during lease renewal when an old lease that is
+no longer being offered has a different FQDN than the renewing lesae. Thus
+the old DNS entries need to be removed.  The first argument includes the client
+and the transaction identification information. The second argument holds the
+details about the lease for which the FQDN information and/or mappings have
+changed. The remaining arguments hold the new FQDN information and flags for
+mappings.
 
 % DHCP6_DDNS_RECEIVE_FQDN %1: received DHCPv6 Client FQDN option: %2
 This debug message is logged when server has found the DHCPv6 Client FQDN Option
index 34c7e5a4d15c6fb01f97dff86d091ce5bc30a864..607ca7640629921435dd3f514411744f0cae14b6 100644 (file)
@@ -1637,12 +1637,6 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
     bool do_rev = false;
     CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*opt_fqdn,
                                                              do_fwd, do_rev);
-    if (!do_fwd && !do_rev) {
-        // Flags indicate there is Nothing to do, get out now.
-        // The Most likely scenario is that we are honoring the client's
-        // request that no updates be done.
-        return;
-    }
 
     // Get the Client Id. It is mandatory and a function creating a response
     // would have thrown an exception if it was missing. Thus throwing
@@ -1668,13 +1662,12 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
 
     // Get all IAs from the answer. For each IA, holding an address we will
     // create a corresponding NameChangeRequest.
-    OptionCollection answer_ias = answer->getOptions(D6O_IA_NA);
-    for (OptionCollection::const_iterator answer_ia =
-             answer_ias.begin(); answer_ia != answer_ias.end(); ++answer_ia) {
+    for (auto answer_ia : answer->getOptions(D6O_IA_NA)) {
         /// @todo IA_NA may contain multiple addresses. We should process
         /// each address individually. Currently we get only one.
         Option6IAAddrPtr iaaddr = boost::static_pointer_cast<
-            Option6IAAddr>(answer_ia->second->getOption(D6O_IAADDR));
+            Option6IAAddr>(answer_ia.second->getOption(D6O_IAADDR));
+
         // We need an address to create a name-to-address mapping.
         // If address is missing for any reason, go to the next IA.
         if (!iaaddr) {
@@ -1682,22 +1675,36 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
         }
 
         // If the lease for iaaddr is in the list of changed leases, we need
-        // to determine if the changes included changes to the FQDN. If there
-        // were  changes to the FQDN then we need to update DNS, otherwise
-        // we do not.
+        // to determine if the changes included changes to the FQDN. If so
+        // then we may need to do a CHG_REMOVE.
         bool extended_only = false;
         for (Lease6Collection::const_iterator l = ctx.currentIA().changed_leases_.begin();
              l != ctx.currentIA().changed_leases_.end(); ++l) {
+
             if ((*l)->addr_ == iaaddr->getAddress()) {
+                // The address is the same so this must be renewal?
                 if ((*l)->hostname_ == opt_fqdn->getDomainName() &&
                     (*l)->fqdn_fwd_ == do_fwd && (*l)->fqdn_rev_ == do_rev) {
+                    // The FQDN is the same, it must be an extension only.
+                    // @todo - If we decide to allow updates on renews, we
+                    // will need to bypass this.
                     extended_only = true;
-                    break;
+                } else {
+                    // The FQDN has changed, queue a CHG_REMOVE of the old data.
+                    // NCR will only be created if the lease hostname is not
+                    // empty and at least one of the direction flags is true
+                    queueNCR(CHG_REMOVE, *l);
                 }
+
+                break;
             }
         }
 
-        if (extended_only) {
+        if (!(do_fwd || do_rev) || (extended_only)) {
+            // Flags indicate no updates needed  or it was an extension of
+            // an existing lease with no FQDN changes.  In the case of the
+            // former, the most likely scenario is that we are honoring the
+            // client's request that no updates be done.
             continue;
         }
 
@@ -2115,7 +2122,7 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
         if (((*l)->hostname_ != ctx.hostname_) || ((*l)->fqdn_fwd_ != do_fwd) ||
             ((*l)->fqdn_rev_ != do_rev)) {
             LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL,
-                      DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE)
+                      DHCP6_DDNS_REMOVE_OLD_LEASE_FQDN)
                 .arg(query->getLabel())
                 .arg((*l)->toText())
                 .arg(ctx.hostname_)
@@ -2850,7 +2857,7 @@ Dhcpv6Srv::processRequest(AllocEngine::ClientContext6& ctx) {
     appendRequestedVendorOptions(request, reply, ctx, co_list);
 
     updateReservedFqdn(ctx, reply);
-    generateFqdn(reply);
+    generateFqdn(reply, ctx);
     createNameChangeRequests(reply, ctx);
 
     return (reply);
@@ -2876,7 +2883,7 @@ Dhcpv6Srv::processRenew(AllocEngine::ClientContext6& ctx) {
     appendRequestedVendorOptions(renew, reply, ctx, co_list);
 
     updateReservedFqdn(ctx, reply);
-    generateFqdn(reply);
+    generateFqdn(reply, ctx);
     createNameChangeRequests(reply, ctx);
 
     return (reply);
@@ -2902,7 +2909,7 @@ Dhcpv6Srv::processRebind(AllocEngine::ClientContext6& ctx) {
     appendRequestedVendorOptions(rebind, reply, ctx, co_list);
 
     updateReservedFqdn(ctx, reply);
-    generateFqdn(reply);
+    generateFqdn(reply, ctx);
     createNameChangeRequests(reply, ctx);
 
     return (reply);
@@ -3599,7 +3606,8 @@ Dhcpv6Srv::updateReservedFqdn(const AllocEngine::ClientContext6& ctx,
 }
 
 void
-Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer) {
+Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer,
+                        AllocEngine::ClientContext6& ctx) {
     if (!answer) {
         isc_throw(isc::Unexpected, "an instance of the object encapsulating"
                   " a message must not be NULL when generating FQDN");
@@ -3664,7 +3672,7 @@ Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer) {
 
        answer->delOption(D6O_CLIENT_FQDN);
        answer->addOption(fqdn);
-
+       ctx.hostname_ = generated_name;
     } catch (const Exception& ex) {
         LOG_ERROR(ddns6_logger, DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL)
             .arg(answer->getLabel())
index 63e9b4369d4dde044edfeebf7090a2413ceea0b1..e421832d03ea236ed50d121992e8d3ea4cb22875 100644 (file)
@@ -653,9 +653,11 @@ protected:
     /// The FQDN option carries response to the client about DNS updates that
     /// server intends to perform for the DNS client. Based on this, the
     /// function will create zero or more @c isc::dhcp_ddns::NameChangeRequest
-    /// objects and store them in the internal queue. Requests created by this
-    /// function are only for adding or updating DNS records. If DNS updates are
-    /// disabled, this method returns immediately.
+    /// objects and store them in the internal queue.  To catch lease renewals
+    /// that alter the FQDN, the function first looks at the context's changed
+    /// list of leases (if any) to determine if DNS entries need to be removed.
+    /// It then looks at the valid leases to determine if any DNS entries need
+    /// to be added. If DNS updates are disabled, this method returns immediately.
     ///
     /// @todo Add support for multiple IAADDR options in the IA_NA.
     ///
@@ -969,10 +971,12 @@ private:
     ///
     /// @param answer Message being sent to a client, which may hold IA_NA
     /// and Client FQDN options to be used to generate name for a client.
+    /// @param ctx Client context.
     ///
     /// @throw isc::Unexpected if specified message is NULL. This is treated
     /// as a programmatic error.
-    void generateFqdn(const Pkt6Ptr& answer);
+    void generateFqdn(const Pkt6Ptr& answer,
+                      AllocEngine::ClientContext6& ctx);
 
     /// @brief Updates statistics for received packets
     /// @param query packet received
index 0929f39a0154cc91ca2a9e60cdf8e72ce4a36776..4895aee79524c6831d1d61dcca3401ae16793b3e 100644 (file)
@@ -2017,12 +2017,6 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
                                        "assigned-nas" : "assigned-pds"),
                     static_cast<int64_t>(1));
             }
-        } else {
-            if (!lease->hasIdenticalFqdn(*old_data)) {
-                // We're not reclaiming the lease but since the FQDN has changed
-                // we have to at least send NCR.
-                queueNCR(CHG_REMOVE, old_data);
-            }
         }
 
         // Now that the lease has been reclaimed, we can go ahead and update it
@@ -2046,7 +2040,6 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
 Lease6Collection
 AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases) {
     Lease6Collection updated_leases;
-    bool remove_queued = false;
     for (Lease6Collection::const_iterator lease_it = leases.begin();
          lease_it != leases.end(); ++lease_it) {
         Lease6Ptr lease(new Lease6(**lease_it));
@@ -2077,13 +2070,6 @@ AllocEngine::updateLeaseData(ClientContext6& ctx, const Lease6Collection& leases
             if (conditionalExtendLifetime(*lease) || fqdn_changed) {
                 ctx.currentIA().changed_leases_.push_back(*lease_it);
                 LeaseMgrFactory::instance().updateLease6(lease);
-
-                // If the FQDN differs, remove existing DNS entries.
-                // We only need one remove.
-                if (fqdn_changed && !remove_queued) {
-                    queueNCR(CHG_REMOVE, *lease_it);
-                    remove_queued = true;
-                }
             }
         }