[5553] Added specific log for possible BOOTP packets

src/bin/dhcp4/dhcp4_messages.mes
    Added DHCP4_PACKET_DROP_0009 for possible bootp packets

src/bin/dhcp4/dhcp4_srv.cc
    Dhcpv4Srv::acceptMessageType()
        rearranged a bit to test explicitly for DHCP_NOTYPE

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
    TEST_F(Dhcpv4SrvTest, acceptMessageType)
    Added tests for packets with no option 53 and for
    type > DHCPLEASEQUERYDONE

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index fef6ef62dc4d988dc509d299e113c864bf6fd009..e7f222ff81f2aa9f8e1982549547aa923edc173b 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -428,6 +428,11 @@ has been temporarily disabled. This affects all received DHCP packets. The
 service may be enabled by the "dhcp-enable" control command or automatically
 after a specified amount of time since receiving "dhcp-disable" command.
 
+% DHCP4_PACKET_DROP_0009 %1: Option 53 missing (no DHCP message type), is this a BOOTP packet?
+This debug message is issued when a packet is dropped because it did contain
+option 53 and thus has no DHCP message type. The most likely explanation is
+that it was BOOTP packet.
+
 % DHCP4_PACKET_NAK_0001 %1: failed to select a subnet for incoming packet, src %2, type %3
 This error message is output when a packet was received from a subnet
 for which the DHCPv4 server has not been configured. The most probable

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 03b3873d73af1a62481b030b3e25a212d95b42fa..ead5560ccb129ec838eabb797f0425d7d5f042f4 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -2828,14 +2828,6 @@ Dhcpv4Srv::acceptMessageType(const Pkt4Ptr& query) const {
         return (false);
     }
 
-    // If we receive a message with a non-existing type, we are logging it.
-    if (type > DHCPLEASEQUERYDONE) {
-        LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_0005)
-            .arg(query->getLabel())
-            .arg(type);
-        return (false);
-    }
-
     // Once we know that the message type is within a range of defined DHCPv4
     // messages, we do a detailed check to make sure that the received message
     // is targeted at server. Note that we could have received some Offer
@@ -2844,16 +2836,36 @@ Dhcpv4Srv::acceptMessageType(const Pkt4Ptr& query) const {
     // safe side. Also, we want to drop other messages which we don't support.
     // All these valid messages that we are not going to process are dropped
     // silently.
-    if ((type != DHCPDISCOVER) && (type != DHCPREQUEST) &&
-        (type != DHCPRELEASE) && (type != DHCPDECLINE) &&
-        (type != DHCPINFORM)) {
-        LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_0006)
-            .arg(query->getLabel())
-            .arg(type);
-        return (false);
+
+    switch(type) {
+        case DHCPDISCOVER:
+        case DHCPREQUEST:
+        case DHCPRELEASE:
+        case DHCPDECLINE:
+        case DHCPINFORM:
+            return (true);
+            break;
+
+        case DHCP_NOTYPE:
+            LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_0009)
+                      .arg(query->getLabel());
+            break;
+        default:
+            // If we receive a message with a non-existing type, we are logging it.
+            if (type > DHCPLEASEQUERYDONE) {
+                LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_0005)
+                          .arg(query->getLabel())
+                          .arg(type);
+            } else {
+                // Exists but we don't support it.
+                LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_0006)
+                      .arg(query->getLabel())
+                      .arg(type);
+            }
+            break;
     }
 
-    return (true);
+    return (false);
 }
 
 bool

diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 25b540c1ffe9d60539988c4b8fd35f45c0d68028..a2bdd6fd7fa4816fa210dfe1dda227e35c13d815 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -36,6 +36,7 @@
 #include <dhcpsrv/host_mgr.h>
 #include <gtest/gtest.h>
 #include <stats/stats_mgr.h>
+#include <util/encode/hex.h>
 #include <boost/scoped_ptr.hpp>
 
 #include <iostream>
