From: Michael Altizer (mialtize) Date: Thu, 11 Feb 2021 19:11:52 +0000 (+0000) Subject: Merge pull request #2739 in SNORT/snort3 from ~MIALTIZE/snort3:binder_stuff2 to master X-Git-Tag: 3.1.2.0~42 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d6b7a10413c2a279ec358b1ed813caa486ef4863;p=thirdparty%2Fsnort3.git Merge pull request #2739 in SNORT/snort3 from ~MIALTIZE/snort3:binder_stuff2 to master Squashed commit of the following: commit b38c4c0fbf677313717ccc289a77cbacb4f047ab Author: Michael Altizer Date: Tue Feb 9 12:13:25 2021 -0500 ftp_telnet: Respect telnet_cmds config for raising 125:1 commit 9ab2924a28b50726a8d185eaae10990d7b224cb6 Author: Michael Altizer Date: Thu Feb 4 12:35:21 2021 -0500 binder: Apply host attribute table information at the beginning of flow setup commit d794f0481b9e1d886fe65ae0cec87e6af33ecd76 Author: Michael Altizer Date: Fri Dec 4 15:26:00 2020 -0500 binder: Use the first match for non-terminal binding usage commit 76d7cea0d784afcab575a173c57c8a65ac0a6153 Author: Michael Altizer Date: Fri Dec 4 15:13:31 2020 -0500 binder: Clean up std namespace usage commit 464fd2c44019b3a48c8c44c6b9c7bed82b3dc0b2 Author: Michael Altizer Date: Fri Dec 4 14:51:22 2020 -0500 inspector_manager: Instantiate default binder as long as a wizard or stream are present commit 7f0be69877ff16e0fc74716c0c73e9850eca1a46 Author: Michael Altizer Date: Thu Dec 3 15:05:37 2020 -0500 module_manager: Enforce interest in global modules only in the default policy commit 0cacbbc73299aecd52ba1f08700fb996c089a8a0 Author: Michael Altizer Date: Wed Dec 16 13:49:06 2020 -0500 action_manager: Remove unused cached reject action --- diff --git a/src/managers/action_manager.cc b/src/managers/action_manager.cc index c85d87cb9..1d7401a83 100644 --- a/src/managers/action_manager.cc +++ b/src/managers/action_manager.cc @@ -54,7 +54,6 @@ struct ActionInst struct IpsActionsConfig { vector clist; - IpsAction* reject = nullptr; }; using ACList = vector; @@ -157,11 +156,6 @@ void ActionManager::instantiate(const ActionApi* api, Module* mod, SnortConfig* // Add this instance to the list of those created for this config sc->ips_actions_config->clist.emplace_back(*cls, act); - // FIXIT-M Either you are static or you're not, make a choice. - // Anyway, if we happen to instantiate an action called reject, cache that for use later. - if ( !sc->ips_actions_config->reject && !strcmp(act->get_name(), "reject") ) - sc->ips_actions_config->reject = act; - RuleListNode* rln = CreateRuleType(sc, api->base.name, api->type, true); // The plugin actions (e.g. reject, react, etc.) are per policy, per mode. @@ -170,7 +164,6 @@ void ActionManager::instantiate(const ActionApi* api, Module* mod, SnortConfig* Actions::Type idx = rln->mode; assert(ips->action[idx] == nullptr); ips->action[idx] = act; - } } diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index 84f0168e6..4235848e8 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -951,10 +951,7 @@ static bool configure(SnortConfig* sc, FrameworkPolicy* fp, bool cloned) sort(fp->ilist.begin(), fp->ilist.end(), PHInstance::comp); fp->vectorize(sc); - // FIXIT-M checking for wizard here would avoid fatals for - // can't bind wizard but this exposes other issues that must - // be fixed first. - if ( fp->session.num and !fp->binder /*and fp->wizard*/ ) + if ( !fp->binder and (fp->session.num or fp->wizard) ) instantiate_default_binder(sc, fp); return ok; diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc index 1a73c35cd..76a41ed16 100644 --- a/src/managers/module_manager.cc +++ b/src/managers/module_manager.cc @@ -610,8 +610,7 @@ static bool end(Module* m, const Parameter* p, const char* s, int idx) static bool interested(Module* m) { - if ( m->get_usage() == Module::GLOBAL && - only_inspection_policy() && !default_inspection_policy() ) + if ( m->get_usage() == Module::GLOBAL && !default_inspection_policy() ) return false; if ( m->get_usage() != Module::INSPECT && only_inspection_policy() ) diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc index a714cc466..fd5a67f90 100644 --- a/src/network_inspectors/binder/binder.cc +++ b/src/network_inspectors/binder/binder.cc @@ -35,7 +35,6 @@ #include "binding.h" using namespace snort; -using namespace std; THREAD_LOCAL ProfileStats bindPerfStats; @@ -330,8 +329,8 @@ struct Stuff bool update(const Binding&); void apply_action(Flow&); - void apply_session(Flow&, const HostAttributesEntry); - void apply_service(Flow&, const HostAttributesEntry); + void apply_session(Flow&); + void apply_service(Flow&); void apply_assistant(Flow&, const char*); }; @@ -348,16 +347,22 @@ bool Stuff::update(const Binding& pb) case BindUse::BW_NONE: break; case BindUse::BW_PASSIVE: - data = pb.use.inspector; + if (!data) + data = pb.use.inspector; break; case BindUse::BW_CLIENT: - client = pb.use.inspector; + if (!client) + client = pb.use.inspector; break; case BindUse::BW_SERVER: - server = pb.use.inspector; + if (!server) + server = pb.use.inspector; break; case BindUse::BW_STREAM: - client = server = pb.use.inspector; + if (!client) + client = pb.use.inspector; + if (!server) + server = pb.use.inspector; break; case BindUse::BW_WIZARD: wizard = pb.use.inspector; @@ -392,7 +397,7 @@ void Stuff::apply_action(Flow& flow) } } -void Stuff::apply_session(Flow& flow, const HostAttributesEntry host) +void Stuff::apply_session(Flow& flow) { if (client) flow.set_client(client); @@ -403,28 +408,13 @@ void Stuff::apply_session(Flow& flow, const HostAttributesEntry host) flow.set_server(server); else if (flow.ssn_server) flow.clear_server(); - - switch (flow.pkt_type) - { - case PktType::IP: - flow.ssn_policy = host ? host->get_frag_policy() : 0; - break; - case PktType::TCP: - flow.ssn_policy = host ? host->get_stream_policy() : 0; - break; - default: - break; - } } -void Stuff::apply_service(Flow& flow, const HostAttributesEntry host) +void Stuff::apply_service(Flow& flow) { if (data) flow.set_data(data); - if (host) - Stream::set_snort_protocol_id(&flow, host, FROM_SERVER); - if (!gadget) gadget = get_gadget(flow); @@ -460,7 +450,7 @@ void Stuff::apply_assistant(Flow& flow, const char* service) class Binder : public Inspector { public: - Binder(vector&, vector&); + Binder(std::vector&, std::vector&); ~Binder() override; void remove_inspector_binding(SnortConfig*, const char*) override; @@ -482,8 +472,8 @@ private: Inspector* find_gadget(Flow&); private: - vector bindings; - vector policy_bindings; + std::vector bindings; + std::vector policy_bindings; Inspector* default_ssn_inspectors[to_utype(PktType::MAX)]{}; }; @@ -542,7 +532,7 @@ public: } }; -Binder::Binder(vector& bv, vector& pbv) +Binder::Binder(std::vector& bv, std::vector& pbv) { bindings = std::move(bv); policy_bindings = std::move(pbv); @@ -640,6 +630,32 @@ void Binder::handle_flow_setup(Flow& flow, bool standby) { Profile profile(bindPerfStats); + // FIXIT-M logic for applying information from the host attribute table likely doesn't belong + // in binder, but it *does* need to occur before the binding lookup (for service information) + const HostAttributesEntry host = HostAttributesManager::find_host(flow.server_ip); + if (host) + { + // Set the fragmentation (IP) or stream (TCP) policy from the host entry + switch (flow.pkt_type) + { + case PktType::IP: + flow.ssn_policy = host->get_frag_policy(); + break; + case PktType::TCP: + flow.ssn_policy = host->get_stream_policy(); + break; + default: + break; + } + + Stream::set_snort_protocol_id(&flow, host, FROM_SERVER); + if (flow.ssn_state.snort_protocol_id != UNKNOWN_PROTOCOL_ID) + { + const SnortConfig* sc = SnortConfig::get_conf(); + flow.service = sc->proto_ref->get_name(flow.ssn_state.snort_protocol_id); + } + } + Stuff stuff; get_bindings(flow, stuff); apply(flow, stuff); @@ -813,11 +829,8 @@ void Binder::apply(Flow& flow, Stuff& stuff) if (flow.flow_state != Flow::FlowState::INSPECT) return; - const HostAttributesEntry host = HostAttributesManager::find_host(flow.server_ip); - - stuff.apply_session(flow, host); - - stuff.apply_service(flow, host); + stuff.apply_session(flow); + stuff.apply_service(flow); } void Binder::apply_assistant(Flow& flow, Stuff& stuff, const char* service) @@ -838,8 +851,8 @@ static void mod_dtor(Module* m) static Inspector* bind_ctor(Module* m) { BinderModule* mod = (BinderModule*)m; - vector& bv = mod->get_bindings(); - vector& pbv = mod->get_policy_bindings(); + std::vector& bv = mod->get_bindings(); + std::vector& pbv = mod->get_policy_bindings(); return new Binder(bv, pbv); } diff --git a/src/service_inspectors/ftp_telnet/ftpp_ui_config.cc b/src/service_inspectors/ftp_telnet/ftpp_ui_config.cc index 60172562e..6d5ff5846 100644 --- a/src/service_inspectors/ftp_telnet/ftpp_ui_config.cc +++ b/src/service_inspectors/ftp_telnet/ftpp_ui_config.cc @@ -57,12 +57,6 @@ FTP_CLIENT_PROTO_CONF::~FTP_CLIENT_PROTO_CONF() FTP_SERVER_PROTO_CONF::FTP_SERVER_PROTO_CONF() { ftp_cmd_lookup_init(&cmd_lookup); - - def_max_param_len = FTPP_UI_CONFIG_FTP_DEF_CMD_PARAM_MAX; - max_cmd_len = MAX_CMD; - - print_commands = data_chan = check_encrypted_data = false; - telnet_cmds = ignore_telnet_erase_cmds = detect_encrypted = false; } FTP_SERVER_PROTO_CONF::~FTP_SERVER_PROTO_CONF() diff --git a/src/service_inspectors/ftp_telnet/ftpp_ui_config.h b/src/service_inspectors/ftp_telnet/ftpp_ui_config.h index a7a5abfd8..0754c30a1 100644 --- a/src/service_inspectors/ftp_telnet/ftpp_ui_config.h +++ b/src/service_inspectors/ftp_telnet/ftpp_ui_config.h @@ -176,15 +176,15 @@ typedef struct s_FTP_CMD_CONF */ struct FTP_SERVER_PROTO_CONF { - unsigned int def_max_param_len; - unsigned int max_cmd_len; + unsigned int def_max_param_len = FTPP_UI_CONFIG_FTP_DEF_CMD_PARAM_MAX; + unsigned int max_cmd_len = MAX_CMD; - bool print_commands; - bool data_chan; - bool check_encrypted_data; - bool telnet_cmds; - bool ignore_telnet_erase_cmds; - bool detect_encrypted; + bool print_commands = false; + bool data_chan = false; + bool check_encrypted_data = false; + bool telnet_cmds = false; + bool ignore_telnet_erase_cmds = false; + bool detect_encrypted = false; CMD_LOOKUP* cmd_lookup; @@ -210,7 +210,6 @@ struct FTP_CLIENT_PROTO_CONF { unsigned int max_resp_len = FTPP_UI_CONFIG_FTP_DEF_RESP_MSG_MAX; - bool data_chan = false; bool bounce = false; bool telnet_cmds = false; bool ignore_telnet_erase_cmds = false; diff --git a/src/service_inspectors/ftp_telnet/pp_ftp.cc b/src/service_inspectors/ftp_telnet/pp_ftp.cc index 40989e021..79b81ed20 100644 --- a/src/service_inspectors/ftp_telnet/pp_ftp.cc +++ b/src/service_inspectors/ftp_telnet/pp_ftp.cc @@ -938,10 +938,8 @@ int initialize_ftp(FTP_SESSION* session, Packet* p, int iMode) int iRet; char ignoreTelnetErase = FTPP_APPLY_TNC_ERASE_CMDS; /* Normalize this packet ala telnet */ - if (((iMode == FTPP_SI_CLIENT_MODE) && - session->client_conf->ignore_telnet_erase_cmds) || - ((iMode == FTPP_SI_SERVER_MODE) && - session->server_conf->ignore_telnet_erase_cmds) ) + if ((iMode == FTPP_SI_CLIENT_MODE && session->client_conf->ignore_telnet_erase_cmds) || + (iMode == FTPP_SI_SERVER_MODE && session->server_conf->ignore_telnet_erase_cmds)) ignoreTelnetErase = FTPP_IGNORE_TNC_ERASE_CMDS; DataBuffer& buf = DetectionEngine::get_alt_buffer(p); @@ -960,8 +958,8 @@ int initialize_ftp(FTP_SESSION* session, Packet* p, int iMode) if ( buf.len ) { /* Normalized data will always be in decode buffer */ - if ( (iMode == FTPP_SI_CLIENT_MODE) || - (iMode == FTPP_SI_SERVER_MODE) ) + if ((iMode == FTPP_SI_CLIENT_MODE && session->client_conf->telnet_cmds) || + (iMode == FTPP_SI_SERVER_MODE && session->server_conf->telnet_cmds)) { DetectionEngine::queue_event(GID_FTP, FTP_TELNET_CMD); return FTPP_ALERT; /* Nothing else to do since we alerted */