From: Ron Dempster (rdempste) Date: Fri, 21 Aug 2020 21:20:28 +0000 (+0000) Subject: Merge pull request #2400 in SNORT/snort3 from ~RDEMPSTE/snort3:deferred_whitelist... X-Git-Tag: 3.0.2-6~47 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f02192f1e43cfbd6d280a3dae1960c0abbb13e0;p=thirdparty%2Fsnort3.git Merge pull request #2400 in SNORT/snort3 from ~RDEMPSTE/snort3:deferred_whitelist to master Squashed commit of the following: commit dcd0bdfa212159b0cf8862084a8c55a7ff1c870d Author: rdempste Date: Mon Aug 3 20:15:57 2020 -0400 flow: add a deferred trust class to allow plugins to defer trusting sessions commit 5c5a962ff08973d70c9f0a29bd0aca1c3476a974 Author: rdempste Date: Thu Jul 23 13:26:31 2020 -0400 managers: immediately stop executing inspectors when inspection is disabled commit ac5e78590bdd8a8ef494077443423b1fa49c7f85 Author: rdempste Date: Thu Jul 23 13:28:26 2020 -0400 packet_io: do not allow trust unless the action is allow or trust commit 5e0c38db8d8ac762068be67677c409c9f183d2ca Author: Ron Dempster (rdempste) Date: Wed Aug 19 15:22:28 2020 -0400 active: remove per packet prevent trust action --- diff --git a/cmake/FindDAQ.cmake b/cmake/FindDAQ.cmake index 907c65583..a420ddcaf 100644 --- a/cmake/FindDAQ.cmake +++ b/cmake/FindDAQ.cmake @@ -67,6 +67,6 @@ if (PKG_CONFIG_EXECUTABLE AND ENABLE_STATIC_DAQ) endif() if (DAQ_STATIC_MODULES) list(SORT DAQ_STATIC_MODULES) - set(DAQ_STATIC_MODULES ${DAQ_STATIC_MODULES} CACHE INTERNAL "Static DAQ modules") + # set(DAQ_STATIC_MODULES ${DAQ_STATIC_MODULES} CACHE INTERNAL "Static DAQ modules") endif() endif() diff --git a/src/flow/CMakeLists.txt b/src/flow/CMakeLists.txt index 34ff1be24..663832c55 100644 --- a/src/flow/CMakeLists.txt +++ b/src/flow/CMakeLists.txt @@ -1,4 +1,5 @@ set (FLOW_INCLUDES + deferred_trust.h expect_cache.h flow.h flow_data.h @@ -10,6 +11,7 @@ set (FLOW_INCLUDES add_library (flow OBJECT ${FLOW_INCLUDES} + deferred_trust.cc expect_cache.cc flow.cc flow_cache.cc diff --git a/src/flow/deferred_trust.cc b/src/flow/deferred_trust.cc new file mode 100644 index 000000000..962203f0a --- /dev/null +++ b/src/flow/deferred_trust.cc @@ -0,0 +1,73 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2020-2020 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- +// deferred_trust.cc author Ron Dempster + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "deferred_trust.h" + +#include "packet_io/active.h" + +using namespace snort; + +void DeferredTrust::set_deferred_trust(unsigned module_id, bool on) +{ + if (on) + { + if (deferred_trust_modules.empty()) + { + if (TRUST_DEFER_DO_TRUST == deferred_trust) + deferred_trust = TRUST_DEFER_DEFERRING; + else + deferred_trust = TRUST_DEFER_ON; + } + auto element = deferred_trust_modules.begin(); + for (; element != deferred_trust_modules.end() && *element != module_id; + ++element); + if (element == deferred_trust_modules.end()) + deferred_trust_modules.emplace_front(module_id); + } + else if (!deferred_trust_modules.empty()) + { + deferred_trust_modules.remove(module_id); + if (deferred_trust_modules.empty()) + { + if (TRUST_DEFER_DEFERRING == deferred_trust) + deferred_trust = TRUST_DEFER_DO_TRUST; + else + deferred_trust = TRUST_DEFER_OFF; + } + } +} + +void DeferredTrust::finalize(Active& active) +{ + if (active.packet_was_dropped()) + clear(); + else if (TRUST_DEFER_DO_TRUST == deferred_trust && active.session_was_allowed()) + active.set_trust(); + else if (TRUST_DEFER_ON == deferred_trust && active.session_was_trusted()) + { + // This is the case where defer was called after session trust while processing + // the same packet + deferred_trust = TRUST_DEFER_DEFERRING; + active.set_allow(); + } +} diff --git a/src/flow/deferred_trust.h b/src/flow/deferred_trust.h new file mode 100644 index 000000000..f9c4266cb --- /dev/null +++ b/src/flow/deferred_trust.h @@ -0,0 +1,90 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2020-2020 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- +// deferred_trust.h author Ron Dempster + +#ifndef DEFERRED_TRUST_H +#define DEFERRED_TRUST_H + +#include +#include + +#include "main/snort_types.h" + +namespace snort +{ + +class Active; +class Flow; +struct Packet; + +class DeferredTrust +{ + // This class is used to delay session trust. This is used in cases where + // a module needs to continue inspecting a session to enforce policy. + // A module calls set_deferred_trust with the on parameter true to begin + // deferring. The sets the state to TRUST_DEFER_ON. If trust session is called + // while deferring, the state is changed to TRUST_DEFER_DEFERRING to the trust action. + // A module calls set_deferred_trust with the on parameter false to stop deferring. + // When all modules have stopped deferring, the state is checked. If the state is + // TRUST_DEFER_DEFERRING, the state is changed to TRUST_DEFER_DO_TRUST. Otherwise, the state + // is set to TRUST_DEFER_OFF. + // The TRUST_DEFER_DO_TRUST state is checked at the end of packet processing. If the state + // is TRUST_DEFER_DO_TRUST and the action is ACT_ALLOW, the session is trusted. + // If a drop, block or reset action occurs while deferring, deferring is stopped and the + // block or blacklist version is enforced. + // The module_id, a unique module identifier created by calling + // FlowData::create_flow_data_id(), is used to track the modules that are currently deferring. + // This allows the module to use whitelist deferring without needing to track the deferring + // state of the module. + enum DeferredTrustState : uint8_t + { + TRUST_DEFER_OFF = 0, + TRUST_DEFER_ON, + TRUST_DEFER_DEFERRING, + TRUST_DEFER_DO_TRUST, + }; + +public: + DeferredTrust() = default; + ~DeferredTrust() = default; + SO_PUBLIC void set_deferred_trust(unsigned module_id, bool on); + bool is_active() + { return TRUST_DEFER_ON == deferred_trust || TRUST_DEFER_DEFERRING == deferred_trust; } + bool try_trust() + { + if (TRUST_DEFER_ON == deferred_trust) + deferred_trust = TRUST_DEFER_DEFERRING; + return TRUST_DEFER_DEFERRING != deferred_trust; + } + bool is_deferred() + { return TRUST_DEFER_DEFERRING == deferred_trust; } + void clear() + { + deferred_trust = TRUST_DEFER_OFF; + deferred_trust_modules.clear(); + } + void finalize(Active&); + +protected: + std::forward_list deferred_trust_modules; + DeferredTrustState deferred_trust = TRUST_DEFER_OFF; +}; + +} + +#endif diff --git a/src/flow/flow.cc b/src/flow/flow.cc index 3dc36b39c..16810e6f6 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -40,7 +40,9 @@ using namespace snort; Flow::Flow() { memory::MemoryCap::update_allocations(sizeof(*this) + sizeof(FlowStash)); - memset(this, 0, sizeof(*this)); + constexpr size_t offset = offsetof(Flow, key); + // FIXIT-L need a struct to zero here to make future proof + memset((uint8_t*)this+offset, 0, sizeof(*this)-offset); } Flow::~Flow() @@ -187,7 +189,9 @@ void Flow::reset(bool do_cleanup) if ( stash ) stash->reset(); - constexpr size_t offset = offsetof(Flow, flow_data); + deferred_trust.clear(); + + constexpr size_t offset = offsetof(Flow, context_chain); // FIXIT-L need a struct to zero here to make future proof memset((uint8_t*)this+offset, 0, sizeof(Flow)-offset); } @@ -231,6 +235,13 @@ void Flow::clear(bool dump_flow_data) clear_gadget(); } +void Flow::trust() +{ + set_ignore_direction(SSN_DIR_BOTH); + set_state(Flow::FlowState::ALLOW); + disable_inspection(); +} + int Flow::set_flow_data(FlowData* fd) { FlowData* old = get_flow_data(fd->get_id()); diff --git a/src/flow/flow.h b/src/flow/flow.h index 58339a1c1..20f083b88 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -30,6 +30,7 @@ #include #include "detection/ips_context_chain.h" +#include "flow/deferred_trust.h" #include "flow/flow_data.h" #include "flow/flow_stash.h" #include "framework/data_bus.h" @@ -150,14 +151,6 @@ struct LwState char ignore_direction; }; -enum DeferredWhitelist -{ - WHITELIST_DEFER_OFF = 0, - WHITELIST_DEFER_ON, - WHITELIST_DEFER_STARTED, - WHITELIST_DEFER_DONE, -}; - // this struct is organized by member size for compactness class SO_PUBLIC Flow { @@ -358,29 +351,36 @@ public: bool is_hard_expiration() { return (ssn_state.session_flags & SSNFLAG_HARD_EXPIRATION) != 0; } - void set_deferred_whitelist(DeferredWhitelist defer_state) - { - if (defer_state == WHITELIST_DEFER_DONE ) - { - if (deferred_whitelist == WHITELIST_DEFER_STARTED ) - deferred_whitelist = WHITELIST_DEFER_DONE; - else - deferred_whitelist = WHITELIST_DEFER_OFF; - } - else - deferred_whitelist = defer_state; - } + void set_deferred_trust(unsigned module_id, bool on) + { deferred_trust.set_deferred_trust(module_id, on); } + + bool cannot_trust() + { return deferred_trust.is_active(); } - DeferredWhitelist get_deferred_whitelist_state() + bool try_trust() + { return deferred_trust.try_trust(); } + + void stop_deferring_trust() + { deferred_trust.clear(); } + + void finalize_trust(Active& active) { - return deferred_whitelist; + deferred_trust.finalize(active); } + void trust(); + + bool trust_is_deferred() + { return deferred_trust.is_deferred(); } + public: // FIXIT-M privatize if possible // fields are organized by initialization and size to minimize // void space and allow for memset of tail end of struct // these fields are const after initialization + DeferredTrust deferred_trust; + + // Anything before this comment is not zeroed during construction const FlowKey* key; BitOp* bitop; FlowHAState* ha_state; @@ -417,8 +417,6 @@ public: // FIXIT-M privatize if possible uint64_t expire_time; - DeferredWhitelist deferred_whitelist = WHITELIST_DEFER_OFF; - unsigned inspection_policy_id; unsigned ips_policy_id; unsigned network_policy_id; diff --git a/src/flow/test/CMakeLists.txt b/src/flow/test/CMakeLists.txt index fe0a2b09f..f199aea6e 100644 --- a/src/flow/test/CMakeLists.txt +++ b/src/flow/test/CMakeLists.txt @@ -1,5 +1,9 @@ add_cpputest( ha_test ) +add_cpputest( deferred_trust_test + SOURCES ../deferred_trust.cc +) + add_cpputest( flow_stash_test SOURCES ../flow_stash.cc ) diff --git a/src/flow/test/deferred_trust_test.cc b/src/flow/test/deferred_trust_test.cc new file mode 100644 index 000000000..3d39c6801 --- /dev/null +++ b/src/flow/test/deferred_trust_test.cc @@ -0,0 +1,141 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2015-2020 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- + +// deferred_trust_test.cc author Ron Dempster +// unit test main + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "flow/deferred_trust.h" +#include "packet_io/active.h" + +#include +#include + +using namespace snort; + +//------------------------------------------------------------------------- +// tests +//------------------------------------------------------------------------- + +TEST_GROUP(deferred_trust_test) +{ + DeferredTrust deferred_trust; +}; + +TEST(deferred_trust_test, set_deferred_trust) +{ + // Disable non-existent module_id + deferred_trust.set_deferred_trust(1, false); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); + + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Disable non-existent module_id, no state change + deferred_trust.set_deferred_trust(2, false); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Disable the only module_id disables deferring + deferred_trust.set_deferred_trust(1, false); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); + + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Enable second module_id + deferred_trust.set_deferred_trust(2, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Disable the first module_id, no state change + deferred_trust.set_deferred_trust(1, false); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Disable the second module_id disables deferring + deferred_trust.set_deferred_trust(2, false); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); + + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Try to trust, change state to deferring + deferred_trust.try_trust(); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(deferred_trust.is_deferred(), "Deferred trust should be deferring"); + // Disable the only module_id disables deferring + deferred_trust.set_deferred_trust(1, false); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); +} + +TEST(deferred_trust_test, finalize) +{ + Active active{}; + active.block_again(); + + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // finalize with blocked packet disables deferring + deferred_trust.finalize(active); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); + + active.set_allow(); + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Try to trust, change state to deferring + deferred_trust.try_trust(); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(deferred_trust.is_deferred(), "Deferred trust should be deferring"); + // Disable the only module_id disables deferring + deferred_trust.set_deferred_trust(1, false); + CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); + // State should be do trust + deferred_trust.finalize(active); + CHECK_TEXT(active.session_was_trusted(), "Session was not trusted from do trust"); + + // Enable with state do trust goes to deferring + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(deferred_trust.is_deferred(), "Deferred trust should be deferring"); + + deferred_trust.clear(); + // Enable + deferred_trust.set_deferred_trust(1, true); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(!deferred_trust.is_deferred(), "Deferred trust should not be deferring"); + // Session is trusted, defer should move to deferring and session should not be trusted + deferred_trust.finalize(active); + CHECK_TEXT(deferred_trust.is_active(), "Deferred trust should be active"); + CHECK_TEXT(deferred_trust.is_deferred(), "Deferred trust should be deferring"); + CHECK_TEXT(!active.session_was_trusted(), "Session was trusted while deferring trust"); + CHECK_TEXT(active.session_was_allowed(), "Session was not allowed while deferring trust"); +} + + +int main(int argc, char** argv) +{ + return CommandLineTestRunner::RunAllTests(argc, argv); +} diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index 524c1160e..b90d418dc 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -210,52 +210,6 @@ static bool process_packet(Packet* p) return true; } -// If necessary, defer returning a WHITELIST until it's safe to do so. -// While deferring, keep inspection turned on. -static void handle_deferred_whitelist(Packet* pkt, DAQ_Verdict& verdict) -{ - if ( !pkt->flow or pkt->flow->deferred_whitelist == WHITELIST_DEFER_OFF ) - return; - - if (verdict == DAQ_VERDICT_BLOCK or verdict == DAQ_VERDICT_BLACKLIST ) - { - // A block/blacklist verdict overrides any earlier whitelist. - pkt->flow->deferred_whitelist = WHITELIST_DEFER_OFF; - return; - } - - if ( pkt->flow->deferred_whitelist == WHITELIST_DEFER_ON ) - { - if ( verdict == DAQ_VERDICT_WHITELIST ) - { - verdict = DAQ_VERDICT_PASS; - pkt->flow->deferred_whitelist = WHITELIST_DEFER_STARTED; - pkt->flow->set_state(Flow::FlowState::INSPECT); - pkt->flow->flags.disable_inspect = false; - } - return; - } - - if ( pkt->flow->deferred_whitelist == WHITELIST_DEFER_STARTED) - { - verdict = DAQ_VERDICT_PASS; - pkt->flow->set_state(Flow::FlowState::INSPECT); - pkt->flow->flags.disable_inspect = false; - return; - } - - if ( pkt->flow->deferred_whitelist == WHITELIST_DEFER_DONE ) - { - // Now that we're done deferring, return the WHITELIST that would - // have happened earlier. - verdict = DAQ_VERDICT_WHITELIST; - - // Turn inspection back off and allow the flow. - pkt->flow->set_state(Flow::FlowState::ALLOW); - pkt->flow->flags.disable_inspect = true; - } -} - // Finalize DAQ message verdict static DAQ_Verdict distill_verdict(Packet* p) { @@ -301,34 +255,13 @@ static DAQ_Verdict distill_verdict(Packet* p) verdict = DAQ_VERDICT_REPLACE; } else if ( act->session_was_trusted() ) - { - if ( p->flow && !act->get_prevent_trust_action() ) - p->flow->disable_inspection(); - - if ( !(act->get_tunnel_bypass() || act->get_prevent_trust_action()) ) - { - verdict = DAQ_VERDICT_WHITELIST; - } - else - { - verdict = DAQ_VERDICT_PASS; - daq_stats.internal_whitelist++; - } - } + verdict = DAQ_VERDICT_WHITELIST; else if ( (p->packet_flags & PKT_IGNORE) || (p->flow && (p->flow->get_ignore_direction() == SSN_DIR_BOTH || p->flow->flow_state == Flow::FlowState::ALLOW)) ) { - if ( !(act->get_tunnel_bypass() || act->get_prevent_trust_action()) ) - { - verdict = DAQ_VERDICT_WHITELIST; - } - else - { - verdict = DAQ_VERDICT_PASS; - daq_stats.internal_whitelist++; - } + verdict = DAQ_VERDICT_WHITELIST; } else if ( p->ptrs.decode_flags & DECODE_PKT_TRUST ) { @@ -339,8 +272,16 @@ static DAQ_Verdict distill_verdict(Packet* p) else verdict = DAQ_VERDICT_PASS; - handle_deferred_whitelist(p, verdict); - + if (DAQ_VERDICT_WHITELIST == verdict) + { + if (p->flow && p->flow->cannot_trust()) + verdict = DAQ_VERDICT_PASS; + else if (act->get_tunnel_bypass()) + { + verdict = DAQ_VERDICT_PASS; + daq_stats.internal_whitelist++; + } + } return verdict; } @@ -395,7 +336,7 @@ void Analyzer::post_process_daq_pkt_msg(Packet* p) if (verdict == DAQ_VERDICT_BLOCK or verdict == DAQ_VERDICT_BLACKLIST) p->active->send_reason_to_daq(*p); - + oops_handler->set_current_message(nullptr); p->pkth = nullptr; // No longer avail after finalize_message. diff --git a/src/main/test/distill_verdict_test.cc b/src/main/test/distill_verdict_test.cc index f4d106df2..e2aeccc0b 100644 --- a/src/main/test/distill_verdict_test.cc +++ b/src/main/test/distill_verdict_test.cc @@ -39,6 +39,12 @@ int SFDAQInstance::finalize_message(DAQ_Msg_h, DAQ_Verdict verdict) mock().actualCall("finalize_message").onObject(this).withParameter("verdict", verdict); return -1; } +void DeferredTrust::finalize(Active&) { } +void DeferredTrust::set_deferred_trust(unsigned module_id, bool on) +{ + deferred_trust = on ? TRUST_DEFER_ON : TRUST_DEFER_OFF; +} +void Flow::trust() { } } using namespace snort; @@ -82,25 +88,25 @@ TEST(distill_verdict_tests, normal_pass) mock().checkExpectations(); } -TEST(distill_verdict_tests, trust_session_whitelist) +TEST(distill_verdict_tests, trust_session_whitelist_on_blocked) { - // Trust session whitelist verdict + // Trust session whitelist verdict on blocked packet does nothing pkt.flow = &flow; pkt.packet_flags = PKT_FROM_CLIENT; flow.flags.disable_inspect = false; flow.ssn_state.ignore_direction = SSN_DIR_NONE; flow.flow_state = Flow::FlowState::INSPECT; act.reset(); + act.drop_packet(&pkt, true); act.trust_session(&pkt); - mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_WHITELIST); + mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_BLOCK); analyzer->post_process_packet(&pkt); mock().checkExpectations(); - CHECK_TEXT(flow.flags.disable_inspect, "Disable inspection should have been called"); } -TEST(distill_verdict_tests, prevent_trust_session_whitelist) +TEST(distill_verdict_tests, trust_session_whitelist) { - // Prevent trust_session whitelist verdict + // Trust session whitelist verdict pkt.flow = &flow; pkt.packet_flags = PKT_FROM_CLIENT; flow.flags.disable_inspect = false; @@ -108,11 +114,9 @@ TEST(distill_verdict_tests, prevent_trust_session_whitelist) flow.flow_state = Flow::FlowState::INSPECT; act.reset(); act.trust_session(&pkt); - act.set_prevent_trust_action(); - mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_PASS); + mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_WHITELIST); analyzer->post_process_packet(&pkt); mock().checkExpectations(); - CHECK_TEXT(!flow.flags.disable_inspect, "Disable inspection should not have been called"); } TEST(distill_verdict_tests, flow_state_whitelist) @@ -130,22 +134,6 @@ TEST(distill_verdict_tests, flow_state_whitelist) CHECK_TEXT(!flow.flags.disable_inspect, "Disable inspection should not have been called"); } -TEST(distill_verdict_tests, prevent_flow_state_whitelist) -{ - // Prevent flow state whitelist verdict - pkt.flow = &flow; - pkt.packet_flags = PKT_FROM_CLIENT; - flow.flags.disable_inspect = false; - flow.ssn_state.ignore_direction = SSN_DIR_NONE; - flow.flow_state = Flow::FlowState::ALLOW; - act.reset(); - act.set_prevent_trust_action(); - mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_PASS); - analyzer->post_process_packet(&pkt); - mock().checkExpectations(); - CHECK_TEXT(!flow.flags.disable_inspect, "Disable inspection should not have been called"); -} - TEST(distill_verdict_tests, ignore_both_whitelist) { // Normal ignore both directions whitelist verdict @@ -161,20 +149,19 @@ TEST(distill_verdict_tests, ignore_both_whitelist) CHECK_TEXT(!flow.flags.disable_inspect, "Disable inspection should not have been called"); } -TEST(distill_verdict_tests, prevent_ignore_both_whitelist) +TEST(distill_verdict_tests, deferred_trust_prevent_whitelist) { - // Prevent ignore both directions whitelist verdict + // Deferred trust prevent whitelist pkt.flow = &flow; pkt.packet_flags = PKT_FROM_CLIENT; flow.flags.disable_inspect = false; - flow.ssn_state.ignore_direction = SSN_DIR_BOTH; - flow.flow_state = Flow::FlowState::INSPECT; + flow.ssn_state.ignore_direction = SSN_DIR_NONE; + flow.flow_state = Flow::FlowState::ALLOW; + flow.set_deferred_trust(53, true); act.reset(); - act.set_prevent_trust_action(); mock().expectNCalls(1, "finalize_message").onObject(di).withParameter("verdict", DAQ_VERDICT_PASS); analyzer->post_process_packet(&pkt); mock().checkExpectations(); - CHECK_TEXT(!flow.flags.disable_inspect, "Disable inspection should not have been called"); } //------------------------------------------------------------------------- diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index f45cad877..d77bc3afa 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -1098,6 +1098,10 @@ static inline void execute( if ( T ) trace_ulogf(snort_trace, TRACE_INSPECTOR_MANAGER, p, "exit %s, elapsed time: %" PRId64" usec\n", inspector_name, TO_USECS(timer.get())); + + // must check between each ::execute() + if ( p->disable_inspect ) + return; } } diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index fdefc7e2b..0eb6a8085 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -648,23 +648,23 @@ void Active::cancel_packet_hold() void Active::trust_session(Packet* p, bool force) { - active_action = ACT_TRUST; - p->packet_flags |= PKT_IGNORE; - DetectionEngine::disable_all(p); + if (ACT_ALLOW < active_action) + return; - if ( p->flow ) - { - p->flow->set_ignore_direction(SSN_DIR_BOTH); - p->flow->set_state(Flow::FlowState::ALLOW); - } + DetectionEngine::disable_all(p); if (force) { + p->packet_flags |= PKT_IGNORE; if ( p->flow ) - p->flow->disable_inspection(); - + { + p->flow->trust(); + p->flow->stop_deferring_trust(); + } p->disable_inspect = true; } + else if (p->flow && p->flow->try_trust()) + active_action = ACT_TRUST; } void Active::block_session(Packet* p, bool force) @@ -796,7 +796,6 @@ void Active::close() void Active::reset() { active_tunnel_bypass = 0; - prevent_trust_action = false; active_status = AST_ALLOW; active_would_reason = WHD_NONE; active_action = ACT_ALLOW; @@ -818,6 +817,13 @@ void Active::execute(Packet* p) (*p->action)->exec(p); *p->action = nullptr; } + + if (p->flow) + { + p->flow->finalize_trust(*p->active); + if (p->active->session_was_trusted()) + p->flow->trust(); + } } void Active::set_default_drop_reason(uint8_t reason_id) diff --git a/src/packet_io/active.h b/src/packet_io/active.h index a817dd572..69e6c2f30 100644 --- a/src/packet_io/active.h +++ b/src/packet_io/active.h @@ -135,6 +135,12 @@ public: void reset_again() { active_action = ACT_RESET; } + void set_trust() + { active_action = ACT_TRUST; } + + void set_allow() + { active_action = ACT_ALLOW; } + bool packet_was_dropped() const { return active_action >= ACT_DROP; } @@ -156,6 +162,9 @@ public: bool session_was_blocked() const { return active_action >= ACT_BLOCK; } + bool session_was_allowed() const + { return active_action <= ACT_ALLOW; } + bool packet_force_dropped() const { return active_status == AST_FORCE; } @@ -171,12 +180,6 @@ public: bool get_tunnel_bypass() const { return active_tunnel_bypass > 0; } - void set_prevent_trust_action() - { prevent_trust_action = true; } - - bool get_prevent_trust_action() const - { return prevent_trust_action; } - void set_delayed_action(ActiveActionType, bool force = false); void set_delayed_action(ActiveActionType, ActiveAction* act, bool force = false); void apply_delayed_action(Packet*); @@ -216,8 +219,6 @@ private: int active_tunnel_bypass; const char* drop_reason; - bool prevent_trust_action; - // these can't be pkt flags because we do the handling // of these flags following all processing and the drop // or response may have been produced by a pseudopacket.