Browse Source

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

Marcin Siodelski 11 years ago
parent
commit
1b86037d33

+ 87 - 63
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;
     }
 

+ 12 - 0
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)

+ 43 - 2
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.

+ 2 - 2
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).