Browse Source

[2681] Handle exceptions thrown when unable to allocate an address

Explicit allocation failures cause the return of packet to the client
indicating that the address allocation has been refused.  Also added
was a "catch" for other exceptions - this causes the packet from the
client that generated the exception to be ignored.
Stephen Morris 12 years ago
parent
commit
9645476425

+ 5 - 0
src/bin/dhcp4/dhcp4_messages.mes

@@ -94,6 +94,11 @@ server is about to open sockets on the specified port.
 The IPv4 DHCP server has received a packet that it is unable to
 interpret. The reason why the packet is invalid is included in the message.
 
+% DHCP4_PACKET_PROCESS_FAIL failed to process received packet: %1
+This is a general catch-all message indicating that the processing of a
+received packet failed.  The reason is given in the message.  The server
+will not send a response but will instead ignore the packet.
+
 % DHCP4_PACKET_RECEIVED %1 (type %2) packet received on interface %3
 A debug message noting that the server has received the specified type of
 packet on the specified interface.  Note that a packet marked as UNKNOWN

+ 39 - 26
src/bin/dhcp4/dhcp4_srv.cc

@@ -136,32 +136,45 @@ Dhcpv4Srv::run() {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
                       .arg(query->toText());
 
-            switch (query->getType()) {
-            case DHCPDISCOVER:
-                rsp = processDiscover(query);
-                break;
-
-            case DHCPREQUEST:
-                rsp = processRequest(query);
-                break;
-
-            case DHCPRELEASE:
-                processRelease(query);
-                break;
-
-            case DHCPDECLINE:
-                processDecline(query);
-                break;
-
-            case DHCPINFORM:
-                processInform(query);
-                break;
-
-            default:
-                // Only action is to output a message if debug is enabled,
-                // and that will be covered by the debug statement before
-                // the "switch" statement.
-                ;
+            try {
+                switch (query->getType()) {
+                case DHCPDISCOVER:
+                    rsp = processDiscover(query);
+                    break;
+
+                case DHCPREQUEST:
+                    rsp = processRequest(query);
+                    break;
+
+                case DHCPRELEASE:
+                    processRelease(query);
+                    break;
+
+                case DHCPDECLINE:
+                    processDecline(query);
+                    break;
+
+                case DHCPINFORM:
+                    processInform(query);
+                    break;
+
+                default:
+                    // Only action is to output a message if debug is enabled,
+                    // and that is covered by the debug statement before the
+                    // "switch" statement.
+                    ;
+                }
+            } catch (const isc::Exception& e) {
+
+                // Catch-all exception (at least for ones based on the isc
+                // Exception class, which covers more or less all that
+                // are explicitly raised in the BIND 10 code).  Just log
+                // the problem and ignore the packet. (The problem is logged
+                // as a debug message because debug is disabled by default -
+                // it prevents a DDOS attack based on the sending of problem
+                // packets.)
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
+                          DHCP4_PACKET_PROCESS_FAIL).arg(e.what());
             }
 
             if (rsp) {

+ 5 - 0
src/bin/dhcp6/dhcp6_messages.mes

@@ -93,6 +93,11 @@ This message indicates that the server failed to grant (in response to
 received REQUEST) a lease for a given client. There may be many reasons for
 such failure. Each specific failure is logged in a separate log entry.
 
+% DHCP6_PACKET_PROCESS_FAIL processing of %1 message received from %2 failed: %3
+This is a general catch-all message indicating that the processing of the
+specified packet type from the indicated address failed.  The reason is given in the
+message.  The server will not send a response but will instead ignore the packet.
+
 % DHCP6_RELEASE address %1 belonging to client duid=%2, iaid=%3 was released properly.
 This debug message indicates that an address was released properly. It
 is a normal operation during client shutdown.

+ 20 - 6
src/bin/dhcp6/dhcp6_srv.cc

@@ -172,8 +172,8 @@ bool Dhcpv6Srv::run() {
                     break;
 
                 case DHCPV6_RELEASE:
-                rsp = processRelease(query);
-                break;
+                    rsp = processRelease(query);
+                    break;
 
                 case DHCPV6_DECLINE:
                     rsp = processDecline(query);
@@ -189,11 +189,26 @@ bool Dhcpv6Srv::run() {
                     // the "switch" statement.
                     ;
                 }
+
             } catch (const RFCViolation& e) {
                 LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_REQUIRED_OPTIONS_CHECK_FAIL)
                     .arg(query->getName())
                     .arg(query->getRemoteAddr())
                     .arg(e.what());
+
+            } catch (const isc::Exception& e) {
+
+                // Catch-all exception (at least for ones based on the isc
+                // Exception class, which covers more or less all that
+                // are explicitly raised in the BIND 10 code).  Just log
+                // the problem and ignore the packet. (The problem is logged
+                // as a debug message because debug is disabled by default -
+                // it prevents a DDOS attack based on the sending of problem
+                // packets.)
+                LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_PROCESS_FAIL)
+                    .arg(query->getName())
+                    .arg(query->getRemoteAddr())
+                    .arg(e.what());
             }
 
             if (rsp) {
@@ -663,11 +678,10 @@ OptionPtr Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         // cause of that failure. The only thing left is to insert
         // status code to pass the sad news to the client.
 
-        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, fake_allocation?
-                  DHCP6_LEASE_ADVERT_FAIL:DHCP6_LEASE_ALLOC_FAIL)
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, fake_allocation ?
+                  DHCP6_LEASE_ADVERT_FAIL : DHCP6_LEASE_ALLOC_FAIL)
             .arg(duid?duid->toText():"(no-duid)")
