diff mbox series

[v6,07/21] net/tcp: Add tcp_parse_auth_options()

Message ID 20230512202311.2845526-8-dima@arista.com
State New
Headers
Series net/tcp: Add TCP-AO support |

Commit Message

Dmitry Safonov May 12, 2023, 8:22 p.m. UTC 
  Introduce a helper that:
(1) shares the common code with TCP-MD5 header options parsing
(2) looks for hash signature only once for both TCP-MD5 and TCP-AO
(3) fails with -EEXIST if any TCP sign option is present twice, see
    RFC5925 (2.2):
    ">> A single TCP segment MUST NOT have more than one TCP-AO in its
    options sequence. When multiple TCP-AOs appear, TCP MUST discard
    the segment."

Co-developed-by: Francesco Ruggeri <fruggeri@arista.com>
Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
Co-developed-by: Salam Noureddine <noureddine@arista.com>
Signed-off-by: Salam Noureddine <noureddine@arista.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/net/tcp.h    | 24 +++++++++++++++++++++++-
 include/net/tcp_ao.h | 17 ++++++++++++++++-
 net/ipv4/tcp.c       |  3 ++-
 net/ipv4/tcp_input.c | 39 +++++++++++++++++++++++++++++----------
 net/ipv4/tcp_ipv4.c  | 15 ++++++++++-----
 net/ipv6/tcp_ipv6.c  | 11 +++++++----
 6 files changed, 87 insertions(+), 22 deletions(-)

Comments

kernel test robot May 12, 2023, 11:02 p.m. UTC | #1 
Hi Dmitry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 47a2ee5d4a0bda05decdda7be0a77e792cdb09a3]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-Safonov/net-tcp-Prepare-tcp_md5sig_pool-for-TCP-AO/20230513-042734
base:   47a2ee5d4a0bda05decdda7be0a77e792cdb09a3
patch link:    https://lore.kernel.org/r/20230512202311.2845526-8-dima%40arista.com
patch subject: [PATCH v6 07/21] net/tcp: Add tcp_parse_auth_options()
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230513/202305130600.uZymcUzw-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/16d692b101c65ae6a5a60530a3461c512f3bc312
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Dmitry-Safonov/net-tcp-Prepare-tcp_md5sig_pool-for-TCP-AO/20230513-042734
        git checkout 16d692b101c65ae6a5a60530a3461c512f3bc312
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305130600.uZymcUzw-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/ipv4/tcp.c: In function 'tcp_inbound_md5_hash':
>> net/ipv4/tcp.c:4506:24: warning: implicit conversion from 'enum <anonymous>' to 'enum skb_drop_reason' [-Wenum-conversion]
    4506 |                 return true;
         |                        ^~~~


vim +4506 net/ipv4/tcp.c

  4477	
  4478	/* Called with rcu_read_lock() */
  4479	enum skb_drop_reason
  4480	tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
  4481			     const void *saddr, const void *daddr,
  4482			     int family, int dif, int sdif)
  4483	{
  4484		/*
  4485		 * This gets called for each TCP segment that arrives
  4486		 * so we want to be efficient.
  4487		 * We have 3 drop cases:
  4488		 * o No MD5 hash and one expected.
  4489		 * o MD5 hash and we're not expecting one.
  4490		 * o MD5 hash and its wrong.
  4491		 */
  4492		const __u8 *hash_location = NULL;
  4493		struct tcp_md5sig_key *hash_expected;
  4494		const struct tcphdr *th = tcp_hdr(skb);
  4495		const struct tcp_sock *tp = tcp_sk(sk);
  4496		int genhash, l3index;
  4497		u8 newhash[16];
  4498	
  4499		/* sdif set, means packet ingressed via a device
  4500		 * in an L3 domain and dif is set to the l3mdev
  4501		 */
  4502		l3index = sdif ? dif : 0;
  4503	
  4504		hash_expected = tcp_md5_do_lookup(sk, l3index, saddr, family);
  4505		if (tcp_parse_auth_options(th, &hash_location, NULL))
> 4506			return true;
  4507	
  4508		/* We've parsed the options - do we have a hash? */
  4509		if (!hash_expected && !hash_location)
  4510			return SKB_NOT_DROPPED_YET;
  4511	
  4512		if (hash_expected && !hash_location) {
  4513			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
  4514			return SKB_DROP_REASON_TCP_MD5NOTFOUND;
  4515		}
  4516	
  4517		if (!hash_expected && hash_location) {
  4518			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
  4519			return SKB_DROP_REASON_TCP_MD5UNEXPECTED;
  4520		}
  4521	
  4522		/* Check the signature.
  4523		 * To support dual stack listeners, we need to handle
  4524		 * IPv4-mapped case.
  4525		 */
  4526		if (family == AF_INET)
  4527			genhash = tcp_v4_md5_hash_skb(newhash,
  4528						      hash_expected,
  4529						      NULL, skb);
  4530		else
  4531			genhash = tp->af_specific->calc_md5_hash(newhash,
  4532								 hash_expected,
  4533								 NULL, skb);
  4534	
  4535		if (genhash || memcmp(hash_location, newhash, 16) != 0) {
  4536			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5FAILURE);
  4537			if (family == AF_INET) {
  4538				net_info_ratelimited("MD5 Hash failed for (%pI4, %d)->(%pI4, %d)%s L3 index %d\n",
  4539						saddr, ntohs(th->source),
  4540						daddr, ntohs(th->dest),
  4541						genhash ? " tcp_v4_calc_md5_hash failed"
  4542						: "", l3index);
  4543			} else {
  4544				net_info_ratelimited("MD5 Hash %s for [%pI6c]:%u->[%pI6c]:%u L3 index %d\n",
  4545						genhash ? "failed" : "mismatch",
  4546						saddr, ntohs(th->source),
  4547						daddr, ntohs(th->dest), l3index);
  4548			}
  4549			return SKB_DROP_REASON_TCP_MD5FAILURE;
  4550		}
  4551		return SKB_NOT_DROPPED_YET;
  4552	}
  4553	EXPORT_SYMBOL(tcp_inbound_md5_hash);
  4554
