netdev - Re: [devel-ipsec] [PATCH ipsec, v2 0/2] xfrm: respect ip proto rules criteria in xfrm dst lookups

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] [day] [month] [year] [list]

Message-ID: <ZtYiig0I3zKimOVB@moon.secunet.de>
Date: Mon, 2 Sep 2024 22:39:38 +0200
From: Antony Antony <antony.antony@...unet.com>
To: Eyal Birger <eyal.birger@...il.com>
CC: <steffen.klassert@...unet.com>, <herbert@...dor.apana.org.au>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<dsahern@...nel.org>, <pabeni@...hat.com>, <netdev@...r.kernel.org>,
	<devel@...ux-ipsec.org>, Antony Antony <antony@...nome.org>
Subject: Re: [devel-ipsec] [PATCH ipsec, v2 0/2] xfrm: respect ip proto rules
 criteria in xfrm dst lookups

On Mon, Sep 02, 2024 at 04:07:17AM -0700, Eyal Birger via Devel wrote:
> This series fixes the route lookup when done for xfrm to regard
> L4 criteria specified in ip rules.

Thanks Eyal for explaining the purpose of this series on the call.
How about something like this for the beginning of the commit message:

'This series fixes the route lookup for the outer packet after
encapsulation, including the L4 criteria specified in IP rules.'

It's just a cosmetic suggestion, so may be improve it if you're planning to
send a new version of the patch series for other reasons.

We ran into this issue before and used workaround, mark instead of L4 in the 
"ip rule" for the outer packet.

> The first patch is a minor refactor to allow passing more parameters
> to dst lookup functions.
> The second patch actually passes L4 information to these lookup functions.
> 
> Signed-off-by: Eyal Birger <eyal.birger@...il.com>

Tested-by: Antony Antony <antony.antony@...unet.com>

And I have a further suggestion to improve this fix make it more generic.  

I was doing the following rule as a work around for ESP-in-UDP tunnels.
ip rule add from all to 192.1.2.23 fwmark 0x1 lookup 50

With your fix I can change it to a L4 rule when using ESP-in-UDP
ip rule add from 192.1.2.45 to 192.1.2.23 ipproto udp dport 4500 lookup 50

However, when not using ESP, without UDP, and rule with "ipproto esp" does 
work.

ip rule add from 192.1.2.45 to 192.1.2.23 ipproto esp lookup 50

So, I have come up with a fix/hack on top of your fix.

@@ -327,6 +327,8 @@ static inline struct dst_entry *xfrm_dst_lookup(struct xfrm_state *x,

+       } else {
+               params.ipproto = IPPROTO_ESP;

With this fix "ipproto esp" rules also works.
see the attached full patch.

regards,
-antony

View attachment "0001-xfrm-use-IPPROTO_ESP-for-route-lookup-without-encaps.patch" of type "text/x-diff" (1005 bytes)