Messages in this thread | | | From | Zhouyi Zhou <> | Date | Sat, 19 Mar 2022 23:31:30 +0800 | Subject | Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt |
| |
Thank Neil and Eric for your valuable advice!
I will do the test and analysis. Please forgive my hasty reply because it will take me some time to fully understand the email. Also please give me about a month to accomplish the test and analysis. On Sat, Mar 19, 2022 at 9:57 PM Neal Cardwell <ncardwell@google.com> wrote: > > On Sat, Mar 19, 2022 at 7:34 AM Zhouyi Zhou <zhouzhouyi@gmail.com> wrote: > > > > Thanks for reviewing my patch > > > > On Sat, Mar 19, 2022 at 7:14 PM Eric Dumazet <edumazet@google.com> wrote: > > > > > > On Sat, Mar 19, 2022 at 4:04 AM <zhouzhouyi@gmail.com> wrote: > > > > > > > > From: Zhouyi Zhou <zhouzhouyi@gmail.com> > > > > > > > > In RFC 793, page 72: "If the ACK acks something not yet sent > > > > (SEG.ACK > SND.NXT) then send an ACK, drop the segment, > > > > and return." > > > > > > > > Fix Linux's behavior according to RFC 793. > > > > > > > > Reported-by: Wei Xu <xuweihf@ustc.edu.cn> > > > > Signed-off-by: Wei Xu <xuweihf@ustc.edu.cn> > > > > Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com> > > > > --- > > > > Thank Florian Westphal for pointing out > > > > the potential duplicated ack bug in patch version 1. > > > > > > I am travelling this week, but I think your patch is not necessary and > > > might actually be bad. > > > > > > Please provide more details of why nobody complained of this until today. > > > > > > Also I doubt you actually fully tested this patch, sending a V2 30 > > > minutes after V1. > > > > > > If yes, please provide a packetdrill test. > > I am a beginner to TCP, although I have submitted once a patch to > > netdev in 2013 (aaa0c23cb90141309f5076ba5e3bfbd39544b985), this is > > first time I learned packetdrill test. > > I think I should do the packetdrill test in the coming days, and > > provide more details of how this (RFC793 related) can happen. > > In addition to a packetdrill test and a more detailed analysis of how > this can happen, and the implications, I think there are at least a > few other issues that need to be considered: > > (1) AFAICT, adding an unconditional ACK if (after(ack, tp->snd_nxt)) > seems to open the potential for attackers to cause DoS attacks with > something like the following: > > (a) attacker injects one data packet in the A->B direction and one > data packet in the B->A direction > > (b) endpoint A sends an ACK for the forged data sent to it, which > will have an ACK beyond B's snd_nxt > > (c) endpoint B sends an ACK for the forged data sent to it, which > will have an ACK beyond A's snd_nxt > > (d) endpoint B receives the ACK sent by A, causing B to send another > ACK beyond A's snd_nxt > > (e) endpoint A receives the ACK sent by B, causing A to send another > ACK beyond B's snd_nxt > > (f) repeat (d) and (e) ad infinitum I will make a full understanding of the above scenery in the coming days. > > So AFAICT an attacker could send two data packets with 1 byte of data > and cause the two endpoints to use up an unbounded amount of CPU and > bandwidth sending ACKs in an "infinite loop". > > To avoid this "infinite loop" of packets, if we really need to add an > ACK in this case then the code should use the tcp_oow_rate_limited() > helper to ensure that such ACKs are rate-limited. For more context on > tcp_oow_rate_limited(), see: > > f06535c599354 Merge branch 'tcp_ack_loops' > 4fb17a6091674 tcp: mitigate ACK loops for connections as tcp_timewait_sock > f2b2c582e8242 tcp: mitigate ACK loops for connections as tcp_sock > a9b2c06dbef48 tcp: mitigate ACK loops for connections as tcp_request_sock > 032ee4236954e tcp: helpers to mitigate ACK loops by rate-limiting > out-of-window dupacks > > Note that f06535c599354 in particular mentions the case discussed in this patch: > > (2) RFC 793 (section 3.9, page 72) says: "If the ACK acknowledges > something not yet sent (SEG.ACK > SND.NXT) then send an ACK". > > (2) Please consider the potential that adding a new ACK in this > scenario may introduce new, unanticipated side channels. For more on > side channels, see: > > https://lwn.net/Articles/696868/ > The TCP "challenge ACK" side channel I will read the article in the days following. > > Principled Unearthing of TCP Side Channel Vulnerabilities > https://dl.acm.org/doi/10.1145/3319535.3354250 I will read the paper too. > > best regards, > neal Best Regards Zhouyi
| |