Improved TFTP. Fixes #2075. (#2149)

Signed-off-by: Toni Uhlig <matzeton@googlemail.com>

4 files changed, 149 insertions, 65 deletions

diff --git a/doc/protocols.rst b/doc/protocols.rst
index 48de396a9..9dc776558 100644
--- a/doc/protocols.rst
+++ b/doc/protocols.rst
@@ -5,6 +5,15 @@ This page provides the list of the protocols/applications supported by nDPI. For
 
 Work in progress!
 
+.. _Proto 96:
+
+`NDPI_PROTOCOL_TFTP`
+====================
+TFTP is a very simple protocol used to transfer files. It is from this that its name comes, Trivial File Transfer Protocol or TFTP.
+
+References: `RFC1350 <https://datatracker.ietf.org/doc/html/rfc1350>`_ and `RFC2347 <https://datatracker.ietf.org/doc/html/rfc2347>`_ and `RFC2349 <https://datatracker.ietf.org/doc/html/rfc2349>`_
+
+
 .. _Proto 338:
 
 `NDPI_PROTOCOL_SRTP`
diff --git a/src/lib/protocols/tftp.c b/src/lib/protocols/tftp.c
index b44746f45..f32a7851c 100644
--- a/src/lib/protocols/tftp.c
+++ b/src/lib/protocols/tftp.c
@@ -29,18 +29,111 @@
 #include "ndpi_api.h"
 #include "ndpi_private.h"
 
-/* see: https://datatracker.ietf.org/doc/html/rfc1350 */
-
 static void ndpi_int_tftp_add_connection(struct ndpi_detection_module_struct
 					 *ndpi_struct, struct ndpi_flow_struct *flow)
 {
   ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_TFTP, NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
 }
 
