snort: validate sw_if_index in attach/detach api handlers

- fixes crash in vpp-debug-verify-master-ubuntu2204-x86_64 CI job
  in test_snort_06_detach_if testcase
- fix similar logic in attach handler
- verify snort direction in attach api message
- add tests verifying attribute validation in attach testcase

Type: fix
Fixes: 102575492c9199259aa5e468f21b46936d7a1ac4

Change-Id: I96fbeb0a7b84f2f238df15b20476ed4086251471
Signed-off-by: Dave Wallace <[email protected]>

diff --git a/src/plugins/snort/snort_api.c b/src/plugins/snort/snort_api.c
index 4016dfa..00b3c3a 100644 (file)
--- a/src/plugins/snort/snort_api.c
+++ b/src/plugins/snort/snort_api.c
@@ -1,3 +1,6 @@
+/* SPDX-License-Identifier: Apache-2.0
+ * Copyright(c) 2025 Cisco Systems, Inc.
+ */
 #include <vlib/vlib.h>
 #include <vnet/plugin/plugin.h>
 #include <snort/snort.h>
@@ -80,17 +83,25 @@ vl_api_snort_interface_attach_t_handler (vl_api_snort_interface_attach_t *mp)
   u8 snort_dir = mp->snort_dir;
   int rv = VNET_API_ERROR_NO_SUCH_ENTRY;
 
-  if (sw_if_index == INDEX_INVALID)
-    rv = VNET_API_ERROR_NO_MATCHING_INTERFACE;
-  else
+  VALIDATE_SW_IF_INDEX (mp);
+  switch (snort_dir)
     {
-      instance = snort_get_instance_by_index (instance_index);
-      if (instance)
-       rv = snort_interface_enable_disable (vm, (char *) instance->name,
-                                            sw_if_index, 1 /* is_enable */,
-                                            snort_dir);
+    case SNORT_INPUT:
+    case SNORT_OUTPUT:
+    case SNORT_INOUT:
+      break;
+    default:
+      rv = VNET_API_ERROR_INVALID_ARGUMENT;
+      goto bad_sw_if_index;
     }
-
+  instance = snort_get_instance_by_index (instance_index);
+  if (instance)
+    {
+      rv = snort_interface_enable_disable (vm, (char *) instance->name,
+                                          sw_if_index, 1 /* is_enable */,
+                                          snort_dir);
+    }
+  BAD_SW_IF_INDEX_LABEL;
   REPLY_MACRO (VL_API_SNORT_INTERFACE_ATTACH_REPLY);
 }
 
@@ -375,8 +386,10 @@ vl_api_snort_interface_detach_t_handler (vl_api_snort_interface_detach_t *mp)
   u32 sw_if_index = clib_net_to_host_u32 (mp->sw_if_index);
   int rv;
 
+  VALIDATE_SW_IF_INDEX (mp);
   rv = snort_interface_disable_all (vm, sw_if_index);
 
+  BAD_SW_IF_INDEX_LABEL;
   REPLY_MACRO (VL_API_SNORT_INTERFACE_DETACH_REPLY);
 }
 

diff --git a/test/test_snort.py b/test/test_snort.py
index c25c0e6..5335091 100644 (file)
--- a/test/test_snort.py
+++ b/test/test_snort.py
@@ -29,7 +29,6 @@ class TestSnort(VppTestCase):
     def test_snort_cli(self):
         # TODO: add a test with packets
         # { cli command : part of the expected reply }
-        print("TEST SNORT CLI")
         commands_replies = {
             "snort create-instance name snortTest queue-size 16 on-disconnect drop": "",
             "snort create-instance name snortTest2 queue-size 16 on-disconnect pass": "",
@@ -114,6 +113,18 @@ class TestSnortVapi(VppTestCase):
         reply = self.vapi.snort_interface_attach(
             instance_index=0, sw_if_index=2, snort_dir=2
         )
+        # verify attaching with an invalid direction is rejected
+        try:
+            reply = self.vapi.snort_interface_attach(
+                instance_index=1, sw_if_index=2, snort_dir=4
+            )
+        except:
+            pass
+        else:
+            self.assertNotEqual(reply.retval, 0)
+        reply = self.vapi.cli("show snort interfaces")
+        self.assertNotIn("snortTest1", reply)
+
         reply = self.vapi.snort_interface_attach(
             instance_index=1, sw_if_index=2, snort_dir=3
         )
@@ -123,6 +134,8 @@ class TestSnortVapi(VppTestCase):
         self.assertIn("input", reply)
         self.assertIn("inout", reply)
         self.assertIn("output", reply)
+
+        # verify attaching a previously attached interface is rejected
         try:
             reply = self.vapi.snort_interface_attach(
                 instance_index=1, sw_if_index=2, snort_dir=2
@@ -131,6 +144,16 @@ class TestSnortVapi(VppTestCase):
             pass
         else:
             self.assertNotEqual(reply.retval, 0)
+
+        # verify attaching an invalid sw_if_index is rejected
+        try:
+            reply = self.vapi.snort_interface_attach(
+                instance_index=1, sw_if_index=3, snort_dir=2
+            )
+        except:
+            pass
+        else:
+            self.assertNotEqual(reply.retval, 0)
         reply = self.vapi.cli("show snort interfaces")
         self.assertIn("snortTest1", reply)
 
@@ -145,6 +168,7 @@ class TestSnortVapi(VppTestCase):
 
     def test_snort_06_detach_if(self):
         """Interfaces can be detached"""
+        # verify detaching an invalid sw_if_index is rejected
         try:
             reply = self.vapi.snort_interface_detach(sw_if_index=3)
         except: