[#4549] Added client id check

diff --git a/changelog_unreleased/4549-BLQ-running-queries-bugs b/changelog_unreleased/4549-BLQ-running-queries-bugs
new file mode 100644 (file)
index 0000000..76a94a7
--- /dev/null
+++ b/changelog_unreleased/4549-BLQ-running-queries-bugs
@@ -0,0 +1,6 @@
+[bug]          fdupont
+       Corrected Bulk Lease Query processing e.g. now
+       return an error on bad dhcp-client-identifier option.
+       Thank you to Qifan Zhang from Palo Alto Networks for
+       reporting the issue.
+       (Gitlab #4549)

diff --git a/src/hooks/dhcp/lease_query/bulk_lease_query4.cc b/src/hooks/dhcp/lease_query/bulk_lease_query4.cc
index e2ff34bf4a31e4ef711a991f2520e84997a6f3d2..378f95bea7ca455a4adefafe5433ffbf1b25c382 100644 (file)
--- a/src/hooks/dhcp/lease_query/bulk_lease_query4.cc
+++ b/src/hooks/dhcp/lease_query/bulk_lease_query4.cc
@@ -113,6 +113,12 @@ BulkLeaseQuery4::init() {
     // Get the client identifier.
     query_client_id_ = query4_->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
     if (query_client_id_) {
+        try {
+            ClientId client_id(query_client_id_->getData());
+        } catch (const Exception&) {
+            sendDone(BLQ_STATUS_MalformedQuery, "bad client id option");
+            return;
+        }
         query_mask_ |= 1;
     }
 

diff --git a/src/hooks/dhcp/lease_query/lease_query_connection.cc b/src/hooks/dhcp/lease_query/lease_query_connection.cc
index 570197ca16d97c0ad10f68bed94fb44b399a12ba..611af1b6bf313d4bf063094e553873eed354467c 100644 (file)
--- a/src/hooks/dhcp/lease_query/lease_query_connection.cc
+++ b/src/hooks/dhcp/lease_query/lease_query_connection.cc
@@ -365,7 +365,7 @@ LeaseQueryConnection::processNextQuery() {
     }
     // Pop and process waiting queries until queue is empty or list full.
     while ((max_concurrent_queries_ == 0) ||
-           (max_concurrent_queries_ >= getNumRunningQueries())) {
+           (max_concurrent_queries_ > getNumRunningQueries())) {
         BlqQueryPtr query = popPendingQuery();
         if (!query) {
             return;

diff --git a/src/hooks/dhcp/lease_query/tests/bulk_lease_query4_unittest.cc b/src/hooks/dhcp/lease_query/tests/bulk_lease_query4_unittest.cc
index ce4e89b71a42dea48de6d8d9195f18eaa7958503..e4c0f5ce2159825d8b1832b3c447ce2f265921a4 100644 (file)
--- a/src/hooks/dhcp/lease_query/tests/bulk_lease_query4_unittest.cc
+++ b/src/hooks/dhcp/lease_query/tests/bulk_lease_query4_unittest.cc
@@ -1047,6 +1047,18 @@ TEST_F(MemfileBulkLeaseQuery4ProcessTest, initQueryInvalidWithStatus) {
             BLQ_STATUS_MalformedQuery,
             "multiple queries"
         },
+        {
+            "bad client id option",
+            [](Pkt4Ptr lq) {
+                const vector<uint8_t> bad = { 1 };
+                OptionPtr cid(new Option(Option::V4,
+                                         DHO_DHCP_CLIENT_IDENTIFIER,
+                                         bad));
+                lq->addOption(cid);
+            },
+            BLQ_STATUS_MalformedQuery,
+            "bad client id option"
+        },
         {
             "empty relay id option",
             [this](Pkt4Ptr lq) {
@@ -1056,8 +1068,8 @@ TEST_F(MemfileBulkLeaseQuery4ProcessTest, initQueryInvalidWithStatus) {
                 rai->addOption(opt);
                 lq->addOption(rai);
             },
-             BLQ_STATUS_MalformedQuery,
-             "empty relay id option"
+            BLQ_STATUS_MalformedQuery,
+            "empty relay id option"
         },
         {
             "empty remote id option",
@@ -1068,8 +1080,8 @@ TEST_F(MemfileBulkLeaseQuery4ProcessTest, initQueryInvalidWithStatus) {
                 rai->addOption(opt);
                 lq->addOption(rai);
             },
-             BLQ_STATUS_MalformedQuery,
-             "empty remote id option"
+            BLQ_STATUS_MalformedQuery,
+            "empty remote id option"
         }
     };