H323: fix false positives (#1916)

6 files changed, 40 insertions, 12 deletions

diff --git a/src/include/ndpi_typedefs.h b/src/include/ndpi_typedefs.h
index 6fa9d5581..d3ccd208c 100644
--- a/src/include/ndpi_typedefs.h
+++ b/src/include/ndpi_typedefs.h
@@ -728,9 +728,6 @@ struct ndpi_flow_tcp_struct {
   /* NDPI_PROTOCOL_IRC */
   u_int8_t irc_stage;
 
-  /* NDPI_PROTOCOL_H323 */
-  u_int8_t h323_valid_packets;
-
   /* NDPI_PROTOCOL_GNUTELLA */
   u_int8_t gnutella_msg_id[3];
 
@@ -1576,6 +1573,9 @@ struct ndpi_flow_struct {
   u_int16_t all_packets_counter;
   u_int16_t packet_direction_complete_counter[2];      // can be 0 - 65000
 
+  /* NDPI_PROTOCOL_H323 */
+  u_int8_t h323_valid_packets;
+
   /* NDPI_PROTOCOL_BITTORRENT */
   u_int8_t bittorrent_stage;		      // can be 0 - 255
   u_int8_t bt_check_performed : 1;
diff --git a/src/lib/protocols/cassandra.c b/src/lib/protocols/cassandra.c
index 25180ac31..56c536bb1 100644
--- a/src/lib/protocols/cassandra.c
+++ b/src/lib/protocols/cassandra.c
@@ -119,7 +119,7 @@ static void ndpi_search_cassandra(struct ndpi_detection_module_struct *ndpi_stru
         ndpi_check_valid_cassandra_opcode(get_u_int8_t(packet->payload, 4)) &&
         ntohl(get_u_int32_t(packet->payload, 5)) <= CASSANDRA_MAX_BODY_SIZE &&
         ntohl(get_u_int32_t(packet->payload, 5)) >= (uint32_t) (packet->payload_packet_len - CASSANDRA_HEADER_LEN) &&
-        flow->l4.tcp.h323_valid_packets == 0 /* To avoid clashing with H323 */ &&
+        flow->h323_valid_packets == 0 /* To avoid clashing with H323 */ &&
         flow->socks4_stage == 0 /* To avoid clashing with SOCKS */) {
       if (flow->packet_counter > 3)
       {
diff --git a/src/lib/protocols/h323.c b/src/lib/protocols/h323.c
index bb088b239..d0e4e8a20 100644
--- a/src/lib/protocols/h323.c
+++ b/src/lib/protocols/h323.c
@@ -68,9 +68,9 @@ static void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct, s
 	  }
 	}
 
-	flow->l4.tcp.h323_valid_packets++;
+	flow->h323_valid_packets++;
 
-	if(flow->l4.tcp.h323_valid_packets >= 2) {
+	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);
 	}
@@ -103,9 +103,12 @@ static void ndpi_search_h323(struct ndpi_detection_module_struct *ndpi_struct, s
 	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) {
-	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;
+	/* 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);
+	}
       } else {
 	NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
 	return;
diff --git a/tests/pcap/h323_false_positive.pcap b/tests/pcap/h323_false_positive.pcap
new file mode 100644
index 000000000..babf5e109
--- /dev/null
+++ b/tests/pcap/h323_false_positive.pcap
diff --git a/tests/result/h323.pcap.out b/tests/result/h323.pcap.out
index fd34cd8d6..790ea2ad5 100644
--- a/tests/result/h323.pcap.out
+++ b/tests/result/h323.pcap.out
@@ -1,9 +1,9 @@
 Guessed flow protos:	0
 
 DPI Packets (TCP):	2	(2.00 pkts/flow)
-DPI Packets (UDP):	1	(1.00 pkts/flow)
+DPI Packets (UDP):	2	(2.00 pkts/flow)
 Confidence DPI              : 2 (flows)
-Num dissector calls: 118 (59.00 diss/flow)
+Num dissector calls: 224 (112.00 diss/flow)
 LRU cache ookla:      0/0/0 (insert/search/found)
 LRU cache bittorrent: 0/0/0 (insert/search/found)
 LRU cache zoom:       0/0/0 (insert/search/found)
@@ -23,5 +23,5 @@ Patricia protocols:   2/2 (search/found)
 
 H323	12	1825	2
 
-	1	UDP 17.2.0.124:2034 <-> 17.2.0.161:1719 [proto: 158/H323][IP: 140/Apple][ClearText][Confidence: DPI][DPI packets: 1][cat: VoIP/10][3 pkts/665 bytes <-> 7 pkts/853 bytes][Goodput ratio: 81/65][80.21 sec][bytes ratio: -0.124 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 247/336 288/13362 330/70142 42/25418][Pkt Len c2s/s2c min/avg/max/stddev: 80/67 222/122 411/176 139/48][PLAIN TEXT (@333333330)][Plen Bins: 20,20,10,10,30,0,0,0,0,0,0,10,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
+	1	UDP 17.2.0.124:2034 <-> 17.2.0.161:1719 [proto: 158/H323][IP: 140/Apple][ClearText][Confidence: DPI][DPI packets: 2][cat: VoIP/10][3 pkts/665 bytes <-> 7 pkts/853 bytes][Goodput ratio: 81/65][80.21 sec][bytes ratio: -0.124 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 247/336 288/13362 330/70142 42/25418][Pkt Len c2s/s2c min/avg/max/stddev: 80/67 222/122 411/176 139/48][PLAIN TEXT (@333333330)][Plen Bins: 20,20,10,10,30,0,0,0,0,0,0,10,0,0,0,0,0,0,0,0,0,0,0,0,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	TCP 17.2.0.124:3032 <-> 17.2.0.122:1720 [proto: 158/H323][IP: 140/Apple][ClearText][Confidence: DPI][DPI packets: 2][cat: VoIP/10][1 pkts/207 bytes <-> 1 pkts/100 bytes][Goodput ratio: 74/46][0.06 sec][PLAIN TEXT (5295672)][Plen Bins: 0,50,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,0,0,0,0,0,0,0,0,0,0,0,0]
diff --git a/tests/result/h323_false_positive.pcap.out b/tests/result/h323_false_positive.pcap.out
new file mode 100644
index 000000000..021874e6f
--- /dev/null
+++ b/tests/result/h323_false_positive.pcap.out
@@ -0,0 +1,25 @@
+Guessed flow protos:	0
+
+DPI Packets (UDP):	4	(4.00 pkts/flow)
+Confidence DPI              : 1 (flows)
+Num dissector calls: 76 (76.00 diss/flow)
+LRU cache ookla:      0/0/0 (insert/search/found)
+LRU cache bittorrent: 5/0/0 (insert/search/found)
+LRU cache zoom:       0/0/0 (insert/search/found)
+LRU cache stun:       0/0/0 (insert/search/found)
+LRU cache tls_cert:   0/0/0 (insert/search/found)
+LRU cache mining:     0/0/0 (insert/search/found)
+LRU cache msteams:    0/0/0 (insert/search/found)
+LRU cache stun_zoom:  0/0/0 (insert/search/found)
+Automa host:          0/0 (search/found)
+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:   2/0 (search/found)
+Patricia risk:        0/0 (search/found)
+Patricia protocols:   2/0 (search/found)
+
+BitTorrent	4	645	1
+
+	1	UDP 10.0.2.15:23994 <-> 112.51.146.106:1719 [proto: 37/BitTorrent][IP: 0/Unknown][ClearText][Confidence: DPI][DPI packets: 4][cat: Download/7][2 pkts/231 bytes <-> 2 pkts/414 bytes][Goodput ratio: 63/80][16.92 sec][Risk: ** Known Proto on Non Std Port **][Risk Score: 50][Risk Info: Expected on port 6771,51413][PLAIN TEXT (target20)][Plen Bins: 0,50,0,25,0,0,0,0,25,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]