diff --git a/CHANGES b/CHANGES index 6219833a9a..714f19e641 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,22 @@ +4.2.0-dev.340 | 2021-11-23 10:10:13 +0000 + + * Accept packets that use tcp segment offloading. (Johanna Amann, Corelight) + + When checksum offloading is enabled, we now forward packets that + have 0 header lengths set - and assume that they have TSO enabled. + + If checksum offloading is not enabled, we drop the packets (GH-1829) + + * Updates to NEWS to cover recent additions. [nomail] [skip ci] (Christian Kreibich, Corelight) + + * Update doc and auxil/zeek-aux submodules [nomail] [skip ci] (Christian Kreibich, Corelight) + + * Update cmake and aux/zeek-aux submodules [nomail] [skip ci] (Christian Kreibich, Corelight) + +4.2.0-dev.333 | 2021-11-17 11:57:04 -0800 + + * Clean up fully after successful Docker btests (Christian Kreibich, Corelight) + 4.2.0-dev.331 | 2021-11-15 10:10:52 -0800 * Fix ref-naming typo in the Github Docker workflow (Christian Kreibich, Corelight) diff --git a/NEWS b/NEWS index ca92bc610e..c29e0ff754 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,12 @@ New Functionality - A new command-line option ``-c`` or ``--capture-unprocessed`` will dump any packets not marked as being processed, similar to the new hook and event above. +- In Zeek plugins, the new cmake function ``zeek_plugin_scripts()`` should be + used alongside ``zeek_plugin_cc()`` and related functions to establish + dependency tracking between Zeek scripts shipped with the plugin and plugin + rebuilds. Previously, updates to included Zeek scripts didn't reliably + trigger a rebuild. + Changed Functionality --------------------- @@ -64,6 +70,17 @@ Changed Functionality - The ``SYN_packet`` record now records TCP timestamps (TSval/TSecr) when available. +- The ``init-plugin`` script now focuses purely on dynamic Zeek plugins. It no + longer generates Zeek packages. To instantiate new Zeek packages, use the + ``zkg create`` command instead. + +- The ``ignore_checksums`` options and the ``-C`` command-line option now additionally cause + Zeek to accept IPv4 packets that provide a length of zero in the total-length IPv4 header + field. When the length is set to zero, the capture length of the packet is used instead. + This can be used to replay traces, or analyze traffic when TCP sequence offloading is enabled + on the local NIC - which typically causes the total-length of affected packets to be set to + zero. + Removed Functionality --------------------- diff --git a/VERSION b/VERSION index a10f45f805..7532658e5f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.2.0-dev.331 +4.2.0-dev.340 diff --git a/auxil/zeek-aux b/auxil/zeek-aux index 00ec86d2da..9100b9d524 160000 --- a/auxil/zeek-aux +++ b/auxil/zeek-aux @@ -1 +1 @@ -Subproject commit 00ec86d2dad3c2d23438431a3ccb07e11dc59543 +Subproject commit 9100b9d524dddfade02f1b4fceb54265a113b68c diff --git a/cmake b/cmake index 9b6b9fe5cc..9ae6612fdd 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit 9b6b9fe5cc454949493dc24c782917f19b41b6ce +Subproject commit 9ae6612fdd6caa76a17752d07c862547d80b6aab diff --git a/doc b/doc index 834027ec7c..43e7a5d288 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 834027ec7c6463aa37c1d8233b0ef0052b2ebf0a +Subproject commit 43e7a5d288e0dd60d4113a8ef8f42ec8df4e080e diff --git a/docker/btest/Makefile b/docker/btest/Makefile index 65d4d65c98..30d0348a72 100644 --- a/docker/btest/Makefile +++ b/docker/btest/Makefile @@ -1,11 +1,11 @@ DIAG=diag.log BTEST=../../auxil/btest/btest -all: cleanup btest-verbose +all: btest-verbose clean # Showing all tests. btest-verbose: @$(BTEST) -d -j -f $(DIAG) -cleanup: - @rm -f $(DIAG) +clean: + @rm -rf $(DIAG) .tmp .btest.failed.dat diff --git a/man/zeek.8 b/man/zeek.8 index 1a667f5630..7b6e046ed6 100644 --- a/man/zeek.8 +++ b/man/zeek.8 @@ -66,7 +66,7 @@ print version and exit print contents of state file .TP \fB\-C\fR,\ \-\-no\-checksums -ignore checksums +When this option is set, Zeek ignores invalid packet checksums and does process the packets. Furthermore, if this option is set Zeek also processes IP packets with a zero total length field, which is typically caused by TCP (TCP Segment Offloading) on the NIC. .TP \fB\-F\fR,\ \-\-force\-dns force DNS diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index a4d4e1ae17..3ab33fb8eb 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -1016,9 +1016,16 @@ const TCP_RESET = 6; ##< Endpoint has sent RST. const UDP_INACTIVE = 0; ##< Endpoint is still inactive. const UDP_ACTIVE = 1; ##< Endpoint has sent something. -## If true, don't verify checksums. Useful for running on altered trace -## files, and for saving a few cycles, but at the risk of analyzing invalid -## data. Note that the ``-C`` command-line option overrides the setting of this +## If true, don't verify checksums, and accept packets that give a length of +## zero in the IPv4 header. This is useful when running against traces of local +## traffic and the NIC checksum offloading feature is enabled. It can also +## be useful for running on altered trace files, and for saving a few cycles +## at the risk of analyzing invalid data. +## With this option, packets that have a value of zero in the total-length field +## of the IPv4 header are also accepted, and the capture-length is used instead. +## The total-length field is commonly set to zero when the NIC sequence offloading +## feature is enabled. +## Note that the ``-C`` command-line option overrides the setting of this ## variable. const ignore_checksums = F &redef; diff --git a/src/IP.cc b/src/IP.cc index 04ac1b6676..330f2b5a85 100644 --- a/src/IP.cc +++ b/src/IP.cc @@ -384,7 +384,13 @@ RecordValPtr IP_Hdr::ToPktHdrVal(RecordValPtr pkt_hdr, int sindex) const auto tcp_hdr = make_intrusive(tcp_hdr_type); int tcp_hdr_len = tp->th_off * 4; - int data_len = PayloadLen() - tcp_hdr_len; + + // account for cases in which the payload length in the TCP header is not set, + // or is set to an impossible value. In these cases, return 0. + int data_len = 0; + auto payload_len = PayloadLen(); + if ( payload_len >= tcp_hdr_len ) + data_len = payload_len - tcp_hdr_len; tcp_hdr->Assign(0, val_mgr->Port(ntohs(tp->th_sport), TRANSPORT_TCP)); tcp_hdr->Assign(1, val_mgr->Port(ntohs(tp->th_dport), TRANSPORT_TCP)); diff --git a/src/IP.h b/src/IP.h index 3c2fbef813..0488290b00 100644 --- a/src/IP.h +++ b/src/IP.h @@ -411,11 +411,18 @@ public: /** * Returns the length of the IP packet's payload (length of packet minus * header length or, for IPv6, also minus length of all extension headers). + * + * Also returns 0 if the IPv4 length field is set to zero - which is, e.g., + * the case when TCP segment offloading is enabled. */ uint16_t PayloadLen() const { if ( ip4 ) - return ntohs(ip4->ip_len) - ip4->ip_hl * 4; + { + // prevent overflow in case of segment offloading/zeroed header length. + auto total_len = ntohs(ip4->ip_len); + return total_len ? total_len - ip4->ip_hl * 4 : 0; + } return ntohs(ip6->ip6_plen) + 40 - ip6_hdrs->TotalLength(); } diff --git a/src/packet_analysis/protocol/ip/IP.cc b/src/packet_analysis/protocol/ip/IP.cc index e7ea5e75da..cb45f02e7e 100644 --- a/src/packet_analysis/protocol/ip/IP.cc +++ b/src/packet_analysis/protocol/ip/IP.cc @@ -81,8 +81,13 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) // TCP segmentation offloading can zero out the ip_len field. Weird("ip_hdr_len_zero", packet); - // Cope with the zero'd out ip_len field by using the caplen. - total_len = packet->cap_len - hdr_size; + if ( detail::ignore_checksums ) + // Cope with the zero'd out ip_len field by using the caplen. + total_len = packet->cap_len - hdr_size; + else + // If this is caused by segmentation offloading, the checksum will + // also be incorrect. If checksum validation is enabled - jus tbail here. + return false; } if ( packet->len < total_len + hdr_size ) @@ -236,7 +241,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) packet->proto = proto; // Double check the lengths one more time before forwarding this on. - if ( packet->ip_hdr->TotalLen() < packet->ip_hdr->HdrLen() ) + if ( total_len < packet->ip_hdr->HdrLen() ) { Weird("bogus_IP_header_lengths", packet); return false; diff --git a/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc b/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc index 16e411c306..81466dbd60 100644 --- a/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc +++ b/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc @@ -119,7 +119,9 @@ bool IPBasedAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pkt bool IPBasedAnalyzer::CheckHeaderTrunc(size_t min_hdr_len, size_t remaining, Packet* packet) { - if ( packet->ip_hdr->PayloadLen() < min_hdr_len ) + // If segment offloading or similar is enabled, the payload len will return 0. + // Thus, let's ignore that case. + if ( packet->ip_hdr->PayloadLen() && packet->ip_hdr->PayloadLen() < min_hdr_len ) { Weird("truncated_header", packet); return false; diff --git a/src/packet_analysis/protocol/tcp/TCP.cc b/src/packet_analysis/protocol/tcp/TCP.cc index 5a45e28438..cdd8a4c80f 100644 --- a/src/packet_analysis/protocol/tcp/TCP.cc +++ b/src/packet_analysis/protocol/tcp/TCP.cc @@ -96,6 +96,10 @@ void TCPAnalyzer::DeliverPacket(Connection* c, double t, bool is_orig, int remai { const u_char* data = pkt->ip_hdr->Payload(); int len = pkt->ip_hdr->PayloadLen(); + // If the header length is zero, tcp checksum offloading is probably enabled + // In this case, let's fix up the length. + if ( pkt->ip_hdr->TotalLen() == 0 ) + len = remaining; auto* adapter = static_cast(c->GetSessionAdapter()); const struct tcphdr* tp = ExtractTCP_Header(data, len, remaining, adapter); diff --git a/testing/btest/Baseline/core.ip-broken-header/ip-bogus-header-weird.log b/testing/btest/Baseline/core.ip-broken-header/ip-bogus-header-weird.log index d5adac806f..179c4fd1a7 100644 --- a/testing/btest/Baseline/core.ip-broken-header/ip-bogus-header-weird.log +++ b/testing/btest/Baseline/core.ip-broken-header/ip-bogus-header-weird.log @@ -8,5 +8,4 @@ #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer source #types time string addr port addr port string string bool string string XXXXXXXXXX.XXXXXX - 118.181.144.194 0 136.255.115.116 0 ip_hdr_len_zero - F zeek IP -XXXXXXXXXX.XXXXXX - 118.181.144.194 0 136.255.115.116 0 bogus_IP_header_lengths - F zeek IP #close XXXX-XX-XX-XX-XX-XX