[PATCH 0/5] net/tcp-md5: Verify segments on TIME_WAIT sockets

[PATCH 0/5] net/tcp-md5: Verify segments on TIME_WAIT sockets

[Dmitry Safonov]

Hi,

Started as a refactoring of tcp_v{4,6}_send_reset(), in order to prepare
it for TCP-AO signing for RST segments. As you can see the previous
TCP-AO-v5 [1] version of those functions, they get too big and nasty.
Patches 1 and 2 seem more-or-less straight-forward to me.

But then another thing that I wanted to fix for TCP-AO-version6 was
accepting of unsigned or incorrectly signed segments on twsk, which is
against RFC5925 (7.2) that requires checking the signature. So, I decided
to give it a shot and fix twsk for TCP-MD5 as well.
That seems more questionable, that's why I'm sending patch 3 as RFC.

And the last part, patches 4 and 5 are paranoid checks made to minimize
cases when inbound segment's signature and RST/ACK reply aren't consistent.
This is direct result of RFC2385 that lacks key rotation mechanism.
Probably, patches 4 and 5 are a bit too much, sending them for review
anyway in case such paranoia makes sense.

Thanks,
          Dmitry

Cc: David Ahern <dsahern@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Leonard Crestez <cdleonard@gmail.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Salam Noureddine <noureddine@arista.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

[1]: https://lore.kernel.org/all/20230403213420.1576559-9-dima@arista.com/T/#u

Dmitry Safonov (5):
  net/tcp: Separate TCP-MD5 signing from tcp_v{4,6}_send_reset()
  net/tcp: Use tcp_v6_md5_hash_skb() instead of .calc_md5_hash()
  [RFC] net/tcp-md5: Verify inbound segments on twsk
  [RFC] net/tcp-md5: Don't send RST if key (dis)appeared
  [RFC] net/tcp-md5: Don't send ACK if key (dis)appears

 include/net/tcp.h   |  18 +++-
 net/ipv4/tcp.c      |   9 +-
 net/ipv4/tcp_ipv4.c | 196 +++++++++++++++++++++++++-------------------
 net/ipv6/tcp_ipv6.c | 130 +++++++++++++++++------------
 4 files changed, 207 insertions(+), 146 deletions(-)


base-commit: ba0ad6ed89fd5dada3b7b65ef2b08e95d449d4ab
-- 
2.40.0

[PATCH 1/5] net/tcp: Separate TCP-MD5 signing from tcp_v{4,6}_send_reset()

[Dmitry Safonov]

