BIND 10 trac3232, updated. 1b86037d33ea30c2bacb991a65a43f5c2a24c747 [3232] Fixed server behavior for non matching address in IA_NA.

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Feb 25 06:09:40 UTC 2014


The branch, trac3232 has been updated
       via  1b86037d33ea30c2bacb991a65a43f5c2a24c747 (commit)
      from  05ba16edd0de17b88b01a5d581ac41295772d09f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 1b86037d33ea30c2bacb991a65a43f5c2a24c747
Author: Marcin Siodelski <marcin at isc.org>
Date:   Tue Feb 25 07:09:19 2014 +0100

    [3232] Fixed server behavior for non matching address in IA_NA.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/dhcp6_srv.cc             |  150 ++++++++++++++++++--------------
 src/bin/dhcp6/dhcp6_srv.h              |   12 +++
 src/bin/dhcp6/tests/rebind_unittest.cc |   45 +++++++++-
 src/lib/dhcpsrv/dhcpsrv_messages.mes   |    4 +-
 4 files changed, 144 insertions(+), 67 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 09bdd74..7d91bf8 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1530,6 +1530,10 @@ OptionPtr
 Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
                        const Pkt6Ptr& query, const Pkt6Ptr& answer,
                        boost::shared_ptr<Option6IA> ia) {
+
+    // Create empty IA_NA option with IAID matching the request.
+    Option6IAPtr ia_rsp(new Option6IA(D6O_IA_NA, ia->getIAID()));
+
     if (!subnet) {
         /// @todo For simpliclty and due to limitations of LeaseMgr we don't
         /// get the binding for the client for which we don't get subnet id.
@@ -1537,15 +1541,13 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         /// The fact that we can't identify the subnet for the returning client
         /// doesn't really mean that the client has no binding. It is possible
         /// that due to server's reconfiguration the subnet has been removed
-        /// or modified since the client has got his lease. However, we think
-        /// it is fine to send NoBinding status code because this code is
-        /// supposed to be sent when server doesn't find the binding for
-        /// the client (which is the case here).
-        Option6IAPtr ia_rsp(new Option6IA(D6O_IA_NA, ia->getIAID()));
-        // Insert status code NoBinding.
+        /// or modified since the client has got his lease. We may need to
+        /// rethink whether it is appropriate to send no binding if the subnet
+        // hasn't been found for the client.
         ia_rsp->addOption(createStatusCode(STATUS_NoBinding,
                           "Sorry, no known leases for this duid/iaid."));
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_EXTEND_NA_UNKNOWN_SUBNET)
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+                  DHCP6_EXTEND_NA_UNKNOWN_SUBNET)
             .arg(query->getName())
             .arg(duid->toText())
             .arg(ia->getIAID());
