Pull request #4363: flow: handle significant groups with unknown group value as non...

Merge in SNORT/snort3 from ~RDEMPSTE/snort3:expected_unknown to master

Squashed commit of the following:

commit f2ff9af2a7393939742716c535411448fd557c27
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Tue May 21 17:09:24 2024 -0400

    flow: clear flow stash when freeing the flow data

commit c4282b8aa7ba3743b4413e2bbe6dc94959fb8e49
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
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 53a6128192d2b1098d6d7f4438b50f52770acbb8..1c833b2b4a12886324733b9a0a8b8c8870e218de 100644 (file)
--- 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<ExpectNode*> ( hash_table->get_user_data(&key) );
     if ( !node )

diff --git a/src/flow/flow.cc b/src/flow/flow.cc
index c870cf692f7b68924dd4767ae60f3df36195e565..8d72022cb4b03805d94036394599cf4c8fb93ea3 100644 (file)
--- 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 732fe2e9bd2c3c2dd92a859019d02ab10bd41170..ed5685ba98daa1847275f6047e735d667932bde6 100644 (file)
--- 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 f7bcb04bbd8fe6ec93bf25534d8228cc8e3fd5ae..6a78d205acdd4dd3776ce04857644799653d3e15 100644 (file)
--- 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 98501336d49734a8c49b3498e6713114de9cdbd8..328f0edd06b7a99be4a2de110b94484eb9da876e 100644 (file)
--- 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 05b5159767048dfa4f088fe88c151c45725a13aa..8a4aaf48abf7dc60cc4753efd8d5a3069ec49795 100644 (file)
--- 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 6f57a74773094d61f6b64a8416bb80fa6f0571f2..53023fc417b0e6b2a43039c79d1b531b771bc8df 100644 (file)
--- 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(