netdev - Re: [PATCH v6 ipsec-next] xfrm: introduce forwarding of ICMP Error messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZbN3KK0vcy37A5y1@gauss3.secunet.de>
Date: Fri, 26 Jan 2024 10:11:04 +0100
From: Steffen Klassert <steffen.klassert@...unet.com>
To: Antony Antony <antony.antony@...unet.com>
CC: Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller"
	<davem@...emloft.net>, <devel@...ux-ipsec.org>, Jakub Kicinski
	<kuba@...nel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v6 ipsec-next] xfrm: introduce forwarding of ICMP Error
 messages

On Fri, Jan 19, 2024 at 12:27:42PM +0100, Antony Antony wrote:
> This commit aligns with RFC 4301, Section 6, and addresses the
> requirement to forward unauthenticated ICMP error messages that do not
> match any xfrm policies. It utilizes the ICMP payload as an skb and
> performs a reverse lookup. If a policy match is found, forward
> the packet.
> 
> The ICMP payload typically contains a partial IP packet that is likely
> responsible for the error message.
> 
> The following error types will be forwarded:
> - IPv4 ICMP error types: ICMP_DEST_UNREACH & ICMP_TIME_EXCEEDED
> - IPv6 ICMPv6 error types: ICMPV6_DEST_UNREACH, ICMPV6_PKT_TOOBIG,
> 			   ICMPV6_TIME_EXCEED
> 
> To implement this feature, a reverse lookup has been added to the xfrm
> forward path, making use of the ICMP payload as the skb.
> 
> To enable this functionality from user space, the XFRM_POLICY_ICMP flag
> should be added to the outgoing and forward policies, and the
> XFRM_STATE_ICMP flag should be set on incoming states.
> 
> e.g.
> ip xfrm policy add flag icmp tmpl
> 
> ip xfrm policy
> src 192.0.2.0/24 dst 192.0.1.0/25
> 	dir out priority 2084302 ptype main flag icmp
> 
> ip xfrm state add ...flag icmp
> 
> ip xfrm state
> root@...t:~#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
> 
> Changes since v5:
> - fix return values bool->int, feedback from Steffen
> 
> Changes since v4:
> - split the series to only ICMP erorr forwarding
> 
> Changes since v3: no code chage
>  - add missing white spaces detected by checkpatch.pl
> 
> Changes since v2: reviewed by Steffen Klassert
>  - user consume_skb instead of kfree_skb for the inner skb
>  - fixed newskb leaks in error paths
>  - free the newskb once inner flow is decoded with change due to
>    commit 7a0207094f1b ("xfrm: policy: replace session decode with flow dissector")
>  - if xfrm_decode_session_reverse() on inner payload fails ignore.
>    do not increment error counter
> 
> Changes since v1:
> - Move IPv6 variable declaration inside IS_ENABLED(CONFIG_IPV6)
> 
> Changes since RFC:
> - Fix calculation of ICMPv6 header length
> 
> Signed-off-by: Antony Antony <antony.antony@...unet.com>

Patch applied, thanks a lot Antony!