-
Notifications
You must be signed in to change notification settings - Fork 853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't try to print any information about gQUIC packets. #960
base: master
Are you sure you want to change the base?
Conversation
Thanks for the patch. |
The 0x51 is just the beginning of the Google QUIC version numbers. I don't have any documentation on it. |
Looking for information about 0x51, I looked at: |
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. |
To detect only IETF QUIC, the |
It could but it would make that function unnecessarily complicated. The versions you pointed out were used before RFC 9000 was released. |
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 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;
}
|
I think that you can do that. However, by time tcpdump 5 ships, I suspect gQUIC will not be enabled anymore, but who knows. |
Why not. But in this case it seems better to remove the |
Not all QUIC packets carry a version number so that would only work for LH packets. |
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.
|
Okay, your patch works for me. Feel free to commit that. |
Fixes #959.