| Message ID | 20221009191643.297623-1-eyal.birger@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [ipsec,v2] xfrm: fix "disable_policy" on ipv4 early demux | expand |
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?
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.
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
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.
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
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!
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
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(+)