Merge pull request #2689 in SNORT/snort3 from ~SBAIGAL/snort3:ftps_eof to master

Squashed commit of the following:

commit bf862aa1e46a75147da1332d0f343faed2b273d6
Author: Steven Baigal (sbaigal) <sbaigal@cisco.com>
Date:   Tue Dec 15 13:09:53 2020 -0500

    ftp: using hold_packet to handle ftp-data eof

diff --git a/src/service_inspectors/ftp_telnet/ftp_data.cc b/src/service_inspectors/ftp_telnet/ftp_data.cc
index 52bc27c30d0fc10eef9c4d2dba0dde36a2fddaa0..a125020757c3ce9ff305e3e6392c0b6e30139576 100644 (file)
--- a/src/service_inspectors/ftp_telnet/ftp_data.cc
+++ b/src/service_inspectors/ftp_telnet/ftp_data.cc
@@ -239,47 +239,16 @@ void FtpDataFlowData::handle_expected(Packet* p)
     }
 }
 
-void FtpDataFlowData::handle_retransmit(Packet* p)
-{
-    FTP_DATA_SESSION* data_ssn = &session;
-
-    if ((data_ssn->eof_seq > 0) and (data_ssn->eof_seq == p->ptrs.tcph->seq() + p->dsize))
-    {
-        // only process the final data segment
-        initFilePosition(&data_ssn->position, get_file_processed_size(p->flow));
-        finalFilePosition(&data_ssn->position);
-
-        FileFlows* file_flows = FileFlows::get_file_flows(p->flow);
-        if (file_flows)
-        {
-            file_flows->file_process(DetectionEngine::get_current_packet(),
-                p->data, 0, SNORT_FILE_END, data_ssn->direction, data_ssn->path_hash);
-
-            eof_handled = true;
-        }
-    }
-}
-
 void FtpDataFlowData::handle_eof(Packet* p)
 {
     FTP_DATA_SESSION* data_ssn = &session;
-    data_ssn->eof_seq = 0;
 
     if (!PROTO_IS_FTP_DATA(data_ssn) || !FTPDataDirection(p, data_ssn))
         return;
 
-    if (p->dsize != 0)
-    {
-        initFilePosition(&data_ssn->position, get_file_processed_size(p->flow));
-        finalFilePosition(&data_ssn->position);
-        eof_handled = true;
-    }
-    else
-    {
-        Active* act = p->active;
-        act->set_delayed_action(Active::ACT_RETRY, true);
-        data_ssn->eof_seq = p->ptrs.tcph->seq();
-    }
+    initFilePosition(&data_ssn->position, get_file_processed_size(p->flow));
+    finalFilePosition(&data_ssn->position);
+    eof_handled = true;
 
     if (data_ssn->mss_changed)
         ftstats.total_sessions_mss_changed++;

diff --git a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc
index 8414d8fb42be37a3f94a40a100cc76c2acbe8bc0..00d6ff21af6a923bc18d01d76cb969916ad8ff4f 100644 (file)
--- a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc
+++ b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc
@@ -39,29 +39,18 @@ void FtpDataSplitter::restart_scan()
     bytes = segs = 0;
 }
 
-static void set_ftp_flush_flag(Flow* flow)
-{
-    FtpDataFlowData* fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id);
-    if ( fdfd )
-        fdfd->session.packet_flags |= FTPDATA_FLG_FLUSH;
-}
-
 StreamSplitter::Status FtpDataSplitter::scan(Packet* pkt, const uint8_t*, uint32_t len,
     uint32_t, uint32_t* fp)
 {
     Flow* flow = pkt->flow;
     assert(flow);
-    FtpDataFlowData* fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id);
 
