netdev - Re: [PATCH net-next] net: PIM register decapsulation and Forwarding.

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

Message-ID: <YoT/tea4TZ2lWN8f@shredder>
Date:   Wed, 18 May 2022 17:16:21 +0300
From:   Ido Schimmel <idosch@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     dsahern@...nel.org, Saranya Panjarathina <plsaranya@...il.com>,
        netdev@...r.kernel.org, Saranya_Panjarathina@...l.com,
        davem@...emloft.net, yoshfuji@...ux-ipv6.org, edumazet@...gle.com,
        pabeni@...hat.com, linux-kernel@...r.kernel.org,
        g_balaji1@...l.com, Nikolay Aleksandrov <razor@...ckwall.org>
Subject: Re: [PATCH net-next] net: PIM register decapsulation and Forwarding.

On Wed, May 18, 2022 at 12:08:35PM +0300, Ido Schimmel wrote:
> On Tue, May 17, 2022 at 05:10:26PM -0700, Jakub Kicinski wrote:
> > On Mon, 16 May 2022 04:29:06 -0700 Saranya Panjarathina wrote:
> > > PIM register packet is decapsulated but not forwarded in RP
> > > 
> > > __pim_rcv decapsulates the PIM register packet and reinjects for forwarding
> > > after replacing the skb->dev to reg_dev (vif with VIFF_Register)
> > > 
> > > Ideally the incoming device should be same as skb->dev where the
> > > original PIM register packet is received. mcache would not have
> > > reg_vif as IIF. Decapsulated packet forwarding is failing
> > > because of IIF mismatch. In RP for this S,G RPF interface would be
> > > skb->dev vif only, so that would be IIF for the cache entry.
> > > 
> > > Signed-off-by: Saranya Panjarathina <plsaranya@...il.com>
> > 
> > Not sure if this can cause any trouble. And why it had become 
> > a problem now, seems like the code has been this way forever.
> > David? Ido?
> 
> Trying to understand the problem:
> 
> 1. The RP has an (*, G) towards the receiver(s) (receiver joins first)
> 2. The RP receives a PIM Register packet encapsulating the packet from
> the source
> 3. The kernel decapsulates the packet and injects it into the Rx path as
> if the packet was received by the pimreg netdev
> 4. The kernel forwards the packet according to the (*, G) route (no RPF
> check)
> 
> At the same time, the PIM Register packet should be received by whatever
> routing daemon is running in user space via a raw socket for the PIM
> protocol. My understanding is that it should cause the RP to send a PIM
> Join towards the FHR, causing the FHR to send two copies of each packet
> from the source: encapsulated in the PIM Register packet and over the
> (S, G) Tree.
> 
> If the RP already has an (S, G) route with IIF of skb->dev and the
> decapsulated packet is injected into the Rx path via skb->dev, then what
> prevents the RP from forwarding the same packet twice towards the
> receiver(s)?
> 
> I'm not a PIM expert so the above might be nonsense. Anyway, I will
> check with someone from the FRR teams who understands PIM better than
> me.

We discussed this patch in FRR slack with the author and PIM experts.
The tl;dr is that the patch is working around what we currently believe
is an FRR bug, which the author will try to fix.

After receiving a PIM Register message on the RP, FRR installs an (S, G)
route with IIF being the interface via which the packet was received
(skb->dev). FRR also sends a PIM Join towards the FHR and eventually a
PIM Register Stop.

The current behavior means that due to RPF assertion, all the
encapsulated traffic from the source is dropped on the RP after FRR
installs the (S, G) route.

The patch is problematic because during the time the FHR sends both
encapsulated and native traffic towards the RP, the RP will forward both
copies towards the receiver(s).

Instead, the suggestion is for FRR to install the initial (S, G) route
with IIF being the pimreg device. This should allow decapsulated traffic
to be forwarded correctly. Native traffic will trigger RPF assertion and
thereby prompt FRR to: a) Replace the IIF from pimreg to the one via
which traffic is received b) Send a PIM Register Stop towards the FHR,
instructing it to stop sending encapsulated traffic.

> 
> > 
> > > diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> > > index 13e6329784fb..7b9586335fb7 100644
> > > --- a/net/ipv4/ipmr.c
> > > +++ b/net/ipv4/ipmr.c
> > > @@ -598,7 +598,7 @@ static int __pim_rcv(struct mr_table *mrt, struct sk_buff *skb,
> > >  	skb->protocol = htons(ETH_P_IP);
> > >  	skb->ip_summed = CHECKSUM_NONE;
> > >  
> > > -	skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
> > > +	skb_tunnel_rx(skb, skb->dev, dev_net(skb->dev));
> > >  
> > >  	netif_rx(skb);
> > >  
> > > diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> > > index 4e74bc61a3db..147e29a818ca 100644
> > > --- a/net/ipv6/ip6mr.c
> > > +++ b/net/ipv6/ip6mr.c
> > > @@ -566,7 +566,7 @@ static int pim6_rcv(struct sk_buff *skb)
> > >  	skb->protocol = htons(ETH_P_IPV6);
> > >  	skb->ip_summed = CHECKSUM_NONE;
> > >  
> > > -	skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
> > > +	skb_tunnel_rx(skb, skb->dev, net);
> > >  
> > >  	netif_rx(skb);
> > >  
> >