@@ -1559,9 +1561,6 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
 
     // Client extending a lease that we don't know about.
     if (!lease) {
-        // Create empty IA_NA option with IAID matching the request.
-        boost::shared_ptr<Option6IA> ia_rsp(new Option6IA(D6O_IA_NA, ia->getIAID()));
-
         // Insert status code NoBinding to indicate that the lease does not
         // exist for this client.
         ia_rsp->addOption(createStatusCode(STATUS_NoBinding,
@@ -1578,63 +1577,84 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
     // Keep the old data in case the callout tells us to skip update.
     Lease6 old_data = *lease;
 
-    // At this point, we have to make make some decisions with respect to the
-    // FQDN option that we have generated as a result of receiving client's
-    // FQDN. In particular, we have to get to know if the DNS update will be
-    // performed or not. It is possible that option is NULL, which is valid
-    // condition if client didn't request DNS updates and server didn't force
-    // the update.
-    bool do_fwd = false;
-    bool do_rev = false;
-    Option6ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
-        Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
-    if (fqdn) {
-        if (fqdn->getFlag(Option6ClientFqdn::FLAG_S)) {
-            do_fwd = true;
-            do_rev = true;
-        } else if (!fqdn->getFlag(Option6ClientFqdn::FLAG_N)) {
-            do_rev = true;
-        }
-    }
+    bool invalid_addr = false;
+    // Check what address the client has sent. The address should match the one
+    // that we have associated with the IAID. If it doesn't match we have two
+    // options: allocate the address for the client, if the server's
+    // configuration allows to do so, or notify the client that his address is
+    // wrong. For now we will just notify the client that the address is wrong,
+    // but both solutions require that we check the contents of the IA_NA option
+    // sent by the client. Without this check we would extend the existing lease
+    // even if the address being carried in the IA_NA is different than the
+    // one we are extending.
+    Option6IAAddrPtr iaaddr = boost::dynamic_pointer_cast<
+        Option6IAAddr>(ia->getOption(D6O_IAADDR));
+    if (iaaddr && (iaaddr->getAddress() != lease->addr_)) {
+        Option6IAAddrPtr zero_lft_addr(new Option6IAAddr(D6O_IAADDR,
+                                                         iaaddr->getAddress(),
+                                                         0, 0));
+        ia_rsp->addOption(zero_lft_addr);
+        // Mark that the client's notion of the address is invalid, so as
+        // we don't update the actual client's lease.
+        invalid_addr = true;
 
-    std::string hostname;
-    if (do_fwd || do_rev) {
-        hostname = fqdn->getDomainName();
-    }
+    } else {
 
-    // If the new FQDN settings have changed for the lease, we need to
-    // delete any existing FQDN records for this lease.
-    if ((lease->hostname_ != hostname) || (lease->fqdn_fwd_ != do_fwd) ||
-        (lease->fqdn_rev_ != do_rev)) {
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
-                  DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE)
-            .arg(lease->toText())
-            .arg(hostname)
-            .arg(do_rev ? "true" : "false")
-            .arg(do_fwd ? "true" : "false");
+        // At this point, we have to make make some decisions with respect
+        // to the FQDN option that we have generated as a result of receiving
+        // client's FQDN. In particular, we have to get to know if the DNS
+        // update will be performed or not. It is possible that option is NULL,
+        // which is valid condition if client didn't request DNS updates and
+        // server didn't force the update.
+        bool do_fwd = false;
+        bool do_rev = false;
+        Option6ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
+            Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
+        if (fqdn) {
+            if (fqdn->getFlag(Option6ClientFqdn::FLAG_S)) {
+                do_fwd = true;
+                do_rev = true;
+            } else if (!fqdn->getFlag(Option6ClientFqdn::FLAG_N)) {
+                do_rev = true;
+            }
+        }
 
-        createRemovalNameChangeRequest(lease);
-    }
+        std::string hostname;
+        if (do_fwd || do_rev) {
+            hostname = fqdn->getDomainName();
+        }
 
-    lease->preferred_lft_ = subnet->getPreferred();
-    lease->valid_lft_ = subnet->getValid();
-    lease->t1_ = subnet->getT1();
-    lease->t2_ = subnet->getT2();
-    lease->cltt_ = time(NULL);
-    lease->hostname_ = hostname;
-    lease->fqdn_fwd_ = do_fwd;
-    lease->fqdn_rev_ = do_rev;
+        // If the new FQDN settings have changed for the lease, we need to
+        // delete any existing FQDN records for this lease.
+        if ((lease->hostname_ != hostname) || (lease->fqdn_fwd_ != do_fwd) ||
+            (lease->fqdn_rev_ != do_rev)) {
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+                      DHCP6_DDNS_LEASE_RENEW_FQDN_CHANGE)
+                .arg(lease->toText())
+                .arg(hostname)
+                .arg(do_rev ? "true" : "false")
+                .arg(do_fwd ? "true" : "false");
 
-    // Create empty IA_NA option with IAID matching the request.
-    boost::shared_ptr<Option6IA> ia_rsp(new Option6IA(D6O_IA_NA, ia->getIAID()));
+            createRemovalNameChangeRequest(lease);
+        }
 
-    ia_rsp->setT1(subnet->getT1());
-    ia_rsp->setT2(subnet->getT2());
+        lease->preferred_lft_ = subnet->getPreferred();
+        lease->valid_lft_ = subnet->getValid();
+        lease->t1_ = subnet->getT1();
+        lease->t2_ = subnet->getT2();
+        lease->cltt_ = time(NULL);
+        lease->hostname_ = hostname;
+        lease->fqdn_fwd_ = do_fwd;
+        lease->fqdn_rev_ = do_rev;
 
-    boost::shared_ptr<Option6IAAddr> addr(new Option6IAAddr(D6O_IAADDR,
-                                          lease->addr_, lease->preferred_lft_,
-                                          lease->valid_lft_));
-    ia_rsp->addOption(addr);
+        ia_rsp->setT1(subnet->getT1());
+        ia_rsp->setT2(subnet->getT2());
+
+        Option6IAAddrPtr addr(new Option6IAAddr(D6O_IAADDR, lease->addr_,
+                                                lease->preferred_lft_,
+                                                lease->valid_lft_));
+        ia_rsp->addOption(addr);
+    }
 
     bool skip = false;
     // Get the callouts specific for the processed message and execute them.
@@ -1670,12 +1690,16 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
     }
 
     if (!skip) {
-        LeaseMgrFactory::instance().updateLease6(lease);
+        // If the client has sent an invalid address, it shouldn't affect the
+        // lease in our lease database.
+        if (!invalid_addr) {
+            LeaseMgrFactory::instance().updateLease6(lease);
+        }
     } else {
         // Copy back the original date to the lease. For MySQL it doesn't make
         // much sense, but for memfile, the Lease6Ptr points to the actual lease
-        // in memfile, so the actual update is performed when we manipulate fields
-        // of returned Lease6Ptr, the actual updateLease6() is no-op.
+        // in memfile, so the actual update is performed when we manipulate
+        // fields of returned Lease6Ptr, the actual updateLease6() is no-op.
         *lease = old_data;
     }
 
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index 722b0a9..e3cedfe 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -280,6 +280,18 @@ protected:
     /// lease is found, an IA_NA response is generated with an appropriate
     /// status code.
     ///
