[2994] Sanity checks in Dhcp4Srv improved

src/bin/dhcp4/dhcp4_srv.cc

@@ -729,6 +729,9 @@ Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
 
 Pkt4Ptr
 Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
+
+    sanityCheck(discover, FORBIDDEN);
+
     Pkt4Ptr offer = Pkt4Ptr
         (new Pkt4(DHCPOFFER, discover->getTransid()));
 
@@ -946,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) {

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);