Re: [PATCH ipsec] xfrm: do unconditional template resolution before pcpu cache check

[Steffen Klassert <steffen.klassert@secunet.com>]

On Thu, Nov 02, 2017 at 06:57:29PM -0400, Paul Moore wrote:
> On Thu, Nov 2, 2017 at 11:46 AM, Florian Westphal <fw@strlen.de> wrote:
> > Stephen Smalley says:
> >  Since 4.14-rc1, the selinux-testsuite has been encountering sporadic
> >  failures during testing of labeled IPSEC. git bisect pointed to
> >  commit ec30d ("xfrm: add xdst pcpu cache").
> >  The xdst pcpu cache is only checking that the policies are the same,
> >  but does not validate that the policy, state, and flow match with respect
> >  to security context labeling.
> >  As a result, the wrong SA could be used and the receiver could end up
> >  performing permission checking and providing SO_PEERSEC or SCM_SECURITY
> >  values for the wrong security context.
> >
> > This fix makes it so that we always do the template resolution, and
> > then checks that the found states match those in the pcpu bundle.
> >
> > This has the disadvantage of doing a bit more work (lookup in state hash
> > table) if we can reuse the xdst entry (we only avoid xdst alloc/free)
> > but we don't add a lot of extra work in case we can't reuse.
> >
> > xfrm_pol_dead() check is removed, reasoning is that
> > xfrm_tmpl_resolve does all needed checks.
> >
> > Cc: Paul Moore <paul@paul-moore.com>
> > Fixes: ec30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
> > Reported-by: Stephen Smalley <sds@tycho.nsa.gov>
> > Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  net/xfrm/xfrm_policy.c | 42 ++++++++++++++++++++++++------------------
> >  1 file changed, 24 insertions(+), 18 deletions(-)
> 
> This looks reasonable and seems like probably the simplest approach to
> me.  I'm building a test kernel with it now, but considering the time
> of day here, I probably will not be able to test it until tomorrow
> morning; however it is important to note that Stephen did test this
> already so please don't wait on my test results - we are likely to be
> running the same tests anyway.
> 
> Acked-by: Paul Moore <paul@paul-moore.com>

Patch applied, thanks everyone!