BIND 10 trac2994, updated. 07074c5295156356a0608fbaf8d706f5eaaaaef8 [2994] Sanity checks in Dhcp4Srv improved

[BIND 10 source code commits]

The branch, trac2994 has been updated
       via  07074c5295156356a0608fbaf8d706f5eaaaaef8 (commit)
       via  a076d799d926d23e67f495df40812cead7425aa4 (commit)
      from  01bb436c45f4d92a3575ff4ad2f5c1cb348ce399 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 07074c5295156356a0608fbaf8d706f5eaaaaef8
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Jul 16 13:58:01 2013 +0200

    [2994] Sanity checks in Dhcp4Srv improved

commit a076d799d926d23e67f495df40812cead7425aa4
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Tue Jul 16 13:29:41 2013 +0200

    [2994] Compilation fix in Dhcp4Srv

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp4/dhcp4_srv.cc                |   23 ++++++++++++++++++++++-
 src/bin/dhcp4/tests/dhcp4_srv_unittest.cc |   13 +++++++++++--
 2 files changed, 33 insertions(+), 3 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 9ab1800..195e3d8 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -608,12 +608,15 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     // allocation.
     bool fake_allocation = (question->getType() == DHCPDISCOVER);
 
+    CalloutHandlePtr callout_handle = getCalloutHandle(question);
+
     // Use allocation engine to pick a lease for this client. Allocation engine
     // will try to honour the hint, but it is just a hint - some other address
     // may be used instead. If fake_allocation is set to false, the lease will
     // be inserted into the LeaseMgr as well.
     Lease4Ptr lease = alloc_engine_->allocateAddress4(subnet, client_id, hwaddr,
-                                                      hint, fake_allocation);
+                                                      hint, fake_allocation,
+                                                      callout_handle);
 
     if (lease) {
         // We have a lease! Let's set it in the packet and send it back to
@@ -726,6 +729,9 @@ Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
 
 Pkt4Ptr
 Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
+
+    sanityCheck(discover, FORBIDDEN);
+
     Pkt4Ptr offer = Pkt4Ptr
         (new Pkt4(DHCPOFFER, discover->getTransid()));
 
@@ -943,6 +949,21 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
         // do nothing here
         ;
     }
+
+    // If there is HWAddress set and it is non-empty, then we're good
+    if (pkt->getHWAddr() && !pkt->getHWAddr()->hwaddr_.empty())
+        return;
+
+    // There has to be something to uniquely identify the client:
+    // either non-zero MAC address or client-id option present (or both)
+    OptionPtr client_id = pkt->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+
+    // If there's no client-id (or a useless one is provided, i.e. 0 length)
+    if (!client_id || client_id->len() == client_id->getHeaderLen()) {
+        isc_throw(RFCViolation, "Missing or useless client-id and no HW address "
+                  " provided in message "
+                  << serverReceivedPacketName(pkt->getType()));
+    }
 }
 
 isc::hooks::CalloutHandlePtr Dhcpv4Srv::getCalloutHandle(const Pkt4Ptr& pkt) {
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 2e4e5aa..4612643 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -1044,6 +1044,7 @@ TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
     Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
     dis->setRemoteAddr(IOAddress("192.0.2.1"));
     dis->setYiaddr(hint);
+    dis->setHWAddr(generateHWAddr(6));
 
     // Pass it to the server and get an offer
     Pkt4Ptr offer = srv->processDiscover(dis);
@@ -1405,8 +1406,9 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
     ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
 
     Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    pkt->setHWAddr(generateHWAddr(6));
 
-    // Client-id is optional for information-request, so
+    // Server-id is optional for information-request, so
     EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
 
     // Empty packet, no server-id
@@ -1420,6 +1422,11 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
     // Server-id is forbidden, but present => exception
     EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
                  RFCViolation);
+
+    // There's no client-id and no HWADDR. Server needs something to
+    // identify the client
+    pkt->setHWAddr(generateHWAddr(0));
+    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
 }
 
 // This test verifies that incoming (positive) RELEASE can be handled properly.
@@ -1791,8 +1798,10 @@ public:
         Pkt4Ptr pkt;
         callout_handle.getArgument("query4", pkt);
 
-        // get rid of the old client-id
+        // get rid of the old client-id (and no HWADDR)
+        vector<uint8_t> mac;
         pkt->delOption(DHO_DHCP_CLIENT_IDENTIFIER);
+        pkt->setHWAddr(1, 0, mac); // HWtype 1, hwardware len = 0
 
         // carry on as usual
         return pkt4_receive_callout(callout_handle);