diff --git a/src/hooks/dhcp/bootp/Makefile.am b/src/hooks/dhcp/bootp/Makefile.am index 6a3be0bb5a..de796531e4 100644 --- a/src/hooks/dhcp/bootp/Makefile.am +++ b/src/hooks/dhcp/bootp/Makefile.am @@ -30,6 +30,7 @@ libdhcp_bootp_la_SOURCES = libdhcp_bootp_la_LDFLAGS = $(AM_LDFLAGS) libdhcp_bootp_la_LDFLAGS += -avoid-version -export-dynamic -module libdhcp_bootp_la_LIBADD = libbootp.la +libdhcp_bootp_la_LIBADD += $(top_builddir)/src/lib/stats/libkea-stats.la libdhcp_bootp_la_LIBADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la libdhcp_bootp_la_LIBADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la libdhcp_bootp_la_LIBADD += $(top_builddir)/src/lib/database/libkea-database.la diff --git a/src/hooks/dhcp/bootp/bootp.dox b/src/hooks/dhcp/bootp/bootp.dox index 3d5d1dc828..c110036bf1 100644 --- a/src/hooks/dhcp/bootp/bootp.dox +++ b/src/hooks/dhcp/bootp/bootp.dox @@ -1,4 +1,4 @@ -// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2010 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 @@ -53,8 +53,8 @@ bootp_callouts.cc. It checks if the necessary parameter is passed and decodes the option configurations. @ref unload() free the configuration. Kea engine checks if the library has functions that match known hook -point names. This library has two such functions: @ref buffer4_receive -and @ref pkt4_receive located in bootp_callouts.cc. +point names. This library has one such function: @ref buffer4_receive +located in bootp_callouts.cc. If the receive query has to dhcp-message-type option then it is a BOOTP one: the BOOTP client class and a DHCPREQUEST dhcp-message-type option diff --git a/src/hooks/dhcp/bootp/bootp_callouts.cc b/src/hooks/dhcp/bootp/bootp_callouts.cc index 3d2dcb87b7..47026fecc9 100644 --- a/src/hooks/dhcp/bootp/bootp_callouts.cc +++ b/src/hooks/dhcp/bootp/bootp_callouts.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2020 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the End User License // Agreement. See COPYING file in the premium/ directory. @@ -8,12 +8,14 @@ #include #include #include +#include using namespace isc; using namespace isc::bootp; using namespace isc::dhcp; using namespace isc::hooks; using namespace isc::log; +using namespace isc::stats; // Functions accessed by the hooks framework use C linkage to avoid the name // mangling that accompanies use of the C++ compiler as well as to avoid @@ -23,7 +25,8 @@ extern "C" { /// @brief This callout is called at the "buffer4_receive" hook. /// /// Ignore DHCP and BOOTREPLY messages. -/// Remaining packets should be BOOTP requests so add the BOOTP client class, +/// Remaining packets should be BOOTP requests so add the BOOTP client class +/// and set the message type to DHCPREQUEST. /// /// @param handle CalloutHandle. /// @@ -34,53 +37,47 @@ int buffer4_receive(CalloutHandle& handle) { handle.getArgument("query4", query); try { - // Take a copy and unpack it. - Pkt4Ptr copy(new Pkt4(&query->data_[0], query->data_.size())); - copy->unpack(); + // Unpack it (TODO check if it was already unpacked). + query->unpack(); - if (copy->getType() != DHCP_NOTYPE) { - // DHCP query. - return (0); + // Not DHCP query nor BOOTP response? + if ((query->getType() == DHCP_NOTYPE) && + (query->getOp() == BOOTREQUEST)) { + + query->addClass("BOOTP"); + query->setType(DHCPREQUEST); + + LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, BOOTP_BOOTP_QUERY) + .arg(query->getLabel()); } - if (copy->getOp() == BOOTREPLY) { - // BOOTP response. - return (0); - } - // BOOTP query. - query->addClass("BOOTP"); - LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, BOOTP_ADDED_CLASS) - .arg(query->getLabel()); - } catch (const std::exception&) { - // Got an error. The query shall very likely be dropped later. - // Note this covers malformed DHCP packets where the message - // type option can't be unpacked, and RFC 951 BOOTP. + } catch (const SkipRemainingOptionsError& ex) { + // An option failed to unpack but we are to attempt to process it + // anyway. Log it and let's hope for the best. + LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, + BOOTP_PACKET_OPTIONS_SKIPPED) + .arg(ex.what()); + } catch (const std::exception& ex) { + // Failed to parse the packet. + LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, + BOOTP_PACKET_UNPACK_FAILED) + .arg(query->getRemoteAddr().toText()) + .arg(query->getLocalAddr().toText()) + .arg(query->getIface()) + .arg(ex.what()); + + // Increase the statistics of parse failures and dropped packets. + StatsMgr::instance().addValue("pkt4-parse-failed", + static_cast(1)); + StatsMgr::instance().addValue("pkt4-receive-drop", + static_cast(1)); + + handle.setStatus(CalloutHandle::NEXT_STEP_DROP); + + return (0); } - return (0); -} - -/// @brief This callout is called at the "pkt4_receive" hook. -/// -/// If in the BOOTP class add a DHCPREQUEST dhcp-message-type. -/// -/// @param handle CalloutHandle. -/// -/// @return 0 upon success, non-zero otherwise. -int pkt4_receive(CalloutHandle& handle) { - // Get the received unpacked message. - Pkt4Ptr query; - handle.getArgument("query4", query); - - try { - if (!query->inClass("BOOTP")) { - return (0); - } - query->setType(DHCPREQUEST); - LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, BOOTP_ADDED_MSGTYPE) - .arg(query->getLabel()); - } catch (const std::exception&) { - // Got an error (should not happen). - } + // Avoid to unpack it a second time! + handle.setStatus(CalloutHandle::NEXT_STEP_SKIP); return (0); } diff --git a/src/hooks/dhcp/bootp/bootp_messages.cc b/src/hooks/dhcp/bootp/bootp_messages.cc index 6724fa7cb2..c097ffd0ba 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.cc +++ b/src/hooks/dhcp/bootp/bootp_messages.cc @@ -1,20 +1,22 @@ -// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Fri Nov 22 2019 01:05 +// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Mon Jan 20 2020 17:15 #include #include #include -extern const isc::log::MessageID BOOTP_ADDED_CLASS = "BOOTP_ADDED_CLASS"; -extern const isc::log::MessageID BOOTP_ADDED_MSGTYPE = "BOOTP_ADDED_MSGTYPE"; +extern const isc::log::MessageID BOOTP_BOOTP_QUERY = "BOOTP_BOOTP_QUERY"; extern const isc::log::MessageID BOOTP_LOAD = "BOOTP_LOAD"; +extern const isc::log::MessageID BOOTP_PACKET_OPTIONS_SKIPPED = "BOOTP_PACKET_OPTIONS_SKIPPED"; +extern const isc::log::MessageID BOOTP_PACKET_UNPACK_FAILED = "BOOTP_PACKET_UNPACK_FAILED"; extern const isc::log::MessageID BOOTP_UNLOAD = "BOOTP_UNLOAD"; namespace { const char* values[] = { - "BOOTP_ADDED_CLASS", "added BOOTP class to a BOOTP query: %1", - "BOOTP_ADDED_MSGTYPE", "added DHCPREQUEST message type to a BOOTP query: %1", + "BOOTP_BOOTP_QUERY", "recognized a BOOTP query: %1", "BOOTP_LOAD", "Bootp hooks library has been loaded", + "BOOTP_PACKET_OPTIONS_SKIPPED", "an error upacking an option, caused subsequent options to be skipped: %1", + "BOOTP_PACKET_UNPACK_FAILED", "failed to parse query from %1 to %2, received over interface %3, reason: %4", "BOOTP_UNLOAD", "Bootp hooks library has been unloaded", NULL }; diff --git a/src/hooks/dhcp/bootp/bootp_messages.h b/src/hooks/dhcp/bootp/bootp_messages.h index 8587dee6c9..29bb524a94 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.h +++ b/src/hooks/dhcp/bootp/bootp_messages.h @@ -1,13 +1,14 @@ -// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Fri Nov 22 2019 01:05 +// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Mon Jan 20 2020 17:15 #ifndef BOOTP_MESSAGES_H #define BOOTP_MESSAGES_H #include -extern const isc::log::MessageID BOOTP_ADDED_CLASS; -extern const isc::log::MessageID BOOTP_ADDED_MSGTYPE; +extern const isc::log::MessageID BOOTP_BOOTP_QUERY; extern const isc::log::MessageID BOOTP_LOAD; +extern const isc::log::MessageID BOOTP_PACKET_OPTIONS_SKIPPED; +extern const isc::log::MessageID BOOTP_PACKET_UNPACK_FAILED; extern const isc::log::MessageID BOOTP_UNLOAD; #endif // BOOTP_MESSAGES_H diff --git a/src/hooks/dhcp/bootp/bootp_messages.mes b/src/hooks/dhcp/bootp/bootp_messages.mes index a78d301ea6..1f6d147bb8 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.mes +++ b/src/hooks/dhcp/bootp/bootp_messages.mes @@ -1,17 +1,25 @@ -# Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2019-2020 Internet Systems Consortium, Inc. ("ISC") -% BOOTP_ADDED_CLASS added BOOTP class to a BOOTP query: %1 -This debug message is printed when the BOOTP client class was added to -a BOOTP query. The query client and transaction identification are -displayed. - -% BOOTP_ADDED_MSGTYPE added DHCPREQUEST message type to a BOOTP query: %1 -This debug message is printed when the DHCPREQUEST message type was -added to a BOOTP query. The query client and transaction -identification are displayed. +% BOOTP_BOOTP_QUERY recognized a BOOTP query: %1 +This debug message is printed when the BOOTP query was recognized. The +BOOTP client class was added and the message type set to DHCPREQUEST. +The query client and transaction identification are displayed. % BOOTP_LOAD Bootp hooks library has been loaded This info message indicates that the Bootp hooks library has been loaded. +% BOOTP_PACKET_OPTIONS_SKIPPED an error upacking an option, caused subsequent options to be skipped: %1 +A debug message issued when an option failed to unpack correctly, making it +impossible to unpack the remaining options in the DHCPv4 query. The server +will still attempt to service the packet. The sole argument provides a +reason for unpacking error. + +% BOOTP_PACKET_UNPACK_FAILED failed to parse query from %1 to %2, received over interface %3, reason: %4 +This debug message is issued when received DHCPv4 query is malformed and +can't be parsed by the buffer4_receive callout. The query will be +dropped by the server. The first three arguments specify source IP address, +destination IP address and the interface. The last argument provides a +reason for failure. + % BOOTP_UNLOAD Bootp hooks library has been unloaded This info message indicates that the Bootp hooks library has been unloaded. diff --git a/src/hooks/dhcp/bootp/tests/Makefile.am b/src/hooks/dhcp/bootp/tests/Makefile.am index 56ef9cba99..1868be78b2 100644 --- a/src/hooks/dhcp/bootp/tests/Makefile.am +++ b/src/hooks/dhcp/bootp/tests/Makefile.am @@ -35,6 +35,7 @@ bootp_unittests_LDFLAGS = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) $(GTEST_LDFLAGS) bootp_unittests_CXXFLAGS = $(AM_CXXFLAGS) bootp_unittests_LDADD = $(top_builddir)/src/hooks/dhcp/bootp/libbootp.la +bootp_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la bootp_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la bootp_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la bootp_unittests_LDADD += $(top_builddir)/src/lib/database/libkea-database.la diff --git a/src/hooks/dhcp/bootp/tests/bootp_unittests.cc b/src/hooks/dhcp/bootp/tests/bootp_unittests.cc index c84bb7259c..7f5657ef6c 100644 --- a/src/hooks/dhcp/bootp/tests/bootp_unittests.cc +++ b/src/hooks/dhcp/bootp/tests/bootp_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2020 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 @@ -24,7 +24,6 @@ using namespace isc::util; extern "C" { extern int buffer4_receive(CalloutHandle& handle); -extern int pkt4_receive(CalloutHandle& handle); } namespace { @@ -48,7 +47,7 @@ public: return(co_manager_); } - /// @brief Tests buffer4_receive and pkt4_receive callout. + /// @brief Tests buffer4_receive callout. /// /// @param pkt The packet to submit. /// @param processed True if the packet must be processed, false otherwise. @@ -56,38 +55,32 @@ public: // Get callout handle. CalloutHandle handle(getCalloutManager()); - // Fill data so it becomes possible to unpack a copy of it. + // Get type. + uint8_t type = pkt->getType(); + + // Get data so it becomes possible to reset it to unpacked state. ASSERT_NO_THROW(pkt->pack()); const OutputBuffer& buffer = pkt->getBuffer(); - pkt->data_.resize(buffer.getLength()); - memmove(&pkt->data_[0], buffer.getData(), pkt->data_.size()); + pkt.reset(new Pkt4(reinterpret_cast(buffer.getData()), + buffer.getLength())); // Set query. handle.setArgument("query4", pkt); - // Get type. - uint8_t type = pkt->getType(); - // Execute buffer4_receive callout. int ret; ASSERT_NO_THROW(ret = buffer4_receive(handle)); EXPECT_EQ(0, ret); + // Verify status (SKIP means unpacked). + EXPECT_EQ(CalloutHandle::NEXT_STEP_SKIP, handle.getStatus()); + // Verify processing. if (processed) { EXPECT_TRUE(pkt->inClass("BOOTP")); - } else { - EXPECT_FALSE(pkt->inClass("BOOTP")); - } - - // Execute pkt4_receive callout. - ASSERT_NO_THROW(ret = pkt4_receive(handle)); - EXPECT_EQ(0, ret); - - // Verify processing. - if (processed) { EXPECT_EQ(DHCPREQUEST, pkt->getType()); } else { + EXPECT_FALSE(pkt->inClass("BOOTP")); EXPECT_EQ(type, pkt->getType()); } }