netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies
@ 2019-07-23  0:20 Petar Penkov
  2019-07-23  0:20 ` [bpf-next 1/6] tcp: tcp_syn_flood_action read port from socket Petar Penkov
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

This patch series introduces a BPF helper function that allows generating SYN
cookies from BPF. Currently, this helper is enabled at both the TC hook and the
XDP hook.

The first two patches in the series add/modify several TCP helper functions to
allow for SKB-less operation, as is the case at the XDP hook.

The third patch introduces the bpf_tcp_gen_syncookie helper function which
generates a SYN cookie for either XDP or TC programs. The return value of
this function contains both the MSS value, encoded in the cookie, and the
cookie itself.

The last three patches sync tools/ and add a test. 

Changes since RFC:
1/ Cookie is returned in host order at Alexei's suggestion
2/ If cookies are not enabled via a sysctl, the helper function returns
   -ENOENT instead of -EINVAL at Lorenz's suggestion
3/ Fixed documentation to properly reflect that MSS is 16 bits at
   Lorenz's suggestion
4/ BPF helper requires TCP length to match ->doff field, rather than to simply
   be no more than 20 bytes at Eric and Alexei's suggestion
5/ Packet type is looked up from the packet version field, rather than from the
   socket. v4 packets are rejected on v6-only sockets but should work with
   dual stack listeners at Eric's suggestion
6/ Removed unnecessary `net` argument from helper function in patch 2 at
   Lorenz's suggestion 
7/ Changed test to only pass MSS option so we can convince the verifier that the
   memory access is not out of bounds

Note that 7/ below illustrates the verifier might need to be extended to allow
passing a variable tcph->doff to the helper function like below:

__u32 thlen = tcph->doff * 4;
if (thlen < sizeof(*tcph))
	return;
__s64 cookie = bpf_tcp_gen_syncookie(sk, ipv4h, 20, tcph, thlen);

Petar Penkov (6):
  tcp: tcp_syn_flood_action read port from socket
  tcp: add skb-less helpers to retrieve SYN cookie
  bpf: add bpf_tcp_gen_syncookie helper
  bpf: sync bpf.h to tools/
  selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers
  selftests/bpf: add test for bpf_tcp_gen_syncookie

 include/net/tcp.h                             | 11 +++
 include/uapi/linux/bpf.h                      | 30 ++++++-
 net/core/filter.c                             | 73 ++++++++++++++++
 net/ipv4/tcp_input.c                          | 84 +++++++++++++++++--
 net/ipv4/tcp_ipv4.c                           |  8 ++
 net/ipv6/tcp_ipv6.c                           |  8 ++
 tools/include/uapi/linux/bpf.h                | 37 +++++++-
 tools/testing/selftests/bpf/bpf_helpers.h     |  3 +
 .../bpf/progs/test_tcp_check_syncookie_kern.c | 48 +++++++++--
 .../selftests/bpf/test_tcp_check_syncookie.sh |  3 +
 .../bpf/test_tcp_check_syncookie_user.c       | 61 ++++++++++++--
 11 files changed, 344 insertions(+), 22 deletions(-)

-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [bpf-next 1/6] tcp: tcp_syn_flood_action read port from socket
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-23  0:20 ` [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie Petar Penkov
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

This allows us to call this function before an SKB has been
allocated.

Signed-off-by: Petar Penkov <ppenkov@google.com>
---
 net/ipv4/tcp_input.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c21e8a22fb3b..8892df6de1d4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6422,9 +6422,7 @@ EXPORT_SYMBOL(inet_reqsk_alloc);
 /*
  * Return true if a syncookie should be sent
  */
-static bool tcp_syn_flood_action(const struct sock *sk,
-				 const struct sk_buff *skb,
-				 const char *proto)
+static bool tcp_syn_flood_action(const struct sock *sk, const char *proto)
 {
 	struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
 	const char *msg = "Dropping request";
@@ -6444,7 +6442,7 @@ static bool tcp_syn_flood_action(const struct sock *sk,
 	    net->ipv4.sysctl_tcp_syncookies != 2 &&
 	    xchg(&queue->synflood_warned, 1) == 0)
 		net_info_ratelimited("%s: Possible SYN flooding on port %d. %s.  Check SNMP counters.\n",
-				     proto, ntohs(tcp_hdr(skb)->dest), msg);
+				     proto, sk->sk_num, msg);
 
 	return want_cookie;
 }
