Store error message from BPF compilation

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

@@ -282,7 +282,7 @@ function install(): bool
 		        $msg=fmt("Compiling packet filter failed"),
 		        $sub=tmp_filter]);
 
-		local error_string = fmt("Bad pcap filter '%s'", tmp_filter);
+		local error_string = fmt("Bad pcap filter '%s': %s", tmp_filter, Pcap::get_filter_state_string(DefaultPcapFilter));
 
 		local pkt_src_error : string = Pcap::error();
 		if ( pkt_src_error != "no error" )

src/iosource/BPF_Program.cc

@@ -78,8 +78,7 @@ BPF_Program::~BPF_Program()
 	FreeCode();
 	}
 
-bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf,
-                          bool optimize)
+bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, bool optimize)
 	{
 	if ( ! pcap )
 		return false;
@@ -88,7 +87,8 @@ bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, st
 
 	if ( pcap_compile(pcap, &m_program, (char*)filter, optimize, netmask) < 0 )
 		{
-		errbuf = util::fmt("pcap_compile(%s): %s", filter, pcap_geterr(pcap));
+		state_message = std::string(pcap_geterr(pcap));
+		state = GetStateFromMessage(state_message);
 		return false;
 		}
 
@@ -99,7 +99,7 @@ bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, st
 	}
 
 bool BPF_Program::Compile(zeek_uint_t snaplen, int linktype, const char* filter, uint32_t netmask,
-                          std::string& errbuf, bool optimize)
+                          bool optimize)
 	{
 	FreeCode();
 
@@ -120,12 +120,19 @@ bool BPF_Program::Compile(zeek_uint_t snaplen, int linktype, const char* filter,
 	int err = pcap_compile_nopcap(snaplen, linktype, &m_program, (char*)filter, optimize, netmask,
 	                              my_error);
 	if ( err < 0 )
-		errbuf = std::string(my_error);
+		{
+		state = GetStateFromMessage(errstr);
+		state_message = util::fmt("pcap_compile(%s): %s", filter, pcap_geterr(pcap);
+		}
 #else
-	int err = pcap_compile_nopcap(static_cast<int>(snaplen), linktype, &m_program, (char*)filter, optimize, netmask);
+	int err = pcap_compile_nopcap(static_cast<int>(snaplen), linktype, &m_program, (char*)filter,
+	                              optimize, netmask);
 
+	// We have no way of knowing what the error actually was because pcap_compile_nocap doesn't
+	// return an error string nor any other information, so just assume every failure is
+	// fatal.
 	if ( err < 0 )
-		errbuf.clear();
+		state = FilterState::FATAL;
 #endif
 
 	if ( err == 0 )
@@ -155,4 +162,12 @@ void BPF_Program::FreeCode()
 		}
 	}
 
+FilterState BPF_Program::GetStateFromMessage(const std::string& err)
+	{
+	if ( err.find("filtering not implemented") != std::string::npos )
+		return FilterState::WARNING;
+
+	return FilterState::FATAL;
+	}
+
 	} // namespace zeek::iosource::detail

src/iosource/BPF_Program.h

@@ -4,6 +4,7 @@
 
 #include <cstdint>
 #include <string>
+
 #include "zeek/util.h"
 
 extern "C"
@@ -11,7 +12,17 @@ extern "C"
 #include <pcap.h>
 	}
 
-namespace zeek::iosource::detail
+namespace zeek::iosource
+	{
+
+enum class FilterState : uint8_t
+	{
+	OK,
+	FATAL, // results in Reporter::Error
+	WARNING // results in Reporter::Warning
+	};
+
+namespace detail
 	{
 
 // BPF_Programs are an abstraction around struct bpf_program,
@@ -33,8 +44,7 @@ public:
 	 *
 	 * @return true on successful compilation, false otherwise.
 	 */
-	bool Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf,
-	             bool optimize = true);
+	bool Compile(pcap_t* pcap, const char* filter, uint32_t netmask, bool optimize = true);
 
 	/**
 	 * Creates a BPF program when no pcap handle is available. The parameters match the usage
@@ -43,7 +53,7 @@ public:
 	 * @return true on successful compilation, false otherwise.
 	 */
 	bool Compile(zeek_uint_t snaplen, int linktype, const char* filter, uint32_t netmask,
-	             std::string& errbuf, bool optimize = true);
+	             bool optimize = true);
 
 	/**
 	 * Returns true if this program currently contains compiled code, false otherwise.
@@ -61,14 +71,30 @@ public:
 	 */
 	bpf_program* GetProgram();
 
