From b30d5702f68fb9d93861ec3ec8e316989fb834e4 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 23 May 2022 17:36:09 -0700 Subject: [PATCH 1/2] Allow pcap pktsrc to use other BPF_Program::Compile method --- src/iosource/BPF_Program.cc | 21 ++++++++++---------- src/iosource/BPF_Program.h | 9 +++++---- src/iosource/PktSrc.cc | 39 +++++++++++++++++++++---------------- src/iosource/PktSrc.h | 4 +++- src/iosource/pcap/Source.cc | 19 ++++++++++++++++++ src/iosource/pcap/Source.h | 1 + 6 files changed, 60 insertions(+), 33 deletions(-) diff --git a/src/iosource/BPF_Program.cc b/src/iosource/BPF_Program.cc index a712a8745b..c0d07cf6ee 100644 --- a/src/iosource/BPF_Program.cc +++ b/src/iosource/BPF_Program.cc @@ -6,6 +6,8 @@ #include +#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, - unsigned int errbuf_len, bool optimize) +bool BPF_Program::Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf, + 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 ) - snprintf(errbuf, errbuf_len, "pcap_compile(%s): %s", filter, pcap_geterr(pcap)); - + errbuf = util::fmt("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 ) - safe_strncpy(errbuf, my_error, errbuf_len); - *errbuf = '\0'; + if ( err < 0 ) + errbuf = std::string(my_error); #else int err = pcap_compile_nopcap(snaplen, linktype, &m_program, (char*)filter, optimize, netmask); - if ( err < 0 && errbuf && errbuf_len ) - *errbuf = '\0'; + if ( err < 0 ) + errbuf.clear(); #endif if ( err == 0 ) diff --git a/src/iosource/BPF_Program.h b/src/iosource/BPF_Program.h index b3025a4c9d..220e67b4f1 100644 --- a/src/iosource/BPF_Program.h +++ b/src/iosource/BPF_Program.h @@ -2,7 +2,8 @@ #pragma once -#include +#include +#include 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, - unsigned int errbuf_len = 0, bool optimize = true); + bool Compile(pcap_t* pcap, const char* filter, uint32_t netmask, std::string& errbuf, + 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. diff --git a/src/iosource/PktSrc.cc b/src/iosource/PktSrc.cc index a265bd6f74..d1876f3cb1 100644 --- a/src/iosource/PktSrc.cc +++ b/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(); + + 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(); - - 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; + auto code = CompileFilter(filter); + if ( ! code ) return false; - } // Store it in vector. if ( index >= static_cast(filters.size()) ) filters.resize(index + 1); - if ( auto old = filters[index] ) + if ( auto* old = filters[index] ) delete old; filters[index] = code; diff --git a/src/iosource/PktSrc.h b/src/iosource/PktSrc.h index 6cb8333874..70229ec243 100644 --- a/src/iosource/PktSrc.h +++ b/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(); diff --git a/src/iosource/pcap/Source.cc b/src/iosource/pcap/Source.cc index 8afe55638a..093a5f8eed 100644 --- a/src/iosource/pcap/Source.cc +++ b/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(); + + 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 ) diff --git a/src/iosource/pcap/Source.h b/src/iosource/pcap/Source.h index af64076671..8c1c88992b 100644 --- a/src/iosource/pcap/Source.h +++ b/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(); From f8bc23d3e1a6542b8e2212537200b805e49992a6 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 23 May 2022 17:36:26 -0700 Subject: [PATCH 2/2] Propagate BPF_Program error message to script land --- scripts/base/frameworks/packet-filter/main.zeek | 11 +++++++++-- .../Baseline.cpp/core.pcap.filter-error/output | 4 ---- .../btest/Baseline/core.pcap.filter-error/output | 4 ---- testing/btest/core/pcap/filter-error.zeek | 15 +++++++++------ 4 files changed, 18 insertions(+), 16 deletions(-) delete mode 100644 testing/btest/Baseline.cpp/core.pcap.filter-error/output delete mode 100644 testing/btest/Baseline/core.pcap.filter-error/output diff --git a/scripts/base/frameworks/packet-filter/main.zeek b/scripts/base/frameworks/packet-filter/main.zeek index 6b97960663..ccf7318469 100644 --- a/scripts/base/frameworks/packet-filter/main.zeek +++ b/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 ) diff --git a/testing/btest/Baseline.cpp/core.pcap.filter-error/output b/testing/btest/Baseline.cpp/core.pcap.filter-error/output deleted file mode 100644 index 42ccac41ce..0000000000 --- a/testing/btest/Baseline.cpp/core.pcap.filter-error/output +++ /dev/null @@ -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" diff --git a/testing/btest/Baseline/core.pcap.filter-error/output b/testing/btest/Baseline/core.pcap.filter-error/output deleted file mode 100644 index c956191ac6..0000000000 --- a/testing/btest/Baseline/core.pcap.filter-error/output +++ /dev/null @@ -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" diff --git a/testing/btest/core/pcap/filter-error.zeek b/testing/btest/core/pcap/filter-error.zeek index 81f4c24cf9..ae9a40d70f 100644 --- a/testing/btest/core/pcap/filter-error.zeek +++ b/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: zeek -r $TRACES/workshop_2011_browse.trace %INPUT >>output 2>&1 +# @TEST-EXEC: grep "kaputt" output | grep -q "syntax error" +# @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(); } - -