BIND 10 #3242: Kea4: check if only remote traffic can be supported

[BIND 10 Development]

#3242: Kea4: check if only remote traffic can be supported
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:
                Type:  defect        |  UnAssigned
            Priority:  high          |                       Status:
           Component:  dhcp4         |  reviewing
            Keywords:                |                    Milestone:  DHCP-
           Sensitive:  0             |  Kea0.9-alpha
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  24            |                 CVSS Scoring:
         Total Hours:  26            |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  10
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => UnAssigned
 * status:  assigned => reviewing


Comment:

 The ticket is now ready for review. I performed a manual system test,
 similar to the one that had been performed before the ticket was
 implemented. The server's behavior seems to be correct.

 '''Summary of changes'''
 There is a change in the server behavior with respect to subnet selection
 for the incoming packet. The server now differentiates between the
 directly connected clients, clients trying to renew leases and clients
 sending their messages through relays. For the directly connected clients
 the server will check interface configuration on which the message has
 been received and if the server configuration doesn't match any subnet the
 message is discarded. The !CfgMgr now needs to have access to the
 !IfaceMgr routines to check interface configuration when it gets the
 message from directly connected clients. This dependency on IfaceMgr
 complicates unit tests as they require fake interfaces to be configured.
 For this reason a new shared library has been created which is used by all
 unit tests that require fake interfaces.

 '''New server behavior'''
 The DHCPv4 server's behavior has changed to accommodate the following
 cases:
 - directly connected client broadcasts a packet over the interface for
 which there is no suitable subnet ---> the server has drop this packet and
 log the message
 - directly connected client broadcasts the message for which there is a
 subnet configured ---> the server should accept the message and process it
 and either respond with ACK or NAK
 - client unicasts its message over any interface --> such message should
 be accepted and processed over any interface, as the client may renew its
 lease
 - client's message is relayed --> the server must accept and process this
 message over any interface

 There is a new function which makes those decisions in the early
 processing stage

 The subnet selection criteria had to be changed also:
 - the subnet for the relayed client is selected using giaddr (no change
 here)
 - the subnet for the renewing client must be selected using client's
 ciaddr if its message is unicast because there is no giaddr present and it
 may come over any interface
 - the subnet for directly connected client which broadcasts the Request is
 selected using the address of the local interface over which the message
 has been received

 '''New !CfgMgr function'''
 There is a new function to select subnet using local interface name.
 Internally, it calls the getSubnet4(address).

 '''New test library to handle fake interfaces'''
 There is a new noinst library src/lib/dhcp/tests/libdhcptest.so which
 provides the means for configuring fake (non existent) interfaces on
 !IfaceMgr. This library is supposed to be used by all unit tests that
 require fake interfaces, including those from src/lib/dhcpsrv,
 src/bin/dhcp4 and src/bin/dhcp6. It was very inconvenient to create
 separate test fixture class for each set of unit tests, therefore this new
 library has been implemented.

 '''Unit tests refactoring'''
 In order to employ the new library I had to refactor a bunch of dhcp4_srv
 unit tests to make use of this library. There is quite a bit of similar,
 little changes in dhcpv4_srv_unittests.cc which configure fake interfaces
 for a test. Because of the hard coded interface configuration provided by
 this library, I had to change the interface names (from eth0 to eth1) for
 numerous tests because the eth1 configuration matches the subnets used by
 unit tests (not eth0). There is still some refactoring that should be done
 for numerous unit tests.

 '''Outstanding work'''
 When reading the RFC2131 I realized that we don't support option 50
 (requested ip address). The RFC2131 makes heavy use of this option, so I
 suppose it may be heavily used by clients too. I submitted #3320 to
 implement support for this option.

 I also updated the #3229 with some additional modifications that should be
 considered. In general I want the openActiveSockets functions to be moved
 out from the DHCPvX server code to the !CfgMgr. I also want to add a new
 log message there to inform that there is no suitable subnet for the
 interface on which the socket is being opened. This was supposed to be
 implemented in this ticket, but since I want to move these functions it
 seems more reasonable to me to add this log message then.

 If this ticket is approved, we will need to refactor other unit tests that
 use fake interfaces to use the new test library. But I don't submit a
 ticket on this yet, because I don't know if this whole work will not be
 shredded. ;)

 '''!ChangeLog'''
 {{{
 XXX.    [bug]           marcin
         b10-dhcp4 server picks a subnet, to assign address for a directly
         connected client, using IP address of the interface on which the
         client's message has been received. If the message is received on
         the interface for which there is no suitable subnet, the message
         is discarded. Also, the subnet for renewing client which unicasts
         its request, is selected using ciaddr.
         (Trac #3242, git abcd)

 }}}

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