Messages in this thread | | | Date | Thu, 21 Sep 2023 17:01:13 +0100 | Subject | Re: [PATCH v12 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets | From | Dmitry Safonov <> |
| |
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
| |