[PATCH v4 04/21] net/tcp: Prevent TCP-MD5 with TCP-AO being set

From: Dmitry Safonov
Date: Wed Feb 15 2023 - 13:34:33 EST

Next message: Dmitry Safonov: "[PATCH v4 05/21] net/tcp: Calculate TCP-AO traffic keys"
Previous message: Dmitry Safonov: "[PATCH v4 03/21] net/tcp: Introduce TCP_AO setsockopt()s"
In reply to: Dmitry Safonov: "[PATCH v4 03/21] net/tcp: Introduce TCP_AO setsockopt()s"
Next in thread: Dmitry Safonov: "[PATCH v4 05/21] net/tcp: Calculate TCP-AO traffic keys"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Be as conservative as possible: if there is TCP-MD5 key for a given peer
regardless of L3 interface - don't allow setting TCP-AO key for the same
peer. According to RFC5925, TCP-AO is supposed to replace TCP-MD5 and
there can't be any switch between both on any connected tuple.
Later it can be relaxed, if there's a use, but in the beginning restrict
any intersection.

Note: it's still should be possible to set both TCP-MD5 and TCP-AO keys
on a listening socket for *different* peers.

Co-developed-by: Francesco Ruggeri <fruggeri@xxxxxxxxxx>
Signed-off-by: Francesco Ruggeri <fruggeri@xxxxxxxxxx>
Co-developed-by: Salam Noureddine <noureddine@xxxxxxxxxx>
Signed-off-by: Salam Noureddine <noureddine@xxxxxxxxxx>
Signed-off-by: Dmitry Safonov <dima@xxxxxxxxxx>
---
include/net/tcp.h | 24 ++++++++++++++++++++++--
include/net/tcp_ao.h | 15 +++++++++++++++
net/ipv4/tcp_ao.c | 12 ++++++++++++
net/ipv4/tcp_ipv4.c | 12 +++++++++---
net/ipv4/tcp_output.c | 36 ++++++++++++++++++++++++++++++++++++
net/ipv6/tcp_ao.c | 17 +++++++++++++++++
net/ipv6/tcp_ipv6.c | 22 ++++++++++++++++++----
7 files changed, 129 insertions(+), 9 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index fb18558053cc..2673cc8e5ed5 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1701,6 +1701,7 @@ int tcp_md5_key_copy(struct sock *sk, const union tcp_md5_addr *addr,

int tcp_md5_do_del(struct sock *sk, const union tcp_md5_addr *addr,
int family, u8 prefixlen, int l3index, u8 flags);
+void tcp_clear_md5_list(struct sock *sk);
struct tcp_md5sig_key *tcp_v4_md5_lookup(const struct sock *sk,
const struct sock *addr_sk);

@@ -1709,14 +1710,23 @@ struct tcp_md5sig_key *tcp_v4_md5_lookup(const struct sock *sk,
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);
+ int family, bool any_l3index);
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;
- return __tcp_md5_do_lookup(sk, l3index, addr, family);
+ return __tcp_md5_do_lookup(sk, l3index, addr, family, false);
+}
+
+static inline struct tcp_md5sig_key *
+tcp_md5_do_lookup_any_l3index(const struct sock *sk,
+ const union tcp_md5_addr *addr, int family)
+{
+ if (!static_branch_unlikely(&tcp_md5_needed.key))
+ return NULL;
+ return __tcp_md5_do_lookup(sk, 0, addr, family, true);
}

enum skb_drop_reason
@@ -1734,6 +1744,13 @@ tcp_md5_do_lookup(const struct sock *sk, int l3index,
return NULL;
}

+static inline struct tcp_md5sig_key *
+tcp_md5_do_lookup_any_l3index(const struct sock *sk,
+ const union tcp_md5_addr *addr, int family)
+{
+ return NULL;
+}
+
static inline enum skb_drop_reason
tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
const void *saddr, const void *daddr,
@@ -2097,6 +2114,9 @@ struct tcp_sock_af_ops {
int optname,
sockptr_t optval,
int optlen);
+ struct tcp_ao_key *(*ao_lookup)(const struct sock *sk,
+ struct sock *addr_sk,
+ int sndid, int rcvid);
#endif
};

diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
index 92c8c3ed42db..d817dac3bb0a 100644
--- a/include/net/tcp_ao.h
+++ b/include/net/tcp_ao.h
@@ -91,12 +91,27 @@ struct tcp_ao_info {
int tcp_parse_ao(struct sock *sk, int cmd, unsigned short int family,
sockptr_t optval, int optlen);
void tcp_ao_destroy_sock(struct sock *sk);
+struct tcp_ao_key *tcp_ao_do_lookup(const struct sock *sk,
+ const union tcp_ao_addr *addr,
+ int family, int sndid, int rcvid, u16 port);
/* ipv4 specific functions */
int tcp_v4_parse_ao(struct sock *sk, int optname, sockptr_t optval, int optlen);
+struct tcp_ao_key *tcp_v4_ao_lookup(const struct sock *sk, struct sock *addr_sk,
+ int sndid, int rcvid);
/* ipv6 specific functions */
+struct tcp_ao_key *tcp_v6_ao_lookup(const struct sock *sk,
+ struct sock *addr_sk,
+ int sndid, int rcvid);
int tcp_v6_parse_ao(struct sock *sk, int cmd,
sockptr_t optval, int optlen);
#else
+static inline struct tcp_ao_key *tcp_ao_do_lookup(const struct sock *sk,
+ const union tcp_ao_addr *addr,
+ int family, int sndid, int rcvid, u16 port)
+{
+ return NULL;
+}
+
static inline void tcp_ao_destroy_sock(struct sock *sk)
{
}
diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index 2dc8a3e946b1..4861ba9c89b9 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -165,6 +165,14 @@ void tcp_ao_destroy_sock(struct sock *sk)
kfree_rcu(ao, rcu);
}