+static size_t tftp_dissect_szstr(struct ndpi_packet_struct const * const packet,
+                                 size_t * const offset,
+                                 char const ** const string_start)
+{
+  if (packet->payload_packet_len <= *offset)
+  {
+    return 0;
+  }
+
+  const union {
+    uint8_t const * const as_ptr;
+    char const * const as_str;
+  } payload = { .as_ptr = packet->payload + *offset };
+
+  size_t len = strnlen(payload.as_str, packet->payload_packet_len - *offset);
+  if (len == 0 ||
+      packet->payload_packet_len <= *offset + len ||
+      payload.as_str[len] != '\0')
+  {
+    return 0;
+  }
+
+  if (string_start != NULL)
+  {
+    *string_start = payload.as_str;
+  }
+  *offset += len + 1;
+  return len;
+}
+
+static int tftp_dissect_mode(struct ndpi_packet_struct const * const packet,
+                             size_t * const offset)
+{
+  static char const * const valid_modes[] = {
+    "netascii", "octet", "mail"
+  };
+  char const * string_start;
+  size_t string_length = tftp_dissect_szstr(packet, offset, &string_start);
+  size_t i;
+
+  if (string_length == 0)
+  {
+    return 1;
+  }
+
+  for (i = 0; i < NDPI_ARRAY_LENGTH(valid_modes); ++i)
+  {
+    if (strncasecmp(string_start, valid_modes[i], string_length) == 0)
+    {
+      break;
+    }
+  }
+
+  return i == NDPI_ARRAY_LENGTH(valid_modes);
+}
+
+static int tftp_dissect_options(struct ndpi_packet_struct const * const packet,
+                                size_t * const offset)
+{
+  static char const * const valid_options[] = {
+    "blksize", "tsize"
+  };
+  uint8_t options_used[NDPI_ARRAY_LENGTH(valid_options)] = {0, 0};
+  size_t i;
+
+  do {
+    char const * string_start;
+    size_t string_length = tftp_dissect_szstr(packet, offset, &string_start);
+
+    if (string_length == 0 ||
+        tftp_dissect_szstr(packet, offset, NULL) == 0 /* value, not interested */)
+    {
+      break;
+    }
+
+    for (i = 0; i < NDPI_ARRAY_LENGTH(valid_options); ++i)
+    {
+      if (strncasecmp(string_start, valid_options[i], string_length) == 0)
+      {
+        break;
+      }
+    }
+
+    if (i == NDPI_ARRAY_LENGTH(valid_options) /* option not found in valid_options */ ||
+        options_used[i] != 0 /* duplicate options are not allowed */)
+    {
+      break;
+    }
+
+    options_used[i] = 1;
+  } while (1);
+
+  return *offset != packet->payload_packet_len;
+}
+
 static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
 			     struct ndpi_flow_struct *flow)
 {
-  struct ndpi_packet_struct *packet = &ndpi_struct->packet;
+  struct ndpi_packet_struct const * const packet = &ndpi_struct->packet;
   u_int16_t block_num;
   u_int16_t prev_num;
 
@@ -68,60 +161,26 @@ static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
         }
 
         {
-          uint8_t mode_found = 0;
-          size_t mode_len;
-          int i;
           size_t filename_len = 0;
-          const char *string;
-          size_t len = 0;
-          bool first = true;
+          size_t offset = 2;
+          char const * filename_start;
+
+          filename_len = tftp_dissect_szstr(packet, &offset, &filename_start);
 
-          /* Skip 2 byte opcode. */
-          for (i = 2; i < packet->payload_packet_len; i++)
+          /* Exclude the flow as TFPT if there was no filename and mode in the first two strings. */
+          if (filename_len == 0 || ndpi_is_printable_buffer((uint8_t *)filename_start, filename_len) == 0)
           {
-            /* Search through the payload until we find a NULL terminated string. */
-            if (packet->payload[i] != '\0')
-            {
-              len++;
-              continue;
-            }
-            string = (const char *)&packet->payload[i - len];
-
-            /* Filename should be immediately after opcode followed by the mode. */
-            if (first)
-            {
-              filename_len = len;
-              len = 0;
-              first = false;
-              continue;
-            }
-
-            char const * const possible_modes[] = { "netascii", "octet", "mail" };
-            uint8_t mode_idx;
-
-            /* Check the string in the payload against the possible TFTP modes. */
-            for(mode_idx = 0; mode_idx < NDPI_ARRAY_LENGTH(possible_modes); ++mode_idx)
-              {
-                mode_len = strlen(possible_modes[mode_idx]);
-                /* Both are now null terminated */
-                if (len != mode_len)
-                {
-                  continue;
-                }
-                if (strncasecmp(string, possible_modes[mode_idx], mode_len) == 0)
-                {
-                  mode_found = 1;
-                  break;
-                }
-              }
-
-            /* Second string searched must've been the mode, break out before any following options. */
-            break;
+            NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+            return;
           }
 
-          /* Exclude the flow as TFPT if there was no filename and mode in the first two strings. */
-          if (filename_len == 0 || ndpi_is_printable_buffer(&packet->payload[2], filename_len) == 0 ||
-              mode_found == 0)
+          if (tftp_dissect_mode(packet, &offset) != 0)
+          {
+            NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+            return;
+          }
+
+          if (tftp_dissect_options(packet, &offset) != 0)
           {
             NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
             return;
@@ -129,7 +188,7 @@ static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
 
           /* Dissect RRQ/WWQ filename. */
           filename_len = ndpi_min(filename_len, sizeof(flow->protos.tftp.filename) - 1);
-          memcpy(flow->protos.tftp.filename, &packet->payload[2], filename_len);
+          memcpy(flow->protos.tftp.filename, filename_start, filename_len);
           flow->protos.tftp.filename[filename_len] = '\0';
 
           /* We have seen enough and do not need any more TFTP packets. */
@@ -158,6 +217,7 @@ static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
 
     case 0x04:
         /* Acknowledgment (ACK) */
+
         if (packet->payload_packet_len != 4 /* ACK has a fixed packet size */)
         {
           NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
@@ -188,7 +248,20 @@ static void ndpi_search_tftp(struct ndpi_detection_module_struct *ndpi_struct,
 
     case 0x06:
         /* Option Acknowledgment (OACK) */
-        /* No fixed lengths as it can include the options from the request. */
+
+        {
+          size_t offset = 2;
+
+          if (tftp_dissect_options(packet, &offset) != 0)
+          {
+            NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+            return;
+          }
+        }
+
+        /* We have seen enough and do not need any more TFTP packets. */
+        NDPI_LOG_INFO(ndpi_struct, "found tftp (OACK)\n");
+        ndpi_int_tftp_add_connection(ndpi_struct, flow);
         break;
 
     default:
diff --git a/tests/cfgs/default/pcap/tftp.pcap b/tests/cfgs/default/pcap/tftp.pcap
index c2e93ceab..3eb613b99 100644
--- a/tests/cfgs/default/pcap/tftp.pcap
+++ b/tests/cfgs/default/pcap/tftp.pcap
diff --git a/tests/cfgs/default/result/tftp.pcap.out b/tests/cfgs/default/result/tftp.pcap.out
index c6e6ab3c2..a73a2d709 100644
--- a/tests/cfgs/default/result/tftp.pcap.out
+++ b/tests/cfgs/default/result/tftp.pcap.out
@@ -1,9 +1,9 @@
 Guessed flow protos:	2
 
-DPI Packets (UDP):	13	(1.86 pkts/flow)
+DPI Packets (UDP):	15	(1.67 pkts/flow)
 Confidence Match by port    : 2 (flows)
-Confidence DPI              : 5 (flows)
-Num dissector calls: 553 (79.00 diss/flow)
+Confidence DPI              : 7 (flows)
+Num dissector calls: 592 (65.78 diss/flow)
 LRU cache ookla:      0/0/0 (insert/search/found)
 LRU cache bittorrent: 0/6/0 (insert/search/found)
 LRU cache zoom:       0/0/0 (insert/search/found)
@@ -17,19 +17,21 @@ Automa domain:        0/0 (search/found)
 Automa tls cert:      0/0 (search/found)
 Automa risk mask:     0/0 (search/found)
 Automa common alpns:  0/0 (search/found)
-Patricia risk mask:   10/0 (search/found)
+Patricia risk mask:   14/0 (search/found)
 Patricia risk mask IPv6: 0/0 (search/found)
 Patricia risk:        0/0 (search/found)
 Patricia risk IPv6:   0/0 (search/found)
-Patricia protocols:   14/0 (search/found)
+Patricia protocols:   18/0 (search/found)
 Patricia protocols IPv6: 0/0 (search/found)
 
-TFTP	107	31296	7
+TFTP	109	31453	9
 
 	1	UDP 192.168.0.10:3445 <-> 192.168.0.253:50618 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 4][cat: DataTransfer/4][49 pkts/26853 bytes <-> 49 pkts/2940 bytes][Goodput ratio: 92/7][< 1 sec][bytes ratio: 0.803 (Upload)][IAT c2s/s2c min/avg/max/stddev: 2/2 3/3 9/7 2/2][Pkt Len c2s/s2c min/avg/max/stddev: 69/60 548/60 558/60 69/0][Risk: ** Known Proto on Non Std Port **][Risk Score: 50][PLAIN TEXT (Network Working Group          )][Plen Bins: 51,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,48,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
 	2	UDP 172.28.5.170:62058 <-> 172.28.5.91:44618 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 4][cat: DataTransfer/4][2 pkts/92 bytes <-> 2 pkts/1116 bytes][Goodput ratio: 9/92][0.00 sec][Risk: ** Known Proto on Non Std Port **][Risk Score: 50][PLAIN TEXT (BCCCCCC)][Plen Bins: 50,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,50,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
-	3	UDP 192.168.0.253:50618 -> 192.168.0.10:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/62 bytes -> 0 pkts/0 bytes][Goodput ratio: 32/0][< 1 sec][Filename: rfc1350.txt][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (1350.txt)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
-	4	UDP 172.28.4.53:54626 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: Match by port][DPI packets: 1][cat: DataTransfer/4][1 pkts/61 bytes -> 0 pkts/0 bytes][Goodput ratio: 31/0][< 1 sec][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
-	5	UDP 172.28.4.53:54627 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/61 bytes -> 0 pkts/0 bytes][Goodput ratio: 31/0][< 1 sec][Filename: sysman.lis][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (sysman.lis)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
-	6	UDP 172.28.5.91:44618 -> 172.28.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/60 bytes -> 0 pkts/0 bytes][Goodput ratio: 30/0][< 1 sec][Filename: zz.bin][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (zz.bin)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
-	7	UDP 172.28.4.53:54632 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: Match by port][DPI packets: 1][cat: DataTransfer/4][1 pkts/51 bytes -> 0 pkts/0 bytes][Goodput ratio: 17/0][< 1 sec][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	3	UDP 192.168.2.45:35840 -> 192.168.2.200:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/87 bytes -> 0 pkts/0 bytes][Goodput ratio: 51/0][< 1 sec][Filename: empty100KB][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (blksize)][Plen Bins: 0,100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	4	UDP 192.168.2.200:47649 -> 192.168.2.45:35840 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/70 bytes -> 0 pkts/0 bytes][Goodput ratio: 39/0][< 1 sec][Risk: ** Known Proto on Non Std Port **** Unidirectional Traffic **][Risk Score: 60][Risk Info: No server to client traffic][PLAIN TEXT (blksize)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	5	UDP 192.168.0.253:50618 -> 192.168.0.10:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/62 bytes -> 0 pkts/0 bytes][Goodput ratio: 32/0][< 1 sec][Filename: rfc1350.txt][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (1350.txt)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	6	UDP 172.28.4.53:54626 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: Match by port][DPI packets: 1][cat: DataTransfer/4][1 pkts/61 bytes -> 0 pkts/0 bytes][Goodput ratio: 31/0][< 1 sec][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	7	UDP 172.28.4.53:54627 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/61 bytes -> 0 pkts/0 bytes][Goodput ratio: 31/0][< 1 sec][Filename: sysman.lis][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (sysman.lis)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	8	UDP 172.28.5.91:44618 -> 172.28.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 1][cat: DataTransfer/4][1 pkts/60 bytes -> 0 pkts/0 bytes][Goodput ratio: 30/0][< 1 sec][Filename: zz.bin][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][PLAIN TEXT (zz.bin)][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	9	UDP 172.28.4.53:54632 -> 172.16.5.170:69 [proto: 96/TFTP][IP: 0/Unknown][ClearText][Confidence: Match by port][DPI packets: 1][cat: DataTransfer/4][1 pkts/51 bytes -> 0 pkts/0 bytes][Goodput ratio: 17/0][< 1 sec][Risk: ** Unidirectional Traffic **][Risk Score: 10][Risk Info: No server to client traffic][Plen Bins: 100,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]