Incorrect conditions in BFCP dissector

Summary

Reproduced for any versions.

In https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-bfcp.c at string 397 is checking the validity of the first byte.

If firs byte is not 0x20 or 0x30, packet is not BFCP.

But, in https://datatracker.ietf.org/doc/rfc8855/ instead of https://datatracker.ietf.org/doc/html/rfc4582#section-5-1, bits 3 and 4 can be "true"

5.1.  COMMON-HEADER Format

   The following is the format of the COMMON-HEADER.

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Ver |R|F| Res |  Primitive    |        Payload Length         |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                         Conference ID                         |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |         Transaction ID        |            User ID            |
   +> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |  | Fragment Offset (if F is set) | Fragment Length (if F is set) |
   +> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |
   +---- These fragment fields are never present
         when using reliable transports

                       Figure 5: COMMON-HEADER format

   Ver:  This 3-bit field defines the version of BFCP to which this
      message adheres.  This specification defines two versions: 1 and
      2.  The version field MUST be set to 1 when using BFCP over a
      reliable transport.  The version field MUST be set to 2 when using
      BFCP over an unreliable transport.  If a floor control server
      receives a message with an unsupported version field value or a
      message with a version number that is not permitted with the
      transport over which it was received, the server MUST indicate it
      does not support the protocol version by sending an Error message
      with parameter value 12 (Unsupported Version).  Note that BFCP
      entities supporting only the [3] subset will not support this
      parameter value.

   R:  The Transaction Responder (R) flag bit has relevance only for use
      of BFCP over an unreliable transport.  When cleared, it indicates
      that this message is a request initiating a new transaction, and
      the Transaction ID that follows has been generated for this
      transaction.  When set, it indicates that this message is a
      response to a previous request, and the Transaction ID that
      follows is the one associated with that request.  When BFCP is
      used over a reliable transport, the flag has no significance and
      MUST be cleared by the sender and MUST be ignored by the receiver.

   F:  The Fragmentation (F) flag bit has relevance only for use of BFCP
      over an unreliable transport.  When cleared, the message is not
      fragmented.  When set, it indicates that the message is a fragment
      of a large, fragmented BFCP message.  (The optional fields
      Fragment Offset and Fragment Length described below are present
      only if the F flag is set).  When BFCP is used over a reliable
      transport, the flag has no significance and MUST be cleared by the
      sender, and the flag MUST be ignored by the receiver.  In the
      latter case, the receiver should also ignore the Fragment Offset
      and Fragment Length fields when processing the COMMON-HEADER.

Therefore, the valid values are now:

• 0x20 -- Ver 1
• 0x40 -- Ver 2, R and F false
• 0x48 -- Ver 2, R false, F true
• 0x50 -- Ver 2, R true, F false
• 0x58 -- Ver 2, R true, F true

but in comment for this code says:

	/* If first_byte of bfcp_packet is a combination of the
	 * version and the I bit. The value must be either 0x20 or 0x30
	 * if the bit is set, otherwise it is not BFCP.
	 */

I don't understand, what is the "I bit". In the old and new implementation, the first byte has only the version bits, R, F, and is reserved.

Its reproduced for any versions.

Steps to reproduce

1. Open attached pcap
2. Filter by BFCP
3. Make sure BFCP is only defined for header 0x20

What is the current bug behavior?

BFCP defined for header first byte:

• 0x20 -- Ver 1
• 0x30 -- incorrect

What is the expected correct behavior?

BFCP defined for header first byte:

• 0x20 -- Ver 1
• 0x40 -- Ver 2, R and F false
• 0x48 -- Ver 2, R false, F true
• 0x50 -- Ver 2, R true, F false
• 0x58 -- Ver 2, R true, F true

Sample capture file

smal.pcapng

Pcap with bits for test

Build information

Version 4.1.0rc0-894-g927ea482ba9c (v4.1.0rc0-894-g927ea482ba9c).

Compiled (64-bit) using Microsoft Visual Studio 2022 (VC++ 14.32, build 31332),
with GLib 2.72.3, with PCRE2, with zlib 1.2.12, with Qt 6.2.3, with libpcap,
with Lua 5.2.4, with GnuTLS 3.6.3 and PKCS #11 support, with Gcrypt 1.10.1, with
Kerberos (MIT), with MaxMind, with nghttp2 1.49.0, with brotli, with LZ4, with
Zstandard, with Snappy, with libxml2 2.9.14, with libsmi 0.4.8, with
QtMultimedia, with automatic updates using WinSparkle 0.5.7, with AirPcap, with
SpeexDSP (using bundled resampler), with Minizip, with binary plugins, with
UTF-8 validation.

Running on 64-bit Windows 10 (22H2), build 19045, with AMD Ryzen 5 5600H with
Radeon Graphics          (with SSE4.2), with 15715 MB of physical memory, with
GLib 2.72.3, with PCRE2 10.40 2022-04-14, with Qt 6.2.3, with Npcap version
1.71, based on libpcap version 1.10.2-PRE-GIT, with c-ares 1.18.1, with GnuTLS
3.6.3, with Gcrypt 1.10.1, with nghttp2 1.49.0, with brotli 1.0.9, with LZ4
1.9.3, with Zstandard 1.5.2, without AirPcap, with light display mode, without
HiDPI, with LC_TYPE=Russian_Russia.utf8, binary plugins supported.