Merge pull request #2645 in SNORT/snort3 from ~NEHASH4/snort3:key_mismatch to master

Squashed commit of the following:

commit 567db0ec9a92eeab9ca8d915f01d8d8f96273d0f
Author: Neha Sharma <nehash4@cisco.com>
Date:   Mon Nov 30 04:48:26 2020 -0500

    high_availability: Adding the check for packet key equals ha key before consume

diff --git a/src/flow/ha.cc b/src/flow/ha.cc
index 685d59ed631ea146b5b1ebeab223528e6dab26ae..ab0712b980ffec1d617b485628f217934aa11c8e 100644 (file)
--- a/src/flow/ha.cc
+++ b/src/flow/ha.cc
@@ -450,7 +450,8 @@ static Flow* consume_ha_update_message(HAMessage& msg, const FlowKey& key, Packe
     return flow;
 }
 
-static Flow* consume_ha_message(HAMessage& msg, Packet* p = nullptr)
+static Flow* consume_ha_message(HAMessage& msg,
+    FlowKey* packet_key = nullptr, Packet* p = nullptr)
 {
     ha_stats.msgs_recv++;
 
@@ -480,6 +481,12 @@ static Flow* consume_ha_message(HAMessage& msg, Packet* p = nullptr)
     if (read_flow_key(msg, hdr, key) == 0)
         return nullptr;
 
+    if (packet_key and !FlowKey::is_equal(packet_key, &key, 0))
+    {
+        ha_stats.key_mismatch++;
+        return nullptr;
+    }
+
     Flow* flow = nullptr;
     switch (hdr->event)
     {
@@ -653,26 +660,20 @@ Flow* HighAvailability::process_daq_import(Packet& p, FlowKey& key)
         if (p.daq_instance->ioctl(DIOCTL_GET_FLOW_HA_STATE, &fhs, sizeof(fhs)) == DAQ_SUCCESS)
         {
             HAMessage ha_msg(fhs.data, fhs.length);
-            flow = consume_ha_message(ha_msg, &p);
+            flow = consume_ha_message(ha_msg, &key, &p);
             ha_stats.daq_imports++;
             // Validate that the imported flow matches up with the given flow key.
             if (flow)
             {
-                if (FlowKey::is_equal(&key, flow->key, 0))
+                if (flow->flow_state == Flow::FlowState::BLOCK
+                    or flow->flow_state == Flow::FlowState::RESET)
                 {
-                    if (flow->flow_state == Flow::FlowState::BLOCK
-                        or flow->flow_state == Flow::FlowState::RESET)
-                    {
-                        flow->disable_inspection();
-                        p.disable_inspect = true;
-                    }
-
-                    // Clear the standby bit so that we don't immediately trigger a new data store
-                    // FIXIT-L streamline the consume process so this doesn't have to be done here
-                    flow->ha_state->clear(FlowHAState::STANDBY);
+                    flow->disable_inspection();
+                    p.disable_inspect = true;
                 }
-                else
-                    flow = nullptr;
+                // Clear the standby bit so that we don't immediately trigger a new data store
+                // FIXIT-L streamline the consume process so this doesn't have to be done here
+                flow->ha_state->clear(FlowHAState::STANDBY);
             }
         }
     }

diff --git a/src/flow/ha_module.cc b/src/flow/ha_module.cc
index 5b43985e041e271ed21da4c59d0931a056b8e7da..05d98f491c9f3442473d0dba9acd3d07b9953e9c 100644 (file)
--- a/src/flow/ha_module.cc
+++ b/src/flow/ha_module.cc
@@ -65,6 +65,7 @@ static const PegInfo ha_pegs[] =
     { CountType::SUM, "delete_msgs_consumed", "deletion messages consumed" },
     { CountType::SUM, "daq_stores", "states stored via daq" },
     { CountType::SUM, "daq_imports", "states imported via daq" },
+    { CountType::SUM, "key_mismatch", "messages received with a flow key mismatch" },
     { CountType::SUM, "msg_version_mismatch", "messages received with a version mismatch" },
     { CountType::SUM, "msg_length_mismatch", "messages received with an inconsistent total length" },
     { CountType::SUM, "truncated_msgs", "truncated messages received" },

diff --git a/src/flow/ha_module.h b/src/flow/ha_module.h
index ca829f2ce1407fea30f0fd30cb572e945e69ed7d..1d599d80a98f275be03e490d3d16f376fae1e6db 100644 (file)
--- a/src/flow/ha_module.h
+++ b/src/flow/ha_module.h
@@ -71,6 +71,7 @@ struct HAStats
     PegCount delete_msgs_consumed;
     PegCount daq_stores;
     PegCount daq_imports;
+    PegCount key_mismatch;
     PegCount msg_version_mismatch;
     PegCount msg_length_mismatch;
     PegCount truncated_msgs;

diff --git a/src/flow/test/ha_test.cc b/src/flow/test/ha_test.cc
index d39433d3b5c06ee2a58a592a05ea4a9ade433f2f..470286724771dd0e946dea3d12b13edf388b3d5b 100644 (file)
--- a/src/flow/test/ha_test.cc
+++ b/src/flow/test/ha_test.cc
@@ -549,12 +549,24 @@ TEST(high_availability_test, consume_error_client_consume)
     CHECK(ha_stats.client_consume_errors == 1);
 }
 
+TEST(high_availability_test, consume_error_key_mismatch)
+{
+    HAMessageHeader hdr[10] = { 0, HA_MESSAGE_VERSION, 0x32, KEY_TYPE_IP4 };
+    HAMessage msg((uint8_t*) &hdr, sizeof(hdr));
+
+    FlowKey packet_key;
+    FlowKey* key = &packet_key;
+    CHECK(consume_ha_message(msg, key, &s_pkt) == nullptr);
+    CHECK(ha_stats.key_mismatch == 1);
+}
+
 TEST(high_availability_test, consume_error_truncated_msg_hdr)
 {
     HAMessageHeader hdr = { };
     HAMessage msg((uint8_t*) &hdr, sizeof(hdr) / 2);
 
-    CHECK(consume_ha_message(msg, &s_pkt) == nullptr);
+    FlowKey* key = nullptr;
+    CHECK(consume_ha_message(msg, key, &s_pkt) == nullptr);
     CHECK(ha_stats.truncated_msgs == 1);
 }
 
@@ -563,7 +575,8 @@ TEST(high_availability_test, consume_error_version_mismatch)
     HAMessageHeader hdr = { 0, HA_MESSAGE_VERSION + 1, 0, 0 };
     HAMessage msg((uint8_t*) &hdr, sizeof(hdr));
 
-    CHECK(consume_ha_message(msg, &s_pkt) == nullptr);
+    FlowKey* key = nullptr;
+    CHECK(consume_ha_message(msg, key, &s_pkt) == nullptr);
     CHECK(ha_stats.msg_version_mismatch == 1);
 }
 
@@ -572,7 +585,8 @@ TEST(high_availability_test, consume_error_length_mismatch)
     HAMessageHeader hdr = { 0, HA_MESSAGE_VERSION, 0x42, 0 };
     HAMessage msg((uint8_t*) &hdr, sizeof(hdr));
 
-    CHECK(consume_ha_message(msg, &s_pkt) == nullptr);
+    FlowKey* key = nullptr;
+    CHECK(consume_ha_message(msg, key, &s_pkt) == nullptr);
     CHECK(ha_stats.msg_length_mismatch == 1);
 }