[3565] The v6 server is now able to allocate new leases during Renew

src/bin/dhcp6/dhcp6_srv.cc

@@ -1587,6 +1587,10 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         hints_count++;
     }
 
+    // If this is a Renew, it's ok to allocate new leases
+    if (query->getType() == DHCPV6_RENEW) {
+        ctx.allow_new_leases_in_renewals_ = true;
+    }
     Lease6Collection leases = alloc_engine_->renewLeases6(ctx);
 
     // Ok, now we have the leases extended. We have:

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -903,20 +903,38 @@ TEST_F(Dhcpv6SrvTest, pdRenewBasic) {
 }
 
 // This test verifies that incoming (invalid) RENEW with an address
-// can be handled properly.
-//
-// This test checks 3 scenarios:
-// 1. there is no such lease at all
-// 2. there is such a lease, but it is assigned to a different IAID
-// 3. there is such a lease, but it belongs to a different client
+// can be handled properly. This has changed with #3565. The server
+// is now able to allocate a lease in Renew if it's available.
+// Previous testRenewReject is now split into 3 tests.
 //
-// expected:
-// - returned REPLY message has copy of client-id
-// - returned REPLY message has server-id
-// - returned REPLY message has IA_NA that includes STATUS-CODE
-// - No lease in LeaseMgr
-TEST_F(Dhcpv6SrvTest, RenewReject) {
-    testRenewReject(Lease::TYPE_NA, IOAddress("2001:db8:1:1::dead"));
+// This test checks the first scenario: There is no lease at all.
+// The server will try to assign it. Since it is not used by anyone else,
+// the server will assign it. This is convenient for various types
+// of recoveries, e.g. when the server lost its database.
+TEST_F(Dhcpv6SrvTest, RenewUnknown) {
+    // False means that the lease should not be created before renewal attempt
+    testRenewBasic(Lease::TYPE_NA, "2001:db8:1:1::abc", "2001:db8:1:1::abc",
+                   128, false);
+}
+
+// This test checks that a client that renews existing lease, but uses
+// a wrong IAID, will be processed correctly. As there is no lease for
+// this (duid, type, iaid) tuple, this is treated as a new IA, regardless
+// if the client inserted an address that is used in a different IA.
+// After #3565 was implemented, the server will attempt to assign a lease.
+// The one that client requested is already used with different IAID, so
+// it will just pick a different lease. This is the second out of three
+// scenarios tests by old RenewReject test.
+TEST_F(Dhcpv6SrvTest, RenewWrongIAID) {
+    testRenewWrongIAID(Lease::TYPE_NA, IOAddress("2001:db8:1:1::abc"));
+}
+
+// This test checks whether client A can renew an address that is currently
+// leased by client B. The server should detect that the lease belong to
+// someone else and assign a different lease. This is the third out of three
+// scenarios tests by old RenewReject test.
+TEST_F(Dhcpv6SrvTest, RenewSomeoneElesesLease) {
+    testRenewSomeoneElsesLease(Lease::TYPE_NA, IOAddress("2001:db8::1"));
 }
 
 // This test verifies that incoming (invalid) RENEW with a prefix

src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -200,7 +200,7 @@ Dhcpv6SrvTest::createIA(isc::dhcp::Lease::Type lease_type,
 void
 Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr,
                               const std::string& renew_addr,
-                              const uint8_t prefix_len) {
+                              const uint8_t prefix_len, bool insert_before_renew) {
     NakedDhcpv6Srv srv(0);
 
     const IOAddress existing(existing_addr);
@@ -213,24 +213,27 @@ Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr
     // Check that the address we are about to use is indeed in pool
     ASSERT_TRUE(subnet_->inPool(type, existing));
 
-    // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
-    // value on purpose. They should be updated during RENEW.
-    Lease6Ptr lease(new Lease6(type, existing, duid_, iaid, 501, 502, 503, 504,
-                               subnet_->getID(), HWAddrPtr(), prefix_len));
-    lease->cltt_ = 1234;
-    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+    Lease6Ptr l;
+    if (insert_before_renew) {
+        // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
+        // value on purpose. They should be updated during RENEW.
+        Lease6Ptr lease(new Lease6(type, existing, duid_, iaid, 501, 502, 503, 504,
+                                   subnet_->getID(), HWAddrPtr(), prefix_len));
+        lease->cltt_ = 1234;
+        ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
-    // Check that the lease is really in the database
-    Lease6Ptr l = LeaseMgrFactory::instance().getLease6(type, existing);
-    ASSERT_TRUE(l);
+        // Check that the lease is really in the database
+        l = LeaseMgrFactory::instance().getLease6(type, existing);
+        ASSERT_TRUE(l);
 
-    // Check that T1, T2, preferred, valid and cltt really set and not using
-    // previous (500, 501, etc.) values
-    EXPECT_NE(l->t1_, subnet_->getT1());
-    EXPECT_NE(l->t2_, subnet_->getT2());
-    EXPECT_NE(l->preferred_lft_, subnet_->getPreferred());
-    EXPECT_NE(l->valid_lft_, subnet_->getValid());
-    EXPECT_NE(l->cltt_, time(NULL));
+        // Check that T1, T2, preferred, valid and cltt really set and not using
+        // previous (500, 501, etc.) values
+        EXPECT_NE(l->t1_, subnet_->getT1());
+        EXPECT_NE(l->t2_, subnet_->getT2());
+        EXPECT_NE(l->preferred_lft_, subnet_->getPreferred());
+        EXPECT_NE(l->valid_lft_, subnet_->getValid());
+        EXPECT_NE(l->cltt_, time(NULL));
+    }
 
     Pkt6Ptr req = createMessage(DHCPV6_RENEW, type, IOAddress(renew_addr),
                                 prefix_len, iaid);
@@ -300,6 +303,114 @@ Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr
 }
 
 void
+Dhcpv6SrvTest::testRenewWrongIAID(Lease::Type type, const IOAddress& addr) {
+
+    NakedDhcpv6Srv srv(0);
+
+    const uint32_t transid = 1234;
+    const uint32_t valid_iaid = 234;
+    const uint32_t bogus_iaid = 456;
+
+    uint8_t prefix_len = (type == Lease::TYPE_PD) ? 128 : pd_pool_->getLength();
+
+    // Quick sanity check that the address we're about to use is ok
+    ASSERT_TRUE(subnet_->inPool(type, addr));
+
+    // Check that the lease is NOT in the database
+    Lease6Ptr l = LeaseMgrFactory::instance().getLease6(type, addr);
+    ASSERT_FALSE(l);
+
+    // GenerateClientId() also sets duid_
+    OptionPtr clientid = generateClientId();
+
+    // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
+    // value on purpose. They should be updated during RENEW.
+    Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid,
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), prefix_len));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // Pass it to the server and hope for a REPLY
+    // Let's create a RENEW
+    Pkt6Ptr renew = createMessage(DHCPV6_RENEW, type, IOAddress(addr), prefix_len,
+                                  bogus_iaid);
+    renew->addOption(clientid);
+    renew->addOption(srv.getServerID());
+
+    // The duid and address matches, but the iaid is different. The server could
+    // respond with NoBinding. However, according to
+    // draft-ietf-dhc-dhcpv6-stateful-issues-10, the server can also assign a
+    // new address. And that's what we expect here.
+    Pkt6Ptr reply = srv.processRenew(renew);
+    checkResponse(reply, DHCPV6_REPLY, transid);
+
+    // Check that IA_NA was returned and that there's an address included
+    boost::shared_ptr<Option6IAAddr>
+        addr_opt = checkIA_NA(reply, bogus_iaid, subnet_->getT1(), subnet_->getT2());
+
+    ASSERT_TRUE(addr_opt);
+
+    // Check that we've got the an address
+    checkIAAddr(addr_opt, addr_opt->getAddress(), Lease::TYPE_NA);
+
+    // Check that we got a different address than was in the database.
+    EXPECT_NE(addr_opt->getAddress().toText(), addr.toText());
+
+    // Check that the lease is really in the database
+    l = checkLease(duid_, reply->getOption(D6O_IA_NA), addr_opt);
+    ASSERT_TRUE(l);
+}
+
+void
+Dhcpv6SrvTest::testRenewSomeoneElsesLease(Lease::Type type, const IOAddress& addr) {
+
+    NakedDhcpv6Srv srv(0);
+    const uint32_t valid_iaid = 234;
+    const uint32_t transid = 1234;
+
+    uint8_t prefix_len = (type == Lease::TYPE_PD) ? 128 : pd_pool_->getLength();
+
+    // GenerateClientId() also sets duid_
+    OptionPtr clientid = generateClientId();
+
+    // Let's create a lease.
+    Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid,
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), prefix_len));
+    ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
+
+    // CASE 3: Lease belongs to a client with different client-id
+    Pkt6Ptr renew = createMessage(DHCPV6_RENEW, type, IOAddress(addr), prefix_len,
+                                  valid_iaid);
+    renew->addOption(generateClientId(13)); // generate different DUID (length 13)
+    renew->addOption(srv.getServerID());
+
+    // The iaid and address matches, but the duid is different.
+    // The server should not renew it, but assign something else.
+    Pkt6Ptr reply = srv.processRenew(renew);
+    checkResponse(reply, DHCPV6_REPLY, transid);
+    OptionPtr tmp = reply->getOption(D6O_IA_NA);
+    ASSERT_TRUE(tmp);
+
+    // Check that IA_?? was returned and that there's proper status code
+    // Check that IA_NA was returned and that there's an address included
+    boost::shared_ptr<Option6IAAddr>
+        addr_opt = checkIA_NA(reply, valid_iaid, subnet_->getT1(), subnet_->getT2());
+
+    ASSERT_TRUE(addr_opt);
+
+    // Check that we've got the an address
+    checkIAAddr(addr_opt, addr_opt->getAddress(), Lease::TYPE_NA);
+
+    // Check that we got a different address than was in the database.
+    EXPECT_NE(addr_opt->getAddress().toText(), addr.toText());
+
+    // Check that the lease is really in the database
+    Lease6Ptr l = checkLease(duid_, reply->getOption(D6O_IA_NA), addr_opt);
+    ASSERT_TRUE(l);
+}
+
+void
 Dhcpv6SrvTest::testRenewReject(Lease::Type type, const IOAddress& addr) {
 
     NakedDhcpv6Srv srv(0);
@@ -349,7 +460,41 @@ Dhcpv6SrvTest::testRenewReject(Lease::Type type, const IOAddress& addr) {
     // Check that IA_?? was returned and that there's proper status code
     boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(tmp);
     ASSERT_TRUE(ia);
-    checkIA_NAStatusCode(ia, STATUS_NoBinding, subnet_->getT1(), subnet_->getT2());
+
+    if (type == Lease::TYPE_PD) {
+        // For PD, the check is easy. NoBinding and no prefixes
+        checkIA_NAStatusCode(ia, STATUS_NoBinding, subnet_->getT1(), subnet_->getT2());
+    } else {
+        // For IA, it's more involved, as the server will reject the address
+        // (and send it with 0 lifetimes), but will also assign a new address.
+
+        // First, check that the requested address is rejected.
+        bool found = false;
+
+        dhcp::OptionCollection options = ia->getOptions();
+        for (isc::dhcp::OptionCollection::iterator opt = options.begin();
+             opt != options.end(); ++opt) {
+            if (opt->second->getType() != D6O_IAADDR) {
+                continue;
+            }
+
+            dhcp::Option6IAAddrPtr opt_addr =
+                boost::dynamic_pointer_cast<isc::dhcp::Option6IAAddr>(opt->second);
+            ASSERT_TRUE(opt_addr);
+
+            if (opt_addr->getAddress() != addr) {
+                // There may be other addresses, e.g. the newly assigned one
+                continue;
+            }
+
+            found = true;
+            EXPECT_NE(0, opt_addr->getPreferred());
+            EXPECT_NE(0, opt_addr->getValid());
+        }
+
+        EXPECT_TRUE(found) << "Expected address " << addr.toText()
+                           << " with zero lifetimes not found.";
+    }
 
     // Check that there is no lease added
     l = LeaseMgrFactory::instance().getLease6(type, addr);
@@ -654,24 +799,27 @@ Dhcpv6SrvTest::compareOptions(const isc::dhcp::OptionPtr& option1,
 void
 NakedDhcpv6SrvTest::checkIA_NAStatusCode(
     const boost::shared_ptr<isc::dhcp::Option6IA>& ia,
-    uint16_t expected_status_code, uint32_t expected_t1, uint32_t expected_t2)
+    uint16_t expected_status_code, uint32_t expected_t1, uint32_t expected_t2,
+    bool check_addr)
 {
     // Make sure there is no address assigned. Depending on the situation,
     // the server will either not return the address at all and sometimes
     // it will return it with zeroed lifetimes.
-    dhcp::OptionCollection options = ia->getOptions();
-    for (isc::dhcp::OptionCollection::iterator opt = options.begin();
-         opt != options.end(); ++opt) {
-        if (opt->second->getType() != D6O_IAADDR) {
-            continue;
+    if (check_addr) {
+        dhcp::OptionCollection options = ia->getOptions();
+        for (isc::dhcp::OptionCollection::iterator opt = options.begin();
+             opt != options.end(); ++opt) {
+            if (opt->second->getType() != D6O_IAADDR) {
+                continue;
+            }
+
+            dhcp::Option6IAAddrPtr addr =
+                boost::dynamic_pointer_cast<isc::dhcp::Option6IAAddr>(opt->second);
+            ASSERT_TRUE(addr);
+
+            EXPECT_EQ(0, addr->getPreferred());
+            EXPECT_EQ(0, addr->getValid());
         }
-
-        dhcp::Option6IAAddrPtr addr =
-            boost::dynamic_pointer_cast<isc::dhcp::Option6IAAddr>(opt->second);
-        ASSERT_TRUE(addr);
-
-        EXPECT_EQ(0, addr->getPreferred());
-        EXPECT_EQ(0, addr->getValid());
     }
 
     // T1, T2 should NOT be zeroed. draft-ietf-dhc-dhcpv6-stateful-issues-10,

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -231,18 +231,26 @@ public:
                              expected_t2);
     }
 
-    // Checks that server rejected IA_NA, i.e. that it has no addresses and
-    // that expected status code really appears there. In some limited cases
-    // (reply to RELEASE) it may be used to verify positive case, where
-    // IA_NA response is expected to not include address.
-    //
-    // Status code indicates type of error encountered (in theory it can also
-    // indicate success, but servers typically don't send success status
-    // as this is the default result and it saves bandwidth)
+    /// @brief Checks that the server inserted expected status code in IA_NA
+    ///
+    /// Check that the server used status code as expected, i.e. that it has
+    /// no addresses (if status code is non-zero) and that expected status
+    /// code really appears there. In some limited cases (reply to RELEASE)
+    /// it may be used to verify positive case, where IA_NA response is
+    /// expected to not include address.
+    ///
+    /// Status code indicates type of error encountered (in theory it can also
+    /// indicate success, but servers typically don't send success status
+    /// as this is the default result and it saves bandwidth)
+    /// @param ia IA_NA container to be checked
+    /// @param expected_status_code expected value in status-code option
+    /// @param expected_t1 expected T1 in IA_NA option
+    /// @param expected_t2 expected T2 in IA_NA option
+    /// @param check_addr whether to check for address with 0 lifetimes
     void checkIA_NAStatusCode
         (const boost::shared_ptr<isc::dhcp::Option6IA>& ia,
          uint16_t expected_status_code, uint32_t expected_t1,
-         uint32_t expected_t2);
+         uint32_t expected_t2, bool check_addr = true);
 
     void checkMsgStatusCode(const isc::dhcp::Pkt6Ptr& msg,
                             uint16_t expected_status)
@@ -464,10 +472,29 @@ public:
     /// @param existing_addr address to be preinserted into the database
     /// @param renew_addr address being sent in RENEW
     /// @param prefix_len length of the prefix (128 for addresses)
+    /// @param insert_before_renew should the lease be inserted into the database
+    ///        before we try renewal?
     void
     testRenewBasic(isc::dhcp::Lease::Type type,
                    const std::string& existing_addr,
-                   const std::string& renew_addr, const uint8_t prefix_len);
+                   const std::string& renew_addr, const uint8_t prefix_len,
+                   bool insert_before_renew = true);
+
+    /// @brief Checks if RENEW with invalid IAID is processed correctly.
+    ///
+    /// @param type lease type (currently only IA_NA is supported)
+    /// @param addr address to be renewed
+    void
+    testRenewWrongIAID(isc::dhcp::Lease::Type type,
+                       const asiolink::IOAddress& addr);
+
+    /// @brief Checks if client A can renew address used by client B
+    ///
+    /// @param type lease type (currently only IA_NA is supported)
+    /// @param addr address to be renewed
+    void
+    testRenewSomeoneElsesLease(isc::dhcp::Lease::Type type,
+                               const asiolink::IOAddress& addr);
 
     /// @brief Performs negative RENEW test
     ///