+	/**
+	 * Returns the state of the compilation process.
+	 */
+	FilterState GetState() const { return state; }
+
+	/**
+	 * Returns an error message, if any, that was returned from the compliation process.
+	 */
+	std::string GetStateMessage() const { return state_message; }
+
 protected:
 	void FreeCode();
 
+	FilterState GetStateFromMessage(const std::string& err);
+
 	// (I like to prefix member variables with m_, makes it clear
 	// in the implementation whether it's a global or not. --ck)
 	bool m_compiled = false;
 	bool m_matches_anything = false;
 	struct bpf_program m_program;
+
+	FilterState state = FilterState::OK;
+	std::string state_message;
 	};
 
-	} // namespace zeek::iosource::detail
+	} // namespace detail
+	} // namespace zeek::iosource

src/iosource/PktSrc.cc

@@ -203,18 +203,17 @@ bool PktSrc::ExtractNextPacketInternal()
 
 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) )
+	if ( ! code->Compile(BifConst::Pcap::snaplen, LinkType(), filter.c_str(), Netmask()) )
 		{
 		std::string msg = util::fmt("cannot compile BPF filter \"%s\"", filter.c_str());
 
-		if ( ! errbuf.empty() )
-			msg += ": " + errbuf;
+		std::string state_msg = code->GetStateMessage();
+		if ( ! state_msg.empty() )
+			msg += ": " + state_msg;
 
 		Error(msg);
-		return nullptr;
 		}
 
 	return code.release();
@@ -225,10 +224,9 @@ bool PktSrc::PrecompileBPFFilter(int index, const std::string& filter)
 	if ( index < 0 )
 		return false;
 
-	// Compile filter.
+	// Compile filter. This will always return a pointer, but may have stored an error
+	// internally.
 	auto code = CompileFilter(filter);
-	if ( ! code )
-		return false;
 
 	// Store it in vector.
 	if ( index >= static_cast<int>(filters.size()) )
@@ -239,7 +237,7 @@ bool PktSrc::PrecompileBPFFilter(int index, const std::string& filter)
 
 	filters[index] = code;
 
-	return true;
+	return code->GetState() != FilterState::FATAL;
 	}
 
 detail::BPF_Program* PktSrc::GetBPFFilter(int index)

src/iosource/PktSrc.h

@@ -5,6 +5,7 @@
 #include <sys/types.h> // for u_char
 #include <vector>
 
+#include "zeek/iosource/BPF_Program.h"
 #include "zeek/iosource/IOSource.h"
 #include "zeek/iosource/Packet.h"
 
