BIND 10 #3279: DHCPv4 server should discard packets with non-matching server id

[BIND 10 Development]

#3279: DHCPv4 server should discard packets with non-matching server id
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  defect        |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp4         |  reviewing
            Keywords:                |                    Milestone:  DHCP-
           Sensitive:  0             |  Kea1.0-alpha
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  16            |                 CVSS Scoring:
         Total Hours:  10.5          |              Defect Severity:
                                     |  Medium
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  .5
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * hours:  2 => .5
 * owner:  tmark => marcin
 * totalhours:  10 => 10.5


Comment:

 Replying to [comment:8 marcin]:
 > Replying to [comment:7 tmark]:
 > > The changes are quite straight forward, I only have a few
 questions/comments:
 > >
 > > General question:
 > >
 > > * In looking at IfaceMgr::hasOpenSocket(), have you done any impact
 assessment on performance?  Granted we are not likely to have more than a
 few interfaces, with maybe one address per interface.
 >
 > I added a comment to say that I am aware of performance penalty. But I
 really don't think it is going to be significant if we just open sockets
 on a couple of interfaces. If the performance penalty is significant, we
 can think about caching the server identifiers, but I don't want to
 complicate this code more than it is complicated now.
 >
 > >
 > > * Should we consider making this check configurable?
 >
 > Unless I misunderstand this question, my answer is no. Accepting or
 rejecting server identifier is a core functionality of DHCPv4 described in
 RFC2131 (section 4.1). How could I make this configurable?
 >
 >
 > >
 > >
 ----------------------------------------------------------------------------------------
 > > Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt)
 > >
 > > You test the address list for size = 1 and return false if it is not.
 I'm
 > > assuming there is no valid case for it have more than 1 entry.  You
 may want
 > > to add a quick comment on this above the size test.
 >
 > Added a comment. There should be no case that the size of the list is
 unequal 1, unless the client has sent a malformed option which we would
 (and want to) catch here.
 >
 > >
 > >
 -----------------------------------------------------------------------------------------
 > >
 > > Unit tests run with valgrind on Fedora 19.  cppcheck passes on OS-X.
 > >
 > >
 > >
 >
 > Thank you QA team!

 Changes are fine, please merge.  Perhaps the ChangeLog should cite the
 RFC. I will leave that to you.

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