linux-kernel - Re: [PATCH v10 net-next 10/23] net/tcp: Wire TCP-AO to request sockets

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: <ZN0nQqIwXp5cQJTR@vergenet.net>
Date:   Wed, 16 Aug 2023 21:45:06 +0200
From:   Simon Horman <horms@...nel.org>
To:     Dmitry Safonov <dima@...sta.com>
Cc:     David Ahern <dsahern@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org,
        Andy Lutomirski <luto@...capital.net>,
        Ard Biesheuvel <ardb@...nel.org>,
        Bob Gilligan <gilligan@...sta.com>,
        Dan Carpenter <error27@...il.com>,
        David Laight <David.Laight@...lab.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Donald Cassidy <dcassidy@...hat.com>,
        Eric Biggers <ebiggers@...nel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Francesco Ruggeri <fruggeri05@...il.com>,
        "Gaillardetz, Dominik" <dgaillar@...na.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Ivan Delalande <colona@...sta.com>,
        Leonard Crestez <cdleonard@...il.com>,
        "Nassiri, Mohammad" <mnassiri@...na.com>,
        Salam Noureddine <noureddine@...sta.com>,
        Simon Horman <simon.horman@...igine.com>,
        "Tetreault, Francois" <ftetreau@...na.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v10 net-next 10/23] net/tcp: Wire TCP-AO to request
 sockets

On Tue, Aug 15, 2023 at 08:14:39PM +0100, Dmitry Safonov wrote:

...

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 8dfa0959e403..7178e8bab922 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c

...

> @@ -1194,9 +1198,51 @@ static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb)
>  static void tcp_v6_reqsk_send_ack(const struct sock *sk, struct sk_buff *skb,
>  				  struct request_sock *req)
>  {
> +	struct tcp_md5sig_key *md5_key = NULL;
> +	const struct in6_addr *addr;
> +	u8 *traffic_key = NULL;
>  	int l3index;
>  
>  	l3index = tcp_v6_sdif(skb) ? tcp_v6_iif_l3_slave(skb) : 0;
> +	addr = &ipv6_hdr(skb)->saddr;
> +
> +	if (tcp_rsk_used_ao(req)) {
> +#ifdef CONFIG_TCP_AO
> +		struct tcp_ao_key *ao_key = NULL;
> +		const struct tcp_ao_hdr *aoh;
> +		u8 keyid = 0;

Hi Dmitry,

keyid is declared and initialised here.

> +
> +		/* Invalid TCP option size or twice included auth */
> +		if (tcp_parse_auth_options(tcp_hdr(skb), NULL, &aoh))
> +			return;
> +		if (!aoh)
> +			return;
> +		ao_key = tcp_v6_ao_do_lookup(sk, addr, aoh->rnext_keyid, -1);
> +		if (unlikely(!ao_key)) {
> +			/* Send ACK with any matching MKT for the peer */
> +			ao_key = tcp_v6_ao_do_lookup(sk, addr, -1, -1);
> +			/* Matching key disappeared (user removed the key?)
> +			 * let the handshake timeout.
> +			 */
> +			if (!ao_key) {
> +				net_info_ratelimited("TCP-AO key for (%pI6, %d)->(%pI6, %d) suddenly disappeared, won't ACK new connection\n",
> +						     addr,
> +						     ntohs(tcp_hdr(skb)->source),
> +						     &ipv6_hdr(skb)->daddr,
> +						     ntohs(tcp_hdr(skb)->dest));
> +				return;
> +			}
> +		}
> +		traffic_key = kmalloc(tcp_ao_digest_size(ao_key), GFP_ATOMIC);
> +		if (!traffic_key)
> +			return;
> +
> +		keyid = aoh->keyid;

And reinitialised here.
But is otherwise unused.

Flagged in a W=1 build with both clang-16 and gcc-13.

> +		tcp_v6_ao_calc_key_rsk(ao_key, traffic_key, req);
> +#endif
> +	} else {
> +		md5_key = tcp_v6_md5_do_lookup(sk, addr, l3index);
> +	}
>  
>  	/* sk->sk_state == TCP_LISTEN -> for regular TCP_SYN_RECV
>  	 * sk->sk_state == TCP_SYN_RECV -> for Fast Open.

...