[#3140] [4133] Added lease query

diff --git a/src/hooks/dhcp/lease_query/lease_query_callouts.cc b/src/hooks/dhcp/lease_query/lease_query_callouts.cc
index d3361161515be5923524cb0529efd7d53b5128a0..d6d54cb6775ff67508df5d38034b2489a71e2009 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_callouts.cc
+++ b/src/hooks/dhcp/lease_query/lease_query_callouts.cc
@@ -139,8 +139,10 @@ int buffer4_receive(CalloutHandle& handle) {
               .arg(LeaseQueryImpl4::leaseQueryLabel(query));
     StatsMgr::instance().addValue("pkt4-lease-query-received", static_cast<int64_t>(1));
 
+    bool invalid = false;
+    bool sending = false;
     try {
-        LeaseQueryImplFactory::getImpl().processQuery(query);
+        LeaseQueryImplFactory::getImpl().processQuery(query, invalid, sending);
     } catch (const std::exception& ex) {
         // Failed to parse the packet.
         LOG_DEBUG(lease_query_logger, DBGLVL_TRACE_BASIC,
@@ -148,9 +150,16 @@ int buffer4_receive(CalloutHandle& handle) {
                   .arg(LeaseQueryImpl4::leaseQueryLabel(query))
                   .arg(ex.what());
 
+        handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+        if (sending) {
+            return (0);
+        }
+        if (!invalid) {
+            StatsMgr::instance().addValue("pkt4-processing-failed",
+                                          static_cast<int64_t>(1));
+        }
         StatsMgr::instance().addValue("pkt4-receive-drop",
                                       static_cast<int64_t>(1));
-        handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
         return (0);
     }
 
@@ -222,8 +231,10 @@ int buffer6_receive(CalloutHandle& handle) {
               .arg(LeaseQueryImpl6::leaseQueryLabel(query));
     StatsMgr::instance().addValue("pkt6-lease-query-received", static_cast<int64_t>(1));
 
+    bool invalid = false;
+    bool sending = false;
     try {
-        LeaseQueryImplFactory::getImpl().processQuery(query);
+        LeaseQueryImplFactory::getImpl().processQuery(query, invalid, sending);
     } catch (const std::exception& ex) {
         // Log that we failed to process the packet.
         LOG_DEBUG(lease_query_logger, DBGLVL_TRACE_BASIC,
@@ -231,9 +242,16 @@ int buffer6_receive(CalloutHandle& handle) {
                   .arg(LeaseQueryImpl6::leaseQueryLabel(query))
                   .arg(ex.what());
 
+        handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
+        if (sending) {
+            return (0);
+        }
+        if (!invalid) {
+            StatsMgr::instance().addValue("pkt6-processing-failed",
+                                          static_cast<int64_t>(1));
+        }
         StatsMgr::instance().addValue("pkt6-receive-drop",
                                       static_cast<int64_t>(1));
-        handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
         return (0);
     }
 

diff --git a/src/hooks/dhcp/lease_query/lease_query_impl.h b/src/hooks/dhcp/lease_query/lease_query_impl.h
index 0fb43c98c735d4ce8e59ee94dd6a8de06f3ac300..2f86a043c9d55bf721a8f6e9cba63f00283e3649 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_impl.h
+++ b/src/hooks/dhcp/lease_query/lease_query_impl.h
@@ -111,7 +111,12 @@ public:
     ///
     /// @param base_query lease query to process. (Implementations
     /// must use dynamic_casting).
-    virtual void processQuery(isc::dhcp::PktPtr base_query) const = 0;
+    /// @param invalid Reference to a flag set to true when the query
+    /// is invalid (used to detect unexpected exceptions).
+    /// @param sending  Reference to a flag set to true when the query was
+    /// processed and response will be built and sent.
+    virtual void processQuery(isc::dhcp::PktPtr base_query,
+                              bool& invalid, bool& sending) const = 0;
 
     /// @brief Keywords for Lease Query configuration.
     static const isc::data::SimpleKeywords LEASE_QUERY_KEYWORDS;

diff --git a/src/hooks/dhcp/lease_query/lease_query_impl4.cc b/src/hooks/dhcp/lease_query/lease_query_impl4.cc
index 02932311ba9ed2ea8a2f4f5ec0565ee7b3fd3280..f92e5c67a27e71d76dc925143688bd5e6f86e150 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_impl4.cc
+++ b/src/hooks/dhcp/lease_query/lease_query_impl4.cc
@@ -47,7 +47,8 @@ LeaseQueryImpl4::LeaseQueryImpl4(const ConstElementPtr config)
 };
 
 void
-LeaseQueryImpl4::processQuery(PktPtr base_query) const {
+LeaseQueryImpl4::processQuery(PktPtr base_query, bool& invalid,
+                              bool& sending) const {
     Pkt4Ptr query = boost::dynamic_pointer_cast<Pkt4>(base_query);
     if (!query) {
         // Shouldn't happen.
@@ -57,10 +58,12 @@ LeaseQueryImpl4::processQuery(PktPtr base_query) const {
     /// - Validates query content
     IOAddress requester_ip = query->getGiaddr();
     if (requester_ip.isV4Zero())  {
+        invalid = true;
         isc_throw(BadValue, "giaddr cannot be 0.0.0.0");
     }
 
     if (!isRequester(requester_ip)) {
+        invalid = true;
         StatsMgr::instance().addValue("pkt4-admin-filtered",
                                       static_cast<int64_t>(1));
         isc_throw(BadValue, "rejecting query from unauthorized requester: "
@@ -69,6 +72,7 @@ LeaseQueryImpl4::processQuery(PktPtr base_query) const {
 
     OptionPtr client_server_id;
     if (!acceptServerId(query, client_server_id)) {
+        invalid = true;
         isc_throw(BadValue, "rejecting query from: "
                   << requester_ip.toText() << ", unknown server-id: "
                   << (client_server_id ? client_server_id->toText() : "malformed"));
@@ -110,6 +114,7 @@ LeaseQueryImpl4::processQuery(PktPtr base_query) const {
         break;
     default:
         // We have some combination of the three which is invalid.
+        invalid = true;
         isc_throw(BadValue, "malformed lease query: "
                   << "ciaddr: [" << ciaddr
                   << "] HWAddr: [" << hwaddr->toText()
@@ -117,6 +122,7 @@ LeaseQueryImpl4::processQuery(PktPtr base_query) const {
                   << "]");
     }
 
+    sending = true;
     Pkt4Ptr response = buildResponse(response_type, query, leases);
     /// Send the response if we have one
     if (response) {

diff --git a/src/hooks/dhcp/lease_query/lease_query_impl4.h b/src/hooks/dhcp/lease_query/lease_query_impl4.h
index cd83648427f52e0d1ab8e860fbbc919132bc2559..3e0aab00cfd1bfc33653b455c253941ef77da8a8 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_impl4.h
+++ b/src/hooks/dhcp/lease_query/lease_query_impl4.h
@@ -38,7 +38,12 @@ public:
     /// - Sends the reply
     ///
     /// @param base_query DHCPv4 lease query to process.
-    virtual void processQuery(dhcp::PktPtr base_query) const;
+    /// @param invalid Reference to a flag set to true when the query
+    /// is invalid (used to detect unexpected exceptions).
+    /// @param sending  Reference to a flag set to true when the query was
+    /// processed and response will be built and sent.
+    virtual void processQuery(isc::dhcp::PktPtr base_query,
+                              bool& invalid, bool& sending) const;
 
     /// @brief Queries for an active lease matching an ip address
     ///

diff --git a/src/hooks/dhcp/lease_query/lease_query_impl6.cc b/src/hooks/dhcp/lease_query/lease_query_impl6.cc
index 5808c1306ece95e8156dd80bd32da44c99d21cca..9cb0313a6304da8b83698d55ce834027507a5e40 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_impl6.cc
+++ b/src/hooks/dhcp/lease_query/lease_query_impl6.cc
@@ -90,7 +90,8 @@ LeaseQueryImpl6::LeaseQueryImpl6(const ConstElementPtr config)
 }
 
 void
-LeaseQueryImpl6::processQuery(PktPtr base_query) const {
+LeaseQueryImpl6::processQuery(PktPtr base_query, bool& invalid,
+                              bool& sending) const {
     Pkt6Ptr query = boost::dynamic_pointer_cast<Pkt6>(base_query);
     if (!query) {
         // Shouldn't happen.
@@ -100,21 +101,29 @@ LeaseQueryImpl6::processQuery(PktPtr base_query) const {
     // Ensure there is a client id
     DuidPtr req_clientid = query->getClientId();
     if (!req_clientid) {
+        invalid = true;
         isc_throw(BadValue, "DHCPV6_LEASEQUERY must supply a D6O_CLIENTID");
     }
 
     // If client sent a server-id, validate it.
-    testServerId(query);
+    try {
+        testServerId(query);
+    } catch (const BadValue&) {
+        invalid = true;
+        throw;
+    }
 
     // Validates query content using the source address.
     IOAddress requester_ip = query->getRemoteAddr();
     if (requester_ip.isV6Zero())  {
         /// Not sure this really possible.
+        invalid = true;
         isc_throw(BadValue, "DHCPV6_LEASEQUERY source address cannot be ::");
     }
 
     if (!isRequester(requester_ip)) {
         // RFC 5007 says we may discard or return STATUS_NotAllowed
+        invalid = true;
         StatsMgr::instance().addValue("pkt6-admin-filtered",
                                       static_cast<int64_t>(1));
         isc_throw(BadValue,
@@ -126,6 +135,7 @@ LeaseQueryImpl6::processQuery(PktPtr base_query) const {
     OptionCustomPtr lq_option = boost::dynamic_pointer_cast<OptionCustom>
                                 (query->getOption(D6O_LQ_QUERY));
     if (!lq_option) {
+        invalid = true;
         isc_throw(BadValue, "DHCPV6_LEASEQUERY must supply a D6O_LQ_QUERY option");
     }
 
@@ -137,6 +147,7 @@ LeaseQueryImpl6::processQuery(PktPtr base_query) const {
         query_link_addr = lq_option->readAddress(1);
     } catch (const std::exception& ex) {
         // unpack() should catch this?
+        invalid = true;
         isc_throw(BadValue, "error reading query field(s):" << ex.what());
     }
 
@@ -182,6 +193,7 @@ LeaseQueryImpl6::processQuery(PktPtr base_query) const {
     }
 
     // Construct the reply.
+    sending = true;
     Pkt6Ptr reply = buildReply(status_opt, query, leases);
     if (reply) {
         sendResponse(reply);

diff --git a/src/hooks/dhcp/lease_query/lease_query_impl6.h b/src/hooks/dhcp/lease_query/lease_query_impl6.h
index 393a3cf0c8d2e1cb7520df446b3f5c4a7a54b921..98acaee3d80dceb823b037b1dc5ff813a72a0df3 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_impl6.h
+++ b/src/hooks/dhcp/lease_query/lease_query_impl6.h
@@ -49,9 +49,14 @@ public:
     /// - Sends the reply
     ///
     /// @param base_query DHCPv6 lease query to process.
+    /// @param invalid Reference to a flag set to true when the query
+    /// is invalid (used to detect unexpected exceptions).
+    /// @param sending  Reference to a flag set to true when the query was
+    /// processed and response will be built and sent.
     /// @throw BadValue if the query is invalid for a number reasons,
     /// including if it comes from an unauthorized requester.
-    virtual void processQuery(dhcp::PktPtr base_query) const;
+    virtual void processQuery(isc::dhcp::PktPtr base_query,
+                              bool& invalid, bool& sending) const;
 
     /// @brief Queries for an active lease matching an ip address.
     ///

diff --git a/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc b/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc
index 50869ecda64ac8b3ec35f31e6c0a257dbfbf8115..c1e36daaa9ed09da7c2e8f13b7d50ff2987a4aa7 100644 (file)
--- a/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc
+++ b/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc
@@ -637,21 +637,33 @@ TEST(LeaseQueryImpl4Test, processQueryInvalidQuery) {
 
     // A v6 packet should get tossed.
     Pkt6Ptr pkt6(new Pkt6(DHCPV6_LEASEQUERY, 0));
-    ASSERT_THROW_MSG(impl->processQuery(pkt6), BadValue,
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(pkt6, invalid, sending), BadValue,
                      "LeaseQueryImpl4 query is not DHCPv4 packet");
+    EXPECT_FALSE(invalid);
+    EXPECT_FALSE(sending);
 
     // An empty giaddr should fail.
     Pkt4Ptr lq(new Pkt4(DHCPLEASEQUERY, 123));
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "giaddr cannot be 0.0.0.0");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Set the pkt4-admin-filtered stat to 0.
     StatsMgr::instance().setValue("pkt4-admin-filtered", static_cast<int64_t>(0));
 
     // An unknown giaddr should fail.
     lq->setGiaddr(IOAddress("192.0.2.2"));
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "rejecting query from unauthorized requester: 192.0.2.2");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Check the stat which was bumped by one.
     ObservationPtr stat = StatsMgr::instance().getObservation("pkt4-admin-filtered");
@@ -724,7 +736,12 @@ TEST(LeaseQueryImpl4Test, processQueryInvalidQuery) {
             lq->addOption(client_id);
         }
 
-        ASSERT_THROW_MSG(impl->processQuery(lq), BadValue, scenario.exp_message_);
+        invalid = false;
+        sending = false;
+        ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
+                         scenario.exp_message_);
+        EXPECT_TRUE(invalid);
+        EXPECT_FALSE(sending);
     }
 }
 

diff --git a/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc b/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc
index 62a072ca1ba5579e61616894c6ee1a2855bce45b..dc8b1361059b9b64733d30319506dd2304a70502 100644 (file)
--- a/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc
+++ b/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc
@@ -681,30 +681,46 @@ TEST_F(MemfileLeaseQueryImpl6ProcessTest, processQueryInvalidQuery) {
 
     // A v4 packet should get tossed.
     Pkt4Ptr pkt4(new Pkt4(DHCPLEASEQUERY, 0));
-    ASSERT_THROW_MSG(impl->processQuery(pkt4), BadValue,
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(pkt4, invalid, sending), BadValue,
                      "LeaseQueryImpl6 query is not DHCPv6 packet");
+    EXPECT_FALSE(invalid);
+    EXPECT_FALSE(sending);
 
     // No client-id option should fail.
     Pkt6Ptr lq(new Pkt6(DHCPV6_LEASEQUERY_REPLY, 123));
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "DHCPV6_LEASEQUERY must supply a D6O_CLIENTID");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Add a client-id option.
     lq->addOption(makeClientIdOption(std::vector<uint8_t>{ 01, 02, 03, 04, 05, 06}));
 
     // Add an a non-matching server id.
     lq->addOption(makeServerIdOption(std::vector<uint8_t>{ 10, 11, 12, 13, 14, 15, 16 }));
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "rejecting DHCPV6_LEASEQUERY from: ::,"
                      " unknown server-id: type=00002, len=00007: 0a:0b:0c:0d:0e:0f:10");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Add a matching server id.
     lq->delOption(D6O_SERVERID);
     lq->addOption(makeServerIdOption(server_id_->getDuid()));
 
     // Source address cannot be ::.
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "DHCPV6_LEASEQUERY source address cannot be ::");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Set the pkt6-admin-filtered stat to 0.
     StatsMgr::instance().setValue("pkt6-admin-filtered", static_cast<int64_t>(0));
@@ -713,8 +729,12 @@ TEST_F(MemfileLeaseQueryImpl6ProcessTest, processQueryInvalidQuery) {
     lq->setRemoteAddr(IOAddress("de:ad:be:ef::"));
 
     // An unknown requester should fail.
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "rejecting DHCPV6_LEASEQUERY from unauthorized requester: de:ad:be:ef::");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 
     // Check the stat which was bumped by one.
     ObservationPtr stat = StatsMgr::instance().getObservation("pkt6-admin-filtered");
@@ -725,8 +745,12 @@ TEST_F(MemfileLeaseQueryImpl6ProcessTest, processQueryInvalidQuery) {
     lq->setRemoteAddr(IOAddress("2001:db8:2::1"));
 
     // A query without a D6O_LQ_QUERY option should fail.
-    ASSERT_THROW_MSG(impl->processQuery(lq), BadValue,
+    invalid = false;
+    sending = false;
+    ASSERT_THROW_MSG(impl->processQuery(lq, invalid, sending), BadValue,
                      "DHCPV6_LEASEQUERY must supply a D6O_LQ_QUERY option");
+    EXPECT_TRUE(invalid);
+    EXPECT_FALSE(sending);
 }
 
 // Verifies the operation of LeaseQueryImpl6::initReply().
@@ -1407,7 +1431,11 @@ TEST_F(MemfileLeaseQueryImpl6ProcessTest, processQueryInvalidWithStatus) {
         Pkt6Ptr lq = makeLeaseQuery(scenario.qry_type_, scenario.qry_iaaddr_,
                                     scenario.qry_cid_);
         // Process the query.
-        ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+        bool invalid = false;
+        bool sending = false;
+        ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+        EXPECT_FALSE(invalid);
+        EXPECT_TRUE(sending);
 
         // We should have generated a LEASE_QUERY_REPLY with a
         // status option containing the expected status.
@@ -1506,7 +1534,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByIpAddressNoActiveLe
         Pkt6Ptr lq = makeQueryByIpAddress(scenario.qry_iaaddr_);
 
         // Process the query.
-        ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+        bool invalid = false;
+        bool sending = false;
+        ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+        EXPECT_FALSE(invalid);
+        EXPECT_TRUE(sending);
 
         // We should have generated a DHCPV6_LEASEQUERY_REPLY with a
         // status option containing the expected status.
@@ -1558,7 +1590,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByIpAddressActiveLeas
     Pkt6Ptr lq = makeQueryByIpAddress(active_lease->addr_);
 
     // Process the query.
-    ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+    EXPECT_FALSE(invalid);
+    EXPECT_TRUE(sending);
 
     // We should have generated a DHCPV6_LEASEQUERY_REPLY with a
     // status option containing the successful status.
@@ -1641,7 +1677,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByClientIdNoActiveLea
     Pkt6Ptr lq = makeQueryByClientId(cid1_);
 
     // Process the query.
-    ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+    EXPECT_FALSE(invalid);
+    EXPECT_TRUE(sending);
 
     // We should have generated a LEASE_QUERY_REPLY with a
     // status option containing the expected status.
@@ -1687,7 +1727,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByClientIdMultipleLin
     Pkt6Ptr lq = makeQueryByClientId(cid1_);
 
     // Process the query.
-    ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+    EXPECT_FALSE(invalid);
+    EXPECT_TRUE(sending);
 
     // We should have generated a LEASE_QUERY_REPLY with a
     // status option containing the expected status.
@@ -1760,7 +1804,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByClientIdActiveLease
     Pkt6Ptr lq = makeQueryByClientId(cid1_);
 
     // Process the query.
-    ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+    bool invalid = false;
+    bool sending = false;
+    ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+    EXPECT_FALSE(invalid);
+    EXPECT_TRUE(sending);
 
     // We should have generated a LEASE_QUERY_REPLY with a
     // status option containing the expected status.
@@ -2020,7 +2068,11 @@ BaseLeaseQueryImpl6ProcessTest<TestLeaseMgrType>::testQueryByIpaddressPDLeases()
         }
 
         // Process the query.
-        ASSERT_NO_THROW_LOG(impl->processQuery(lq));
+        bool invalid = false;
+        bool sending = false;
+        ASSERT_NO_THROW_LOG(impl->processQuery(lq, invalid, sending));
+        EXPECT_FALSE(invalid);
+        EXPECT_TRUE(sending);
 
         // We should have generated a LEASE_QUERY_REPLY with a
         // status option containing the expected status.