@@ -6487,7 +6485,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 	 */
 	if ((net->ipv4.sysctl_tcp_syncookies == 2 ||
 	     inet_csk_reqsk_queue_is_full(sk)) && !isn) {
-		want_cookie = tcp_syn_flood_action(sk, skb, rsk_ops->slab_name);
+		want_cookie = tcp_syn_flood_action(sk, rsk_ops->slab_name);
 		if (!want_cookie)
 			goto drop;
 	}
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
  2019-07-23  0:20 ` [bpf-next 1/6] tcp: tcp_syn_flood_action read port from socket Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-24  6:05   ` kbuild test robot
  2019-07-24  6:19   ` kbuild test robot
  2019-07-23  0:20 ` [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper Petar Penkov
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

This patch allows generation of a SYN cookie before an SKB has been
allocated, as is the case at XDP.

Signed-off-by: Petar Penkov <ppenkov@google.com>
---
 include/net/tcp.h    | 11 +++++++
 net/ipv4/tcp_input.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_ipv4.c  |  8 +++++
 net/ipv6/tcp_ipv6.c  |  8 +++++
 4 files changed, 103 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index cca3c59b98bf..a128e22c0d5d 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -414,6 +414,17 @@ void tcp_parse_options(const struct net *net, const struct sk_buff *skb,
 		       int estab, struct tcp_fastopen_cookie *foc);
 const u8 *tcp_parse_md5sig_option(const struct tcphdr *th);
 
+/*
+ *	BPF SKB-less helpers
+ */
+u16 tcp_v4_get_syncookie(struct sock *sk, struct iphdr *iph,
+			 struct tcphdr *tch, u32 *cookie);
+u16 tcp_v6_get_syncookie(struct sock *sk, struct ipv6hdr *iph,
+			 struct tcphdr *tch, u32 *cookie);
+u16 tcp_get_syncookie(struct request_sock_ops *rsk_ops,
+		      const struct tcp_request_sock_ops *af_ops,
+		      struct sock *sk, void *iph, struct tcphdr *tch,
+		      u32 *cookie);
 /*
  *	TCP v4 functions exported for the inet6 API
  */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8892df6de1d4..893b275a6d49 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3782,6 +3782,49 @@ static void smc_parse_options(const struct tcphdr *th,
 #endif
 }
 
+/* Try to parse the MSS option from the TCP header. Return 0 on failure, clamped
+ * value on success.
+ */
+static u16 tcp_parse_mss_option(const struct tcphdr *th, u16 user_mss)
+{
+	const unsigned char *ptr = (const unsigned char *)(th + 1);
+	int length = (th->doff * 4) - sizeof(struct tcphdr);
+	u16 mss = 0;
+
+	while (length > 0) {
+		int opcode = *ptr++;
+		int opsize;
+
+		switch (opcode) {
+		case TCPOPT_EOL:
+			return mss;
+		case TCPOPT_NOP:	/* Ref: RFC 793 section 3.1 */
+			length--;
+			continue;
+		default:
+			if (length < 2)
+				return mss;
+			opsize = *ptr++;
+			if (opsize < 2) /* "silly options" */
+				return mss;
+			if (opsize > length)
+				return mss;	/* fail on partial options */
+			if (opcode == TCPOPT_MSS && opsize == TCPOLEN_MSS) {
+				u16 in_mss = get_unaligned_be16(ptr);
+
+				if (in_mss) {
+					if (user_mss && user_mss < in_mss)
+						in_mss = user_mss;
+					mss = in_mss;
+				}
+			}
+			ptr += opsize - 2;
+			length -= opsize;
+		}
+	}
+	return mss;
+}
+
 /* Look for tcp options. Normally only called on SYN and SYNACK packets.
  * But, this can also be called on packets in the established flow when
  * the fast version below fails.
@@ -6464,6 +6507,39 @@ static void tcp_reqsk_record_syn(const struct sock *sk,
 	}
 }
 
+u16 tcp_get_syncookie(struct request_sock_ops *rsk_ops,
+		      const struct tcp_request_sock_ops *af_ops,
+		      struct sock *sk, void *iph, struct tcphdr *th,
+		      u32 *cookie)
+{
+	u16 mss = 0;
+#ifdef CONFIG_SYN_COOKIES
+	bool is_v4 = rsk_ops->family == AF_INET;
+	struct tcp_sock *tp = tcp_sk(sk);
+
+	if (sock_net(sk)->ipv4.sysctl_tcp_syncookies != 2 &&
+	    !inet_csk_reqsk_queue_is_full(sk))
+		return 0;
+
+	if (!tcp_syn_flood_action(sk, rsk_ops->slab_name))
+		return 0;
+
+	if (sk_acceptq_is_full(sk)) {
+		NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
+		return 0;
+	}
+
+	mss = tcp_parse_mss_option(th, tp->rx_opt.user_mss);
+	if (!mss)
+		mss = af_ops->mss_clamp;
+
+	tcp_synq_overflow(sk);
+	*cookie = is_v4 ? __cookie_v4_init_sequence(iph, th, &mss)
+			: __cookie_v6_init_sequence(iph, th, &mss);
+#endif
+	return mss;
+}
+
 int tcp_conn_request(struct request_sock_ops *rsk_ops,
 		     const struct tcp_request_sock_ops *af_ops,
 		     struct sock *sk, struct sk_buff *skb)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index d57641cb3477..0e06e59784bd 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1515,6 +1515,14 @@ static struct sock *tcp_v4_cookie_check(struct sock *sk, struct sk_buff *skb)
 	return sk;
 }
 
+u16 tcp_v4_get_syncookie(struct sock *sk, struct iphdr *iph,
+			 struct tcphdr *tch, u32 *cookie)
+{
+	return tcp_get_syncookie(&tcp_request_sock_ops,
+				 &tcp_request_sock_ipv4_ops, sk, iph, tch,
+				 cookie);
+}
+
 /* The socket must have it's spinlock held when we get
  * here, unless it is a TCP_LISTEN socket.
  *
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 5da069e91cac..102f68c3152d 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1063,6 +1063,14 @@ static struct sock *tcp_v6_cookie_check(struct sock *sk, struct sk_buff *skb)
 	return sk;
 }
 
+u16 tcp_v6_get_syncookie(struct sock *sk, struct ipv6hdr *iph,
+			 struct tcphdr *tch, u32 *cookie)
+{
+	return tcp_get_syncookie(&tcp6_request_sock_ops,
+				 &tcp_request_sock_ipv6_ops, sk, iph, tch,
+				 cookie);
+}
+
 static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 {
 	if (skb->protocol == htons(ETH_P_IP))
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
  2019-07-23  0:20 ` [bpf-next 1/6] tcp: tcp_syn_flood_action read port from socket Petar Penkov
  2019-07-23  0:20 ` [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-23 12:33   ` Toke Høiland-Jørgensen
  2019-07-23  0:20 ` [bpf-next 4/6] bpf: sync bpf.h to tools/ Petar Penkov
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

This helper function allows BPF programs to try to generate SYN
cookies, given a reference to a listener socket. The function works
from XDP and with an skb context since bpf_skc_lookup_tcp can lookup a
socket in both cases.

Signed-off-by: Petar Penkov <ppenkov@google.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
---
 include/uapi/linux/bpf.h | 30 ++++++++++++++++-
 net/core/filter.c        | 73 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6f68438aa4ed..20baee7b2219 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2713,6 +2713,33 @@ union bpf_attr {
  *		**-EPERM** if no permission to send the *sig*.
  *
  *		**-EAGAIN** if bpf program can try again.
+ *
+ * s64 bpf_tcp_gen_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
+ *	Description
+ *		Try to issue a SYN cookie for the packet with corresponding
+ *		IP/TCP headers, *iph* and *th*, on the listening socket in *sk*.
+ *
+ *		*iph* points to the start of the IPv4 or IPv6 header, while
+ *		*iph_len* contains **sizeof**\ (**struct iphdr**) or
+ *		**sizeof**\ (**struct ip6hdr**).
+ *
+ *		*th* points to the start of the TCP header, while *th_len*
+ *		contains the length of the TCP header.
+ *
+ *	Return
+ *		On success, lower 32 bits hold the generated SYN cookie in
+ *		followed by 16 bits which hold the MSS value for that cookie,
+ *		and the top 16 bits are unused.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EINVAL** SYN cookie cannot be issued due to error
+ *
+ *		**-ENOENT** SYN cookie should not be issued (no SYN flood)
+ *
+ *		**-ENOTSUPP** kernel configuration does not enable SYN cookies
+ *
+ *		**-EPROTONOSUPPORT** IP packet version is not 4 or 6
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2824,7 +2851,8 @@ union bpf_attr {
 	FN(strtoul),			\
 	FN(sk_storage_get),		\
 	FN(sk_storage_delete),		\
-	FN(send_signal),
+	FN(send_signal),		\
+	FN(tcp_gen_syncookie),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 47f6386fb17a..92114271eff6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5850,6 +5850,75 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = {
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
+BPF_CALL_5(bpf_tcp_gen_syncookie, struct sock *, sk, void *, iph, u32, iph_len,
+	   struct tcphdr *, th, u32, th_len)
+{
+#ifdef CONFIG_SYN_COOKIES
+	u32 cookie;
+	u16 mss;
+
+	if (unlikely(th_len < sizeof(*th) || th_len != th->doff * 4))
+		return -EINVAL;
+
+	if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
+		return -EINVAL;
+
+	if (!sock_net(sk)->ipv4.sysctl_tcp_syncookies)
+		return -ENOENT;
+
+	if (!th->syn || th->ack || th->fin || th->rst)
+		return -EINVAL;
+
+	if (unlikely(iph_len < sizeof(struct iphdr)))
+		return -EINVAL;
+
+	/* 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:
+		if (sk->sk_family == AF_INET6 && sk->sk_ipv6only)
+			return -EINVAL;
+
+		mss = tcp_v4_get_syncookie(sk, iph, th, &cookie);
+		break;
+
+#if IS_BUILTIN(CONFIG_IPV6)
+	case 6:
+		if (unlikely(iph_len < sizeof(struct ipv6hdr)))
+			return -EINVAL;
+
+		if (sk->sk_family != AF_INET6)
+			return -EINVAL;
+
+		mss = tcp_v6_get_syncookie(sk, iph, th, &cookie);
+		break;
+#endif /* CONFIG_IPV6 */
+
+	default:
+		return -EPROTONOSUPPORT;
+	}
+	if (mss <= 0)
+		return -ENOENT;
+
+	return cookie | ((u64)mss << 32);
+#else
+	return -ENOTSUPP;
+#endif /* CONFIG_SYN_COOKIES */
+}
+
+static const struct bpf_func_proto bpf_tcp_gen_syncookie_proto = {
+	.func		= bpf_tcp_gen_syncookie,
+	.gpl_only	= true, /* __cookie_v*_init_sequence() is GPL */
+	.pkt_access	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg5_type	= ARG_CONST_SIZE,
+};
+
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -6135,6 +6204,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_tcp_check_syncookie_proto;
 	case BPF_FUNC_skb_ecn_set_ce:
 		return &bpf_skb_ecn_set_ce_proto;
