diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index 3d1d2ed8a2..9358d26ac2 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -5625,7 +5625,7 @@ Supported DHCP Standards The following standards are currently supported: - *BOOTP Vendor Information Extensions*, `RFC - 1497 `__: This requires an open + 1497 `__: This requires the open source BOOTP hook to be loaded. - *Dynamic Host Configuration Protocol*, `RFC diff --git a/src/hooks/dhcp/bootp/bootp.dox b/src/hooks/dhcp/bootp/bootp.dox index 0df6f63e08..3d5d1dc828 100644 --- a/src/hooks/dhcp/bootp/bootp.dox +++ b/src/hooks/dhcp/bootp/bootp.dox @@ -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 one such function: @ref pkt4_receive -located in bootp_callouts.cc. +point names. This library has two such functions: @ref buffer4_receive +and @ref pkt4_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 5afbec6658..dcc4465a99 100644 --- a/src/hooks/dhcp/bootp/bootp_callouts.cc +++ b/src/hooks/dhcp/bootp/bootp_callouts.cc @@ -20,11 +20,48 @@ using namespace isc::log; // issues related to namespaces. extern "C" { -/// @brief This callout is called at the "pkt4_receive" hook. +/// @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 and a DHCPREQUEST dhcp-message-type. +/// Remaining packets should be BOOTP requests so add the BOOTP client class, +/// +/// @param handle CalloutHandle. +/// +/// @return 0 upon success, non-zero otherwise. +int buffer4_receive(CalloutHandle& handle) { + // Get the received unpacked message. + Pkt4Ptr query; + handle.getArgument("query4", query); + + try { + // Take a copy and unpack it. + Pkt4Ptr copy(new Pkt4(&query->data_[0], query->data_.size())); + copy->unpack(); + + if (copy->getType() != DHCP_NOTYPE) { + // DHCP query. + return (0); + } + 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. + } + + 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. /// @@ -35,25 +72,14 @@ int pkt4_receive(CalloutHandle& handle) { handle.getArgument("query4", query); try { - if (query->getType() != DHCP_NOTYPE) { - // DHCP query. + if (!query->inClass("BOOTP")) { return (0); } - if (query->getOp() == BOOTREPLY) { - // BOOTP response. - return (0); - } - // BOOTP query. - query->addClass("BOOTP"); query->setType(DHCPREQUEST); - LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, BOOTP_PROCESSED) + LOG_DEBUG(bootp_logger, DBGLVL_TRACE_BASIC, BOOTP_ADDED_MSGTYPE) .arg(query->getLabel()); - } catch (const std::exception& ex) { - // Got an error (should not happen). The query shall very likely - // be dropped later. - LOG_ERROR(bootp_logger, BOOTP_PROCESS_ERROR) - .arg(query->getLabel()) - .arg(ex.what()); + } catch (const std::exception&) { + // Got an error (should not happen). } return (0); diff --git a/src/hooks/dhcp/bootp/bootp_messages.cc b/src/hooks/dhcp/bootp/bootp_messages.cc index 3833fcf408..6724fa7cb2 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.cc +++ b/src/hooks/dhcp/bootp/bootp_messages.cc @@ -1,20 +1,20 @@ -// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Mon Nov 18 2019 09:30 +// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Fri Nov 22 2019 01:05 #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_LOAD = "BOOTP_LOAD"; -extern const isc::log::MessageID BOOTP_PROCESSED = "BOOTP_PROCESSED"; -extern const isc::log::MessageID BOOTP_PROCESS_ERROR = "BOOTP_PROCESS_ERROR"; 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_LOAD", "Bootp hooks library has been loaded", - "BOOTP_PROCESSED", "processed BOOTP query: %1", - "BOOTP_PROCESS_ERROR", "%1: failed to process packet: %2", "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 a60630d0fd..8587dee6c9 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.h +++ b/src/hooks/dhcp/bootp/bootp_messages.h @@ -1,13 +1,13 @@ -// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Mon Nov 18 2019 09:30 +// File created from ../../../../src/hooks/dhcp/bootp/bootp_messages.mes on Fri Nov 22 2019 01:05 #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_LOAD; -extern const isc::log::MessageID BOOTP_PROCESSED; -extern const isc::log::MessageID BOOTP_PROCESS_ERROR; 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 db3d5ccbdd..a78d301ea6 100644 --- a/src/hooks/dhcp/bootp/bootp_messages.mes +++ b/src/hooks/dhcp/bootp/bootp_messages.mes @@ -1,16 +1,17 @@ # Copyright (C) 2019 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_LOAD Bootp hooks library has been loaded This info message indicates that the Bootp hooks library has been loaded. -% BOOTP_PROCESSED processed BOOTP query: %1 -This debug message is printed when a BOOTP query was processed. -The query client and transaction identification are displayed. - -% BOOTP_PROCESS_ERROR %1: failed to process packet: %2 -This error message indicates an error during processing of a packet. -The first argument contains the client and transaction identification -information. The second argument includes the details of the error. - % 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/bootp_unittests.cc b/src/hooks/dhcp/bootp/tests/bootp_unittests.cc index 7d9eb7c851..c84bb7259c 100644 --- a/src/hooks/dhcp/bootp/tests/bootp_unittests.cc +++ b/src/hooks/dhcp/bootp/tests/bootp_unittests.cc @@ -20,8 +20,10 @@ using namespace isc; using namespace isc::bootp; using namespace isc::dhcp; using namespace isc::hooks; +using namespace isc::util; extern "C" { +extern int buffer4_receive(CalloutHandle& handle); extern int pkt4_receive(CalloutHandle& handle); } @@ -46,31 +48,46 @@ public: return(co_manager_); } - /// @brief Tests pkt4_receive callout. + /// @brief Tests buffer4_receive and pkt4_receive callout. /// /// @param pkt The packet to submit. /// @param processed True if the packet must be processed, false otherwise. - void pkt4_receiveCall(Pkt4Ptr& pkt, bool processed) { + void calloutsCall(Pkt4Ptr& pkt, bool processed) { // Get callout handle. CalloutHandle handle(getCalloutManager()); + // Fill data so it becomes possible to unpack a copy of it. + ASSERT_NO_THROW(pkt->pack()); + const OutputBuffer& buffer = pkt->getBuffer(); + pkt->data_.resize(buffer.getLength()); + memmove(&pkt->data_[0], buffer.getData(), pkt->data_.size()); + // Set query. handle.setArgument("query4", pkt); // Get type. uint8_t type = pkt->getType(); - // Execute pkt4_receive callout. + // Execute buffer4_receive callout. int ret; - ASSERT_NO_THROW(ret = pkt4_receive(handle)); + ASSERT_NO_THROW(ret = buffer4_receive(handle)); EXPECT_EQ(0, ret); // Verify processing. if (processed) { EXPECT_TRUE(pkt->inClass("BOOTP")); - EXPECT_EQ(DHCPREQUEST, pkt->getType()); } 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_EQ(type, pkt->getType()); } } @@ -82,19 +99,19 @@ public: // Verifies that DHCPDISCOVER goes through unmodified. TEST_F(BootpTest, dhcpDiscover) { Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 12345)); - pkt4_receiveCall(pkt, false); + calloutsCall(pkt, false); } // Verifies that DHCPREQUEST goes through unmodified. TEST_F(BootpTest, dhcpRequest) { Pkt4Ptr pkt(new Pkt4(DHCPREQUEST, 12345)); - pkt4_receiveCall(pkt, false); + calloutsCall(pkt, false); } // Verifies that DHCPOFFER goes through unmodified. TEST_F(BootpTest, dhcpOffer) { Pkt4Ptr pkt(new Pkt4(DHCPOFFER, 12345)); - pkt4_receiveCall(pkt, false); + calloutsCall(pkt, false); } // Verifies that BOOTREPLY goes through unmodified. @@ -104,7 +121,7 @@ TEST_F(BootpTest, bootReply) { pkt->setType(DHCP_NOTYPE); pkt->delOption(DHO_DHCP_MESSAGE_TYPE); ASSERT_EQ(BOOTREPLY, pkt->getOp()); - pkt4_receiveCall(pkt, false); + calloutsCall(pkt, false); } // Verifies that BOOTREQUEST is recognized and processed. @@ -114,7 +131,7 @@ TEST_F(BootpTest, bootRequest) { pkt->setType(DHCP_NOTYPE); pkt->delOption(DHO_DHCP_MESSAGE_TYPE); ASSERT_EQ(BOOTREQUEST, pkt->getOp()); - pkt4_receiveCall(pkt, true); + calloutsCall(pkt, true); } } // end of anonymous namespace diff --git a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc index 36f2ad1f47..9285181cec 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc @@ -210,8 +210,9 @@ TEST_F(HAImplTest, buffer4Receive) { // The client class should be assigned to the message to indicate that the // server1 should process this message. - ASSERT_EQ(1, query4->getClasses().size()); - EXPECT_EQ("HA_server1", *(query4->getClasses().cbegin())); + ASSERT_EQ(2, query4->getClasses().size()); + EXPECT_TRUE(query4->inClass("ALL")); + EXPECT_TRUE(query4->inClass("HA_server1")); // Check that the message has been parsed. The DHCP message type should // be set in this case. @@ -298,8 +299,9 @@ TEST_F(HAImplTest, buffer6Receive) { // The client class should be assigned to the message to indicate that the // server1 should process this message. - ASSERT_EQ(1, query6->getClasses().size()); - EXPECT_EQ("HA_server1", *(query6->getClasses().cbegin())); + ASSERT_EQ(2, query6->getClasses().size()); + EXPECT_TRUE(query6->inClass("ALL")); + EXPECT_TRUE(query6->inClass("HA_server1")); // Check that the message has been parsed. The DHCP message type should // be set in this case. diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index cc7c2739cc..907e38d722 100644 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2019 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 @@ -98,6 +98,10 @@ Pkt::inClass(const std::string& client_class) { void Pkt::addClass(const std::string& client_class, bool required) { + // Always have ALL first. + if (classes_.empty()) { + classes_.insert("ALL"); + } ClientClasses& classes = !required ? classes_ : required_classes_; if (!classes.contains(client_class)) { classes.insert(client_class);