+    /// @todo The behavior of this function will need to be extended to support
+    /// draft-ietf-dhc-dhcpv6-stateful-issues. This draft modifies the behavior
+    /// described in RFC3315 with respect to Renew and Rebind processing. Key
+    /// changes are (version -05):
+    /// - Renewing and Rebinding client MAY request additional bindings by
+    /// putting an IA for all bindings it desires but has been unable to obtain.
+    /// Server MAY allocate addresses if it finds that they are appropriate for
+    /// the link that client is attached to.
+    /// - When receiving Rebind, if the server determines that the addresses are
+    /// not appropriate for the link the client is attached to, the server MAY
+    /// send the IA with address lifetimes set to 0 or discard the message.
+    ///
     /// @param subnet subnet the sender belongs to
     /// @param duid client's duid
     /// @param query client's message (Renew or Rebind)
diff --git a/src/bin/dhcp6/tests/rebind_unittest.cc b/src/bin/dhcp6/tests/rebind_unittest.cc
index a3a0c5d..7aba0ba 100644
--- a/src/bin/dhcp6/tests/rebind_unittest.cc
+++ b/src/bin/dhcp6/tests/rebind_unittest.cc
@@ -460,6 +460,44 @@ TEST_F(RebindTest, relayedClientLostLease) {
     EXPECT_EQ(STATUS_NoBinding, client.getStatusCode(0));
 }
 
