[PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Eyal Birger]

The commit in the "Fixes" tag tried to avoid a case where policy check
is ignored due to dst caching in next hops.

However, when the traffic is locally consumed, the dst may be cached
in a local TCP or UDP socket as part of early demux. In this case the
"disable_policy" flag is not checked as ip_route_input_noref() was only
called before caching, and thus, packets after the initial packet in a
flow will be dropped if not matching policies.

Fix by checking the "disable_policy" flag also when a valid dst is
already available.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
Reported-by: Monil Patel <monil191989@gmail.com>
Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>

----

v2: use dev instead of skb->dev
---
 net/ipv4/ip_input.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 1b512390b3cf..e880ce77322a 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -366,6 +366,11 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 					   iph->tos, dev);
 		if (unlikely(err))
 			goto drop_error;
+	} else {
+		struct in_device *in_dev = __in_dev_get_rcu(dev);
+
+		if (in_dev && IN_DEV_ORCONF(in_dev, NOPOLICY))
+			IPCB(skb)->flags |= IPSKB_NOPOLICY;
 	}
 
 #ifdef CONFIG_IP_ROUTE_CLASSID
-- 
2.34.1

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Nicolas Dichtel]

Le 09/10/2022 à 21:16, Eyal Birger a écrit :
> The commit in the "Fixes" tag tried to avoid a case where policy check
> is ignored due to dst caching in next hops.
> 
> However, when the traffic is locally consumed, the dst may be cached
> in a local TCP or UDP socket as part of early demux. In this case the
> "disable_policy" flag is not checked as ip_route_input_noref() was only
> called before caching, and thus, packets after the initial packet in a
> flow will be dropped if not matching policies.
> 
> Fix by checking the "disable_policy" flag also when a valid dst is
> already available.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
> Reported-by: Monil Patel <monil191989@gmail.com>
> Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
> 

Is there the same problem with ipv6?

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Eyal Birger]

Hi Nicolas,

On Mon, Oct 10, 2022 at 11:28 AM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 09/10/2022 à 21:16, Eyal Birger a écrit :
> > The commit in the "Fixes" tag tried to avoid a case where policy check
> > is ignored due to dst caching in next hops.
> >
> > However, when the traffic is locally consumed, the dst may be cached
> > in a local TCP or UDP socket as part of early demux. In this case the
> > "disable_policy" flag is not checked as ip_route_input_noref() was only
> > called before caching, and thus, packets after the initial packet in a
> > flow will be dropped if not matching policies.
> >
> > Fix by checking the "disable_policy" flag also when a valid dst is
> > already available.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
> > Reported-by: Monil Patel <monil191989@gmail.com>
> > Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
> > Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
> >
>
> Is there the same problem with ipv6?

The issue is specific to IPv4 as the original fix was only relevant
to IPv4.

I also tested a similar scenario using IPv6 addresses and did not see
a problem.

Thanks!
Eyal.

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Nicolas Dichtel]

Le 10/10/2022 à 12:29, Eyal Birger a écrit :
> Hi Nicolas,
> 
> On Mon, Oct 10, 2022 at 11:28 AM Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>>
>> Le 09/10/2022 à 21:16, Eyal Birger a écrit :
>>> The commit in the "Fixes" tag tried to avoid a case where policy check
>>> is ignored due to dst caching in next hops.
>>>
>>> However, when the traffic is locally consumed, the dst may be cached
>>> in a local TCP or UDP socket as part of early demux. In this case the
>>> "disable_policy" flag is not checked as ip_route_input_noref() was only
>>> called before caching, and thus, packets after the initial packet in a
>>> flow will be dropped if not matching policies.
>>>
>>> Fix by checking the "disable_policy" flag also when a valid dst is
>>> already available.
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
>>> Reported-by: Monil Patel <monil191989@gmail.com>
>>> Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
>>> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
>>>
>>
>> Is there the same problem with ipv6?
> 
> The issue is specific to IPv4 as the original fix was only relevant
> to IPv4.
> 
> I also tested a similar scenario using IPv6 addresses and did not see
> a problem.
Thanks. Is it possible to write a selftest with this scenario?


Regards,
Nicolas

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Eyal Birger]

On Mon, Oct 10, 2022 at 3:19 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 10/10/2022 à 12:29, Eyal Birger a écrit :
> > Hi Nicolas,
> >
> > On Mon, Oct 10, 2022 at 11:28 AM Nicolas Dichtel
> > <nicolas.dichtel@6wind.com> wrote:
> >>
> >> Le 09/10/2022 à 21:16, Eyal Birger a écrit :
> >>> The commit in the "Fixes" tag tried to avoid a case where policy check
> >>> is ignored due to dst caching in next hops.
> >>>
> >>> However, when the traffic is locally consumed, the dst may be cached
> >>> in a local TCP or UDP socket as part of early demux. In this case the
> >>> "disable_policy" flag is not checked as ip_route_input_noref() was only
> >>> called before caching, and thus, packets after the initial packet in a
> >>> flow will be dropped if not matching policies.
> >>>
> >>> Fix by checking the "disable_policy" flag also when a valid dst is
> >>> already available.
> >>>
> >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
> >>> Reported-by: Monil Patel <monil191989@gmail.com>
> >>> Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
> >>> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
> >>>
> >>
> >> Is there the same problem with ipv6?
> >
> > The issue is specific to IPv4 as the original fix was only relevant
> > to IPv4.
> >
> > I also tested a similar scenario using IPv6 addresses and did not see
> > a problem.
> Thanks. Is it possible to write a selftest with this scenario?

I can add one targeting ipsec-next.

Do you perhaps know which is the current preferred flavor for such selftests
for ipsec - C or bash?

Eyal.

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Nicolas Dichtel]

Le 10/10/2022 à 14:52, Eyal Birger a écrit :
[snip]
>> Thanks. Is it possible to write a selftest with this scenario?
> 
> I can add one targeting ipsec-next.
> 
> Do you perhaps know which is the current preferred flavor for such selftests
> for ipsec - C or bash?
I would say bash.
There are a lot of examples here:
https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git/tree/tools/testing/selftests/net


Regards,
Nicolas

Re: [PATCH ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux

[Steffen Klassert]

On Sun, Oct 09, 2022 at 10:16:43PM +0300, Eyal Birger wrote:
> The commit in the "Fixes" tag tried to avoid a case where policy check
> is ignored due to dst caching in next hops.
> 
> However, when the traffic is locally consumed, the dst may be cached
> in a local TCP or UDP socket as part of early demux. In this case the
> "disable_policy" flag is not checked as ip_route_input_noref() was only
> called before caching, and thus, packets after the initial packet in a
> flow will be dropped if not matching policies.
> 
> Fix by checking the "disable_policy" flag also when a valid dst is
> already available.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
> Reported-by: Monil Patel <monil191989@gmail.com>
> Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>

Applied, thanks Eyal!