]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
Merge pull request #1039 in SNORT/snort3 from it_control to master
authorRuss Combs (rucombs) <rucombs@cisco.com>
Wed, 18 Oct 2017 18:40:12 +0000 (14:40 -0400)
committerRuss Combs (rucombs) <rucombs@cisco.com>
Wed, 18 Oct 2017 18:40:12 +0000 (14:40 -0400)
Squashed commit of the following:

commit 3add48e8026be96032f8d257729c84de0adfe42d
Author: Russ Combs <rucombs@cisco.com>
Date:   Tue Oct 17 07:15:54 2017 -0400

    inspector_manager: cleanup

commit ccc7243718c0945517b2abed3904e6c8d0ea332d
Author: Russ Combs <rucombs@cisco.com>
Date:   Sat Oct 14 09:47:47 2017 -0400

    inspectors: tweak dispatch logic for optimal control

commit 9e8396d8937a5875a3a299e118ab8b2efc393d1a
Author: Russ Combs <rucombs@cisco.com>
Date:   Tue Oct 10 11:43:52 2017 -0400

    appid: handle sip events before packets

commit 8be99056f8be259392ec72cf57050658c5db4580
Author: Russ Combs <rucombs@cisco.com>
Date:   Tue Oct 10 07:38:24 2017 -0400

    inspectors: remove cruft

commit 2245f6c55f243fa04c44273a3aa283f16a322381
Author: Russ Combs <rucombs@cisco.com>
Date:   Mon Oct 9 21:20:21 2017 -0400

    inspectors: packet types do not eval defragged packets

commit a0635f1279f99ba1cb27626a1df7c516da470487
Author: Russ Combs <rucombs@cisco.com>
Date:   Sun Oct 8 10:00:50 2017 -0400

    inspectors: add control type and ensure appid is run ahead of other controls

13 files changed:
src/flow/flow.h
src/framework/codec.cc
src/framework/inspector.h
src/main/snort.cc
src/managers/inspector_manager.cc
src/managers/inspector_manager.h
src/network_inspectors/appid/appid_inspector.cc
src/network_inspectors/appid/detector_plugins/detector_sip.cc
src/network_inspectors/reputation/reputation_inspect.cc
src/packet_io/active.cc
src/packet_io/active.h
src/protocols/packet_manager.cc
src/stream/stream.cc

index 135a79e125d7da9e52444ee7cdf612a5319573ab..379d9d99f4b58800818745ccaa0223267cb8db88 100644 (file)
@@ -215,7 +215,7 @@ public:
     { return (ssn_state.session_flags & SSNFLAG_BLOCK) != 0; }
 
     bool full_inspection() const
-    { return flow_state <= FlowState::INSPECT; }
+    { return (flow_state <= FlowState::INSPECT) and !is_inspection_disabled(); }
 
     void set_state(FlowState fs)
     { flow_state = fs; }
@@ -269,14 +269,10 @@ public:
     }
 
     void disable_inspection()
-    {
-        disable_inspect = true;
-    }
+    { disable_inspect = true; }
 
-    bool is_inspection_disabled()
-    {
-        return disable_inspect;
-    }
+    bool is_inspection_disabled() const
+    { return disable_inspect; }
 
     bool is_offloaded() const
     { return flow_flags & FLOW_IS_OFFLOADED; }
index fdfd257282813b6e822c8bd4f0cc19428a1a0ef4..66e5571429f374000f438a62c67e375f3f1cead9 100644 (file)
@@ -77,11 +77,8 @@ Buffer::Buffer(uint8_t* buf, uint32_t size) :
     off(0)
 { }
 
-void Codec::codec_event(const CodecData& codec, CodecSid sid)
+void Codec::codec_event(const CodecData&, CodecSid sid)
 {
-    if ( codec.codec_flags & CODEC_STREAM_REBUILT )
-        return;
-
     DetectionEngine::queue_event(GID_DECODE, sid);
 }
 
