diff mbox series

[net-next,2/5] tcp: TFO: search for correct cookie and accept data

Message ID 20181214224007.54813-3-cpaasch@apple.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series tcp: Introduce a TFO key-pool for clean cookie-rotation | expand

Commit Message

Christoph Paasch Dec. 14, 2018, 10:40 p.m. UTC 
This change allows to search for the right cookie and accepts old ones
(announcing a new one if it has changed).

__tcp_fastopen_cookie_gen_with_ctx() allows to generate a cookie based
on a given TFO-context. A later patch will cleanup the duplicate code.

Signed-off-by: Christoph Paasch <cpaasch@apple.com>
---
 include/uapi/linux/snmp.h |   1 +
 net/ipv4/proc.c           |   1 +
 net/ipv4/tcp_fastopen.c   | 105 +++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 101 insertions(+), 6 deletions(-)

Comments

Eric Dumazet Dec. 17, 2018, 6:30 a.m. UTC | #1 
On 12/14/2018 02:40 PM, Christoph Paasch wrote:
> This change allows to search for the right cookie and accepts old ones
> (announcing a new one if it has changed).
> 
> __tcp_fastopen_cookie_gen_with_ctx() allows to generate a cookie based
> on a given TFO-context. A later patch will cleanup the duplicate code.

How long is kept the secondary (old) context ?

I do not know exact crypto_cipher_encrypt_one() cost, but it looks like
your patch could double the cost of some TFO based attacks ?
Christoph Paasch Dec. 17, 2018, 10:59 p.m. UTC | #2 
On 16/12/18 - 22:30:51, Eric Dumazet wrote:
> 
> 
> On 12/14/2018 02:40 PM, Christoph Paasch wrote:
> > This change allows to search for the right cookie and accepts old ones
> > (announcing a new one if it has changed).
> > 
> > __tcp_fastopen_cookie_gen_with_ctx() allows to generate a cookie based
> > on a given TFO-context. A later patch will cleanup the duplicate code.
> 
> How long is kept the secondary (old) context ?

There is no time-limit on keeping the older context.

In an older version of this series I had the pool-size as a sysctl so one
could try out different configurations. For us, a size of 2 was good enough.

I could bring that back if you think it's useful.

> I do not know exact crypto_cipher_encrypt_one() cost, but it looks like
> your patch could double the cost of some TFO based attacks ?

True, we are doing more crypto if we are getting a lot of invalid or old cookies.
I don't have a good answer to that besides that one should probably disable
TFO at that point ;-)

On the other hand, AFAICS tcp_conn_request will end up setting want_cookie
to true under SYN-flooding so we won't even enter tcp_try_fastopen if it's
really an attack.



Christoph
diff mbox series

Patch

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 86dc24a96c90..74904e9d1b72 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -283,6 +283,7 @@  enum
 	LINUX_MIB_TCPACKCOMPRESSED,		/* TCPAckCompressed */
 	LINUX_MIB_TCPZEROWINDOWDROP,		/* TCPZeroWindowDrop */
 	LINUX_MIB_TCPRCVQDROP,			/* TCPRcvQDrop */
+	LINUX_MIB_TCPFASTOPENPASSIVEALTKEY,	/* TCPFastOpenPassiveAltKey */
 	__LINUX_MIB_MAX
 };
 
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index c3610b37bb4c..58daef27a560 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -291,6 +291,7 @@  static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("TCPAckCompressed", LINUX_MIB_TCPACKCOMPRESSED),
 	SNMP_MIB_ITEM("TCPZeroWindowDrop", LINUX_MIB_TCPZEROWINDOWDROP),
 	SNMP_MIB_ITEM("TCPRcvQDrop", LINUX_MIB_TCPRCVQDROP),
+	SNMP_MIB_ITEM("TCPFastOpenPassiveAltKey", LINUX_MIB_TCPFASTOPENPASSIVEALTKEY),
 	SNMP_MIB_SENTINEL
 };
 
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index c52d5b8eabf0..e856262ef4c2 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -176,6 +176,41 @@  static bool __tcp_fastopen_cookie_gen(struct sock *sk, const void *path,
 	return ok;
 }
 