-            .arg(ia->getIAID())
-            .arg(subnet->toText());
+            .arg(ia->getIAID());
 
         ia_rsp->addOption(createStatusCode(STATUS_NoAddrsAvail,
                           "Sorry, no address could be allocated."));

+ 74 - 56
src/lib/dhcpsrv/alloc_engine.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dhcpsrv/alloc_engine.h>
+#include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 
 #include <cstring>
@@ -224,40 +225,49 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
     // left), but this has one major problem. We exactly control allocation
     // moment, but we currently do not control expiration time at all
 
-    unsigned int i = attempts_;
-    do {
-        IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
+    try {
+        unsigned int i = attempts_;
+        do {
+            IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
 
-        /// @todo: check if the address is reserved once we have host support
-        /// implemented
+            /// @todo: check if the address is reserved once we have host support
+            /// implemented
 
-        Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(candidate);
-        if (!existing) {
-            // there's no existing lease for selected candidate, so it is
-            // free. Let's allocate it.
-            Lease6Ptr lease = createLease6(subnet, duid, iaid, candidate,
-                                          fake_allocation);
-            if (lease) {
-                return (lease);
+            Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(candidate);
+            if (!existing) {
+                // there's no existing lease for selected candidate, so it is
+                // free. Let's allocate it.
+                Lease6Ptr lease = createLease6(subnet, duid, iaid, candidate,
+                                              fake_allocation);
+                if (lease) {
+                    return (lease);
+                }
+
+                // Although the address was free just microseconds ago, it may have
+                // been taken just now. If the lease insertion fails, we continue
+                // allocation attempts.
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, duid, iaid,
+                                              fake_allocation));
+                }
             }
 
-            // Although the address was free just microseconds ago, it may have
-            // been taken just now. If the lease insertion fails, we continue
-            // allocation attempts.
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, duid, iaid,
-                                          fake_allocation));
-            }
-        }
+            // Continue trying allocation until we run out of attempts
+            // (or attempts are set to 0, which means infinite)
+            --i;
+        } while ((i > 0) || !attempts_);
 
-        // continue trying allocation until we run out of attempts
-        // (or attempts are set to 0, which means infinite)
-        --i;
-    } while ( i || !attempts_);
+        // Unable to allocate an address, return an empty lease.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS6_ALLOC_FAIL).arg(attempts_);
+
+    } catch (const isc::Exception& e) {
+
+        // Some other error, return an empty lease.
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_ADDRESS6_ALLOC_ERROR).arg(e.what());
+    }
 
-    isc_throw(AllocFailed, "Failed to allocate address after " << attempts_
-              << " tries");
+    return (Lease6Ptr());
 }
 
 Lease4Ptr
@@ -342,40 +352,48 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
     // left), but this has one major problem. We exactly control allocation
     // moment, but we currently do not control expiration time at all
 
-    unsigned int i = attempts_;
-    do {
-        IOAddress candidate = allocator_->pickAddress(subnet, clientid, hint);
+    try {
+        unsigned int i = attempts_;
+        do {
+            IOAddress candidate = allocator_->pickAddress(subnet, clientid, hint);
 
-        /// @todo: check if the address is reserved once we have host support
-        /// implemented
+            /// @todo: check if the address is reserved once we have host support
+            /// implemented
 
-        Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate);
-        if (!existing) {
-            // there's no existing lease for selected candidate, so it is
-            // free. Let's allocate it.
-            Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, candidate,
-                                          fake_allocation);
-            if (lease) {
-                return (lease);
+            Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate);
+            if (!existing) {
+                // there's no existing lease for selected candidate, so it is
+                // free. Let's allocate it.
+                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, candidate,
+                                              fake_allocation);
+                if (lease) {
+                    return (lease);
+                }
+
+                // Although the address was free just microseconds ago, it may have
+                // been taken just now. If the lease insertion fails, we continue
+                // allocation attempts.
+            } else {
+                if (existing->expired()) {
+                    return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
+                                              fake_allocation));
+                }
             }
 
