search

private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
527 stars 156 forks source link

Retry token tied to retry_source_connection_id causes the valid Retry token to be invalid when initial_source_connection_id is used. #1528

Closed QUICTester closed 1 year ago

QUICTester commented 1 year ago

Hi, when testing commit d2f01093, we discovered that Picoquic's Retry token is tied to retry_source_connection_id.

According to RFC 9000, the client must include the received Retry token in all its Initial packets. Therefore, the client may send an Initial packet with the Retry token using initial_source_connection_id. Tying the Retry token with retry_source_connection_id will cause the token verification to fail when initial_source_connection_id is used.

huitema commented 1 year ago

Your tester is wrong. The spec (section 7.3 of RFC 9000) says that the client copies the "source connection ID" or the retry packet into the destination connection ID of the new Initial packet. This is tested in the interop runner, see test "R". See also section 17.2.5.1, "The server includes a connection ID of its choice in the Source Connection ID field. This value MUST NOT be equal to the Destination Connection ID field of the packet sent by the client. A client MUST discard a Retry packet that contains a Source Connection ID field that is identical to the Destination Connection ID field of its Initial packet. The client MUST use the value from the Source Connection ID field of the Retry packet in the Destination Connection ID field of subsequent packets that it sends."

QUICTester commented 1 year ago

To reproduce this behavior 1) Client sends an Initial packet carrying a Crypto frame that contains the Client Hello message. 2) Client receives a Retry packet from the server. 3) Client sends an Initial packet with the Retry token, carrying a Ping frame. 4) Client receives an Initial packet carrying an ACK frame from the server (Retry token verification is successful). 5) Client sends an Initial packet with the Retry token, carrying a Crypto frame that contains the Client Hello message. (Client has now used the initial_source_connection_id as the Destination Connection ID) 6) Client receives an Initial packet carrying a ConnectionClose frame saying the Retry token is invalid (Retry token verification is unsuccessful).

Due to the Retry token tied to the retry_source_connection_id, the retry token verification will only be valid once in each connection, although a valid Retry token is given.

huitema commented 1 year ago

Nope. Step 5 is wrong. The client must not do that. It must set the "Destination CID" at step 3-4-5 to the source CID of the retry packet. The only other acceptable value at step 5 would be the source CID of the ACK frame received at step 4.

QUICTester commented 1 year ago

Yes, exactly. At step 4, the server has changed its Source Connection ID for the ACK frame. Later when the client sends the Crypto frame carrying the Client Hello message with the Source Connection ID received at step 4, the Retry token verification will fail.

initial_source_connection_id = This is the value that the endpoint included in the Source Connection ID field of the first Initial packet it sends for the connection. (which is set at step 4 by the server in the example above).

huitema commented 1 year ago

The code was written specifically to check that the client follows the spec. This is a tricky spec.

QUICTester commented 12 months ago

Hi, Thank you for your response and insights.

Maybe the spec can be clearer in this case? As we are currently working on ensuring the QUIC implementations follow the QUIC specification, testing QUIC implementations, and also hope that our work can make QUIC better for the community.

huitema commented 12 months ago

You should probably take that discussion to the QUIC WG mailing list.