Merge remote-tracking branch 'origin/topic/timw/1995-pcap-filter-error-messages'

* origin/topic/timw/1995-pcap-filter-error-messages:
  Propagate BPF_Program error message to script land
  Allow pcap pktsrc to use other BPF_Program::Compile method

CHANGES

@@ -1,3 +1,9 @@
+5.0.0-dev.485 | 2022-05-25 12:09:42 -0700
+
+  * Propagate BPF_Program error message to script land (Tim Wojtulewicz, Corelight)
+
+  * Allow pcap pktsrc to use other BPF_Program::Compile method (Tim Wojtulewicz, Corelight)
+
 5.0.0-dev.482 | 2022-05-25 09:40:18 -0700
 
   * Bump Spicy and spicy-plugin. (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-5.0.0-dev.482
+5.0.0-dev.485

scripts/base/frameworks/packet-filter/main.zeek

@@ -281,10 +281,17 @@ function install(): bool
 		NOTICE([$note=Compile_Failure,
 		        $msg=fmt("Compiling packet filter failed"),
 		        $sub=tmp_filter]);
+
+		local error_string = fmt("Bad pcap filter '%s'", tmp_filter);
+
+		local pkt_src_error : string = Pcap::error();
+		if ( pkt_src_error != "no error" )
+			error_string = pkt_src_error;
+
 		if ( network_time() == 0.0 )
-			Reporter::fatal(fmt("Bad pcap filter '%s'", tmp_filter));
+			Reporter::fatal(error_string);
 		else
-			Reporter::warning(fmt("Bad pcap filter '%s'", tmp_filter));
+			Reporter::warning(error_string);
 		}
 	local diff = current_time()-ts;
 	if ( diff > max_filter_compile_time )

src/iosource/BPF_Program.cc

@@ -6,6 +6,8 @@
 
 #include <string.h>
 
+#include "zeek/util.h"
+
 #ifdef DONT_HAVE_LIBPCAP_PCAP_FREECODE
 extern "C"
 	{
@@ -76,8 +78,8 @@ BPF_Program::~BPF_Program()
 	FreeCode();
 	}
 
-bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, char* errbuf,
+bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf,
-                          unsigned int errbuf_len, bool optimize)
+                          bool optimize)
 	{
 	if ( ! pcap )
 		return false;
@@ -86,9 +88,7 @@ bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, ch
 
 	if ( pcap_compile(pcap, &m_program, (char*)filter, optimize, netmask) < 0 )
 		{
-		if ( errbuf )
+		errbuf = util::fmt("pcap_compile(%s): %s", filter, pcap_geterr(pcap));
-			snprintf(errbuf, errbuf_len, "pcap_compile(%s): %s", filter, pcap_geterr(pcap));
-
 		return false;
 		}
 
@@ -99,7 +99,7 @@ bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, ch
 	}
 
 bool BPF_Program::Compile(int snaplen, int linktype, const char* filter, uint32_t netmask,
-                          char* errbuf, unsigned int errbuf_len, bool optimize)
+                          std::string& errbuf, bool optimize)
 	{
 	FreeCode();
 
@@ -119,14 +119,13 @@ bool BPF_Program::Compile(int snaplen, int linktype, const char* filter, uint32_
 
 	int err = pcap_compile_nopcap(snaplen, linktype, &m_program, (char*)filter, optimize, netmask,
 	                              my_error);
-	if ( err < 0 && errbuf )
+	if ( err < 0 )
-		safe_strncpy(errbuf, my_error, errbuf_len);
+		errbuf = std::string(my_error);
-	*errbuf = '\0';
 #else
 	int err = pcap_compile_nopcap(snaplen, linktype, &m_program, (char*)filter, optimize, netmask);
 
-	if ( err < 0 && errbuf && errbuf_len )
+	if ( err < 0 )
-		*errbuf = '\0';
+		errbuf.clear();
 #endif
 
 	if ( err == 0 )

src/iosource/BPF_Program.h

@@ -2,7 +2,8 @@
 
 #pragma once
 
-#include <stdint.h>
+#include <cstdint>
+#include <string>
 
 extern "C"
 	{
@@ -26,14 +27,14 @@ public:
 	// Creates a BPF program for the given pcap handle.
 	// Parameters are like in pcap_compile(). Returns true
 	// for successful compilation, false otherwise.
-	bool Compile(pcap_t* pcap, const char* filter, uint32_t netmask, char* errbuf = nullptr,
+	bool Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf,
-	             unsigned int errbuf_len = 0, bool optimize = true);
+	             bool optimize = true);
 
 	// Creates a BPF program when no pcap handle is around,
 	// similarly to pcap_compile_nopcap(). Parameters are
 	// similar. Returns true on success.
 	bool Compile(int snaplen, int linktype, const char* filter, uint32_t netmask,
-	             char* errbuf = nullptr, unsigned int errbuf_len = 0, bool optimize = true);
+	             std::string& errbuf, bool optimize = true);
 
 	// Returns true if this program currently contains compiled
 	// code, false otherwise.

src/iosource/PktSrc.cc

@@ -201,35 +201,40 @@ bool PktSrc::ExtractNextPacketInternal()
 	return false;
 	}
 
