| 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
| ||
|
Message-ID: <20231016220235.80560-1-kuniyu@amazon.com>
Date: Mon, 16 Oct 2023 15:02:35 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <sdf@...gle.com>
CC: <andrii@...nel.org>, <ast@...nel.org>, <bpf@...r.kernel.org>,
<daniel@...earbox.net>, <davem@...emloft.net>, <dsahern@...nel.org>,
<edumazet@...gle.com>, <haoluo@...gle.com>, <john.fastabend@...il.com>,
<jolsa@...nel.org>, <kpsingh@...nel.org>, <kuba@...nel.org>,
<kuni1840@...il.com>, <kuniyu@...zon.com>, <martin.lau@...ux.dev>,
<mykolal@...com>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
<song@...nel.org>, <yonghong.song@...ux.dev>
Subject: Re: [PATCH v1 bpf-next 06/11] bpf: tcp: Add SYN Cookie validation SOCK_OPS hook.
From: Stanislav Fomichev <sdf@...gle.com>
Date: Mon, 16 Oct 2023 13:38:25 -0700
> 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?
Good catch!
I skipped calling tcp_synq_overflow() in the previous patch but should
have incremented LINUX_MIB_SYNCOOKIESSENT. Will fix in v2.
Thanks!
Powered by blists - more mailing lists