@@ -2458,7 +2459,7 @@ TEST_F(Dhcpv4SrvTest, option43LastResort) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -2549,7 +2550,7 @@ TEST_F(Dhcpv4SrvTest, option43BadRaw) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -2713,7 +2714,7 @@ TEST_F(Dhcpv4SrvTest, option43RawGlobal) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -2808,7 +2809,7 @@ TEST_F(Dhcpv4SrvTest, option43RawClass) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -2920,7 +2921,7 @@ TEST_F(Dhcpv4SrvTest, option43Class) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -3054,7 +3055,7 @@ TEST_F(Dhcpv4SrvTest, option43ClassPriority) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -3194,7 +3195,7 @@ TEST_F(Dhcpv4SrvTest, option43Classes) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add a vendor-class-identifier (code 60)
     OptionPtr opt = offer->getOption(DHO_VENDOR_CLASS_IDENTIFIER);
     EXPECT_TRUE(opt);
@@ -3305,7 +3306,7 @@ TEST_F(Dhcpv4SrvTest, privateOption) {
 
     // Check if we get response at all
     checkResponse(offer, DHCPOFFER, 1234);
-    
+
     // Processing should add an option with code 234
     OptionPtr opt = offer->getOption(234);
     EXPECT_TRUE(opt);
@@ -3968,7 +3969,7 @@ TEST_F(Dhcpv4SrvTest, acceptMessageType) {
         DHCPLEASEUNKNOWN,
         DHCPLEASEACTIVE,
         DHCPBULKLEASEQUERY,
-        DHCPLEASEQUERYDONE
+        DHCPLEASEQUERYDONE,
     };
     size_t not_allowed_size = sizeof(not_allowed) / sizeof(not_allowed[0]);
     // Actually check that the server will drop these messages.
@@ -3977,6 +3978,54 @@ TEST_F(Dhcpv4SrvTest, acceptMessageType) {
                                                             1234))))
             << "Test failed for message type " << i;
     }
+
+    // Verify that we drop packets with no option 53
+    // Make a BOOTP packet (i.e. no option 53)
+    std::vector<uint8_t> bin;
+    const char* bootp_txt =
+        "01010601002529b629b600000000000000000000000000000ace5001944452fe711700"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "000000000000000000000000000000000000000000000000000063825363521b010400"
+        "020418020600237453fc48090b0000118b06010401020300ff00000000000000000000"
+        "0000000000000000000000000000000000000000";
+
+    isc::util::encode::decodeHex(bootp_txt, bin);
+    Pkt4Ptr pkt(new Pkt4(&bin[0], bin.size()));
+    pkt->unpack();
+    ASSERT_EQ(DHCP_NOTYPE, pkt->getType());
+    EXPECT_FALSE(srv.acceptMessageType(Pkt4Ptr(new Pkt4(&bin[0], bin.size()))));
+
+    // Verify that we drop packets with types > DHCPLEASEQUERYDONE
+    // Make Discover with type changed to 0xff
+    std::vector<uint8_t> bin2;
+    const char* invalid_msg_type =
+        "010106015d05478d000000000000000000000000000000000afee20120e52ab8151400"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000000000000000000000"
+        "0000000000000000000000000000000000000000000000000000638253633501ff3707"
+        "0102030407067d3c0a646f63736973332e303a7d7f0000118b7a010102057501010102"
+        "010303010104010105010106010107010f0801100901030a01010b01180c01010d0200"
+        "400e0200100f010110040000000211010014010015013f160101170101180104190104"
+        "1a01041b01201c01021d01081e01201f01102001102101022201012301002401002501"
+        "01260200ff2701012b59020345434d030b45434d3a45524f55544552040d3242523232"
+        "39553430303434430504312e3034060856312e33332e30330707322e332e3052320806"
+        "30303039354209094347333030304443520a074e657467656172fe01083d0fff2ab815"
+        "140003000120e52ab81514390205dc5219010420000002020620e52ab8151409090000"
+        "118b0401020300ff";
+
+    bin.clear();
+    isc::util::encode::decodeHex(invalid_msg_type, bin);
+    pkt.reset(new Pkt4(&bin[0], bin.size()));
+    pkt->unpack();
+    ASSERT_EQ(0xff, pkt->getType());
+    EXPECT_FALSE(srv.acceptMessageType(pkt));
 }
 
 // Test checks whether statistic is bumped up appropriately when Decline