From: Mike Stepanek (mstepane) Date: Fri, 20 Sep 2019 16:40:59 +0000 (-0400) Subject: Merge pull request #1754 in SNORT/snort3 from ~THOPETER/snort3:small_seg to master X-Git-Tag: 3.0.0-262~35 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21852aee32f1ec638c419626951436a6c6b995cd;p=thirdparty%2Fsnort3.git Merge pull request #1754 in SNORT/snort3 from ~THOPETER/snort3:small_seg to master Squashed commit of the following: commit 89c55ebeecd380736f5caa3a63a3d18f0835ae49 Author: Tom Peters Date: Thu Sep 19 11:49:18 2019 -0400 stream: cleanup --- diff --git a/src/stream/libtcp/tcp_state_handler.cc b/src/stream/libtcp/tcp_state_handler.cc index b5a1d1769..3a2bc3300 100644 --- a/src/stream/libtcp/tcp_state_handler.cc +++ b/src/stream/libtcp/tcp_state_handler.cc @@ -37,17 +37,10 @@ using namespace std; TcpStateHandler::TcpStateHandler(TcpStreamTracker::TcpState state, TcpStateMachine& tsm) - : tsm(&tsm), tcp_state(state) { tsm.register_state_handler(state, *this); } -//TcpStateHandler::TcpStateHandler() : -// tsm(nullptr), tcp_state(TcpStreamTracker::TCP_CLOSED), session(*(new -// TcpStreamSession(nullptr))) -//{ -//} - bool TcpStateHandler::do_pre_sm_packet_actions(TcpSegmentDescriptor&, TcpStreamTracker&) { return true; diff --git a/src/stream/libtcp/tcp_state_handler.h b/src/stream/libtcp/tcp_state_handler.h index b14bc247f..629b19eef 100644 --- a/src/stream/libtcp/tcp_state_handler.h +++ b/src/stream/libtcp/tcp_state_handler.h @@ -36,21 +36,6 @@ public: virtual bool eval(TcpSegmentDescriptor&, TcpStreamTracker&); - TcpStreamTracker::TcpState get_tcp_state() const - { - return tcp_state; - } - - const TcpStateMachine* get_tsm() const - { - return tsm; - } - - void set_tsm(const TcpStateMachine* tsm) - { - this->tsm = tsm; - } - virtual bool do_pre_sm_packet_actions(TcpSegmentDescriptor&, TcpStreamTracker&); virtual bool do_post_sm_packet_actions(TcpSegmentDescriptor&, TcpStreamTracker&); @@ -67,9 +52,6 @@ protected: virtual bool fin_recv(TcpSegmentDescriptor&, TcpStreamTracker&) { return true; } virtual bool rst_sent(TcpSegmentDescriptor&, TcpStreamTracker&) { return true; } virtual bool rst_recv(TcpSegmentDescriptor&, TcpStreamTracker&) { return true; } - - const TcpStateMachine* tsm; - TcpStreamTracker::TcpState tcp_state; }; #endif diff --git a/src/stream/paf.cc b/src/stream/paf.cc index 203561e70..a307358eb 100644 --- a/src/stream/paf.cc +++ b/src/stream/paf.cc @@ -160,8 +160,6 @@ static inline bool paf_eval ( StreamSplitter* ss, PAF_State* ps, PafAux& px, Packet* pkt, uint32_t flags, const uint8_t* data, uint32_t len) { - uint16_t fuzz = 0; // FIXIT-L PAF add a little zippedy-do-dah - switch ( ps->paf ) { case StreamSplitter::SEARCH: @@ -178,7 +176,7 @@ static inline bool paf_eval ( ps->paf = StreamSplitter::SEARCH; return true; } - if ( px.len >= ss->max(pkt->flow) + fuzz ) + if ( px.len >= ss->max(pkt->flow) ) { px.ft = FT_MAX; return false; @@ -187,7 +185,7 @@ static inline bool paf_eval ( case StreamSplitter::LIMIT: // if we are within PAF_LIMIT_FUZZ character of paf_max ... - if ( px.len + PAF_LIMIT_FUZZ >= ss->max(pkt->flow) + fuzz) + if ( px.len + PAF_LIMIT_FUZZ >= ss->max(pkt->flow)) { px.ft = FT_LIMIT; ps->paf = StreamSplitter::LIMITED; @@ -292,21 +290,19 @@ int32_t paf_check ( px.idx = total - len; // if 'total' is greater than the maximum paf_max AND 'total' is greater - // than paf_max bytes + fuzz (i.e. after we have finished analyzing the + // than paf_max bytes (i.e. after we have finished analyzing the // current segment, total bytes analyzed will be greater than the - // configured (fuzz + paf_max) == (ss->max() + fuzz), we must ensure a flush - // occurs at the paf_max byte. So, we manually set the data's length and + // configured paf_max == ss->max(), we must ensure a flush + // occurs at the paf_max byte. So, we manually set the data's length and // total queued bytes (px.len) to guarantee that at most paf_max bytes will // be analyzed and flushed since the last flush point. It should also be // noted that we perform the check here rather in in paf_flush() to // avoid scanning the same data twice. The first scan would analyze the // entire segment and the second scan would analyze this segments // unflushed data. - uint16_t fuzz = 0; // FIXIT-L PAF add a little zippedy-do-dah - - if ( total >= MAX_PAF_MAX && total > ss->max(pkt->flow) + fuzz ) + if ( total >= MAX_PAF_MAX && total > ss->max(pkt->flow) ) { - px.len = MAX_PAF_MAX + fuzz; + px.len = MAX_PAF_MAX; len = len + px.len - total; } else @@ -319,7 +315,7 @@ int32_t paf_check ( px.ft = FT_NOP; uint32_t idx = px.idx; - bool cont = paf_eval(ss, ps, px, pkt, *flags, data, len); + const bool cont = paf_eval(ss, ps, px, pkt, *flags, data, len); if ( px.ft != FT_NOP ) { @@ -344,7 +340,7 @@ int32_t paf_check ( if ( ps->paf == StreamSplitter::ABORT ) *flags = 0; - else if ( (ps->paf != StreamSplitter::FLUSH) && (px.len > ss->max(pkt->flow)+fuzz) ) + else if ( (ps->paf != StreamSplitter::FLUSH) && (px.len > ss->max(pkt->flow)) ) { px.ft = FT_MAX; uint32_t fp = paf_flush(ps, px, flags); diff --git a/src/stream/tcp/tcp_event_logger.cc b/src/stream/tcp/tcp_event_logger.cc index c77e9300b..2c7c08487 100644 --- a/src/stream/tcp/tcp_event_logger.cc +++ b/src/stream/tcp/tcp_event_logger.cc @@ -32,27 +32,6 @@ #include "tcp_module.h" -#define EVENT_SYN_ON_EST 0x00000001 -#define EVENT_DATA_ON_SYN 0x00000002 -#define EVENT_DATA_ON_CLOSED 0x00000004 -#define EVENT_BAD_TIMESTAMP 0x00000008 -#define EVENT_WINDOW_TOO_LARGE 0x00000010 -#define EVENT_DATA_AFTER_RESET 0x00000020 -#define EVENT_SESSION_HIJACK_CLIENT 0x00000040 -#define EVENT_SESSION_HIJACK_SERVER 0x00000080 -#define EVENT_DATA_WITHOUT_FLAGS 0x00000100 -#define EVENT_4WHS 0x00000200 -#define EVENT_NO_TIMESTAMP 0x00000400 -#define EVENT_BAD_RST 0x00000800 -#define EVENT_BAD_FIN 0x00001000 -#define EVENT_BAD_ACK 0x00002000 -#define EVENT_DATA_AFTER_RST_RCVD 0x00004000 -#define EVENT_WINDOW_SLAM 0x00008000 -#define EVENT_NO_3WHS 0x00010000 -#define EVENT_BAD_SEGMENT 0x00020000 -#define EVENT_EXCESSIVE_OVERLAP 0x00040000 -#define EVENT_MAX_SMALL_SEGS_EXCEEDED 0x00080000 - struct tcp_event_sid { uint32_t event_id; diff --git a/src/stream/tcp/tcp_event_logger.h b/src/stream/tcp/tcp_event_logger.h index 7e0ffeb12..185e2c254 100644 --- a/src/stream/tcp/tcp_event_logger.h +++ b/src/stream/tcp/tcp_event_logger.h @@ -51,7 +51,6 @@ class TcpEventLogger public: TcpEventLogger() = default; - void clear_tcp_events() { tcp_events = 0; diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc index 7b03e3858..69db3d00d 100644 --- a/src/stream/tcp/tcp_module.cc +++ b/src/stream/tcp/tcp_module.cc @@ -136,10 +136,10 @@ THREAD_LOCAL TcpStats tcpStats; static const Parameter stream_tcp_small_params[] = { { "count", Parameter::PT_INT, "0:2048", "0", - "limit number of small segments queued" }, + "number of consecutive TCP small segments considered to be excessive (129:12)" }, { "maximum_size", Parameter::PT_INT, "0:2048", "0", - "limit number of small segments queued" }, + "minimum bytes for a TCP segment not to be considered small (129:12)" }, { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } }; diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc index 77d3d198c..e71344c01 100644 --- a/src/stream/tcp/tcp_reassembler.cc +++ b/src/stream/tcp/tcp_reassembler.cc @@ -202,16 +202,12 @@ int TcpReassembler::add_reassembly_segment( TcpReassemblerState& trs, TcpSegmentDescriptor& tsd, int16_t len, uint32_t slide, uint32_t trunc_len, uint32_t seq, TcpSegmentNode* left) { - TcpSegmentNode* tsn = nullptr; - int32_t newSize = len - slide - trunc_len; + const int32_t new_size = len - slide - trunc_len; + assert(new_size >= 0); - if ( newSize <= 0 ) + if ( new_size <= 0 ) { - // FIXIT-L ideally newSize would not go negative - //assert(newSize == 0); - - // zero size data because of trimming. Don't insert it - + // Zero size data because of trimming. Don't insert it. inc_tcp_discards(); trs.tracker->normalizer.trim_win_payload(tsd); @@ -219,18 +215,18 @@ int TcpReassembler::add_reassembly_segment( } // FIXIT-L don't allocate overlapped part - tsn = TcpSegmentNode::init(tsd); + TcpSegmentNode* const tsn = TcpSegmentNode::init(tsd); tsn->offset = slide; - tsn->c_len = (uint16_t)newSize; - tsn->i_len = (uint16_t)newSize; + tsn->c_len = (uint16_t)new_size; + tsn->i_len = (uint16_t)new_size; tsn->i_seq = tsn->c_seq = seq; tsn->ts = tsd.get_ts(); // FIXIT-M the urgent ptr handling is broken... urg_offset could be set here but currently // not actually referenced anywhere else. In 2.9.7 the FlushStream function did reference // this field but that code has been lost... urg ptr handling needs to be reviewed and fixed - //tsn->urg_offset = trs.tracker->normalizer.set_urg_offset(tsd.get_tcph(), tsd.get_seg_len()); + // tsn->urg_offset = trs.tracker->normalizer.set_urg_offset(tsd.get_tcph(), tsd.get_seg_len()); queue_reassembly_segment(trs, left, tsn); @@ -1287,6 +1283,9 @@ int TcpReassembler::insert_segment_in_seglist( { /* Adjust slide so that is correct relative to orig seq */ trs.sos.slide = trs.sos.seq - tsd.get_seg_seq(); + // FIXIT-L for some reason length - slide - trunc_len is sometimes negative + if (trs.sos.len - trs.sos.slide - trs.sos.trunc_len < 0) + return STREAM_INSERT_OK; rc = add_reassembly_segment( trs, tsd, trs.sos.len, trs.sos.slide, trs.sos.trunc_len, trs.sos.seq, trs.sos.left); } @@ -1309,7 +1308,7 @@ int TcpReassembler::queue_packet_for_reassembly( if ( SEQ_GT(trs.tracker->r_win_base, tsd.get_seg_seq() ) ) { - int32_t offset = trs.tracker->r_win_base - tsd.get_seg_seq(); + const int32_t offset = trs.tracker->r_win_base - tsd.get_seg_seq(); if ( offset < tsd.get_seg_len() ) { diff --git a/src/stream/tcp/tcp_reassembler.h b/src/stream/tcp/tcp_reassembler.h index 3a10387da..93c6ad0c8 100644 --- a/src/stream/tcp/tcp_reassembler.h +++ b/src/stream/tcp/tcp_reassembler.h @@ -25,9 +25,6 @@ #include "stream/stream.h" #include "stream/tcp/segment_overlap_editor.h" -class TcpSession; -class TcpStreamTracker; - class TcpReassembler : public SegmentOverlapEditor { public: diff --git a/src/stream/tcp/tcp_reassemblers.h b/src/stream/tcp/tcp_reassemblers.h index 745a1abc8..cf386e748 100644 --- a/src/stream/tcp/tcp_reassemblers.h +++ b/src/stream/tcp/tcp_reassemblers.h @@ -28,10 +28,11 @@ class TcpReassemblerFactory { public: static TcpReassembler* create(StreamPolicy); +private: + TcpReassemblerFactory() = delete; }; class TcpReassemblerPolicy - { public: TcpReassemblerPolicy() = default; diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index bf02529cf..269f4b18c 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -113,7 +113,8 @@ void TcpSession::restart(Packet* p) assert(p->flow == flow); DetectionEngine::onload(flow); - TcpStreamTracker* talker, * listener; + TcpStreamTracker* talker; + TcpStreamTracker* listener; if (p->is_from_server()) { diff --git a/src/stream/tcp/tcp_state_close_wait.h b/src/stream/tcp/tcp_state_close_wait.h index a34cc0591..3cb7ab7bf 100644 --- a/src/stream/tcp/tcp_state_close_wait.h +++ b/src/stream/tcp/tcp_state_close_wait.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateCloseWait : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_closed.h b/src/stream/tcp/tcp_state_closed.h index 9880ec7cd..2018c09b0 100644 --- a/src/stream/tcp/tcp_state_closed.h +++ b/src/stream/tcp/tcp_state_closed.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateClosed : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_closing.h b/src/stream/tcp/tcp_state_closing.h index e2f9aac97..05e98017c 100644 --- a/src/stream/tcp/tcp_state_closing.h +++ b/src/stream/tcp/tcp_state_closing.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateClosing : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_established.h b/src/stream/tcp/tcp_state_established.h index df0c18ef9..a2b8c3938 100644 --- a/src/stream/tcp/tcp_state_established.h +++ b/src/stream/tcp/tcp_state_established.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateEstablished : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_fin_wait1.h b/src/stream/tcp/tcp_state_fin_wait1.h index b184b23c6..73a3bbd35 100644 --- a/src/stream/tcp/tcp_state_fin_wait1.h +++ b/src/stream/tcp/tcp_state_fin_wait1.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateFinWait1 : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_fin_wait2.h b/src/stream/tcp/tcp_state_fin_wait2.h index 48ba34a64..efa879c8c 100644 --- a/src/stream/tcp/tcp_state_fin_wait2.h +++ b/src/stream/tcp/tcp_state_fin_wait2.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateFinWait2 : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_last_ack.h b/src/stream/tcp/tcp_state_last_ack.h index 67c9a008d..4b5598b5d 100644 --- a/src/stream/tcp/tcp_state_last_ack.h +++ b/src/stream/tcp/tcp_state_last_ack.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateLastAck : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_listen.h b/src/stream/tcp/tcp_state_listen.h index d324b9183..21541d52e 100644 --- a/src/stream/tcp/tcp_state_listen.h +++ b/src/stream/tcp/tcp_state_listen.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateListen : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_none.h b/src/stream/tcp/tcp_state_none.h index 3223d17d3..e487d913b 100644 --- a/src/stream/tcp/tcp_state_none.h +++ b/src/stream/tcp/tcp_state_none.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateNone : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_syn_recv.cc b/src/stream/tcp/tcp_state_syn_recv.cc index 0d9840bd7..0ac5ffb7e 100644 --- a/src/stream/tcp/tcp_state_syn_recv.cc +++ b/src/stream/tcp/tcp_state_syn_recv.cc @@ -167,10 +167,7 @@ bool TcpStateSynRecv::rst_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk) if ( trk.normalizer.validate_rst(tsd) ) { Flow* flow = tsd.get_flow(); - flow->set_session_flags(SSNFLAG_RESET); - if ( trk.normalizer.is_tcp_ips_enabled() ) - tcp_state = TcpStreamTracker::TCP_LISTEN; } else { diff --git a/src/stream/tcp/tcp_state_syn_recv.h b/src/stream/tcp/tcp_state_syn_recv.h index 718bba863..bf3d2423f 100644 --- a/src/stream/tcp/tcp_state_syn_recv.h +++ b/src/stream/tcp/tcp_state_syn_recv.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateSynRecv : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_syn_sent.h b/src/stream/tcp/tcp_state_syn_sent.h index ebfadaba7..b4cc79754 100644 --- a/src/stream/tcp/tcp_state_syn_sent.h +++ b/src/stream/tcp/tcp_state_syn_sent.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateSynSent : public TcpStateHandler { public: diff --git a/src/stream/tcp/tcp_state_time_wait.h b/src/stream/tcp/tcp_state_time_wait.h index 4dc715b42..42dc1168e 100644 --- a/src/stream/tcp/tcp_state_time_wait.h +++ b/src/stream/tcp/tcp_state_time_wait.h @@ -24,8 +24,6 @@ #include "stream/libtcp/tcp_state_handler.h" -class TcpSession; - class TcpStateTimeWait : public TcpStateHandler { public: