From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Thu, 9 Jan 2020 16:41:20 +0000 (+0000)
Subject: Merge pull request #1905 in SNORT/snort3 from ~KATHARVE/snort3:http_file to master
X-Git-Tag: 3.0.0-268~64
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed844f571cbec22563b951347b15c41f4b9224e7;p=thirdparty%2Fsnort3.git

Merge pull request #1905 in SNORT/snort3 from ~KATHARVE/snort3:http_file to master

Squashed commit of the following:

commit fb5585357bd605ecaf77b59b6afa1b7dbf1237e9
Author: Katura Harvey <katharve@cisco.com>
Date:   Wed Dec 4 10:36:52 2019 -0500

    http_inspect: process multiple files simultaneously over HTTP/1.1
---

diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc
index 91b9ce08a..e77995ecd 100644
--- a/src/file_api/file_flows.cc
+++ b/src/file_api/file_flows.cc
@@ -121,7 +121,15 @@ FilePolicyBase* FileFlows::get_file_policy(Flow* flow)
 
 void FileFlows::set_current_file_context(FileContext* ctx)
 {
+    // If we finished processing a file context object last time, delete it
+    if (current_context_delete_pending)
+    {
+        delete current_context;
+        current_context_delete_pending = false;
+    }
     current_context = ctx;
+    // Not using current_file_id so clear it
+    current_file_id = 0;
 }
 
 FileContext* FileFlows::get_current_file_context()
@@ -142,9 +150,11 @@ uint64_t FileFlows::get_new_file_instance()
 FileFlows::~FileFlows()
 {
     delete(main_context);
+    if (current_context_delete_pending)
+        delete(current_context);
 
     // Delete any remaining FileContexts stored on the flow
-    for (auto const& elem : flow_file_contexts)
+    for (auto const& elem : partially_processed_contexts)
     {
         delete elem.second;
     }
@@ -175,17 +185,25 @@ FileContext* FileFlows::find_main_file_context(FilePosition pos, FileDirection d
     return context;
 }
 
-FileContext* FileFlows::get_file_context(uint64_t file_id, bool to_create)
+FileContext* FileFlows::get_partially_processed_context(uint64_t file_id)
 {
-    FileContext *context = nullptr;
+    auto elem = partially_processed_contexts.find(file_id);
+    if (elem != partially_processed_contexts.end())
+        return elem->second;
+    return nullptr;
+}
+
+FileContext* FileFlows::get_file_context(uint64_t file_id, bool to_create,
+    uint64_t multi_file_processing_id)
+{
+    // First check if this file is currently being processed
+    if (!multi_file_processing_id)
+        multi_file_processing_id = file_id;
+    FileContext *context = get_partially_processed_context(multi_file_processing_id);
 
-    // First check if this file is currently being processed and is stored on the file flows object
-    auto elem = flow_file_contexts.find(file_id);
-    if (elem != flow_file_contexts.end())
-        context = elem->second;
     // Otherwise check if it has been fully processed and is in the file cache. If the file is not
     // in the cache, don't add it.
-    else
+    if (!context)
     {
         FileCache* file_cache = FileService::get_file_cache();
         assert(file_cache);
@@ -197,7 +215,7 @@ FileContext* FileFlows::get_file_context(uint64_t file_id, bool to_create)
     {
         // If we have reached the max file per flow limit, alert and increment the peg count
         FileConfig* fc = get_file_config(SnortConfig::get_conf());
-        if (flow_file_contexts.size() == fc->max_files_per_flow)
+        if (partially_processed_contexts.size() == fc->max_files_per_flow)
         {
             file_counts.files_over_flow_limit_not_processed++;
             events.create_event(EVENT_FILE_DROPPED_OVER_LIMIT);
@@ -205,23 +223,24 @@ FileContext* FileFlows::get_file_context(uint64_t file_id, bool to_create)
         else
         {
             context = new FileContext;
-            flow_file_contexts[file_id] = context;
-            if (flow_file_contexts.size() > file_counts.max_concurrent_files_per_flow)
-                file_counts.max_concurrent_files_per_flow = flow_file_contexts.size();
+            partially_processed_contexts[multi_file_processing_id] = context;
+            if (partially_processed_contexts.size() > file_counts.max_concurrent_files_per_flow)
+                file_counts.max_concurrent_files_per_flow = partially_processed_contexts.size();
         }
     }
 
-    current_file_id = file_id;
     return context;
 }
 
-void FileFlows::remove_file_context(uint64_t file_id)
+// Remove a file context from the flow's partially processed store. Don't delete the context
+// yet because detection needs access; pointer is stored in current_context. The file context will
+// be deleted when the next file is processed
+void FileFlows::remove_processed_file_context(uint64_t file_id)
 {
-    auto elem = flow_file_contexts.find(file_id);
-    if (elem == flow_file_contexts.end())
-        return;
-    delete elem->second;
-    flow_file_contexts.erase(file_id);
+    FileContext *context = get_partially_processed_context(file_id);
+    partially_processed_contexts.erase(file_id);
+    if (context)
+        current_context_delete_pending = true;
 }
 
 /* This function is used to process file that is sent in pieces
@@ -231,21 +250,26 @@ void FileFlows::remove_file_context(uint64_t file_id)
  *    false: ignore this file
  */
 bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_data,
-    int data_size, uint64_t offset, FileDirection dir)
+    int data_size, uint64_t offset, FileDirection dir, uint64_t multi_file_processing_id,
+    FilePosition position)
 {
     int64_t file_depth = FileService::get_max_file_depth();
     bool continue_processing;
+    if (!multi_file_processing_id)
+        multi_file_processing_id = file_id;
 
     if ((file_depth < 0) or (offset > (uint64_t)file_depth))
     {
         return false;
     }
 
-    FileContext* context = get_file_context(file_id, true);
+    FileContext* context = get_file_context(file_id, true, multi_file_processing_id);
 
     if (!context)
         return false;
 
+    set_current_file_context(context);
+
     if (!context->get_processed_bytes())
     {
         context->check_policy(flow, dir, file_policy);
@@ -264,14 +288,14 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da
             continue_processing = context->process(p, file_data, data_size, position,
                     file_policy);
             if (context->processing_complete)
-                remove_file_context(file_id);
+                remove_processed_file_context(multi_file_processing_id);
             return continue_processing;
         }
     }
 
-    continue_processing = context->process(p, file_data, data_size, offset, file_policy);
+    continue_processing = context->process(p, file_data, data_size, offset, file_policy, position);
     if (context->processing_complete)
-        remove_file_context(file_id);
+        remove_processed_file_context(multi_file_processing_id);
     return continue_processing;
 }
 
