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: <ZS2fQXqhjRlG64kZ@google.com>
Date: Mon, 16 Oct 2023 13:38:25 -0700
From: Stanislav Fomichev <sdf@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Mykola Lysenko <mykolal@...com>, Kuniyuki Iwashima <kuni1840@...il.com>, bpf@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v1 bpf-next 06/11] bpf: tcp: Add SYN Cookie validation
 SOCK_OPS hook.

On 10/13, Kuniyuki Iwashima wrote:
> This patch adds a new SOCK_OPS hook to validate arbitrary SYN Cookie.
> 
> When the kernel receives ACK for SYN Cookie, the hook is invoked with
> bpf_sock_ops.op == BPF_SOCK_OPS_CHECK_SYNCOOKIE_CB if the listener has
> BPF_SOCK_OPS_SYNCOOKIE_CB_FLAG set by bpf_sock_ops_cb_flags_set().
> 
> The BPF program can access the following information to validate ISN:
> 
>   bpf_sock_ops.sk      : 4-tuple
>   bpf_sock_ops.skb     : TCP header
>   bpf_sock_ops.args[0] : ISN
> 
> The program must decode MSS and set it to bpf_sock_ops.replylong[0].
> 
> By default, the kernel validates SYN Cookie before allocating reqsk, but
> the hook is invoked after allocating reqsk to keep the user interface
> consistent with BPF_SOCK_OPS_GEN_SYNCOOKIE_CB.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
>  include/net/tcp.h              | 12 ++++++
>  include/uapi/linux/bpf.h       | 20 +++++++---
>  net/ipv4/syncookies.c          | 73 +++++++++++++++++++++++++++-------
>  net/ipv6/syncookies.c          | 44 +++++++++++++-------
>  tools/include/uapi/linux/bpf.h | 20 +++++++---
>  5 files changed, 130 insertions(+), 39 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 676618c89bb7..90d95acdc34a 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2158,6 +2158,18 @@ static inline __u32 cookie_init_sequence(const struct tcp_request_sock_ops *ops,
>  	__NET_INC_STATS(sock_net(sk), LINUX_MIB_SYNCOOKIESSENT);
>  	return ops->cookie_init_seq(skb, mss);
>  }
> +
> +#ifdef CONFIG_CGROUP_BPF
> +int bpf_skops_cookie_check(struct sock *sk, struct request_sock *req,
> +			   struct sk_buff *skb);
> +#else
> +static inline int bpf_skops_cookie_check(struct sock *sk, struct request_sock *req,
> +					 struct sk_buff *skb)
> +{
> +	return 0;
> +}
> +#endif
> +
>  #else
>  static inline __u32 cookie_init_sequence(const struct tcp_request_sock_ops *ops,
>  					 const struct sock *sk, struct sk_buff *skb,
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d3cc530613c0..e6f1507d7895 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6738,13 +6738,16 @@ enum {
>  	 * options first before the BPF program does.
>  	 */
>  	BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG = (1<<6),
> -	/* Call bpf when the kernel generates SYN Cookie (ISN) for SYN+ACK.
> +	/* Call bpf when the kernel generates SYN Cookie (ISN) for SYN+ACK
> +	 * and validates ACK for SYN Cookie.
>  	 *
> -	 * The bpf prog will be called to encode MSS into SYN Cookie with
> -	 * sock_ops->op == BPF_SOCK_OPS_GEN_SYNCOOKIE_CB.
> +	 * The bpf prog will be first called to encode MSS into SYN Cookie
> +	 * with sock_ops->op == BPF_SOCK_OPS_GEN_SYNCOOKIE_CB.  Then, the
> +	 * bpf prog will be called to decode MSS from SYN Cookie with
> +	 * sock_ops->op == BPF_SOCK_OPS_CHECK_SYNCOOKIE_CB.
>  	 *
> -	 * Please refer to the comment in BPF_SOCK_OPS_GEN_SYNCOOKIE_CB for
> -	 * input and output.
> +	 * Please refer to the comment in BPF_SOCK_OPS_GEN_SYNCOOKIE_CB and
> +	 * BPF_SOCK_OPS_CHECK_SYNCOOKIE_CB for input and output.
>  	 */
>  	BPF_SOCK_OPS_SYNCOOKIE_CB_FLAG = (1<<7),
>  /* Mask of all currently supported cb flags */
> @@ -6868,6 +6871,13 @@ enum {
>  					 *
>  					 * replylong[0]: ISN
>  					 */
> +	BPF_SOCK_OPS_CHECK_SYNCOOKIE_CB,/* Validate SYN Cookie and set
> +					 * MSS.
> +					 *
> +					 * args[0]: ISN
> +					 *
> +					 * replylong[0]: MSS
> +					 */
>  };
>  
>  /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 514f1a4abdee..b1dd415863ff 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -317,6 +317,37 @@ struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
>  }
>  EXPORT_SYMBOL_GPL(cookie_tcp_reqsk_alloc);
>  
> +#if IS_ENABLED(CONFIG_CGROUP_BPF) && IS_ENABLED(CONFIG_SYN_COOKIES)
> +int bpf_skops_cookie_check(struct sock *sk, struct request_sock *req, struct sk_buff *skb)
> +{
> +	struct bpf_sock_ops_kern sock_ops;
> +
> +	memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
> +
> +	sock_ops.op = BPF_SOCK_OPS_CHECK_SYNCOOKIE_CB;
> +	sock_ops.sk = req_to_sk(req);
> +	sock_ops.args[0] = tcp_rsk(req)->snt_isn;
> +
> +	bpf_skops_init_skb(&sock_ops, skb, tcp_hdrlen(skb));
> +
> +	if (BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk))
> +		goto err;
> +
> +	if (!sock_ops.replylong[0])
> +		goto err;
> +
> +	__NET_INC_STATS(sock_net(sk), LINUX_MIB_SYNCOOKIESRECV);

I don't see LINUX_MIB_SYNCOOKIESSENT being incremented in the
previous patch, so maybe also don't touch the mib here? The bpf
program can do the counting if needed?

Or, alternatively, add LINUX_MIB_SYNCOOKIESSENT to
the BPF_SOCK_OPS_GEN_SYNCOOKIE_CB path?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