| Message ID | 20220513203402.1290131-1-eyal.birger@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [ipsec,v2] xfrm: fix "disable_policy" flag use when arriving from different devices | expand |
Le 13/05/2022 à 22:34, Eyal Birger a écrit : > In IPv4 setting the "disable_policy" flag on a device means no policy > should be enforced for traffic originating from the device. This was > implemented by seting the DST_NOPOLICY flag in the dst based on the > originating device. > > However, dsts are cached in nexthops regardless of the originating > devices, in which case, the DST_NOPOLICY flag value may be incorrect. > > Consider the following setup: > > +------------------------------+ > | ROUTER | > +-------------+ | +-----------------+ | > | ipsec src |----|-|ipsec0 | | > +-------------+ | |disable_policy=0 | +----+ | > | +-----------------+ |eth1|-|----- > +-------------+ | +-----------------+ +----+ | > | noipsec src |----|-|eth0 | | > +-------------+ | |disable_policy=1 | | > | +-----------------+ | > +------------------------------+ > > Where ROUTER has a default route towards eth1. > > dst entries for traffic arriving from eth0 would have DST_NOPOLICY > and would be cached and therefore can be reused by traffic originating > from ipsec0, skipping policy check. > > Fix by setting a IPSKB_NOPOLICY flag in IPCB and observing it instead > of the DST in IN/FWD IPv4 policy checks. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com> > Signed-off-by: Eyal Birger <eyal.birger@gmail.com> Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
On Fri, May 13, 2022 at 11:34:02PM +0300, Eyal Birger wrote: > In IPv4 setting the "disable_policy" flag on a device means no policy > should be enforced for traffic originating from the device. This was > implemented by seting the DST_NOPOLICY flag in the dst based on the > originating device. > > However, dsts are cached in nexthops regardless of the originating > devices, in which case, the DST_NOPOLICY flag value may be incorrect. > > Consider the following setup: > > +------------------------------+ > | ROUTER | > +-------------+ | +-----------------+ | > | ipsec src |----|-|ipsec0 | | > +-------------+ | |disable_policy=0 | +----+ | > | +-----------------+ |eth1|-|----- > +-------------+ | +-----------------+ +----+ | > | noipsec src |----|-|eth0 | | > +-------------+ | |disable_policy=1 | | > | +-----------------+ | > +------------------------------+ > > Where ROUTER has a default route towards eth1. > > dst entries for traffic arriving from eth0 would have DST_NOPOLICY > and would be cached and therefore can be reused by traffic originating > from ipsec0, skipping policy check. > > Fix by setting a IPSKB_NOPOLICY flag in IPCB and observing it instead > of the DST in IN/FWD IPv4 policy checks. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com> > Signed-off-by: Eyal Birger <eyal.birger@gmail.com> > > --- > > v2: set IPSKB_NOPOLICY in ip_route_input_mc() as needed Applied, thanks a lot Eyal!
diff --git a/include/net/ip.h b/include/net/ip.h index 3984f2c39c4b..0161137914cf 100644 --- a/include/net/ip.h +++ b/include/net/ip.h @@ -56,6 +56,7 @@ struct inet_skb_parm { #define IPSKB_DOREDIRECT BIT(5) #define IPSKB_FRAG_PMTU BIT(6) #define IPSKB_L3SLAVE BIT(7) +#define IPSKB_NOPOLICY BIT(8) u16 frag_max_size; }; diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 6fb899ff5afc..d2efddce65d4 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1093,6 +1093,18 @@ static inline bool __xfrm_check_nopolicy(struct net *net, struct sk_buff *skb, return false; } +static inline bool __xfrm_check_dev_nopolicy(struct sk_buff *skb, + int dir, unsigned short family) +{ + if (dir != XFRM_POLICY_OUT && family == AF_INET) { + /* same dst may be used for traffic originating from + * devices with different policy settings. + */ + return IPCB(skb)->flags & IPSKB_NOPOLICY; + } + return skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY); +} + static inline int __xfrm_policy_check2(struct sock *sk, int dir, struct sk_buff *skb, unsigned int family, int reverse) @@ -1104,7 +1116,7 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir, return __xfrm_policy_check(sk, ndir, skb, family); return __xfrm_check_nopolicy(net, skb, dir) || - (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) || + __xfrm_check_dev_nopolicy(skb, dir, family) || __xfrm_policy_check(sk, ndir, skb, family); } diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 98c6f3429593..fe5d14ef5c4d 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -1726,6 +1726,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr, struct in_device *in_dev = __in_dev_get_rcu(dev); unsigned int flags = RTCF_MULTICAST; struct rtable *rth; + bool no_policy; u32 itag = 0; int err; @@ -1736,8 +1737,12 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr, if (our) flags |= RTCF_LOCAL; + no_policy = IN_DEV_ORCONF(in_dev, NOPOLICY); + if (no_policy) + IPCB(skb)->flags |= IPSKB_NOPOLICY; + rth = rt_dst_alloc(dev_net(dev)->loopback_dev, flags, RTN_MULTICAST, - IN_DEV_ORCONF(in_dev, NOPOLICY), false); + no_policy, false); if (!rth) return -ENOBUFS; @@ -1795,7 +1800,7 @@ static int __mkroute_input(struct sk_buff *skb, struct rtable *rth; int err; struct in_device *out_dev; - bool do_cache; + bool do_cache, no_policy; u32 itag = 0; /* get a working reference to the output device */ @@ -1840,6 +1845,10 @@ static int __mkroute_input(struct sk_buff *skb, } } + no_policy = IN_DEV_ORCONF(in_dev, NOPOLICY); + if (no_policy) + IPCB(skb)->flags |= IPSKB_NOPOLICY; + fnhe = find_exception(nhc, daddr); if (do_cache) { if (fnhe) @@ -1852,8 +1861,7 @@ static int __mkroute_input(struct sk_buff *skb, } } - rth = rt_dst_alloc(out_dev->dev, 0, res->type, - IN_DEV_ORCONF(in_dev, NOPOLICY), + rth = rt_dst_alloc(out_dev->dev, 0, res->type, no_policy, IN_DEV_ORCONF(out_dev, NOXFRM)); if (!rth) { err = -ENOBUFS; @@ -2228,6 +2236,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr, struct rtable *rth; struct flowi4 fl4; bool do_cache = true; + bool no_policy; /* IP on this device is disabled. */ @@ -2346,6 +2355,10 @@ out: return err; RT_CACHE_STAT_INC(in_brd); local_input: + no_policy = IN_DEV_ORCONF(in_dev, NOPOLICY); + if (no_policy) + IPCB(skb)->flags |= IPSKB_NOPOLICY; + do_cache &= res->fi && !itag; if (do_cache) { struct fib_nh_common *nhc = FIB_RES_NHC(*res); @@ -2360,7 +2373,7 @@ out: return err; rth = rt_dst_alloc(ip_rt_get_dev(net, res), flags | RTCF_LOCAL, res->type, - IN_DEV_ORCONF(in_dev, NOPOLICY), false); + no_policy, false); if (!rth) goto e_nobufs;
In IPv4 setting the "disable_policy" flag on a device means no policy should be enforced for traffic originating from the device. This was implemented by seting the DST_NOPOLICY flag in the dst based on the originating device. However, dsts are cached in nexthops regardless of the originating devices, in which case, the DST_NOPOLICY flag value may be incorrect. Consider the following setup: +------------------------------+ | ROUTER | +-------------+ | +-----------------+ | | ipsec src |----|-|ipsec0 | | +-------------+ | |disable_policy=0 | +----+ | | +-----------------+ |eth1|-|----- +-------------+ | +-----------------+ +----+ | | noipsec src |----|-|eth0 | | +-------------+ | |disable_policy=1 | | | +-----------------+ | +------------------------------+ Where ROUTER has a default route towards eth1. dst entries for traffic arriving from eth0 would have DST_NOPOLICY and would be cached and therefore can be reused by traffic originating from ipsec0, skipping policy check. Fix by setting a IPSKB_NOPOLICY flag in IPCB and observing it instead of the DST in IN/FWD IPv4 policy checks. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com> Signed-off-by: Eyal Birger <eyal.birger@gmail.com> --- v2: set IPSKB_NOPOLICY in ip_route_input_mc() as needed --- include/net/ip.h | 1 + include/net/xfrm.h | 14 +++++++++++++- net/ipv4/route.c | 23 ++++++++++++++++++----- 3 files changed, 32 insertions(+), 6 deletions(-)