From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 23 Sep 2024 08:36:09 +0000 (+0200)
Subject: [#3758] [#3565] Prevent declining expired and released
X-Git-Tag: Kea-2.6.2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a9c21a21b54a4b8c5a60d5bc7f8da6a30a4b49a;p=thirdparty%2Fkea.git

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

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 8e2e57bcc9..eb9cb136db 100644
--- 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 782b3aaf65..325184fa3b 100644
--- 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 d8b0dd7813..ff5e83c95c 100644
--- 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 92f511d7c2..e5c28ee0d3 100644
--- 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