netdev - Re: Potential violation of RFC793 3.9, missing challenge ACK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 21 May 2024 12:33:05 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: hotaka.miyazaki@...ertrust.co.jp, netdev@...r.kernel.org
Subject: Re: Potential violation of RFC793 3.9, missing challenge ACK

On Tue, 2024-05-21 at 12:12 +0200, Eric Dumazet wrote:
> On Tue, May 21, 2024 at 11:47 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > 
> > On Thu, 2024-05-16 at 16:12 +0900, hotaka.miyazaki@...ertrust.co.jp
> > wrote:
> > > Hello.
> > > 
> > > I have a question about the following part of the tcp_ack function in net/ipv4/tcp_input.c.
> > > ```
> > >       /* If the ack includes data we haven't sent yet, discard
> > >       * this segment (RFC793 Section 3.9).
> > >       */
> > >       if (after(ack, tp->snd_nxt))
> > >         return -SKB_DROP_REASON_TCP_ACK_UNSENT_DATA;
> > > ```
> > > I think that this part violates RFC793 3.9 (and the equivalent part in RFC9293 (3.10.7.4)).
> > > 
> > > According to the RFC, “If the ACK acks something not yet sent (SEG.ACK > SND.NXT) then send an ACK, drop the segment, and return“ [1].
> > > However, the code appears not to ack before discarding a segment.
> > 
> > Note that in some cases the ack is generated by the caller, see:
> > 
> > https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_input.c#L6703
> > 
> > In any case not sending the challenge ack does not look a violation to
> > me, as the RFC suggest (it uses 'should') and does not impose (with a
> > 'must') such acks . Sending them back too freely opens-up to possible
> > security issue.
> 
> Yes, this behavior was added in 2009, we lived 15 years with it.
> 
> I do not see a pressing reason to send challenge acks here.
> 
> Hotaka, please explain why this would help a valid use case (I am not
> speaking of broken middleboxes)
> 
> > 
> > side note, @Eric: it looks like we can send 2 challenge ack for half-
> > closed socket hitting RFC 5961 5.2 mitigations?!?
> 
> Sorry, can you elaborate ?

Oops, I did not consider __tcp_oow_rate_limited() nor
FLAG_NO_CHALLENGE_ACK.

The code path I was looking at is:
https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_input.c#L6703
reaching:
https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_input.c#L3874
Indeed no duplicate acks, sorry for the noise.

Paolo