index c33112093665fadff82ac4726ed5780afe187c14..f3da503a45c95a3480b4928b3df64f42904793d9 100644 (file)
@@ -153,16 +153,20 @@ public:
     T* data;
 };
 
+// at present there is no sequencing among like types except that appid
+// is always first among controls.
+
 enum InspectorType
 {
-    IT_PASSIVE,  // config only, or data consumer
+    IT_PASSIVE,  // config only, or data consumer (eg file_log, binder, ftp_client)
     IT_BINDER,   // maps config to traffic
     IT_WIZARD,   // guesses service inspector
-    IT_PACKET,   // processes raw packets
-    IT_NETWORK,  // process packets w/o service
-    IT_STREAM,   // flow tracking and reassembly
-    IT_SERVICE,  // reassemble and process service PDUs
-    IT_PROBE,    // process all packets after above
+    IT_PACKET,   // processes raw packets only (eg normalize, capture)
+    IT_STREAM,   // flow tracking and reassembly (eg ip, tcp, udp)
+    IT_NETWORK,  // process packets w/o service (eg arp, bo, rep)
+    IT_SERVICE,  // extract and analyze service PDUs (eg dce, http, ssl)
+    IT_CONTROL,  // process all packets before detection (eg appid)
+    IT_PROBE,    // process all packets after detection (eg perf_monitor, port_scan)
     IT_MAX
 };
 
index b8a88f1b5745f4417b8e062a80594ab6a3538bd1..492b62d53c503f3875948138a4e0895be0dce7d9 100644 (file)
@@ -853,7 +853,7 @@ DAQ_Verdict Snort::process_packet(
 {
     aux_counts.rx_bytes += pkthdr->caplen;
 
-    PacketManager::decode(p, pkthdr, pkt);
+    PacketManager::decode(p, pkthdr, pkt, is_frag);
     assert(p->pkth && p->pkt);
 
     PacketTracer::add_header_info(p);
index cb0df4ef5335c28847a50474ebcf3d6d4094faca..3cf952e0fae23676b19e54891551f7afbbf72240 100644 (file)
@@ -46,6 +46,7 @@ using namespace std;
 // FIXIT-L define module names just once
 #define bind_id "binder"
 #define wiz_id "wizard"
+#define app_id "appid"
 
 //-------------------------------------------------------------------------
 // list stuff
@@ -185,8 +186,28 @@ struct PHVector
 
     void add(PHInstance* p)
     { vec[num++] = p; }
+
+    void add_control(PHInstance*);
 };
 
+// FIXIT-L a more sophisticated approach to handling controls etc. may be
+// warranted such as a configuration or priority scheme (a la 2X).  for
+// now we only require that appid run first among controls.
+
+void PHVector::add_control(PHInstance* p)
+{
+    const char* name = p->pp_class.api.base.name;
+
+    if ( strcmp(name, app_id) or !num )
+        add(p);
+
+    else
+    {
+        add(vec[0]);
+        vec[0] = p;
+    }
+}
+
 struct FrameworkPolicy
 {
     PHInstanceList ilist;
@@ -196,6 +217,7 @@ struct FrameworkPolicy
     PHVector network;
     PHVector session;
     PHVector service;
+    PHVector control;
     PHVector probe;
 
     Inspector* binder;
@@ -213,6 +235,7 @@ void FrameworkPolicy::vectorize()
     network.alloc(ilist.size());
     session.alloc(ilist.size());
     service.alloc(ilist.size());
+    control.alloc(ilist.size());
     probe.alloc(ilist.size());
 
     for ( auto* p : ilist )
@@ -248,6 +271,10 @@ void FrameworkPolicy::vectorize()
             wizard = p->handler;
             break;
 
+        case IT_CONTROL:
+            control.add_control(p);
+            break;
+
         case IT_PROBE:
             probe.add(p);
             break;
@@ -479,16 +506,6 @@ Inspector* InspectorManager::get_binder()
     return pi->framework_policy->binder;
 }
 
-Inspector* InspectorManager::get_wizard()
-{
-    InspectionPolicy* pi = get_inspection_policy();
-
-    if ( !pi || !pi->framework_policy )
-        return nullptr;
-
-    return pi->framework_policy->wizard;
-}
-
 // FIXIT-P cache get_inspector() returns or provide indexed lookup
 Inspector* InspectorManager::get_inspector(const char* key, bool dflt_only)
 {
@@ -897,20 +914,14 @@ void InspectorManager::bumble(Packet* p)
         flow->session->restart(p);
 }
 
-bool InspectorManager::full_inspection(FrameworkPolicy* fp, Packet* p)
+void InspectorManager::full_inspection(Packet* p)
 {
     Flow* flow = p->flow;
 
-    if ( !flow->service )
-        ::execute(p, fp->network.vec, fp->network.num);
-
-    else if ( flow->clouseau and !p->is_cooked() )
+    if ( flow->service and flow->clouseau and !p->is_cooked() )
         bumble(p);
 
-    if ( p->disable_inspect )
-        return false;
-
-    else if ( !p->dsize )
+    if ( !p->dsize )
         DetectionEngine::disable_content(p);
 
     else if ( flow->gadget && flow->gadget->likes(p) )
@@ -918,34 +929,50 @@ bool InspectorManager::full_inspection(FrameworkPolicy* fp, Packet* p)
         flow->gadget->eval(p);
         s_clear = true;
     }
-
-    return true;
 }
 
+// FIXIT-M split stream base processing out of it_session so that flow lookup
+// can be done first to avoid executing it_packet on disabled flows.  also
+// leverage knowledge of flow creation so that reputation (possibly a new
+// it_xxx) is run just once per flow (and all non-flow packets).
+
 void InspectorManager::execute(Packet* p)
 {
     FrameworkPolicy* fp = get_inspection_policy()->framework_policy;
     assert(fp);
 
-    // FIXIT-M blocked flows should not be normalized
     if ( !p->is_cooked() )
         ::execute(p, fp->packet.vec, fp->packet.num);
 
     if ( !p->has_paf_payload() )
         ::execute(p, fp->session.vec, fp->session.num);
 
-    if( p->disable_inspect )
-        return;
-
-    Flow* flow = p->flow;
+    // must check between each ::execute()
+    if ( p->disable_inspect )
+       return;
 
-    if ( !flow )
+    if ( !p->flow )
+    {
         ::execute(p, fp->network.vec, fp->network.num);
 
-    else if ( flow->full_inspection() )
+        if ( p->disable_inspect )
+           return;
+
+        ::execute(p, fp->control.vec, fp->control.num);
+    }
+    else
     {
-        if(!full_inspection(fp, p))
-            return;
+        if ( !p->flow->service )
+            ::execute(p, fp->network.vec, fp->network.num);
+
+        if ( p->disable_inspect )
+           return;
+
+        if ( p->flow->full_inspection() )
+            full_inspection(p);
+
+        if ( !p->disable_inspect and !p->flow->is_inspection_disabled() )
+            ::execute(p, fp->control.vec, fp->control.num);
     }
 }
 
index 117c27ac5df3176d112e452fef13182cf74852f1..aa449a7113e236874b501bf78fa371c5a586d9a2 100644 (file)
@@ -62,7 +62,6 @@ public:
     SO_PUBLIC static Inspector* get_inspector(const char* key, bool dflt_only = false);
 
     SO_PUBLIC static Inspector* get_binder();
-    static Inspector* get_wizard();
 
     SO_PUBLIC static Inspector* acquire(const char* key, SnortConfig*);
     SO_PUBLIC static void release(Inspector*);
@@ -89,7 +88,7 @@ public:
 
 private:
     static void bumble(Packet*);
