[(no branch, rebasing 1815-add-new-drop-points)] [(no branch, rebasing 1815-add-new...

diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst
index 40b97296dc99d03855651b969269562266b353c6..5b540154ba3088b48d0978287a30e0d977e0ec08 100644 (file)
--- 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 182174d9b57db3fadceb874d287b594897c44da6..ce259fc96e988cf4bbc783a28b35589330fe14b4 100644 (file)
--- 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 a5ea410f1c35908f7d603521eb2cd6d87981eb8f..39ba709146b3e8ea1e25cbc0f0308a7bfb6199a4 100644 (file)
--- 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 17bb42fa7b1caaf3aa98a702b2f7d0e850e4b4ab..0db82906aeaa150130d1a9d1ccd4165af5baf74c 100644 (file)
--- 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 cfdc9c7fa3321145f9cb21a623a06f7aee6dd1b0..b8b9f6bcd2800fa7b2417551e3eadfd00122107d 100644 (file)
--- 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<int64_t>(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 dd26c5a78e631adc4313b24deee6093591335712..c8dafa1b9aa2e70ac672e1e456d8612f0e360bef 100644 (file)
--- 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 a7269e7c94ecc1084e780054ba0e63d317738511..b7f4ab8e76f1badf651d506b5c37e6238c9ef2e9 100644 (file)
--- 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 7197187bef07d27d39bd6984fa59b5ea2507cd9c..3a4353c6dac152252baf96c8b2e12dad595db24d 100644 (file)
--- 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 ddc99597901be42b03ef43f2e070fd992d13d33c..f0a25e0fd54356fe039c9bda0d438c2c9126a093 100644 (file)
--- 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 2f99ec3790cfc9a363affcad91e3334c09d23ffd..495a560a8f4356dc3112d62a5c95f4623448e301 100644 (file)
--- 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.