mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dmitry Safonov <dima@arista.com>
To: linux-kernel@vger.kernel.org, David Ahern <dsahern@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Cc: Dmitry Safonov <dima@arista.com>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Leonard Crestez <cdleonard@gmail.com>,
	Salam Noureddine <noureddine@arista.com>,
	netdev@vger.kernel.org
Subject: [RFC 3/5] net/tcp-md5: Verify inbound segments on twsk
Date: Tue,  9 May 2023 23:16:06 +0100	[thread overview]
Message-ID: <20230509221608.2569333-4-dima@arista.com> (raw)
In-Reply-To: <20230509221608.2569333-1-dima@arista.com>

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


  parent reply	other threads:[~2023-05-09 22:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-09 22:16 [PATCH 0/5] net/tcp-md5: Verify segments on TIME_WAIT sockets Dmitry Safonov
2023-05-09 22:16 ` [PATCH 1/5] net/tcp: Separate TCP-MD5 signing from tcp_v{4,6}_send_reset() Dmitry Safonov
2023-05-09 22:16 ` [PATCH 2/5] net/tcp: Use tcp_v6_md5_hash_skb() instead of .calc_md5_hash() Dmitry Safonov
2023-05-09 22:16 ` Dmitry Safonov [this message]
2023-05-09 22:16 ` [RFC 4/5] net/tcp-md5: Don't send RST if key (dis)appeared Dmitry Safonov
2023-05-09 22:16 ` [RFC 5/5] net/tcp-md5: Don't send ACK if key (dis)appears Dmitry Safonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230509221608.2569333-4-dima@arista.com \
    --to=dima@arista.com \
    --cc=0x7f454c46@gmail.com \
    --cc=cdleonard@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=noureddine@arista.com \
    --cc=pabeni@redhat.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®