netdev - Re: [PATCH v2 12/25] tcp: ipv6: Add AO signing for tcp_v6_send

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: <37c1a2c7-3bfa-d36d-075f-a0065b8a05c1@gmail.com>
Date:   Tue, 2 Nov 2021 20:44:28 -0600
From:   David Ahern <dsahern@...il.com>
To:     Leonard Crestez <cdleonard@...il.com>,
        David Ahern <dsahern@...nel.org>,
        Shuah Khan <shuah@...nel.org>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Kuniyuki Iwashima <kuniyu@...zon.co.jp>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Yuchung Cheng <ycheng@...gle.com>,
        Francesco Ruggeri <fruggeri@...sta.com>,
        Mat Martineau <mathew.j.martineau@...ux.intel.com>,
        Christoph Paasch <cpaasch@...le.com>,
        Ivan Delalande <colona@...sta.com>,
        Priyaranjan Jha <priyarjha@...gle.com>, netdev@...r.kernel.org,
        linux-crypto@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 12/25] tcp: ipv6: Add AO signing for
 tcp_v6_send_response

On 11/1/21 10:34 AM, Leonard Crestez wrote:
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 96a29caf56c7..68f9545e4347 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -902,13 +902,37 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>  	struct sock *ctl_sk = net->ipv6.tcp_sk;
>  	unsigned int tot_len = sizeof(struct tcphdr);
>  	__be32 mrst = 0, *topt;
>  	struct dst_entry *dst;
>  	__u32 mark = 0;
> +#ifdef CONFIG_TCP_AUTHOPT
> +	struct tcp_authopt_info *authopt_info = NULL;
> +	struct tcp_authopt_key_info *authopt_key_info = NULL;
> +	u8 authopt_rnextkeyid;
> +#endif
>  
>  	if (tsecr)
>  		tot_len += TCPOLEN_TSTAMP_ALIGNED;
> +#ifdef CONFIG_TCP_AUTHOPT

I realize MD5 is done this way, but new code can always strive to be
better. Put this and the one below in helpers such that this logic is in
the authopt.h file and the intrusion here is a one liner that either
compiles in or out based on the config setting.

> +	/* Key lookup before SKB allocation */
> +	if (static_branch_unlikely(&tcp_authopt_needed) && sk) {
> +		if (sk->sk_state == TCP_TIME_WAIT)
> +			authopt_info = tcp_twsk(sk)->tw_authopt_info;
> +		else
> +			authopt_info = rcu_dereference(tcp_sk(sk)->authopt_info);
> +
> +		if (authopt_info) {
> +			authopt_key_info = __tcp_authopt_select_key(sk, authopt_info, sk,
> +								    &authopt_rnextkeyid);
> +			if (authopt_key_info) {
> +				tot_len += TCPOLEN_AUTHOPT_OUTPUT;
> +				/* Don't use MD5 */
> +				key = NULL;
> +			}
> +		}
> +	}
> +#endif
>  #ifdef CONFIG_TCP_MD5SIG
>  	if (key)
>  		tot_len += TCPOLEN_MD5SIG_ALIGNED;
>  #endif
>  
> @@ -961,10 +985,24 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>  		tcp_v6_md5_hash_hdr((__u8 *)topt, key,
>  				    &ipv6_hdr(skb)->saddr,
>  				    &ipv6_hdr(skb)->daddr, t1);
>  	}
>  #endif
> +#ifdef CONFIG_TCP_AUTHOPT
> +	/* Compute the TCP-AO mac. Unlike in the ipv4 case we have a real SKB */
> +	if (static_branch_unlikely(&tcp_authopt_needed) && authopt_key_info) {
> +		*topt++ = htonl((TCPOPT_AUTHOPT << 24) |
> +				(TCPOLEN_AUTHOPT_OUTPUT << 16) |
> +				(authopt_key_info->send_id << 8) |
> +				(authopt_rnextkeyid));
> +		tcp_authopt_hash((char *)topt,
> +				 authopt_key_info,
> +				 authopt_info,
> +				 (struct sock *)sk,
> +				 buff);
> +	}
> +#endif
>  
>  	memset(&fl6, 0, sizeof(fl6));
>  	fl6.daddr = ipv6_hdr(skb)->saddr;
>  	fl6.saddr = ipv6_hdr(skb)->daddr;
>  	fl6.flowlabel = label;
>