Reduce false positives for H.323 over TCP (#2164)

Co-authored-by: 0xA50C1A1 <mage.wizard88@gmail.com>

1 files changed, 55 insertions, 67 deletions

diff --git a/src/lib/protocols/h323.c b/src/lib/protocols/h323.c
index 4cafd4392..ecab1cac6 100644
--- a/src/lib/protocols/h323.c
+++ b/src/lib/protocols/h323.c
@@ -26,94 +26,82 @@
 #include "ndpi_api.h"
 #include "ndpi_private.h"
 
+static void ndpi_int_h323_add_connection(struct ndpi_detection_module_struct *ndpi_struct,
+                             struct ndpi_flow_struct *flow) 
+{
+  NDPI_LOG_INFO(ndpi_struct, "found H323\n");
+  ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_H323,
+                             NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
+}
 
-struct tpkt {
-  u_int8_t version, reserved;
-  u_int16_t len;
-};
-
-static void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct, struct ndpi_flow_struct *flow)
+static void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct,
+                             struct ndpi_flow_struct *flow)
 {
   struct ndpi_packet_struct *packet = &ndpi_struct->packet;
   u_int16_t dport = 0, sport = 0;
 
   NDPI_LOG_DBG(ndpi_struct, "search H323\n");
 
-  /*
-    The TPKT protocol is used by ISO 8072 (on port 102)
-    and H.323. So this check below is to avoid ambiguities
-  */
-  if((packet->tcp != NULL) && (packet->tcp->dest != ntohs(102))) {
-    NDPI_LOG_DBG2(ndpi_struct, "calculated dport over tcp\n");
-
-    /* H323  */
-    if(packet->payload_packet_len > 5
-       && (packet->payload[0] == 0x03)
-       && (packet->payload[1] == 0x00)) {
-      struct tpkt *t = (struct tpkt*)packet->payload;
-      u_int16_t len = ntohs(t->len);
-
-      if(packet->payload_packet_len == len) {
-	flow->h323_valid_packets++;
-
-	if(flow->h323_valid_packets >= 2) {
-	  NDPI_LOG_INFO(ndpi_struct, "found H323 broadcast\n");
-	  ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_H323, NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
-	}
-      } else {
-	/* This is not H.323 */
-	NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
-	return;
+  /* TPKT header length + Q.931 header length without IE */
+  if ((packet->payload_packet_len) > 10 && (packet->tcp != NULL)) {
+    if ((packet->payload[0] == 0x03) && 
+        (packet->payload[1] == 0x00) &&
+        (ntohs(get_u_int16_t(packet->payload, 2)) == packet->payload_packet_len))
+    {
+      /* Check Q.931 Protocol Discriminator and call reference value length */
+      if ((packet->payload[4] == 0x08) && ((packet->payload[5] & 0xF) <= 3)) {
+        ndpi_int_h323_add_connection(ndpi_struct, flow);
+        return;
       }
     }
-  } else if(packet->udp != NULL) {
+  } else if (packet->udp != NULL) {
     sport = ntohs(packet->udp->source), dport = ntohs(packet->udp->dest);
     NDPI_LOG_DBG2(ndpi_struct, "calculated dport over udp\n");
 
-    if(packet->payload_packet_len >= 6 && packet->payload[0] == 0x80 && packet->payload[1] == 0x08 &&
-       (packet->payload[2] == 0xe7 || packet->payload[2] == 0x26) &&
-       packet->payload[4] == 0x00 && packet->payload[5] == 0x00)
-      {
-	NDPI_LOG_INFO(ndpi_struct, "found H323 broadcast\n");
-	ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_H323, NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
-	return;
-      }
+    if (packet->payload_packet_len >= 6 && packet->payload[0] == 0x80 &&
+        packet->payload[1] == 0x08 &&
+        (packet->payload[2] == 0xe7 || packet->payload[2] == 0x26) &&
+        packet->payload[4] == 0x00 && packet->payload[5] == 0x00)
+    {
+      ndpi_int_h323_add_connection(ndpi_struct, flow);
+      return;
+    }
     /* H323  */
-    if(sport == 1719 || dport == 1719) {
-      if((packet->payload_packet_len > 5)
-	 && (packet->payload[0] == 0x16)
-	 && (packet->payload[1] == 0x80)
-	 && (packet->payload[4] == 0x06)
-	 && (packet->payload[5] == 0x00)) {
-	NDPI_LOG_INFO(ndpi_struct, "found H323 broadcast\n");
-	ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_H323, NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
-	return;
-      } else if(packet->payload_packet_len >= 20 && packet->payload_packet_len <= 117) {
-	/* This check is quite generic: let's check another packet...*/
-	flow->h323_valid_packets++;
-	if(flow->h323_valid_packets >= 2) {
-	  NDPI_LOG_INFO(ndpi_struct, "found H323 broadcast\n");
-	  ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_H323, NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
-	}
+    if (sport == 1719 || dport == 1719) {
+      if ((packet->payload_packet_len > 5) && (packet->payload[0] == 0x16) &&
+          (packet->payload[1] == 0x80) && (packet->payload[4] == 0x06) &&
+          (packet->payload[5] == 0x00))
+      {
+        ndpi_int_h323_add_connection(ndpi_struct, flow);
+        return;
+      } else if (packet->payload_packet_len >= 20 &&
+                 packet->payload_packet_len <= 117) {
+        /* This check is quite generic: let's check another packet...*/
+        flow->h323_valid_packets++;
+        if (flow->h323_valid_packets >= 2) {
+          ndpi_int_h323_add_connection(ndpi_struct, flow);
+          return;
+        }
       } else {
-	NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
-	return;
+        NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+        return;
       }
     }
   }
-  
-  if(flow->packet_counter > 5)
+
+  if (flow->packet_counter > 5)
     NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
 }
 
-void init_h323_dissector(struct ndpi_detection_module_struct *ndpi_struct, u_int32_t *id)
+void init_h323_dissector(struct ndpi_detection_module_struct *ndpi_struct,
+                         u_int32_t *id)
 {
-  ndpi_set_bitmask_protocol_detection("H323", ndpi_struct, *id,
-				      NDPI_PROTOCOL_H323,
-				      ndpi_search_h323,
-				      NDPI_SELECTION_BITMASK_PROTOCOL_V4_V6_TCP_OR_UDP_WITH_PAYLOAD_WITHOUT_RETRANSMISSION,
-				      SAVE_DETECTION_BITMASK_AS_UNKNOWN,
-				      ADD_TO_DETECTION_BITMASK);
+  ndpi_set_bitmask_protocol_detection("H323", ndpi_struct, *id, 
+      NDPI_PROTOCOL_H323,
+      ndpi_search_h323,
+      NDPI_SELECTION_BITMASK_PROTOCOL_V4_V6_TCP_OR_UDP_WITH_PAYLOAD_WITHOUT_RETRANSMISSION,
+      SAVE_DETECTION_BITMASK_AS_UNKNOWN,
+      ADD_TO_DETECTION_BITMASK);
 
   *id += 1;
 }