private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
561 stars 165 forks source link

QUICv2: update to latest VN draft #1422

Closed IvanNardi closed 1 year ago

IvanNardi commented 1 year ago

See: https://datatracker.ietf.org/doc/html/draft-ietf-quic-version-negotiation-14

IvanNardi commented 1 year ago

Looks good. Are we sure that this is the last update?

Sorry for the delay. It seems likely, but I am not an expert of the IETF processes... To be cautions, do you prefer I rename the PR to something like "QUICv2: update to VN draft-14"?

huitema commented 1 year ago

According to the IETF Datatracker the draft draft-ietf-quic-v2-10.txt is "in the RFC Editor Queue", which means it is approved for publication and can only undergo very minor changes, like fixing typos. I checked the deltas. My only concern is the picoquic_retry_protection_v2 variable. This one changed in section 3.3.3 to:

   The key and nonce used for the Retry Integrity Tag (Section 5.8 of
   [QUIC-TLS]) change to:

   secret =
     0xc4dd2484d681aefa4ff4d69c2c20299984a765a5d3c31982f38fc74162155e9f
   key = 0x8fb4b01b56ac48e260fbcbcead7ccc92
   nonce = 0xd86969bc2d7c6d9990efb04a

   The secret is the sha256sum of "QUICv2 retry secret".  The key and
   nonce are derived from this secret with the labels "quicv2 key" and
   "quicv2 iv", respectively.

I think that the code still has the old version:

0x3425c20cf88779df2ff71e8abfa78249891e763bbed2f13c048343d348c060e2 0xc4dd2484d681aefa4ff4d69c2c20299984a765a5d3c31982f38fc74162155e9f

I think we only need to replace the old string by the new one, but I will ask Martin if there is a test vector available.

Correction: There are test vectors in the appendix of the V2 draft. Might be a good idea to add the tests.

huitema commented 1 year ago

Checking in as is, because at least the PR enables proper connection setup with V2. I will develop another PR for fixing and testing the retry protection.