+struct tcp_ao_key *tcp_v4_ao_lookup(const struct sock *sk, struct sock *addr_sk,
+ int sndid, int rcvid)
+{
+ union tcp_ao_addr *addr = (union tcp_ao_addr *)&addr_sk->sk_daddr;
+
+ return tcp_ao_do_lookup(sk, addr, AF_INET, sndid, rcvid, 0);
+}
+
static int tcp_ao_current_rnext(struct sock *sk, u16 tcpa_flags,
u8 tcpa_sndid, u8 tcpa_rcvid)
{
@@ -619,6 +627,10 @@ static int tcp_ao_add_cmd(struct sock *sk, unsigned short int family,
if (cmd.tcpa_keyflags & ~TCP_AO_KEYF_ALL)
return -EINVAL;

+ /* Don't allow keys for peers that have a matching TCP-MD5 key */
+ if (tcp_md5_do_lookup_any_l3index(sk, addr, family))
+ return -EKEYREJECTED;
+
ao_info = rcu_dereference_protected(tcp_sk(sk)->ao_info,
lockdep_sock_is_held(sk));

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 60695942ee2b..b143f5f0ca38 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1073,7 +1073,7 @@ static bool better_md5_match(struct tcp_md5sig_key *old, struct tcp_md5sig_key *
/* Find the Key structure for an address. */
struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk, int l3index,
const union tcp_md5_addr *addr,
- int family)
+ int family, bool any_l3index)
{
const struct tcp_sock *tp = tcp_sk(sk);
struct tcp_md5sig_key *key;
@@ -1092,7 +1092,8 @@ struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk, int l3index,
lockdep_sock_is_held(sk)) {
if (key->family != family)
continue;
- if (key->flags & TCP_MD5SIG_FLAG_IFINDEX && key->l3index != l3index)
+ if (!any_l3index && key->flags & TCP_MD5SIG_FLAG_IFINDEX &&
+ key->l3index != l3index)
continue;
if (family == AF_INET) {
mask = inet_make_mask(key->prefixlen);
@@ -1304,7 +1305,7 @@ int tcp_md5_do_del(struct sock *sk, const union tcp_md5_addr *addr, int family,
}
EXPORT_SYMBOL(tcp_md5_do_del);

-static void tcp_clear_md5_list(struct sock *sk)
+void tcp_clear_md5_list(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
struct tcp_md5sig_key *key;
@@ -1374,6 +1375,10 @@ static int tcp_v4_parse_md5_keys(struct sock *sk, int optname,
if (cmd.tcpm_keylen > TCP_MD5SIG_MAXKEYLEN)
return -EINVAL;

+ /* Don't allow keys for peers that have a matching TCP-AO key */
+ if (tcp_ao_do_lookup(sk, addr, AF_INET, -1, -1, 0))
+ return -EKEYREJECTED;
+
return tcp_md5_do_add(sk, addr, AF_INET, prefixlen, l3index, flags,
cmd.tcpm_key, cmd.tcpm_keylen);
}
@@ -2268,6 +2273,7 @@ static const struct tcp_sock_af_ops tcp_sock_ipv4_specific = {
.md5_parse = tcp_v4_parse_md5_keys,
#endif
#ifdef CONFIG_TCP_AO
+ .ao_lookup = tcp_v4_ao_lookup,
.ao_parse = tcp_v4_parse_ao,
#endif
};
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 71d01cf3c13e..3fe291e4a6a2 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3839,6 +3839,42 @@ int tcp_connect(struct sock *sk)

tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB, 0, NULL);

+#if defined(CONFIG_TCP_MD5SIG) && defined(CONFIG_TCP_AO)
+ /* Has to be checked late, after setting daddr/saddr/ops.
+ * Return error if the peer has both a md5 and a tcp-ao key
+ * configured as this is ambiguous.
+ */
+ if (unlikely(rcu_dereference_protected(tp->md5sig_info,
+ lockdep_sock_is_held(sk)))) {
+ bool needs_md5 = !!tp->af_specific->md5_lookup(sk, sk);
+ bool needs_ao = !!tp->af_specific->ao_lookup(sk, sk, -1, -1);
+
+ if (needs_md5 && needs_ao)
+ return -EKEYREJECTED;
+
+ /* If we have a matching md5 key and no matching tcp-ao key
+ * then free up ao_info if allocated.
+ */
+ if (needs_md5) {
+ tcp_ao_destroy_sock(sk);
+ } else if (needs_ao) {
+ tcp_clear_md5_list(sk);
+ kfree(rcu_replace_pointer(tp->md5sig_info, NULL,
+ lockdep_sock_is_held(sk)));
+ }
+ }
+#endif
+#ifdef CONFIG_TCP_AO
+ if (unlikely(rcu_dereference_protected(tp->ao_info,
+ lockdep_sock_is_held(sk)))) {
+ /* Don't allow connecting if ao is configured but no
+ * matching key is found.
+ */
+ if (!tp->af_specific->ao_lookup(sk, sk, -1, -1))
+ return -EKEYREJECTED;
+ }
+#endif
+
if (inet_csk(sk)->icsk_af_ops->rebuild_header(sk))
return -EHOSTUNREACH; /* Routing failure or similar. */

diff --git a/net/ipv6/tcp_ao.c b/net/ipv6/tcp_ao.c
index 049ddbabe049..a56ab0a9cf55 100644
--- a/net/ipv6/tcp_ao.c
+++ b/net/ipv6/tcp_ao.c
@@ -12,6 +12,23 @@
#include <net/tcp.h>
#include <net/ipv6.h>

+struct tcp_ao_key *tcp_v6_ao_do_lookup(const struct sock *sk,
+ const struct in6_addr *addr,
+ int sndid, int rcvid)
+{
+ return tcp_ao_do_lookup(sk, (union tcp_ao_addr *)addr, AF_INET6,
+ sndid, rcvid, 0);
+}
+
+struct tcp_ao_key *tcp_v6_ao_lookup(const struct sock *sk,
+ struct sock *addr_sk,
+ int sndid, int rcvid)
+{
+ struct in6_addr *addr = &addr_sk->sk_v6_daddr;
+
+ return tcp_v6_ao_do_lookup(sk, addr, sndid, rcvid);
+}
+
int tcp_v6_parse_ao(struct sock *sk, int cmd,
sockptr_t optval, int optlen)
{
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9edb84eb457b..8b38c112affb 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -601,6 +601,7 @@ static int tcp_v6_parse_md5_keys(struct sock *sk, int optname,
{
struct tcp_md5sig cmd;
struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)&cmd.tcpm_addr;
+ union tcp_ao_addr *addr;
int l3index = 0;
u8 prefixlen;
u8 flags;
@@ -655,13 +656,24 @@ static int tcp_v6_parse_md5_keys(struct sock *sk, int optname,
if (cmd.tcpm_keylen > TCP_MD5SIG_MAXKEYLEN)
return -EINVAL;

- if (ipv6_addr_v4mapped(&sin6->sin6_addr))
- return tcp_md5_do_add(sk, (union tcp_md5_addr *)&sin6->sin6_addr.s6_addr32[3],
+ if (ipv6_addr_v4mapped(&sin6->sin6_addr)) {
+ addr = (union tcp_md5_addr *)&sin6->sin6_addr.s6_addr32[3];
+
+ /* Don't allow keys for peers that have a matching TCP-AO key */
+ if (tcp_ao_do_lookup(sk, addr, AF_INET, -1, -1, 0))
+ return -EKEYREJECTED;
+ return tcp_md5_do_add(sk, addr,
AF_INET, prefixlen, l3index, flags,
cmd.tcpm_key, cmd.tcpm_keylen);
+ }
+
+ addr = (union tcp_md5_addr *)&sin6->sin6_addr;
+
+ /* Don't allow keys for peers that have a matching TCP-AO key */
+ if (tcp_ao_do_lookup(sk, addr, AF_INET6, -1, -1, 0))
+ return -EKEYREJECTED;

- return tcp_md5_do_add(sk, (union tcp_md5_addr *)&sin6->sin6_addr,
- AF_INET6, prefixlen, l3index, flags,
+ return tcp_md5_do_add(sk, addr, AF_INET6, prefixlen, l3index, flags,
cmd.tcpm_key, cmd.tcpm_keylen);
}

@@ -1902,6 +1914,7 @@ static const struct tcp_sock_af_ops tcp_sock_ipv6_specific = {
.md5_parse = tcp_v6_parse_md5_keys,
#endif
#ifdef CONFIG_TCP_AO
+ .ao_lookup = tcp_v6_ao_lookup,
.ao_parse = tcp_v6_parse_ao,
#endif
};
@@ -1933,6 +1946,7 @@ static const struct tcp_sock_af_ops tcp_sock_ipv6_mapped_specific = {
.md5_parse = tcp_v6_parse_md5_keys,
#endif
#ifdef CONFIG_TCP_AO
+ .ao_lookup = tcp_v6_ao_lookup,
.ao_parse = tcp_v6_parse_ao,
#endif
};
--
2.39.1

Next message: Dmitry Safonov: "[PATCH v4 05/21] net/tcp: Calculate TCP-AO traffic keys"
Previous message: Dmitry Safonov: "[PATCH v4 03/21] net/tcp: Introduce TCP_AO setsockopt()s"
In reply to: Dmitry Safonov: "[PATCH v4 03/21] net/tcp: Introduce TCP_AO setsockopt()s"
Next in thread: Dmitry Safonov: "[PATCH v4 05/21] net/tcp: Calculate TCP-AO traffic keys"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]