From: Naveen Gujje (ngujje) Date: Thu, 10 Dec 2020 06:12:49 +0000 (+0000) Subject: Merge pull request #2645 in SNORT/snort3 from ~NEHASH4/snort3:key_mismatch to master X-Git-Tag: 3.0.3-6~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b2485123fa39d41374f5c3a5e1b2fc6965a30eb;p=thirdparty%2Fsnort3.git Merge pull request #2645 in SNORT/snort3 from ~NEHASH4/snort3:key_mismatch to master Squashed commit of the following: commit 567db0ec9a92eeab9ca8d915f01d8d8f96273d0f Author: Neha Sharma 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 685d59ed6..ab0712b98 100644 --- 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 5b43985e0..05d98f491 100644 --- 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 ca829f2ce..1d599d80a 100644 --- 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 d39433d3b..470286724 100644 --- 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); }