diff --git a/CHANGES b/CHANGES index 1354d35375..a88eb654ed 100644 --- a/CHANGES +++ b/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) diff --git a/NEWS b/NEWS index 406073dcf5..af792aed78 100644 --- a/NEWS +++ b/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 --------------------- diff --git a/VERSION b/VERSION index 482a210378..50a581e667 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.1.0-dev.639 +8.1.0-dev.642 diff --git a/src/analyzer/protocol/quic/QUIC.spicy b/src/analyzer/protocol/quic/QUIC.spicy index a071d1ea19..835cfb342c 100644 --- a/src/analyzer/protocol/quic/QUIC.spicy +++ b/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(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); }; ############## diff --git a/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/conn.log.cut b/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/conn.log.cut new file mode 100644 index 0000000000..46d72b1541 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/quic.log b/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/quic.log new file mode 100644 index 0000000000..10648c9e82 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/ssl.log b/testing/btest/Baseline/scripts.base.protocols.quic.39264-rand/ssl.log new file mode 100644 index 0000000000..b3df132751 --- /dev/null +++ b/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 diff --git a/testing/btest/Traces/quic/quic-39264-rand.pcap b/testing/btest/Traces/quic/quic-39264-rand.pcap new file mode 100644 index 0000000000..0a639199a2 Binary files /dev/null and b/testing/btest/Traces/quic/quic-39264-rand.pcap differ diff --git a/testing/btest/scripts/base/protocols/quic/39264-rand.zeek b/testing/btest/scripts/base/protocols/quic/39264-rand.zeek new file mode 100644 index 0000000000..7c0258ea0f --- /dev/null +++ b/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