netdev - Re: [PATCH ipsec-next 5/6] xfrm: add RX datapath protection for IPsec full offload mode

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YoHhH++2sBvyy+8d@unreal>
Date:   Mon, 16 May 2022 08:29:03 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        netdev@...r.kernel.org, Raed Salem <raeds@...dia.com>,
        ipsec-devel <devel@...ux-ipsec.org>
Subject: Re: [PATCH ipsec-next 5/6] xfrm: add RX datapath protection for
 IPsec full offload mode

On Fri, May 13, 2022 at 05:02:54PM +0200, Steffen Klassert wrote:
> On Tue, May 10, 2022 at 01:36:56PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> > 
> > Traffic received by device with enabled IPsec full offload should be
> > forwarded to the stack only after decryption, packet headers and
> > trailers removed.
> > 
> > Such packets are expected to be seen as normal (non-XFRM) ones, while
> > not-supported packets should be dropped by the HW.
> > 
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > ---
> >  include/net/xfrm.h | 55 +++++++++++++++++++++++++++-------------------
> >  1 file changed, 32 insertions(+), 23 deletions(-)
> > 
> > diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> > index 21be19ece4f7..9f9250fe1c4d 100644
> > --- a/include/net/xfrm.h
> > +++ b/include/net/xfrm.h
> > @@ -1094,6 +1094,29 @@ xfrm_state_addr_cmp(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x, un
> >  	return !0;
> >  }
> >  
> > +#ifdef CONFIG_XFRM
> > +static inline struct xfrm_state *xfrm_input_state(struct sk_buff *skb)
> > +{
> > +	struct sec_path *sp = skb_sec_path(skb);
> > +
> > +	return sp->xvec[sp->len - 1];
> > +}
> > +#endif
> > +
> > +static inline struct xfrm_offload *xfrm_offload(struct sk_buff *skb)
> > +{
> > +#ifdef CONFIG_XFRM
> > +	struct sec_path *sp = skb_sec_path(skb);
> > +
> > +	if (!sp || !sp->olen || sp->len != sp->olen)
> > +		return NULL;
> > +
> > +	return &sp->ovec[sp->olen - 1];
> > +#else
> > +	return NULL;
> > +#endif
> > +}
> > +
> >  #ifdef CONFIG_XFRM
> >  int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
> >  			unsigned short family);
> > @@ -1113,6 +1136,15 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir,
> >  {
> >  	struct net *net = dev_net(skb->dev);
> >  	int ndir = dir | (reverse ? XFRM_POLICY_MASK + 1 : 0);
> > +	struct xfrm_offload *xo = xfrm_offload(skb);
> > +	struct xfrm_state *x;
> > +
> > +	if (xo) {
> > +		x = xfrm_input_state(skb);
> > +		if (x->xso.type == XFRM_DEV_OFFLOAD_FULL)
> > +			return (xo->flags & CRYPTO_DONE) &&
> > +			       (xo->status & CRYPTO_SUCCESS);
> > +	}
> 
> We can not exit without doing the policy check here. The inner
> packet could still match a block policy in software. Maybe
> we can reset the secpath and do the policy check.

We checked that both policy and state were offloaded. In such case,
driver returned that everything ok and the packet is handled.

SW policy will be in lower priority, so we won't catch it.

Thanks