netdev - RE: [PATCH bpf-next v2 2/3] bpf: Add helpers to issue and check SYN cookies in XDP

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]

Date:   Mon, 24 Jan 2022 23:54:46 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Maxim Mikityanskiy <maximmi@...dia.com>, bpf@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org
Cc:     Tariq Toukan <tariqt@...dia.com>, Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Petar Penkov <ppenkov@...gle.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Shuah Khan <shuah@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Joe Stringer <joe@...ium.io>,
        Florent Revest <revest@...omium.org>,
        linux-kselftest@...r.kernel.org,
        Toke Høiland-Jørgensen <toke@...e.dk>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Florian Westphal <fw@...len.de>,
        Maxim Mikityanskiy <maximmi@...dia.com>
Subject: RE: [PATCH bpf-next v2 2/3] bpf: Add helpers to issue and check SYN
 cookies in XDP

Maxim Mikityanskiy wrote:
> The new helpers bpf_tcp_raw_{gen,check}_syncookie allow an XDP program
> to generate SYN cookies in response to TCP SYN packets and to check
> those cookies upon receiving the first ACK packet (the final packet of
> the TCP handshake).
> 
> Unlike bpf_tcp_{gen,check}_syncookie these new helpers don't need a
> listening socket on the local machine, which allows to use them together
> with synproxy to accelerate SYN cookie generation.
> 
> Signed-off-by: Maxim Mikityanskiy <maximmi@...dia.com>
> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
> ---

[...]

> +
> +BPF_CALL_4(bpf_tcp_raw_check_syncookie, void *, iph, u32, iph_len,
> +	   struct tcphdr *, th, u32, th_len)
> +{
> +#ifdef CONFIG_SYN_COOKIES
> +	u32 cookie;
> +	int ret;
> +
> +	if (unlikely(th_len < sizeof(*th)))
> +		return -EINVAL;
> +
> +	if (!th->ack || th->rst || th->syn)
> +		return -EINVAL;
> +
> +	if (unlikely(iph_len < sizeof(struct iphdr)))
> +		return -EINVAL;
> +
> +	cookie = ntohl(th->ack_seq) - 1;
> +
> +	/* Both struct iphdr and struct ipv6hdr have the version field at the
> +	 * same offset so we can cast to the shorter header (struct iphdr).
> +	 */
> +	switch (((struct iphdr *)iph)->version) {
> +	case 4:

Did you consider just exposing __cookie_v4_check() and __cookie_v6_check()?
My code at least has already run the code above before it would ever call
this helper so all the other bits are duplicate. The only reason to build
it this way, as I see it, is either code can call it blindly without doing 
4/v6 switch. or to make it look and feel like 'tc' world, but its already
dropped the ok so its a bit different already and ifdef TC/XDP could
hanlde the different parts.


> +		ret = __cookie_v4_check((struct iphdr *)iph, th, cookie);
> +		break;
> +
> +#if IS_BUILTIN(CONFIG_IPV6)
> +	case 6:
> +		if (unlikely(iph_len < sizeof(struct ipv6hdr)))
> +			return -EINVAL;
> +
> +		ret = __cookie_v6_check((struct ipv6hdr *)iph, th, cookie);
> +		break;
> +#endif /* CONFIG_IPV6 */
> +
> +	default:
> +		return -EPROTONOSUPPORT;
> +	}
> +
> +	if (ret > 0)
> +		return 0;
> +
> +	return -EACCES;
> +#else
> +	return -EOPNOTSUPP;
> +#endif
> +}