From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Thu, 10 Mar 2022 17:24:19 +0000 (+0000)
Subject: Pull request #3295: control: make sure reload commands with empty argument is handled... 
X-Git-Tag: 3.1.26.0~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f24dfd91c3777b4761806c5495d3ba8890e54284;p=thirdparty%2Fsnort3.git

Pull request #3295: control: make sure reload commands with empty argument is handled correctly

Merge in SNORT/snort3 from ~SBAIGAL/snort3:lua_bug to master

Squashed commit of the following:

commit 593cce30daa0338ee81bde5837c92c9ac1341d4b
Author: Steven Baigal (sbaigal) <sbaigal@cisco.com>
Date:   Thu Mar 3 15:22:11 2022 -0500

    control: make sure reload commands with empty argument is handled correctly
---

diff --git a/src/main.cc b/src/main.cc
index 0e67c224b..2bd7ecdb4 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -359,18 +359,15 @@ int main_rotate_stats(lua_State* L)
 int main_reload_config(lua_State* L)
 {
     ControlConn* ctrlcon = ControlConn::query_from_lua(L);
-    if ( !ReloadTracker::start(ctrlcon) )
-    {
-        send_response(ctrlcon, "== reload pending; retry\n");
-        return 0;
-    }
     const char* fname =  nullptr;
     const char* plugin_path =  nullptr;
 
     if ( L )
     {
         Lua::ManageStack(L, 1);
-        fname = luaL_checkstring(L, 1);
+        if (lua_gettop(L) >= 1)
+            fname = luaL_checkstring(L, 1);
+
         if (lua_gettop(L) > 1)
         {
             plugin_path = luaL_checkstring(L, 2);
@@ -380,6 +377,11 @@ int main_reload_config(lua_State* L)
         }
     }
 
+    if ( !ReloadTracker::start(ctrlcon) )
+    {
+        send_response(ctrlcon, "== reload pending; retry\n");
+        return 0;
+    }
     send_response(ctrlcon, ".. reloading configuration\n");
     ReloadTracker::update(ctrlcon,"start loading ...");
     const SnortConfig* old = SnortConfig::get_conf();
@@ -434,29 +436,30 @@ int main_reload_config(lua_State* L)
 
 int main_reload_policy(lua_State* L)
 {
-    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
-    if ( !ReloadTracker::start(ctrlcon) )
-    {
-        send_response(ctrlcon, "== reload pending; retry\n");
-        return 0;
-    }
     const char* fname =  nullptr;
 
     if ( L )
     {
         Lua::ManageStack(L, 1);
-        fname = luaL_checkstring(L, 1);
+        if (lua_gettop(L) >= 1)
+            fname = luaL_checkstring(L, 1);
     }
 
-    if ( fname and *fname )
-        send_response(ctrlcon, ".. reloading policy\n");
-    else
+    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
+    if ( !fname or *fname == '\0' )
     {
-        ReloadTracker::failed(ctrlcon, "filename required");
         send_response(ctrlcon, "== filename required\n");
         return 0;
     }
 
+    if ( !ReloadTracker::start(ctrlcon) )
+    {
+        send_response(ctrlcon, "== reload pending; retry\n");
+        return 0;
+    }
+
+    send_response(ctrlcon, ".. reloading policy\n");
+
     SnortConfig* old = SnortConfig::get_main_conf();
     SnortConfig* sc = Snort::get_updated_policy(old, fname, nullptr);
 
@@ -479,29 +482,30 @@ int main_reload_policy(lua_State* L)
 
 int main_reload_module(lua_State* L)
 {
-    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
-    if ( !ReloadTracker::start(ctrlcon) )
-    {
-        send_response(ctrlcon, "== reload pending; retry\n");
-        return 0;
-    }
     const char* fname =  nullptr;
 
     if ( L )
     {
         Lua::ManageStack(L, 1);
-        fname = luaL_checkstring(L, 1);
+        if (lua_gettop(L) >= 1)
+            fname = luaL_checkstring(L, 1);
     }
 
-    if ( fname and *fname )
-        send_response(ctrlcon, ".. reloading module\n");
-    else
+    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
+    if ( !fname or *fname == '\0' )
     {
-        ReloadTracker::failed(ctrlcon, "module name required");
         send_response(ctrlcon, "== module name required\n");
         return 0;
     }
 
+    if ( !ReloadTracker::start(ctrlcon) )
+    {
+        send_response(ctrlcon, "== reload pending; retry\n");
+        return 0;
+    }
+
+    send_response(ctrlcon, ".. reloading module\n");
+
     SnortConfig* old = SnortConfig::get_main_conf();
     SnortConfig* sc = Snort::get_updated_module(old, fname);
 
@@ -534,13 +538,6 @@ int main_reload_daq(lua_State* L)
 
 int main_reload_hosts(lua_State* L)
 {
-    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
-    if ( !ReloadTracker::start(ctrlcon) )
-    {
-        send_response(ctrlcon, "== reload pending; retry\n");
-        return 0;
-    }
-
     SnortConfig* sc = SnortConfig::get_main_conf();
     const char* fname;
 
@@ -552,20 +549,24 @@ int main_reload_hosts(lua_State* L)
     else
         fname = sc->attribute_hosts_file.c_str();
 
-    if ( fname and *fname )
+    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
+    if ( !fname or *fname == '\0' )
     {
-        std::string msg = "Reloading Host attribute table from ";
-        msg += fname;
-        ReloadTracker::update(ctrlcon, msg.c_str());
-        send_response(ctrlcon, ".. reloading hosts table\n");
+        send_response(ctrlcon, "== filename required\n");
+        return 0;
     }
-    else
+
+    if ( !ReloadTracker::start(ctrlcon) )
     {
-        ReloadTracker::failed(ctrlcon, "host attribute table filename required.");
-        send_response(ctrlcon, "== filename required\n");
+        send_response(ctrlcon, "== reload pending; retry\n");
         return 0;
     }
 
+    std::string msg = "Reloading Host attribute table from ";
+    msg += fname;
+    ReloadTracker::update(ctrlcon, msg.c_str());
+    send_response(ctrlcon, ".. reloading hosts table\n");
+
     if ( !HostAttributesManager::load_hosts_file(sc, fname) )
     {
         ReloadTracker::failed(ctrlcon, "failed to load host table.");
@@ -587,29 +588,30 @@ int main_reload_hosts(lua_State* L)
 
 int main_delete_inspector(lua_State* L)
 {
-    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
-    if ( !ReloadTracker::start(ctrlcon) )
-    {
-        send_response(ctrlcon, "== delete pending; retry\n");
-        return 0;
-    }
     const char* iname =  nullptr;
 
     if ( L )
     {
         Lua::ManageStack(L, 1);
-        iname = luaL_checkstring(L, 1);
+        if (lua_gettop(L) >= 1)
+            iname = luaL_checkstring(L, 1);
     }
 
-    if ( iname and *iname )
-        send_response(ctrlcon, ".. deleting inspector\n");
-    else
+    ControlConn* ctrlcon = ControlConn::query_from_lua(L);
+    if ( !iname or *iname == '\0' )
     {
-        ReloadTracker::failed(ctrlcon, "inspector name required.");
         send_response(ctrlcon, "== inspector name required\n");
         return 0;
     }
 
+    if ( !ReloadTracker::start(ctrlcon) )
+    {
+        send_response(ctrlcon, "== delete pending; retry\n");
+        return 0;
+    }
+
+    send_response(ctrlcon, ".. deleting inspector\n");
+
     SnortConfig* old = SnortConfig::get_main_conf();
     SnortConfig* sc = Snort::get_updated_policy(old, nullptr, iname);
 
diff --git a/src/main/reload_tracker.cc b/src/main/reload_tracker.cc
index 8ca021d90..73dd1bbff 100644
--- a/src/main/reload_tracker.cc
+++ b/src/main/reload_tracker.cc
@@ -52,10 +52,12 @@ bool ReloadTracker::start(ControlConn* ctrlcon)
 
 void ReloadTracker::end(ControlConn* ctrlcon)
 {
-    if (ctrl == ctrlcon and reload_in_progress)
-        LogMessage("Reload ended. [%s]\n", current_command.c_str());
-    else
-        ErrorMessage("Reload session mismatched for ending tracker\n");
+#ifdef NDEBUG
+    UNUSED(ctrlcon);
+#else
+    assert(ctrl == ctrlcon and reload_in_progress);
+#endif
+    LogMessage("Reload ended. [%s]\n", current_command.c_str());
     current_command.clear();
     ctrl = nullptr;
     reload_in_progress = false;
@@ -63,10 +65,12 @@ void ReloadTracker::end(ControlConn* ctrlcon)
 
 void ReloadTracker::failed(ControlConn* ctrlcon, const char* reason)
 {
-    if (ctrl == ctrlcon and reload_in_progress)
-        LogMessage("Reload failed! %s [%s]\n", reason, current_command.c_str());
-    else
-        ErrorMessage("Reload session mismatched for failing tracker\n");
+#ifdef NDEBUG
+    UNUSED(ctrlcon);
+#else
+    assert(ctrl == ctrlcon and reload_in_progress);
+#endif
+    LogMessage("Reload failed! %s [%s]\n", reason, current_command.c_str());
     current_command.clear();
     ctrl = nullptr;
     reload_in_progress = false;
@@ -74,9 +78,10 @@ void ReloadTracker::failed(ControlConn* ctrlcon, const char* reason)
 
 void ReloadTracker::update(ControlConn* ctrlcon, const char* status)
 {
-    if (ctrl == ctrlcon and reload_in_progress)
-        LogMessage("Reload update: %s [%s]\n", status, current_command.c_str());
-    else
-        ErrorMessage("Reload session mismatched for updating tracker\n");
+#ifdef NDEBUG
+    UNUSED(ctrlcon);
+#else
+    assert(ctrl == ctrlcon and reload_in_progress);
+#endif
+    LogMessage("Reload update: %s [%s]\n", status, current_command.c_str());
 }
-