netdev - Re: [PATCH] net/tcp: Only produce AO/MD5 logs if there are any keys

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: <d3de02c2-5158-4cae-b6f7-02d0fe7dae35@arista.com>
Date: Thu, 4 Jan 2024 13:57:49 +0000
From: Dmitry Safonov <dima@...sta.com>
To: Eric Dumazet <edumazet@...gle.com>, "David S. Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: Christian Kujau <lists@...dbynature.de>,
 Salam Noureddine <noureddine@...sta.com>,
 Dmitry Safonov <0x7f454c46@...il.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net/tcp: Only produce AO/MD5 logs if there are any keys

On 1/4/24 13:42, Dmitry Safonov wrote:
> User won't care about inproper hash options in the TCP header if they
> don't use neither TCP-AO nor TCP-MD5. Yet, those logs can add up in
> syslog, while not being a real concern to the host admin:
>> kernel: TCP: TCP segment has incorrect auth options set for XX.20.239.12.54681->XX.XX.90.103.80 [S]
> 
> Keep silent and avoid logging when there aren't any keys in the system.
> 
> Side-note: I also defined static_branch_tcp_*() helpers to avoid more
> ifdeffery, going to remove more ifdeffery further with their help.
> 
> Reported-by: Christian Kujau <lists@...dbynature.de>
> Closes: https://lore.kernel.org/all/f6b59324-1417-566f-a976-ff2402718a8d@nerdbynature.de/

Probably, it also can have

Fixes: 2717b5adea9e ("net/tcp: Add tcp_hash_fail() ratelimited logs")

> Signed-off-by: Dmitry Safonov <dima@...sta.com>
> ---
>  include/net/tcp.h    |  2 --
>  include/net/tcp_ao.h | 26 +++++++++++++++++++++++---
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 144ba48bb07b..87f0e6c2e1f2 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1788,8 +1788,6 @@ struct tcp_md5sig_key *tcp_v4_md5_lookup(const struct sock *sk,
>  					 const struct sock *addr_sk);
>  
>  #ifdef CONFIG_TCP_MD5SIG
> -#include <linux/jump_label.h>
> -extern struct static_key_false_deferred tcp_md5_needed;
>  struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk, int l3index,
>  					   const union tcp_md5_addr *addr,
>  					   int family, bool any_l3index);
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index 647781080613..b04afced4cc9 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -127,12 +127,35 @@ struct tcp_ao_info {
>  	struct rcu_head		rcu;
>  };
>  
> +#ifdef CONFIG_TCP_MD5SIG
> +#include <linux/jump_label.h>
> +extern struct static_key_false_deferred tcp_md5_needed;
> +#define static_branch_tcp_md5()	static_branch_unlikely(&tcp_md5_needed.key)
> +#else
> +#define static_branch_tcp_md5()	false
> +#endif
> +#ifdef CONFIG_TCP_AO
> +/* TCP-AO structures and functions */
> +#include <linux/jump_label.h>
> +extern struct static_key_false_deferred tcp_ao_needed;
> +#define static_branch_tcp_ao()	static_branch_unlikely(&tcp_ao_needed.key)
> +#else
> +#define static_branch_tcp_ao()	false
> +#endif
> +
> +static inline bool tcp_hash_should_produce_warnings(void)
> +{
> +	return static_branch_tcp_md5() || static_branch_tcp_ao();
> +}
> +
>  #define tcp_hash_fail(msg, family, skb, fmt, ...)			\
>  do {									\
>  	const struct tcphdr *th = tcp_hdr(skb);				\
>  	char hdr_flags[6];						\
>  	char *f = hdr_flags;						\
>  									\
> +	if (!tcp_hash_should_produce_warnings())			\
> +		break;							\
>  	if (th->fin)							\
>  		*f++ = 'F';						\
>  	if (th->syn)							\
> @@ -159,9 +182,6 @@ do {									\
>  
>  #ifdef CONFIG_TCP_AO
>  /* TCP-AO structures and functions */
> -#include <linux/jump_label.h>
> -extern struct static_key_false_deferred tcp_ao_needed;
> -
>  struct tcp4_ao_context {
>  	__be32		saddr;
>  	__be32		daddr;
> 
> ---
> base-commit: ac865f00af293d081356bec56eea90815094a60e
> change-id: 20240104-tcp_hash_fail-logs-daa1a4dde694
> 
> Best regards,

-- 
Dmitry