-    if (!fdfd)
-        return SEARCH;
+    FtpDataFlowData* fdfd = nullptr;
+
     if ( len )
     {
         if(expected_seg_size == 0)
         {
-            // FIXIT-M: Can we do better than this guess if no MSS is specified?
-            // Malware detection won't work if expected_seg_size doesn't match
-            // the payload lengths on packets before the last packet.
             expected_seg_size = 1448;
 
             if(flow->session and flow->pkt_type == PktType::TCP)
@@ -72,43 +61,40 @@ StreamSplitter::Status FtpDataSplitter::scan(Packet* pkt, const uint8_t*, uint32
                     expected_seg_size -= tcp_options_len;
             }
         }
-
+        segs++;
+        bytes += len;
         if ( len != expected_seg_size )
         {
+            fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id);
+            if (!fdfd)
+                return SEARCH;
+
             ftstats.total_packets_mss_changed++;
             fdfd->session.mss_changed = true;
-            if (fdfd->session.bytes_seen == 0)
-            {
-                // Segmented pkt is  smaller than expected_seg_size
-                set_ftp_flush_flag(flow);
-            }
-            else if (pkt->ptrs.tcph and !pkt->ptrs.tcph->is_fin())
+            expected_seg_size = len;
+
+            if (pkt->ptrs.tcph and !pkt->ptrs.tcph->is_fin())
             {
-                Active* act = pkt->active;
-                // add packet to retry queue to consider this is not end of ftp flow
-                if (!(pkt->flow->flags.trigger_detained_packet_event))
-                    act->set_delayed_action(Active::ACT_RETRY, true);
+                // set flag for signature calculation in case this is the last packet
+                fdfd->session.packet_flags |= FTPDATA_FLG_FLUSH;
+                pkt->active->hold_packet(pkt);
+                return SEARCH;
             }
-            expected_seg_size = len;
-            restart_scan();
-            *fp = len;
-               fdfd->session.bytes_seen += len;
-            return FLUSH;
-        }
-        else
-        {
-            segs++;
-            bytes += len;
         }
-        fdfd->session.bytes_seen += len;
+    }
 
-        if ((segs >= 2 and bytes >= min) or (pkt->ptrs.tcph and pkt->ptrs.tcph->is_fin()))
-        {
-            // Either FIN or smaller size do FLUSH to continue inspection
-            restart_scan();
-            *fp = len;
-            return FLUSH;
-        }
+    if ((segs >= 2 and bytes >= min) or (pkt->ptrs.tcph and pkt->ptrs.tcph->is_fin()))
+    {
+        fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id);
+        if (!fdfd)
+            return SEARCH;
+
+        restart_scan();
+        *fp = len;
+        // avoid unnecessary signature calc by clearing the flag set by detained packet
+        if (fdfd->session.packet_flags & FTPDATA_FLG_FLUSH)
+            fdfd->session.packet_flags &= ~FTPDATA_FLG_FLUSH; 
+        return FLUSH;
     }
 
     return SEARCH;

diff --git a/src/service_inspectors/ftp_telnet/ftpp_si.h b/src/service_inspectors/ftp_telnet/ftpp_si.h
index ac1d77831e6a5704a7c58a4f4f67347792fc36b8..e792ba75ffda37b7ba750b2f140e6da7530c3f97 100644 (file)
--- a/src/service_inspectors/ftp_telnet/ftpp_si.h
+++ b/src/service_inspectors/ftp_telnet/ftpp_si.h
@@ -212,12 +212,10 @@ struct FTP_DATA_SESSION
     FTP_TELNET_SESSION ft_ssn;
     snort::FlowKey ftp_key;
     char* filename;
-    uint32_t eof_seq;
     size_t path_hash;
     int data_chan;
     int file_xfer_info;
     FilePosition position;
-    uint32_t bytes_seen;
     unsigned char mode;
     unsigned char packet_flags;
     bool direction;
@@ -235,7 +233,6 @@ public:
 
     void handle_expected(snort::Packet*) override;
     void handle_eof(snort::Packet*) override;
-    void handle_retransmit(snort::Packet*) override;
     size_t size_of() override
     { return sizeof(*this); }