search

quic-go / quic-go

A QUIC implementation in pure Go
https://quic-go.net
MIT License
10.13k stars 1.32k forks source link

fix DOS against SentPacketHandler when ACKing too many packets #246

Closed marten-seemann closed 8 years ago

marten-seemann commented 8 years ago

An attacker could send ACK frames and never raise the LowestAcked value. Then we have to iterate over all packets between LowestAcked and LargestAcked in SentPacketHandler.ReceivedAcked. This attack would only be limited (after implementing #186) when the number of randomly skipped packets gets larger than 255 (the maximum number of AckRanges in an ACK), and since we might skip one packet out of 1000, this is not an effective measure to mitigate this attack.

Ignoring all ACKed packets below the SentPacketHandler.LargestInOrderAcked might be a good idea for performance reasons in the case of a non-malicious client, but probably is not a suitable DOS mitigation, since by not ACKing a packet with low packet number (e.g. packet 1), an attacker could prevent this value from increasing (fast enough).

What I'd suggest is to introduce a maximum value for AckFrame.LargestAcked - AckFrame.LowestAcked, around O(5000) or so.

lucas-clemente commented 8 years ago

FFR: We decided to change the SentPacketHandler to keep all sent packets in a list and limit their number to ~500. This way we have ca (500+255) / (255 * 2 + 20) = 1.4 operations per byte of data, instead of the 5000 / (255*2+20) = 9.4 of the above proposal.