BIND 10 #2681: DHCPv6 server crashes on Solicit and Request when pool has been exhausted.

[BIND 10 Development]

#2681: DHCPv6 server crashes  on Solicit and Request when pool has been exhausted.
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  defect        |  marcin
            Priority:  high          |                       Status:
           Component:  dhcp6         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130214
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  High
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => marcin


Comment:

 > '''src/bin/dhcp4/dhcp4_messages.mes'''
 > The formats of the DHCP4_PACKET_PROCESS_FAIL and
 DHCP6_PACKET_PROCESS_FAIL are inconsistent. The former takes just one
 argument that is used to log an exception string. The latter takes 2 more
 arguments that are used to log the name of the packet and remote address.
 The purpose of the debug message is to help root cause issues so it should
 provide as much information as possible. Therefore, I suggest that the
 debug message for DHCPv4 server provides the same level of information as
 the message logged by DHCPv6 server.
 Modified to include the hardware address (if present) of the source
 packet. (Information in the DHCP56 packet is not available in the V4
 packet.)

 > '''src/lib/dhcpsrv/alloc_engine.cc'''
 > Please update the date in the copyright header to 2013.
 Updated.


 > allocateAddress6: Line 205: The reuseExpiredLease() function may throw
 at least two types of exceptions derived from isc::Exception. These are:
 !NoSuchLease and !DbOperationError. If these exceptions are thrown from
 the line 205 it is not caught. Note that if the same function throws an
 exception a few lines further (line 251) the DHCPSRV_ADDRESS6_ALLOC_ERROR
 is logged.
 >
 > This has another implication that the client will not receive the
 !NoAddrsAvail status code because the code section that starts in
 dhcp6_srv.cc, line 654 will not be executed.
 Good catch.  I've extended the range of the "try" block to cover the
 entire method.  This does leave the possibility of a database problem
 generating multiple log messages, one per failed address allocation
 attempt, but that this something we will need to address in the future.


 > allocateAddress4: This function suffers from the same issue as the
 allocateAddress6 function. Thankfully, we don't need to reply to the
 client that we could not allocate the lease and the exceptions will be
 eventually caught by the try-catch surrounding processX() functions.
 Nevertheless, for this function we should choose whether we catch
 exceptions, log an error within the function and return NULL pointer OR
 that we catch exceptions externally. The current code returns a
 combination of exceptions and NULL pointers for similar issues.
 Same solution applied - all isc::Exceptions are caught and a null lease
 returned (which matches the description of the method in the .h file).

 > We should add a !ChangeLog entry for this bug fix.
 How about:
 {{{
 XXX.    [bug]           stephen
         Fixed problem where the DHCP server crashed if it
         ran out of addresses.  Such a condition now causes
         a packet to be returned to the client refusing the
         allocation of an address.
         (Trac #2681, git XXX)
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2681#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development