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: #5957

diff --git a/src/runmode-af-packet.c b/src/runmode-af-packet.c
index 6b0e974bb1f80d4df09ec5c6accbb392407ebd86..6f3d77adff3044810068905163b038273c2e1682 100644 (file)
--- a/src/runmode-af-packet.c
+++ b/src/runmode-af-packet.c
@@ -57,6 +57,7 @@
 #include "util-byte.h"
 
 #include "source-af-packet.h"
+#include "util-bpf.h"
 
 extern intmax_t max_pending_packets;
 
@@ -205,7 +206,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;
@@ -244,14 +244,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(
-                    "%s: using command-line provided bpf filter '%s'", iface, aconf->bpf_filter);
-        }
-    }
-
     /* Find initial node */
     af_packet_node = ConfGetNode("af-packet");
     if (af_packet_node == NULL) {
@@ -435,16 +427,7 @@ static void *ParseAFPConfig(const char *iface)
                 iface);
     }
 
-    /*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("%s: bpf filter %s", iface, 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 7649e3fc0fce8ce8602d270467830fd00a70cf16..b961bb5713d5eb33636d4e1fb258287513d53c0b 100644 (file)
--- a/src/runmode-netmap.c
+++ b/src/runmode-netmap.c
@@ -48,6 +48,7 @@
 #include "source-netmap.h"
 #include "util-conf.h"
 #include "suricata.h"
+#include "util-bpf.h"
 
 extern intmax_t max_pending_packets;
 
@@ -205,14 +206,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("%s: using command-line provided bpf filter '%s'", iface, ns->bpf_filter);
-        }
-    }
-
     if (if_root == NULL && if_default == NULL) {
         SCLogInfo("%s: unable to find netmap config for interface \"%s\" or \"default\", using "
                   "default values",
@@ -242,16 +235,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("%s: using bpf filter %s", iface, 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 c7ab688958352e6e1927bf2b87dd10a007151618..2de535fc275b970fec3b596060d92e286bfc1592 100644 (file)
--- a/src/runmode-pfring.c
+++ b/src/runmode-pfring.c
@@ -21,7 +21,8 @@
 #include "conf.h"
 #include "runmodes.h"
 #include "source-pfring.h"
-
+#include "suricata.h"
+#include "util-bpf.h"
 #include "util-debug.h"
 #include "util-time.h"
 #include "util-cpu.h"
@@ -70,9 +71,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);
     }
 }
@@ -201,7 +199,6 @@ static void *ParsePfringConfig(const char *iface)
     const char *tmpctype = NULL;
     cluster_type default_ctype = CLUSTER_FLOW;
     int getctype = 0;
-    const char *bpf_filter = NULL;
     int bool_val;
 
     if (unlikely(pfconf == NULL)) {
@@ -312,30 +309,10 @@ 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("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("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 (EngineModeIsIPS()) {
+        FatalError("IPS mode not supported in PF_RING.");
     }
 
     if (ConfGet("pfring.cluster-type", &tmpctype) == 1) {

diff --git a/src/source-pfring.h b/src/source-pfring.h
index 163568a9e04803fe94ca8b84763aa80a23039e14..6b170ee78d525d7f295835d6e0267e614f39286c 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 a8f3f841094d040a7e1c63d5d4a8dd0bc4359bd4..29494f8ebfb231adc1f2d77b3a887efbcf4050e4 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -141,6 +141,7 @@
 #include "util-running-modes.h"
 #include "util-signal.h"
 #include "util-time.h"
+#include "util-validate.h"
 
 /*
  * we put this here, because we only use it here in main.
@@ -456,15 +457,9 @@ static int SetBpfString(int argc, char *argv[])
     if (bpf_len == 0)
         return TM_ECODE_OK;
 
-    if (EngineModeIsIPS()) {
-        SCLogError("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;
@@ -502,11 +497,6 @@ static void SetBpfStringFromFile(char *filename)
     FILE *fp = NULL;
     size_t nm = 0;
 
-    if (EngineModeIsIPS()) {
-        FatalError("BPF filter not available in IPS mode."
-                   " Use firewall filtering if possible.");
-    }
-
     fp = fopen(filename, "r");
     if (fp == NULL) {
         SCLogError("Failed to open file %s", filename);

diff --git a/src/util-bpf.c b/src/util-bpf.c
index b57dd0f512c2d5a96361e5a38ea07c1ab5a165a3..443b311c60874426376afd44add6763890e6e82e 100644 (file)
--- a/src/util-bpf.c
+++ b/src/util-bpf.c
@@ -24,6 +24,31 @@
 #include "suricata-common.h"
 #include "util-bpf.h"
 #include "threads.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,