From: Mike Stepanek (mstepane) Date: Tue, 17 Dec 2019 13:48:33 +0000 (+0000) Subject: Merge pull request #1891 in SNORT/snort3 from ~KATHARVE/snort3:multiple_file_contexts... X-Git-Tag: 3.0.0-267~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=879654568d31371608d03fe5d7aa338c7ec5e50c;p=thirdparty%2Fsnort3.git Merge pull request #1891 in SNORT/snort3 from ~KATHARVE/snort3:multiple_file_contexts_fix to master Squashed commit of the following: commit 7c77f290e40591555e152bb286838efc08054758 Author: Katura Harvey Date: Fri Dec 6 14:00:17 2019 -0500 file_api: When multiple files are processed simultaneously per flow, store the files on the flow, not in the cache. Don't cache files until the signature has been computed --- diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc index 8e94f8969..8d323ef6c 100644 --- a/src/file_api/file_cache.cc +++ b/src/file_api/file_cache.cc @@ -207,7 +207,7 @@ FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create, hashKey.file_id = file_id; FileContext* file = find(hashKey, timeout); if (to_create and !file) - file = add(hashKey, timeout); + file = add(hashKey, timeout); return file; } @@ -225,14 +225,12 @@ FileVerdict FileCache::check_verdict(Packet* p, FileInfo* file, FileVerdict verdict = policy->type_lookup(p, file); if ( file->get_file_sig_sha256() and - ((verdict == FILE_VERDICT_UNKNOWN) || - (verdict == FILE_VERDICT_STOP_CAPTURE))) + ((verdict == FILE_VERDICT_UNKNOWN) or (verdict == FILE_VERDICT_STOP_CAPTURE))) { verdict = policy->signature_lookup(p, file); } - if ((verdict == FILE_VERDICT_UNKNOWN) || - (verdict == FILE_VERDICT_STOP_CAPTURE)) + if ((verdict == FILE_VERDICT_UNKNOWN) or (verdict == FILE_VERDICT_STOP_CAPTURE)) { verdict = file->verdict; } @@ -286,6 +284,9 @@ bool FileCache::apply_verdict(Packet* p, FileContext* file_ctx, FileVerdict verd // can't reset session inside a session act->set_delayed_action(Active::ACT_RESET, true); break; + case FILE_VERDICT_STOP_CAPTURE: + file_ctx->stop_file_capture(); + return false; case FILE_VERDICT_PENDING: packet_gettimeofday(&now); diff --git a/src/file_api/file_config.h b/src/file_api/file_config.h index 1345a1ee3..2c83da5ad 100644 --- a/src/file_api/file_config.h +++ b/src/file_api/file_config.h @@ -37,6 +37,7 @@ #define DEFAULT_FILE_CAPTURE_MIN_SIZE 0 // 0 #define DEFAULT_FILE_CAPTURE_BLOCK_SIZE 32768 // 32 KiB #define DEFAULT_MAX_FILES_CACHED 65536 +#define DEFAULT_MAX_FILES_PER_FLOW 32 #define FILE_ID_NAME "file_id" #define FILE_ID_HELP "configure file identification" @@ -65,6 +66,7 @@ public: int64_t capture_block_size = DEFAULT_FILE_CAPTURE_BLOCK_SIZE; int64_t file_depth = 0; int64_t max_files_cached = DEFAULT_MAX_FILES_CACHED; + uint64_t max_files_per_flow = DEFAULT_MAX_FILES_PER_FLOW; int64_t show_data_depth = DEFAULT_FILE_SHOW_DATA_DEPTH; bool trace_type = false; diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc index 88377fc7f..91b9ce08a 100644 --- a/src/file_api/file_flows.cc +++ b/src/file_api/file_flows.cc @@ -29,6 +29,7 @@ #include "file_flows.h" +#include "detection/detection_engine.h" #include "main/snort_config.h" #include "managers/inspector_manager.h" #include "protocols/packet.h" @@ -36,7 +37,9 @@ #include "file_cache.h" #include "file_config.h" #include "file_lib.h" +#include "file_module.h" #include "file_service.h" +#include "file_stats.h" using namespace snort; @@ -139,6 +142,12 @@ uint64_t FileFlows::get_new_file_instance() FileFlows::~FileFlows() { delete(main_context); + + // Delete any remaining FileContexts stored on the flow + for (auto const& elem : flow_file_contexts) + { + delete elem.second; + } } FileContext* FileFlows::find_main_file_context(FilePosition pos, FileDirection dir, size_t index) @@ -168,15 +177,53 @@ FileContext* FileFlows::find_main_file_context(FilePosition pos, FileDirection d FileContext* FileFlows::get_file_context(uint64_t file_id, bool to_create) { - // search for file based on id to support multiple files - FileCache* file_cache = FileService::get_file_cache(); - assert(file_cache); + FileContext *context = nullptr; + + // 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 + { + FileCache* file_cache = FileService::get_file_cache(); + assert(file_cache); + context = file_cache->get_file(flow, file_id, false); + } + + // If we haven't found the context, create it and store it on the file flows object + if (!context and 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) + { + file_counts.files_over_flow_limit_not_processed++; + events.create_event(EVENT_FILE_DROPPED_OVER_LIMIT); + } + 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(); + } + } - FileContext* context = file_cache->get_file(flow, file_id, to_create); current_file_id = file_id; return context; } +void FileFlows::remove_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); +} + /* This function is used to process file that is sent in pieces * * Return: @@ -187,8 +234,9 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da int data_size, uint64_t offset, FileDirection dir) { int64_t file_depth = FileService::get_max_file_depth(); + bool continue_processing; - if ((file_depth < 0)or (offset > (uint64_t)file_depth)) + if ((file_depth < 0) or (offset > (uint64_t)file_depth)) { return false; } @@ -204,21 +252,27 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da context->set_file_id(file_id); } - if (context->verdict != FILE_VERDICT_UNKNOWN) + if (context->processing_complete and context->verdict != FILE_VERDICT_UNKNOWN) { /*A new file session, but policy might be different*/ context->check_policy(flow, dir, file_policy); - if ((context->get_file_sig_sha256()) - || !context->is_file_signature_enabled()) + if ((context->get_file_sig_sha256()) || !context->is_file_signature_enabled()) { /* Just check file type and signature */ FilePosition position = SNORT_FILE_FULL; - return context->process(p, file_data, data_size, position, file_policy); + continue_processing = context->process(p, file_data, data_size, position, + file_policy); + if (context->processing_complete) + remove_file_context(file_id); + return continue_processing; } } - return context->process(p, file_data, data_size, offset, file_policy); + continue_processing = context->process(p, file_data, data_size, offset, file_policy); + if (context->processing_complete) + remove_file_context(file_id); + return continue_processing; } /* diff --git a/src/file_api/file_flows.h b/src/file_api/file_flows.h index 91df59d58..12703039b 100644 --- a/src/file_api/file_flows.h +++ b/src/file_api/file_flows.h @@ -25,11 +25,15 @@ #include "flow/flow.h" #include "main/snort_types.h" +#include "utils/event_gen.h" #include "file_api.h" #include "file_module.h" #include "file_policy.h" +#include + +using FileEventGen = EventGen; namespace snort { @@ -95,6 +99,8 @@ 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); @@ -105,6 +111,9 @@ private: bool gen_signature = false; Flow* flow = nullptr; FilePolicyBase* file_policy = nullptr; + + std::unordered_map flow_file_contexts; + FileEventGen events; }; } #endif diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc index 315cf0d41..5d7b80197 100644 --- a/src/file_api/file_lib.cc +++ b/src/file_api/file_lib.cc @@ -422,6 +422,7 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, config_file_type(false); config_file_signature(false); update_file_size(data_size, position); + processing_complete = true; stop_file_capture(); return false; } @@ -631,6 +632,7 @@ void FileContext::update_file_size(int data_size, FilePosition position) { file_size = processed_bytes; processed_bytes = 0; + processing_complete = true; } } diff --git a/src/file_api/file_lib.h b/src/file_api/file_lib.h index b05d1d8ba..c2cb8a1da 100644 --- a/src/file_api/file_lib.h +++ b/src/file_api/file_lib.h @@ -80,6 +80,7 @@ public: FileState get_file_state() { return file_state; } FileVerdict verdict = FILE_VERDICT_UNKNOWN; + bool processing_complete = false; struct timeval pending_expire_time = {0, 0}; protected: diff --git a/src/file_api/file_module.cc b/src/file_api/file_module.cc index 8ab23ecfb..8817217b9 100644 --- a/src/file_api/file_module.cc +++ b/src/file_api/file_module.cc @@ -152,6 +152,9 @@ static const Parameter file_id_params[] = { "max_files_cached", Parameter::PT_INT, "8:max53", "65536", "maximal number of files cached in memory" }, + { "max_files_per_flow", Parameter::PT_INT, "1:max53", "32", + "maximal number of files able to be concurrently processed per flow" }, + { "enable_type", Parameter::PT_BOOL, nullptr, "true", "enable type ID" }, @@ -190,6 +193,8 @@ static const PegInfo file_pegs[] = { CountType::SUM, "total_files", "number of files processed" }, { CountType::SUM, "total_file_data", "number of file data bytes processed" }, { CountType::SUM, "cache_failures", "number of file cache add failures" }, + { CountType::SUM, "files_not_processed", "number of files not processed due to per-flow limit" }, + { CountType::MAX, "max_concurrent_files", "maximum files processed concurrently on a flow" }, { CountType::END, nullptr, nullptr } }; @@ -207,6 +212,17 @@ const PegInfo* FileIdModule::get_pegs() const PegCount* FileIdModule::get_counts() const { return (PegCount*)&file_counts; } +static const RuleMap file_id_rules[] = +{ + { EVENT_FILE_DROPPED_OVER_LIMIT, "file not processed due to per flow limit" }, + { 0, nullptr } +}; + +const RuleMap* FileIdModule::get_rules() const +{ + return file_id_rules; +} + void FileIdModule::sum_stats(bool accumulate_now_stats) { file_stats_sum(); @@ -250,6 +266,9 @@ bool FileIdModule::set(const char*, Value& v, SnortConfig*) else if ( v.is("max_files_cached") ) fc->max_files_cached = v.get_int64(); + else if ( v.is("max_files_per_flow") ) + fc->max_files_per_flow = v.get_uint64(); + else if ( v.is("enable_type") ) { if ( v.get_bool() ) diff --git a/src/file_api/file_module.h b/src/file_api/file_module.h index 9e5092d04..ad25c694e 100644 --- a/src/file_api/file_module.h +++ b/src/file_api/file_module.h @@ -32,6 +32,8 @@ // file_id module //------------------------------------------------------------------------- +static const uint32_t FILE_ID_GID = 150; + class FileIdModule : public snort::Module { public: @@ -54,10 +56,10 @@ public: void show_dynamic_stats() override; - // FIXIT-L delete file_id gid when bogus rules are eliminated - // (this ensures those rules don't fire on every packet) unsigned get_gid() const override - { return 146; } + { return FILE_ID_GID; } + + const snort::RuleMap* get_rules() const override; private: FileMagicRule rule; @@ -66,5 +68,12 @@ private: FileConfig *fc = nullptr; }; +enum FileSid +{ + EVENT__NONE = -1, + EVENT_FILE_DROPPED_OVER_LIMIT = 1, + EVENT__MAX_VALUE +}; + #endif diff --git a/src/file_api/file_segment.cc b/src/file_api/file_segment.cc index eec10fdd7..70145b5bc 100644 --- a/src/file_api/file_segment.cc +++ b/src/file_api/file_segment.cc @@ -218,6 +218,7 @@ int FileSegments::process(Packet* p, const uint8_t* file_data, uint64_t data_siz else if ((current_offset < context->get_file_size()) && (current_offset < offset)) { add(file_data, data_size, offset); + return 1; } return ret; diff --git a/src/file_api/file_stats.h b/src/file_api/file_stats.h index 6623f60d6..0799cf50f 100644 --- a/src/file_api/file_stats.h +++ b/src/file_api/file_stats.h @@ -35,6 +35,8 @@ struct FileCounts PegCount files_total; PegCount file_data_total; PegCount cache_add_fails; + PegCount files_over_flow_limit_not_processed; + PegCount max_concurrent_files_per_flow; PegCount files_buffered_total; PegCount files_released_total; PegCount files_freed_total; diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc index ff0389e09..afdd977dd 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2.cc +++ b/src/service_inspectors/dce_rpc/dce_smb2.cc @@ -433,6 +433,7 @@ static void DCE2_Smb2CloseCmd(DCE2_SmbSsnData* ssd, const Smb2Hdr*, DCE2_Smb2ProcessFileData(ssd, nullptr, 0, dir); } + // FIXIT-L Close should probably remove file contexts from FileFlows } /********************************************************************