Re: [PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO

From: Eric Dumazet
Date: Tue Sep 12 2023 - 12:47:54 EST

Next message: Jonathan Cameron: "Re: [PATCH RFC v2 14/18] dax/region: Support DAX device creation on dynamic DAX regions"
Previous message: Eric Dumazet: "Re: [PATCH v11 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets"
In reply to: Dmitry Safonov: "[PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO"
Next in thread: Dmitry Safonov: "Re: [PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Sep 11, 2023 at 11:04 PM Dmitry Safonov <dima@xxxxxxxxxx> wrote:
>
> Similarly to RST segments, wire SYN-ACKs to TCP-AO.
> tcp_rsk_used_ao() is handy here to check if the request socket used AO
> and needs a signature on the outgoing segments.
>
> Co-developed-by: Francesco Ruggeri <fruggeri@xxxxxxxxxx>
> Signed-off-by: Francesco Ruggeri <fruggeri@xxxxxxxxxx>
> Co-developed-by: Salam Noureddine <noureddine@xxxxxxxxxx>
> Signed-off-by: Salam Noureddine <noureddine@xxxxxxxxxx>
> Signed-off-by: Dmitry Safonov <dima@xxxxxxxxxx>
> Acked-by: David Ahern <dsahern@xxxxxxxxxx>
> ---
> include/net/tcp.h | 3 +++
> include/net/tcp_ao.h | 6 +++++
> net/ipv4/tcp_ao.c | 22 +++++++++++++++++
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_output.c | 57 ++++++++++++++++++++++++++++++++++++++-----
> net/ipv6/tcp_ao.c | 22 +++++++++++++++++
> net/ipv6/tcp_ipv6.c | 1 +
> 7 files changed, 106 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 5daa2e98e6a3..56f4180443c7 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2179,6 +2179,9 @@ struct tcp_request_sock_ops {
> struct request_sock *req,
> int sndid, int rcvid);
> int (*ao_calc_key)(struct tcp_ao_key *mkt, u8 *key, struct request_sock *sk);
> + int (*ao_synack_hash)(char *ao_hash, struct tcp_ao_key *mkt,
> + struct request_sock *req, const struct sk_buff *skb,
> + int hash_offset, u32 sne);
> #endif
> #ifdef CONFIG_SYN_COOKIES
> __u32 (*cookie_init_seq)(const struct sk_buff *skb,
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index d26d98f1b048..c922d2e31d08 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -144,6 +144,9 @@ int tcp_ao_prepare_reset(const struct sock *sk, struct sk_buff *skb,
> int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen);
> struct tcp_ao_key *tcp_v4_ao_lookup(const struct sock *sk, struct sock *addr_sk,
> int sndid, int rcvid);
> +int tcp_v4_ao_synack_hash(char *ao_hash, struct tcp_ao_key *mkt,
> + struct request_sock *req, const struct sk_buff *skb,
> + int hash_offset, u32 sne);
> int tcp_v4_ao_calc_key_sk(struct tcp_ao_key *mkt, u8 *key,
> const struct sock *sk,
> __be32 sisn, __be32 disn, bool send);
> @@ -178,6 +181,9 @@ int tcp_v6_ao_hash_skb(char *ao_hash, struct tcp_ao_key *key,
> const struct sock *sk, const struct sk_buff *skb,
> const u8 *tkey, int hash_offset, u32 sne);
> int tcp_v6_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen);
> +int tcp_v6_ao_synack_hash(char *ao_hash, struct tcp_ao_key *ao_key,
> + struct request_sock *req, const struct sk_buff *skb,
> + int hash_offset, u32 sne);
> void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb);
> void tcp_ao_connect_init(struct sock *sk);
> void tcp_ao_syncookie(struct sock *sk, const struct sk_buff *skb,
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index b114f3d901a0..0d8ea381300b 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -568,6 +568,28 @@ int tcp_v4_ao_hash_skb(char *ao_hash, struct tcp_ao_key *key,
> tkey, hash_offset, sne);
> }
>
> +int tcp_v4_ao_synack_hash(char *ao_hash, struct tcp_ao_key *ao_key,
> + struct request_sock *req, const struct sk_buff *skb,
> + int hash_offset, u32 sne)
> +{
> + void *hash_buf = NULL;
> + int err;
> +
> + hash_buf = kmalloc(tcp_ao_digest_size(ao_key), GFP_ATOMIC);
> + if (!hash_buf)
> + return -ENOMEM;
> +
> + err = tcp_v4_ao_calc_key_rsk(ao_key, hash_buf, req);
> + if (err)
> + goto out;
> +
> + err = tcp_ao_hash_skb(AF_INET, ao_hash, ao_key, req_to_sk(req), skb,
> + hash_buf, hash_offset, sne);
> +out:
> + kfree(hash_buf);
> + return err;
> +}
> +
> struct tcp_ao_key *tcp_v4_ao_lookup_rsk(const struct sock *sk,
> struct request_sock *req,
> int sndid, int rcvid)
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 9a4ffcc965f3..c40da33d988b 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1675,6 +1675,7 @@ const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
> #ifdef CONFIG_TCP_AO
> .ao_lookup = tcp_v4_ao_lookup_rsk,
> .ao_calc_key = tcp_v4_ao_calc_key_rsk,
> + .ao_synack_hash = tcp_v4_ao_synack_hash,
> #endif
> #ifdef CONFIG_SYN_COOKIES
> .cookie_init_seq = cookie_v4_init_sequence,
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 44c97e6ddd50..c9d6decef443 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -900,6 +900,7 @@ static unsigned int tcp_synack_options(const struct sock *sk,
> unsigned int mss, struct sk_buff *skb,
> struct tcp_out_options *opts,
> const struct tcp_md5sig_key *md5,
> + const struct tcp_ao_key *ao,
> struct tcp_fastopen_cookie *foc,
> enum tcp_synack_type synack_type,
> struct sk_buff *syn_skb)
> @@ -921,6 +922,14 @@ static unsigned int tcp_synack_options(const struct sock *sk,
> ireq->tstamp_ok &= !ireq->sack_ok;
> }
> #endif
> +#ifdef CONFIG_TCP_AO
> + if (ao) {
> + opts->options |= OPTION_AO;
> + remaining -= tcp_ao_len(ao);
> + ireq->tstamp_ok &= !ireq->sack_ok;
> + }
> +#endif
> + WARN_ON_ONCE(md5 && ao);
>
> /* We always send an MSS option. */
> opts->mss = mss;
> @@ -3727,6 +3736,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
> struct inet_request_sock *ireq = inet_rsk(req);
> const struct tcp_sock *tp = tcp_sk(sk);
> struct tcp_md5sig_key *md5 = NULL;
> + struct tcp_ao_key *ao_key = NULL;
> struct tcp_out_options opts;
> struct sk_buff *skb;
> int tcp_header_size;
> @@ -3777,16 +3787,43 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
> tcp_rsk(req)->snt_synack = tcp_skb_timestamp_us(skb);
> }
>
> -#ifdef CONFIG_TCP_MD5SIG
> +#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
> rcu_read_lock();
> - md5 = tcp_rsk(req)->af_specific->req_md5_lookup(sk, req_to_sk(req));
> #endif
> + if (tcp_rsk_used_ao(req)) {
> +#ifdef CONFIG_TCP_AO
> + u8 maclen = tcp_rsk(req)->maclen;
> + u8 keyid = tcp_rsk(req)->ao_keyid;
> +
> + ao_key = tcp_sk(sk)->af_specific->ao_lookup(sk, req_to_sk(req),
> + keyid, -1);
> + /* If there is no matching key - avoid sending anything,
> + * especially usigned segments. It could try harder and lookup
> + * for another peer-matching key, but the peer has requested
> + * ao_keyid (RFC5925 RNextKeyID), so let's keep it simple here.
> + */
> + if (unlikely(!ao_key || tcp_ao_maclen(ao_key) != maclen)) {
> + rcu_read_unlock();
> + skb_dst_drop(skb);

This does look necessary ? kfree_skb(skb) should also skb_dst_drop(skb);

> + kfree_skb(skb);
> + net_warn_ratelimited("TCP-AO: the keyid %u with maclen %u|%u from SYN packet is not present - not sending SYNACK\n",
> + keyid, maclen,
> + ao_key ? tcp_ao_maclen(ao_key) : 0);

dereferencing ao_key after rcu_read_unlock() is a bug.

> + return NULL;
> + }
> +#endif
> + } else {
> +#ifdef CONFIG_TCP_MD5SIG
> + md5 = tcp_rsk(req)->af_specific->req_md5_lookup(sk,
> + req_to_sk(req));
> +#endif
> + }
> skb_set_hash(skb, READ_ONCE(tcp_rsk(req)->txhash), PKT_HASH_TYPE_L4);
> /* bpf program will be interested in the tcp_flags */
> TCP_SKB_CB(skb)->tcp_flags = TCPHDR_SYN | TCPHDR_ACK;
> tcp_header_size = tcp_synack_options(sk, req, mss, skb, &opts, md5,
> - foc, synack_type,
> - syn_skb) + sizeof(*th);
> + ao_key, foc, synack_type, syn_skb)
> + + sizeof(*th);
>
> skb_push(skb, tcp_header_size);
> skb_reset_transport_header(skb);
> @@ -3806,7 +3843,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
>
> /* RFC1323: The window in SYN & SYN/ACK segments is never scaled. */
> th->window = htons(min(req->rsk_rcv_wnd, 65535U));
> - tcp_options_write(th, NULL, NULL, &opts, NULL);
> + tcp_options_write(th, NULL, tcp_rsk(req), &opts, ao_key);
> th->doff = (tcp_header_size >> 2);
> TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTSEGS);
>
> @@ -3814,7 +3851,15 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
> /* Okay, we have all we need - do the md5 hash if needed */
> if (md5)
> tcp_rsk(req)->af_specific->calc_md5_hash(opts.hash_location,
> - md5, req_to_sk(req), skb);
> + md5, req_to_sk(req), skb);
> +#endif
> +#ifdef CONFIG_TCP_AO
> + if (ao_key)
> + tcp_rsk(req)->af_specific->ao_synack_hash(opts.hash_location,
> + ao_key, req, skb,
> + opts.hash_location - (u8 *)th, 0);
> +#endif
> +#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
> rcu_read_unlock();
> #endif
>
> diff --git a/net/ipv6/tcp_ao.c b/net/ipv6/tcp_ao.c
> index c9a6fa84f6ce..99753e12c08c 100644
> --- a/net/ipv6/tcp_ao.c
> +++ b/net/ipv6/tcp_ao.c
> @@ -144,3 +144,25 @@ int tcp_v6_parse_ao(struct sock *sk, int cmd,
> {
> return tcp_parse_ao(sk, cmd, AF_INET6, optval, optlen);
> }
> +
> +int tcp_v6_ao_synack_hash(char *ao_hash, struct tcp_ao_key *ao_key,
> + struct request_sock *req, const struct sk_buff *skb,
> + int hash_offset, u32 sne)
> +{
> + void *hash_buf = NULL;
> + int err;
> +
> + hash_buf = kmalloc(tcp_ao_digest_size(ao_key), GFP_ATOMIC);
> + if (!hash_buf)
> + return -ENOMEM;
> +
> + err = tcp_v6_ao_calc_key_rsk(ao_key, hash_buf, req);
> + if (err)
> + goto out;
> +
> + err = tcp_ao_hash_skb(AF_INET6, ao_hash, ao_key, req_to_sk(req), skb,
> + hash_buf, hash_offset, sne);
> +out:
> + kfree(hash_buf);
> + return err;
> +}
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index c060cd964f91..f57617d2921a 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -845,6 +845,7 @@ const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
> #ifdef CONFIG_TCP_AO
> .ao_lookup = tcp_v6_ao_lookup_rsk,
> .ao_calc_key = tcp_v6_ao_calc_key_rsk,
> + .ao_synack_hash = tcp_v6_ao_synack_hash,
> #endif
> #ifdef CONFIG_SYN_COOKIES
> .cookie_init_seq = cookie_v6_init_sequence,
> --
> 2.41.0
>

Next message: Jonathan Cameron: "Re: [PATCH RFC v2 14/18] dax/region: Support DAX device creation on dynamic DAX regions"
Previous message: Eric Dumazet: "Re: [PATCH v11 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets"
In reply to: Dmitry Safonov: "[PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO"
Next in thread: Dmitry Safonov: "Re: [PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]