From: Russ Combs (rucombs) Date: Tue, 18 Feb 2020 01:02:52 +0000 (+0000) Subject: Merge pull request #2012 in SNORT/snort3 from ~RUCOMBS/snort3:new_stuff to master X-Git-Tag: 3.0.0-268~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eba0666ec70337bd520b9b142df68d521db6fed7;p=thirdparty%2Fsnort3.git Merge pull request #2012 in SNORT/snort3 from ~RUCOMBS/snort3:new_stuff to master Squashed commit of the following: commit 442e97ad2054e74f008a9f800f1e99930e24e8af Author: russ Date: Sat Feb 15 10:27:32 2020 -0500 gtp_inspect: fix default port binding The default snort.lua had a port binding to type = 'gtp' which is a typo. The service is 'gtp' and the inspector is 'gtp_inspect'. Due to a flaw in lookup, the inspector was being matched by service. To avoid confusing type and service the lookups were separated. However, we silenty covert the old type = 'gtp' bidings to type = 'gtp_inspect' until RC at which point this deprecated usage support will be removed. commit 215bd1e4829550183ae36198a3764245a3669cba Author: russ Date: Sat Feb 15 10:08:29 2020 -0500 inspectors: ensure correct lookup by type, name, or service commit db649915eddbf805d9f587dd985ec9bd254b4f37 Author: russ Date: Sat Feb 8 10:12:19 2020 -0500 metadata: add --metadata-filter to load matching rules only Rule metadata is a comma separated list of name-value tokens, eg: metadata:impact_flag red,policy security-ips drop,ruleset community; --metadata-filter f will load only rules where f appears in one of the metadata tokens. "policy security" and "security-ips" would both match the above example. Rules that are filtered out are counted as "total rules not loaded" in the startup output. --- diff --git a/lua/snort.lua b/lua/snort.lua index f7ac1adc4..7c91abdc6 100644 --- a/lua/snort.lua +++ b/lua/snort.lua @@ -114,7 +114,7 @@ binder = { when = { proto = 'tcp', ports = '53', role='server' }, use = { type = 'dns' } }, { when = { proto = 'tcp', ports = '111', role='server' }, use = { type = 'rpc_decode' } }, { when = { proto = 'tcp', ports = '502', role='server' }, use = { type = 'modbus' } }, - { when = { proto = 'tcp', ports = '2123 2152 3386', role='server' }, use = { type = 'gtp' } }, + { when = { proto = 'tcp', ports = '2123 2152 3386', role='server' }, use = { type = 'gtp_inspect' } }, { when = { proto = 'tcp', service = 'dcerpc' }, use = { type = 'dce_tcp' } }, { when = { proto = 'udp', service = 'dcerpc' }, use = { type = 'dce_udp' } }, diff --git a/src/detection/treenodes.h b/src/detection/treenodes.h index 9e7f19580..73d86697d 100644 --- a/src/detection/treenodes.h +++ b/src/detection/treenodes.h @@ -152,6 +152,7 @@ struct OptTreeNode static constexpr Flag WARNED_FP = 0x01; static constexpr Flag STATELESS = 0x02; static constexpr Flag RULE_STATE = 0x04; + static constexpr Flag META_MATCH = 0x08; /* metadata about signature */ SigInfo sigInfo; @@ -183,19 +184,19 @@ struct OptTreeNode { flags |= WARNED_FP; } bool warned_fp() const - { return flags & WARNED_FP; } + { return (flags & WARNED_FP) != 0; } void set_stateless() { flags |= STATELESS; } bool stateless() const - { return flags & STATELESS; } + { return (flags & STATELESS) != 0; } void set_enabled(IpsPolicy::Enable e) { enable = e; flags |= RULE_STATE; } - bool is_rule_state_stub() - { return flags & RULE_STATE; } + bool is_rule_state_stub() const + { return (flags & RULE_STATE) != 0; } bool enabled_somewhere() const { @@ -205,6 +206,12 @@ struct OptTreeNode return false; } + + void set_metadata_match() + { flags |= META_MATCH; } + + bool metadata_matched() const + { return (flags & META_MATCH) != 0; } }; typedef int (* RuleOptEvalFunc)(void*, Cursor&, snort::Packet*); diff --git a/src/ips_options/ips_metadata.cc b/src/ips_options/ips_metadata.cc index 53296727d..962882de0 100644 --- a/src/ips_options/ips_metadata.cc +++ b/src/ips_options/ips_metadata.cc @@ -21,9 +21,11 @@ #include "config.h" #endif +#include "detection/treenodes.h" #include "framework/decode_data.h" #include "framework/ips_option.h" #include "framework/module.h" +#include "main/snort_config.h" using namespace snort; using namespace std; @@ -43,23 +45,36 @@ static const Parameter s_params[] = }; #define s_help \ - "rule option for conveying arbitrary name, value data within the rule text" + "rule option for conveying arbitrary comma-separated name, value data within the rule text" class MetadataModule : public Module { public: MetadataModule() : Module(s_name, s_help, s_params) { } + + bool begin(const char*, int, SnortConfig*) override; bool set(const char*, Value&, SnortConfig*) override; Usage get_usage() const override { return DETECT; } + + bool match; }; -bool MetadataModule::set(const char*, Value& v, SnortConfig*) +bool MetadataModule::begin(const char*, int, SnortConfig*) +{ + match = false; + return true; +} + +bool MetadataModule::set(const char*, Value& v, SnortConfig* sc) { if ( !v.is("*") ) return false; + if ( !match and !sc->metadata_filter.empty() ) + match = strstr(v.get_string(), sc->metadata_filter.c_str()) != nullptr; + return true; } @@ -77,8 +92,11 @@ static void mod_dtor(Module* m) delete m; } -static IpsOption* metadata_ctor(Module*, OptTreeNode*) +static IpsOption* metadata_ctor(Module* p, OptTreeNode* otn) { + MetadataModule* m = (MetadataModule*)p; + if ( m->match ) + otn->set_metadata_match(); return nullptr; } diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc index 9e1157fef..e60bc5a3c 100644 --- a/src/main/snort_config.cc +++ b/src/main/snort_config.cc @@ -450,13 +450,14 @@ void SnortConfig::merge(SnortConfig* cmd_line) file_mask = cmd_line->file_mask; if ( !cmd_line->chroot_dir.empty() ) - { chroot_dir = cmd_line->chroot_dir; - } if ( cmd_line->dirty_pig ) dirty_pig = cmd_line->dirty_pig; + if ( !cmd_line->metadata_filter.empty() ) + metadata_filter = cmd_line->metadata_filter; + daq_config->overlay(cmd_line->daq_config); if (cmd_line->mpls_stack_depth != DEFAULT_LABELCHAIN_LENGTH) diff --git a/src/main/snort_config.h b/src/main/snort_config.h index a636d806e..d379f902a 100644 --- a/src/main/snort_config.h +++ b/src/main/snort_config.h @@ -326,6 +326,7 @@ public: uint32_t event_log_id = 0; SfCidr obfuscation_net; std::string bpf_filter; + std::string metadata_filter; //------------------------------------------------------ // FIXIT-L non-module stuff - separate config from derived state? diff --git a/src/main/snort_module.cc b/src/main/snort_module.cc index b6b9e6841..9720f3017 100644 --- a/src/main/snort_module.cc +++ b/src/main/snort_module.cc @@ -419,6 +419,9 @@ static const Parameter s_params[] = { "--mem-check", Parameter::PT_IMPLIED, nullptr, nullptr, "like -T but also compile search engines" }, + { "--metadata-filter", Parameter::PT_STRING, nullptr, nullptr, + " load only rules containing filter string in metadata if set" }, + { "--nostamps", Parameter::PT_IMPLIED, nullptr, nullptr, "don't include timestamps in log file names" }, @@ -901,6 +904,9 @@ bool SnortModule::set(const char*, Value& v, SnortConfig* sc) else if ( v.is("--mem-check") ) sc->run_flags |= (RUN_FLAG__TEST | RUN_FLAG__MEM_CHECK); + else if ( v.is("--metadata-filter") ) + sc->metadata_filter = v.get_string(); + else if ( v.is("--nostamps") ) sc->set_no_logging_timestamps(true); diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index 2ebb73c18..68e804d52 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -382,32 +382,46 @@ void InspectorManager::empty_trash() // policy stuff //------------------------------------------------------------------------- -// FIXIT-L allowing lookup by name or type or key is kinda hinky -// would be helpful to have specific lookups static bool get_instance( - FrameworkPolicy* fp, const char* keyword, bool dflt_only, + FrameworkPolicy* fp, const char* keyword, std::vector::iterator& it) { for ( it = fp->ilist.begin(); it != fp->ilist.end(); ++it ) { - PHInstance* p = *it; - if ( !p->name.empty() && p->name == keyword ) + if ( (*it)->name == keyword ) return true; + } + return false; +} - else if ( !strcmp(p->pp_class.api.base.name, keyword) ) - return (p->name.empty() || !dflt_only) ? true : false; +static PHInstance* get_instance_by_type(FrameworkPolicy* fp, const char* keyword) +{ + std::vector::iterator it; - else if ( p->pp_class.api.service && !strcmp(p->pp_class.api.service, keyword) ) - return true; + for ( it = fp->ilist.begin(); it != fp->ilist.end(); ++it ) + { + if ( !strcmp((*it)->pp_class.api.base.name, keyword) ) + return *it; } - return false; + return nullptr; +} + +static PHInstance* get_instance_by_service(FrameworkPolicy* fp, const char* keyword) +{ + std::vector::iterator it; + + for ( it = fp->ilist.begin(); it != fp->ilist.end(); ++it ) + { + if ( (*it)->pp_class.api.service && !strcmp((*it)->pp_class.api.service, keyword) ) + return *it; + } + return nullptr; } -static PHInstance* get_instance( - FrameworkPolicy* fp, const char* keyword, bool dflt_only = false) +static PHInstance* get_instance(FrameworkPolicy* fp, const char* keyword) { std::vector::iterator it; - return get_instance(fp, keyword, dflt_only, it) ? *it : nullptr; + return get_instance(fp, keyword, it) ? *it : nullptr; } static PHInstance* get_new( @@ -417,7 +431,7 @@ static PHInstance* get_new( bool reloaded = false; std::vector::iterator old_it; - if ( get_instance(fp, keyword, false, old_it) ) + if ( get_instance(fp, keyword, old_it) ) { if ( Snort::is_reloading() ) { @@ -520,7 +534,7 @@ bool InspectorManager::inspector_exists_in_any_policy(const char* key, SnortConf if ( !pi || !pi->framework_policy ) continue; - const PHInstance* const p = get_instance(pi->framework_policy, key); + const PHInstance* const p = get_instance_by_type(pi->framework_policy, key); if ( p ) return true; @@ -544,7 +558,22 @@ Inspector* InspectorManager::get_inspector(const char* key, bool dflt_only, Snor if ( !pi || !pi->framework_policy ) return nullptr; - PHInstance* p = get_instance(pi->framework_policy, key, dflt_only); + PHInstance* p = get_instance(pi->framework_policy, key); + + if ( !p ) + return nullptr; + + return p->handler; +} + +Inspector* InspectorManager::get_inspector_by_service(const char* key) +{ + InspectionPolicy* pi = get_inspection_policy(); + + if ( !pi || !pi->framework_policy ) + return nullptr; + + PHInstance* p = get_instance_by_service(pi->framework_policy, key); if ( !p ) return nullptr; @@ -558,13 +587,13 @@ bool InspectorManager::delete_inspector(SnortConfig* sc, const char* iname) FrameworkPolicy* fp = sc->policy_map->get_inspection_policy()->framework_policy; std::vector::iterator old_it; - if ( get_instance(fp, iname, false, old_it) ) + if ( get_instance(fp, iname, old_it) ) { (*old_it)->set_reloaded(RELOAD_TYPE_DELETED); fp->ilist.erase(old_it); ok = true; std::vector::iterator bind_it; - if ( get_instance(fp, bind_id, false, bind_it) ) + if ( get_instance(fp, bind_id, bind_it) ) { (*bind_it)->handler->remove_inspector_binding(sc, iname); } @@ -780,11 +809,10 @@ void InspectorManager::instantiate( PHInstance* ppi = get_new(ppc, fp, keyword, mod, sc); - if ( !ppi ) + if ( ppi ) + ppi->set_name(keyword); + else ParseError("can't instantiate inspector: '%s'.", keyword); - - else if ( name ) - ppi->set_name(name); } } @@ -872,7 +900,7 @@ static bool configure(SnortConfig* sc, FrameworkPolicy* fp, bool cloned) if ( new_ins or reenabled_ins ) { std::vector::iterator old_binder; - if ( get_instance(fp, bind_id, false, old_binder) ) + if ( get_instance(fp, bind_id, old_binder) ) { if ( new_ins and fp->default_binder ) { @@ -955,7 +983,7 @@ void InspectorManager::print_config(SnortConfig* sc) for ( auto* p : pi->framework_policy->ilist ) { std::string inspector_name(p->pp_class.api.base.name); - if ( !p->name.empty() ) + if ( p->name != inspector_name ) inspector_name += " (" + p->name + "):"; else inspector_name += ":"; diff --git a/src/managers/inspector_manager.h b/src/managers/inspector_manager.h index f18c38632..d2248aa94 100644 --- a/src/managers/inspector_manager.h +++ b/src/managers/inspector_manager.h @@ -60,6 +60,7 @@ public: SO_PUBLIC static Inspector* get_inspector(const char* key, bool dflt_only = false, SnortConfig* sc = nullptr); + SO_PUBLIC static Inspector* get_inspector_by_service(const char*); SO_PUBLIC static Binder* get_binder(); diff --git a/src/network_inspectors/binder/bind_module.cc b/src/network_inspectors/binder/bind_module.cc index 45aea6681..e8fad1044 100644 --- a/src/network_inspectors/binder/bind_module.cc +++ b/src/network_inspectors/binder/bind_module.cc @@ -279,6 +279,8 @@ bool BinderModule::set(const char* fqn, Value& v, SnortConfig*) else if ( v.is("type") ) { work->use.type = v.get_string(); + if ( work->use.type == "gtp" ) + work->use.type = "gtp_inspect"; use_type_count++; } else diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc index ece0ec4f4..9790710dc 100644 --- a/src/network_inspectors/binder/binder.cc +++ b/src/network_inspectors/binder/binder.cc @@ -445,14 +445,14 @@ static Inspector* get_gadget(Flow* flow) const char* s = SnortConfig::get_conf()->proto_ref->get_name(flow->ssn_state.snort_protocol_id); - return InspectorManager::get_inspector(s); + return InspectorManager::get_inspector_by_service(s); } static Inspector* get_gadget_by_id(const char* service) { const SnortProtocolId id = SnortConfig::get_conf()->proto_ref->find(service); const char* s = SnortConfig::get_conf()->proto_ref->get_name(id); - return InspectorManager::get_inspector(s); + return InspectorManager::get_inspector_by_service(s); } //------------------------------------------------------------------------- @@ -889,15 +889,13 @@ void Binder::set_binding(SnortConfig* sc, Binding* pb) if ( pb->use.action != BindUse::BA_INSPECT ) return; - const char* key; - if ( pb->use.svc.empty() ) - key = pb->use.name.c_str(); - else - key = pb->use.svc.c_str(); - + const char* key = pb->use.name.c_str(); Module* m = ModuleManager::get_module(key); bool is_global = m ? m->get_usage() == Module::GLOBAL : false; - if ( (pb->use.object = InspectorManager::get_inspector(key, is_global, sc)) ) + + pb->use.object = InspectorManager::get_inspector(key, is_global, sc); + + if ( pb->use.object ) { switch ( ((Inspector*)pb->use.object)->get_api()->type ) { diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index e01b2621d..fae711f51 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -68,6 +68,7 @@ struct rule_count_t }; static int rule_count = 0; +static int skip_count = 0; static int detect_rule_count = 0; static int builtin_rule_count = 0; static int so_rule_count = 0; @@ -860,6 +861,7 @@ int get_rule_count() void parse_rule_init() { rule_count = 0; + skip_count = 0; detect_rule_count = 0; builtin_rule_count = 0; so_rule_count = 0; @@ -884,6 +886,7 @@ void parse_rule_print() LogLabel("rule counts"); LogCount("total rules loaded", rule_count); + LogCount("total rules not loaded", skip_count); LogCount("text rules", detect_rule_count); LogCount("builtin rules", builtin_rule_count); LogCount("so rules", so_rule_count); @@ -1118,6 +1121,7 @@ void parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* otn) if ( s_ignore ) { s_ignore = false; + skip_count++; return; } @@ -1157,6 +1161,15 @@ void parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* otn) return; } + if ( !sc->metadata_filter.empty() and !otn->metadata_matched() ) + { + OtnFree(otn); + FreeRuleTreeNode(&rtn); + ClearIpsOptionsVars(); + skip_count++; + return; + } + /* The IPs in the test node get freed in ProcessHeadNode if there is * already a matching RTN. The portobjects will get freed when the * port var table is freed */ @@ -1234,7 +1247,6 @@ void parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* otn) sc->port_tables, new_rtn, otn, rtn.snort_protocol_id, sc->fast_pattern_config) ) ParseError("Failed to finish a port list rule."); - // Clear ips_option vars ClearIpsOptionsVars(); }