From: Naveen Gujje (ngujje) Date: Wed, 28 Oct 2020 05:24:57 +0000 (+0000) Subject: Merge pull request #2411 in SNORT/snort3 from ~KBHANDAN/snort3:cant_drop_keep_flow... X-Git-Tag: 3.0.3-5~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8d4a6c7d3688be80f85b4a400e34b808679ee8a0;p=thirdparty%2Fsnort3.git Merge pull request #2411 in SNORT/snort3 from ~KBHANDAN/snort3:cant_drop_keep_flow to master Squashed commit of the following: commit 6e55f9f908a913e223d29a5dc7c6722a15927437 Author: Kaushal Bhandankar Date: Wed Aug 19 15:09:58 2020 -0400 flow: do not remove the flow during pruning/reload during IPS event with block action --- diff --git a/src/detection/detection_engine.cc b/src/detection/detection_engine.cc index 95ded1d87..0a4a0c9da 100644 --- a/src/detection/detection_engine.cc +++ b/src/detection/detection_engine.cc @@ -218,7 +218,15 @@ void DetectionEngine::finish_inspect(Packet* p, bool inspected) log_events(p); if ( p->active ) + { + if ( p->active->session_was_blocked() and ( p->active->keep_pruned_flow() or + ( p->active->keep_timedout_flow() and ( p->is_tcp() or p->pseudo_type == PSEUDO_PKT_TCP ) ) ) ) + { + p->flow->ssn_state.session_flags |= SSNFLAG_KEEP_FLOW; + } + p->active->apply_delayed_action(p); + } p->context->post_detection(); diff --git a/src/flow/flow.cc b/src/flow/flow.cc index 16810e6f6..a3d724e20 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -141,6 +141,28 @@ inline void Flow::clean() filtering_state.clear(); } +void Flow::flush(bool do_cleanup) +{ + if ( session ) + { + DetectionEngine::onload(this); + + if ( do_cleanup ) + { + DetectionEngine::set_next_packet(); + DetectionEngine de; + + session->flush(); + de.get_context()->clear_callbacks(); + } + else + session->clear(); + } + + if ( was_blocked() ) + free_flow_data(); +} + void Flow::reset(bool do_cleanup) { if ( session ) diff --git a/src/flow/flow.h b/src/flow/flow.h index f6d3d766e..e90ee0686 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -77,6 +77,7 @@ #define SSNFLAG_ABORT_SERVER 0x20000000 #define SSNFLAG_HARD_EXPIRATION 0x40000000 +#define SSNFLAG_KEEP_FLOW 0x80000000 #define SSNFLAG_NONE 0x00000000 /* nothing, an MT bag of chips */ @@ -93,8 +94,8 @@ #define STREAM_STATE_MIDSTREAM 0x0040 #define STREAM_STATE_TIMEDOUT 0x0080 #define STREAM_STATE_UNREACH 0x0100 -#define STREAM_STATE_CLOSED 0x0800 -#define STREAM_STATE_BLOCK_PENDING 0x1000 +#define STREAM_STATE_CLOSED 0x0200 +#define STREAM_STATE_BLOCK_PENDING 0x0400 class BitOp; class Session; @@ -172,6 +173,7 @@ public: void init(PktType); void term(); + void flush(bool do_cleanup = true); void reset(bool do_cleanup = true); void restart(bool dump_flow_data = true); void clear(bool dump_flow_data = true); @@ -230,6 +232,9 @@ public: uint32_t clear_session_flags(uint32_t ssn_flags) { return ssn_state.session_flags &= ~ssn_flags; } + uint32_t clear_session_state(uint32_t ssn_state) + { return session_state &= ~ssn_state; } + void set_to_client_detection(bool enable); void set_to_server_detection(bool enable); diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc index ee29e4335..e46324a6e 100644 --- a/src/flow/flow_cache.cc +++ b/src/flow/flow_cache.cc @@ -24,12 +24,14 @@ #include "flow/flow_cache.h" +#include "detection/detection_engine.h" #include "hash/hash_defs.h" #include "hash/zhash.h" #include "helpers/flag_context.h" #include "ips_options/ips_flowbits.h" #include "memory/memory_cap.h" #include "packet_io/active.h" +#include "packet_tracer/packet_tracer.h" #include "time/packet_time.h" #include "utils/stats.h" @@ -170,11 +172,22 @@ void FlowCache::remove(Flow* flow) memory::MemoryCap::update_deallocations(config.proto[to_utype(flow->key->pkt_type)].cap_weight); } -void FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup) +bool FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup) { + if ( !flow->was_blocked() ) + { + flow->flush(do_cleanup); + if ( flow->ssn_state.session_flags & SSNFLAG_KEEP_FLOW ) + { + flow->ssn_state.session_flags &= ~SSNFLAG_KEEP_FLOW; + return false; + } + } + flow->reset(do_cleanup); prune_stats.update(reason); remove(flow); + return true; } void FlowCache::retire(Flow* flow) @@ -192,36 +205,43 @@ unsigned FlowCache::prune_stale(uint32_t thetime, const Flow* save_me) unsigned pruned = 0; auto flow = static_cast(hash_table->lru_first()); - while ( flow and pruned <= cleanup_flows ) { -#if 0 - // FIXIT-RC this loops forever if 1 flow in cache - if (flow == save_me) + PacketTracerSuspend pt_susp; + + while ( flow and pruned <= cleanup_flows ) { - break; - if ( hash_table->get_count() == 1 ) +#if 0 + // FIXIT-RC this loops forever if 1 flow in cache + if (flow == save_me) + { break; + if ( hash_table->get_count() == 1 ) + break; - hash_table->lru_touch(); - } + hash_table->lru_touch(); + } #else - // Reached the current flow. This *should* be the newest flow - if ( flow == save_me ) - break; + // Reached the current flow. This *should* be the newest flow + if ( flow == save_me ) + break; #endif - if ( flow->is_suspended() ) - break; + if ( flow->is_suspended() ) + break; - if ( flow->last_data_seen + config.pruning_timeout >= thetime ) - break; + if ( flow->last_data_seen + config.pruning_timeout >= thetime ) + break; - flow->ssn_state.session_flags |= SSNFLAG_TIMEDOUT; - release(flow, PruneReason::IDLE); - ++pruned; + flow->ssn_state.session_flags |= SSNFLAG_TIMEDOUT; + if ( release(flow, PruneReason::IDLE) ) + ++pruned; - flow = static_cast(hash_table->lru_first()); + flow = static_cast(hash_table->lru_first()); + } } + if ( PacketTracer::is_active() and pruned ) + PacketTracer::log("Flow: Pruned %u flows\n", pruned); + return pruned; } @@ -235,24 +255,31 @@ unsigned FlowCache::prune_unis(PktType pkt_type) unsigned pruned = 0; FlowUniList* uni_list; - if ( pkt_type == PktType::IP ) - uni_list = uni_ip_flows; - else - uni_list = uni_flows; - - Flow* flow = uni_list->get_oldest_uni(); - while ( (uni_list->get_count() > max_uni) && flow && (pruned < cleanup_flows) ) { - Flow* prune_me = flow; - flow = uni_list->get_prev(prune_me); + PacketTracerSuspend pt_susp; - if ( prune_me->was_blocked() ) - continue; + if ( pkt_type == PktType::IP ) + uni_list = uni_ip_flows; + else + uni_list = uni_flows; + + Flow* flow = uni_list->get_oldest_uni(); + while ( (uni_list->get_count() > max_uni) && flow && (pruned < cleanup_flows) ) + { + Flow* prune_me = flow; + flow = uni_list->get_prev(prune_me); - release(prune_me, PruneReason::UNI); - ++pruned; + if ( prune_me->was_blocked() ) + continue; + + if ( release(prune_me, PruneReason::UNI) ) + ++pruned; + } } + if ( PacketTracer::is_active() and pruned ) + PacketTracer::log("Flow: Pruned %u flows\n", pruned); + return pruned; } @@ -264,44 +291,51 @@ unsigned FlowCache::prune_excess(const Flow* save_me) assert(max_cap > 0); unsigned pruned = 0; - unsigned blocks = 0; // initially skip offloads but if that doesn't work the hash table is iterated from the // beginning again. prune offloads at that point. unsigned ignore_offloads = hash_table->get_num_nodes(); - while ( hash_table->get_num_nodes() > max_cap and hash_table->get_num_nodes() > blocks ) { - auto flow = static_cast(hash_table->lru_first()); - assert(flow); // holds true because hash_table->get_count() > 0 + PacketTracerSuspend pt_susp; + unsigned blocks = 0; - if ( (save_me and flow == save_me) or flow->was_blocked() or - (flow->is_suspended() and ignore_offloads) ) + while ( hash_table->get_num_nodes() > max_cap and hash_table->get_num_nodes() > blocks ) { - // check for non-null save_me above to silence analyzer - // "called C++ object pointer is null" here - if ( flow->was_blocked() ) - ++blocks; - - // FIXIT-M we should update last_data_seen upon touch to ensure - // the hash_table LRU list remains sorted by time - hash_table->lru_touch(); + auto flow = static_cast(hash_table->lru_first()); + assert(flow); // holds true because hash_table->get_count() > 0 + + if ( (save_me and flow == save_me) or flow->was_blocked() or + (flow->is_suspended() and ignore_offloads) ) + { + // check for non-null save_me above to silence analyzer + // "called C++ object pointer is null" here + if ( flow->was_blocked() ) + ++blocks; + + // FIXIT-M we should update last_data_seen upon touch to ensure + // the hash_table LRU list remains sorted by time + hash_table->lru_touch(); + } + else + { + flow->ssn_state.session_flags |= SSNFLAG_PRUNED; + if ( release(flow, PruneReason::EXCESS) ) + ++pruned; + } + if ( ignore_offloads > 0 ) + --ignore_offloads; } - else + + if (!pruned and hash_table->get_num_nodes() > max_cap) { - flow->ssn_state.session_flags |= SSNFLAG_PRUNED; - release(flow, PruneReason::EXCESS); + prune_one(PruneReason::EXCESS, true); ++pruned; } - if ( ignore_offloads > 0 ) - --ignore_offloads; } - if (!pruned and hash_table->get_num_nodes() > max_cap) - { - prune_one(PruneReason::EXCESS, true); - ++pruned; - } + if ( PacketTracer::is_active() and pruned ) + PacketTracer::log("Flow: Pruned %u flows\n", pruned); return pruned; } @@ -325,38 +359,45 @@ bool FlowCache::prune_one(PruneReason reason, bool do_cleanup) unsigned FlowCache::timeout(unsigned num_flows, time_t thetime) { ActiveSuspendContext act_susp(Active::ASP_TIMEOUT); + unsigned retired = 0; - auto flow = static_cast(hash_table->lru_current()); + { + PacketTracerSuspend pt_susp; - if ( !flow ) - flow = static_cast(hash_table->lru_first()); + auto flow = static_cast(hash_table->lru_current()); - while ( flow and (retired < num_flows) ) - { - if ( flow->is_hard_expiration() ) - { - if ( flow->expire_time > (uint64_t) thetime ) - break; - } - else if ( flow->last_data_seen + config.proto[to_utype(flow->key->pkt_type)].nominal_timeout > thetime ) - break; + if ( !flow ) + flow = static_cast(hash_table->lru_first()); - if ( HighAvailabilityManager::in_standby(flow) or - flow->is_suspended() ) + while ( flow and (retired < num_flows) ) { - flow = static_cast(hash_table->lru_next()); - continue; - } + if ( flow->is_hard_expiration() ) + { + if ( flow->expire_time > (uint64_t) thetime ) + break; + } + else if ( flow->last_data_seen + config.proto[to_utype(flow->key->pkt_type)].nominal_timeout > thetime ) + break; - flow->ssn_state.session_flags |= SSNFLAG_TIMEDOUT; - release(flow, PruneReason::IDLE); + if ( HighAvailabilityManager::in_standby(flow) or + flow->is_suspended() ) + { + flow = static_cast(hash_table->lru_next()); + continue; + } - ++retired; + flow->ssn_state.session_flags |= SSNFLAG_TIMEDOUT; + if ( release(flow, PruneReason::IDLE) ) + ++retired; - flow = static_cast(hash_table->lru_current()); + flow = static_cast(hash_table->lru_current()); + } } + if ( PacketTracer::is_active() and retired ) + PacketTracer::log("Flow: Timed out %u flows\n", retired); + return retired; } @@ -368,7 +409,7 @@ unsigned FlowCache::delete_active_flows(unsigned mode, unsigned num_to_delete, u auto flow = static_cast(hash_table->lru_first()); assert(flow); if ( (mode == ALLOWED_FLOWS_ONLY and (flow->was_blocked() || flow->is_suspended())) - or (mode == OFFLOADED_FLOWS_TOO and flow->was_blocked()) ) + or (mode == OFFLOADED_FLOWS_TOO and flow->was_blocked()) ) { hash_table->lru_touch(); continue; @@ -404,25 +445,32 @@ unsigned FlowCache::delete_flows(unsigned num_to_delete) unsigned deleted = 0; - // delete from the free list first... - while ( num_to_delete ) { - Flow* flow = (Flow*)hash_table->pop(); - if ( !flow ) - break; + PacketTracerSuspend pt_susp; - delete flow; - delete_stats.update(FlowDeleteState::FREELIST); - memory::MemoryCap::update_deallocations(sizeof(HashNode) + sizeof(FlowKey)); + // delete from the free list first... + while ( num_to_delete ) + { + Flow* flow = (Flow*)hash_table->pop(); + if ( !flow ) + break; - --flows_allocated; - ++deleted; - --num_to_delete; + delete flow; + delete_stats.update(FlowDeleteState::FREELIST); + memory::MemoryCap::update_deallocations(sizeof(HashNode) + sizeof(FlowKey)); + + --flows_allocated; + ++deleted; + --num_to_delete; + } + + unsigned mode = ALLOWED_FLOWS_ONLY; + while ( num_to_delete && mode <= ALL_FLOWS ) + num_to_delete = delete_active_flows(mode++, num_to_delete, deleted); } - unsigned mode = ALLOWED_FLOWS_ONLY; - while ( num_to_delete && mode <= ALL_FLOWS ) - num_to_delete = delete_active_flows(mode++, num_to_delete, deleted); + if ( PacketTracer::is_active() and deleted ) + PacketTracer::log("Flow: Deleted %u flows\n", deleted); return deleted; } diff --git a/src/flow/flow_cache.h b/src/flow/flow_cache.h index f84d1fbcd..750569c98 100644 --- a/src/flow/flow_cache.h +++ b/src/flow/flow_cache.h @@ -53,7 +53,7 @@ public: snort::Flow* find(const snort::FlowKey*); snort::Flow* allocate(const snort::FlowKey*); - void release(snort::Flow*, PruneReason = PruneReason::NONE, bool do_cleanup = true); + bool release(snort::Flow*, PruneReason = PruneReason::NONE, bool do_cleanup = true); unsigned prune_stale(uint32_t thetime, const snort::Flow* save_me); unsigned prune_excess(const snort::Flow* save_me); diff --git a/src/flow/session.h b/src/flow/session.h index 0fcd598fb..ac4d478a0 100644 --- a/src/flow/session.h +++ b/src/flow/session.h @@ -59,6 +59,7 @@ public: snort::Packet*, uint32_t /*gid*/, uint32_t /*sid*/, uint32_t /*event_id*/, uint32_t /*event_second*/) { return 0; } + virtual void flush() { } virtual void flush_client(snort::Packet*) { } virtual void flush_server(snort::Packet*) { } virtual void flush_talker(snort::Packet*, bool /*final_flush */ = false) { } diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index fe5936835..ce16061be 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -60,6 +60,8 @@ void PacketTracer::log(const char*, ...) { } void PacketTracer::open_file() { } void PacketTracer::dump_to_daq(Packet*) { } void PacketTracer::reset() { } +void PacketTracer::pause() { } +void PacketTracer::unpause() { } void Active::set_drop_reason(char const*) { } Packet::Packet(bool) { } Packet::~Packet() { } @@ -70,6 +72,7 @@ ExpectCache::~ExpectCache() { } DetectionEngine::~DetectionEngine() { } void Flow::init(PktType) { } void Flow::term() { } +void Flow::flush(bool) { } void Flow::reset(bool) { } void Flow::free_flow_data() { } void set_network_policy(const SnortConfig*, unsigned) { } diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index 09cdb7de7..5515374f1 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -169,7 +169,7 @@ int ExpectCache::add_flow(const Packet*, return 1; } -void FlowCache::release(Flow*, PruneReason, bool) { } +bool FlowCache::release(Flow*, PruneReason, bool) { } TEST_GROUP(stale_flow) { }; diff --git a/src/network_inspectors/packet_tracer/packet_tracer.h b/src/network_inspectors/packet_tracer/packet_tracer.h index 1477e798d..6aaf92863 100644 --- a/src/network_inspectors/packet_tracer/packet_tracer.h +++ b/src/network_inspectors/packet_tracer/packet_tracer.h @@ -112,6 +112,14 @@ SO_PUBLIC extern THREAD_LOCAL PacketTracer* s_pkt_trace; inline bool PacketTracer::is_active() { return s_pkt_trace ? s_pkt_trace->active : false; } -} +struct PacketTracerSuspend +{ + PacketTracerSuspend() + { PacketTracer::pause(); } + ~PacketTracerSuspend() + { PacketTracer::unpause(); } +}; + +} #endif diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index 0eb6a8085..de39b0e4b 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -549,14 +549,12 @@ void Active::update_status(const Packet* p, bool force) { update_status_actionable(p); - if(!active_status) + if ( !active_status ) cant_drop(); } - else if ( force ) active_status = AST_FORCE; - - else if ( active_status != AST_FORCE) + else if ( active_status != AST_FORCE ) { update_status_actionable(p); } @@ -568,7 +566,7 @@ void Active::daq_update_status(const Packet* p) { update_status_actionable(p); - if(!active_status) + if ( !active_status ) cant_drop(); } else if ( active_status != AST_FORCE ) @@ -838,7 +836,7 @@ void Active::map_drop_reason_id(const char* verdict_reason, uint8_t id) void Active::set_drop_reason(const char* reason) { - if ( drop_reason == nullptr ) + if ( !drop_reason and !is_suspended() ) drop_reason = reason; } @@ -853,7 +851,7 @@ int Active::get_drop_reason_id() void Active::send_reason_to_daq(Packet& p) { - if ( drop_reason == nullptr ) + if ( !drop_reason ) return; int reason = get_drop_reason_id(); diff --git a/src/packet_io/active.h b/src/packet_io/active.h index 20cd1facc..051b17a48 100644 --- a/src/packet_io/active.h +++ b/src/packet_io/active.h @@ -73,6 +73,9 @@ public: s_suspend_reason = suspend_reason; } + static bool is_suspended() + { return s_suspend; } + static void resume() { s_suspend = false; @@ -153,6 +156,12 @@ public: bool can_partial_block_session() const { return active_status == AST_CANT and s_suspend_reason > ASP_NONE and s_suspend_reason != ASP_TIMEOUT; } + bool keep_pruned_flow() const + { return ( s_suspend_reason == ASP_PRUNE ) or ( s_suspend_reason == ASP_RELOAD ); } + + bool keep_timedout_flow() const + { return ( s_suspend_reason == ASP_TIMEOUT ); } + bool packet_retry_requested() const { return active_action == ACT_RETRY; } diff --git a/src/stream/stream.cc b/src/stream/stream.cc index 90f5781a9..2d106997b 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -225,7 +225,7 @@ void Stream::check_flow_closed(Packet* p) if (PacketTracer::is_active()) PacketTracer::log("Stream: pending block, drop\n"); } - flow->session_state &= ~STREAM_STATE_BLOCK_PENDING; + flow->clear_session_state(STREAM_STATE_BLOCK_PENDING); } } @@ -621,7 +621,7 @@ static void active_response(Packet* p, Flow* lwssn) ++lwssn->response_count; lwssn->set_expire(p, delay); - lwssn->session_state &= ~(STREAM_STATE_DROP_CLIENT|STREAM_STATE_DROP_SERVER); + lwssn->clear_session_state(STREAM_STATE_DROP_CLIENT|STREAM_STATE_DROP_SERVER); } } diff --git a/src/stream/tcp/tcp_ha.cc b/src/stream/tcp/tcp_ha.cc index 1ad16a654..edc32c4a9 100644 --- a/src/stream/tcp/tcp_ha.cc +++ b/src/stream/tcp/tcp_ha.cc @@ -50,8 +50,8 @@ void TcpHA::deactivate_session(Flow* flow) if ( flow->session ) ((TcpSession*)(flow->session))->clear_session(true, true, false); - flow->session_state &= ~( STREAM_STATE_SYN | STREAM_STATE_SYN_ACK | - STREAM_STATE_ACK | STREAM_STATE_ESTABLISHED ); + flow->clear_session_state(STREAM_STATE_SYN | STREAM_STATE_SYN_ACK | + STREAM_STATE_ACK | STREAM_STATE_ESTABLISHED); assert( flow->ha_state ); flow->clear_session_flags( SSNFLAG_SEEN_CLIENT | SSNFLAG_SEEN_SERVER ); diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index 3d1a4e091..7d622cf98 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -911,6 +911,22 @@ void TcpSession::flush_talker(Packet* p, bool final_flush) flush_tracker( client, p, PKT_FROM_SERVER, final_flush); } +void TcpSession::flush() +{ + if ( !tcp_init ) + return; + + //FIXIT-L Cleanup tcp_init and lws_init as they have some side effect in TcpSession::clear_session + lws_init = false; + tcp_init = false; + + client.reassembler.flush_queued_segments(flow, true); + server.reassembler.flush_queued_segments(flow, true); + + lws_init = true; + tcp_init = true; +} + void TcpSession::set_extra_data(Packet* p, uint32_t xid) { TcpStreamTracker& st = p->ptrs.ip_api.get_src()->equals(flow->client_ip) ? server : client; @@ -1099,17 +1115,3 @@ int TcpSession::process(Packet* p) return process_tcp_packet(tsd, p); } - -void TcpSession::flush() -{ - if ( ( server.reassembler.is_segment_pending_flush() ) || - (client.reassembler.is_segment_pending_flush() ) ) - { - // FIXIT-L flush_queued_segments is basically a noop when the 'clear' parameter - // is passed in as false... review what happens in 2.9.x probably related - // to ssl encryption support - server.reassembler.flush_queued_segments(flow, false); - client.reassembler.flush_queued_segments(flow, false); - } -} - diff --git a/src/stream/tcp/tcp_stream_session.h b/src/stream/tcp/tcp_stream_session.h index b9aa98ccd..6ad83b3ff 100644 --- a/src/stream/tcp/tcp_stream_session.h +++ b/src/stream/tcp/tcp_stream_session.h @@ -70,7 +70,6 @@ public: virtual void update_perf_base_state(char) = 0; virtual void clear_session( bool free_flow_data, bool flush_segments, bool restart, snort::Packet* p = nullptr) = 0; - virtual void flush() = 0; virtual TcpStreamTracker::TcpState get_talker_state(TcpSegmentDescriptor&) = 0; virtual TcpStreamTracker::TcpState get_listener_state(TcpSegmentDescriptor&) = 0; TcpStreamTracker::TcpState get_peer_state(TcpStreamTracker* me)