From: Ron Dempster (rdempste) Date: Thu, 27 Jun 2024 17:57:10 +0000 (+0000) Subject: Pull request #4363: flow: handle significant groups with unknown group value as non... X-Git-Tag: 3.3.1.0~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27e9b2d8aae78043e3f14776cac2368124eefb52;p=thirdparty%2Fsnort3.git Pull request #4363: flow: handle significant groups with unknown group value as non-group flow keys Merge in SNORT/snort3 from ~RDEMPSTE/snort3:expected_unknown to master Squashed commit of the following: commit f2ff9af2a7393939742716c535411448fd557c27 Author: Ron Dempster (rdempste) Date: Tue May 21 17:09:24 2024 -0400 flow: clear flow stash when freeing the flow data commit c4282b8aa7ba3743b4413e2bbe6dc94959fb8e49 Author: Ron Dempster (rdempste) Date: Tue Jun 25 11:28:27 2024 -0400 flow: handle significant groups with unknown group value as non-group flow keys --- diff --git a/src/flow/expect_cache.cc b/src/flow/expect_cache.cc index 53a612819..1c833b2b4 100644 --- a/src/flow/expect_cache.cc +++ b/src/flow/expect_cache.cc @@ -338,13 +338,10 @@ int ExpectCache::add_flow(const Packet *ctrlPkt, PktType type, IpProtocol ip_pro // This code assumes that the expected session is in the opposite direction of the control session // when groups are significant - bool reversed_key = (ctrlPkt->pkth->flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS) - ? key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, - srvIP, srvPort, vlanId, mplsId, ctrlPkt->pkth->address_space_id, ctrlPkt->pkth->tenant_id, ctrlPkt->pkth->egress_group, - ctrlPkt->pkth->ingress_group) - : key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, - srvIP, srvPort, vlanId, mplsId, *ctrlPkt->pkth); - + bool reversed_key = key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, + srvIP, srvPort, vlanId, mplsId, ctrlPkt->pkth->address_space_id, ctrlPkt->pkth->tenant_id, + 0 != (ctrlPkt->pkth->flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS), + ctrlPkt->pkth->egress_group, ctrlPkt->pkth->ingress_group); bool new_node = false; ExpectNode* node = static_cast ( hash_table->get_user_data(&key) ); if ( !node ) diff --git a/src/flow/flow.cc b/src/flow/flow.cc index c870cf692..8d72022cb 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -246,8 +246,8 @@ FlowData* Flow::get_flow_data(unsigned id) const void Flow::free_flow_data(FlowData* fd) { - if ( !fd ) return; - flow_data.erase(fd->get_id()); + if ( fd ) + flow_data.erase(fd->get_id()); } void Flow::free_flow_data(uint32_t proto) @@ -258,7 +258,11 @@ void Flow::free_flow_data(uint32_t proto) void Flow::free_flow_data() { if ( flow_data.empty() ) + { + if (stash) + stash->reset(); return; + } const SnortConfig* sc = SnortConfig::get_conf(); PolicySelector* ps = sc->policy_map->get_policy_selector(); NetworkPolicy* np = nullptr; @@ -287,6 +291,8 @@ void Flow::free_flow_data() } flow_data.clear(); + if (stash) + stash->reset(); if (ps) { @@ -300,7 +306,7 @@ void Flow::call_handlers(Packet* p, bool eof) { for (auto& fd_pair : flow_data) { - FlowData* fd = fd_pair.second.get(); + FlowData* fd = fd_pair.second.get(); if ( eof ) fd->handle_eof(p); else diff --git a/src/flow/flow_key.cc b/src/flow/flow_key.cc index 732fe2e9b..ed5685ba9 100644 --- a/src/flow/flow_key.cc +++ b/src/flow/flow_key.cc @@ -234,7 +234,7 @@ bool FlowKey::init( const SfIp *srcIP, uint16_t srcPort, const SfIp *dstIP, uint16_t dstPort, uint16_t vlanId, uint32_t mplsId, - uint32_t addrSpaceId, uint32_t tid, + uint32_t addrSpaceId, uint32_t tid, bool significant_groups, int16_t ingress_group, int16_t egress_group) { bool reversed; @@ -266,7 +266,7 @@ bool FlowKey::init( padding = flags.padding_bits = 0; - flags.group_used = (ingress_group != DAQ_PKTHDR_UNKNOWN and egress_group != DAQ_PKTHDR_UNKNOWN); + flags.group_used = significant_groups; init_groups(ingress_group, egress_group, reversed); return reversed; @@ -308,56 +308,12 @@ bool FlowKey::init( init_mpls(sc, mplsId); padding = flags.padding_bits = 0; - flags.group_used = ((pkt_hdr.flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS) != 0); + flags.group_used = 0 != (pkt_hdr.flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS); init_groups(pkt_hdr.ingress_group, pkt_hdr.egress_group, reversed); return reversed; } -bool FlowKey::init( - const SnortConfig* sc, - PktType type, IpProtocol ip_proto, - const SfIp *srcIP, const SfIp *dstIP, - uint32_t id, uint16_t vlanId, - uint32_t mplsId, uint32_t addrSpaceId, - uint32_t tid, int16_t ingress_group, - int16_t egress_group) -{ - // to avoid confusing 2 different datagrams or confusing a datagram - // with a session, we don't order the addresses and we set version - - uint16_t srcPort = id & 0xFFFF; - uint16_t dstPort = id >> 16; - bool reversed; - - if (srcIP->is_ip4() && dstIP->is_ip4()) - { - version = 4; - reversed = init4(ip_proto, srcIP, srcPort, dstIP, dstPort, false); - ip_protocol = (uint8_t)ip_proto; - } - else - { - version = 6; - reversed = init6(ip_proto, srcIP, srcPort, dstIP, dstPort, false); - ip_protocol = 0; - } - - pkt_type = type; - tenant_id = tid; - - init_vlan(sc, vlanId); - init_address_space(sc, addrSpaceId); - init_mpls(sc, mplsId); - - padding = flags.padding_bits = 0; - - flags.group_used = (ingress_group != DAQ_PKTHDR_UNKNOWN and egress_group != DAQ_PKTHDR_UNKNOWN); - init_groups(ingress_group, egress_group, reversed); - - return reversed; -} - bool FlowKey::init( const SnortConfig* sc, PktType type, IpProtocol ip_proto, @@ -394,7 +350,7 @@ bool FlowKey::init( padding = flags.padding_bits = 0; - flags.group_used = ((pkt_hdr.flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS) != 0); + flags.group_used = 0 != (pkt_hdr.flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS); init_groups(pkt_hdr.ingress_group, pkt_hdr.egress_group, reversed); return reversed; diff --git a/src/flow/flow_key.h b/src/flow/flow_key.h index f7bcb04bb..6a78d205a 100644 --- a/src/flow/flow_key.h +++ b/src/flow/flow_key.h @@ -70,22 +70,15 @@ struct SO_PUBLIC FlowKey uint8_t padding_bits : 7; } flags; - /* The init() functions return true if the key IP/port fields were actively - normalized, reversing the source and destination addresses internally. - The IP-only init() will always return false as we will not reorder its - addresses at this time. */ + // The init() functions return true if the key IP/port fields were actively + // normalized, reversing the source and destination addresses internally. + // The IP-only init() will always return false as we will not reorder its + // addresses at this time. bool init( const SnortConfig*, PktType, IpProtocol, const snort::SfIp *srcIP, uint16_t srcPort, const snort::SfIp *dstIP, uint16_t dstPort, - uint16_t vlanId, uint32_t mplsId, uint32_t addrSpaceId, uint32_t tid, - int16_t group_h = DAQ_PKTHDR_UNKNOWN, int16_t group_l = DAQ_PKTHDR_UNKNOWN); - - bool init( - const SnortConfig*, PktType, IpProtocol, - const snort::SfIp *srcIP, const snort::SfIp *dstIP, - uint32_t id, uint16_t vlanId, - uint32_t mplsId, uint32_t addrSpaceId, uint32_t tid, + uint16_t vlanId, uint32_t mplsId, uint32_t addrSpaceId, uint32_t tid, bool significant_groups, int16_t group_h = DAQ_PKTHDR_UNKNOWN, int16_t group_l = DAQ_PKTHDR_UNKNOWN); bool init( @@ -94,6 +87,7 @@ struct SO_PUBLIC FlowKey const snort::SfIp *dstIP, uint16_t dstPort, uint16_t vlanId, uint32_t mplsId, const DAQ_PktHdr_t&); + // IP fragment key bool init( const SnortConfig*, PktType, IpProtocol, const snort::SfIp *srcIP, const snort::SfIp *dstIP, diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index 98501336d..328f0edd0 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -98,7 +98,7 @@ bool FlowKey::init( const SfIp*, uint16_t, const SfIp*, uint16_t, uint16_t, uint32_t, - uint32_t, uint32_t, int16_t, int16_t) + uint32_t, uint32_t, bool, int16_t, int16_t) { return true; } @@ -113,17 +113,6 @@ bool FlowKey::init( return true; } -bool FlowKey::init( - const SnortConfig*, - PktType, IpProtocol, - const SfIp*, const SfIp*, - uint32_t, uint16_t, - uint32_t, uint32_t, uint32_t, int16_t, - int16_t) -{ - return true; -} - bool FlowKey::init( const SnortConfig*, PktType, IpProtocol, diff --git a/src/stream/stream.cc b/src/stream/stream.cc index 05b515976..8a4aaf48a 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -93,14 +93,14 @@ Flow* Stream::get_flow( const SfIp* srcIP, uint16_t srcPort, const SfIp* dstIP, uint16_t dstPort, uint16_t vlan, uint32_t mplsId, uint32_t addressSpaceId, - uint32_t tenant_id, + uint32_t tenant_id, bool significant_groups, int16_t ingress_group, int16_t egress_group) { FlowKey key; const SnortConfig* sc = SnortConfig::get_conf(); key.init(sc, type, proto, srcIP, srcPort, dstIP, dstPort, vlan, mplsId, - addressSpaceId, tenant_id, ingress_group, egress_group); + addressSpaceId, tenant_id, significant_groups, ingress_group, egress_group); return get_flow(&key); } diff --git a/src/stream/stream.h b/src/stream/stream.h index 6f57a7477..53023fc41 100644 --- a/src/stream/stream.h +++ b/src/stream/stream.h @@ -202,7 +202,7 @@ public: PktType type, IpProtocol proto, const snort::SfIp* a1, uint16_t p1, const snort::SfIp* a2, uint16_t p2, uint16_t vlanId, uint32_t mplsId, uint32_t addrSpaceId, - uint32_t tenant_id, int16_t ingress_group = DAQ_PKTHDR_UNKNOWN, + uint32_t tenant_id, bool significant_groups, int16_t ingress_group = DAQ_PKTHDR_UNKNOWN, int16_t egress_group = DAQ_PKTHDR_UNKNOWN); static Flow* get_flow(