Merge remote-tracking branch 'origin/topic/johanna/gh-1829'

* origin/topic/johanna/gh-1829:
  Add documentation for GH-1829
  Packets with TSO: address review feedback.
  Accept packets that use tcp segment offloading.

Fixes GH-1829

CHANGES

@@ -1,3 +1,18 @@
+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)

NEWS

@@ -74,6 +74,13 @@ Changed Functionality
   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
 ---------------------
 

VERSION

@@ -1 +1 @@
-4.2.0-dev.333
+4.2.0-dev.340

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

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;
 

src/IP.cc

@@ -384,7 +384,13 @@ RecordValPtr IP_Hdr::ToPktHdrVal(RecordValPtr pkt_hdr, int sindex) const
 			auto tcp_hdr = make_intrusive<RecordVal>(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));

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();
 		}

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);
 
+		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;

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;

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<TCPSessionAdapter*>(c->GetSessionAdapter());
 
 	const struct tcphdr* tp = ExtractTCP_Header(data, len, remaining, adapter);

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