Add an heuristic to detect/ignore some anomalous TCP ACK packets (#1948)

In some networks, there are some anomalous TCP flows where the smallest
ACK packets have some kind of zero padding.
It looks like the IP and TCP headers in those frames wrongly consider the
0x00 Ethernet padding bytes as part of the TCP payload.
While this kind of packets is perfectly valid per-se, in some conditions
they might be treated by the TCP reassembler logic as (partial) overlaps,
deceiving the classification engine.
Add an heuristic to detect these packets and to ignore them, allowing
correct detection/classification.

This heuristic is configurable. Default value:
* in the library, it is disabled
* in `ndpiReader` and in the fuzzers, it is enabled (to ease testing)

Credit to @vel21ripn for the initial patch.

Close #1946

2 files changed, 34 insertions, 1 deletions

diff --git a/src/include/ndpi_typedefs.h b/src/include/ndpi_typedefs.h
index d3ccd208c..2e8c76d26 100644
--- a/src/include/ndpi_typedefs.h
+++ b/src/include/ndpi_typedefs.h
@@ -1273,6 +1273,8 @@ struct ndpi_detection_module_struct {
 
   u_int32_t aggressiveness_ookla;
 
+  int tcp_ack_paylod_heuristic;
+
   u_int16_t ndpi_to_user_proto_id[NDPI_MAX_NUM_CUSTOM_PROTOCOLS]; /* custom protocolId mapping */
   ndpi_proto_defaults_t proto_defaults[NDPI_MAX_SUPPORTED_PROTOCOLS+NDPI_MAX_NUM_CUSTOM_PROTOCOLS];
 
@@ -1691,6 +1693,17 @@ typedef enum {
     ndpi_dont_init_risk_ptree      = (1 << 14),
     ndpi_dont_load_cachefly_list   = (1 << 15),
     ndpi_track_flow_payload        = (1 << 16),
+    /* In some networks, there are some anomalous TCP flows where
+       the smallest ACK packets have some kind of zero padding.
+       It looks like the IP and TCP headers in those frames wrongly consider the
+       0x00 Ethernet padding bytes as part of the TCP payload.
+       While this kind of packets is perfectly valid per-se, in some conditions
+       they might be treated by the TCP reassembler logic as (partial) overlaps,
+       deceiving the classification engine.
+       Add an heuristic to detect these packets and to ignore them, allowing
+       correct detection/classification.
+       See #1946 for other details */
+    ndpi_enable_tcp_ack_payload_heuristic = (1 << 17),
   } ndpi_prefs;
 
 typedef struct {
diff --git a/src/lib/ndpi_main.c b/src/lib/ndpi_main.c
index 4d83def4d..cab17cc5f 100644
--- a/src/lib/ndpi_main.c
+++ b/src/lib/ndpi_main.c
@@ -2962,6 +2962,9 @@ struct ndpi_detection_module_struct *ndpi_init_detection_module(ndpi_init_prefs
 
   ndpi_str->aggressiveness_ookla = NDPI_AGGRESSIVENESS_OOKLA_TLS;
 
+  if(prefs & ndpi_enable_tcp_ack_payload_heuristic)
+    ndpi_str->tcp_ack_paylod_heuristic = 1;
+
   for(i = 0; i < NUM_CUSTOM_CATEGORIES; i++)
     ndpi_snprintf(ndpi_str->custom_category_labels[i], CUSTOM_CATEGORY_LABEL_LEN, "User custom category %u",
 	     (unsigned int) (i + 1));
@@ -5509,6 +5512,20 @@ static u_int8_t ndpi_is_multi_or_broadcast(struct ndpi_packet_struct *packet) {
 
 /* ************************************************ */
 
+static int tcp_ack_padding(struct ndpi_packet_struct *packet) {
+  const struct ndpi_tcphdr *tcph = packet->tcp;
+  if(tcph && tcph->ack && !tcph->psh &&
+     packet->payload_packet_len < 8 &&
+     packet->payload_packet_len > 1 /* To avoid TCP keep-alives */) {
+    int i;
+    for(i = 0; i < packet->payload_packet_len; i++)
+      if(packet->payload[i] != 0)
+        return 0;
+    return 1;
+  }
+  return 0;
+}
+
 void ndpi_connection_tracking(struct ndpi_detection_module_struct *ndpi_str,
 			      struct ndpi_flow_struct *flow) {
   if(!flow) {
@@ -5597,7 +5614,10 @@ void ndpi_connection_tracking(struct ndpi_detection_module_struct *ndpi_str,
 	}
       }
 
-      if(flow->next_tcp_seq_nr[0] == 0 || flow->next_tcp_seq_nr[1] == 0 ||
+      if(ndpi_str->tcp_ack_paylod_heuristic && tcp_ack_padding(packet)) {
+        NDPI_LOG_DBG2(ndpi_str, "TCP ACK with zero padding. Ignoring\n");
+        packet->tcp_retransmission = 1;
+      } else if(flow->next_tcp_seq_nr[0] == 0 || flow->next_tcp_seq_nr[1] == 0 ||
 	 (tcph->syn && flow->packet_counter == 0)) {
 	/* initialize tcp sequence counters */
 	/* the ack flag needs to be set to get valid sequence numbers from the other