bpf: refactor the BPF code and postpone querying of the engine mode

BPF codebase queried engine mode earlier than it was determined from
the configuration file/command line. As a result it used the default (IDS)
mode where it could've been configured later on to the IPS mode.
This could lead into an undefined behavior as some Suricata modules behave
according to the engine mode.

PF-Ring, Netmap and AF-Packet all shared almost identical code for
determining the engine mode. It was put into one common function.
Omitted the usage of SCStrdup function in PF-Ring module as it is
uppercased during thread initialization phase.

Ticket: #5958

diff --git a/src/runmode-af-packet.c b/src/runmode-af-packet.c
index ddff56ab6e0cee11aab5c8610a6c644308f16eab..e15203b3c0660c6d8eb684759471e814adc57635 100644 (file)
--- a/src/runmode-af-packet.c
+++ b/src/runmode-af-packet.c
@@ -54,6 +54,7 @@
 #include "util-ioctl.h"
 #include "util-ebpf.h"
 #include "util-byte.h"
+#include "util-bpf.h"
 
 #include "source-af-packet.h"
 
@@ -118,7 +119,6 @@ static void *ParseAFPConfig(const char *iface)
     const char *copymodestr;
     intmax_t value;
     int boolval;
-    const char *bpf_filter = NULL;
     const char *out_iface = NULL;
     int cluster_type = PACKET_FANOUT_HASH;
     const char *ebpf_file = NULL;
@@ -157,14 +157,6 @@ static void *ParseAFPConfig(const char *iface)
     aconf->ebpf_t_config.cpus_count = UtilCpuGetNumProcessorsConfigured();
 #endif
 
-    if (ConfGet("bpf-filter", &bpf_filter) == 1) {
-        if (strlen(bpf_filter) > 0) {
-            aconf->bpf_filter = bpf_filter;
-            SCLogConfig("Going to use command-line provided bpf filter '%s'",
-                       aconf->bpf_filter);
-        }
-    }
-
     /* Find initial node */
     af_packet_node = ConfGetNode("af-packet");
     if (af_packet_node == NULL) {
@@ -370,16 +362,7 @@ static void *ParseAFPConfig(const char *iface)
                                        "tracking issues, use it at your own risk.");
     }
 