-    static bool full_inspection(FrameworkPolicy*, Packet*);
+    static void full_inspection(Packet*);
 };
 
 #endif
index f88a93547d5d96eb5548be7a16e06d5952ae5783..362142ef7373c3348feaec45d68c4f908de77aa1 100644 (file)
@@ -264,7 +264,7 @@ const InspectApi appid_inspector_api =
         mod_ctor,
         mod_dtor
     },
-    IT_NETWORK,
+    IT_CONTROL,
     (uint16_t)PktType::ANY_IP,
     nullptr, // buffers
     nullptr, // service
index ef668646f22facb4457510614d60534ff9da3ed9..f24db8fc9dcdd6e989d0c0e18307502f9b74ec57 100644 (file)
@@ -472,28 +472,16 @@ void SipEventHandler::handle(DataEvent& event, Flow* flow)
     AppIdSession* asd = nullptr;
     SipEvent& sip_event = (SipEvent&)event;
 
-#ifdef DEBUG_APP_ID_SESSIONS
-    {
-        char src_ip[INET6_ADDRSTRLEN];
-        char dst_ip[INET6_ADDRSTRLEN];
-        const SfIp* ip;
-
-        src_ip[0] = 0;
-        ip = p->ptrs.ip_api.get_src();
-        sfip_ntop(ip, src_ip, sizeof(src_ip));
-        dst_ip[0] = 0;
-        ip = p->ptrs.ip_api.get_dst();
-        sfip_ntop(ip, dst_ip, sizeof(dst_ip));
-        fprintf(SF_DEBUG_FILE, "AppId Sip Snort Callback Session %s-%u -> %s-%u %d\n", src_ip,
-            (unsigned)p->src_port, dst_ip, (unsigned)p->dst_port, IsTCP(p) ? IpProtocol::TCP :
-            IpProtocol::UDP);
-    }
-#endif
     if ( flow )
         asd = appid_api.get_appid_session(flow);
 
     if ( !asd )
-        return;
+    {
+        const Packet* p = sip_event.get_packet();
+        IpProtocol protocol = p->is_tcp() ? IpProtocol::TCP : IpProtocol::UDP;
+        int direction = p->is_from_client() ? APP_ID_FROM_INITIATOR : APP_ID_FROM_RESPONDER;
+        asd = AppIdSession::allocate_session(p, protocol, direction);
+    }
 
     client_handler(sip_event, asd);
     service_handler(sip_event, asd);
index f0774e27d38bd0b15d1395481d677d6844608d53..7cc15f7e021a21c3ab4343a1fe90bb0993d78cf3 100644 (file)
@@ -301,13 +301,7 @@ static void snort_reputation(ReputationConfig* config, Packet* p)
         Active::drop_packet(p, true);
         // disable all preproc analysis and detection for this packet
         DetectionEngine::disable_all(p);
-        p->disable_inspect = true;
-        if (p->flow)
-        {
-            p->flow->set_state(Flow::FlowState::BLOCK);
-            p->flow->disable_inspection();
-        }
-
+        Active::block_session(p, true);
         reputationstats.blacklisted++;
     }
     else if (MONITORED == decision)
@@ -320,12 +314,7 @@ static void snort_reputation(ReputationConfig* config, Packet* p)
         DetectionEngine::queue_event(GID_REPUTATION, REPUTATION_EVENT_WHITELIST);
         p->packet_flags |= PKT_IGNORE;
         DetectionEngine::disable_all(p);
-        p->disable_inspect = true;
-        if (p->flow)
-        {
-            p->flow->set_state(Flow::FlowState::ALLOW);
-            p->flow->disable_inspection();
-        }
+        Active::allow_session(p);
         reputationstats.whitelisted++;
     }
 }
index 3cd3895e060734a8a788bfa1ae261129b97a25a6..912d8824e9912f75bc759e646cb4a165b1b9f013 100644 (file)
@@ -430,16 +430,31 @@ bool Active::daq_retry_packet(const Packet *p)
     return retry_queued;
 }
 
