BFCP: fix check on payload length and extract metadata (#2854)

We should be able to identified this protocol on the first packet,
without keeping any state

Close #2745

7 files changed, 49 insertions, 30 deletions

diff --git a/example/ndpiReader.c b/example/ndpiReader.c
index d26505062..a9b89c6ba 100644
--- a/example/ndpiReader.c
+++ b/example/ndpiReader.c
@@ -2088,6 +2088,11 @@ static void printFlow(u_int32_t id, struct ndpi_flow_info *flow, u_int16_t threa
             fprintf(out, "[User-agent: %s]", flow->fast_cgi.user_agent);
           }
         break;
+
+      case INFO_BFCP:
+        fprintf(out, "[Conference Id: %d]", flow->bfcp.conference_id);
+        fprintf(out, "[User Id: %d]", flow->bfcp.user_id);
+        break;
       }
 
     if(flow->ssh_tls.advertised_alpns)
diff --git a/example/reader_util.c b/example/reader_util.c
index 8fc8db21e..3efeaa595 100644
--- a/example/reader_util.c
+++ b/example/reader_util.c
@@ -1503,6 +1503,12 @@ void process_ndpi_collected_info(struct ndpi_workflow * workflow, struct ndpi_fl
     if(flow->ndpi_flow->protos.sip.to_imsi[0] != '\0')
       ndpi_snprintf(flow->sip.to_imsi, sizeof(flow->sip.to_imsi), "%s", flow->ndpi_flow->protos.sip.to_imsi);
   }
+  /* BFCP */
+  else if(is_ndpi_proto(flow, NDPI_PROTOCOL_BFCP)) {
+    flow->info_type = INFO_BFCP;
+    flow->bfcp.conference_id = flow->ndpi_flow->protos.bfcp.conference_id;
+    flow->bfcp.user_id = flow->ndpi_flow->protos.bfcp.user_id;
+  }
   /* TELNET */
   else if(is_ndpi_proto(flow, NDPI_PROTOCOL_TELNET)) {
     if(flow->ndpi_flow->protos.telnet.username[0] != '\0')
diff --git a/example/reader_util.h b/example/reader_util.h
index 4b2b2284c..7ff527916 100644
--- a/example/reader_util.h
+++ b/example/reader_util.h
@@ -172,6 +172,7 @@ enum info_type {
     INFO_NATPMP,
     INFO_SIP,
     INFO_FASTCGI,
+    INFO_BFCP,
 };
 
 typedef struct {
@@ -277,6 +278,11 @@ typedef struct ndpi_flow_info {
       char user_agent[32];
       char url[64];
     } fast_cgi;
+
+    struct {
+      u_int32_t conference_id;
+      u_int16_t user_id;
+    } bfcp;
   };
 
   ndpi_serializer ndpi_flow_serializer;
diff --git a/src/include/ndpi_typedefs.h b/src/include/ndpi_typedefs.h
index fba4bc270..d96da8c57 100644
--- a/src/include/ndpi_typedefs.h
+++ b/src/include/ndpi_typedefs.h
@@ -1636,6 +1636,11 @@ struct ndpi_flow_struct {
       char url[64];
     } fast_cgi;
 
+    struct {
+      u_int32_t conference_id;
+      u_int16_t user_id;
+    } bfcp;
+
   } protos;
 
   /* **Packet** metadata for flows where monitoring is enabled. It is reset after each packet! */
