[master] Merge branch 'trac5387'

src/bin/dhcp6/dhcp6_srv.cc

@@ -48,6 +48,7 @@
 
 #include <util/encode/hex.h>
 #include <util/io_utilities.h>
+#include <util/pointer_util.h>
 #include <util/range_utilities.h>
 #include <log/logger.h>
 #include <cryptolink/cryptolink.h>
@@ -1826,9 +1827,15 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
     // For the leases that we just retired, send the addresses with 0 lifetimes.
     for (Lease6Collection::const_iterator l = ctx.currentIA().old_leases_.begin();
                                           l != ctx.currentIA().old_leases_.end(); ++l) {
-        Option6IAAddrPtr iaaddr(new Option6IAAddr(D6O_IAADDR,
-                                                  (*l)->addr_, 0, 0));
-        ia_rsp->addOption(iaaddr);
+
+        // Send an address with zero lifetimes only when this lease belonged to
+        // this client. Do not send it when we're reusing an old lease that belonged
+        // to someone else.
+        if (equalValues(query->getClientId(), (*l)->duid_)) {
+            Option6IAAddrPtr iaaddr(new Option6IAAddr(D6O_IAADDR,
+                                                      (*l)->addr_, 0, 0));
+            ia_rsp->addOption(iaaddr);
+        }
 
         // Now remove this address from the hints list.
         AllocEngine::ResourceType hint_type((*l)->addr_, 128);
@@ -2012,6 +2019,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
     // already, inform the client that he can't have them.
     for (AllocEngine::HintContainer::const_iterator prefix = hints.begin();
          prefix != hints.end(); ++prefix) {
+
         // Send the prefix with the zero lifetimes only if the prefix
         // contains non-zero value. A zero value indicates that the hint was
         // for the prefix length.

src/bin/dhcp6/tests/dhcp6_client.cc

@@ -16,6 +16,7 @@
 #include <dhcp/option6_status_code.h>
 #include <dhcp/pkt6.h>
 #include <dhcpsrv/lease.h>
+#include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/pool.h>
 #include <dhcp6/tests/dhcp6_client.h>
 #include <util/buffer.h>
@@ -635,10 +636,14 @@ Dhcp6Client::generateIAFromLeases(const Pkt6Ptr& query,
 }
 
 void
-Dhcp6Client::fastFwdTime(const uint32_t secs) {
+Dhcp6Client::fastFwdTime(const uint32_t secs, const bool update_server) {
     // Iterate over all leases and move their cltt backwards.
     for (size_t i = 0; i < config_.leases_.size(); ++i) {
         config_.leases_[i].cltt_ -= secs;
+        if (update_server) {
+            Lease6Ptr lease(new Lease6(config_.leases_[i]));
+            LeaseMgrFactory::instance().updateLease6(lease);
+        }
     }
 }
 

src/bin/dhcp6/tests/dhcp6_client.h

@@ -325,7 +325,11 @@ public:
     /// lease has been later updated (e.g. as a result of Rebind) as it is
     /// expected that the fresh lease has cltt set to "now" (not to the time
     /// in the past).
-    void fastFwdTime(const uint32_t secs);
+    ///
+    /// @param secs Number of seconds by which the time should be moved.
+    /// @param update_server Indicates if the leases should be updated on the
+    /// server.
+    void fastFwdTime(const uint32_t secs, const bool update_server = false);
 
     /// @brief Returns DUID option used by the client.
     OptionPtr getClientId() const;

src/bin/dhcp6/tests/host_unittest.cc

@@ -50,6 +50,14 @@ namespace {
 /// - Configuration 5:
 ///   - Used to test that host specific vendor options override globally
 ///     specified vendor options.
+///
+/// - Configuration 6:
+///   - One subnet with very short pool, i.e. two addresses
+///
+/// - Configuration 7:
+///   - Similar to Configuration 6, but one of the addresses reserved to client
+///     with the DUID 04:03:02:01.
+///
 const char* CONFIGS[] = {
     // Configuration 0:
     "{ "
@@ -256,8 +264,64 @@ const char* CONFIGS[] = {
         "        } ]"
         "    } ]"
         " } ]"
-    "}"
+    "}",
+
+    // Configuration 6:
+    "{ "
+        "\"interfaces-config\": {"
+        "  \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"host-reservation-identifiers\": [ \"duid\" ],"
+        "\"valid-lifetime\": 40, "
+        "\"preferred-lifetime\": 30,"
+        "\"rebind-timer\": 20, "
+        "\"renew-timer\": 10, "
+        "\"subnet6\": [ "
+        " { "
+        "    \"subnet\": \"2001:db8:1::/48\", "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::2\" } ],"
+        "    \"pd-pools\": ["
+        "        {"
+        "            \"prefix\": \"3000::\","
+        "            \"prefix-len\": 119,"
+        "            \"delegated-len\": 120"
+        "        }"
+        "    ],"
+        "    \"interface\" : \"eth0\""
+        "} ]"
+    "}",
 
+    // Configuration 7:
+    "{ "
+        "\"interfaces-config\": {"
+        "  \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"host-reservation-identifiers\": [ \"duid\" ],"
+        "\"valid-lifetime\": 40, "
+        "\"preferred-lifetime\": 30,"
+        "\"rebind-timer\": 20, "
+        "\"renew-timer\": 10, "
+        "\"subnet6\": [ "
+        " { "
+        "    \"subnet\": \"2001:db8:1::/48\", "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::2\" } ],"
+        "    \"pd-pools\": ["
+        "        {"
+        "            \"prefix\": \"3000::\","
+        "            \"prefix-len\": 119,"
+        "            \"delegated-len\": 120"
+        "        }"
+        "    ],"
+        "    \"interface\" : \"eth0\","
+        "    \"reservations\": ["
+        "        {"
+        "            \"duid\": \"04:03:02:01\","
+        "            \"ip-addresses\": [ \"2001:db8:1::2\" ],"
+        "            \"prefixes\": [ \"3000::100/120\" ]"
+        "        }"
+        "    ]"
+        "} ]"
+    "}"
 };
 
 /// @brief Base class representing leases and hints conveyed within IAs.
@@ -1787,5 +1851,72 @@ TEST_F(HostTest, multipleIAsConflict) {
                                           IAID(4)));
 }
 
+// This test verifies a scenario in which a client trying to renew a
+// lease is refused this lease because it has been reserved to another
+// client. The client is assigned another available lease from a
+// dynamic pool by reusing an expired lease.
+TEST_F(HostTest, conflictResolutionReuseExpired) {
+    Dhcp6Client client1;
+
+    ASSERT_NO_THROW(configure(CONFIGS[6], *client1.getServer()));
+
+    // First client performs 4-way exchange and obtains an address and
+    // prefix indicated in hints.
+    requestIA(client1, Hint(IAID(1), "2001:db8:1::1"));
+    requestIA(client1, Hint(IAID(2), "3000::/120"));
+
+    ASSERT_NO_THROW(client1.doSARR());
+
+    // Make sure the client has obtained requested leases.
+    ASSERT_TRUE(client1.hasLeaseForAddress(IOAddress("2001:db8:1::1"), IAID(1)));
+    ASSERT_TRUE(client1.hasLeaseForPrefix(IOAddress("3000::"), 120));
+
+    // Create another client which is assigned another lease.
+    Dhcp6Client client2(client1.getServer());
+
+    // Second client performs 4-way exchange and obtains an address and
+    // prefix indicated in hints.
+    requestIA(client2, Hint(IAID(1), "2001:db8:1::2"));
+    requestIA(client2, Hint(IAID(2), "3000::100/120"));
+
+    ASSERT_NO_THROW(client2.doSARR());
+
+    // Make sure the client has obtained requested leases.
+    ASSERT_TRUE(client2.hasLeaseForAddress(IOAddress("2001:db8:1::2"), IAID(1)));
+    ASSERT_TRUE(client2.hasLeaseForPrefix(IOAddress("3000::100"), 120));
+
+    // Fast forward time to simulate aging of leases. After that, both leases are
+    // expired because their valid lifetime is 40s. The second argument indicates
+    // that the leases should also be updated on the server.
+    client1.fastFwdTime(60, true);
+    client2.fastFwdTime(60, true);
+
+    // Reconfigure the server, so as the address 2001:db8:1::2 and prefix
+    // 3000::10/120 is now reserved for another client.
+    ASSERT_NO_THROW(configure(CONFIGS[7], *client1.getServer()));
+
+    client1.clearRequestedIAs();
+    client2.clearRequestedIAs();
+
+    // Try to renew the address of 2001:db8:1::2 and prefix 3000::100/120.
+    ASSERT_NO_THROW(client2.doRenew());
+
+    // The renewed address and prefix are now reserved for another client so
+    // available leases  should be allocated instead.
+    EXPECT_TRUE(client2.hasLeaseForAddress(IOAddress("2001:db8:1::1")));
+    EXPECT_TRUE(client2.hasLeaseForPrefix(IOAddress("3000::"), 120));
+    // The previously allocated leases should now be returned with zero lifetimes.
+    EXPECT_TRUE(client2.hasLeaseWithZeroLifetimeForAddress(IOAddress("2001:db8:1::2")));
+    EXPECT_TRUE(client2.hasLeaseWithZeroLifetimeForPrefix(IOAddress("3000::100"), 120));
+
+    // We've had a bug in DHCPv6 server that reused lease (allocated previously to
+    // a different client) was returned to the client reusing leases. This a big issue
+    // because effectively a client reusing an expired lease would get this lease twice:
+    // with non-zero lifetimes and the second time with zero lifetimes. This is seriously
+    // confusing for the clients. This checks tha the bug has been eliminated.
+    EXPECT_FALSE(client2.hasLeaseWithZeroLifetimeForAddress(IOAddress("2001:db8:1::1")));
+    EXPECT_FALSE(client2.hasLeaseWithZeroLifetimeForPrefix(IOAddress("3000::"), 120));
+}
+
 
 } // end of anonymous namespace