diff --git a/CHANGES b/CHANGES index 47041d4269..097e8296a1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,16 @@ +3.2.0-dev.783 | 2020-06-11 23:21:41 -0700 + + * Compare pcap_next_ex() result to PCAP_ERROR/PCAP_ERROR_BREAK (Jon Siwek, Corelight) + + * GH-977: Improve pcap error handling (Jon Siwek, Corelight) + + Switches from pcap_next() to pcap_next_ex() to better handle all error + conditions. This allows, for example, to have a non-zero exit code for + a Zeek process that fails to fully process all packets in a pcap file. + + * Remove not-useful code in iosource::Manager::OpenPktSrc (Jon Siwek, Corelight) + 3.2.0-dev.779 | 2020-06-11 23:17:46 -0700 * Rename BroType to zeek::Type (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index 8563353333..aa1082ff75 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.779 +3.2.0-dev.783 diff --git a/src/iosource/Manager.cc b/src/iosource/Manager.cc index d97b585894..ec13f32871 100644 --- a/src/iosource/Manager.cc +++ b/src/iosource/Manager.cc @@ -371,10 +371,6 @@ PktSrc* Manager::OpenPktSrc(const std::string& path, bool is_live) PktSrc* ps = (*component->Factory())(npath, is_live); assert(ps); - if ( ! ps->IsOpen() && ps->IsError() ) - // Set an error message if it didn't open successfully. - ps->Error("could not open"); - DBG_LOG(DBG_PKTIO, "Created packet source of type %s for %s", component->Name().c_str(), npath.c_str()); Register(ps); diff --git a/src/iosource/pcap/Source.cc b/src/iosource/pcap/Source.cc index 8df5111da0..897cea0f25 100644 --- a/src/iosource/pcap/Source.cc +++ b/src/iosource/pcap/Source.cc @@ -26,7 +26,6 @@ PcapSource::PcapSource(const std::string& path, bool is_live) props.path = path; props.is_live = is_live; pd = nullptr; - memset(¤t_hdr, 0, sizeof(current_hdr)); } void PcapSource::Open() @@ -197,29 +196,47 @@ bool PcapSource::ExtractNextPacket(Packet* pkt) if ( ! pd ) return false; - const u_char* data = pcap_next(pd, ¤t_hdr); + const u_char* data; + pcap_pkthdr* header; - if ( ! data ) - { - // Source has gone dry. If it's a network interface, this just means - // it's timed out. If it's a file, though, then the file has been - // exhausted. - if ( ! props.is_live ) - Close(); + int res = pcap_next_ex(pd, &header, &data); + switch ( res ) { + case PCAP_ERROR_BREAK: // -2 + // Exhausted pcap file, no more packets to read. + assert(! props.is_live); + Close(); return false; - } + case PCAP_ERROR: // -1 + // Error occurred while reading the packet. + if ( props.is_live ) + reporter->Error("failed to read a packet from %s: %s", + props.path.data(), pcap_geterr(pd)); + else + reporter->FatalError("failed to read a packet from %s: %s", + props.path.data(), pcap_geterr(pd)); + return false; + case 0: + // Read from live interface timed out (ok). + return false; + case 1: + // Read a packet without problem. + break; + default: + reporter->InternalError("unhandled pcap_next_ex return value: %d", res); + return false; + } - pkt->Init(props.link_type, ¤t_hdr.ts, current_hdr.caplen, current_hdr.len, data); + pkt->Init(props.link_type, &header->ts, header->caplen, header->len, data); - if ( current_hdr.len == 0 || current_hdr.caplen == 0 ) + if ( header->len == 0 || header->caplen == 0 ) { Weird("empty_pcap_header", pkt); return false; } ++stats.received; - stats.bytes_received += current_hdr.len; + stats.bytes_received += header->len; return true; } diff --git a/src/iosource/pcap/Source.h b/src/iosource/pcap/Source.h index 24271b6613..8ad53b6731 100644 --- a/src/iosource/pcap/Source.h +++ b/src/iosource/pcap/Source.h @@ -39,8 +39,6 @@ private: Stats stats; pcap_t *pd; - - struct pcap_pkthdr current_hdr; }; } diff --git a/testing/btest/Traces/http/no-uri.pcap b/testing/btest/Traces/http/no-uri.pcap index 86c42097a1..47a81fb225 100644 Binary files a/testing/btest/Traces/http/no-uri.pcap and b/testing/btest/Traces/http/no-uri.pcap differ diff --git a/testing/btest/Traces/trunc/trunc-hdr.pcap b/testing/btest/Traces/trunc/trunc-hdr.pcap index 689128f2bb..c7820a3685 100644 Binary files a/testing/btest/Traces/trunc/trunc-hdr.pcap and b/testing/btest/Traces/trunc/trunc-hdr.pcap differ