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

[Antony Antony <antony.antony@secunet.com>]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

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@gmail.com>

Tested-by: Antony Antony <antony.antony@secunet.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

[-- Attachment #2: 0001-xfrm-use-IPPROTO_ESP-for-route-lookup-without-encaps.patch --]
[-- Type: text/x-diff, Size: 1003 bytes --]

From 54cfdfaab12270784623c60d91baf499765e50f5 Mon Sep 17 00:00:00 2001
From: Antony Antony <antony.antony@secunet.com>
Date: Mon, 2 Sep 2024 22:08:15 +0200
Subject: [PATCH ipsec] xfrm: use IPPROTO_ESP for route lookup without encapsulation

When there is no UDP or TCP encapsulation, use IPPROTO_ESP for route
lookup. This ensures that "ip rule" entries like the following match
correctly:
 ip rule add from 192.1.2.45 to 192.1.2.23 ipproto esp lookup 50

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_policy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 64bfd1390df0..9b0b1b448dce 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -327,6 +327,8 @@ static inline struct dst_entry *xfrm_dst_lookup(struct xfrm_state *x,
 			params.uli.ports.dport = x->encap->encap_dport;
 			break;
 		}
+	} else {
+		params.ipproto = IPPROTO_ESP;
 	}

 	dst = __xfrm_dst_lookup(family, &params);
--
2.43.0