Separate TCP-MD5 part from the generic TCP code, cleaning it up from
MD5-related ifdeffery (this is most noticeable on ipv4 part). Mostly,
it is refactoring, but with a small bonus: now RST sending functions can
nicely get tcp_md5_needed static key check, making them faster on systems
without TCP-MD5 keys.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 net/ipv4/tcp_ipv4.c | 177 +++++++++++++++++++++++---------------------
 net/ipv6/tcp_ipv6.c | 106 ++++++++++++++------------
 2 files changed, 152 insertions(+), 131 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 39bda2b1066e..b1056a4af60f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -655,6 +655,97 @@ void tcp_v4_send_check(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(tcp_v4_send_check);
 
+#define REPLY_OPTIONS_LEN	(MAX_TCP_OPTION_SPACE / sizeof(__be32))
+
+static bool tcp_v4_md5_sign_reset(struct net *net, const struct sock *sk,
+				  struct sk_buff *skb, struct ip_reply_arg *arg,
+				  struct tcphdr *reply,
+				  __be32 reply_options[REPLY_OPTIONS_LEN])
+{
+#ifdef CONFIG_TCP_MD5SIG
+	const struct tcphdr *th = tcp_hdr(skb);
+	struct tcp_md5sig_key *key = NULL;
+	const __u8 *hash_location = NULL;
+	unsigned char newhash[16];
+	struct sock *sk1 = NULL;
+	int genhash;
+
+	hash_location = tcp_parse_md5sig_option(th);
+	/* Fastpath: no keys in system, don't send RST iff segment is signed */
+	if (!static_branch_unlikely(&tcp_md5_needed.key))
+		return !!hash_location;
+
+	rcu_read_lock();
+	if (sk && sk_fullsock(sk)) {
+		const union tcp_md5_addr *addr;
+		int l3index;
+
+		/* sdif set, means packet ingressed via a device
+		 * in an L3 domain and inet_iif is set to it.
+		 */
+		l3index = tcp_v4_sdif(skb) ? inet_iif(skb) : 0;
+		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
+		key = tcp_md5_do_lookup(sk, l3index, addr, AF_INET);
+	} else if (hash_location) {
+		const union tcp_md5_addr *addr;
+		int sdif = tcp_v4_sdif(skb);
+		int dif = inet_iif(skb);
+		int l3index;
+
+		/*
+		 * active side is lost. Try to find listening socket through
+		 * source port, and then find md5 key through listening socket.
+		 * we are not loose security here:
+		 * Incoming packet is checked with md5 hash with finding key,
+		 * no RST generated if md5 hash doesn't match.
+		 */
+		sk1 = __inet_lookup_listener(net, net->ipv4.tcp_death_row.hashinfo,
+					     NULL, 0, ip_hdr(skb)->saddr,
+					     th->source, ip_hdr(skb)->daddr,
+					     ntohs(th->source), dif, sdif);
+		/* don't send rst if it can't find key */
+		if (!sk1) {
+			rcu_read_unlock();
+			return true;
+		}
+
+		/* sdif set, means packet ingressed via a device
+		 * in an L3 domain and dif is set to it.
+		 */
+		l3index = sdif ? dif : 0;
+		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
+		key = tcp_md5_do_lookup(sk1, l3index, addr, AF_INET);
+		if (!key) {
+			rcu_read_unlock();
+			return true;
+		}
+
+		genhash = tcp_v4_md5_hash_skb(newhash, key, NULL, skb);
+		if (genhash || memcmp(hash_location, newhash, 16) != 0) {
+			rcu_read_unlock();
+			return true;
+		}
+	}
+
+	if (key) {
+		reply_options[0] = htonl((TCPOPT_NOP << 24) |
+				   (TCPOPT_NOP << 16) |
+				   (TCPOPT_MD5SIG << 8) |
+				   TCPOLEN_MD5SIG);
+		/* Update length and the length the header thinks exists */
+		arg->iov[0].iov_len += TCPOLEN_MD5SIG_ALIGNED;
+		reply->doff = arg->iov[0].iov_len / 4;
+
+		tcp_v4_md5_hash_hdr((__u8 *)&reply_options[1],
+				    key, ip_hdr(skb)->saddr,
+				    ip_hdr(skb)->daddr, reply);
+	}
+	rcu_read_unlock();
+#endif
+
+	return false;
+}
+
 /*
  *	This routine will send an RST to the other tcp.
  *
@@ -668,27 +759,14 @@ EXPORT_SYMBOL(tcp_v4_send_check);
  *	Exception: precedence violation. We do not implement it in any case.
  */
 
-#ifdef CONFIG_TCP_MD5SIG
-#define OPTION_BYTES TCPOLEN_MD5SIG_ALIGNED
-#else
-#define OPTION_BYTES sizeof(__be32)
-#endif
-
 static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 {
 	const struct tcphdr *th = tcp_hdr(skb);
 	struct {
 		struct tcphdr th;
-		__be32 opt[OPTION_BYTES / sizeof(__be32)];
+		__be32 opt[REPLY_OPTIONS_LEN];
 	} rep;
 	struct ip_reply_arg arg;
-#ifdef CONFIG_TCP_MD5SIG
-	struct tcp_md5sig_key *key = NULL;
-	const __u8 *hash_location = NULL;
-	unsigned char newhash[16];
-	int genhash;
-	struct sock *sk1 = NULL;
-#endif
 	u64 transmit_time = 0;
 	struct sock *ctl_sk;
 	struct net *net;
@@ -723,70 +801,8 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 	arg.iov[0].iov_len  = sizeof(rep.th);
 
 	net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
-#ifdef CONFIG_TCP_MD5SIG
-	rcu_read_lock();
-	hash_location = tcp_parse_md5sig_option(th);
-	if (sk && sk_fullsock(sk)) {
-		const union tcp_md5_addr *addr;
-		int l3index;
-
-		/* sdif set, means packet ingressed via a device
-		 * in an L3 domain and inet_iif is set to it.
-		 */
-		l3index = tcp_v4_sdif(skb) ? inet_iif(skb) : 0;
-		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
-		key = tcp_md5_do_lookup(sk, l3index, addr, AF_INET);
-	} else if (hash_location) {
-		const union tcp_md5_addr *addr;
-		int sdif = tcp_v4_sdif(skb);
-		int dif = inet_iif(skb);
-		int l3index;
-
-		/*
-		 * active side is lost. Try to find listening socket through
-		 * source port, and then find md5 key through listening socket.
-		 * we are not loose security here:
-		 * Incoming packet is checked with md5 hash with finding key,
-		 * no RST generated if md5 hash doesn't match.
-		 */
-		sk1 = __inet_lookup_listener(net, net->ipv4.tcp_death_row.hashinfo,
-					     NULL, 0, ip_hdr(skb)->saddr,
-					     th->source, ip_hdr(skb)->daddr,
-					     ntohs(th->source), dif, sdif);
-		/* don't send rst if it can't find key */
-		if (!sk1)
-			goto out;
-
-		/* sdif set, means packet ingressed via a device
-		 * in an L3 domain and dif is set to it.
-		 */
-		l3index = sdif ? dif : 0;
-		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
-		key = tcp_md5_do_lookup(sk1, l3index, addr, AF_INET);
-		if (!key)
-			goto out;
-
-
-		genhash = tcp_v4_md5_hash_skb(newhash, key, NULL, skb);
-		if (genhash || memcmp(hash_location, newhash, 16) != 0)
-			goto out;
-
-	}
-
-	if (key) {
-		rep.opt[0] = htonl((TCPOPT_NOP << 24) |
-				   (TCPOPT_NOP << 16) |
-				   (TCPOPT_MD5SIG << 8) |
-				   TCPOLEN_MD5SIG);
-		/* Update length and the length the header thinks exists */
-		arg.iov[0].iov_len += TCPOLEN_MD5SIG_ALIGNED;
-		rep.th.doff = arg.iov[0].iov_len / 4;
-
-		tcp_v4_md5_hash_hdr((__u8 *) &rep.opt[1],
-				     key, ip_hdr(skb)->saddr,
-				     ip_hdr(skb)->daddr, &rep.th);
-	}
-#endif
+	if (tcp_v4_md5_sign_reset(net, sk, skb, &arg, &rep.th, rep.opt))
+		return;
 	/* Can't co-exist with TCPMD5, hence check rep.opt[0] */
 	if (rep.opt[0] == 0) {
 		__be32 mrst = mptcp_reset_option(skb);
@@ -842,11 +858,6 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 	__TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
 	__TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
 	local_bh_enable();
-
-#ifdef CONFIG_TCP_MD5SIG
-out:
-	rcu_read_unlock();
-#endif
 }
 
 /* The code following below sending ACKs in SYN-RECV and TIME-WAIT states
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 7132eb213a7a..42792bc5b9bf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -977,18 +977,67 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
 	kfree_skb(buff);
 }
 
+#ifdef CONFIG_TCP_MD5SIG
+static int tcp_v6_md5_lookup_reset_key(struct net *net, const struct sock *sk,
+		struct sk_buff *skb, struct tcp_md5sig_key **key,
+		const struct tcphdr *th, struct ipv6hdr *ipv6h)
+{
+	const __u8 *hash_location = NULL;
+	int genhash, l3index;
+
+	hash_location = tcp_parse_md5sig_option(th);
+	if (!static_branch_unlikely(&tcp_md5_needed.key))
+		return !!hash_location;
+
+	if (sk && sk_fullsock(sk)) {
+		/* sdif set, means packet ingressed via a device
+		 * in an L3 domain and inet_iif is set to it.
+		 */
+		l3index = tcp_v6_sdif(skb) ? tcp_v6_iif_l3_slave(skb) : 0;
+		*key = tcp_v6_md5_do_lookup(sk, &ipv6h->saddr, l3index);
+	} else if (hash_location) {
+		int dif = tcp_v6_iif_l3_slave(skb);
+		int sdif = tcp_v6_sdif(skb);
+		unsigned char newhash[16];
+		struct sock *sk1;
+
+		/*
+		 * active side is lost. Try to find listening socket through
+		 * source port, and then find md5 key through listening socket.
+		 * we are not loose security here:
+		 * Incoming packet is checked with md5 hash with finding key,
+		 * no RST generated if md5 hash doesn't match.
+		 */
+		sk1 = inet6_lookup_listener(net, net->ipv4.tcp_death_row.hashinfo,
+					    NULL, 0, &ipv6h->saddr, th->source,
+					    &ipv6h->daddr, ntohs(th->source),
+					    dif, sdif);
+		if (!sk1)
+			return -ENOKEY;
+
+		/* sdif set, means packet ingressed via a device
+		 * in an L3 domain and dif is set to it.
+		 */
+		l3index = tcp_v6_sdif(skb) ? dif : 0;
+
+		*key = tcp_v6_md5_do_lookup(sk1, &ipv6h->saddr, l3index);
+		if (!*key)
+			return -ENOKEY;
+
+		genhash = tcp_v6_md5_hash_skb(newhash, *key, NULL, skb);
+		if (genhash || memcmp(hash_location, newhash, 16) != 0)
+			return -EKEYREJECTED;
+	}
+	return 0;
+}
+#endif
+
 static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 {
 	const struct tcphdr *th = tcp_hdr(skb);
 	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
-	u32 seq = 0, ack_seq = 0;
 	struct tcp_md5sig_key *key = NULL;
-#ifdef CONFIG_TCP_MD5SIG
-	const __u8 *hash_location = NULL;
-	unsigned char newhash[16];
-	int genhash;
-	struct sock *sk1 = NULL;
-#endif
+	u32 seq = 0, ack_seq = 0;
 	__be32 label = 0;
 	u32 priority = 0;
 	struct net *net;
@@ -1007,47 +1056,8 @@ static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 	net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
 #ifdef CONFIG_TCP_MD5SIG
 	rcu_read_lock();
-	hash_location = tcp_parse_md5sig_option(th);
-	if (sk && sk_fullsock(sk)) {
-		int l3index;
-
-		/* sdif set, means packet ingressed via a device
-		 * in an L3 domain and inet_iif is set to it.
-		 */
-		l3index = tcp_v6_sdif(skb) ? tcp_v6_iif_l3_slave(skb) : 0;
-		key = tcp_v6_md5_do_lookup(sk, &ipv6h->saddr, l3index);
-	} else if (hash_location) {
-		int dif = tcp_v6_iif_l3_slave(skb);
-		int sdif = tcp_v6_sdif(skb);
-		int l3index;
-
-		/*
-		 * active side is lost. Try to find listening socket through
-		 * source port, and then find md5 key through listening socket.
-		 * we are not loose security here:
-		 * Incoming packet is checked with md5 hash with finding key,
-		 * no RST generated if md5 hash doesn't match.
-		 */
-		sk1 = inet6_lookup_listener(net, net->ipv4.tcp_death_row.hashinfo,
-					    NULL, 0, &ipv6h->saddr, th->source,
-					    &ipv6h->daddr, ntohs(th->source),
-					    dif, sdif);
-		if (!sk1)
-			goto out;
-
-		/* sdif set, means packet ingressed via a device
-		 * in an L3 domain and dif is set to it.
-		 */
-		l3index = tcp_v6_sdif(skb) ? dif : 0;
-
-		key = tcp_v6_md5_do_lookup(sk1, &ipv6h->saddr, l3index);
-		if (!key)
-			goto out;
-
-		genhash = tcp_v6_md5_hash_skb(newhash, key, NULL, skb);
-		if (genhash || memcmp(hash_location, newhash, 16) != 0)
-			goto out;
-	}
+	if (tcp_v6_md5_lookup_reset_key(net, sk, skb, &key, th, ipv6h))
+		goto out;
 #endif
 
 	if (th->ack)
-- 
2.40.0

[PATCH 2/5] net/tcp: Use tcp_v6_md5_hash_skb() instead of .calc_md5_hash()

[Dmitry Safonov]

Using af-specific callback requires the socket to be full (struct tcp_sock).
Using tcp_v6_md5_hash_skb() instead, depending on passed family
parameter makes it possible to use it for non-full sockets as well (if
key-lookup succeeds). Next commit uses tcp_inbound_md5_hash() to verify
segments on twsk.
This seems quite safe to do, as pre-commit 7bbb765b7349 ("net/tcp: Merge
TCP-MD5 inbound callbacks") ip-version-specific functions
tcp_v{4,6}_inbound_md5_hash were calling
tcp_v4_md5_hash_skb()/tcp_v6_md5_hash_skb().

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/net/tcp.h   | 11 +++++++++++
 net/ipv4/tcp.c      |  9 +++------
 net/ipv6/tcp_ipv6.c |  6 ++----
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 04a31643cda3..e127fc685ca6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1676,6 +1676,17 @@ struct tcp_md5sig_pool {
 /* - functions */
 int tcp_v4_md5_hash_skb(char *md5_hash, const struct tcp_md5sig_key *key,
 			const struct sock *sk, const struct sk_buff *skb);
+#if IS_ENABLED(CONFIG_IPV6)
+int tcp_v6_md5_hash_skb(char *md5_hash, const struct tcp_md5sig_key *key,
+			const struct sock *sk, const struct sk_buff *skb);
+#else
+static inline int tcp_v6_md5_hash_skb(char *md5_hash,
+			const struct tcp_md5sig_key *key,
+			const struct sock *sk, const struct sk_buff *skb)
+{
+	return -EPROTONOSUPPORT;
+}
+#endif
 int tcp_md5_do_add(struct sock *sk, const union tcp_md5_addr *addr,
 		   int family, u8 prefixlen, int l3index, u8 flags,
 		   const u8 *newkey, u8 newkeylen);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 20db115c38c4..c1897a039ff5 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4570,7 +4570,6 @@ tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
 	const __u8 *hash_location = NULL;
 	struct tcp_md5sig_key *hash_expected;
 	const struct tcphdr *th = tcp_hdr(skb);
-	const struct tcp_sock *tp = tcp_sk(sk);
 	int genhash, l3index;
 	u8 newhash[16];
 
@@ -4601,13 +4600,11 @@ tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
 	 * IPv4-mapped case.
 	 */
 	if (family == AF_INET)
-		genhash = tcp_v4_md5_hash_skb(newhash,
-					      hash_expected,
+		genhash = tcp_v4_md5_hash_skb(newhash, hash_expected,
 					      NULL, skb);
 	else
-		genhash = tp->af_specific->calc_md5_hash(newhash,
-							 hash_expected,
-							 NULL, skb);
+		genhash = tcp_v6_md5_hash_skb(newhash, hash_expected,
+					      NULL, skb);
 
 	if (genhash || memcmp(hash_location, newhash, 16) != 0) {
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5FAILURE);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 42792bc5b9bf..574398a89970 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -732,10 +732,8 @@ static int tcp_v6_md5_hash_hdr(char *md5_hash, const struct tcp_md5sig_key *key,
 	return 1;
 }
 
-static int tcp_v6_md5_hash_skb(char *md5_hash,
-			       const struct tcp_md5sig_key *key,
-			       const struct sock *sk,
-			       const struct sk_buff *skb)
+int tcp_v6_md5_hash_skb(char *md5_hash, const struct tcp_md5sig_key *key,
+			const struct sock *sk, const struct sk_buff *skb)
 {
 	const struct in6_addr *saddr, *daddr;
 	struct tcp_md5sig_pool *hp;
-- 
2.40.0

[RFC 3/5] net/tcp-md5: Verify inbound segments on twsk

[Dmitry Safonov]

It seems rare for BGP to have twsk socket and quite unlikely on server
side, in addition I don't see any major concern of destroying twsk early
by unsigned segments.
But on the other hand, it seems better not to change TCP state by
unsigned inbound segments and fixing this seems not hard.
So, lets avoid replying or doing any TCP state changes as long as the
segments weren't verified.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/net/tcp.h   |  7 ++++---
 net/ipv4/tcp_ipv4.c |  9 +++++++--
 net/ipv6/tcp_ipv6.c | 10 ++++++++--
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index e127fc685ca6..db13dc7558f4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1705,12 +1705,16 @@ extern struct static_key_false_deferred tcp_md5_needed;
 struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk, int l3index,
 					   const union tcp_md5_addr *addr,
 					   int family);
+
+#define tcp_twsk_md5_key(twsk)	((twsk)->tw_md5_key)
 static inline struct tcp_md5sig_key *
 tcp_md5_do_lookup(const struct sock *sk, int l3index,
 		  const union tcp_md5_addr *addr, int family)
 {
 	if (!static_branch_unlikely(&tcp_md5_needed.key))
 		return NULL;
+	if (unlikely(sk->sk_state == TCP_TIME_WAIT))
+		return tcp_twsk_md5_key(tcp_twsk(sk));
 	return __tcp_md5_do_lookup(sk, l3index, addr, family);
 }
 
@@ -1718,9 +1722,6 @@ enum skb_drop_reason
 tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
 		     const void *saddr, const void *daddr,
 		     int family, int dif, int sdif);
-
-
-#define tcp_twsk_md5_key(twsk)	((twsk)->tw_md5_key)
 #else
 static inline struct tcp_md5sig_key *
 tcp_md5_do_lookup(const struct sock *sk, int l3index,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b1056a4af60f..f5b870943dcb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -676,7 +676,7 @@ static bool tcp_v4_md5_sign_reset(struct net *net, const struct sock *sk,
 		return !!hash_location;
 
 	rcu_read_lock();
-	if (sk && sk_fullsock(sk)) {
+	if (sk && sk->sk_state != TCP_NEW_SYN_RECV) {
 		const union tcp_md5_addr *addr;
 		int l3index;
 
@@ -2195,8 +2195,13 @@ int tcp_v4_rcv(struct sk_buff *skb)
 	goto discard_it;
 
 do_time_wait:
-	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
 		drop_reason = SKB_DROP_REASON_XFRM_POLICY;
+	else
+		drop_reason = tcp_inbound_md5_hash(sk, skb,
+						   &iph->saddr, &iph->daddr,
+						   AF_INET, dif, sdif);
+	if (drop_reason) {
 		inet_twsk_put(inet_twsk(sk));
 		goto discard_it;
 	}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 574398a89970..3756a43367a3 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -987,7 +987,7 @@ static int tcp_v6_md5_lookup_reset_key(struct net *net, const struct sock *sk,
 	if (!static_branch_unlikely(&tcp_md5_needed.key))
 		return !!hash_location;
 
-	if (sk && sk_fullsock(sk)) {
+	if (sk && sk->sk_state != TCP_NEW_SYN_RECV) {
 		/* sdif set, means packet ingressed via a device
 		 * in an L3 domain and inet_iif is set to it.
 		 */
@@ -1795,8 +1795,14 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 	goto discard_it;
 
 do_time_wait:
-	if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+	if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
 		drop_reason = SKB_DROP_REASON_XFRM_POLICY;
+	else
+		drop_reason = tcp_inbound_md5_hash(sk, skb,
+						   &hdr->saddr, &hdr->daddr,
+						   AF_INET6, dif, sdif);
+
+	if (drop_reason) {
 		inet_twsk_put(inet_twsk(sk));
 		goto discard_it;
 	}
-- 
2.40.0

[RFC 4/5] net/tcp-md5: Don't send RST if key (dis)appeared

[Dmitry Safonov]

Seems cheap at this place as both key and hash_location were looked up
until now.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 net/ipv4/tcp_ipv4.c | 10 ++++++++++
 net/ipv6/tcp_ipv6.c |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f5b870943dcb..d94cd5e70d58 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -686,6 +686,16 @@ static bool tcp_v4_md5_sign_reset(struct net *net, const struct sock *sk,
 		l3index = tcp_v4_sdif(skb) ? inet_iif(skb) : 0;
 		addr = (union tcp_md5_addr *)&ip_hdr(skb)->saddr;
 		key = tcp_md5_do_lookup(sk, l3index, addr, AF_INET);
+		/* This segment should have been already verified by
+		 * tcp_inbound_md5_hash(). But that might raced with userspace
+		 * adding or deleting keys. So, follow the logic of
+		 * tcp_inbound_md5_hash() and avoid replying with TCP-MD5 sign
+		 * on non-signed segments and vice-versa.
+		 */
+		if (unlikely(!!key != !!hash_location)) {
+			rcu_read_unlock();
+			return true;
+		}
 	} else if (hash_location) {
 		const union tcp_md5_addr *addr;
 		int sdif = tcp_v4_sdif(skb);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3756a43367a3..498dfa194b8b 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -993,6 +993,14 @@ static int tcp_v6_md5_lookup_reset_key(struct net *net, const struct sock *sk,
 		 */
 		l3index = tcp_v6_sdif(skb) ? tcp_v6_iif_l3_slave(skb) : 0;
 		*key = tcp_v6_md5_do_lookup(sk, &ipv6h->saddr, l3index);
+		/* This segment should have been already verified by
+		 * tcp_inbound_md5_hash(). But that might raced with userspace
+		 * adding or deleting keys. So, follow the logic of
+		 * tcp_inbound_md5_hash() and avoid replying with TCP-MD5 sign
+		 * on non-signed segments and vice-versa.
+		 */
+		if (unlikely(!!*key != !!hash_location))
+			return -ENOKEY;
 	} else if (hash_location) {
 		int dif = tcp_v6_iif_l3_slave(skb);
 		int sdif = tcp_v6_sdif(skb);
-- 
2.40.0

[RFC 5/5] net/tcp-md5: Don't send ACK if key (dis)appears

[Dmitry Safonov]

To mirror RST paranoid checks and tcp_inbound_md5_hash().

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 net/ipv4/tcp_ipv4.c | 2 ++
 net/ipv6/tcp_ipv6.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index d94cd5e70d58..0c8893240f70 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -918,6 +918,8 @@ static void tcp_v4_send_ack(const struct sock *sk,
 	rep.th.window  = htons(win);
 
 #ifdef CONFIG_TCP_MD5SIG
+	if (unlikely(!!key != !!tcp_parse_md5sig_option(th)))
+		return;
 	if (key) {
 		int offset = (tsecr) ? 3 : 0;
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 498dfa194b8b..4131ada9fabf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -863,6 +863,8 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
 	if (tsecr)
 		tot_len += TCPOLEN_TSTAMP_ALIGNED;
 #ifdef CONFIG_TCP_MD5SIG
+	if (!rst && unlikely(!!key != !!tcp_parse_md5sig_option(th)))
+		return;
 	if (key)
 		tot_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif
-- 
2.40.0