+	case BPF_FUNC_tcp_gen_syncookie:
+		return &bpf_tcp_gen_syncookie_proto;
 #endif
 	default:
 		return bpf_base_func_proto(func_id);
@@ -6174,6 +6245,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_xdp_skc_lookup_tcp_proto;
 	case BPF_FUNC_tcp_check_syncookie:
 		return &bpf_tcp_check_syncookie_proto;
+	case BPF_FUNC_tcp_gen_syncookie:
+		return &bpf_tcp_gen_syncookie_proto;
 #endif
 	default:
 		return bpf_base_func_proto(func_id);
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [bpf-next 4/6] bpf: sync bpf.h to tools/
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
                   ` (2 preceding siblings ...)
  2019-07-23  0:20 ` [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-23  0:20 ` [bpf-next 5/6] selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers Petar Penkov
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

Sync updated documentation for bpf_redirect_map.

Sync the bpf_tcp_gen_syncookie helper function definition with the one
in tools/uapi.

Signed-off-by: Petar Penkov <ppenkov@google.com>
---
 tools/include/uapi/linux/bpf.h | 37 +++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index f506c68b2612..20baee7b2219 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1571,8 +1571,11 @@ union bpf_attr {
  * 		but this is only implemented for native XDP (with driver
  * 		support) as of this writing).
  *
- * 		All values for *flags* are reserved for future usage, and must
- * 		be left at zero.
+ * 		The lower two bits of *flags* are used as the return code if
+ * 		the map lookup fails. This is so that the return value can be
+ * 		one of the XDP program return codes up to XDP_TX, as chosen by
+ * 		the caller. Any higher bits in the *flags* argument must be
+ * 		unset.
  *
  * 		When used to redirect packets to net devices, this helper
  * 		provides a high performance increase over **bpf_redirect**\ ().
@@ -2710,6 +2713,33 @@ union bpf_attr {
  *		**-EPERM** if no permission to send the *sig*.
  *
  *		**-EAGAIN** if bpf program can try again.
+ *
+ * s64 bpf_tcp_gen_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
+ *	Description
+ *		Try to issue a SYN cookie for the packet with corresponding
+ *		IP/TCP headers, *iph* and *th*, on the listening socket in *sk*.
+ *
+ *		*iph* points to the start of the IPv4 or IPv6 header, while
+ *		*iph_len* contains **sizeof**\ (**struct iphdr**) or
+ *		**sizeof**\ (**struct ip6hdr**).
+ *
+ *		*th* points to the start of the TCP header, while *th_len*
+ *		contains the length of the TCP header.
+ *
+ *	Return
+ *		On success, lower 32 bits hold the generated SYN cookie in
+ *		followed by 16 bits which hold the MSS value for that cookie,
+ *		and the top 16 bits are unused.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EINVAL** SYN cookie cannot be issued due to error
+ *
+ *		**-ENOENT** SYN cookie should not be issued (no SYN flood)
+ *
+ *		**-ENOTSUPP** kernel configuration does not enable SYN cookies
+ *
+ *		**-EPROTONOSUPPORT** IP packet version is not 4 or 6
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2821,7 +2851,8 @@ union bpf_attr {
 	FN(strtoul),			\
 	FN(sk_storage_get),		\
 	FN(sk_storage_delete),		\
-	FN(send_signal),
+	FN(send_signal),		\
+	FN(tcp_gen_syncookie),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [bpf-next 5/6] selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
                   ` (3 preceding siblings ...)
  2019-07-23  0:20 ` [bpf-next 4/6] bpf: sync bpf.h to tools/ Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-23  0:20 ` [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie Petar Penkov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

Expose bpf_tcp_gen_syncookie to selftests.

Signed-off-by: Petar Penkov <ppenkov@google.com>
---
 tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5a3d92c8bec8..19f01e967402 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -228,6 +228,9 @@ static void *(*bpf_sk_storage_get)(void *map, struct bpf_sock *sk,
 static int (*bpf_sk_storage_delete)(void *map, struct bpf_sock *sk) =
 	(void *)BPF_FUNC_sk_storage_delete;
 static int (*bpf_send_signal)(unsigned sig) = (void *)BPF_FUNC_send_signal;
+static long long (*bpf_tcp_gen_syncookie)(struct bpf_sock *sk, void *ip,
+					  int ip_len, void *tcp, int tcp_len) =
+	(void *) BPF_FUNC_tcp_gen_syncookie;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
                   ` (4 preceding siblings ...)
  2019-07-23  0:20 ` [bpf-next 5/6] selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers Petar Penkov
@ 2019-07-23  0:20 ` Petar Penkov
  2019-07-23  9:37   ` Lorenz Bauer
  2019-07-23  6:30 ` [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Eric Dumazet
  2019-07-23 10:27 ` Lorenz Bauer
  7 siblings, 1 reply; 15+ messages in thread
From: Petar Penkov @ 2019-07-23  0:20 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

From: Petar Penkov <ppenkov@google.com>

Modify the existing bpf_tcp_check_syncookie test to also generate a
SYN cookie, pass the packet to the kernel, and verify that the two
cookies are the same (and both valid). Since cloned SKBs are skipped
during generic XDP, this test does not issue a SYN cookie when run in
XDP mode. We therefore only check that a valid SYN cookie was issued at
the TC hook.

Additionally, verify that the MSS for that SYN cookie is within
expected range.

Signed-off-by: Petar Penkov <ppenkov@google.com>
---
 .../bpf/progs/test_tcp_check_syncookie_kern.c | 48 +++++++++++++--
 .../selftests/bpf/test_tcp_check_syncookie.sh |  3 +
 .../bpf/test_tcp_check_syncookie_user.c       | 61 ++++++++++++++++---
 3 files changed, 99 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c b/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c
index 1ab095bcacd8..d8803dfa8d32 100644
--- a/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c
@@ -19,10 +19,29 @@
 struct bpf_map_def SEC("maps") results = {
 	.type = BPF_MAP_TYPE_ARRAY,
 	.key_size = sizeof(__u32),
-	.value_size = sizeof(__u64),
-	.max_entries = 1,
+	.value_size = sizeof(__u32),
+	.max_entries = 3,
 };
 
+static __always_inline __s64 gen_syncookie(void *data_end, struct bpf_sock *sk,
+					   void *iph, __u32 ip_size,
+					   struct tcphdr *tcph)
+{
+	__u32 thlen = tcph->doff * 4;
+
+	if (tcph->syn && !tcph->ack) {
+		// packet should only have an MSS option
+		if (thlen != 24)
+			return 0;
+
+		if ((void *)tcph + thlen > data_end)
+			return 0;
+
+		return bpf_tcp_gen_syncookie(sk, iph, ip_size, tcph, thlen);
+	}
+	return 0;
+}
+
 static __always_inline void check_syncookie(void *ctx, void *data,
 					    void *data_end)
 {
@@ -33,8 +52,10 @@ static __always_inline void check_syncookie(void *ctx, void *data,
 	struct ipv6hdr *ipv6h;
 	struct tcphdr *tcph;
 	int ret;
+	__u32 key_mss = 2;
+	__u32 key_gen = 1;
 	__u32 key = 0;
-	__u64 value = 1;
+	__s64 seq_mss;
 
 	ethh = data;
 	if (ethh + 1 > data_end)
@@ -66,6 +87,9 @@ static __always_inline void check_syncookie(void *ctx, void *data,
 		if (sk->state != BPF_TCP_LISTEN)
 			goto release;
 
+		seq_mss = gen_syncookie(data_end, sk, ipv4h, sizeof(*ipv4h),
+					tcph);
+
 		ret = bpf_tcp_check_syncookie(sk, ipv4h, sizeof(*ipv4h),
 					      tcph, sizeof(*tcph));
 		break;
@@ -95,6 +119,9 @@ static __always_inline void check_syncookie(void *ctx, void *data,
 		if (sk->state != BPF_TCP_LISTEN)
 			goto release;
 
+		seq_mss = gen_syncookie(data_end, sk, ipv6h, sizeof(*ipv6h),
+					tcph);
+
 		ret = bpf_tcp_check_syncookie(sk, ipv6h, sizeof(*ipv6h),
 					      tcph, sizeof(*tcph));
 		break;
@@ -103,8 +130,19 @@ static __always_inline void check_syncookie(void *ctx, void *data,
 		return;
 	}
 
-	if (ret == 0)
-		bpf_map_update_elem(&results, &key, &value, 0);
+	if (seq_mss > 0) {
+		__u32 cookie = (__u32)seq_mss;
+		__u32 mss = seq_mss >> 32;
+
+		bpf_map_update_elem(&results, &key_gen, &cookie, 0);
+		bpf_map_update_elem(&results, &key_mss, &mss, 0);
+	}
+
+	if (ret == 0) {
+		__u32 cookie = bpf_ntohl(tcph->ack_seq) - 1;
+
+		bpf_map_update_elem(&results, &key, &cookie, 0);
+	}
 
 release:
 	bpf_sk_release(sk);
diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh b/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh
index d48e51716d19..9b3617d770a5 100755
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh
@@ -37,6 +37,9 @@ setup()
 	ns1_exec ip link set lo up
 
 	ns1_exec sysctl -w net.ipv4.tcp_syncookies=2
+	ns1_exec sysctl -w net.ipv4.tcp_window_scaling=0
+	ns1_exec sysctl -w net.ipv4.tcp_timestamps=0
+	ns1_exec sysctl -w net.ipv4.tcp_sack=0
 
 	wait_for_ip 127.0.0.1
 	wait_for_ip ::1
diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
index 87829c86c746..b9e991d43155 100644
--- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
+++ b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2018 Facebook
 // Copyright (c) 2019 Cloudflare
 
+#include <limits.h>
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -77,7 +78,7 @@ static int connect_to_server(int server_fd)
 	return fd;
 }
 
-static int get_map_fd_by_prog_id(int prog_id)
+static int get_map_fd_by_prog_id(int prog_id, bool *xdp)
 {
 	struct bpf_prog_info info = {};
 	__u32 info_len = sizeof(info);
@@ -104,6 +105,8 @@ static int get_map_fd_by_prog_id(int prog_id)
 		goto err;
 	}
 
+	*xdp = info.type == BPF_PROG_TYPE_XDP;
+
 	map_fd = bpf_map_get_fd_by_id(map_ids[0]);
 	if (map_fd < 0)
 		log_err("Failed to get fd by map id %d", map_ids[0]);
@@ -113,18 +116,32 @@ static int get_map_fd_by_prog_id(int prog_id)
 	return map_fd;
 }
 
-static int run_test(int server_fd, int results_fd)
+static int run_test(int server_fd, int results_fd, bool xdp)
 {
 	int client = -1, srv_client = -1;
 	int ret = 0;
 	__u32 key = 0;
-	__u64 value = 0;
+	__u32 key_gen = 1;
+	__u32 key_mss = 2;
+	__u32 value = 0;
+	__u32 value_gen = 0;
+	__u32 value_mss = 0;
 
 	if (bpf_map_update_elem(results_fd, &key, &value, 0) < 0) {
 		log_err("Can't clear results");
 		goto err;
 	}
 
+	if (bpf_map_update_elem(results_fd, &key_gen, &value_gen, 0) < 0) {
+		log_err("Can't clear results");
+		goto err;
+	}
+
+	if (bpf_map_update_elem(results_fd, &key_mss, &value_mss, 0) < 0) {
+		log_err("Can't clear results");
+		goto err;
+	}
+
 	client = connect_to_server(server_fd);
 	if (client == -1)
 		goto err;
@@ -140,8 +157,35 @@ static int run_test(int server_fd, int results_fd)
 		goto err;
 	}
 
-	if (value != 1) {
-		log_err("Didn't match syncookie: %llu", value);
+	if (value == 0) {
+		log_err("Didn't match syncookie: %u", value);
+		goto err;
+	}
+
+	if (bpf_map_lookup_elem(results_fd, &key_gen, &value_gen) < 0) {
+		log_err("Can't lookup result");
+		goto err;
+	}
+
+	if (xdp && value_gen == 0) {
+		// SYN packets do not get passed through generic XDP, skip the
+		// rest of the test.
+		printf("Skipping XDP cookie check\n");
+		goto out;
+	}
+
+	if (bpf_map_lookup_elem(results_fd, &key_mss, &value_mss) < 0) {
+		log_err("Can't lookup result");
+		goto err;
+	}
+
+	if (value != value_gen) {
+		log_err("BPF generated cookie does not match kernel one");
+		goto err;
+	}
+
+	if (value_mss < 536 || value_mss > USHRT_MAX) {
+		log_err("Unexpected MSS retrieved");
 		goto err;
 	}
 
@@ -163,13 +207,14 @@ int main(int argc, char **argv)
 	int server_v6 = -1;
 	int results = -1;
 	int err = 0;
+	bool xdp;
 
 	if (argc < 2) {
 		fprintf(stderr, "Usage: %s prog_id\n", argv[0]);
 		exit(1);
 	}
 
-	results = get_map_fd_by_prog_id(atoi(argv[1]));
+	results = get_map_fd_by_prog_id(atoi(argv[1]), &xdp);
 	if (results < 0) {
 		log_err("Can't get map");
 		goto err;
@@ -194,10 +239,10 @@ int main(int argc, char **argv)
 	if (server_v6 == -1)
 		goto err;
 
-	if (run_test(server, results))
+	if (run_test(server, results, xdp))
 		goto err;
 
-	if (run_test(server_v6, results))
+	if (run_test(server_v6, results, xdp))
 		goto err;
 
 	printf("ok\n");
-- 
2.22.0.657.g960e92d24f-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
                   ` (5 preceding siblings ...)
  2019-07-23  0:20 ` [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie Petar Penkov
@ 2019-07-23  6:30 ` Eric Dumazet
  2019-07-23 10:27 ` Lorenz Bauer
  7 siblings, 0 replies; 15+ messages in thread
From: Eric Dumazet @ 2019-07-23  6:30 UTC (permalink / raw)
  To: Petar Penkov
  Cc: netdev, bpf, David Miller, Alexei Starovoitov, Daniel Borkmann,
	lmb, Stanislav Fomichev, Petar Penkov

On Tue, Jul 23, 2019 at 2:20 AM Petar Penkov <ppenkov.kernel@gmail.com> wrote:
>
> From: Petar Penkov <ppenkov@google.com>
>
> This patch series introduces a BPF helper function that allows generating SYN
> cookies from BPF. Currently, this helper is enabled at both the TC hook and the
> XDP hook.


Please provide performance numbers ?

We want to know if we increase performance under synflood, or if it
does not change, or even decrease ;)

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie
  2019-07-23  0:20 ` [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie Petar Penkov
@ 2019-07-23  9:37   ` Lorenz Bauer
  2019-07-23 20:46     ` Alexei Starovoitov
  0 siblings, 1 reply; 15+ messages in thread
From: Lorenz Bauer @ 2019-07-23  9:37 UTC (permalink / raw)
  To: Petar Penkov
  Cc: Networking, bpf, davem, Alexei Starovoitov, Daniel Borkmann,
	Eric Dumazet, Stanislav Fomichev, Petar Penkov

On Tue, 23 Jul 2019 at 01:20, Petar Penkov <ppenkov.kernel@gmail.com> wrote:
> +static __always_inline __s64 gen_syncookie(void *data_end, struct bpf_sock *sk,
> +                                          void *iph, __u32 ip_size,
> +                                          struct tcphdr *tcph)
> +{
> +       __u32 thlen = tcph->doff * 4;
> +
> +       if (tcph->syn && !tcph->ack) {
> +               // packet should only have an MSS option
> +               if (thlen != 24)
> +                       return 0;

Just for my own understanding: without this the verifier complains since
thlen is not a known value, even though it is in bounds due to the check below?

> +
> +               if ((void *)tcph + thlen > data_end)
> +                       return 0;
> +
> +               return bpf_tcp_gen_syncookie(sk, iph, ip_size, tcph, thlen);
> +       }
> +       return 0;
> +}
> +

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies
  2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
                   ` (6 preceding siblings ...)
  2019-07-23  6:30 ` [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Eric Dumazet
@ 2019-07-23 10:27 ` Lorenz Bauer
  7 siblings, 0 replies; 15+ messages in thread
From: Lorenz Bauer @ 2019-07-23 10:27 UTC (permalink / raw)
  To: Petar Penkov
  Cc: Networking, bpf, davem, Alexei Starovoitov, Daniel Borkmann,
	Eric Dumazet, Stanislav Fomichev, Petar Penkov

On Tue, 23 Jul 2019 at 01:20, Petar Penkov <ppenkov.kernel@gmail.com> wrote:
>
> From: Petar Penkov <ppenkov@google.com>
>
> This patch series introduces a BPF helper function that allows generating SYN
> cookies from BPF. Currently, this helper is enabled at both the TC hook and the
> XDP hook.
>
> The first two patches in the series add/modify several TCP helper functions to
> allow for SKB-less operation, as is the case at the XDP hook.
>
> The third patch introduces the bpf_tcp_gen_syncookie helper function which
> generates a SYN cookie for either XDP or TC programs. The return value of
> this function contains both the MSS value, encoded in the cookie, and the
> cookie itself.
>
> The last three patches sync tools/ and add a test.

Reviewed-by: Lorenz Bauer <lmb@cloudflare.com>

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper
  2019-07-23  0:20 ` [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper Petar Penkov
@ 2019-07-23 12:33   ` Toke Høiland-Jørgensen
  2019-07-24  0:15     ` Petar Penkov
  0 siblings, 1 reply; 15+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-07-23 12:33 UTC (permalink / raw)
  To: Petar Penkov, netdev, bpf
  Cc: davem, ast, daniel, edumazet, lmb, sdf, Petar Penkov

Petar Penkov <ppenkov.kernel@gmail.com> writes:

> From: Petar Penkov <ppenkov@google.com>
>
> This helper function allows BPF programs to try to generate SYN
> cookies, given a reference to a listener socket. The function works
> from XDP and with an skb context since bpf_skc_lookup_tcp can lookup a
> socket in both cases.
>
> Signed-off-by: Petar Penkov <ppenkov@google.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/uapi/linux/bpf.h | 30 ++++++++++++++++-
>  net/core/filter.c        | 73 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 6f68438aa4ed..20baee7b2219 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2713,6 +2713,33 @@ union bpf_attr {
>   *		**-EPERM** if no permission to send the *sig*.
>   *
>   *		**-EAGAIN** if bpf program can try again.
> + *
> + * s64 bpf_tcp_gen_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
> + *	Description
> + *		Try to issue a SYN cookie for the packet with corresponding
> + *		IP/TCP headers, *iph* and *th*, on the listening socket in *sk*.
> + *
> + *		*iph* points to the start of the IPv4 or IPv6 header, while
> + *		*iph_len* contains **sizeof**\ (**struct iphdr**) or
> + *		**sizeof**\ (**struct ip6hdr**).
> + *
> + *		*th* points to the start of the TCP header, while *th_len*
> + *		contains the length of the TCP header.
> + *
> + *	Return
> + *		On success, lower 32 bits hold the generated SYN cookie in
> + *		followed by 16 bits which hold the MSS value for that cookie,
> + *		and the top 16 bits are unused.
> + *
> + *		On failure, the returned value is one of the following:
> + *
> + *		**-EINVAL** SYN cookie cannot be issued due to error
> + *
> + *		**-ENOENT** SYN cookie should not be issued (no SYN flood)
> + *
> + *		**-ENOTSUPP** kernel configuration does not enable SYN
> cookies

nit: This should be EOPNOTSUPP - the other one is for NFS...

> + *
> + *		**-EPROTONOSUPPORT** IP packet version is not 4 or 6
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2824,7 +2851,8 @@ union bpf_attr {
>  	FN(strtoul),			\
>  	FN(sk_storage_get),		\
>  	FN(sk_storage_delete),		\
> -	FN(send_signal),
> +	FN(send_signal),		\
> +	FN(tcp_gen_syncookie),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 47f6386fb17a..92114271eff6 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5850,6 +5850,75 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = {
>  	.arg5_type	= ARG_CONST_SIZE,
>  };
>  
> +BPF_CALL_5(bpf_tcp_gen_syncookie, struct sock *, sk, void *, iph, u32, iph_len,
> +	   struct tcphdr *, th, u32, th_len)
> +{
> +#ifdef CONFIG_SYN_COOKIES
> +	u32 cookie;
> +	u16 mss;
> +
> +	if (unlikely(th_len < sizeof(*th) || th_len != th->doff * 4))
> +		return -EINVAL;
> +
> +	if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
> +		return -EINVAL;
> +
> +	if (!sock_net(sk)->ipv4.sysctl_tcp_syncookies)
> +		return -ENOENT;
> +
> +	if (!th->syn || th->ack || th->fin || th->rst)
> +		return -EINVAL;
> +
> +	if (unlikely(iph_len < sizeof(struct iphdr)))
> +		return -EINVAL;
> +
> +	/* 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:
> +		if (sk->sk_family == AF_INET6 && sk->sk_ipv6only)
> +			return -EINVAL;
> +
> +		mss = tcp_v4_get_syncookie(sk, iph, th, &cookie);
> +		break;
> +
> +#if IS_BUILTIN(CONFIG_IPV6)
> +	case 6:
> +		if (unlikely(iph_len < sizeof(struct ipv6hdr)))
> +			return -EINVAL;
> +
> +		if (sk->sk_family != AF_INET6)
> +			return -EINVAL;
> +
> +		mss = tcp_v6_get_syncookie(sk, iph, th, &cookie);
> +		break;
> +#endif /* CONFIG_IPV6 */
> +
> +	default:
> +		return -EPROTONOSUPPORT;
> +	}
> +	if (mss <= 0)
> +		return -ENOENT;
> +
> +	return cookie | ((u64)mss << 32);
> +#else
> +	return -ENOTSUPP;

See above

> +#endif /* CONFIG_SYN_COOKIES */
> +}
> +
> +static const struct bpf_func_proto bpf_tcp_gen_syncookie_proto = {
> +	.func		= bpf_tcp_gen_syncookie,
> +	.gpl_only	= true, /* __cookie_v*_init_sequence() is GPL */
> +	.pkt_access	= true,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
> +	.arg2_type	= ARG_PTR_TO_MEM,
> +	.arg3_type	= ARG_CONST_SIZE,
> +	.arg4_type	= ARG_PTR_TO_MEM,
> +	.arg5_type	= ARG_CONST_SIZE,
> +};
> +
>  #endif /* CONFIG_INET */
>  
>  bool bpf_helper_changes_pkt_data(void *func)
> @@ -6135,6 +6204,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_tcp_check_syncookie_proto;
>  	case BPF_FUNC_skb_ecn_set_ce:
>  		return &bpf_skb_ecn_set_ce_proto;
> +	case BPF_FUNC_tcp_gen_syncookie:
> +		return &bpf_tcp_gen_syncookie_proto;
>  #endif
>  	default:
>  		return bpf_base_func_proto(func_id);
> @@ -6174,6 +6245,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_xdp_skc_lookup_tcp_proto;
>  	case BPF_FUNC_tcp_check_syncookie:
>  		return &bpf_tcp_check_syncookie_proto;
> +	case BPF_FUNC_tcp_gen_syncookie:
> +		return &bpf_tcp_gen_syncookie_proto;
>  #endif
>  	default:
>  		return bpf_base_func_proto(func_id);
> -- 
> 2.22.0.657.g960e92d24f-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie
  2019-07-23  9:37   ` Lorenz Bauer
@ 2019-07-23 20:46     ` Alexei Starovoitov
  0 siblings, 0 replies; 15+ messages in thread
From: Alexei Starovoitov @ 2019-07-23 20:46 UTC (permalink / raw)
  To: Lorenz Bauer
  Cc: Petar Penkov, Networking, bpf, davem, Alexei Starovoitov,
	Daniel Borkmann, Eric Dumazet, Stanislav Fomichev, Petar Penkov,
	yhs

On Tue, Jul 23, 2019 at 10:37:29AM +0100, Lorenz Bauer wrote:
> On Tue, 23 Jul 2019 at 01:20, Petar Penkov <ppenkov.kernel@gmail.com> wrote:
> > +static __always_inline __s64 gen_syncookie(void *data_end, struct bpf_sock *sk,
> > +                                          void *iph, __u32 ip_size,
> > +                                          struct tcphdr *tcph)
> > +{
> > +       __u32 thlen = tcph->doff * 4;
> > +
> > +       if (tcph->syn && !tcph->ack) {
> > +               // packet should only have an MSS option
> > +               if (thlen != 24)
> > +                       return 0;
> 
> Just for my own understanding: without this the verifier complains since
> thlen is not a known value, even though it is in bounds due to the check below?

the verifier understands only constant part of the packet pointer.
Without additional 'if' above the statement:
if ((void *)tcph + thlen > data_end)
will add variables length 'thlen' to pkt pointer which will become
another pkt pointer (with different id).
That pointer would need 'pkt + const_range > data_end' to have valid access.

We hit this issue in the past when folks wanted to use bpf_csum_diff() helper
with variable size.
It's possible to extend the verifier to support that but it's intrusive,
since variable part would need to passed around to a bunch of check* functions.
I think it's tricky, but doable. Looking forward to patches :)

> > +
> > +               if ((void *)tcph + thlen > data_end)
> > +                       return 0;
> > +
> > +               return bpf_tcp_gen_syncookie(sk, iph, ip_size, tcph, thlen);
> > +       }
> > +       return 0;
> > +}
> > +
> 
> -- 
> Lorenz Bauer  |  Systems Engineer
> 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
> 
> www.cloudflare.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper
  2019-07-23 12:33   ` Toke Høiland-Jørgensen
@ 2019-07-24  0:15     ` Petar Penkov
  0 siblings, 0 replies; 15+ messages in thread
From: Petar Penkov @ 2019-07-24  0:15 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Petar Penkov, Networking, bpf, David S . Miller,
	Alexei Starovoitov, Daniel Borkmann, Eric Dumazet, lmb,
	Stanislav Fomichev

On Tue, Jul 23, 2019 at 5:33 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Petar Penkov <ppenkov.kernel@gmail.com> writes:
>
> > From: Petar Penkov <ppenkov@google.com>
> >
> > This helper function allows BPF programs to try to generate SYN
> > cookies, given a reference to a listener socket. The function works
> > from XDP and with an skb context since bpf_skc_lookup_tcp can lookup a
> > socket in both cases.
> >
> > Signed-off-by: Petar Penkov <ppenkov@google.com>
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > ---
> >  include/uapi/linux/bpf.h | 30 ++++++++++++++++-
> >  net/core/filter.c        | 73 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 6f68438aa4ed..20baee7b2219 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2713,6 +2713,33 @@ union bpf_attr {
> >   *           **-EPERM** if no permission to send the *sig*.
> >   *
> >   *           **-EAGAIN** if bpf program can try again.
> > + *
> > + * s64 bpf_tcp_gen_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
> > + *   Description
> > + *           Try to issue a SYN cookie for the packet with corresponding
> > + *           IP/TCP headers, *iph* and *th*, on the listening socket in *sk*.
> > + *
> > + *           *iph* points to the start of the IPv4 or IPv6 header, while
> > + *           *iph_len* contains **sizeof**\ (**struct iphdr**) or
> > + *           **sizeof**\ (**struct ip6hdr**).
> > + *
> > + *           *th* points to the start of the TCP header, while *th_len*
> > + *           contains the length of the TCP header.
> > + *
> > + *   Return
> > + *           On success, lower 32 bits hold the generated SYN cookie in
> > + *           followed by 16 bits which hold the MSS value for that cookie,
> > + *           and the top 16 bits are unused.
> > + *
> > + *           On failure, the returned value is one of the following:
> > + *
> > + *           **-EINVAL** SYN cookie cannot be issued due to error
> > + *
> > + *           **-ENOENT** SYN cookie should not be issued (no SYN flood)
> > + *
> > + *           **-ENOTSUPP** kernel configuration does not enable SYN
> > cookies
>
> nit: This should be EOPNOTSUPP - the other one is for NFS...
Will correct this in a v2, thanks for catching that!

>
> > + *
> > + *           **-EPROTONOSUPPORT** IP packet version is not 4 or 6
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)                \
> >       FN(unspec),                     \
> > @@ -2824,7 +2851,8 @@ union bpf_attr {
> >       FN(strtoul),                    \
> >       FN(sk_storage_get),             \
> >       FN(sk_storage_delete),          \
> > -     FN(send_signal),
> > +     FN(send_signal),                \
> > +     FN(tcp_gen_syncookie),
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 47f6386fb17a..92114271eff6 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5850,6 +5850,75 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = {
> >       .arg5_type      = ARG_CONST_SIZE,
> >  };
> >
> > +BPF_CALL_5(bpf_tcp_gen_syncookie, struct sock *, sk, void *, iph, u32, iph_len,
> > +        struct tcphdr *, th, u32, th_len)
> > +{
> > +#ifdef CONFIG_SYN_COOKIES
> > +     u32 cookie;
> > +     u16 mss;
> > +
> > +     if (unlikely(th_len < sizeof(*th) || th_len != th->doff * 4))
> > +             return -EINVAL;
> > +
> > +     if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
> > +             return -EINVAL;
> > +
> > +     if (!sock_net(sk)->ipv4.sysctl_tcp_syncookies)
> > +             return -ENOENT;
> > +
> > +     if (!th->syn || th->ack || th->fin || th->rst)
> > +             return -EINVAL;
> > +
> > +     if (unlikely(iph_len < sizeof(struct iphdr)))
> > +             return -EINVAL;
> > +
> > +     /* 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:
> > +             if (sk->sk_family == AF_INET6 && sk->sk_ipv6only)
> > +                     return -EINVAL;
> > +
> > +             mss = tcp_v4_get_syncookie(sk, iph, th, &cookie);
> > +             break;
> > +
> > +#if IS_BUILTIN(CONFIG_IPV6)
> > +     case 6:
> > +             if (unlikely(iph_len < sizeof(struct ipv6hdr)))
> > +                     return -EINVAL;
> > +
> > +             if (sk->sk_family != AF_INET6)
> > +                     return -EINVAL;
> > +
> > +             mss = tcp_v6_get_syncookie(sk, iph, th, &cookie);
> > +             break;
> > +#endif /* CONFIG_IPV6 */
> > +
> > +     default:
> > +             return -EPROTONOSUPPORT;
> > +     }
> > +     if (mss <= 0)
> > +             return -ENOENT;
> > +
> > +     return cookie | ((u64)mss << 32);
> > +#else
> > +     return -ENOTSUPP;
>
> See above
>
> > +#endif /* CONFIG_SYN_COOKIES */
> > +}
> > +
> > +static const struct bpf_func_proto bpf_tcp_gen_syncookie_proto = {
> > +     .func           = bpf_tcp_gen_syncookie,
> > +     .gpl_only       = true, /* __cookie_v*_init_sequence() is GPL */
> > +     .pkt_access     = true,
> > +     .ret_type       = RET_INTEGER,
> > +     .arg1_type      = ARG_PTR_TO_SOCK_COMMON,
> > +     .arg2_type      = ARG_PTR_TO_MEM,
> > +     .arg3_type      = ARG_CONST_SIZE,
> > +     .arg4_type      = ARG_PTR_TO_MEM,
> > +     .arg5_type      = ARG_CONST_SIZE,
> > +};
> > +
> >  #endif /* CONFIG_INET */
> >
> >  bool bpf_helper_changes_pkt_data(void *func)
> > @@ -6135,6 +6204,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >               return &bpf_tcp_check_syncookie_proto;
> >       case BPF_FUNC_skb_ecn_set_ce:
> >               return &bpf_skb_ecn_set_ce_proto;
> > +     case BPF_FUNC_tcp_gen_syncookie:
> > +             return &bpf_tcp_gen_syncookie_proto;
> >  #endif
> >       default:
> >               return bpf_base_func_proto(func_id);
> > @@ -6174,6 +6245,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >               return &bpf_xdp_skc_lookup_tcp_proto;
> >       case BPF_FUNC_tcp_check_syncookie:
> >               return &bpf_tcp_check_syncookie_proto;
> > +     case BPF_FUNC_tcp_gen_syncookie:
> > +             return &bpf_tcp_gen_syncookie_proto;
> >  #endif
> >       default:
> >               return bpf_base_func_proto(func_id);
> > --
> > 2.22.0.657.g960e92d24f-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie
  2019-07-23  0:20 ` [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie Petar Penkov
@ 2019-07-24  6:05   ` kbuild test robot
  2019-07-24  6:19   ` kbuild test robot
  1 sibling, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2019-07-24  6:05 UTC (permalink / raw)
  To: Petar Penkov
  Cc: kbuild-all, netdev, bpf, davem, ast, daniel, edumazet, lmb, sdf,
	Petar Penkov

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

Hi Petar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Petar-Penkov/Introduce-a-BPF-helper-to-generate-SYN-cookies/20190723-235628
base:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-e004-201929 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: net/ipv4/tcp_input.o: in function `tcp_get_syncookie':
>> net/ipv4/tcp_input.c:6538: undefined reference to `__cookie_v6_init_sequence'

vim +6538 net/ipv4/tcp_input.c

  6509	
  6510	u16 tcp_get_syncookie(struct request_sock_ops *rsk_ops,
  6511			      const struct tcp_request_sock_ops *af_ops,
  6512			      struct sock *sk, void *iph, struct tcphdr *th,
  6513			      u32 *cookie)
  6514	{
  6515		u16 mss = 0;
  6516	#ifdef CONFIG_SYN_COOKIES
  6517		bool is_v4 = rsk_ops->family == AF_INET;
  6518		struct tcp_sock *tp = tcp_sk(sk);
  6519	
  6520		if (sock_net(sk)->ipv4.sysctl_tcp_syncookies != 2 &&
  6521		    !inet_csk_reqsk_queue_is_full(sk))
  6522			return 0;
  6523	
  6524		if (!tcp_syn_flood_action(sk, rsk_ops->slab_name))
  6525			return 0;
  6526	
  6527		if (sk_acceptq_is_full(sk)) {
  6528			NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
  6529			return 0;
  6530		}
  6531	
  6532		mss = tcp_parse_mss_option(th, tp->rx_opt.user_mss);
  6533		if (!mss)
  6534			mss = af_ops->mss_clamp;
  6535	
  6536		tcp_synq_overflow(sk);
  6537		*cookie = is_v4 ? __cookie_v4_init_sequence(iph, th, &mss)
> 6538				: __cookie_v6_init_sequence(iph, th, &mss);
  6539	#endif
  6540		return mss;
  6541	}
  6542	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34310 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie
  2019-07-23  0:20 ` [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie Petar Penkov
  2019-07-24  6:05   ` kbuild test robot
@ 2019-07-24  6:19   ` kbuild test robot
  1 sibling, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2019-07-24  6:19 UTC (permalink / raw)
  To: Petar Penkov
  Cc: kbuild-all, netdev, bpf, davem, ast, daniel, edumazet, lmb, sdf,
	Petar Penkov

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

Hi Petar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Petar-Penkov/Introduce-a-BPF-helper-to-generate-SYN-cookies/20190723-235628
base:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: parisc-c8000_defconfig (attached as .config)
compiler: hppa64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   hppa64-linux-ld: arch/parisc/kernel/entry.o(.text.hot+0x22b8): cannot reach preempt_schedule_irq
   arch/parisc/kernel/entry.o: In function `intr_do_preempt':
   (.text.hot+0x22b8): relocation truncated to fit: R_PARISC_PCREL22F against symbol `preempt_schedule_irq' defined in .sched.text section in kernel/sched/core.o
   net/ipv4/tcp_input.o: In function `.LC240':
>> (.data.rel.ro+0x718): undefined reference to `__cookie_v6_init_sequence'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 18429 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-07-24  6:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23  0:20 [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Petar Penkov
2019-07-23  0:20 ` [bpf-next 1/6] tcp: tcp_syn_flood_action read port from socket Petar Penkov
2019-07-23  0:20 ` [bpf-next 2/6] tcp: add skb-less helpers to retrieve SYN cookie Petar Penkov
2019-07-24  6:05   ` kbuild test robot
2019-07-24  6:19   ` kbuild test robot
2019-07-23  0:20 ` [bpf-next 3/6] bpf: add bpf_tcp_gen_syncookie helper Petar Penkov
2019-07-23 12:33   ` Toke Høiland-Jørgensen
2019-07-24  0:15     ` Petar Penkov
2019-07-23  0:20 ` [bpf-next 4/6] bpf: sync bpf.h to tools/ Petar Penkov
2019-07-23  0:20 ` [bpf-next 5/6] selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers Petar Penkov
2019-07-23  0:20 ` [bpf-next 6/6] selftests/bpf: add test for bpf_tcp_gen_syncookie Petar Penkov
2019-07-23  9:37   ` Lorenz Bauer
2019-07-23 20:46     ` Alexei Starovoitov
2019-07-23  6:30 ` [bpf-next 0/6] Introduce a BPF helper to generate SYN cookies Eric Dumazet
2019-07-23 10:27 ` Lorenz Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).