+static void __tcp_fastopen_cookie_gen_with_ctx(struct request_sock *req,
+					       struct sk_buff *syn,
+					       struct tcp_fastopen_cookie *foc,
+					       struct tcp_fastopen_context *ctx)
+{
+	if (req->rsk_ops->family == AF_INET) {
+		const struct iphdr *iph = ip_hdr(syn);
+		__be32 path[4] = { iph->saddr, iph->daddr, 0, 0 };
+
+		crypto_cipher_encrypt_one(ctx->tfm, foc->val, (void *)path);
+		foc->len = TCP_FASTOPEN_COOKIE_SIZE;
+		return;
+	}
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (req->rsk_ops->family == AF_INET6) {
+		const struct ipv6hdr *ip6h = ipv6_hdr(syn);
+		struct tcp_fastopen_cookie tmp;
+		struct in6_addr *buf;
+		int i;
+
+		crypto_cipher_encrypt_one(ctx->tfm, tmp.val, (void *)&ip6h->saddr);
+
+		buf = &tmp.addr;
+		for (i = 0; i < 4; i++)
+			buf->s6_addr32[i] ^= ip6h->daddr.s6_addr32[i];
+
+		crypto_cipher_encrypt_one(ctx->tfm, foc->val, (void *)buf);
+		foc->len = TCP_FASTOPEN_COOKIE_SIZE;
+
+		return;
+	}
+#endif
+}
+
 /* Generate the fastopen cookie by doing aes128 encryption on both
  * the source and destination addresses. Pad 0s for IPv4 or IPv4-mapped-IPv6
  * addresses. For the longer IPv6 addresses use CBC-MAC.
@@ -256,6 +291,55 @@  void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
 		tcp_fin(sk);
 }
 
+static bool tcp_fastopen_cookie_gen_search(struct sock *sk,
+					   struct request_sock *req,
+					   struct sk_buff *syn,
+					   struct tcp_fastopen_cookie *valid_foc,
+					   struct tcp_fastopen_cookie *orig)
+{
+	struct tcp_fastopen_cookie search_foc = { .len = -1 };
+	struct tcp_fastopen_cookie *foc = &search_foc;
+	struct tcp_fastopen_context *ctx;
+	int copied = 0;
+
+	rcu_read_lock();
+
+	ctx = rcu_dereference(inet_csk(sk)->icsk_accept_queue.fastopenq.ctx);
+	if (!ctx)
+		ctx = rcu_dereference(sock_net(sk)->ipv4.tcp_fastopen_ctx);
+
+	while (ctx) {
+		__tcp_fastopen_cookie_gen_with_ctx(req, syn, foc, ctx);
+
+		if (foc->len == orig->len &&
+		    !memcmp(foc->val, orig->val, foc->len)) {
+			rcu_read_unlock();
+
+			if (copied) {
+				struct net *net = read_pnet(&inet_rsk(req)->ireq_net);
+
+				NET_INC_STATS(net,
+					      LINUX_MIB_TCPFASTOPENPASSIVEALTKEY);
+			}
+			return true;
+		}
+
+		/* We need to check older possible cookies, thus set valid_foc
+		 * so that the latest one will be announced to the peer.
+		 */
+		if (!copied) {
+			memcpy(valid_foc, foc, sizeof(*foc));
+			copied = 1;
+		}
+
+		ctx = rcu_dereference(ctx->next);
+	}
+
+	rcu_read_unlock();
+
+	return false;
+}
+
 static struct sock *tcp_fastopen_create_child(struct sock *sk,
 					      struct sk_buff *skb,
 					      struct request_sock *req)
@@ -390,11 +474,11 @@  struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
 	    tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
 		goto fastopen;
 
-	if (foc->len >= 0 &&  /* Client presents or requests a cookie */
-	    tcp_fastopen_cookie_gen(sk, req, skb, &valid_foc) &&
-	    foc->len == TCP_FASTOPEN_COOKIE_SIZE &&
-	    foc->len == valid_foc.len &&
-	    !memcmp(foc->val, valid_foc.val, foc->len)) {
+	if (foc->len == 0) {
+		/* Client requests a cookie. */
+		tcp_fastopen_cookie_gen(sk, req, skb, &valid_foc);
+	} else if (foc->len > 0 &&
+		   tcp_fastopen_cookie_gen_search(sk, req, skb, &valid_foc, foc)) {
 		/* Cookie is valid. Create a (full) child socket to accept
 		 * the data in SYN before returning a SYN-ACK to ack the
 		 * data. If we fail to create the socket, fall back and
@@ -406,7 +490,16 @@  struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
 fastopen:
 		child = tcp_fastopen_create_child(sk, skb, req);
 		if (child) {
-			foc->len = -1;
+			if (valid_foc.len != -1) {
+				/* Client used an old cookie, we announce the
+				 * latests one to the client.
+				 */
+				valid_foc.exp = foc->exp;
+				*foc = valid_foc;
+			} else {
+				foc->len = -1;
+			}
+
 			NET_INC_STATS(sock_net(sk),
 				      LINUX_MIB_TCPFASTOPENPASSIVE);
 			return child;