@@ -1656,10 +1661,6 @@ struct ndpi_flow_struct {
   /* NDPI_PROTOCOL_TEAMVIEWER */
   u_int8_t teamviewer_stage : 3;
 
-  /* NDPI_PROTOCOL_BFCP */
-  u_int8_t bfcp_stage:1;
-  u_int32_t bfcp_conference_id;
-
   /* NDPI_PROTOCOL_OPENVPN */
   u_int8_t ovpn_session_id[2][8];
   u_int8_t ovpn_alg_standard_state : 2;
diff --git a/src/lib/protocols/bfcp.c b/src/lib/protocols/bfcp.c
index 530e5c4fa..8e7917b28 100644
--- a/src/lib/protocols/bfcp.c
+++ b/src/lib/protocols/bfcp.c
@@ -43,9 +43,15 @@ static void ndpi_search_bfcp(struct ndpi_detection_module_struct *ndpi_struct,
   }
 
   u_int8_t version = (packet->payload[0] >> 5) & 0x07;
-  u_int8_t reserved = (packet->payload[0] >> 3) & 0x01;
+  u_int8_t reserved = (packet->payload[0] & 0x03);
 
-  if (version != 1 || reserved != 0) {
+  /* RFC4582: 1
+     RFC8855: 1 on TCP, 2 on UDP */
+  if (!(version == 1 ||
+        (version == 2 && flow->l4_proto == IPPROTO_UDP))) {
+    goto not_bfcp;
+  }
+  if (reserved != 0) {
     goto not_bfcp;
   }
 
@@ -54,24 +60,18 @@ static void ndpi_search_bfcp(struct ndpi_detection_module_struct *ndpi_struct,
     goto not_bfcp;
   }
 
-  u_int16_t bfcp_payload_len = packet->payload_packet_len - 12;
-  if (bfcp_payload_len != ntohs(get_u_int16_t(packet->payload, 2))) {
+  u_int16_t length = ntohs(get_u_int16_t(packet->payload, 2));
+  if (12 + length * 4 != packet->payload_packet_len) {
     goto not_bfcp;
   }
 
-  u_int32_t conference_id = ntohl(get_u_int32_t(packet->payload, 4));
-  if (!flow->bfcp_stage) {
-    flow->bfcp_conference_id = conference_id;
-    flow->bfcp_stage = 1;
-    return;
-  }
+  NDPI_LOG_INFO(ndpi_struct, "found BFCP\n");
+  ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_BFCP,
+                                               NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
 
-  if (flow->bfcp_stage && flow->bfcp_conference_id == conference_id) {
-    NDPI_LOG_INFO(ndpi_struct, "found BFCP\n");
-    ndpi_set_detected_protocol(ndpi_struct, flow, NDPI_PROTOCOL_BFCP,
-                               NDPI_PROTOCOL_UNKNOWN, NDPI_CONFIDENCE_DPI);
-    return;
-  }
+  flow->protos.bfcp.conference_id = ntohl(get_u_int32_t(packet->payload, 4));
+  flow->protos.bfcp.user_id = ntohs(get_u_int16_t(packet->payload, 10));
+  return;
 
 not_bfcp:
   NDPI_EXCLUDE_DISSECTOR(ndpi_struct, flow);
diff --git a/tests/cfgs/default/pcap/bfcp.pcapng b/tests/cfgs/default/pcap/bfcp.pcapng
index 98bd554cc..c312897a5 100644
--- a/tests/cfgs/default/pcap/bfcp.pcapng
+++ b/tests/cfgs/default/pcap/bfcp.pcapng
diff --git a/tests/cfgs/default/result/bfcp.pcapng.out b/tests/cfgs/default/result/bfcp.pcapng.out
index 2061a3590..28575d3e7 100644
--- a/tests/cfgs/default/result/bfcp.pcapng.out
+++ b/tests/cfgs/default/result/bfcp.pcapng.out
@@ -1,14 +1,14 @@
-DPI Packets (TCP):	6	(6.00 pkts/flow)
-DPI Packets (UDP):	2	(2.00 pkts/flow)
-Confidence DPI              : 2 (flows)
-Num dissector calls: 333 (166.50 diss/flow)
+DPI Packets (TCP):	4	(4.00 pkts/flow)
+DPI Packets (UDP):	2	(1.00 pkts/flow)
+Confidence DPI              : 3 (flows)
+Num dissector calls: 417 (139.00 diss/flow)
 LRU cache ookla:      0/0/0 (insert/search/found)
 LRU cache bittorrent: 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 fpc_dns:    0/2/0 (insert/search/found)
+LRU cache fpc_dns:    0/1/0 (insert/search/found)
 Automa host:          0/0 (search/found)
 Automa domain:        0/0 (search/found)
 Automa tls cert:      0/0 (search/found)
@@ -18,12 +18,13 @@ Patricia risk mask:   0/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:   4/0 (search/found)
+Patricia protocols:   6/0 (search/found)
 Patricia protocols IPv6: 0/0 (search/found)
 
-BFCP	32	2224	2
+BFCP	65	4458	3
 
-Acceptable                      32 2224          2            
+Acceptable                      65 4458          3            
 
-	1	TCP 127.0.0.1:58984 <-> 127.0.0.1:5070 [proto: 32/BFCP][IP: 0/Unknown][ClearText][Confidence: DPI][FPC: 0/Unknown, Confidence: Unknown][DPI packets: 6][cat: Video/26][10 pkts/704 bytes <-> 6 pkts/512 bytes][Goodput ratio: 5/21][123.11 sec][bytes ratio: 0.158 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 0/0 12579/31975 92304/92304 30186/42684][Pkt Len c2s/s2c min/avg/max/stddev: 66/66 70/85 78/150 5/29][TCP Fingerprint: 2_64_65495_db1b9381215d/Unknown][Plen Bins: 83,0,16,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,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 192.168.3.134:57020 <-> 192.168.9.100:16500 [proto: 32/BFCP][IP: 0/Unknown][ClearText][Confidence: DPI][FPC: 0/Unknown, Confidence: Unknown][DPI packets: 2][cat: Video/26][7 pkts/470 bytes <-> 9 pkts/538 bytes][Goodput ratio: 34/30][16.68 sec][bytes ratio: -0.067 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 4/0 102/2377 449/16165 174/5631][Pkt Len c2s/s2c min/avg/max/stddev: 60/54 67/60 90/98 10/14][Plen Bins: 87,12,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,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 10.0.200.73:3238 <-> 10.0.102.79:36633 [proto: 32/BFCP][IP: 0/Unknown][ClearText][Confidence: DPI][FPC: 32/BFCP, Confidence: DPI][DPI packets: 1][cat: Video/26][15 pkts/1170 bytes <-> 18 pkts/1064 bytes][Goodput ratio: 46/29][30.22 sec][Conference Id: 1][User Id: 2][bytes ratio: 0.047 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 53/53 2043/1956 3945/5000 1050/1401][Pkt Len c2s/s2c min/avg/max/stddev: 58/54 78/59 94/94 12/12][Plen Bins: 60,39,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,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 127.0.0.1:58984 <-> 127.0.0.1:5070 [proto: 32/BFCP][IP: 0/Unknown][ClearText][Confidence: DPI][FPC: 0/Unknown, Confidence: Unknown][DPI packets: 4][cat: Video/26][10 pkts/704 bytes <-> 6 pkts/512 bytes][Goodput ratio: 5/21][123.11 sec][Conference Id: 1927653396][User Id: 2055][bytes ratio: 0.158 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 0/0 12579/31975 92304/92304 30186/42684][Pkt Len c2s/s2c min/avg/max/stddev: 66/66 70/85 78/150 5/29][TCP Fingerprint: 2_64_65495_db1b9381215d/Unknown][Plen Bins: 83,0,16,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,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.3.134:57020 <-> 192.168.9.100:16500 [proto: 32/BFCP][IP: 0/Unknown][ClearText][Confidence: DPI][FPC: 32/BFCP, Confidence: DPI][DPI packets: 1][cat: Video/26][7 pkts/470 bytes <-> 9 pkts/538 bytes][Goodput ratio: 34/30][16.68 sec][Conference Id: 1927653397][User Id: 2056][bytes ratio: -0.067 (Mixed)][IAT c2s/s2c min/avg/max/stddev: 4/0 102/2377 449/16165 174/5631][Pkt Len c2s/s2c min/avg/max/stddev: 60/54 67/60 90/98 10/14][Plen Bins: 87,12,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]