Merge pull request #2739 in SNORT/snort3 from ~MIALTIZE/snort3:binder_stuff2 to master

Squashed commit of the following:

commit b38c4c0fbf677313717ccc289a77cbacb4f047ab
Author: Michael Altizer <mialtize@cisco.com>
Date:   Tue Feb 9 12:13:25 2021 -0500

    ftp_telnet: Respect telnet_cmds config for raising 125:1

commit 9ab2924a28b50726a8d185eaae10990d7b224cb6
Author: Michael Altizer <mialtize@cisco.com>
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 <mialtize@cisco.com>
Date:   Fri Dec 4 15:26:00 2020 -0500

    binder: Use the first match for non-terminal binding usage

commit 76d7cea0d784afcab575a173c57c8a65ac0a6153
Author: Michael Altizer <mialtize@cisco.com>
Date:   Fri Dec 4 15:13:31 2020 -0500

    binder: Clean up std namespace usage

commit 464fd2c44019b3a48c8c44c6b9c7bed82b3dc0b2
Author: Michael Altizer <mialtize@cisco.com>
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 <mialtize@cisco.com>
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 <mialtize@cisco.com>
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 c85d87cb9c3a8dbae5caf519ada1707d2797f656..1d7401a83c0686c9c02097ea7fb4faae773466eb 100644 (file)
--- a/src/managers/action_manager.cc
+++ b/src/managers/action_manager.cc
@@ -54,7 +54,6 @@ struct ActionInst
 struct IpsActionsConfig
 {
     vector<ActionInst> clist;
-    IpsAction* reject = nullptr;
 };
 
 using ACList = vector<ActionClass>;
@@ -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 84f0168e677cb2dc4973265aa672b1656c9a43f4..4235848e8b98266580bf56dad291675dc9060dd0 100644 (file)
--- 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 1a73c35cd37dd6a1902e62f220eaa9ca4a630b33..76a41ed16c31695070426447e501c8e0f0b8f91f 100644 (file)
--- 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 a714cc4668df61d313d5667b25366f9d3c35a750..fd5a67f908de482762a73dad6de2a0e5d81160d4 100644 (file)
--- 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<Binding>&, vector<Binding>&);
+    Binder(std::vector<Binding>&, std::vector<Binding>&);
     ~Binder() override;
 
     void remove_inspector_binding(SnortConfig*, const char*) override;
@@ -482,8 +472,8 @@ private:
     Inspector* find_gadget(Flow&);
 
 private:
-    vector<Binding> bindings;
-    vector<Binding> policy_bindings;
+    std::vector<Binding> bindings;
+    std::vector<Binding> policy_bindings;
     Inspector* default_ssn_inspectors[to_utype(PktType::MAX)]{};
 };
 
@@ -542,7 +532,7 @@ public:
     }
 };
 
-Binder::Binder(vector<Binding>& bv, vector<Binding>& pbv)
+Binder::Binder(std::vector<Binding>& bv, std::vector<Binding>& 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<Binding>& bv = mod->get_bindings();
-    vector<Binding>& pbv = mod->get_policy_bindings();
+    std::vector<Binding>& bv = mod->get_bindings();
+    std::vector<Binding>& 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 60172562ee38c219c27283d0a693198c1674d9c9..6d5ff5846797f90e13df31c2431271a0ba9cf926 100644 (file)
--- 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 a7a5abfd8b651f711dff53bab8eeed5f46f86e2b..0754c30a1e76d77bef0cf128b0d63872304513ad 100644 (file)
--- 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 40989e021e718225f3cebbddef53087c7aac88d6..79b81ed201cc34af72f70ef024a74b66f942ae30 100644 (file)
--- 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 */