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

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 0e67c224b4e9afd8b33ccc8fdd56e3e44f8fa5cd..2bd7ecdb48ff8af520e610bb19847220e18bd132 100644 (file)
--- 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 8ca021d9015384464b4f02b1669e470aae6c0792..73dd1bbff239dfdb08df10bc31e357d3df1fc65e 100644 (file)
--- 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());
 }
-