From: Michael Altizer (mialtize) Date: Mon, 24 Oct 2016 20:00:28 +0000 (-0400) Subject: Merge pull request #685 in SNORT/snort3 from bugfix_block to master X-Git-Tag: 3.0.0-233~212 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbb4a0e9db1284e6020e79a8bc5c1c5eeda9a9dc;p=thirdparty%2Fsnort3.git Merge pull request #685 in SNORT/snort3 from bugfix_block to master Squashed commit of the following: commit b38a7ac35e05c658250f88e6341a99f0b5c2f511 Author: mdagon Date: Thu Oct 20 16:21:52 2016 -0400 In case of blocked packet, delay clearing flow till the end of inspection --- diff --git a/src/detection/detect.cc b/src/detection/detect.cc index 268340763..307285ea8 100644 --- a/src/detection/detect.cc +++ b/src/detection/detect.cc @@ -108,13 +108,6 @@ void snort_inspect(Packet* p) check_tags_flag = 1; - // clear closed sessions here after inspection since non-stream - // inspectors may depend on flow information - // FIXIT-H but this result in double clearing? should normal - // clear_session() calls be deleted from stream? this is a - // performance hit on short-lived flows - Stream::check_flow_closed(p); - /* ** By checking tagging here, we make sure that we log the ** tagged packet whether it generates an alert or not. @@ -124,11 +117,21 @@ void snort_inspect(Packet* p) if ( inspected ) InspectorManager::clear(p); + + // clear closed sessions here after inspection since non-stream + // inspectors may depend on flow information + // FIXIT-H but this result in double clearing? should normal + // clear_session() calls be deleted from stream? this is a + // performance hit on short-lived flows + Stream::check_flow_closed(p); } Profile profile(eventqPerfStats); SnortEventqLog(p); SnortEventqReset(); + + // Handle block pending state + Stream::check_flow_block_pending(p); } void snort_log(Packet* p) diff --git a/src/flow/flow.h b/src/flow/flow.h index 3ed21a398..1bbcb858b 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -82,6 +82,7 @@ #define STREAM_STATE_CLOSED 0x0800 #define STREAM_STATE_IGNORE 0x1000 #define STREAM_STATE_NO_PICKUP 0x2000 +#define STREAM_STATE_BLOCK_PENDING 0x4000 // FIXIT-L move to appid class if/when the application ids array // is moved @@ -176,7 +177,7 @@ public: void set_mpls_layer_per_dir(Packet*); Layer get_mpls_layer_per_dir(bool); - uint32_t update_session_flags( uint32_t flags ) + uint32_t update_session_flags(uint32_t flags) { return ssn_state.session_flags = flags; } @@ -342,10 +343,10 @@ public: // FIXIT-M privatize if possible uint16_t ssn_policy; uint16_t session_state; - uint8_t inner_client_ttl, inner_server_ttl; - uint8_t outer_client_ttl, outer_server_ttl; + uint8_t inner_client_ttl, inner_server_ttl; + uint8_t outer_client_ttl, outer_server_ttl; - uint8_t response_count; + uint8_t response_count; bool disable_inspect; private: diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index e2ac7537b..9109bde59 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -254,7 +254,7 @@ bool Active::send_data( uint32_t sent = 0; const uint16_t maxPayload = PacketManager::encode_get_max_payload(p); - if(maxPayload) + if (maxPayload) { do { @@ -270,7 +270,8 @@ bool Active::send_data( buf += toSend; sent += toSend; - } while(blen -= toSend); + } + while (blen -= toSend); } plen = 0; @@ -282,7 +283,6 @@ bool Active::send_data( s_send(p->pkth, !(flags & ENC_FLAG_FWD), seg, plen); - if (flags & ENC_FLAG_RST_CLNT) { sent++; @@ -417,7 +417,7 @@ void Active::block_session(const Packet* p, bool force) active_action = ACT_BLOCK; if ( force or SnortConfig::inline_mode() or SnortConfig::treat_drop_as_ignore() ) - Stream::drop_flow(p); + Stream::block_flow(p); } void Active::reset_session(const Packet* p, bool force) @@ -452,7 +452,7 @@ void Active::apply_delayed_action(const Packet* p) { bool force = (active_status == AST_FORCE); - switch(delayed_active_action) + switch (delayed_active_action) { case ACT_PASS: break; @@ -471,6 +471,7 @@ void Active::apply_delayed_action(const Packet* p) delayed_active_action = ACT_PASS; } + //-------------------------------------------------------------------- bool Active::open(const char* dev) diff --git a/src/stream/stream.cc b/src/stream/stream.cc index d8557836d..4f8425346 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -164,6 +164,24 @@ FlowData* Stream::get_flow_data( // session status //------------------------------------------------------------------------- +void Stream::check_flow_block_pending(Packet* p) +{ + Flow* flow = p->flow; + + if ( !flow ) + return; + + if (flow->session_state & STREAM_STATE_BLOCK_PENDING) + { + flow->session->clear(); + flow->set_state(Flow::FlowState::BLOCK); + + if ( !(p->packet_flags & PKT_STATELESS) ) + drop_traffic(flow, SSN_DIR_BOTH); + flow->session_state &= ~STREAM_STATE_BLOCK_PENDING; + } +} + void Stream::check_flow_closed(Packet* p) { Flow* flow = p->flow; @@ -301,6 +319,17 @@ void Stream::drop_traffic(Flow* flow, char dir) } } +void Stream::block_flow(const Packet* p) +{ + Flow* flow = p->flow; + + if (!flow) + return; + + // Postpone clear till inspection is completed + flow->session_state |= STREAM_STATE_BLOCK_PENDING; +} + void Stream::drop_flow(const Packet* p) { Flow* flow = p->flow; @@ -368,7 +397,7 @@ bool Stream::expected_flow(Flow* f, Packet* p) int Stream::set_application_protocol_id_expected( const Packet* ctrlPkt, PktType type, IpProtocol ip_proto, const sfip_t* srcIP, uint16_t srcPort, - const sfip_t* dstIP, uint16_t dstPort, + const sfip_t* dstIP, uint16_t dstPort, int16_t appId, FlowData* fd) { assert(flow_con); diff --git a/src/stream/stream.h b/src/stream/stream.h index 7fa80c140..8555f3499 100644 --- a/src/stream/stream.h +++ b/src/stream/stream.h @@ -47,7 +47,7 @@ // sequence must match FRAG_POLICY_* enum in stream_ip.h (1-based) #define IP_POLICIES \ - "first | linux | bsd | bsd_right | last | windows | solaris" + "first | linux | bsd | bsd_right | last | windows | solaris" // sequence must match STREAM_POLICY_* defines in tcp_session.cc (1-based) #define TCP_POLICIES \ @@ -123,6 +123,10 @@ public: // subsequent packets received on this flow are dropped. static void drop_flow(const Packet*); + // Mark flow session as block pending. Resources will be released + // at the end of inspection + static void block_flow(const Packet*); + // FIXIT-L flush_request() / flush_response() are misnomers in ips mode and may cause errors // Flush queued data on the listener side of a stream flow. The listener is the side of the @@ -133,7 +137,7 @@ public: // Flush queued data on the talker side of a stream flow. The talker is the side of the // connection the packet originated from, so if the Packet is from the client, then the // client side tracker is flushed. - static void flush_response(Packet*); // flush talker + static void flush_response(Packet*); // flush talker // Add session alert - true if added static bool add_flow_alert(Flow*, Packet*, uint32_t gid, uint32_t sid); @@ -185,7 +189,7 @@ public: // Snort does not have an active packet that is relevant. static FlowData* get_flow_data( PktType type, IpProtocol proto, - const sfip_t *a1, uint16_t p1, const sfip_t *a2, uint16_t p2, + const sfip_t* a1, uint16_t p1, const sfip_t* a2, uint16_t p2, uint16_t vlanId, uint32_t mplsId, uint16_t addrSpaceId, unsigned flow_id); // Get pointer to application data for a flow using the FlowKey as the lookup criteria @@ -195,12 +199,15 @@ public: // cases where Snort does not have an active packet that is relevant. static Flow* get_flow( PktType type, IpProtocol proto, - const sfip_t *a1, uint16_t p1, const sfip_t *a2, uint16_t p2, + const sfip_t* a1, uint16_t p1, const sfip_t* a2, uint16_t p2, uint16_t vlanId, uint32_t mplsId, uint16_t addrSpaceId); // Delete the session if it is in the closed session state. static void check_flow_closed(Packet*); + // Handle session block pending state + static void check_flow_block_pending(Packet*); + // Create a session key from the Packet static FlowKey* get_flow_key(Packet*);