From: Thomas Markwalder Date: Fri, 23 Feb 2018 14:15:33 +0000 (-0500) Subject: [5553] Added specific log for possible BOOTP packets X-Git-Tag: ha_checkpoints12~12^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2Ftrac5553;p=thirdparty%2Fkea.git [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 fef6ef62dc..e7f222ff81 100644 --- 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 03b3873d73..ead5560ccb 100644 --- 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 25b540c1ff..a2bdd6fd7f 100644 --- 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 #include #include +#include #include #include @@ -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 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 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