+detail::BPF_Program* PktSrc::CompileFilter(const std::string& filter)
+	{
+	std::string errbuf;
+	auto code = std::make_unique<detail::BPF_Program>();
+
+	if ( ! code->Compile(BifConst::Pcap::snaplen, LinkType(), filter.c_str(), Netmask(), errbuf) )
+		{
+		std::string msg = util::fmt("cannot compile BPF filter \"%s\"", filter.c_str());
+
+		if ( ! errbuf.empty() )
+			msg += ": " + errbuf;
+
+		Error(msg);
+		return nullptr;
+		}
+
+	return code.release();
+	}
+
 bool PktSrc::PrecompileBPFFilter(int index, const std::string& filter)
 	{
 	if ( index < 0 )
 		return false;
 
-	char errbuf[PCAP_ERRBUF_SIZE];
-
 	// Compile filter.
-	auto* code = new detail::BPF_Program();
+	auto code = CompileFilter(filter);
-
+	if ( ! code )
-	if ( ! code->Compile(BifConst::Pcap::snaplen, LinkType(), filter.c_str(), Netmask(), errbuf,
-	                     sizeof(errbuf)) )
-		{
-		std::string msg = util::fmt("cannot compile BPF filter \"%s\"", filter.c_str());
-
-		if ( *errbuf )
-			msg += ": " + std::string(errbuf);
-
-		Error(msg);
-
-		delete code;
 		return false;
-		}
 
 	// Store it in vector.
 	if ( index >= static_cast<int>(filters.size()) )
 		filters.resize(index + 1);
 
-	if ( auto old = filters[index] )
+	if ( auto* old = filters[index] )
 		delete old;
 
 	filters[index] = code;

src/iosource/PktSrc.h

@@ -111,7 +111,7 @@ public:
 	 *
 	 * @return True on success, false if a problem occurred.
 	 */
-	bool PrecompileBPFFilter(int index, const std::string& filter);
+	virtual bool PrecompileBPFFilter(int index, const std::string& filter);
 
 	/**
 	 * Returns the precompiled BPF filter associated with a given index,
@@ -336,6 +336,8 @@ protected:
 	 */
 	virtual void DoneWithPacket() = 0;
 
+	virtual detail::BPF_Program* CompileFilter(const std::string& filter);
+
 private:
 	// Internal helper for ExtractNextPacket().
 	bool ExtractNextPacketInternal();

src/iosource/pcap/Source.cc

@@ -268,6 +268,25 @@ bool PcapSource::PrecompileFilter(int index, const std::string& filter)
 	return PktSrc::PrecompileBPFFilter(index, filter);
 	}
 
+detail::BPF_Program* PcapSource::CompileFilter(const std::string& filter)
+	{
+	std::string errbuf;
+	auto code = std::make_unique<detail::BPF_Program>();
+
+	if ( ! code->Compile(pd, filter.c_str(), Netmask(), errbuf) )
+		{
+		std::string msg = util::fmt("cannot compile BPF filter \"%s\"", filter.c_str());
+
+		if ( ! errbuf.empty() )
+			msg += ": " + errbuf;
+
+		Error(msg);
+		return nullptr;
+		}
+
+	return code.release();
+	}
+
 bool PcapSource::SetFilter(int index)
 	{
 	if ( ! pd )

src/iosource/pcap/Source.h

@@ -31,6 +31,7 @@ protected:
 	bool PrecompileFilter(int index, const std::string& filter) override;
 	bool SetFilter(int index) override;
 	void Statistics(Stats* stats) override;
+	detail::BPF_Program* CompileFilter(const std::string& filter) override;
 
 private:
 	void OpenLive();

testing/btest/Baseline.cpp/core.pcap.filter-error/output

@@ -1,4 +0,0 @@
-### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-fatal error: Bad pcap filter 'kaputt'
-----
-error, cannot compile BPF filter "kaputt, too"

testing/btest/Baseline/core.pcap.filter-error/output

@@ -1,4 +0,0 @@
-### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-fatal error in <...>/main.zeek, line 285: Bad pcap filter 'kaputt'
-----
-error, cannot compile BPF filter "kaputt, too"

testing/btest/core/pcap/filter-error.zeek

@@ -1,9 +1,14 @@
-# @TEST-EXEC-FAIL: zeek -r $TRACES/workshop_2011_browse.trace -f "kaputt" >>output 2>&1
+# Due to the instability of the output from libpcap when it comes to errors when compiling
+# filters, we can't rely on a fixed baseline here to diff against. Instead, just do some
+# greps to validate that we got a syntax error in the output with the string that we passed
+# as a filter.
+
+# @TEST-EXEC-FAIL: zeek -r $TRACES/workshop_2011_browse.trace -f "kaputt" >output 2>&1
 # @TEST-EXEC-FAIL: test -e conn.log
-# @TEST-EXEC: echo ---- >>output
+# @TEST-EXEC: grep "kaputt" output | grep -q "syntax error"
-# @TEST-EXEC: zeek -r $TRACES/workshop_2011_browse.trace  %INPUT >>output 2>&1
+# @TEST-EXEC: zeek -r $TRACES/workshop_2011_browse.trace  %INPUT >output 2>&1
 # @TEST-EXEC: test -e conn.log
-# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output
+# @TEST-EXEC: grep "kaputt, too" output | grep -q "syntax error"
 
 redef enum PcapFilterID += { A };
 
@@ -12,5 +17,3 @@ event zeek_init()
 	if ( ! Pcap::precompile_pcap_filter(A, "kaputt, too") )
 		print "error", Pcap::error();
 	}
-
-