[#3758] [#3565] Prevent declining expired and released

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 8e2e57bcc90123de5c12bb7b24d41317015eafe7..eb9cb136db5f2a2e6a16e78df75862350662501c 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -4021,8 +4021,13 @@ Dhcpv4Srv::processDecline(Pkt4Ptr& decline, AllocEngine::ClientContext4Ptr& cont
         client_id.reset(new ClientId(opt_clientid->getData()));
     }
 
-    // Check if the client attempted to decline a lease it doesn't own.
-    if (!lease->belongsToClient(decline->getHWAddr(), client_id)) {
+    // Check if the client attempted to decline an expired lease or a lease
+    // it doesn't own. Declining expired leases is typically a client
+    // misbehavior and may lead to pool exhaustion in case of a storm of
+    // such declines. Only decline the lease if the lease has been recently
+    // allocated to the client.
+    if (lease->expired() || lease->state_ != Lease::STATE_DEFAULT ||
+        !lease->belongsToClient(decline->getHWAddr(), client_id)) {
 
         // Get printable hardware addresses
         string client_hw = decline->getHWAddr() ?

diff --git a/src/bin/dhcp4/tests/decline_unittest.cc b/src/bin/dhcp4/tests/decline_unittest.cc
index 782b3aaf65c7a7fd38f43bb4537e9521ff56d828..325184fa3b6441f010c3f13da60a3348b491f4c4 100644 (file)
--- a/src/bin/dhcp4/tests/decline_unittest.cc
+++ b/src/bin/dhcp4/tests/decline_unittest.cc
@@ -364,4 +364,77 @@ TEST_F(DeclineTest, declineNonMatchingIPAddress) {
     EXPECT_EQ(Lease::STATE_DEFAULT, lease->state_);
 }
 
+// Test that the released lease cannot be declined.
+TEST_F(DeclineTest, declineAfterRelease) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer(), true, true, true, false, LEASE_AFFINITY_ENABLED);
+    // Perform 4-way exchange to obtain a new lease.
+    acquireLease(client);
+
+    // Remember the acquired address.
+    IOAddress leased_address = client.config_.lease_.addr_;
+
+    // Release the acquired lease.
+    client.doRelease();
+
+    // Try to decline the released address.
+    client.config_.lease_.addr_ = leased_address;
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // released state.
+    Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(leased_address);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_RELEASED, lease->state_);
+}
+
+// Test that the expired lease cannot be declined.
+TEST_F(DeclineTest, declineAfterExpire) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    acquireLease(client);
+
+    // Age the lease (expire it).
+    auto lease = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_);
+    ASSERT_TRUE(lease);
+    lease->cltt_ -= 7200;
+    LeaseMgrFactory::instance().updateLease4(lease);
+
+    // Try to decline the expired lease.
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // default state.
+    lease = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_DEFAULT, lease->state_);
+}
+
+// Test that expired-reclaimed lease cannot be declined.
+TEST_F(DeclineTest, declineAfterReclamation) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    acquireLease(client);
+
+    // Reclaim the lease.
+    auto lease = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_);
+    ASSERT_TRUE(lease);
+    lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
+    LeaseMgrFactory::instance().updateLease4(lease);
+
+    // Try to decline the reclaimed lease.
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // reclaimed state.
+    lease = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_EXPIRED_RECLAIMED, lease->state_);
+}
+
 } // end of anonymous namespace

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index d8b0dd781393dc9fa70deb9490bcf5b5238a989e..ff5e83c95ccff10bb9b63e7531ae4b4be5ca875b 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -4069,7 +4069,7 @@ Dhcpv6Srv::declineIA(const Pkt6Ptr& decline, const DuidPtr& duid,
         Lease6Ptr lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
                                                                 decline_addr->getAddress());
 
-        if (!lease) {
+        if (!lease || lease->expired() || lease->state_ != Lease::STATE_DEFAULT) {
             // Client trying to decline a lease that we don't know about.
             LOG_INFO(lease6_logger, DHCP6_DECLINE_FAIL_NO_LEASE)
                 .arg(decline->getLabel()).arg(decline_addr->getAddress().toText());

diff --git a/src/bin/dhcp6/tests/decline_unittest.cc b/src/bin/dhcp6/tests/decline_unittest.cc
index 92f511d7c2e22cbdf73ef6f5deea25279ce4bda4..e5c28ee0d3adb9bb95565886545ac70bf1b967a2 100644 (file)
--- a/src/bin/dhcp6/tests/decline_unittest.cc
+++ b/src/bin/dhcp6/tests/decline_unittest.cc
@@ -329,4 +329,92 @@ TEST_F(DeclineTest, noIAs) {
                       NO_IA, SHOULD_FAIL);
 }
 
+// Test that the released lease cannot be declined.
+TEST_F(DeclineTest, declineAfterRelease) {
+    Dhcp6Client client;
+    uint32_t iaid = 1;
+    client.requestAddress(iaid);
+
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    client.doSARR();
+    auto leases = client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(iaid));
+
+    // Release the acquired lease.
+    auto lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, leases[0].addr_);
+    lease->state_ = Lease::STATE_RELEASED;
+    LeaseMgrFactory::instance().updateLease6(lease);
+
+    // Try to decline the released address.
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // released state.
+    lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, lease->addr_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_RELEASED, lease->state_);
+}
+
+// Test that the released lease cannot be declined.
+TEST_F(DeclineTest, declineAfterExpire) {
+    Dhcp6Client client;
+    uint32_t iaid = 1;
+    client.requestAddress(iaid);
+
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    client.doSARR();
+    auto leases = client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(iaid));
+
+    // Release the acquired lease.
+    auto lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, leases[0].addr_);
+    lease->cltt_ -= 7200;
+    LeaseMgrFactory::instance().updateLease6(lease);
+
+    // Try to decline the expired lease.
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // default state.
+    lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, lease->addr_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_DEFAULT, lease->state_);
+}
+
+// Test that expired-reclaimed lease cannot be declined.
+TEST_F(DeclineTest, declineAfterReclamation) {
+    Dhcp6Client client;
+    uint32_t iaid = 1;
+    client.requestAddress(iaid);
+
+    // Configure DHCP server.
+    configure(DECLINE_CONFIGS[0], *client.getServer());
+    // Perform 4-way exchange to obtain a new lease.
+    client.doSARR();
+    auto leases = client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(iaid));
+
+    // Release the acquired lease.
+    auto lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, leases[0].addr_);
+    lease->state_ = Lease::STATE_EXPIRED_RECLAIMED;
+    LeaseMgrFactory::instance().updateLease6(lease);
+
+    // Try to decline the reclaimed lease.
+    ASSERT_NO_THROW(client.doDecline());
+
+    // The address should not be declined. It should still be in the
+    // reclaimed state.
+    lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, lease->addr_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ(Lease::STATE_EXPIRED_RECLAIMED, lease->state_);
+}
+
+
 } // end of anonymous namespace