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 for Android: free password hash cracker in your pocket

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

Message-ID: <1f42042dbfe9b413cded5e5d59cd3933ec08ed08.camel@redhat.com>
Date: Tue, 21 May 2024 11:47:47 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: hotaka.miyazaki@...ertrust.co.jp, netdev@...r.kernel.org
Cc: Eric Dumazet <edumazet@...gle.com>
Subject: Re: Potential violation of RFC793 3.9, missing challenge ACK

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.

side note, @Eric: it looks like we can send 2 challenge ack for half-
closed socket hitting RFC 5961 5.2 mitigations?!?

Cheers,

Paolo