Don't try to print any information about gQUIC packets.

[rpaulo]

Fixes #959.

[fxlb]

Thanks for the patch.
Have you documentation about "magic" value 0x51?
Is a test is needed for the "Short Header" case?

[rpaulo]

The 0x51 is just the beginning of the Google QUIC version numbers. I don't have any documentation on it.
Short headers have no version number, so nothing is needed there.

[fxlb]

Looking for information about 0x51, I looked at:
https://datatracker.ietf.org/doc/html/draft-ietf-quic-transport-32
And found in it the link:
https://github.com/quicwg/base-drafts/wiki/QUIC-Versions
0x51 is not the only non-IETF QUIC version value...

[rpaulo]

I was told that 0x51 will be the last version before the transition to IETF QUIC is finalized. Regarding the other versions, we should let the authors of them add support to tcpdump if it’s important to them.

[fxlb]

To detect only IETF QUIC, the quic_detect function could be enhanced to check, in the long header case, than the version is 0, 1 or 0x?a?a?a?a.

[rpaulo]

It could but it would make that function unnecessarily complicated. The versions you pointed out were used before RFC 9000 was released.

[fxlb]

Yes, but tcpdump needs to behave correctly with any input (gquic, pre-rfc9000, experiments, etc.). And with a fuzzed/corrupted capture file the version may be any random value...
Why not a quic_detect() update like (or better idea?):

diff --git a/print-quic.c b/print-quic.c
index be1a5450..14069638 100644
--- a/print-quic.c
+++ b/print-quic.c
@@ -114,9 +114,23 @@ quic_detect(netdissect_options *ndo, const u_char *p, const u_int len)
 		return 0;
 	first_octet = GET_U_1(p);
 	/* All QUIC packets must have the Fixed Bit set to 1. */
-	if ((first_octet & 0x40) == 0x40)
-		return 1;
-	else
+	if ((first_octet & 0x40) == 0x40) {
+		if (first_octet & 0x80) {
+			/* Long Header (Header Form set to 1) */
+			uint32_t version;
+			if (len < 6)
+				return 0;
+			p++;
+			version = GET_BE_U_4(p);
+			if (version == 0 || version == 1 ||
+			    (version & 0x0f0f0f0f) == 0x0a0a0a0a)
+				return 1;
+			else
+				return 0;
+		} else
+			/* Short Header */
+			return 1;
+	} else
 		return 0;
 }
 

[rpaulo]

I think that you can do that. However, by time tcpdump 5 ships, I suspect gQUIC will not be enabled anymore, but who knows.
I do think that quic_detect() should be as simple as possible. If we were to add full support for proprietary protocols like gQUIC then it would have to be different, but right now, I don't see much benefit in spending time on anything else but IETF QUIC.

[fxlb]

I do think that quic_detect() should be as simple as possible.

Why not. But in this case it seems better to remove the 0x51 version test (particular case) and test version like:
version == 0 || version == 1 || (version & 0x0f0f0f0f) == 0x0a0a0a0a)
All other versions should give : "not RFC9000 packet"

[rpaulo]

Not all QUIC packets carry a version number so that would only work for LH packets.

[fxlb]

On 12/01/2022 17:11, Rui Paulo wrote: Not all QUIC packets carry a version number so that would only work for LH packets.

Exactly. This is what I mean.

[rpaulo]

Okay, your patch works for me. Feel free to commit that.