-void Active::block_session(const Packet* p, bool force)
+void Active::allow_session(Packet* p)
+{
+    active_action = ACT_PASS;
+
+    if ( p->flow )
+    {
+        p->flow->set_state(Flow::FlowState::ALLOW);
+        p->flow->disable_inspection();
+    }
+
+    p->disable_inspect = true;
+}
+
+void Active::block_session(Packet* p, bool force)
 {
     update_status(p, force);
     active_action = ACT_BLOCK;
 
     if ( force or SnortConfig::inline_mode() or SnortConfig::treat_drop_as_ignore() )
         Stream::block_flow(p);
+
+    p->disable_inspect = true;
 }
 
-void Active::reset_session(const Packet* p, bool force)
+void Active::reset_session(Packet* p, bool force)
 {
     update_status(p, force);
     active_action = ACT_RESET;
@@ -457,6 +472,8 @@ void Active::reset_session(const Packet* p, bool force)
             p->flow->set_state(Flow::FlowState::RESET);
         }
     }
+
+    p->disable_inspect = true;
 }
 
 void Active::set_delayed_action(ActiveAction action, bool force)
@@ -467,7 +484,7 @@ void Active::set_delayed_action(ActiveAction action, bool force)
         active_status = AST_FORCE;
 }
 
-void Active::apply_delayed_action(const Packet* p)
+void Active::apply_delayed_action(Packet* p)
 {
     bool force = (active_status == AST_FORCE);
 
index 89da24e1adb12e4ba7b8e3e7b5dbf794eb0dc80f..aa8cd023aadd89e9ed1d8dd562b63161ad12dc56 100644 (file)
@@ -85,10 +85,12 @@ public:
 
     static void drop_packet(const Packet*, bool force = false);
     static void daq_drop_packet(const Packet*);
-    static bool daq_retry_packet(const Packet *p);
+    static bool daq_retry_packet(const Packet*);
 
-    static void block_session(const Packet* p, bool force = false);
-    static void reset_session(const Packet* p, bool force = false);
+    static void allow_session(Packet*);
+
+    static void block_session(Packet*, bool force = false);
+    static void reset_session(Packet*, bool force = false);
 
     static void block_again()
     { active_action = ACT_BLOCK; }
@@ -125,7 +127,7 @@ public:
 
     static void set_delayed_action(ActiveAction, bool force = false);
 
-    static void apply_delayed_action(const Packet* p);
+    static void apply_delayed_action(Packet*);
 
 private:
     static bool open(const char*);
index 44ed6dd7e5159a6ad06a9d156af6e6c25e2e5355..9ab31e9ba29ad783957df36c1a32f6fd1ba42517 100644 (file)
@@ -279,8 +279,7 @@ void PacketManager::decode(
                 s_stats[other_codecs]++;
 
                 if ( (to_utype(ProtocolId::MIN_UNASSIGNED_IP_PROTO) <= to_utype(prev_prot_id)) &&
-                    (to_utype(prev_prot_id) <= std::numeric_limits<uint8_t>::max()) &&
-                    !(codec_data.codec_flags & CODEC_STREAM_REBUILT) )
+                    (to_utype(prev_prot_id) <= std::numeric_limits<uint8_t>::max()) )
                 {
                     DetectionEngine::queue_event(GID_DECODE, DECODE_IP_UNASSIGNED_PROTO);
                 }
index 009c8510476fd0585c76bf255a76b7fc69d60d49..6bd52f91ca914882d75032786f55de843d574071 100644 (file)
@@ -305,6 +305,8 @@ void Stream::block_flow(const Packet* p)
 
     // Postpone clear till inspection is completed
     flow->session_state |= STREAM_STATE_BLOCK_PENDING;
+
+    flow->disable_inspection();
 }
 
 void Stream::drop_flow(const Packet* p)