Merge remote-tracking branch 'origin/topic/awelzel/4847-quic-discard-fixed-bit-is-zero'

* origin/topic/awelzel/4847-quic-discard-fixed-bit-is-zero:
  QUIC: Skip packets with fixed_bit 0

CHANGES

@@ -1,3 +1,10 @@
+8.1.0-dev.642 | 2025-10-09 09:16:33 +0200
+
+  * GH-4847: QUIC: Skip packets with fixed_bit 0 (Arne Welzel, Corelight)
+
+    The RFC specifies that QUIC packets with unset fixed_bit need to be
+    discarded. Do so.
+
 8.1.0-dev.639 | 2025-10-08 09:21:05 +0200
 
   * logging/writers: Disable heartbeats (Arne Welzel, Corelight)

NEWS

@@ -73,6 +73,9 @@ Changed Functionality
 - The ``get_current_packet_header()`` now populates the returned record also for
   fragmented IP datagrams.
 
+- The QUIC parser discards packets with the fixed_bit field set to 0, rather than
+  continuing to parse potentially running into analyzer violations.
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-8.1.0-dev.639
+8.1.0-dev.642

src/analyzer/protocol/quic/QUIC.spicy

@@ -516,6 +516,16 @@ type Packet = unit(from_client: bool, context: Context&) {
     # Peek into the first byte and determine the header type.
     first_byte: bitfield(8) {
         header_form: 7 &convert=HeaderForm($$);
+
+        # The next bit (0x40) of byte 0 is set to 1. Packets containing a zero
+        # value for this bit are not valid packets in this version and MUST be
+        # discarded. A value of 1 for this bit allows QUIC to coexist with other
+        # protocols; see [RFC7983].
+        #
+        # https://datatracker.ietf.org/doc/html/rfc9000#name-short-header-packets
+        #
+        # We use this to avoid parsing the short/long header below.
+        fixed_bit: 6;
     };
 
     # TODO: Consider bitfield based look-ahead-parsing in the switch below
@@ -525,84 +535,94 @@ type Packet = unit(from_client: bool, context: Context&) {
     }
 
     # Depending on the header, parse it and update the src/dest ConnectionID's
-    switch (self.first_byte.header_form) {
-        HeaderForm::SHORT -> short_header: ShortHeader(context.client_cid_len);
-        HeaderForm::LONG -> long_header: LongHeaderPacket {
-            # For now, only allow a change of src/dest ConnectionID's for INITIAL packets.
+    switch (cast<bool>(self.first_byte.fixed_bit)) {
+        True -> {
+            switch (self.first_byte.header_form) {
+                HeaderForm::SHORT -> short_header: ShortHeader(context.client_cid_len);
+                HeaderForm::LONG -> long_header: LongHeaderPacket {
+                    # For now, only allow a change of src/dest ConnectionID's for INITIAL packets.
 
-            # If we see a retry packet from the responder, reset the decryption
-            # context such that the next DCID from the client is used for decryption.
-            if (self.long_header.is_retry) {
-                reset_crypto(context);
+                    # If we see a retry packet from the responder, reset the decryption
+                    # context such that the next DCID from the client is used for decryption.
+                    if (self.long_header.is_retry) {
+                        reset_crypto(context);
 
-                self.crypto = Null;
-                self.crypto_sink = Null;
+                        self.crypto = Null;
+                        self.crypto_sink = Null;
+                    }
+                }
+            };
+
+            : void {
+                if (self?.long_header && can_decrypt(self.long_header, context, self.crypto))
+                # If we have parsed an initial packet that we can decrypt the payload,
+                # determine the size to store into a buffer.
+                self.packet_size = self.offset();
             }
+
+            # Buffer the whole packet if we determined we have a chance to decrypt.
+            packet_data: bytes &parse-at=self.start &size=self.packet_size if(self.packet_size > 0) {
+
+                if (from_client) {
+                    context.server_cid_len = self.long_header.dest_conn_id_len;
+                    context.client_cid_len = self.long_header.src_conn_id_len;
+
+                    # This is the first INITIAL packet we attempt to decrypt and it is
+                    # coming from the client. Use its destination connection ID for
+                    # decryption purposes.
+                    if (!context.initial_destination_conn_id) {
+                        context.initial_destination_conn_id = self.long_header.dest_conn_id;
+                    }
+
+                    # This means that here, we can try to decrypt the initial packet!
+                    # All data is accessible via the `long_header` unit
+                    self.decrypted_data = decrypt_crypto_payload(
+                        self.long_header.version,
+                        self.packet_data,
+                        *context.initial_destination_conn_id,
+                        self.long_header.encrypted_offset,
+                        self.long_header.payload_length,
+                        from_client
+                    );
+                } else {
+                    context.server_cid_len = self.long_header.src_conn_id_len;
+                    context.client_cid_len = self.long_header.dest_conn_id_len;
+
+                    self.decrypted_data = decrypt_crypto_payload(
+                        self.long_header.version,
+                        self.packet_data,
+                        *context.initial_destination_conn_id,
+                        self.long_header.encrypted_offset,
+                        self.long_header.payload_length,
+                        from_client
+                    );
+                }
+
+                # We attempted decryption, but it failed. Just reject the
+                # input and assume Zeek will disable the analyzer for this
+                # connection.
+                if (|self.decrypted_data| == 0)
+                    throw "decryption failed";
+
+                # We were able to decrypt the INITIAL packet. Confirm QUIC!
+                spicy::accept_input();
+            }
+
+            # If this packet has a SHORT header, consume until &eod, there's nothing
+            # we can do with it anyhow, but only if fixed_bit == 1.
+            : ShortPacketPayload if(self.first_byte.header_form == HeaderForm::SHORT);
+
+            # If this was packet with a long header and decrypted data exists, attempt
+            # to parse the plain QUIC frames from it.
+            frames: Frame(self.long_header, from_client, self.crypto, self.crypto_sink)[] &parse-from=self.decrypted_data if(self.first_byte.header_form == HeaderForm::LONG && |self.decrypted_data| > 0);
+        }
+        False -> {
+            # Consume the packet if fixed_bit is not 1. Basically discard it.
+            #
+            # # TODO: Raise QUIC::discarded_packet() when this happens.
+            : skip bytes &eod;
         }
     };
-
-    : void {
-        if (self?.long_header && can_decrypt(self.long_header, context, self.crypto))
-        # If we have parsed an initial packet that we can decrypt the payload,
-        # determine the size to store into a buffer.
-        self.packet_size = self.offset();
-    }
-
-    # Buffer the whole packet if we determined we have a chance to decrypt.
-    packet_data: bytes &parse-at=self.start &size=self.packet_size if(self.packet_size > 0) {
-
-        if (from_client) {
-            context.server_cid_len = self.long_header.dest_conn_id_len;
-            context.client_cid_len = self.long_header.src_conn_id_len;
-
-            # This is the first INITIAL packet we attempt to decrypt and it is
-            # coming from the client. Use its destination connection ID for
-            # decryption purposes.
-            if (!context.initial_destination_conn_id) {
-                context.initial_destination_conn_id = self.long_header.dest_conn_id;
-            }
-
-            # This means that here, we can try to decrypt the initial packet!
-            # All data is accessible via the `long_header` unit
-            self.decrypted_data = decrypt_crypto_payload(
-                self.long_header.version,
-                self.packet_data,
-                *context.initial_destination_conn_id,
-                self.long_header.encrypted_offset,
-                self.long_header.payload_length,
-                from_client
-            );
-        } else {
-            context.server_cid_len = self.long_header.src_conn_id_len;
-            context.client_cid_len = self.long_header.dest_conn_id_len;
-
-            self.decrypted_data = decrypt_crypto_payload(
-                self.long_header.version,
-                self.packet_data,
-                *context.initial_destination_conn_id,
-                self.long_header.encrypted_offset,
-                self.long_header.payload_length,
-                from_client
-            );
-        }
-
-        # We attempted decryption, but it failed. Just reject the
-        # input and assume Zeek will disable the analyzer for this
-        # connection.
-        if (|self.decrypted_data| == 0)
-            throw "decryption failed";
-
-        # We were able to decrypt the INITIAL packet. Confirm QUIC!
-        spicy::accept_input();
-    }
-
-    # If this packet has a SHORT header, consume until &eod, there's nothing
-    # we can do with it anyhow.
-    : ShortPacketPayload if(self.first_byte.header_form == HeaderForm::SHORT);
-
-    # If this was packet with a long header and decrypted data exists, attempt
-    # to parse the plain QUIC frames from it.
-    frames: Frame(self.long_header, from_client, self.crypto, self.crypto_sink)[] &parse-from=self.decrypted_data if(self.first_byte.header_form == HeaderForm::LONG && |self.decrypted_data| > 0);
 };
 
 ##############

testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/conn.log.cut

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	history	service
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	Dd	quic,ssl

testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/quic.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	quic
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	client_initial_dcid	client_scid	server_scid	server_name	client_protocol	history
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	765b:b77b:dad1:7e73:56f9:5a0f:52a3:f4f	39264	725f:1f71:525f:a3b:525f:3673:525f:3673	443	1	ceef7990f5bb4071	1e6dc7	eeef7990f5bb4071	tr6.snapchat.com	h3	IIISZiiiiishIH
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/ssl.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ssl
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	cipher	curve	server_name	resumed	last_alert	next_protocol	established	ssl_history	cert_chain_fps	client_cert_chain_fps	sni_matches_cert
+#types	time	string	addr	port	addr	port	string	string	string	string	bool	string	string	bool	string	vector[string]	vector[string]	bool
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	765b:b77b:dad1:7e73:56f9:5a0f:52a3:f4f	39264	725f:1f71:525f:a3b:525f:3673:525f:3673	443	TLSv13	TLS_AES_128_GCM_SHA256	x25519	tr6.snapchat.com	T	-	-	F	Cs	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/quic/39264-rand.zeek

@@ -0,0 +1,9 @@
+# @TEST-DOC: Regression test for #4847, QUIC packets with fixed_bit 0 are discarded.
+
+# @TEST-REQUIRES: ${SCRIPTS}/have-spicy
+# @TEST-EXEC: zeek -r $TRACES/quic/quic-39264-rand.pcap base/protocols/quic
+# @TEST-EXEC: test ! -f analyzer.log || cat analyzer.log >&2
+# @TEST-EXEC: zeek-cut -m ts uid history service < conn.log > conn.log.cut
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: btest-diff ssl.log
+# @TEST-EXEC: btest-diff quic.log