From: Francis Dupont Date: Thu, 29 Apr 2021 14:07:43 +0000 (+0200) Subject: [(no branch, rebasing 1815-add-new-drop-points)] [(no branch, rebasing 1815-add-new... X-Git-Tag: Kea-1.9.8~64 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=02596244f570dd6cb20a5ce83aa173a74c0bd0c2;p=thirdparty%2Fkea.git [(no branch, rebasing 1815-add-new-drop-points)] [(no branch, rebasing 1815-add-new-drop-points)] [#1815] Checkpoint: v4 part done --- diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst index 40b97296dc..5b540154ba 100644 --- a/doc/sphinx/arm/classify.rst +++ b/doc/sphinx/arm/classify.rst @@ -85,17 +85,22 @@ The classification process is conducted in several steps: class. After a subnet is selected, the server determines whether there is a reservation for a given client. Therefore, it is not possible to use KNOWN/UNKNOWN classes to select a shared network or - a subnet, nor to make DROP class dependent of KNOWN/UNKNOWN classes. + a subnet. -9. If needed, addresses and prefixes from pools are assigned, possibly +9. When the incoming packet belongs the special class, `DROP`, it is + dropped and an informational message is logged with the packet + information. Since Kea version 1.9.8 it is allowed to make DROP + class dependent of KNOWN/UNKNOWN classes. + +10. If needed, addresses and prefixes from pools are assigned, possibly based on the class information when some pools are reserved for class members. -10. Classes marked as "required" are evaluated in the order in which +11. Classes marked as "required" are evaluated in the order in which they are listed: first the shared network, then the subnet, and finally the pools that assigned resources belong to. -11. Options are assigned, again possibly based on the class information +12. Options are assigned, again possibly based on the class information in the order that classes were associated with the incoming packet. For DHCPv4 private and code 43 options, this includes class local option definitions. diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc index 182174d9b5..ce259fc96e 100644 --- a/src/bin/dhcp4/dhcp4_messages.cc +++ b/src/bin/dhcp4/dhcp4_messages.cc @@ -106,6 +106,7 @@ extern const isc::log::MessageID DHCP4_PACKET_DROP_0009 = "DHCP4_PACKET_DROP_000 extern const isc::log::MessageID DHCP4_PACKET_DROP_0010 = "DHCP4_PACKET_DROP_0010"; extern const isc::log::MessageID DHCP4_PACKET_DROP_0011 = "DHCP4_PACKET_DROP_0011"; extern const isc::log::MessageID DHCP4_PACKET_DROP_0012 = "DHCP4_PACKET_DROP_0012"; +extern const isc::log::MessageID DHCP4_PACKET_DROP_0013 = "DHCP4_PACKET_DROP_0013"; extern const isc::log::MessageID DHCP4_PACKET_NAK_0001 = "DHCP4_PACKET_NAK_0001"; extern const isc::log::MessageID DHCP4_PACKET_NAK_0002 = "DHCP4_PACKET_NAK_0002"; extern const isc::log::MessageID DHCP4_PACKET_NAK_0003 = "DHCP4_PACKET_NAK_0003"; @@ -259,6 +260,7 @@ const char* values[] = { "DHCP4_PACKET_DROP_0010", "dropped as member of the special class 'DROP': %1", "DHCP4_PACKET_DROP_0011", "dropped as sent by the same client than a packet being processed by another thread: dropped %1 by thread %2 as duplicate of %3 processed by %4", "DHCP4_PACKET_DROP_0012", "dropped as sent by the same client than a packet being processed by another thread: dropped %1 by thread %2 as duplicate of %3 processed by %4", + "DHCP4_PACKET_DROP_0013", "dropped as member of the special class 'DROP' after host reservation lookup: %1", "DHCP4_PACKET_NAK_0001", "%1: failed to select a subnet for incoming packet, src %2, type %3", "DHCP4_PACKET_NAK_0002", "%1: invalid address %2 requested by INIT-REBOOT", "DHCP4_PACKET_NAK_0003", "%1: failed to advertise a lease, client sent ciaddr %2, requested-ip-address %3", diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h index a5ea410f1c..39ba709146 100644 --- a/src/bin/dhcp4/dhcp4_messages.h +++ b/src/bin/dhcp4/dhcp4_messages.h @@ -107,6 +107,7 @@ extern const isc::log::MessageID DHCP4_PACKET_DROP_0009; extern const isc::log::MessageID DHCP4_PACKET_DROP_0010; extern const isc::log::MessageID DHCP4_PACKET_DROP_0011; extern const isc::log::MessageID DHCP4_PACKET_DROP_0012; +extern const isc::log::MessageID DHCP4_PACKET_DROP_0013; extern const isc::log::MessageID DHCP4_PACKET_NAK_0001; extern const isc::log::MessageID DHCP4_PACKET_NAK_0002; extern const isc::log::MessageID DHCP4_PACKET_NAK_0003; diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 17bb42fa7b..0db82906ae 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -589,6 +589,11 @@ processing is finished. Packet details and thread identifiers are included for both packets in this warning message. +% DHCP4_PACKET_DROP_0013 dropped as member of the special class 'DROP' after host reservation lookup: %1 +This debug message is emitted when an incoming packet was classified +after host reservation lookup into the special class 'DROP' and dropped. +The packet details are displayed. + % 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 cfdc9c7fa3..b8b9f6bcd2 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -146,7 +146,8 @@ namespace dhcp { Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, const Pkt4Ptr& query, - const Subnet4Ptr& subnet) + const Subnet4Ptr& subnet, + bool& drop) : alloc_engine_(alloc_engine), query_(query), resp_(), context_(new AllocEngine::ClientContext4()) { @@ -254,6 +255,17 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, .arg(query_->getLabel()) .arg(classes.toText()); } + + // Check the DROP special class. + if (query_->inClass("DROP")) { + LOG_DEBUG(packet4_logger, DBGLVL_TRACE_BASIC, DHCP4_PACKET_DROP_0013) + .arg(query_->toText()); + isc::stats::StatsMgr::instance().addValue("pkt4-receive-drop", + static_cast(1)); + drop = true; + } + + } void @@ -2978,13 +2990,20 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) { sanityCheck(discover, FORBIDDEN); bool drop = false; - Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover, drop)); + Subnet4Ptr subnet = selectSubnet(discover, drop); // Stop here if selectSubnet decided to drop the packet if (drop) { return (Pkt4Ptr()); } + Dhcpv4Exchange ex(alloc_engine_, discover, subnet, drop); + + // Stop here if Dhcpv4Exchange constructir decided to drop the packet + if (drop) { + return (Pkt4Ptr()); + } + if (MultiThreadingMgr::instance().getMode()) { // The lease reclamation cannot run at the same time. ReadLockGuard share(alloc_engine_->getReadWriteMutex()); @@ -3042,13 +3061,20 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request, AllocEngine::ClientContext4Ptr& cont sanityCheck(request, OPTIONAL); bool drop = false; - Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request, drop)); + Subnet4Ptr subnet = selectSubnet(request, drop); // Stop here if selectSubnet decided to drop the packet if (drop) { return (Pkt4Ptr()); } + Dhcpv4Exchange ex(alloc_engine_, request, subnet, drop); + + // Stop here if Dhcpv4Exchange constructir decided to drop the packet + if (drop) { + return (Pkt4Ptr()); + } + // Note that we treat REQUEST message uniformly, regardless if this is a // first request (requesting for new address), renewing existing address // or even rebinding. @@ -3390,13 +3416,20 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) { sanityCheck(inform, OPTIONAL); bool drop = false; - Dhcpv4Exchange ex(alloc_engine_, inform, selectSubnet(inform, drop)); + Subnet4Ptr subnet = selectSubnet(inform, drop); // Stop here if selectSubnet decided to drop the packet if (drop) { return (Pkt4Ptr()); } + Dhcpv4Exchange ex(alloc_engine_, inform, subnet, drop); + + // Stop here if Dhcpv4Exchange constructir decided to drop the packet + if (drop) { + return (Pkt4Ptr()); + } + Pkt4Ptr ack = ex.getResponse(); // If this is global reservation or the subnet doesn't belong to a shared diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index dd26c5a78e..c8dafa1b9a 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -73,8 +73,9 @@ public: /// used by the server. /// @param query Pointer to the client message. /// @param subnet Pointer to the subnet to which the client belongs. + /// @param drop if it is true the packet will be dropped. Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, const Pkt4Ptr& query, - const Subnet4Ptr& subnet); + const Subnet4Ptr& subnet, bool& drop); /// @brief Initializes the instance of the response message. /// diff --git a/src/bin/dhcp4/tests/classify_unittest.cc b/src/bin/dhcp4/tests/classify_unittest.cc index a7269e7c94..b7f4ab8e76 100644 --- a/src/bin/dhcp4/tests/classify_unittest.cc +++ b/src/bin/dhcp4/tests/classify_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2021 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 @@ -82,6 +82,17 @@ namespace { /// - 1 pool: 10.0.0.10-10.0.0.100 /// - the following class defined: option[93].hex == 0x0009, DROP /// +/// - Configuration 6: +/// - Used for the DROP class and reservations. +/// - 1 subnet: 10.0.0.0/24 +/// - 1 pool: 10.0.0.10-10.0.0.100 +/// - 1 reservation for HW address 'aa:bb:cc:dd:ee:ff' +/// - the following class defined: not member('KNOWN'), DROP +/// (the not member also verifies that the DROP class is set only +/// after the host reservation lookup) +/// @note the reservation includes a hostname because raw reservations are +/// not yet allowed. +/// const char* CONFIGS[] = { // Configuration 0 "{ \"interfaces-config\": {" @@ -297,6 +308,26 @@ const char* CONFIGS[] = { " \"id\": 1," " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]" " } ]" + "}", + + // Configuration 6 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"client-classes\": [" + "{" + " \"name\": \"DROP\"," + " \"test\": \"member('UNKNOWN')\"" + "}]," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"reservations\": [ {" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," + " \"hostname\": \"allowed\" } ]" + " } ]" "}" }; @@ -1164,4 +1195,40 @@ TEST_F(ClassifyTest, dropClass) { EXPECT_EQ(1, drop_stat->getInteger().first); } +// This test checks the handling for the DROP special class at the host +// reservation classification point. +TEST_F(ClassifyTest, dropClassHR) { + Dhcp4Client client(Dhcp4Client::SELECTING); + + // Configure DHCP server. + configure(CONFIGS[6], *client.getServer()); + + // Set the HW address to the reservation. + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + + // Send the discover. + client.doDiscover(); + + // Reservation match: no drop. + EXPECT_TRUE(client.getContext().response_); + + // Retry with another HW address. + Dhcp4Client client2(Dhcp4Client::SELECTING); + client2.setHWAddress("aa:bb:cc:dd:ee:fe"); + + // Send the discover. + client2.doDiscover(); + + // No reservation, dropped. + EXPECT_FALSE(client2.getContext().response_); + + // There should also be pkt4-receive-drop stat bumped up. + stats::StatsMgr& mgr = stats::StatsMgr::instance(); + stats::ObservationPtr drop_stat = mgr.getObservation("pkt4-receive-drop"); + + // This statistic must be present and must be set to 1. + ASSERT_TRUE(drop_stat); + EXPECT_EQ(1, drop_stat->getInteger().first); +} + } // end of anonymous namespace diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index 7197187bef..3a4353c6da 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc @@ -730,8 +730,9 @@ Dhcpv4SrvTest::configureWithStatus(const std::string& config, NakedDhcpv4Srv& sr Dhcpv4Exchange Dhcpv4SrvTest::createExchange(const Pkt4Ptr& query) { bool drop = false; - Dhcpv4Exchange ex(srv_.alloc_engine_, query, - srv_.selectSubnet(query, drop)); + Subnet4Ptr subnet = srv_.selectSubnet(query, drop); + EXPECT_FALSE(drop); + Dhcpv4Exchange ex(srv_.alloc_engine_, query, subnet, drop); EXPECT_FALSE(drop); return (ex); } diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc index ddc9959790..f0a25e0fd5 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc @@ -224,10 +224,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, isc_throw(DhcpConfigError, "special class '" << name << "' only-if-required flag must be false"); } - if (depend_on_known) { - isc_throw(DhcpConfigError, "special class '" << name - << "' must not depend on 'KNOWN'/'UNKNOWN' classes"); - } + // depend_on_known is now allowed } // Add the client class definition diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc index 2f99ec3790..495a560a8f 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc @@ -1258,8 +1258,7 @@ TEST_F(ClientClassDefListParserTest, builtinCheckError) { EXPECT_THROW(parseClientClassDefList(cfg_text, AF_INET6), DhcpConfigError); } -// Verifies that the special DROP class can't be required or -// dependent on KNOWN/UNKNOWN +// Verifies that the special DROP class can't be required. TEST_F(ClientClassDefListParserTest, dropCheckError) { std::string cfg_text = "[ \n" @@ -1281,6 +1280,7 @@ TEST_F(ClientClassDefListParserTest, dropCheckError) { EXPECT_THROW(parseClientClassDefList(cfg_text, AF_INET), DhcpConfigError); + // This constraint was relaxed in #1815. cfg_text = "[ \n" " { \n" @@ -1289,7 +1289,7 @@ TEST_F(ClientClassDefListParserTest, dropCheckError) { " } \n" "] \n"; - EXPECT_THROW(parseClientClassDefList(cfg_text, AF_INET6), DhcpConfigError); + EXPECT_NO_THROW(parseClientClassDefList(cfg_text, AF_INET6)); } // Verify the ability to configure lease lifetime triplet.