From: Davis McPherson -X (davmcphe - XORIANT CORPORATION at Cisco) Date: Fri, 9 May 2025 20:24:39 +0000 (+0000) Subject: Pull request #4719: flow: implement a per flow check of the packet timestamp and... X-Git-Tag: 3.8.1.0~19 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=6f43f108cd67db2c6b3d1aa0943c530fbd9acbf3;p=thirdparty%2Fsnort3.git Pull request #4719: flow: implement a per flow check of the packet timestamp and drop packets if the timestamp is earlier than the timestamp of the previous packet Merge in SNORT/snort3 from ~DAVMCPHE/snort3:drop_stale_packets to master Squashed commit of the following: commit 27a0456758a6713b2c5cdc94f3d2c59eaa9aa9dc Author: davis mcpherson Date: Mon May 5 23:18:17 2025 -0400 snort2lua: add include for cstdint to provide standard c++ integer types commit 63de2df3d4e5c871a0069b646c0a5c06588d9aa7 Author: davis mcpherson Date: Fri Apr 4 14:45:29 2025 -0400 flow: implement a per flow check of the packet timestamp and drop packets if the timestamp is earlier than the timestamp of the previous packet flow: always count stale packets, only drop if that is enabled by config, set default value for drop_stale_packets to false (disabled) --- diff --git a/src/flow/flow.h b/src/flow/flow.h index 96be826f5..ea789acaa 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -479,6 +479,8 @@ public: // FIXIT-M privatize if possible uint64_t expire_time = 0; unsigned network_policy_id = 0; + struct timeval prev_packet_time = {0, 0}; + unsigned inspection_policy_id = 0; unsigned ips_policy_id = 0; unsigned reload_id = 0; diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index 9d03eb64e..4de64434e 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -20,6 +20,8 @@ #include "config.h" #endif +#include + #include #include "flow_control.h" @@ -35,6 +37,7 @@ #include "pub_sub/intrinsic_event_ids.h" #include "pub_sub/packet_events.h" #include "stream/stream.h" +#include "utils/stats.h" #include "utils/util.h" #include "expect_cache.h" @@ -401,6 +404,36 @@ static bool want_flow(PktType type, Packet* p) return false; } +static void log_stale_packet(snort::Packet *p, snort::Flow *flow, bool drop_packet) +{ + char ts_flow[TIMEBUF_SIZE]; + char ts_pkt[TIMEBUF_SIZE]; + ts_print((const struct timeval *)&p->pkth->ts, ts_pkt); + ts_print((const struct timeval *)&flow->prev_packet_time, ts_flow); + + if ( drop_packet ) + PacketTracer::log("Flow: Dropping stale packet. current packet ts: %s < previous packet ts: %s.\n", + ts_pkt, ts_flow); + else + PacketTracer::log("Flow: Detected stale packet, dropping disabled. current packet ts: %s < previous packet ts: %s.\n", + ts_pkt, ts_flow); +} + +static inline bool is_packet_stale(const Flow* flow, const Packet* p) +{ + return timercmp(&flow->prev_packet_time, &p->pkth->ts, >); +} + +static void drop_stale_packet(snort::Packet *p, snort::Flow *flow) +{ + // This is a stale packet, ignore it. + p->active->set_drop_reason("snort"); + p->active->drop_packet(p); + p->disable_inspect = true; + if ( PacketTracer::is_active() ) + log_stale_packet(p, flow, true); +} + bool FlowControl::process(PktType type, Packet* p, bool* new_flow) { if ( !get_proto_session[to_utype(type)] ) @@ -410,8 +443,26 @@ bool FlowControl::process(PktType type, Packet* p, bool* new_flow) bool reversed = set_key(&key, p); Flow* flow = cache->find(&key); - if (flow) + if ( flow ) + { + if ( !p->is_retry() and is_packet_stale(flow, p) ) + { + flow->session->count_stale_packet(); + + if ( p->context->conf->drop_stale_packets() ) + { + drop_stale_packet(p, flow); + return true; + } + else + { + if ( PacketTracer::is_active() ) + log_stale_packet(p, flow, false); + } + } + flow = stale_flow_cleanup(cache, flow, p); + } bool new_ha_flow = false; if ( !flow ) @@ -469,6 +520,7 @@ unsigned FlowControl::process(Flow* flow, Packet* p, bool new_ha_flow) unsigned news = 0; flow->previous_ssn_state = flow->ssn_state; + flow->prev_packet_time = p->pkth->ts; p->flow = flow; p->disable_inspect = flow->is_inspection_disabled(); diff --git a/src/flow/session.h b/src/flow/session.h index 79b894cea..8fe000389 100644 --- a/src/flow/session.h +++ b/src/flow/session.h @@ -79,6 +79,7 @@ public: virtual uint8_t missing_in_reassembled(uint8_t /*dir*/) const { return SSN_MISSING_NONE; } virtual bool set_packet_action_to_hold(snort::Packet*) { return false; } + virtual void count_stale_packet() { } protected: Session(snort::Flow* f) { flow = f; } @@ -96,7 +97,8 @@ public: { CountType::SUM, "created", module " session trackers created" }, \ { CountType::SUM, "released", module " session trackers released" }, \ { CountType::SUM, "timeouts", module " session timeouts" }, \ - { CountType::SUM, "prunes", module " session prunes" } + { CountType::SUM, "prunes", module " session prunes" }, \ + { CountType::SUM, "stale_packets", module " stale packets" } // See above. Add to end of stats array. #define SESSION_STATS \ @@ -105,7 +107,8 @@ public: PegCount created; \ PegCount released; \ PegCount timeouts; \ - PegCount prunes + PegCount prunes; \ + PegCount stale_packets // Do not change the semantics of max. Max = the highest seen during the perf interval. // To obtain max over the entire run, determine the maximum of reported max pegs. diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 2ad175ec8..5e83dc2f4 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -105,6 +105,7 @@ void Flow::set_client_initiate(Packet*) { } void Flow::set_direction(Packet*) { } void Flow::set_mpls_layer_per_dir(Packet*) { } void packet_gettimeofday(struct timeval* ) { } +SO_PUBLIC void ts_print(const struct timeval*, char*, bool) { } time_t packet_time() { return 0; } diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index 707c83f97..db361d134 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -88,6 +88,7 @@ bool ExpectCache::check(Packet*, Flow*) { return true; } Flow* HighAvailabilityManager::import(Packet&, FlowKey&) { return nullptr; } bool FlowCache::move_to_allowlist(snort::Flow*) { return true; } uint64_t FlowCache::get_lru_flow_count(uint8_t) const { return 0; } +SO_PUBLIC void snort::ts_print(const struct timeval*, char*, bool) { } namespace snort { diff --git a/src/main/snort_config.h b/src/main/snort_config.h index c35260cad..015bb2f8b 100644 --- a/src/main/snort_config.h +++ b/src/main/snort_config.h @@ -78,6 +78,7 @@ enum RunFlag #ifdef SHELL RUN_FLAG__SHELL = 0x02000000, #endif + RUN_FLAG__DROP_STALE_PACKETS = 0x04000000, }; enum OutputFlag @@ -675,6 +676,9 @@ public: bool ip_frags_only() const { return (run_flags & RUN_FLAG__IP_FRAGS_ONLY) != 0; } + bool drop_stale_packets() const + { return (run_flags & RUN_FLAG__DROP_STALE_PACKETS) != 0; } + void clear_run_flags(RunFlag flag) { run_flags &= ~flag; } diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc index 8d2fe1692..ce3ad5f39 100644 --- a/src/stream/base/stream_module.cc +++ b/src/stream/base/stream_module.cc @@ -113,6 +113,9 @@ static const Parameter s_params[] = { "allowlist_cache", Parameter::PT_TABLE, allowlist_cache_params, nullptr, "configure allowlist cache" }, + { "drop_stale_packets", Parameter::PT_BOOL, nullptr, "false", + "enable dropping of packets with stale timestamp" }, + FLOW_TYPE_TABLE("ip_cache", "ip", ip_params), FLOW_TYPE_TABLE("icmp_cache", "icmp", icmp_params), FLOW_TYPE_TABLE("tcp_cache", "tcp", tcp_params), @@ -433,6 +436,15 @@ bool StreamModule::set(const char* fqn, Value& v, SnortConfig* c) else if ( !strcmp(fqn, "stream.udp_cache.idle_timeout") ) config.flow_cache_cfg.proto[to_utype(PktType::UDP)].nominal_timeout = v.get_uint32(); + else if ( v.is("drop_stale_packets") ) + { + config.drop_stale_packets = v.get_bool(); + if (config.drop_stale_packets) + c->set_run_flags(RUN_FLAG__DROP_STALE_PACKETS); + else + c->clear_run_flags(RUN_FLAG__DROP_STALE_PACKETS); + return true; + } else { assert(!strcmp(fqn, "stream.user_cache.idle_timeout")); @@ -601,7 +613,8 @@ void StreamModuleConfig::show() const ConfigLogger::log_value("pruning_timeout", flow_cache_cfg.pruning_timeout); ConfigLogger::log_value("prune_flows", flow_cache_cfg.prune_flows); ConfigLogger::log_limit("require_3whs", hs_timeout, -1, hs_timeout < 0 ? hs_timeout : -1); - + ConfigLogger::log_value("drop_stale_packets", drop_stale_packets ? "enabled" : "disabled"); + for (int i = to_utype(PktType::IP); i < to_utype(PktType::PDU); ++i) { std::string tmp; diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index 50e8f9e72..1a1b78d27 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -125,7 +125,8 @@ struct StreamModuleConfig #endif uint32_t held_packet_timeout = 1000; // in milliseconds int hs_timeout = -1; - + bool drop_stale_packets = false; + void show() const; }; diff --git a/src/stream/icmp/icmp_session.h b/src/stream/icmp/icmp_session.h index 07aa405a1..9b126cb05 100644 --- a/src/stream/icmp/icmp_session.h +++ b/src/stream/icmp/icmp_session.h @@ -20,8 +20,11 @@ #define ICMP_SESSION_H #include + #include "flow/session.h" +#include "icmp_module.h" + class IcmpSession : public Session { public: @@ -31,7 +34,8 @@ public: bool setup(snort::Packet*) override; int process(snort::Packet*) override; void clear() override; - + void count_stale_packet() override + { icmpStats.stale_packets++; } public: uint32_t echo_count = 0; struct timeval ssn_time = {}; diff --git a/src/stream/ip/ip_session.h b/src/stream/ip/ip_session.h index cfc285e6c..a5c4832a9 100644 --- a/src/stream/ip/ip_session.h +++ b/src/stream/ip/ip_session.h @@ -25,6 +25,8 @@ struct Fragment; struct FragEngine; +extern THREAD_LOCAL IpStats ip_stats; + /* Only track a certain number of alerts per session */ #define MAX_FRAG_ALERTS 8 @@ -84,12 +86,12 @@ public: bool add_alert(snort::Packet*, uint32_t gid, uint32_t sid) override; bool check_alerted(snort::Packet*, uint32_t gid, uint32_t sid) override; + void count_stale_packet() override + { ip_stats.stale_packets++; } public: FragTracker tracker = {}; }; -extern THREAD_LOCAL IpStats ip_stats; - #endif diff --git a/src/stream/tcp/tcp_session.h b/src/stream/tcp/tcp_session.h index cf74001fb..e83939f16 100644 --- a/src/stream/tcp/tcp_session.h +++ b/src/stream/tcp/tcp_session.h @@ -119,6 +119,9 @@ public: void handle_data_segment(TcpSegmentDescriptor&, bool flush = true); bool validate_packet_established_session(TcpSegmentDescriptor&); + void count_stale_packet() override + { tcpStats.stale_packets++; } + TcpStreamTracker client; TcpStreamTracker server; TcpStreamConfig* tcp_config = nullptr; diff --git a/src/stream/udp/udp_session.cc b/src/stream/udp/udp_session.cc index 5abe5c203..e03bdd9de 100644 --- a/src/stream/udp/udp_session.cc +++ b/src/stream/udp/udp_session.cc @@ -194,3 +194,7 @@ int UdpSession::process(Packet* p) return 0; } +void UdpSession::count_stale_packet() +{ + udpStats.stale_packets++; +} diff --git a/src/stream/udp/udp_session.h b/src/stream/udp/udp_session.h index 3884c3acf..6e5b43128 100644 --- a/src/stream/udp/udp_session.h +++ b/src/stream/udp/udp_session.h @@ -25,6 +25,7 @@ #include "flow/session.h" class SO_PUBLIC UdpSession : public Session + { public: UdpSession(snort::Flow*); @@ -34,8 +35,9 @@ public: void update_direction(char dir, const snort::SfIp*, uint16_t port) override; int process(snort::Packet*) override; void clear() override; + void count_stale_packet() override; -public: + public: struct timeval ssn_time = {}; uint64_t payload_bytes_seen_client = 0; uint64_t payload_bytes_seen_server = 0; diff --git a/tools/snort2lua/config_states/config_ignore_ports.cc b/tools/snort2lua/config_states/config_ignore_ports.cc index 4ddfebb5d..9e19eb976 100644 --- a/tools/snort2lua/config_states/config_ignore_ports.cc +++ b/tools/snort2lua/config_states/config_ignore_ports.cc @@ -17,6 +17,7 @@ //-------------------------------------------------------------------------- // config_ignore_ports.cc author Josh Rosenbaum +#include #include #include #include diff --git a/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc b/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc index 2a41decb8..c752da4d5 100644 --- a/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc +++ b/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc @@ -23,6 +23,7 @@ #include "pps_dcerpc_server.h" +#include #include #include diff --git a/tools/snort2lua/preprocessor_states/pps_frag3_engine.cc b/tools/snort2lua/preprocessor_states/pps_frag3_engine.cc index d5c1fcf4d..277294249 100644 --- a/tools/snort2lua/preprocessor_states/pps_frag3_engine.cc +++ b/tools/snort2lua/preprocessor_states/pps_frag3_engine.cc @@ -17,6 +17,7 @@ //-------------------------------------------------------------------------- // pps_frag3_engine.cc author Josh Rosenbaum +#include #include #include diff --git a/tools/snort2lua/preprocessor_states/pps_stream5_tcp.cc b/tools/snort2lua/preprocessor_states/pps_stream5_tcp.cc index 1e97a58ad..1ce62689d 100644 --- a/tools/snort2lua/preprocessor_states/pps_stream5_tcp.cc +++ b/tools/snort2lua/preprocessor_states/pps_stream5_tcp.cc @@ -17,6 +17,7 @@ //-------------------------------------------------------------------------- // pps_stream_tcp.cc author Josh Rosenbaum +#include #include #include #include diff --git a/tools/snort2lua/rule_states/rule_gid_sid.cc b/tools/snort2lua/rule_states/rule_gid_sid.cc index 7f2b84ae4..44404a3cc 100644 --- a/tools/snort2lua/rule_states/rule_gid_sid.cc +++ b/tools/snort2lua/rule_states/rule_gid_sid.cc @@ -25,6 +25,7 @@ // sid. // Handle 2 cases: sid was read before/after gid. +#include #include #include