Merge pull request #1754 in SNORT/snort3 from ~THOPETER/snort3:small_seg to master

Squashed commit of the following:

commit 89c55ebeecd380736f5caa3a63a3d18f0835ae49
Author: Tom Peters <thopeter@cisco.com>
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 b5a1d17699edfdd7043cd4bb5866afc01941d570..3a2bc33003f002ed466fb86d24c02738171d7f46 100644 (file)
--- 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 b14bc247f51f23dadd40b82e5579f4eaaeebfd17..629b19eef43fbaf6cdfef6a9435efb37737acefe 100644 (file)
--- 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 203561e706b041297608505238ccb5d5e0f03f20..a307358ebb16f8be04d062efbe654722ba89e53c 100644 (file)
--- 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 c77e9300bc732285dbabf53f35fbeb660a29deb3..2c7c08487dd419a9fdf810486767d8f30921cb79 100644 (file)
--- 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 7e0ffeb12b1cd8e6a356149421a7717e86b3e0f7..185e2c254eab4011d3d001a52d9f09961cc3da1c 100644 (file)
--- 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 7b03e3858b95baf6c687975ccbf48a23e47d2384..69db3d00d7f3971d459c57371d46356a53ffff02 100644 (file)
--- 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 77d3d198c1b593236e6b09792570f24903592549..e71344c01496c52ccb63f35b981b39609537f8c0 100644 (file)
--- 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 3a10387da82d95e5344a4013efe2b941e23233c6..93c6ad0c8f45c09e562d19e7be2f1ef907ab6d68 100644 (file)
--- 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 745a1abc801aa3aaaac36ecfa1e8aac321f77274..cf386e748baf7b70c2f87b02c841f6abc6b015b8 100644 (file)
--- 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 bf02529cf27f0cf95d3fda59026bf3fb26d716fa..269f4b18c2d3c775ffd222081ff521dd20c70ce4 100644 (file)
--- 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 a34cc0591b02e8a61cebc47dcc31132319acf52d..3cb7ab7bf4a23b57c76d1f83227f2894a237f259 100644 (file)
--- 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 9880ec7cdc690f136155906c9404799cead5fba1..2018c09b037f0a32f04e4acd8699843e56864d79 100644 (file)
--- 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 e2f9aac97f5d7b96c80a606646719a4f4ebacb47..05e98017ca6abc9ce80c7b6f7ef9f20c6cb20521 100644 (file)
--- 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 df0c18ef930dca5152c2529689852091670cd43b..a2b8c393866d5157453e9e33859eb13d0e733830 100644 (file)
--- 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 b184b23c6319cb08b5e330796a9b005a3c6943d0..73a3bbd352bed5096dcc7aaaea1eedf826e0c503 100644 (file)
--- 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 48ba34a643c973281146a34ef91fa56a6764f51a..efa879c8c60ec7cd1fceb5ce7343942b174c2b84 100644 (file)
--- 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 67c9a008d008691d230723bbb5590bd4b378e12b..4b5598b5d1e9cdfa89bb279eb2367ef077db5bce 100644 (file)
--- 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 d324b918395d854448154e8b71f045410bf7981f..21541d52eaccd64149272b00ed36a32125e7fafe 100644 (file)
--- 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 3223d17d36f9194a836c63a8cf9d3b4c0a266602..e487d913b209e13aa959ec052f7aae282e2a6e23 100644 (file)
--- 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 0d9840bd7a89c5eb4d2f636e46d388ea961fd9fa..0ac5ffb7e99b5e0bb7397dfc417ffdec60af2692 100644 (file)
--- 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 718bba863c8c84c1d2e2495bb83193abeea17d75..bf3d2423fa864a126572edbca623c675692f88e1 100644 (file)
--- 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 ebfadaba7a6115ce9fc49611d367a2a3510a90a3..b4cc79754d86e91812b0ab8e763fea017c54af1e 100644 (file)
--- 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 4dc715b424c38f98407765c496be516d5e83a85a..42dc1168e15c4602a64bb37f4fff9641fbd39ec4 100644 (file)
--- 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: