Skip to content

Commit

Permalink
net/tcp: Prevent TCP-MD5 with TCP-AO being set
Browse files Browse the repository at this point in the history
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@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>
Acked-by: David Ahern <dsahern@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Dmitry Safonov authored and David S. Miller committed Oct 27, 2023
1 parent 4954f17 commit 0aadc73
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 9 deletions.
43 changes: 41 additions & 2 deletions include/net/tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,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);

Expand All @@ -1786,14 +1787,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
Expand All @@ -1811,6 +1821,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,
Expand Down Expand Up @@ -2177,6 +2194,9 @@ struct tcp_sock_af_ops {
#endif
#ifdef CONFIG_TCP_AO
int (*ao_parse)(struct sock *sk, 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
};

Expand Down Expand Up @@ -2588,4 +2608,23 @@ static inline u64 tcp_transmit_time(const struct sock *sk)
return 0;
}

static inline bool tcp_ao_required(struct sock *sk, const void *saddr,
int family)
{
#ifdef CONFIG_TCP_AO
struct tcp_ao_info *ao_info;
struct tcp_ao_key *ao_key;

ao_info = rcu_dereference_check(tcp_sk(sk)->ao_info,
lockdep_sock_is_held(sk));
if (!ao_info)
return false;

ao_key = tcp_ao_do_lookup(sk, saddr, family, -1, -1);
if (ao_info->ao_required || ao_key)
return true;
#endif
return false;
}

#endif /* _TCP_H */
13 changes: 13 additions & 0 deletions include/net/tcp_ao.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,24 @@ 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);
/* ipv4 specific functions */
int tcp_v4_parse_ao(struct sock *sk, int cmd, 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 */
int tcp_v6_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen);
struct tcp_ao_key *tcp_v6_ao_lookup(const struct sock *sk,
struct sock *addr_sk, int sndid, int rcvid);
#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)
{
return NULL;
}

static inline void tcp_ao_destroy_sock(struct sock *sk)
{
}
Expand Down
47 changes: 47 additions & 0 deletions net/ipv4/tcp_ao.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ static struct tcp_ao_key *__tcp_ao_do_lookup(const struct sock *sk,
return NULL;
}

struct tcp_ao_key *tcp_ao_do_lookup(const struct sock *sk,
const union tcp_ao_addr *addr,
int family, int sndid, int rcvid)
{
return __tcp_ao_do_lookup(sk, addr, family, U8_MAX, sndid, rcvid);
}

static struct tcp_ao_info *tcp_ao_alloc_info(gfp_t flags)
{
struct tcp_ao_info *ao;
Expand Down Expand Up @@ -162,6 +169,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);
}

static bool tcp_ao_can_set_current_rnext(struct sock *sk)
{
/* There aren't current/rnext keys on TCP_LISTEN sockets */
Expand Down Expand Up @@ -497,6 +512,10 @@ static int tcp_ao_add_cmd(struct sock *sk, unsigned short int family,
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 = setsockopt_ao_info(sk);
if (IS_ERR(ao_info))
return PTR_ERR(ao_info);
Expand Down Expand Up @@ -698,6 +717,31 @@ static int tcp_ao_del_cmd(struct sock *sk, unsigned short int family,
return -ENOENT;
}

/* cmd.ao_required makes a socket TCP-AO only.
* Don't allow any md5 keys for any l3intf on the socket together with it.
* Restricting it early in setsockopt() removes a check for
* ao_info->ao_required on inbound tcp segment fast-path.
*/
static int tcp_ao_required_verify(struct sock *sk)
{
#ifdef CONFIG_TCP_MD5SIG
const struct tcp_md5sig_info *md5sig;

if (!static_branch_unlikely(&tcp_md5_needed.key))
return 0;

md5sig = rcu_dereference_check(tcp_sk(sk)->md5sig_info,
lockdep_sock_is_held(sk));
if (!md5sig)
return 0;

if (rcu_dereference_check(hlist_first_rcu(&md5sig->head),
lockdep_sock_is_held(sk)))
return 1;
#endif
return 0;
}

static int tcp_ao_info_cmd(struct sock *sk, unsigned short int family,
sockptr_t optval, int optlen)
{
Expand Down Expand Up @@ -732,6 +776,9 @@ static int tcp_ao_info_cmd(struct sock *sk, unsigned short int family,
first = true;
}

if (cmd.ao_required && tcp_ao_required_verify(sk))
return -EKEYREJECTED;

/* For sockets in TCP_CLOSED it's possible set keys that aren't
* matching the future peer (address/port/VRF/etc),
* tcp_ao_connect_init() will choose a correct matching MKT
Expand Down
14 changes: 11 additions & 3 deletions net/ipv4/tcp_ipv4.c
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,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;
Expand All @@ -1101,7 +1101,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);
Expand Down Expand Up @@ -1313,7 +1314,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;
Expand Down Expand Up @@ -1383,6 +1384,12 @@ 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.
* See the comment in tcp_ao_add_cmd()
*/
if (tcp_ao_required(sk, addr, AF_INET))
return -EKEYREJECTED;

return tcp_md5_do_add(sk, addr, AF_INET, prefixlen, l3index, flags,
cmd.tcpm_key, cmd.tcpm_keylen);
}
Expand Down Expand Up @@ -2279,6 +2286,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
};
Expand Down
47 changes: 47 additions & 0 deletions net/ipv4/tcp_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -3931,6 +3931,53 @@ 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_ao = !!tp->af_specific->ao_lookup(sk, sk, -1, -1);
bool needs_md5 = !!tp->af_specific->md5_lookup(sk, sk);
struct tcp_ao_info *ao_info;

ao_info = rcu_dereference_check(tp->ao_info,
lockdep_sock_is_held(sk));
if (ao_info) {
/* This is an extra check: tcp_ao_required() in
* tcp_v{4,6}_parse_md5_keys() should prevent adding
* md5 keys on ao_required socket.
*/
needs_ao |= ao_info->ao_required;
WARN_ON_ONCE(ao_info->ao_required && needs_md5);
}
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. */

Expand Down
17 changes: 17 additions & 0 deletions net/ipv6/tcp_ao.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,23 @@
#include <net/tcp.h>
#include <net/ipv6.h>

static 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);
}

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)
{
Expand Down
26 changes: 22 additions & 4 deletions net/ipv6/tcp_ipv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,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;
Expand Down Expand Up @@ -654,13 +655,28 @@ 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.
* See the comment in tcp_ao_add_cmd()
*/
if (tcp_ao_required(sk, addr, AF_INET))
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.
* See the comment in tcp_ao_add_cmd()
*/
if (tcp_ao_required(sk, addr, AF_INET6))
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);
}

Expand Down Expand Up @@ -1903,6 +1919,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
};
Expand Down Expand Up @@ -1934,6 +1951,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
};
Expand Down

0 comments on commit 0aadc73

Please sign in to comment.