diff mbox series

Patch

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 50d255ac3500..e93f9f0fb464 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -434,7 +434,6 @@  int tcp_mmap(struct file *file, struct socket *sock,
 void tcp_parse_options(const struct net *net, const struct sk_buff *skb,
 		       struct tcp_options_received *opt_rx,
 		       int estab, struct tcp_fastopen_cookie *foc);
-const u8 *tcp_parse_md5sig_option(const struct tcphdr *th);
 
 /*
  *	BPF SKB-less helpers
@@ -2535,6 +2534,29 @@  static inline u64 tcp_transmit_time(const struct sock *sk)
 	return 0;
 }
 
+static inline int tcp_parse_auth_options(const struct tcphdr *th,
+		const u8 **md5_hash, const struct tcp_ao_hdr **aoh)
+{
+	const u8 *md5_tmp, *ao_tmp;
+	int ret;
+
+	ret = tcp_do_parse_auth_options(th, &md5_tmp, &ao_tmp);
+	if (ret)
+		return ret;
+
+	if (md5_hash)
+		*md5_hash = md5_tmp;
+
+	if (aoh) {
+		if (!ao_tmp)
+			*aoh = NULL;
+		else
+			*aoh = (struct tcp_ao_hdr *)(ao_tmp - 2);
+	}
+
+	return 0;
+}
+
 static inline bool tcp_ao_required(struct sock *sk, const void *saddr,
 				   int family)
 {
diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
index ee32af145bba..72fc87cf58bf 100644
--- a/include/net/tcp_ao.h
+++ b/include/net/tcp_ao.h
@@ -156,7 +156,9 @@  int tcp_v6_parse_ao(struct sock *sk, int cmd,
 		    sockptr_t optval, int optlen);
 void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb);
 void tcp_ao_connect_init(struct sock *sk);
-
+void tcp_ao_syncookie(struct sock *sk, const struct sk_buff *skb,
+		      struct tcp_request_sock *treq,
+		      unsigned short int family);
 #else /* CONFIG_TCP_AO */
 
 static inline struct tcp_ao_key *tcp_ao_do_lookup(const struct sock *sk,
@@ -179,4 +181,17 @@  static inline void tcp_ao_connect_init(struct sock *sk)
 }
 #endif
 
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
+int tcp_do_parse_auth_options(const struct tcphdr *th,
+			      const u8 **md5_hash, const u8 **ao_hash);
+#else
+static int tcp_do_parse_auth_options(const struct tcphdr *th,
+				     const u8 **md5_hash, const u8 **ao_hash)
+{
+	*md5_hash = NULL;
+	*ao_hash = NULL;
+	return 0;
+}
+#endif
+
 #endif /* _TCP_AO_H */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index fa94d5c416b0..fe30260be4c2 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4502,7 +4502,8 @@  tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
 	l3index = sdif ? dif : 0;
 
 	hash_expected = tcp_md5_do_lookup(sk, l3index, saddr, family);
-	hash_location = tcp_parse_md5sig_option(th);
+	if (tcp_parse_auth_options(th, &hash_location, NULL))
+		return true;
 
 	/* We've parsed the options - do we have a hash? */
 	if (!hash_expected && !hash_location)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2a9010b4ac8c..9626c703caed 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4202,39 +4202,58 @@  static bool tcp_fast_parse_options(const struct net *net,
 	return true;
 }
 
-#ifdef CONFIG_TCP_MD5SIG
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
 /*
- * Parse MD5 Signature option
+ * Parse Signature options
  */
-const u8 *tcp_parse_md5sig_option(const struct tcphdr *th)
+int tcp_do_parse_auth_options(const struct tcphdr *th,
+			      const u8 **md5_hash, const u8 **ao_hash)
 {
 	int length = (th->doff << 2) - sizeof(*th);
 	const u8 *ptr = (const u8 *)(th + 1);
+	unsigned int minlen = TCPOLEN_MD5SIG;
+
+	if (IS_ENABLED(CONFIG_TCP_AO))
+		minlen = sizeof(struct tcp_ao_hdr) + 1;
+
+	*md5_hash = NULL;
+	*ao_hash = NULL;
 
 	/* If not enough data remaining, we can short cut */
-	while (length >= TCPOLEN_MD5SIG) {
+	while (length >= minlen) {
 		int opcode = *ptr++;
 		int opsize;
 
 		switch (opcode) {
 		case TCPOPT_EOL:
-			return NULL;
+			return 0;
 		case TCPOPT_NOP:
 			length--;
 			continue;
 		default:
 			opsize = *ptr++;
 			if (opsize < 2 || opsize > length)
-				return NULL;
-			if (opcode == TCPOPT_MD5SIG)
-				return opsize == TCPOLEN_MD5SIG ? ptr : NULL;
+				return -EINVAL;
+			if (opcode == TCPOPT_MD5SIG) {
+				if (opsize != TCPOLEN_MD5SIG)
+					return -EINVAL;
+				if (unlikely(*md5_hash || *ao_hash))
+					return -EEXIST;
+				*md5_hash = ptr;
+			} else if (opcode == TCPOPT_AO) {
+				if (opsize <= sizeof(struct tcp_ao_hdr))
+					return -EINVAL;
+				if (unlikely(*md5_hash || *ao_hash))
+					return -EEXIST;
+				*ao_hash = ptr;
+			}
 		}
 		ptr += opsize - 2;
 		length -= opsize;
 	}
-	return NULL;
+	return 0;
 }
-EXPORT_SYMBOL(tcp_parse_md5sig_option);
+EXPORT_SYMBOL(tcp_do_parse_auth_options);
 #endif
 
 /* Sorry, PAWS as specified is broken wrt. pure-ACKs -DaveM
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index bd9b9118e1f0..a88465ed5d39 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -668,7 +668,9 @@  EXPORT_SYMBOL(tcp_v4_send_check);
  *	Exception: precedence violation. We do not implement it in any case.
  */
 
-#ifdef CONFIG_TCP_MD5SIG
+#ifdef CONFIG_TCP_AO
+#define OPTION_BYTES MAX_TCP_OPTION_SPACE
+#elif defined(CONFIG_TCP_MD5SIG)
 #define OPTION_BYTES TCPOLEN_MD5SIG_ALIGNED
 #else
 #define OPTION_BYTES sizeof(__be32)
@@ -684,7 +686,7 @@  static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 	struct ip_reply_arg arg;
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key *key = NULL;
-	const __u8 *hash_location = NULL;
+	const __u8 *md5_hash_location = NULL;
 	unsigned char newhash[16];
 	int genhash;
 	struct sock *sk1 = NULL;
@@ -724,8 +726,11 @@  static void tcp_v4_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
+	/* Invalid TCP option size or twice included auth */
+	if (tcp_parse_auth_options(tcp_hdr(skb), &md5_hash_location, NULL))
+		return;
+
 	rcu_read_lock();
-	hash_location = tcp_parse_md5sig_option(th);
 	if (sk && sk_fullsock(sk)) {
 		const union tcp_md5_addr *addr;
 		int l3index;
@@ -736,7 +741,7 @@  static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 		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) {
+	} else if (md5_hash_location) {
 		const union tcp_md5_addr *addr;
 		int sdif = tcp_v4_sdif(skb);
 		int dif = inet_iif(skb);
@@ -768,7 +773,7 @@  static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 
 
 		genhash = tcp_v4_md5_hash_skb(newhash, key, NULL, skb);
-		if (genhash || memcmp(hash_location, newhash, 16) != 0)
+		if (genhash || memcmp(md5_hash_location, newhash, 16) != 0)
 			goto out;
 
 	}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 7ff755e27686..0074d1f1f8a5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -993,7 +993,7 @@  static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 	u32 seq = 0, ack_seq = 0;
 	struct tcp_md5sig_key *key = NULL;
 #ifdef CONFIG_TCP_MD5SIG
-	const __u8 *hash_location = NULL;
+	const __u8 *md5_hash_location = NULL;
 	unsigned char newhash[16];
 	int genhash;
 	struct sock *sk1 = NULL;
@@ -1015,8 +1015,11 @@  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
+	/* Invalid TCP option size or twice included auth */
+	if (tcp_parse_auth_options(th, &md5_hash_location, NULL))
+		return;
+
 	rcu_read_lock();
-	hash_location = tcp_parse_md5sig_option(th);
 	if (sk && sk_fullsock(sk)) {
 		int l3index;
 
@@ -1025,7 +1028,7 @@  static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 		 */
 		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) {
+	} else if (md5_hash_location) {
 		int dif = tcp_v6_iif_l3_slave(skb);
 		int sdif = tcp_v6_sdif(skb);
 		int l3index;
@@ -1054,7 +1057,7 @@  static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 			goto out;
 
 		genhash = tcp_v6_md5_hash_skb(newhash, key, NULL, skb);
-		if (genhash || memcmp(hash_location, newhash, 16) != 0)
+		if (genhash || memcmp(md5_hash_location, newhash, 16) != 0)
 			goto out;
 	}
 #endif