quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.57k stars 364 forks source link

Maintain in-flight counters separately per path #1777

Closed Ralith closed 3 months ago

Ralith commented 3 months ago

Fixes #1775, theoretically. @nemethf can you confirm?

RFC9000 §9.4:

The capacity available on the new path might not be the same as the old path. Packets sent on the old path MUST NOT contribute to congestion control or RTT estimation for the new path.

We accomplish this by tracking the first packet number on each path, which allows the logic that removes acked/lost packets from congestion control counters to determine which, if any, known path a packet was sent on.

nemethf commented 3 months ago

Fixes #1775, theoretically. @nemethf can you confirm?

Yes, it does solve my original issue and the traffic quickly migrates to the new path. Thank you.


Looking at the captured traffic, I was a bit surprised:

 No. -     Time -             Source -           Dest -             Proto -        Length -         Info -
 278       2.897193           10.0.2.1           10.0.3.1           QUIC           78               Protected Payload (KP0), DCID=7abe9d2420aaddb1, PKN: 101, PING, PADDING
 279       2.907467           10.0.3.1           10.0.1.1           QUIC           1248             Protected Payload (KP0), DCID=033dda038eeebb0b, PKN: 219, PC, PADDING
 280       2.907472           10.0.3.1           10.0.2.1           HTTP3          1248             Protected Payload (KP0), DCID=033dda038eeebb0b, PKN: 220, ACK_ECN, PC, RC, STREAM(3)

In other words the server sends payload alongside with the PATH_CHALLENGE. I did not know this was possible, but RFC9000 permits it: " An endpoint MAY send data to an unvalidated peer address, but it MUST protect against potential attacks as described in Sections 9.3.1 and 9.3.2." So it's all good. Thanks again.

Ralith commented 3 months ago

Yep, that's the intended behavior. Separate logic applies an anti-amplification budget to all unvalidated paths to prevent this from being too much of an amplification hazard if someone attempts the rather esoteric off-path packet forwarding attack. Thanks for confirming!