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

Fix not decrypting server's INITIAL packets #18

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

awelzel
Copy link

@awelzel awelzel commented Jan 26, 2024

  • Don't stop processing INITIAL packets after the first one, it might have just contained ACK frames.
  • Rename set_conn() to set_session()
  • Also log client_scid

See commits for a bit more information. I wanted to use quic.log as an example and these all came up :-)

set_conn() should be about the c$conn record. Most other base scripts
for protocols use set_session(), so do the same.
The original logic stopped decrypting any INITIAL packets after the
first. The Firefox/cloudflare pcaps actually show that the server
replies with a QUIC INITAL packet containing just ACK frames and no
CRYPTO frames. Only the second QUIC INITIAL packet from the server
then contains the CRYPTO frames.

There's no good reason to stop decryption attempts, either we succeed
down the road and then stop, or we fail and raise analyzer violations.
@awelzel awelzel force-pushed the topic/awelzel/fix-server-initial-with-ack branch from 0c34efc to 67126dc Compare January 26, 2024 19:09
Seem reasonable give we log the server SCID. Interestingly, the Chromium
examples actually have zero length (empty) source connection IDs. I wonder
if that's part of their "protocol ossification avoidance" effort.
@awelzel awelzel force-pushed the topic/awelzel/fix-server-initial-with-ack branch from 67126dc to d5c3151 Compare January 26, 2024 19:20
@awelzel awelzel requested a review from bbannier January 29, 2024 09:26
@awelzel awelzel merged commit f131eb1 into main Jan 30, 2024
3 checks passed
@awelzel awelzel deleted the topic/awelzel/fix-server-initial-with-ack branch January 30, 2024 20:28
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 this pull request may close these issues.

1 participant