[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65981909-e330-dc91-a977-032d427cdd6a@arista.com>
Date: Thu, 21 Sep 2023 17:01:13 +0100
From: Dmitry Safonov <dima@...sta.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: David Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.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 v12 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing
packets
On 9/21/23 12:20, Paolo Abeni wrote:
> 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?
Yes, you read it right: current_key can be changed at any moment, when
the peer asks to start a rotation (tcp_inbound_ao_hash() on RX does
that). So, here we have to provide the fetched key as ao_key->maclen
(the length of MAC, the authentication/hash may be different between
different keys).
> 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.
Thanks,
Dmitry
Powered by blists - more mailing lists