-            // Although the address was free just microseconds ago, it may have
-            // been taken just now. If the lease insertion fails, we continue
-            // allocation attempts.
-        } else {
-            if (existing->expired()) {
-                return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                          fake_allocation));
-            }
-        }
+            // Continue trying allocation until we run out of attempts
+            // (or attempts are set to 0, which means infinite)
+            --i;
+        } while ((i > 0) || !attempts_);
+
+        // Unable to allocate an address, return an empty lease.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_FAIL).arg(attempts_);
 
-        // Continue trying allocation until we run out of attempts
-        // (or attempts are set to 0, which means infinite)
-        --i;
-    } while ( i || !attempts_);
+    } catch (const isc::Exception& e) {
 
-    isc_throw(AllocFailed, "Failed to allocate address after " << attempts_
-              << " tries");
+        // Some other error, return an empty lease.
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_ADDRESS4_ALLOC_ERROR).arg(e.what());
+    }
+    return (Lease4Ptr());
 }
 
 Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,

+ 40 - 0
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -14,6 +14,46 @@
 
 $NAMESPACE isc::dhcp
 
+% DHCPSRV_ADDRESS4_ALLOC_ERROR error during attempt to allocate an IPv4 address: %1
+An error occurred during an attempt to allocate an IPv4 address, the
+reason for the failure being contained in the message.  The server will
+return a message to the client refusing a lease.
+
+% DHCPSRV_ADDRESS4_ALLOC_FAIL failed to allocate an IPv4 address after %1 attempt(s)
+THE DHCP allocation engine gave up trying to allocate an IPv4 address
+after the specified number of attempts.  This probably means that the
+address pool from which the allocation is being attempted is either
+empty, or very nearly empty.  As a result, the client will have been
+refused a lease.
+
+This message may indicate that your address pool is too small for the
+number of clients you are trying to service and should be expanded.
+Alternatively, if the you know that the number of concurrently active
+clients is less than the addresses you have available, you may want to
+consider reducing the lease lifetime.  In this way, addresses allocated
+to clients that are no longer active on the network will become available
+available sooner.
+
+% DHCPSRV_ADDRESS6_ALLOC_ERROR error during attempt to allocate an IPv6 address: %1
+An error occurred during an attempt to allocate an IPv6 address, the
+reason for the failure being contained in the message.  The server will
+return a message to the client refusing a lease.
+
+% DHCPSRV_ADDRESS6_ALLOC_FAIL failed to allocate an IPv6 address after %1 attempt(s)
+The DHCP allocation engine gave up trying to allocate an IPv6 address
+after the specified number of attempts.  This probably means that the
+address pool from which the allocation is being attempted is either
+empty, or very nearly empty.  As a result, the client will have been
+refused a lease.
+
+This message may indicate that your address pool is too small for the
+number of clients you are trying to service and should be expanded.
+Alternatively, if the you know that the number of concurrently active
+clients is less than the addresses you have available, you may want to
+consider reducing the lease lifetime.  In this way, addresses allocated
+to clients that are no longer active on the network will become available
+available sooner.
+
 % DHCPSRV_CFGMGR_ADD_SUBNET4 adding subnet %1
 A debug message reported when the DHCP configuration manager is adding the
 specified IPv4 subnet to its database.

+ 6 - 5
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -461,9 +461,9 @@ TEST_F(AllocEngine6Test, outOfAddresses6) {
 
     // There is just a single address in the pool and allocated it to someone
     // else, so the allocation should fail
-
-    EXPECT_THROW(engine->allocateAddress6(subnet_, duid_, iaid_, IOAddress("::"),false),
-                 AllocFailed);
+    Lease6Ptr lease2 = engine->allocateAddress6(subnet_, duid_, iaid_,
+                                                IOAddress("::"), false);
+    EXPECT_FALSE(lease2);
 }
 
 // This test checks if an expired lease can be reused in SOLICIT (fake allocation)
@@ -838,8 +838,9 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
     // There is just a single address in the pool and allocated it to someone
     // else, so the allocation should fail
 
-    EXPECT_THROW(engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),false),
-                 AllocFailed);
+    Lease4Ptr lease2 = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
+                                                IOAddress("0.0.0.0"), false);
+    EXPECT_FALSE(lease2);
 }
 
 // This test checks if an expired lease can be reused in DISCOVER (fake allocation)