[PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[zhouzhouyi]

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.
--
 net/ipv4/tcp_input.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bfe4112e000c..4bbf85d7ea8c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3771,11 +3771,13 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		goto old_ack;
 	}
 
-	/* If the ack includes data we haven't sent yet, discard
-	 * this segment (RFC793 Section 3.9).
+	/* If the ack includes data we haven't sent yet, then send
+	 * an ack, drop this segment, and return (RFC793 Section 3.9 page 72).
 	 */
-	if (after(ack, tp->snd_nxt))
-		return -1;
+	if (after(ack, tp->snd_nxt)) {
+		tcp_send_ack(sk);
+		return -2;
+	}
 
 	if (after(ack, prior_snd_una)) {
 		flag |= FLAG_SND_UNA_ADVANCED;
@@ -6385,6 +6387,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 	struct request_sock *req;
 	int queued = 0;
 	bool acceptable;
+	int ret;
 
 	switch (sk->sk_state) {
 	case TCP_CLOSE:
@@ -6451,14 +6454,16 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 		return 0;
 
 	/* step 5: check the ACK field */
-	acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
-				      FLAG_UPDATE_TS_RECENT |
-				      FLAG_NO_CHALLENGE_ACK) > 0;
+	ret = tcp_ack(sk, skb, FLAG_SLOWPATH |
+				FLAG_UPDATE_TS_RECENT |
+				FLAG_NO_CHALLENGE_ACK);
+	acceptable = ret > 0;
 
 	if (!acceptable) {
 		if (sk->sk_state == TCP_SYN_RECV)
 			return 1;	/* send one RST */
-		tcp_send_challenge_ack(sk);
+		if (ret > -2)
+			tcp_send_challenge_ack(sk);
 		goto discard;
 	}
 	switch (sk->sk_state) {
-- 
2.25.1

Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[Eric Dumazet]

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.

Thank you.

> --
>  net/ipv4/tcp_input.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index bfe4112e000c..4bbf85d7ea8c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3771,11 +3771,13 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>                 goto old_ack;
>         }
>
> -       /* If the ack includes data we haven't sent yet, discard
> -        * this segment (RFC793 Section 3.9).
> +       /* If the ack includes data we haven't sent yet, then send
> +        * an ack, drop this segment, and return (RFC793 Section 3.9 page 72).
>          */
> -       if (after(ack, tp->snd_nxt))
> -               return -1;
> +       if (after(ack, tp->snd_nxt)) {
> +               tcp_send_ack(sk);
> +               return -2;
> +       }
>
>         if (after(ack, prior_snd_una)) {
>                 flag |= FLAG_SND_UNA_ADVANCED;
> @@ -6385,6 +6387,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>         struct request_sock *req;
>         int queued = 0;
>         bool acceptable;
> +       int ret;
>
>         switch (sk->sk_state) {
>         case TCP_CLOSE:
> @@ -6451,14 +6454,16 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>                 return 0;
>
>         /* step 5: check the ACK field */
> -       acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
> -                                     FLAG_UPDATE_TS_RECENT |
> -                                     FLAG_NO_CHALLENGE_ACK) > 0;
> +       ret = tcp_ack(sk, skb, FLAG_SLOWPATH |
> +                               FLAG_UPDATE_TS_RECENT |
> +                               FLAG_NO_CHALLENGE_ACK);
> +       acceptable = ret > 0;
>
>         if (!acceptable) {
>                 if (sk->sk_state == TCP_SYN_RECV)
>                         return 1;       /* send one RST */
> -               tcp_send_challenge_ack(sk);
> +               if (ret > -2)
> +                       tcp_send_challenge_ack(sk);
>                 goto discard;
>         }
>         switch (sk->sk_state) {
> --
> 2.25.1
>

Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[Zhouyi Zhou]

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.

Apologize sincerely in advance if I have made noise.

Thank you for your time

Sincerely
Zhouyi
>
> Thank you.
>
> > --
> >  net/ipv4/tcp_input.c | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index bfe4112e000c..4bbf85d7ea8c 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -3771,11 +3771,13 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> >                 goto old_ack;
> >         }
> >
> > -       /* If the ack includes data we haven't sent yet, discard
> > -        * this segment (RFC793 Section 3.9).
> > +       /* If the ack includes data we haven't sent yet, then send
> > +        * an ack, drop this segment, and return (RFC793 Section 3.9 page 72).
> >          */
> > -       if (after(ack, tp->snd_nxt))
> > -               return -1;
> > +       if (after(ack, tp->snd_nxt)) {
> > +               tcp_send_ack(sk);
> > +               return -2;
> > +       }
> >
> >         if (after(ack, prior_snd_una)) {
> >                 flag |= FLAG_SND_UNA_ADVANCED;
> > @@ -6385,6 +6387,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
> >         struct request_sock *req;
> >         int queued = 0;
> >         bool acceptable;
> > +       int ret;
> >
> >         switch (sk->sk_state) {
> >         case TCP_CLOSE:
> > @@ -6451,14 +6454,16 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
> >                 return 0;
> >
> >         /* step 5: check the ACK field */
> > -       acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
> > -                                     FLAG_UPDATE_TS_RECENT |
> > -                                     FLAG_NO_CHALLENGE_ACK) > 0;
> > +       ret = tcp_ack(sk, skb, FLAG_SLOWPATH |
> > +                               FLAG_UPDATE_TS_RECENT |
> > +                               FLAG_NO_CHALLENGE_ACK);
> > +       acceptable = ret > 0;
> >
> >         if (!acceptable) {
> >                 if (sk->sk_state == TCP_SYN_RECV)
> >                         return 1;       /* send one RST */
> > -               tcp_send_challenge_ack(sk);
> > +               if (ret > -2)
> > +                       tcp_send_challenge_ack(sk);
> >                 goto discard;
> >         }
> >         switch (sk->sk_state) {
> > --
> > 2.25.1
> >

Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[Neal Cardwell]

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

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

  Principled Unearthing of TCP Side Channel Vulnerabilities
  https://dl.acm.org/doi/10.1145/3319535.3354250

best regards,
neal

Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[Zhouyi Zhou]

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

Re: [PATCH v2] net:ipv4: send an ack when seg.ack > snd.nxt

[Zhouyi Zhou]

Hi

Thank you for reviewing my patch and guiding me to do the research ;-)

1. Abstract
I performed packetdrill test and security analysis of the patch. I
think we shouldn't apply my patch to the Linux kernel now because of
security challenges.

2. Introduction
RFC 793 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."
So, I tried a patch to implement the above.

3. Packetdrill test result
Before apply my patch:
http://154.220.3.115/May062022/run_all.orig.log
After apply my patch:
http://154.220.3.115/May062022/run_all.new.log

Comparing these two logs, I found the difference is caused by
a) the surplus ack from my patch: invalid_ack.pkt (ipv4-mapped-v6 ipv4 ipv6)
b) tcp busy time checking fail: tcp-info-sndbuf-limited.pkt
(ipv4-mapped-v6 ipv4 ipv6)
c) packet time error: close-remote-fin-then-close.pkt (ipv6)

I conclude that the patch does not introduce a serious problem to the
TCP stack from packetdrill's point of view.

4. Possible infinite loop introduced by packet injection
(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

If we apply the patch, nothing can stop this infinite loop, we can
only limit the rate of new ack.

To note that, off path attack can achieve above attack by means of TCP
off path exploits [1] [2]

5. Possible new, unanticipated side channels
To mitigate the infinite loop in section 4, we should limit the rate
of the ack. Limiting the rate of global ack has the potential of
introducing new, unanticipated side channels.

6. Conclusion
I think we shouldn't apply my patch to the Linux kernel now because of
security challenges.


[1] "Off-Path TCP Exploits: Global Rate Limit Considered Dangerous"
https://www.usenix.org/conference/usenixsecurity16/technical-sessions/presentation/cao
[2] "Principled Unearthing of TCP Side Channel Vulnerabilities"
https://dl.acm.org/doi/10.1145/3319535.3354250

Thanks
Zhouyi

On Sat, Mar 19, 2022 at 11:31 PM Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
>
> 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