netdev - Re: [RESEND PATCH 2/2] TCP: pass accepted-TFO indication through getsockopt

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADVnQym3FmsuCOZdjhf+G42xryOmGLT23gBiHdam2fDcMv7TFg@mail.gmail.com>
Date: Tue, 22 Apr 2025 11:50:03 -0400
From: Neal Cardwell <ncardwell@...gle.com>
To: Jeremy Harris <jgh@...m.org>
Cc: netdev@...r.kernel.org, edumazet@...gle.com
Subject: Re: [RESEND PATCH 2/2] TCP: pass accepted-TFO indication through getsockopt

On Wed, Apr 16, 2025 at 5:15 AM Jeremy Harris <jgh@...m.org> wrote:
>
> Signed-off-by: Jeremy Harris <jgh@...m.org>

Regarding the proposed commit title:

  TCP: pass accepted-TFO indication through getsockopt

Please use something more like:

  tcp: fastopen: pass TFO child indication through getsockopt

> ---
>  include/uapi/linux/tcp.h | 1 +
>  net/ipv4/tcp.c           | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index dc8fdc80e16b..ae8c5a8af0e5 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -184,6 +184,7 @@ enum tcp_fastopen_client_fail {
>  #define TCPI_OPT_ECN_SEEN      16 /* we received at least one packet with ECT */
>  #define TCPI_OPT_SYN_DATA      32 /* SYN-ACK acked data in SYN sent or rcvd */
>  #define TCPI_OPT_USEC_TS       64 /* usec timestamps */
> +#define TCPI_OPT_TFO_SEEN      128 /* we accepted a Fast Open option on SYN */

IMHO this bit name is slightly misleading, and does not match the comment.

Sometimes when a SYN is received with a TFO option the server will
fail to create a child because the TFO cookie is incorrect. In such a
case, a TFO option is "seen", but TFO is not *used* because the TFO
cookie is incorrect; so this "TFO_SEEN" bit would be 0 even though a
TFO option was "seen". IMHO that is confusing/misleading.

When this bit is set, we know not only that the "Received SYN includes
Fast Open option" (comment from the previous patch), but we also know
that the TFO cookie was correct and a child socket was created.

So I would suggest a more specific bit name, something like:

+#define TCPI_OPT_TFO_CHILD      128 /* child from a Fast Open option on SYN */

+       if (tp->syn_fastopen_child)
+               info->tcpi_options |= TCPI_OPT_TFO_CHILD;

thanks,
neal