Skip to content
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

Parse coalesced packets after keys of contained packet are discarded #1834

Open
jmuecke opened this issue Aug 20, 2024 · 2 comments · May be fixed by #1922
Open

Parse coalesced packets after keys of contained packet are discarded #1834

jmuecke opened this issue Aug 20, 2024 · 2 comments · May be fixed by #1922

Comments

@jmuecke
Copy link

jmuecke commented Aug 20, 2024

I used quiche as QUIC client to contact a quic-go server.
The quic-go server sends an instant acknowledgment to the ClientHello, but then is slow to follow up the handshake and holds a large certificate.

Scenario:
The client correctly uses the estimated RTT to send probe packets in the form of Initials with PING frames.
Once the server continues to reply with the first batch of the Handshake packets (PKN: 0 and 1; Wireshark no 10, 11) after a long delay, these are acknowledged by quiche.
However, subsequent Handshake packets (PKN 2 and 3 of the server; Wireshark no 12, 14), are dropped by quiche.

Client log:

[2024-07-26T14:53:54.712689625Z TRACE quiche] 3f2d2ea78cab71732a6858b967c0293e3944b8dd dropped epoch 0 state
[2024-07-26T14:53:54.712960352Z TRACE quiche] 3f2d2ea78cab71732a6858b967c0293e3944b8dd dropped invalid packet
[2024-07-26T14:53:54.712977024Z TRACE quiche_apps::client] 0.0.0.0:51097: processed 1252 bytes
[2024-07-26T14:53:54.712993775Z TRACE quiche_apps::client] 0.0.0.0:51097: recv() would block
[2024-07-26T14:53:54.713008192Z TRACE quiche_apps::client] done reading
[2024-07-26T14:53:54.713065550Z TRACE quiche_apps::client] 0.0.0.0:51097 -> 193.167.100.100:443: done writing
[2024-07-26T14:53:54.713130532Z TRACE quiche_apps::client] 0.0.0.0:51097: got 1252 bytes
[2024-07-26T14:53:54.713158474Z TRACE quiche] 3f2d2ea78cab71732a6858b967c0293e3944b8dd dropped invalid packet
[2024-07-26T14:53:54.713174544Z TRACE quiche_apps::client] 0.0.0.0:51097: processed 1252 bytes
[2024-07-26T14:53:54.713191005Z TRACE quiche_apps::client] 0.0.0.0:51097: recv() would block
[2024-07-26T14:53:54.713205362Z TRACE quiche_apps::client] done reading
[2024-07-26T14:53:54.713232112Z TRACE quiche_apps::client] 0.0.0.0:51097 -> 193.167.100.100:443: done writing
[2024-07-26T14:53:54.714669187Z TRACE quiche_apps::client] 0.0.0.0:51097: got 1252 bytes

I believe the reason for dropping packets 12 and 14 is that they contain Initial ACKs, for which the keys have already been discarded (see client-server log: dropped epoch state).
quiche seems to drop the entire UDP datagram, which might contain additional QUIC packets.
Dropping the information requires an additional round trip, to detect and re-transmit the data (Wireshark no. 18, 20).

This can also be observed in Qlog. Here, the Handshake packets (PKN: 2 and 3) are missing.

Expected behavior:
Do not drop entire datagrams.
Since the header protection does not cover the long packet type and length fields, it should be possible to skip the contained Initial, but parse coalesced packets as long as the keys are available, which would render the client more efficient.

What do you think?

Attached is the pcap, qlog and client output.
quiche-coalesced-packets.pcapng.gz
client-3f2d2ea78cab71732a6858b967c0293e3944b8dd.qlog.gz
client-log.txt

ghedo added a commit that referenced this issue Jan 23, 2025
When datagram with Initial+Handshake coalesced packets is received after
the Initial epoch has already been dropped, we would drop the entire
datagram, causing the peer to resend the Handshake packet.

Instead we should just skip over the Initial packet and continue
processing the datagram.

Closes #1834.
ghedo added a commit that referenced this issue Jan 23, 2025
When datagram with Initial+Handshake coalesced packets is received after
the Initial epoch has already been dropped, we would drop the entire
datagram, causing the peer to resend the Handshake packet.

Instead we should just skip over the Initial packet and continue
processing the datagram.

Closes #1834.
@ghedo
Copy link
Member

ghedo commented Jan 23, 2025

Sorry for the delay. I do agree this is an issue, and I made #1922 which I think should fix this. However I wasn't able to reproduce the issue in a test case yet.

@jmuecke
Copy link
Author

jmuecke commented Jan 24, 2025

Thanks! I think #1922 does not fully solve this.

Coalesced packets with changing/discarded keys can happen for any combination of coalesced packets.
This can happen with further with 0-RTT packets, which is handled in recv_single.

The remaining case would be a Handshake packet coalesced with a 1-RTT/short header packet. This is currently not handled by the change in #1922 and does not seem to be handled anywhere else in recv_single.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@ghedo @jmuecke and others