From: Dmitry Safonov <dima@arista.com>
To: 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: linux-kernel@vger.kernel.org, Dmitry Safonov <dima@arista.com>,
Andy Lutomirski <luto@amacapital.net>,
Ard Biesheuvel <ardb@kernel.org>,
Bob Gilligan <gilligan@arista.com>,
Dan Carpenter <error27@gmail.com>,
David Laight <David.Laight@aculab.com>,
Dmitry Safonov <0x7f454c46@gmail.com>,
Donald Cassidy <dcassidy@redhat.com>,
Eric Biggers <ebiggers@kernel.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Francesco Ruggeri <fruggeri05@gmail.com>,
"Gaillardetz, Dominik" <dgaillar@ciena.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
Ivan Delalande <colona@arista.com>,
Leonard Crestez <cdleonard@gmail.com>,
"Nassiri, Mohammad" <mnassiri@ciena.com>,
Salam Noureddine <noureddine@arista.com>,
Simon Horman <simon.horman@corigine.com>,
"Tetreault, Francois" <ftetreau@ciena.com>,
netdev@vger.kernel.org
Subject: [PATCH v11 net-next 16/23] net/tcp: Ignore specific ICMPs for TCP-AO connections
Date: Mon, 11 Sep 2023 22:03:36 +0100 [thread overview]
Message-ID: <20230911210346.301750-17-dima@arista.com> (raw)
In-Reply-To: <20230911210346.301750-1-dima@arista.com>
Similarly to IPsec, RFC5925 prescribes:
">> A TCP-AO implementation MUST default to ignore incoming ICMPv4
messages of Type 3 (destination unreachable), Codes 2-4 (protocol
unreachable, port unreachable, and fragmentation needed -- ’hard
errors’), and ICMPv6 Type 1 (destination unreachable), Code 1
(administratively prohibited) and Code 4 (port unreachable) intended
for connections in synchronized states (ESTABLISHED, FIN-WAIT-1, FIN-
WAIT-2, CLOSE-WAIT, CLOSING, LAST-ACK, TIME-WAIT) that match MKTs."
A selftest (later in patch series) verifies that this attack is not
possible in this TCP-AO implementation.
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>
---
include/net/tcp_ao.h | 10 ++++++-
include/uapi/linux/snmp.h | 1 +
include/uapi/linux/tcp.h | 4 ++-
net/ipv4/proc.c | 1 +
net/ipv4/tcp_ao.c | 58 +++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_ipv4.c | 7 +++++
net/ipv6/tcp_ipv6.c | 7 +++++
7 files changed, 86 insertions(+), 2 deletions(-)
diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
index 5c5d16b6f9f9..4c290c647272 100644
--- a/include/net/tcp_ao.h
+++ b/include/net/tcp_ao.h
@@ -24,6 +24,7 @@ struct tcp_ao_counters {
atomic64_t pkt_bad;
atomic64_t key_not_found;
atomic64_t ao_required;
+ atomic64_t dropped_icmp;
};
struct tcp_ao_key {
@@ -92,7 +93,8 @@ struct tcp_ao_info {
struct tcp_ao_key *rnext_key;
struct tcp_ao_counters counters;
u32 ao_required :1,
- __unused :31;
+ accept_icmps :1,
+ __unused :30;
__be32 lisn;
__be32 risn;
/* Sequence Number Extension (SNE) are upper 4 bytes for SEQ,
@@ -189,6 +191,7 @@ int tcp_ao_calc_traffic_key(struct tcp_ao_key *mkt, u8 *key, void *ctx,
unsigned int len, struct tcp_sigpool *hp);
void tcp_ao_destroy_sock(struct sock *sk, bool twsk);
void tcp_ao_time_wait(struct tcp_timewait_sock *tcptw, struct tcp_sock *tp);
+bool tcp_ao_ignore_icmp(const struct sock *sk, int type, int code);
enum skb_drop_reason tcp_inbound_ao_hash(struct sock *sk,
const struct sk_buff *skb, unsigned short int family,
const struct request_sock *req,
@@ -264,6 +267,11 @@ static inline void tcp_ao_syncookie(struct sock *sk, const struct sk_buff *skb,
{
}
+static inline bool tcp_ao_ignore_icmp(const struct sock *sk, int type, int code)
+{
+ return false;
+}
+
static inline enum skb_drop_reason tcp_inbound_ao_hash(struct sock *sk,
const struct sk_buff *skb, unsigned short int family,
const struct request_sock *req, const struct tcp_ao_hdr *aoh)
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 06ddf4cd295c..47a6b47da66f 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -300,6 +300,7 @@ enum
LINUX_MIB_TCPAOBAD, /* TCPAOBad */
LINUX_MIB_TCPAOKEYNOTFOUND, /* TCPAOKeyNotFound */
LINUX_MIB_TCPAOGOOD, /* TCPAOGood */
+ LINUX_MIB_TCPAODROPPEDICMPS, /* TCPAODroppedIcmps */
__LINUX_MIB_MAX
};
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 3fe0612ec59a..ca7ed18ce67b 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -392,7 +392,8 @@ struct tcp_ao_info_opt { /* setsockopt(TCP_AO_INFO) */
set_rnext :1, /* corresponding ::rnext */
ao_required :1, /* don't accept non-AO connects */
set_counters :1, /* set/clear ::pkt_* counters */
- reserved :28; /* must be 0 */
+ accept_icmps :1, /* accept incoming ICMPs */
+ reserved :27; /* must be 0 */
__u16 reserved2; /* padding, must be 0 */
__u8 current_key; /* KeyID to set as Current_key */
__u8 rnext; /* KeyID to set as Rnext_key */
@@ -400,6 +401,7 @@ struct tcp_ao_info_opt { /* setsockopt(TCP_AO_INFO) */
__u64 pkt_bad; /* failed verification */
__u64 pkt_key_not_found; /* could not find a key to verify */
__u64 pkt_ao_required; /* segments missing TCP-AO sign */
+ __u64 pkt_dropped_icmp; /* ICMPs that were ignored */
} __attribute__((aligned(8)));
/* setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) */
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 3f643cd29cfe..5d3c9c96773e 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -302,6 +302,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPAOBad", LINUX_MIB_TCPAOBAD),
SNMP_MIB_ITEM("TCPAOKeyNotFound", LINUX_MIB_TCPAOKEYNOTFOUND),
SNMP_MIB_ITEM("TCPAOGood", LINUX_MIB_TCPAOGOOD),
+ SNMP_MIB_ITEM("TCPAODroppedIcmps", LINUX_MIB_TCPAODROPPEDICMPS),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index 2283203f1ac5..a8af93972ee5 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -15,6 +15,7 @@
#include <net/tcp.h>
#include <net/ipv6.h>
+#include <net/icmp.h>
int tcp_ao_calc_traffic_key(struct tcp_ao_key *mkt, u8 *key, void *ctx,
unsigned int len, struct tcp_sigpool *hp)
@@ -44,6 +45,60 @@ int tcp_ao_calc_traffic_key(struct tcp_ao_key *mkt, u8 *key, void *ctx,
return 1;
}
+bool tcp_ao_ignore_icmp(const struct sock *sk, int type, int code)
+{
+ bool ignore_icmp = false;
+ struct tcp_ao_info *ao;
+
+ /* RFC5925, 7.8:
+ * >> A TCP-AO implementation MUST default to ignore incoming ICMPv4
+ * messages of Type 3 (destination unreachable), Codes 2-4 (protocol
+ * unreachable, port unreachable, and fragmentation needed -- ’hard
+ * errors’), and ICMPv6 Type 1 (destination unreachable), Code 1
+ * (administratively prohibited) and Code 4 (port unreachable) intended
+ * for connections in synchronized states (ESTABLISHED, FIN-WAIT-1, FIN-
+ * WAIT-2, CLOSE-WAIT, CLOSING, LAST-ACK, TIME-WAIT) that match MKTs.
+ */
+ if (sk->sk_family == AF_INET) {
+ if (type != ICMP_DEST_UNREACH)
+ return false;
+ if (code < ICMP_PROT_UNREACH || code > ICMP_FRAG_NEEDED)
+ return false;
+ } else {
+ if (type != ICMPV6_DEST_UNREACH)
+ return false;
+ if (code != ICMPV6_ADM_PROHIBITED && code != ICMPV6_PORT_UNREACH)
+ return false;
+ }
+
+ rcu_read_lock();
+ switch (sk->sk_state) {
+ case TCP_TIME_WAIT:
+ ao = rcu_dereference(tcp_twsk(sk)->ao_info);
+ break;
+ case TCP_SYN_SENT:
+ case TCP_SYN_RECV:
+ case TCP_LISTEN:
+ case TCP_NEW_SYN_RECV:
+ /* RFC5925 specifies to ignore ICMPs *only* on connections
+ * in synchronized states.
+ */
+ rcu_read_unlock();
+ return false;
+ default:
+ ao = rcu_dereference(tcp_sk(sk)->ao_info);
+ }
+
+ if (ao && !ao->accept_icmps) {
+ ignore_icmp = true;
+ __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAODROPPEDICMPS);
+ atomic64_inc(&ao->counters.dropped_icmp);
+ }
+ rcu_read_unlock();
+
+ return ignore_icmp;
+}
+
/* Optimized version of tcp_ao_do_lookup(): only for sockets for which
* it's known that the keys in ao_info are matching peer's
* family/address/VRF/etc.
@@ -1035,6 +1090,7 @@ int tcp_ao_copy_all_matching(const struct sock *sk, struct sock *newsk,
new_ao->lisn = htonl(tcp_rsk(req)->snt_isn);
new_ao->risn = htonl(tcp_rsk(req)->rcv_isn);
new_ao->ao_required = ao->ao_required;
+ new_ao->accept_icmps = ao->accept_icmps;
if (family == AF_INET) {
addr = (union tcp_ao_addr *)&newsk->sk_daddr;
@@ -1741,9 +1797,11 @@ static int tcp_ao_info_cmd(struct sock *sk, unsigned short int family,
atomic64_set(&ao_info->counters.pkt_bad, cmd.pkt_bad);
atomic64_set(&ao_info->counters.key_not_found, cmd.pkt_key_not_found);
atomic64_set(&ao_info->counters.ao_required, cmd.pkt_ao_required);
+ atomic64_set(&ao_info->counters.dropped_icmp, cmd.pkt_dropped_icmp);
}
ao_info->ao_required = cmd.ao_required;
+ ao_info->accept_icmps = cmd.accept_icmps;
if (new_current)
WRITE_ONCE(ao_info->current_key, new_current);
if (new_rnext)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0f702b33fdef..a4aef27b0f72 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -493,6 +493,8 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
return -ENOENT;
}
if (sk->sk_state == TCP_TIME_WAIT) {
+ /* To increase the counter of ignored icmps for TCP-AO */
+ tcp_ao_ignore_icmp(sk, type, code);
inet_twsk_put(inet_twsk(sk));
return 0;
}
@@ -506,6 +508,11 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
return 0;
}
+ if (tcp_ao_ignore_icmp(sk, type, code)) {
+ sock_put(sk);
+ return 0;
+ }
+
bh_lock_sock(sk);
/* If too many ICMPs get dropped on busy
* servers this needs to be solved differently.
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 59719c8bc5ac..60c2129890ec 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -395,6 +395,8 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
}
if (sk->sk_state == TCP_TIME_WAIT) {
+ /* To increase the counter of ignored icmps for TCP-AO */
+ tcp_ao_ignore_icmp(sk, type, code);
inet_twsk_put(inet_twsk(sk));
return 0;
}
@@ -405,6 +407,11 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
return 0;
}
+ if (tcp_ao_ignore_icmp(sk, type, code)) {
+ sock_put(sk);
+ return 0;
+ }
+
bh_lock_sock(sk);
if (sock_owned_by_user(sk) && type != ICMPV6_PKT_TOOBIG)
__NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
--
2.41.0
next prev parent reply other threads:[~2023-09-12 2:29 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-11 21:03 [PATCH v11 net-next 00/23] net/tcp: Add TCP-AO support Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 01/23] net/tcp: Prepare tcp_md5sig_pool for TCP-AO Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 02/23] net/tcp: Add TCP-AO config and structures Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 03/23] net/tcp: Introduce TCP_AO setsockopt()s Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 04/23] net/tcp: Prevent TCP-MD5 with TCP-AO being set Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 05/23] net/tcp: Calculate TCP-AO traffic keys Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing packets Dmitry Safonov
2023-09-12 16:47 ` Eric Dumazet
2023-09-12 20:19 ` Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 07/23] net/tcp: Add tcp_parse_auth_options() Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 08/23] net/tcp: Add AO sign to RST packets Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 09/23] net/tcp: Add TCP-AO sign to twsk Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 10/23] net/tcp: Wire TCP-AO to request sockets Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 11/23] net/tcp: Sign SYN-ACK segments with TCP-AO Dmitry Safonov
2023-09-12 16:47 ` Eric Dumazet
2023-09-12 20:26 ` Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 12/23] net/tcp: Verify inbound TCP-AO signed segments Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 13/23] net/tcp: Add TCP-AO segments counters Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 14/23] net/tcp: Add TCP-AO SNE support Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 15/23] net/tcp: Add tcp_hash_fail() ratelimited logs Dmitry Safonov
2023-09-11 21:03 ` Dmitry Safonov [this message]
2023-09-11 21:03 ` [PATCH v11 net-next 17/23] net/tcp: Add option for TCP-AO to (not) hash header Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 18/23] net/tcp: Add TCP-AO getsockopt()s Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 19/23] net/tcp: Allow asynchronous delete for TCP-AO keys (MKTs) Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 20/23] net/tcp: Add static_key for TCP-AO Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 21/23] net/tcp: Wire up l3index to TCP-AO Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 22/23] net/tcp: Add TCP_AO_REPAIR Dmitry Safonov
2023-09-11 21:03 ` [PATCH v11 net-next 23/23] Documentation/tcp: Add TCP-AO documentation 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=20230911210346.301750-17-dima@arista.com \
--to=dima@arista.com \
--cc=0x7f454c46@gmail.com \
--cc=David.Laight@aculab.com \
--cc=ardb@kernel.org \
--cc=cdleonard@gmail.com \
--cc=colona@arista.com \
--cc=davem@davemloft.net \
--cc=dcassidy@redhat.com \
--cc=dgaillar@ciena.com \
--cc=dsahern@kernel.org \
--cc=ebiederm@xmission.com \
--cc=ebiggers@kernel.org \
--cc=edumazet@google.com \
--cc=error27@gmail.com \
--cc=fruggeri05@gmail.com \
--cc=ftetreau@ciena.com \
--cc=gilligan@arista.com \
--cc=herbert@gondor.apana.org.au \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mnassiri@ciena.com \
--cc=netdev@vger.kernel.org \
--cc=noureddine@arista.com \
--cc=pabeni@redhat.com \
--cc=simon.horman@corigine.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;
as well as URLs for NNTP newsgroup(s).