@@ -300,9 +324,13 @@ bool FileFlows::file_process(Packet* p, const uint8_t* file_data, int data_size,
     return context->process(p, file_data, data_size, position, file_policy);
 }
 
-void FileFlows::set_file_name(const uint8_t* fname, uint32_t name_size)
+void FileFlows::set_file_name(const uint8_t* fname, uint32_t name_size, uint64_t file_id)
 {
-    FileContext* context = get_current_file_context();
+    FileContext* context;
+    if (file_id)
+        context = get_file_context(file_id, false);
+    else
+        context = get_current_file_context();
     if ( !context )
         return;
 
diff --git a/src/file_api/file_flows.h b/src/file_api/file_flows.h
index 12703039b..a7268ff49 100644
--- a/src/file_api/file_flows.h
+++ b/src/file_api/file_flows.h
@@ -69,12 +69,17 @@ public:
 
     void set_current_file_context(FileContext*);
 
-    // Get file context based on file id, create it if not existed
-    FileContext* get_file_context(uint64_t file_id, bool to_create);
+    // Get file context based on file id, create it if does not exist
+    FileContext* get_file_context(uint64_t file_id, bool to_create,
+        uint64_t multi_file_processing_id=0);
+    // Get a partially processed file context from the flow object
+    FileContext* get_partially_processed_context(uint64_t file_id);
+    // Remove a file from the flow object when processing is complete
+    void remove_processed_file_context(uint64_t file_id);
 
     uint64_t get_new_file_instance();
 
-    void set_file_name(const uint8_t* fname, uint32_t name_size);
+    void set_file_name(const uint8_t* fname, uint32_t name_size, uint64_t file_id=0);
 
     void set_sig_gen_state( bool enable )
     {
@@ -89,7 +94,8 @@ public:
 
     // This is used for each file context. Support multiple files per session
     bool file_process(Packet* p, uint64_t file_id, const uint8_t* file_data,
-        int data_size, uint64_t offset, FileDirection);
+        int data_size, uint64_t offset, FileDirection, uint64_t multi_file_processing_id=0,
+        FilePosition=SNORT_FILE_POSITION_UNKNOWN);
 
     static unsigned file_flow_data_id;
 
@@ -99,8 +105,6 @@ public:
     size_t size_of() override
     { return sizeof(*this); }
 
-    void remove_file_context(uint64_t file_id);
-
 private:
     void init_file_context(FileDirection, FileContext*);
     FileContext* find_main_file_context(FilePosition, FileDirection, size_t id = 0);
@@ -112,7 +116,8 @@ private:
     Flow* flow = nullptr;
     FilePolicyBase* file_policy = nullptr;
 
-    std::unordered_map<uint64_t, FileContext*> flow_file_contexts;
+    std::unordered_map<uint64_t, FileContext*> partially_processed_contexts;
+    bool current_context_delete_pending = false;
     FileEventGen events;
 };
 }
diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc
index 5d7b80197..ec38336b8 100644
--- a/src/file_api/file_lib.cc
+++ b/src/file_api/file_lib.cc
@@ -488,11 +488,11 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size,
 }
 
 bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size,
-    uint64_t offset, FilePolicyBase* policy)
+    uint64_t offset, FilePolicyBase* policy, FilePosition position)
 {
     if (!file_segments)
         file_segments = new FileSegments(this);
-    return file_segments->process(p, file_data, data_size, offset, policy);
+    return file_segments->process(p, file_data, data_size, offset, policy, position);
 }
 
 /*
diff --git a/src/file_api/file_lib.h b/src/file_api/file_lib.h
index c2cb8a1da..1958c213c 100644
--- a/src/file_api/file_lib.h
+++ b/src/file_api/file_lib.h
@@ -115,7 +115,8 @@ public:
     //    true: continue processing/log/block this file
     //    false: ignore this file
     bool process(Packet*, const uint8_t* file_data, int data_size, FilePosition, FilePolicyBase*);
-    bool process(Packet*, const uint8_t* file_data, int data_size, uint64_t offset, FilePolicyBase*);
+    bool process(Packet*, const uint8_t* file_data, int data_size, uint64_t offset, FilePolicyBase*,
+        FilePosition position=SNORT_FILE_POSITION_UNKNOWN);
     void process_file_type(const uint8_t* file_data, int data_size, FilePosition);
     void process_file_signature_sha256(const uint8_t* file_data, int data_size, FilePosition);
     void update_file_size(int data_size, FilePosition position);
diff --git a/src/file_api/file_segment.cc b/src/file_api/file_segment.cc
index 70145b5bc..149103e82 100644
--- a/src/file_api/file_segment.cc
+++ b/src/file_api/file_segment.cc
@@ -153,9 +153,10 @@ FilePosition FileSegments::get_file_position(uint64_t data_size, uint64_t file_s
 }
 
 int FileSegments::process_one(Packet* p, const uint8_t* file_data, int data_size,
-    FilePolicyBase* policy)
+    FilePolicyBase* policy, FilePosition position)
 {
-    FilePosition position = get_file_position(data_size, context->get_file_size());
+    if (position == SNORT_FILE_POSITION_UNKNOWN)
+        position = get_file_position(data_size, context->get_file_size());
 
     return context->process(p, file_data, data_size, position, policy);
 }
@@ -193,7 +194,7 @@ int FileSegments::process_all(Packet* p, FilePolicyBase* policy)
  *    0: ignore this file
  */
 int FileSegments::process(Packet* p, const uint8_t* file_data, uint64_t data_size,
-    uint64_t offset, FilePolicyBase* policy)
+    uint64_t offset, FilePolicyBase* policy, FilePosition position)
 {
     int ret = 0;
 
@@ -205,7 +206,7 @@ int FileSegments::process(Packet* p, const uint8_t* file_data, uint64_t data_siz
     // Walk through the segments that can be flushed
     if (current_offset == offset)
     {
-        ret =  process_one(p, file_data, data_size, policy);
+        ret =  process_one(p, file_data, data_size, policy, position);
         current_offset += data_size;
         if (!ret)
         {
diff --git a/src/file_api/file_segment.h b/src/file_api/file_segment.h
index d644f33a2..de00a774d 100644
--- a/src/file_api/file_segment.h
+++ b/src/file_api/file_segment.h
@@ -55,7 +55,7 @@ public:
     // Process file segments with current_offset specified. If file segment is out of order,
     // it will be put into the file segments queue.
     int process(snort::Packet*, const uint8_t* file_data, uint64_t data_size, uint64_t offset,
-        snort::FilePolicyBase*);
+        snort::FilePolicyBase*, FilePosition position=SNORT_FILE_POSITION_UNKNOWN);
 
 private:
     FileSegment* head = nullptr;
@@ -64,7 +64,8 @@ private:
 
     void add(const uint8_t* file_data, uint64_t data_size, uint64_t offset);
     FilePosition get_file_position(uint64_t data_size, uint64_t file_size);
-    int process_one(snort::Packet*, const uint8_t* file_data, int data_size, snort::FilePolicyBase*);
+    int process_one(snort::Packet*, const uint8_t* file_data, int data_size, snort::FilePolicyBase*,
+        FilePosition position=SNORT_FILE_POSITION_UNKNOWN);
     int process_all(snort::Packet*, snort::FilePolicyBase*);
 };
 
diff --git a/src/service_inspectors/http_inspect/http_msg_body.cc b/src/service_inspectors/http_inspect/http_msg_body.cc
index 38173f923..26e8bb88f 100644
--- a/src/service_inspectors/http_inspect/http_msg_body.cc
+++ b/src/service_inspectors/http_inspect/http_msg_body.cc
@@ -224,7 +224,7 @@ void HttpMsgBody::do_file_processing(const Field& file_data)
     if (!session_data->mime_state[source_id])
     {
         FileFlows* file_flows = FileFlows::get_file_flows(flow);
-        const bool download = (source_id == SRC_SERVER);
+        const FileDirection dir = source_id == SRC_SERVER ? FILE_DOWNLOAD : FILE_UPLOAD;
 
         size_t file_index = 0;
 
@@ -233,8 +233,8 @@ void HttpMsgBody::do_file_processing(const Field& file_data)
             file_index = request->get_http_uri()->get_file_proc_hash();
         }
 
-        if (file_flows->file_process(p, file_data.start(), fp_length,
-            file_position, !download, file_index))
+        if (file_flows->file_process(p, file_index, file_data.start(), fp_length, body_octets, dir,
+            transaction->get_file_processing_id(source_id), file_position))
         {
             session_data->file_depth_remaining[source_id] -= fp_length;
 
diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc
index 86799351f..1bcd3bf3e 100644
--- a/src/service_inspectors/http_inspect/http_msg_header.cc
+++ b/src/service_inspectors/http_inspect/http_msg_header.cc
@@ -312,53 +312,45 @@ void HttpMsgHeader::prepare_body()
 
 void HttpMsgHeader::setup_file_processing()
 {
-    // FIXIT-M Bidirectional file processing is problematic so we don't do it. When the library
-    // fully supports it remove the outer if statement that prevents it from being done.
-    if (session_data->file_depth_remaining[1-source_id] <= 0)
+    // Generate the unique file id for file processing
+    transaction->set_file_processing_id(source_id, get_transaction_id());
+
+    if ((session_data->file_depth_remaining[source_id] = FileService::get_max_file_depth()) < 0)
     {
-        if ((session_data->file_depth_remaining[source_id] = FileService::get_max_file_depth())
-             < 0)
-        {
-           session_data->file_depth_remaining[source_id] = 0;
-           return;
-        }
+        session_data->file_depth_remaining[source_id] = 0;
+        return;
+    }
 
-        // Do we meet all the conditions for MIME file processing?
-        if (source_id == SRC_CLIENT)
+    // Do we meet all the conditions for MIME file processing?
+    if (source_id == SRC_CLIENT)
+    {
+        const Field& content_type = get_header_value_raw(HEAD_CONTENT_TYPE);
+        if (content_type.length() > 0)
         {
-            const Field& content_type = get_header_value_raw(HEAD_CONTENT_TYPE);
-            if (content_type.length() > 0)
+            if (boundary_present(content_type))
             {
-                if (boundary_present(content_type))
-                {
-                    session_data->mime_state[source_id] =
-                        new MimeSession(&decode_conf, &mime_conf);
-                    // Show file processing the Content-Type header as if it were regular data.
-                    // This will enable it to find the boundary string.
-                    // FIXIT-L develop a proper interface for passing the boundary string.
-                    // This interface is a leftover from when OHI pushed whole messages through
-                    // this interface.
-                    Packet* p = DetectionEngine::get_current_packet();
-                    session_data->mime_state[source_id]->process_mime_data(p,
-                        content_type.start(), content_type.length(), true,
-                        SNORT_FILE_POSITION_UNKNOWN);
-                    session_data->mime_state[source_id]->process_mime_data(p,
-                        (const uint8_t*)"\r\n", 2, true, SNORT_FILE_POSITION_UNKNOWN);
-                }
+                session_data->mime_state[source_id] = new MimeSession(&decode_conf, &mime_conf);
+                // Show file processing the Content-Type header as if it were regular data.
+                // This will enable it to find the boundary string.
+                // FIXIT-L develop a proper interface for passing the boundary string.
+                // This interface is a leftover from when OHI pushed whole messages through
+                // this interface.
+                Packet* p = DetectionEngine::get_current_packet();
+                session_data->mime_state[source_id]->process_mime_data(p,
+                    content_type.start(), content_type.length(), true,
+                    SNORT_FILE_POSITION_UNKNOWN);
+                session_data->mime_state[source_id]->process_mime_data(p,
+                    (const uint8_t*)"\r\n", 2, true, SNORT_FILE_POSITION_UNKNOWN);
             }
         }
-
-        // Otherwise do regular file processing
-        if (session_data->mime_state[source_id] == nullptr)
-        {
-            FileFlows* file_flows = FileFlows::get_file_flows(flow);
-            if (!file_flows)
-                session_data->file_depth_remaining[source_id] = 0;
-        }
     }
-    else
+
+    // Otherwise do regular file processing
+    if (session_data->mime_state[source_id] == nullptr)
     {
-        session_data->file_depth_remaining[source_id] = 0;
+        FileFlows* file_flows = FileFlows::get_file_flows(flow);
+        if (!file_flows)
+            session_data->file_depth_remaining[source_id] = 0;
     }
 }
 
diff --git a/src/service_inspectors/http_inspect/http_msg_section.h b/src/service_inspectors/http_inspect/http_msg_section.h
index d43cbc0b8..95c4569e5 100644
--- a/src/service_inspectors/http_inspect/http_msg_section.h
+++ b/src/service_inspectors/http_inspect/http_msg_section.h
@@ -75,6 +75,8 @@ public:
     void clear();
     bool is_clear() { return cleared; }
 
+    uint64_t get_transaction_id() { return trans_num; }
+
     HttpMsgSection* next = nullptr;
 
 #ifdef REG_TEST
diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc
index 643ba4c3f..d4b0a833b 100644
--- a/src/service_inspectors/http_inspect/http_transaction.cc
+++ b/src/service_inspectors/http_inspect/http_transaction.cc
@@ -32,8 +32,11 @@
 #include "http_msg_status.h"
 #include "http_msg_trailer.h"
 
+#include "hash/hashfcn.h"
+
 using namespace HttpCommon;
 using namespace HttpEnums;
+using namespace snort;
 
 static void delete_section_list(HttpMsgSection* section_list)
 {
@@ -265,3 +268,14 @@ void HttpTransaction::set_one_hundred_response()
     second_response_expected = true;
 }
 
+void HttpTransaction::set_file_processing_id(const SourceId source_id,
+    const uint64_t transaction_id)
+{
+    const int data_len = sizeof(source_id) + sizeof(transaction_id);
+    uint8_t data[data_len];
+    memcpy(data, (void*)&source_id, sizeof(source_id));
+    uint32_t offset = sizeof(source_id);
+    memcpy(data + offset, (void*)&transaction_id, sizeof(transaction_id));
+
+    file_processing_id[source_id] = str_to_hash(data, data_len);
+}
diff --git a/src/service_inspectors/http_inspect/http_transaction.h b/src/service_inspectors/http_inspect/http_transaction.h
index 26b319cf0..ddeedcbb6 100644
--- a/src/service_inspectors/http_inspect/http_transaction.h
+++ b/src/service_inspectors/http_inspect/http_transaction.h
@@ -71,6 +71,12 @@ public:
 
     HttpTransaction* next = nullptr;
 
+    // Each file processed has a unique id per flow: hash(source_id, transaction_id)
+    void set_file_processing_id(const HttpCommon::SourceId source_id,
+        const uint64_t transaction_id);
+    uint64_t get_file_processing_id(HttpCommon::SourceId source_id)
+        { return file_processing_id[source_id]; }
+
 private:
     HttpTransaction() = default;
     void discard_section(HttpMsgSection* section);
@@ -86,6 +92,8 @@ private:
     HttpInfractions* infractions[2] = { nullptr, nullptr };
     HttpEventGen* events[2] = { nullptr, nullptr };
 
+    uint64_t file_processing_id[2] = { 0, 0 };
+
     bool response_seen = false;
     bool one_hundred_response = false;
     bool second_response_expected = false;
diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc
index 9c727e2a2..d93c03f8c 100644
--- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc
+++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc
@@ -45,6 +45,7 @@ FlowData::FlowData(unsigned, Inspector*) {}
 FlowData::~FlowData() = default;
 int DetectionEngine::queue_event(unsigned int, unsigned int, Actions::Type) { return 0; }
 fd_status_t File_Decomp_StopFree(fd_session_t*) { return File_Decomp_OK; }
+size_t str_to_hash(unsigned char const*, size_t) { return 0; }
 }
 
 THREAD_LOCAL PegCount HttpModule::peg_counts[1];