From 4685613f44fa85d143cdbf04d16e49f35b04fe24 Mon Sep 17 00:00:00 2001 From: "Russ Combs (rucombs)" Date: Wed, 18 Oct 2017 14:40:12 -0400 Subject: [PATCH] Merge pull request #1039 in SNORT/snort3 from it_control to master Squashed commit of the following: commit 3add48e8026be96032f8d257729c84de0adfe42d Author: Russ Combs Date: Tue Oct 17 07:15:54 2017 -0400 inspector_manager: cleanup commit ccc7243718c0945517b2abed3904e6c8d0ea332d Author: Russ Combs Date: Sat Oct 14 09:47:47 2017 -0400 inspectors: tweak dispatch logic for optimal control commit 9e8396d8937a5875a3a299e118ab8b2efc393d1a Author: Russ Combs Date: Tue Oct 10 11:43:52 2017 -0400 appid: handle sip events before packets commit 8be99056f8be259392ec72cf57050658c5db4580 Author: Russ Combs Date: Tue Oct 10 07:38:24 2017 -0400 inspectors: remove cruft commit 2245f6c55f243fa04c44273a3aa283f16a322381 Author: Russ Combs Date: Mon Oct 9 21:20:21 2017 -0400 inspectors: packet types do not eval defragged packets commit a0635f1279f99ba1cb27626a1df7c516da470487 Author: Russ Combs Date: Sun Oct 8 10:00:50 2017 -0400 inspectors: add control type and ensure appid is run ahead of other controls --- src/flow/flow.h | 12 +-- src/framework/codec.cc | 5 +- src/framework/inspector.h | 16 ++-- src/main/snort.cc | 2 +- src/managers/inspector_manager.cc | 87 ++++++++++++------- src/managers/inspector_manager.h | 3 +- .../appid/appid_inspector.cc | 2 +- .../appid/detector_plugins/detector_sip.cc | 24 ++--- .../reputation/reputation_inspect.cc | 15 +--- src/packet_io/active.cc | 23 ++++- src/packet_io/active.h | 10 ++- src/protocols/packet_manager.cc | 3 +- src/stream/stream.cc | 2 + 13 files changed, 112 insertions(+), 92 deletions(-) diff --git a/src/flow/flow.h b/src/flow/flow.h index 135a79e12..379d9d99f 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -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; } diff --git a/src/framework/codec.cc b/src/framework/codec.cc index fdfd25728..66e557142 100644 --- a/src/framework/codec.cc +++ b/src/framework/codec.cc @@ -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); } diff --git a/src/framework/inspector.h b/src/framework/inspector.h index c33112093..f3da503a4 100644 --- a/src/framework/inspector.h +++ b/src/framework/inspector.h @@ -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 }; diff --git a/src/main/snort.cc b/src/main/snort.cc index b8a88f1b5..492b62d53 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -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); diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index cb0df4ef5..3cf952e0f 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -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); } } diff --git a/src/managers/inspector_manager.h b/src/managers/inspector_manager.h index 117c27ac5..aa449a711 100644 --- a/src/managers/inspector_manager.h +++ b/src/managers/inspector_manager.h @@ -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 diff --git a/src/network_inspectors/appid/appid_inspector.cc b/src/network_inspectors/appid/appid_inspector.cc index f88a93547..362142ef7 100644 --- a/src/network_inspectors/appid/appid_inspector.cc +++ b/src/network_inspectors/appid/appid_inspector.cc @@ -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 diff --git a/src/network_inspectors/appid/detector_plugins/detector_sip.cc b/src/network_inspectors/appid/detector_plugins/detector_sip.cc index ef668646f..f24db8fc9 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_sip.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_sip.cc @@ -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); diff --git a/src/network_inspectors/reputation/reputation_inspect.cc b/src/network_inspectors/reputation/reputation_inspect.cc index f0774e27d..7cc15f7e0 100644 --- a/src/network_inspectors/reputation/reputation_inspect.cc +++ b/src/network_inspectors/reputation/reputation_inspect.cc @@ -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++; } } diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index 3cd3895e0..912d8824e 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -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); diff --git a/src/packet_io/active.h b/src/packet_io/active.h index 89da24e1a..aa8cd023a 100644 --- a/src/packet_io/active.h +++ b/src/packet_io/active.h @@ -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*); diff --git a/src/protocols/packet_manager.cc b/src/protocols/packet_manager.cc index 44ed6dd7e..9ab31e9ba 100644 --- a/src/protocols/packet_manager.cc +++ b/src/protocols/packet_manager.cc @@ -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::max()) && - !(codec_data.codec_flags & CODEC_STREAM_REBUILT) ) + (to_utype(prev_prot_id) <= std::numeric_limits::max()) ) { DetectionEngine::queue_event(GID_DECODE, DECODE_IP_UNASSIGNED_PROTO); } diff --git a/src/stream/stream.cc b/src/stream/stream.cc index 009c85104..6bd52f91c 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -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) -- 2.47.3