From e14b695497f858efc3c10d532e8e166838e10c80 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 28 Oct 2021 16:55:02 +0200 Subject: [PATCH] Accept packets that use tcp segment offloading. 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. Addresses GH-1829 --- src/IP.h | 6 +++++- src/packet_analysis/protocol/ip/IP.cc | 11 ++++++++--- src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc | 4 +++- src/packet_analysis/protocol/tcp/TCP.cc | 4 ++++ .../core.ip-broken-header/ip-bogus-header-weird.log | 1 - 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/IP.h b/src/IP.h index 3c2fbef813..2b670cf72e 100644 --- a/src/IP.h +++ b/src/IP.h @@ -415,7 +415,11 @@ public: 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 785d914498..e18e46b2c1 100644 --- a/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc +++ b/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc @@ -115,7 +115,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