From patchwork Sun Dec 19 00:28:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antony Antony X-Patchwork-Id: 12686571 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1160C433F5 for ; Sun, 19 Dec 2021 00:28:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231617AbhLSA21 (ORCPT ); Sat, 18 Dec 2021 19:28:27 -0500 Received: from a.mx.secunet.com ([62.96.220.36]:58956 "EHLO a.mx.secunet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229911AbhLSA21 (ORCPT ); Sat, 18 Dec 2021 19:28:27 -0500 Received: from localhost (localhost [127.0.0.1]) by a.mx.secunet.com (Postfix) with ESMTP id C1FF82058E; Sun, 19 Dec 2021 01:28:25 +0100 (CET) X-Virus-Scanned: by secunet Received: from a.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id G-NSGS0I67UO; Sun, 19 Dec 2021 01:28:25 +0100 (CET) Received: from mailout1.secunet.com (mailout1.secunet.com [62.96.220.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by a.mx.secunet.com (Postfix) with ESMTPS id 0AC0B2053D; Sun, 19 Dec 2021 01:28:25 +0100 (CET) Received: from cas-essen-02.secunet.de (unknown [10.53.40.202]) by mailout1.secunet.com (Postfix) with ESMTP id EB08780004A; Sun, 19 Dec 2021 01:28:24 +0100 (CET) Received: from mbx-essen-02.secunet.de (10.53.40.198) by cas-essen-02.secunet.de (10.53.40.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Sun, 19 Dec 2021 01:28:24 +0100 Received: from moon.secunet.de (172.18.149.1) by mbx-essen-02.secunet.de (10.53.40.198) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17; Sun, 19 Dec 2021 01:28:24 +0100 Date: Sun, 19 Dec 2021 01:28:16 +0100 From: Antony Antony To: Steffen Klassert CC: Herbert Xu , Antony Antony , "David S. Miller" , Subject: [RFC PATCH ipsec-next] xfrm: add forwarding ICMP error message Message-ID: Reply-To: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Organization: secunet X-ClientProxiedBy: cas-essen-01.secunet.de (10.53.40.201) To mbx-essen-02.secunet.de (10.53.40.198) X-EXCLAIMER-MD-CONFIG: 2c86f778-e09b-4440-8b15-867914633a10 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org X-Patchwork-State: RFC IETF RFC 4301, Section 6, requires a configurable option to forward unauthenticated ICMP error message that does not match any policies using. Use reverse of ICMP payload, which will be partial IP packet. Add this reverse lookup (using ICMP payload as skb) for xfrm forward path. To enable this add the flag XFRM_POLICY_ICMP to fwd and out policy and the flag XFRM_STATE_ICMP on incoming SA. ip xfrm policy add flag icmp tmpl ip xfrm policy src 192.0.2.0/24 dst 192.0.1.0/25 dir fwd priority 2084302 ptype main flag icmp ip xfrm state add ...flag icmp ip xfrm state root@west:~#ip x s src 192.1.2.23 dst 192.1.2.45 proto esp spi 0xa7b76872 reqid 16389 mode tunnel replay-window 32 flag icmp af-unspec Signed-off-by: Antony Antony --- net/xfrm/xfrm_policy.c | 137 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 2 deletions(-) -- 2.30.2 diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 9341298b2a70..8505c36413c7 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -3475,6 +3476,123 @@ static inline int secpath_has_nontransport(const struct sec_path *sp, int k, int return 0; } +static bool icmp_err_packet(const struct flowi *fl, unsigned short family) +{ + const struct flowi6 *fl6 = &fl->u.ip6; + const struct flowi4 *fl4 = &fl->u.ip4; + + if (family == AF_INET) + if (fl4->flowi4_proto == IPPROTO_ICMP && + (fl4->fl4_icmp_type == ICMP_DEST_UNREACH || + fl4->fl4_icmp_type == ICMP_TIME_EXCEEDED)) + return false; + +#if IS_ENABLED(CONFIG_IPV6) + if (fl6->flowi6_proto == IPPROTO_ICMPV6 && + (fl6->fl6_icmp_type == ICMPV6_DEST_UNREACH || + fl6->fl6_icmp_type == ICMPV6_TIME_EXCEED)) + return false; +#endif + return true; +} + +static struct sk_buff *xfrm_icmp_flow_decode(struct sk_buff *skb, + unsigned short family, + struct flowi *fl, + struct flowi *fl1) +{ + struct net *net = dev_net(skb->dev); + struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC); + + if (!pskb_pull(newskb, (sizeof(struct iphdr) + sizeof(struct icmphdr)))) + return NULL; + skb_reset_network_header(newskb); + + if (xfrm_decode_session_reverse(newskb, fl1, family) < 0) { + kfree_skb(newskb); + XFRM_INC_STATS(net, LINUX_MIB_XFRMINHDRERROR); + return NULL; + } + + /* inherit more from the old flow ??? + * the inner skb may have these values different from outer skb + */ + + fl1->flowi_oif = fl->flowi_oif; + fl1->flowi_mark = fl->flowi_mark; + fl1->flowi_tos = fl->flowi_tos; + nf_nat_decode_session(newskb, fl1, family); + + return newskb; +} + +static bool xfrm_sa_icmp_flow(struct sk_buff *skb, + unsigned short family, const struct xfrm_selector *sel, + struct flowi *fl) +{ + bool ret = false; + + if (!icmp_err_packet(fl, family)) { + struct flowi fl1; + struct sk_buff *newskb = xfrm_icmp_flow_decode(skb, family, fl, &fl1); + + if (!newskb) + return ret; + + ret = xfrm_selector_match(sel, &fl1, family); + kfree_skb(newskb); + } + + return ret; +} + +static inline +struct xfrm_policy *xfrm_in_fwd_icmp(struct sk_buff *skb, struct flowi *fl, + unsigned short family, u32 if_id) +{ + struct xfrm_policy *pol = NULL; + + if (!icmp_err_packet(fl, family)) { + struct flowi fl1; + struct net *net = dev_net(skb->dev); + struct sk_buff *newskb = xfrm_icmp_flow_decode(skb, family, fl, &fl1); + + if (!newskb) + return pol; + pol = xfrm_policy_lookup(net, &fl1, family, XFRM_POLICY_FWD, if_id); + kfree_skb(newskb); + } + + return pol; +} + +static inline +struct dst_entry *xfrm_out_fwd_icmp(struct sk_buff *skb, struct flowi *fl, + unsigned short family, struct dst_entry *dst) +{ + if (!icmp_err_packet(fl, family)) { + struct net *net = dev_net(skb->dev); + struct dst_entry *dst2; + struct flowi fl1; + struct sk_buff *newskb = xfrm_icmp_flow_decode(skb, family, fl, &fl1); + + if (!newskb) + return dst; + + dst2 = xfrm_lookup(net, dst, &fl1, NULL, XFRM_LOOKUP_QUEUE); + + kfree_skb(newskb); + + if (IS_ERR(dst2)) + return dst; + + if (dst2->xfrm) + dst = dst2; + } + + return dst; +} + int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, unsigned short family) { @@ -3521,9 +3639,17 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, for (i = sp->len - 1; i >= 0; i--) { struct xfrm_state *x = sp->xvec[i]; + int ret = 0; + if (!xfrm_selector_match(&x->sel, &fl, family)) { - XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH); - return 0; + ret = true; + if (x->props.flags & XFRM_STATE_ICMP && + xfrm_sa_icmp_flow(skb, family, &x->sel, &fl)) + ret = false; + if (ret) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMINSTATEMISMATCH); + return 0; + } } } } @@ -3546,6 +3672,9 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, return 0; } + if (!pol && dir == XFRM_POLICY_FWD) + pol = xfrm_in_fwd_icmp(skb, &fl, family, if_id); + if (!pol) { if (!xfrm_default_allow(net, dir)) { XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS); @@ -3675,6 +3804,10 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family) res = 0; dst = NULL; } + + if (dst && !dst->xfrm) + dst = xfrm_out_fwd_icmp(skb, &fl, family, dst); + skb_dst_set(skb, dst); return res; } From patchwork Thu Oct 26 14:45:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antony Antony X-Patchwork-Id: 13437632 X-Patchwork-Delegate: kuba@kernel.org Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D982B8829 for ; Thu, 26 Oct 2023 14:45:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from a.mx.secunet.com (a.mx.secunet.com [62.96.220.36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F41291AA for ; Thu, 26 Oct 2023 07:45:57 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by a.mx.secunet.com (Postfix) with ESMTP id BEA6720743; Thu, 26 Oct 2023 16:45:56 +0200 (CEST) X-Virus-Scanned: by secunet Received: from a.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tzVISsJ9N-Bk; Thu, 26 Oct 2023 16:45:56 +0200 (CEST) Received: from mailout1.secunet.com (mailout1.secunet.com [62.96.220.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by a.mx.secunet.com (Postfix) with ESMTPS id 237C4206B0; Thu, 26 Oct 2023 16:45:56 +0200 (CEST) Received: from cas-essen-01.secunet.de (unknown [10.53.40.201]) by mailout1.secunet.com (Postfix) with ESMTP id 17A1780004A; Thu, 26 Oct 2023 16:45:56 +0200 (CEST) Received: from mbx-essen-02.secunet.de (10.53.40.198) by cas-essen-01.secunet.de (10.53.40.201) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.32; Thu, 26 Oct 2023 16:45:55 +0200 Received: from moon.secunet.de (172.18.149.1) by mbx-essen-02.secunet.de (10.53.40.198) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.32; Thu, 26 Oct 2023 16:45:55 +0200 Date: Thu, 26 Oct 2023 16:45:49 +0200 From: Antony Antony To: Steffen Klassert CC: Herbert Xu , Antony Antony , "David S. Miller" , , Jakub Kicinski , Subject: [PATCH ipsec-next 2/2] xfrm: fix source address in icmp error generation from IPsec gateway Message-ID: Reply-To: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: first-class Priority: normal Organization: secunet X-ClientProxiedBy: cas-essen-01.secunet.de (10.53.40.201) To mbx-essen-02.secunet.de (10.53.40.198) X-EXCLAIMER-MD-CONFIG: 2c86f778-e09b-4440-8b15-867914633a10 X-Patchwork-Delegate: kuba@kernel.org When enabling support for xfrm lookup using reverse ICMP payload, We have identified an issue where the source address of the IPv4 e.g "Destination Host Unreachable" message is incorrect. The IPv6 appear to do the right thing. Here is example of incorrect source address for ICMP error response. When sending a ping to an unreachable host, the sender would receive an ICMP unreachable response with a fake source address. Rather the address of the host that generated ICMP Unreachable message. This is confusing and incorrect. Example: ping -W 9 -w 5 -c 1 10.1.4.3 PING 10.1.4.3 (10.1.4.3) 56(84) bytes of data. >From 10.1.4.3 icmp_seq=1 Destination Host Unreachable Notice : packet has the source address of the ICMP "Unreachable host!" This issue can be traced back to commit 415b3334a21a ("icmp: Fix regression in nexthop resolution during replies.") which introduced a change that copied the source address from the ICMP payload. This commit would force to use source address from the gatway/host. The ICMP error message source address correctly set from the host. After fixing: ping -W 5 -c 1 10.1.4.3 PING 10.1.4.3 (10.1.4.3) 56(84) bytes of data. >From 10.1.3.2 icmp_seq=1 Destination Host Unreachable Here is an example to reporduce: export AB="10.1" for i in 1 2 3 4 5; do h="host${i}" ip netns add ${h} ip -netns ${h} link set lo up ip netns exec ${h} sysctl -wq net.ipv4.ip_forward=1 if [ $i -lt 5 ]; then ip -netns ${h} link add eth0 type veth peer name eth10${i} ip -netns ${h} addr add "${AB}.${i}.1/24" dev eth0 ip -netns ${h} link set up dev eth0 fi done for i in 1 2 3 4 5; do h="host${i}" p=$((i - 1)) ph="host${p}" # connect to previous host if [ $i -gt 1 ]; then ip -netns ${ph} link set eth10${p} netns ${h} ip -netns ${h} link set eth10${p} name eth1 ip -netns ${h} link set up dev eth1 ip -netns ${h} addr add "${AB}.${p}.2/24" dev eth1 fi # add forward routes for k in $(seq ${i} $((5 - 1))); do ip -netns ${h} route 2>/dev/null | (grep "${AB}.${k}.0" 2>/dev/null) || \ ip -netns ${h} route add "${AB}.${k}.0/24" via "${AB}.${i}.2" 2>/dev/nul done # add reverse routes for k in $(seq 1 $((i - 2))); do ip -netns ${h} route 2>/dev/null | grep "${AB}.${k}.0" 2>/dev/null || \ ip -netns ${h} route add "${AB}.${k}.0/24" via "${AB}.${p}.1" 2>/dev/nul done done ip netns exec host1 ping -q -W 2 -w 1 -c 1 10.1.4.2 2>&1>/dev/null && echo "success 10.1.4.2 reachable" || echo "ERROR" ip netns exec host1 ping -W 9 -w 5 -c 1 10.1.4.3 || echo "note the source address of unreachble" ip -netns host1 route flush cache ip netns exec host3 nft add table inet filter ip netns exec host3 nft add chain inet filter FORWARD { type filter hook forward priority filter\; policy drop \; } ip netns exec host3 nft add rule inet filter FORWARD counter ip protocol icmp drop ip netns exec host3 nft add rule inet filter FORWARD counter ip protocol esp accept ip netns exec host3 nft add rule inet filter FORWARD counter drop ip -netns host2 xfrm policy add src 10.1.1.0/24 dst 10.1.4.0/24 dir out \ flag icmp tmpl src 10.1.2.1 dst 10.1.3.2 proto esp reqid 1 mode tunnel ip -netns host2 xfrm policy add src 10.1.4.0/24 dst 10.1.1.0/24 dir in \ tmpl src 10.1.3.2 dst 10.1.2.1 proto esp reqid 2 mode tunnel ip -netns host2 xfrm policy add src 10.1.4.0/24 dst 10.1.1.0/24 dir fwd \ flag icmp tmpl src 10.1.3.2 dst 10.1.2.1 proto esp reqid 2 mode tunnel ip -netns host2 xfrm state add src 10.1.2.1 dst 10.1.3.2 proto esp spi 1 \ reqid 1 replay-window 1 mode tunnel aead 'rfc4106(gcm(aes))' \ 0x1111111111111111111111111111111111111111 96 \ sel src 10.1.1.0/24 dst 10.1.4.0/24 ip -netns host2 xfrm state add src 10.1.3.2 dst 10.1.2.1 proto esp spi 2 \ flag icmp reqid 2 replay-window 10 mode tunnel aead 'rfc4106(gcm(aes))' \ 0x2222222222222222222222222222222222222222 96 ip -netns host4 xfrm policy add src 10.1.4.0/24 dst 10.1.1.0/24 dir out \ flag icmp tmpl src 10.1.3.2 dst 10.1.2.1 proto esp reqid 1 mode tunnel ip -netns host4 xfrm policy add src 10.1.1.0/24 dst 10.1.4.0/24 dir in \ tmpl src 10.1.2.1 dst 10.1.3.2 proto esp reqid 2 mode tunnel ip -netns host4 xfrm policy add src 10.1.1.0/24 dst 10.1.4.0/24 dir fwd \ flag icmp tmpl src 10.1.2.1 dst 10.1.3.2 proto esp reqid 2 mode tunnel ip -netns host4 xfrm state add src 10.1.3.2 dst 10.1.2.1 proto esp spi 2 \ reqid 1 replay-window 1 mode tunnel aead 'rfc4106(gcm(aes))' \ 0x2222222222222222222222222222222222222222 96 ip -netns host4 xfrm state add src 10.1.2.1 dst 10.1.3.2 proto esp spi 1 \ reqid 2 replay-window 20 flag icmp mode tunnel aead 'rfc4106(gcm(aes))' \ 0x1111111111111111111111111111111111111111 96 \ sel src 10.1.1.0/24 dst 10.1.4.0/24 ip netns exec host1 ping -W 5 -c 1 10.1.4.2 2>&1 > /dev/null && echo "" ip netns exec host1 ping -W 5 -c 1 10.1.4.3 || echo "note source address" Again before the fix ping -W 5 -c 1 10.1.4.3 >From 10.1.4.3 icmp_seq=1 Destination Host Unreachable After the fix >From 10.1.3.2 icmp_seq=1 Destination Host Unreachable Signed-off-by: Antony Antony --- net/ipv4/icmp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index e63a3bf99617..bec234637122 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -555,7 +555,6 @@ static struct rtable *icmp_route_lookup(struct net *net, XFRM_LOOKUP_ICMP); if (!IS_ERR(rt2)) { dst_release(&rt->dst); - memcpy(fl4, &fl4_dec, sizeof(*fl4)); rt = rt2; } else if (PTR_ERR(rt2) == -EPERM) { if (rt)