netdev - Re: [PATCH net 1/1] xfrm: fix source address in icmp error generation from IPsec gateway

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: <Zg_uP2G8HIury_lI@Antony2201.local>
Date: Fri, 5 Apr 2024 14:27:43 +0200
From: Antony Antony <antony@...nome.org>
To: Michael Richardson <mcr@...delman.ca>
Cc: Antony Antony <antony@...nome.org>, antony.antony@...unet.com,
	Herbert Xu <herbert@...dor.apana.org.au>, netdev@...r.kernel.org,
	David Ahern <dsahern@...nel.org>, devel@...ux-ipsec.org,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net 1/1] xfrm: fix source address in icmp error
 generation from IPsec gateway

Hi Michael,

thanks for your clarifications.

On Thu, Apr 04, 2024 at 11:35:57AM -0400, Michael Richardson wrote:
> 
> Antony Antony <antony@...nome.org> wrote:
>     > On Thu, Apr 04, 2024 at 10:39:17AM -0400, Michael Richardson wrote:
>     >>
>     >> Antony Antony <antony@...nome.org> wrote: > Indeed, 10.1.3.2 does not
>     >> match the policy. However, notice the "flag > icmp" in the above
>     >> line. That means the policy lookup will use the > inner payload for
>     >> policy lookup as specified in RFC 4301, Section 6, > which will
>     >> match. The inner payload 10.1.4.1 <=> 10.1.4.3 will match > the
>     >> policy.
>     >>
>     >> How is "flag icmp" communicated via IKEv2?
> 
>     > As far as I'm aware, it isn't communicated via IKEv2. I believe it's
>     > considered a local policy, and possibly specified in BCP.
> 
>     > However, how is communicating it over IKEv2 relevant to this kernel
>     > patch? I don't see any connection! If there is one, please
>     > elaborate. Without a clear link, the netdev maintainers might reject
>     > this patch.
> 
> because, we are using a custom Linux kernel flag to get the ICMP back into
> the tunnel, then the other end might not accept the packet if it doesn't have
> a similiar configuration.

The use of "flags icmp" option here for ICMP handling within tunnels is 
directly aligned with the standards outlined in IPsec RFC 4301, section 6.2.  
This section mandates that both sender and receiver must be configurable to 
verify inner payload header information against the SA it traverses. It 
explicitly states, "an IPsec implementation MUST be configurable to check 
that this payload header information is consistent with the SA via which it 
arrives." Furthermore, it details the required handling of inbound ICMP 
error messages that don't match the SA's traffic selectors, emphasizing the 
necessity for special processing in such cases.
Therefore, the need for IKEv2 in this context does not arise, as Linux 
kernel approach is fully compliant with the specified standards.

> 
>     >> Won't the other gateway just drop this packet?
> 
>     > That's would be a local choice, fate of an ICMP message:), akin to ICMP
>     > errors elsewhere. Let's not dive into filtering choices and PMTU for
>     > now:)
> 
> No, it's not. It's up to IKEv2 to configure those flags.
> Your choice requires extra flags.  The previous behaviour was rather
> ingenious because it guaranteed that the packet always fit into the tunnel.
> (the bug was that it didn't do it for IPv6 as well)

While the current behavior might seem beneficial, it has a major flaw. 
The generated response matches the IPsec policy only because it uses 
incorrect source addresses. This is address spoofing by the IPsec peer.
This should be considered a bug, not a feature. In contrast, the correct 
approach, as IPv6 demonstrates, avoids such tactics. Ensuring IPv4 also 
adheres to standards is crucial. Thus, applying this fix is an improvment.

I think the patch should be applied, and leave it to the kernel community's 
insights and judgment.

> 
>     > Just thinking out loud, I haven't seen forwarding ICMP error messages
>     > negotiated in other tunneling protocols like MPLS or pptp...., if I
>     > recall correctly, QUIC does indeed have it specified.
> 
> So, what?  They aren't L3 tunnel protocols are they?

Based on RFC 4301, section 6—as referenced in the quote above—it explicitly 
mandates that implementing a locally configurable option/
Therefore, negotiation over IKEv2 seems unnecessary.

> MPLS is L2.5, pptp is L2 and QUIC is L4.

-antony