+TEST_F(RebindTest, relayedClientChangingAddress) {
+    Dhcp6Client client;
+    // Configure client to request IA_NA.
+    client.useNA();
+    // Make 4-way exchange to get the lease.
+    ASSERT_NO_FATAL_FAILURE(requestLease(2, 2, client));
+    // Keep the client's lease for future reference.
+    Lease6 lease_client = client.getLease(0);
+    // Modify the address of the lease record that client stores. The server
+    // should check that the address is invalid (hasn't been allocated for
+    // the particular IAID).
+    client.config_.leases_[0].lease_.addr_ = IOAddress("3000::100");
+    // Try to Rebind. The client will use correct IAID but will specify a
+    // wrong address. The server will discover that the client has a binding
+    // but the address will not match.
+    ASSERT_NO_THROW(client.doRebind());
+    // Make sure that the server has discarded client's message. In such case,
+    // the message sent back to the client should be NULL.
+    EXPECT_TRUE(client.getContext().response_)
+        << "The server discarded the Rebind message, while it should have"
+        " sent a response indicating that the client should stop using the"
+        " lease, by setting lifetime values to 0.";
+    // Get the client's lease.
+    ASSERT_EQ(1, client.getLeaseNum());
+    Lease6 lease_client2 = client.getLease(0);
+    // The lifetimes should be set to 0, as an explicit notification to the
+    // client to stop using invalid prefix.
+    EXPECT_EQ(0, lease_client2.valid_lft_);
+    EXPECT_EQ(0, lease_client2.preferred_lft_);
+    // Check that server still has the same lease.
+    Lease6Ptr lease_server = checkLease(lease_client);
+    EXPECT_TRUE(lease_server);
+    // Make sure that the lease in the data base hasn't been addected.
+    EXPECT_NE(0, lease_server->valid_lft_);
+    EXPECT_NE(0, lease_server->preferred_lft_);
+}
+
+
 TEST_F(RebindTest, directClientPD) {
     Dhcp6Client client;
     // Configure client to request IA_PD.
@@ -558,7 +596,7 @@ TEST_F(RebindTest, directClientPDChangingPrefix) {
     // Modify the Prefix of the lease record that client stores. The server
     // should check that the prefix is invalid (hasn't been allocated for
     // the particular IAID).
-    client.config_.leases_[0].lease_.addr_ = IOAddress("2001:db8:1:10::1");
+    client.config_.leases_[0].lease_.addr_ = IOAddress("2001:db8:1:10::");
     // Try to Rebind. The client will use correct IAID but will specify a
     // wrong prefix. The server will discover that the client has a binding
     // but the prefix will not match. According to the RFC3633, section 12.2.
@@ -580,7 +618,10 @@ TEST_F(RebindTest, directClientPDChangingPrefix) {
     EXPECT_EQ(0, lease_client2.preferred_lft_);
     // Check that server still has the same lease.
     Lease6Ptr lease_server = checkLease(lease_client);
-    EXPECT_TRUE(lease_server);
+    ASSERT_TRUE(lease_server);
+    // Make sure that the lease in the data base hasn't been addected.
+    EXPECT_NE(0, lease_server->valid_lft_);
+    EXPECT_NE(0, lease_server->preferred_lft_);
 }
 
 /// @todo Extend PD tests for relayed messages.
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index e8dd2e7..d00fa06 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -203,12 +203,12 @@ A debug message issued when the server is attempting to obtain a set of
 IPv4 leases from the memory file database for a client with the specified
 hardware address.
 
-% DHCPSRV_MEMFILE_GET_IAID_DUID obtaining IPv4 leases for IAID %1 and DUID %2
+% DHCPSRV_MEMFILE_GET_IAID_DUID obtaining IPv6 leases for IAID %1 and DUID %2
 A debug message issued when the server is attempting to obtain a set of
 IPv6 lease from the memory file database for a client with the specified
 IAID (Identity Association ID) and DUID (DHCP Unique Identifier).
 
-% DHCPSRV_MEMFILE_GET_IAID_SUBID_DUID obtaining IPv4 leases for IAID %1, Subnet ID %2 and DUID %3
+% DHCPSRV_MEMFILE_GET_IAID_SUBID_DUID obtaining IPv6 leases for IAID %1, Subnet ID %2 and DUID %3
 A debug message issued when the server is attempting to obtain an IPv6
 lease from the memory file database for a client with the specified IAID
 (Identity Association ID), Subnet ID and DUID (DHCP Unique Identifier).



More information about the bind10-changes mailing list