netdev - Re: [PATCH v12 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets

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: <d0e332b4326eec032b77c7492f501f3c1fbe8242.camel@redhat.com>
Date: Thu, 21 Sep 2023 13:20:34 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Dmitry Safonov <dima@...sta.com>, David Ahern <dsahern@...nel.org>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "David S.
 Miller" <davem@...emloft.net>
Cc: 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 v12 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing
 packets

On Mon, 2023-09-18 at 20:00 +0100, Dmitry Safonov wrote:
> @@ -615,19 +616,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
>   * (but it may well be that other scenarios fail similarly).
>   */
>  static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> -			      struct tcp_out_options *opts)
> +			      struct tcp_out_options *opts,
> +			      struct tcp_key *key)
>  {
>  	__be32 *ptr = (__be32 *)(th + 1);
>  	u16 options = opts->options;	/* mungable copy */
>  
> -	if (unlikely(OPTION_MD5 & options)) {
> +	if (tcp_key_is_md5(key)) {
>  		*ptr++ = htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
>  			       (TCPOPT_MD5SIG << 8) | TCPOLEN_MD5SIG);
>  		/* overload cookie hash location */
>  		opts->hash_location = (__u8 *)ptr;
>  		ptr += 4;
> -	}
> +	} else if (tcp_key_is_ao(key)) {
> +#ifdef CONFIG_TCP_AO
>
> +		struct tcp_ao_key *rnext_key;
> +		struct tcp_ao_info *ao_info;
> +		u8 maclen;
>  
> +		ao_info = rcu_dereference_check(tp->ao_info,
> +				lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> +		rnext_key = READ_ONCE(ao_info->rnext_key);
> +		if (WARN_ON_ONCE(!rnext_key))
> +			goto out_ao;
> +		maclen = tcp_ao_maclen(key->ao_key);

It looks like only TCP_AO really accesses 'key', and TCP_AO can easily
fetch it from ao_info. Can the AO key change in between
tcp_get_current_key() and here?

Otherwise I think it would be better omitting the 'key' argument here
and use the 'options' flag to pick TCP_AO vs MD5.

And:

	if (unlikely(OPTION_MD5 & options)) {

could possibly be moved under a CONFIG_MD5 compiler conditional.

Cheers,

Paolo