quicwg / base-drafts

Internet-Drafts that make up the base QUIC specification
https://quicwg.org
1.63k stars 204 forks source link

missing description of what happens when receiving an ACK for an unsent packet #2298

Closed marten-seemann closed 5 years ago

marten-seemann commented 5 years ago

I can't find text about this case anywhere.

A peer that receives an acknowledgement for a packet that it didn't send should be required to close the connection. I'm not sure which error code to use for that though, none of those currently defined really matches this case, so you'd have to use INTERNAL_ERROR. We might want to consider introducing a more specific error code.

kazuho commented 5 years ago

I'm not sure which error code to use for that though, none of those currently defined really matches this case, so you'd have to use INTERNAL_ERROR.

I think you are expected to use PROTOCOL_VIOLATION in such case. Quoting from editors' draft: An endpoint detected an error with protocol compliance that was not covered by more specific error codes.

A peer that receives an acknowledgement for a packet that it didn't send should be required to close the connection.

FWIW, I think that the requirement should be no stronger than "SHOULD if detected", to avoid endpoints memoizing the gaps for the previous 231 range. I assume that endpoints would typically just drop packets carrying a PN that is deemed too old.

mikkelfj commented 5 years ago

This is always considered to be an Optimistic ACK attack. The only response is to treat the peer as hostile.

https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#rfc.section.21.3

mikkelfj commented 5 years ago

@kazuho

I assume that endpoints would typically just drop packets carrying a PN that is deemed too old.

If an endpoint cannot prove it didn't send the packet, it should just ignore the ACK for that particular packet. The congestion controller should not be sensitive to packets that old, so the Optimistic ACK attack is harmless in this case. Dropping the entire packet containing an ACK with an old packet is likely too strong - ignoring the ACK for the packet is sufficient. If a peer continually sends ACK's for very old packets, that might go under weird behaviour similar to sending too many gaps, and an implementation could decide the peer is being too confused and close the connection with an error.

mikkelfj commented 5 years ago

FYI: the reason this always needs to be treated as hostile (when provable) is because it is the only defence against the potentially very harmful optimistic ack attack. There are very few other other reasons to have gaps in the range, such as when having multiple concurrent transmit queues.