-    /*load af_packet bpf filter*/
-    /* command line value has precedence */
-    if (ConfGet("bpf-filter", &bpf_filter) != 1) {
-        if (ConfGetChildValueWithDefault(if_root, if_default, "bpf-filter", &bpf_filter) == 1) {
-            if (strlen(bpf_filter) > 0) {
-                aconf->bpf_filter = bpf_filter;
-                SCLogConfig("Going to use bpf filter %s", aconf->bpf_filter);
-            }
-        }
-    }
+    ConfSetBPFFilter(if_root, if_default, iface, &aconf->bpf_filter);
 
     if (ConfGetChildValueWithDefault(if_root, if_default, "ebpf-lb-file", &ebpf_file) != 1) {
         aconf->ebpf_lb_file = NULL;

diff --git a/src/runmode-netmap.c b/src/runmode-netmap.c
index a83ad8cb6498fb5bef87ad7fd1d78f343ff6a6b2..794a623d66cb2777439942001b88b399653b9610 100644 (file)
--- a/src/runmode-netmap.c
+++ b/src/runmode-netmap.c
@@ -52,6 +52,7 @@
 #include "util-runmodes.h"
 #include "util-ioctl.h"
 #include "util-byte.h"
+#include "util-bpf.h"
 
 #if HAVE_NETMAP
 #define NETMAP_WITH_LIBS
@@ -142,15 +143,6 @@ static int ParseNetmapSettings(NetmapIfaceSettings *ns, const char *iface,
         ns->real = true;
     }
 
-    const char *bpf_filter = NULL;
-    if (ConfGet("bpf-filter", &bpf_filter) == 1) {
-        if (strlen(bpf_filter) > 0) {
-            ns->bpf_filter = bpf_filter;
-            SCLogInfo("Going to use command-line provided bpf filter '%s'",
-                    ns->bpf_filter);
-        }
-    }
-
     if (if_root == NULL && if_default == NULL) {
         SCLogInfo("Unable to find netmap config for "
                 "interface \"%s\" or \"default\", using default values",
@@ -181,16 +173,7 @@ static int ParseNetmapSettings(NetmapIfaceSettings *ns, const char *iface,
         }
     }
 
-    /* load netmap bpf filter */
-    /* command line value has precedence */
-    if (ns->bpf_filter == NULL) {
-        if (ConfGetChildValueWithDefault(if_root, if_default, "bpf-filter", &bpf_filter) == 1) {
-            if (strlen(bpf_filter) > 0) {
-                ns->bpf_filter = bpf_filter;
-                SCLogInfo("Going to use bpf filter %s", ns->bpf_filter);
-            }
-        }
-    }
+    ConfSetBPFFilter(if_root, if_default, iface, &ns->bpf_filter);
 
     int boolval = 0;
     (void)ConfGetChildValueBoolWithDefault(if_root, if_default, "disable-promisc", (int *)&boolval);

diff --git a/src/runmode-pfring.c b/src/runmode-pfring.c
index 528d850760f0d988a5f662189299bc65ce7ec37d..f3cef34c9cffe2cfa7f0edf6bf816c07774acf9b 100644 (file)
--- a/src/runmode-pfring.c
+++ b/src/runmode-pfring.c
@@ -23,6 +23,7 @@
 #include "source-pfring.h"
 #include "output.h"
 
+#include "util-bpf.h"
 #include "util-debug.h"
 #include "util-time.h"
 #include "util-cpu.h"
@@ -72,9 +73,6 @@ static void PfringDerefConfig(void *conf)
 {
     PfringIfaceConfig *pfp = (PfringIfaceConfig *)conf;
     if (SC_ATOMIC_SUB(pfp->ref, 1) == 1) {
-        if (pfp->bpf_filter) {
-            SCFree(pfp->bpf_filter);
-        }
         SCFree(pfp);
     }
 }
@@ -204,7 +202,6 @@ static void *ParsePfringConfig(const char *iface)
     const char *tmpctype = NULL;
     cluster_type default_ctype = CLUSTER_ROUND_ROBIN;
     int getctype = 0;
-    const char *bpf_filter = NULL;
     int bool_val;
 
     if (unlikely(pfconf == NULL)) {
@@ -313,32 +310,11 @@ static void *ParsePfringConfig(const char *iface)
         }
     }
 
-    /*load pfring bpf filter*/
-    /* command line value has precedence */
-    if (ConfGet("bpf-filter", &bpf_filter) == 1) {
-        if (strlen(bpf_filter) > 0) {
-            pfconf->bpf_filter = SCStrdup(bpf_filter);
-            if (unlikely(pfconf->bpf_filter == NULL)) {
-                SCLogError(SC_ERR_MEM_ALLOC,
-                           "Can't allocate BPF filter string");
-            } else {
-                SCLogDebug("Going to use command-line provided bpf filter %s",
-                           pfconf->bpf_filter);
-            }
-        }
-    } else {
-        if (ConfGetChildValueWithDefault(if_root, if_default, "bpf-filter", &bpf_filter) == 1) {
-            if (strlen(bpf_filter) > 0) {
-                pfconf->bpf_filter = SCStrdup(bpf_filter);
-                if (unlikely(pfconf->bpf_filter == NULL)) {
-                    SCLogError(SC_ERR_MEM_ALLOC,
-                               "Can't allocate BPF filter string");
-                } else {
-                    SCLogDebug("Going to use bpf filter %s",
-                               pfconf->bpf_filter);
-                }
-            }
-        }
+    ConfSetBPFFilter(if_root, if_default, pfconf->iface, &pfconf->bpf_filter);
+
+    if (pfconf->bpf_filter != NULL && EngineModeIsIPS()) {
+        FatalError(SC_ERR_NOT_SUPPORTED,
+                "BPF filter not available in IPS mode. Use firewall filtering if possible.");
     }
 
     if (ConfGet("pfring.cluster-type", &tmpctype) == 1) {

diff --git a/src/source-pfring.h b/src/source-pfring.h
index 31fb745c90315636dcd509b596b41a2800bcfb24..3433fb82e2711713cda6fc37c44b0cf2d773d2b5 100644 (file)
--- a/src/source-pfring.h
+++ b/src/source-pfring.h
@@ -44,7 +44,7 @@ typedef struct PfringIfaceConfig_
     /* number of threads */
     int threads;
 
-    char *bpf_filter;
+    const char *bpf_filter;
 
     ChecksumValidationMode checksum_mode;
     SC_ATOMIC_DECLARE(unsigned int, ref);

diff --git a/src/suricata.c b/src/suricata.c
index 0f7bb5ccb2fc6b14c179fc2ef23d349b128f53d1..e7f6f72d9e43b465238405cb1e00e73cd8ac6f36 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -471,16 +471,9 @@ static int SetBpfString(int argc, char *argv[])
     if (bpf_len == 0)
         return TM_ECODE_OK;
 
-    if (EngineModeIsIPS()) {
-        SCLogError(SC_ERR_NOT_SUPPORTED,
-                   "BPF filter not available in IPS mode."
-                   " Use firewall filtering if possible.");
-        return TM_ECODE_FAILED;
-    }
-
     bpf_filter = SCMalloc(bpf_len);
     if (unlikely(bpf_filter == NULL))
-        return TM_ECODE_OK;
+        return TM_ECODE_FAILED;
     memset(bpf_filter, 0x00, bpf_len);
 
     tmpindex = optind;

diff --git a/src/util-bpf.c b/src/util-bpf.c
index 934aca9ad2d6e1eabc9eb260186140b758d54a7a..dd43201ad64ed04e2ac4f03c4a751e7efc8e29b0 100644 (file)
--- a/src/util-bpf.c
+++ b/src/util-bpf.c
@@ -25,6 +25,31 @@
 #include "suricata-common.h"
 #include "suricata.h"
 #include "util-bpf.h"
+#include "conf.h"
+#include "util-debug.h"
+
+void ConfSetBPFFilter(
+        ConfNode *if_root, ConfNode *if_default, const char *iface, const char **bpf_filter)
+{
+    if (*bpf_filter != NULL) {
+        SCLogInfo("BPF filter already configured");
+        return;
+    }
+
+    /* command line value has precedence */
+    if (ConfGet("bpf-filter", bpf_filter) == 1) {
+        if (strlen(*bpf_filter) > 0) {
+            SCLogConfig("%s: using command-line provided bpf filter '%s'", iface, *bpf_filter);
+        }
+    } else if (ConfGetChildValueWithDefault(if_root, if_default, "bpf-filter", bpf_filter) ==
+               1) { // reading from a file
+        if (strlen(*bpf_filter) > 0) {
+            SCLogConfig("%s: using file provided bpf filter %s", iface, *bpf_filter);
+        }
+    } else {
+        SCLogDebug("No BPF filter found, skipping");
+    }
+}
 
 #if !defined __OpenBSD__
 

diff --git a/src/util-bpf.h b/src/util-bpf.h
index f38d31d5badbc5a5e93c0bdd1be407649a97b899..341c15845331e733ce9e9d24361ecafc1941b058 100644 (file)
--- a/src/util-bpf.h
+++ b/src/util-bpf.h
@@ -24,6 +24,11 @@
 #ifndef __UTIL_BPF_H__
 #define __UTIL_BPF_H__
 
+#include "conf.h"
+
+void ConfSetBPFFilter(
+        ConfNode *if_root, ConfNode *if_default, const char *iface, const char **bpf_filter);
+
 #if !defined __OpenBSD__
 
 int SCBPFCompile(int snaplen_arg, int linktype_arg, struct bpf_program *program,