From: Michael Altizer (mialtize) Date: Tue, 5 Jun 2018 16:36:09 +0000 (-0400) Subject: Merge pull request #1245 in SNORT/snort3 from policy_binder to master X-Git-Tag: 3.0.0-246~69 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=57e89252e84f84400895cc3f51de5402cff252fa;p=thirdparty%2Fsnort3.git Merge pull request #1245 in SNORT/snort3 from policy_binder to master Squashed commit of the following: commit 15692b642c595cbe3f2e91b331223c48e122b80b Author: Michael Altizer Date: Fri Jun 1 12:44:01 2018 -0400 inspector: Rename ::update() to ::remove_inspector_binding() to better reflect what it does commit ee640980e85427fe26cc87ecd2da804d93f7c1e7 Author: Michael Altizer Date: Tue May 22 16:02:39 2018 -0400 ips: Remove unused IPS module stats commit a87aa0b7ded6c2b3eab621884ab450477d5abeb9 Author: Michael Altizer Date: Tue May 22 15:13:57 2018 -0400 appid: Fix format specifier warning commit 7dbbc1cd3399b8bd233261e326b08e03ca15b8f1 Author: Michael Altizer Date: Tue May 22 15:13:21 2018 -0400 policy: Export querying policies by user ID and setting runtime policies commit b3b61cb7148b80b5b96d4a5d6e60c3bd90e89021 Author: Michael Altizer Date: Mon May 21 11:33:39 2018 -0400 packet_tracer: Report user policy IDs and add network policy commit 5ca3c3f4f0f75db35a2d5145efff115894a4b160 Author: Michael Altizer Date: Mon May 21 11:32:44 2018 -0400 policy: Add the ability to set network policy based on user-specified ID commit 3143add070f30009d0b607bc8028030dc54acd83 Author: Michael Altizer Date: Wed May 16 15:20:12 2018 -0400 binder: Make two passes at binder rules - one for policy IDs and then everything else commit 5d9e9ada1e18636a06bc9c1598997b174b4e4121 Author: Michael Altizer Date: Sat Apr 28 22:25:23 2018 -0400 profiler: Don't clobber max entry count when recursing --- diff --git a/src/framework/inspector.h b/src/framework/inspector.h index 130577b5f..75aa7800f 100644 --- a/src/framework/inspector.h +++ b/src/framework/inspector.h @@ -70,7 +70,10 @@ public: // return verification status virtual bool configure(SnortConfig*) { return true; } virtual void show(SnortConfig*) { } - virtual void update(SnortConfig*, const char*) { } + + // Specific to Binders to notify them of an inspector being removed from the policy + // FIXIT-L Probably shouldn't be part of the base Inspector class + virtual void remove_inspector_binding(SnortConfig*, const char*) { } // packet thread functions // tinit, tterm called on default policy instance only diff --git a/src/main/modules.cc b/src/main/modules.cc index 3b8fce357..7f06a5e66 100644 --- a/src/main/modules.cc +++ b/src/main/modules.cc @@ -1077,7 +1077,10 @@ bool NetworkModule::set(const char*, Value& v, SnortConfig* sc) p->decoder_drop = v.get_bool(); else if ( v.is("id") ) + { p->user_policy_id = v.get_long(); + sc->policy_map->set_user_network(p); + } else if ( v.is("min_ttl") ) p->min_ttl = (uint8_t)v.get_long(); @@ -1207,26 +1210,12 @@ static const Parameter ips_params[] = #define ips_help \ "configure IPS rule processing" -THREAD_LOCAL IpsModuleStats snort::ips_module_stats; - -const PegInfo ips_module_pegs[] = -{ - { CountType::SUM, "invalid_policy_ids", "Number of times an invalid policy ID was provided" }, - { CountType::END, nullptr, nullptr } -}; - class IpsModule : public Module { public: IpsModule() : Module("ips", ips_help, ips_params) { } bool set(const char*, Value&, SnortConfig*) override; - const PegInfo* get_pegs() const override - { return ips_module_pegs; } - - PegCount* get_counts() const override - { return (PegCount*) &ips_module_stats; } - Usage get_usage() const override { return DETECT; } }; diff --git a/src/main/modules.h b/src/main/modules.h index 4e326b348..c3b4b5f15 100644 --- a/src/main/modules.h +++ b/src/main/modules.h @@ -33,15 +33,5 @@ void module_init(); extern Trace TRACE_NAME(detection); // FIXIT-L refactor detection module out -struct IpsModuleStats -{ - PegCount invalid_policy_ids; -}; - -namespace snort -{ -SO_PUBLIC extern THREAD_LOCAL IpsModuleStats ips_module_stats; -} - #endif diff --git a/src/main/policy.cc b/src/main/policy.cc index d6806f79d..07e1cda02 100644 --- a/src/main/policy.cc +++ b/src/main/policy.cc @@ -262,22 +262,23 @@ IpsPolicy* get_ips_policy() InspectionPolicy* get_default_inspection_policy(SnortConfig* sc) { return sc->policy_map->get_inspection_policy(0); } -void set_user_ips_policy(unsigned policy_id) +void set_ips_policy(IpsPolicy* p) +{ s_detection_policy = p; } + +void set_network_policy(NetworkPolicy* p) +{ s_traffic_policy = p; } + +IpsPolicy* get_user_ips_policy(SnortConfig* sc, unsigned policy_id) { - IpsPolicy *p = SnortConfig::get_conf()->policy_map->get_user_ips(policy_id); - if(!p) - { - snort::ips_module_stats.invalid_policy_ids++; - return; - } + return sc->policy_map->get_user_ips(policy_id); +} - s_detection_policy = p; +NetworkPolicy* get_user_network_policy(SnortConfig* sc, unsigned policy_id) +{ + return sc->policy_map->get_user_network(policy_id); } } // namespace snort -void set_network_policy(NetworkPolicy* p) -{ s_traffic_policy = p; } - void set_network_policy(SnortConfig* sc, unsigned i) { PolicyMap* pm = sc->policy_map; @@ -297,9 +298,6 @@ void set_inspection_policy(SnortConfig* sc, unsigned i) set_inspection_policy(pm->get_inspection_policy(i)); } -void set_ips_policy(IpsPolicy* p) -{ s_detection_policy = p; } - void set_ips_policy(SnortConfig* sc, unsigned i) { PolicyMap* pm = sc->policy_map; diff --git a/src/main/policy.h b/src/main/policy.h index ab6897e07..c9be172e5 100644 --- a/src/main/policy.h +++ b/src/main/policy.h @@ -191,9 +191,15 @@ public: void set_user_ips(IpsPolicy* p) { user_ips[p->user_policy_id] = p; } + void set_user_network(NetworkPolicy* p) + { user_network[p->user_policy_id] = p; } + IpsPolicy* get_user_ips(unsigned user_id) { return user_ips[user_id]; } + NetworkPolicy* get_user_network(unsigned user_id) + { return user_network[user_id]; } + InspectionPolicy* get_inspection_policy(unsigned i = 0) { return i < inspection_policy.size() ? inspection_policy[i] : nullptr; } @@ -223,6 +229,7 @@ private: std::unordered_map> shell_map; std::unordered_map user_inspection; std::unordered_map user_ips; + std::unordered_map user_network; bool cloned = false; @@ -243,16 +250,17 @@ SO_PUBLIC NetworkPolicy* get_network_policy(); SO_PUBLIC InspectionPolicy* get_inspection_policy(); SO_PUBLIC IpsPolicy* get_ips_policy(); SO_PUBLIC InspectionPolicy* get_default_inspection_policy(snort::SnortConfig*); -SO_PUBLIC void set_user_ips_policy(unsigned policy_id); +SO_PUBLIC void set_ips_policy(IpsPolicy* p); +SO_PUBLIC void set_network_policy(NetworkPolicy* p); +SO_PUBLIC IpsPolicy* get_user_ips_policy(snort::SnortConfig* sc, unsigned policy_id); +SO_PUBLIC NetworkPolicy* get_user_network_policy(snort::SnortConfig* sc, unsigned policy_id); } -void set_network_policy(NetworkPolicy*); void set_network_policy(snort::SnortConfig*, unsigned = 0); void set_inspection_policy(InspectionPolicy*); void set_inspection_policy(snort::SnortConfig*, unsigned = 0); -void set_ips_policy(IpsPolicy*); void set_ips_policy(snort::SnortConfig*, unsigned = 0); void set_policies(snort::SnortConfig*, Shell*); diff --git a/src/main/snort.cc b/src/main/snort.cc index 568ebd9c6..8eefc9ee0 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -957,9 +957,10 @@ DAQ_Verdict Snort::packet_callback( if (PacketTracer::is_active()) { - PacketTracer::log("NAP id %u, IPS id %u, Verdict %s\n", - get_network_policy()->policy_id, get_ips_policy()->policy_id, - SFDAQ::verdict_to_string(verdict)); + PacketTracer::log("Policies: Network %u, Inspection %u, Detection %u\n", + get_network_policy()->user_policy_id, get_inspection_policy()->user_policy_id, + get_ips_policy()->user_policy_id); + PacketTracer::log("Verdict: %s\n", SFDAQ::verdict_to_string(verdict)); PacketTracer::dump(pkthdr); } diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index 5c0e8773e..c6b9e6600 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -557,7 +557,7 @@ bool InspectorManager::delete_inspector(SnortConfig* sc, const char* iname) std::vector::iterator bind_it; if ( get_instance(fp, "binder", false, bind_it) ) { - (*bind_it)->handler->update(sc, iname); + (*bind_it)->handler->remove_inspector_binding(sc, iname); } } diff --git a/src/network_inspectors/appid/tp_lib_handler.cc b/src/network_inspectors/appid/tp_lib_handler.cc index b463c660d..3b4a2c6fb 100644 --- a/src/network_inspectors/appid/tp_lib_handler.cc +++ b/src/network_inspectors/appid/tp_lib_handler.cc @@ -96,7 +96,7 @@ int TPLibHandler::LoadCallback(const char* const path, int /* indent */) if ( (tp_module->api_version() != THIRD_PARTY_APP_ID_API_VERSION) || (tp_module->module_name().empty()) ) { - ErrorMessage("Ignoring incomplete 3rd party AppID module (%s, %d, %s)!\n", + ErrorMessage("Ignoring incomplete 3rd party AppID module (%s, %u, %s)!\n", path, tp_module->api_version(), tp_module->module_name().empty() ? "empty" : tp_module->module_name().c_str()); diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc index e118a39f6..8a679300d 100644 --- a/src/network_inspectors/binder/binder.cc +++ b/src/network_inspectors/binder/binder.cc @@ -573,7 +573,7 @@ public: void show(SnortConfig*) override { LogMessage("Binder\n"); } - void update(SnortConfig*, const char*) override; + void remove_inspector_binding(SnortConfig*, const char*) override; bool configure(SnortConfig*) override; @@ -649,7 +649,7 @@ bool Binder::configure(SnortConfig* sc) return true; } -void Binder::update(SnortConfig*, const char* name) +void Binder::remove_inspector_binding(SnortConfig*, const char* name) { vector::iterator it; for ( it = bindings.begin(); it != bindings.end(); ++it ) @@ -781,56 +781,73 @@ void Binder::set_binding(SnortConfig*, Binding* pb) // down. performance should be the focus of the next iteration. void Binder::get_bindings(Flow* flow, Stuff& stuff, Packet* p) { - Binding* pb; - unsigned i, sz = bindings.size(); + unsigned sz = bindings.size(); - for ( i = 0; i < sz; i++ ) + // Evaluate policy ID bindings first + // FIXIT-P The way these are being used, the policy bindings should be a separate list if not a + // separate table entirely + // FIXIT-L This will select the first policy ID of each type that it finds and ignore the rest. + // It gets potentially hairy if people start specifying overlapping policy types in + // overlapping rules. + bool inspection_set = false, ips_set = false, network_set = false; + for ( unsigned i = 0; i < sz; i++ ) { - pb = bindings[i]; + Binding* pb = bindings[i]; - if ( !pb->check_all(flow, p) ) + // Skip any rules that don't contain an ID for a policy type we haven't set yet. + if ( (!pb->use.inspection_index or inspection_set) and + (!pb->use.ips_index or ips_set) and + (!pb->use.network_index or network_set) ) continue; - if ( !pb->use.ips_index and !pb->use.inspection_index and !pb->use.network_index ) - { - if ( stuff.update(pb) ) - return; - else - continue; - } + if ( !pb->check_all(flow, p) ) + continue; - if ( pb->use.inspection_index ) + if ( pb->use.inspection_index and !inspection_set ) { set_inspection_policy(SnortConfig::get_conf(), pb->use.inspection_index - 1); flow->inspection_policy_id = pb->use.inspection_index - 1; + inspection_set = true; } - if ( pb->use.ips_index ) + if ( pb->use.ips_index and !ips_set ) { set_ips_policy(SnortConfig::get_conf(), pb->use.ips_index - 1); flow->ips_policy_id = pb->use.ips_index - 1; + ips_set = true; } - if ( pb->use.network_index ) + if ( pb->use.network_index and !network_set ) { set_network_policy(SnortConfig::get_conf(), pb->use.network_index - 1); flow->network_policy_id = pb->use.network_index - 1; + network_set = true; } + } + + Binder* sub = (Binder*)InspectorManager::get_binder(); - Binder* sub = (Binder*)InspectorManager::get_binder(); + // If policy selection produced a new binder to use, use that instead. + if ( sub && sub != this ) + { + sub->get_bindings(flow, stuff, p); + return; + } + + // If we got here, that means that a sub-policy with a binder was not invoked. + // Continue using this binder for the rest of processing. + for ( unsigned i = 0; i < sz; i++ ) + { + Binding* pb = bindings[i]; - // If selected sub-policy is IPS, inspection policy wont - // change and get_binder() will return this binder. Keep - // checking rules in case a new inspection policy is specified - // after. - if ( sub == this ) + if ( pb->use.ips_index or pb->use.inspection_index or pb->use.network_index ) continue; - if ( sub ) - { - sub->get_bindings(flow, stuff, p); + if ( !pb->check_all(flow, p) ) + continue; + + if ( stuff.update(pb) ) return; - } } } diff --git a/src/profiler/profiler_printer.h b/src/profiler/profiler_printer.h index c5ce3cfb4..e8bbf3bef 100644 --- a/src/profiler/profiler_printer.h +++ b/src/profiler/profiler_printer.h @@ -110,14 +110,17 @@ public: int max_depth) { auto& entries = cur.children; + unsigned num_entries; if ( !count || count > entries.size() ) - count = entries.size(); + num_entries = entries.size(); + else + num_entries = count; if ( sort ) - std::partial_sort(entries.begin(), entries.begin() + count, entries.end(), sort); + std::partial_sort(entries.begin(), entries.begin() + num_entries, entries.end(), sort); - for ( unsigned i = 0; i < count; ++i ) + for ( unsigned i = 0; i < num_entries; ++i ) { auto& entry = entries[i];