@@ -13,11 +14,6 @@ struct pcap_pkthdr;
 namespace zeek::iosource
 	{
 
-namespace detail
-	{
-class BPF_Program;
-	}
-
 /**
  * Base class for packet sources.
  */
@@ -102,7 +98,7 @@ public:
 	 * Precompiles a BPF filter and associates the given index with it.
 	 * The compiled filter will be then available via \a GetBPFFilter().
 	 *
-	 * This is primarily a helper for packet source implementation that
+	 * This is primarily a helper for packet source implementations that
 	 * want to apply BPF filtering to their packets.
 	 *
 	 * @param index The index to associate with the filter.
@@ -139,7 +135,8 @@ public:
 	 *
 	 * @param pkt The content of the packet to filter.
 	 *
-	 * @return True if it maches. 	 */
+	 * @return True if it matches.
+	 */
 	bool ApplyBPFFilter(int index, const struct pcap_pkthdr* hdr, const u_char* pkt);
 
 	/**
@@ -158,9 +155,9 @@ public:
 	 * Precompiles a filter and associates a given index with it. The
 	 * filter syntax is defined by the packet source's implenentation.
 	 *
-	 * Derived classes must implement this to implement their filtering.
-	 * If they want to use BPF but don't support it natively, they can
-	 * call the corresponding helper method provided by \a PktSrc.
+	 * Derived classes can override this method to implement their own
+	 * filtering. If not overriden, it uses the pcap-based BPF filtering
+	 * by default.
 	 *
 	 * @param index The index to associate with the filter
 	 *
@@ -169,7 +166,10 @@ public:
 	 * @return True on success, false if a problem occurred or filtering
 	 * is not supported.
 	 */
-	virtual bool PrecompileFilter(int index, const std::string& filter) = 0;
+	virtual bool PrecompileFilter(int index, const std::string& filter)
+		{
+		return PrecompileBPFFilter(index, filter);
+		}
 
 	/**
 	 * Activates a precompiled filter with the given index.
@@ -336,6 +336,16 @@ protected:
 	 */
 	virtual void DoneWithPacket() = 0;
 
+	/**
+	 * Performs the actual filter compilation. This can be overridden to
+	 * provide a different implementation of the compiilation called by
+	 * PrecompileBPFFilter(). This is primarily used by the pcap source
+	 * use a different version of BPF_Filter::Compile;
+	 *
+	 * @param filter the filtering string being compiled.
+	 *
+	 * @return The compiled filter or nullptr if compilation failed.
+	 */
 	virtual detail::BPF_Program* CompileFilter(const std::string& filter);
 
 private:

src/iosource/pcap/Source.cc

@@ -263,25 +263,19 @@ void PcapSource::DoneWithPacket()
 	// Nothing to do.
 	}
 
-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) )
+	if ( ! code->Compile(pd, filter.c_str(), Netmask()) )
 		{
 		std::string msg = util::fmt("cannot compile BPF filter \"%s\"", filter.c_str());
 
-		if ( ! errbuf.empty() )
-			msg += ": " + errbuf;
+		std::string state_msg = code->GetStateMessage();
+		if ( ! state_msg.empty() )
+			msg += ": " + state_msg;
 
 		Error(msg);
-		return nullptr;
 		}
 
 	return code.release();
@@ -310,14 +304,16 @@ bool PcapSource::SetFilter(int index)
 		// since the default scripts will always attempt to compile
 		// and install a default filter
 		}
-	else
+	else if ( auto program = code->GetProgram() )
 		{
-		if ( pcap_setfilter(pd, code->GetProgram()) < 0 )
+		if ( pcap_setfilter(pd, program) < 0 )
 			{
 			PcapError();
 			return false;
 			}
 		}
+	else if ( code->GetState() != FilterState::OK )
+		return false;
 
 #ifndef HAVE_LINUX
 	// Linux doesn't clear counters when resetting filter.

src/iosource/pcap/Source.h

@@ -28,9 +28,9 @@ protected:
 	void Close() override;
 	bool ExtractNextPacket(Packet* pkt) override;
 	void DoneWithPacket() override;
-	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:

src/iosource/pcap/pcap.bif

@@ -8,6 +8,7 @@ const bufsize: count;
 %%{
 #include <pcap.h>
 
+#include "zeek/iosource/BPF_Program.h"
 #include "zeek/iosource/Manager.h"
 %%}
 
@@ -44,8 +45,13 @@ function precompile_pcap_filter%(id: PcapFilterID, s: string%): bool
 	bool success = true;
 
 	zeek::iosource::PktSrc* ps = zeek::iosource_mgr->GetPktSrc();
-	if ( ps && ! ps->PrecompileFilter(id->AsInt(), s->CheckString()) )
+	if ( ps )
+		{
+		bool compiled = ps->PrecompileFilter(id->AsInt(), s->CheckString());
+		auto filter = ps->GetBPFFilter(id->AsInt());
+		if ( ! compiled || ( filter && filter->GetState() != zeek::iosource::FilterState::OK ) )
 			success = false;
+		}
 
 	return zeek::val_mgr->Bool(success);
 	%}
@@ -99,7 +105,7 @@ function error%(%): string
 	if ( ps )
 		{
 		const char* err = ps->ErrorMsg();
-		if ( *err )
+		if ( err && *err )
 			return zeek::make_intrusive<zeek::StringVal>(err);
 		}