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

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: <20171103092740.GQ11292@secunet.com>
Date:   Fri, 3 Nov 2017 10:27:40 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Paul Moore <paul@...l-moore.com>
CC:     Florian Westphal <fw@...len.de>, <netdev@...r.kernel.org>,
        Stephen Smalley <sds@...ho.nsa.gov>
Subject: Re: [PATCH ipsec] xfrm: do unconditional template resolution before
 pcpu cache check

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@...len.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@...l-moore.com>
> > Fixes: ec30d78c14a813db39a647b6a348b428 ("xfrm: add xdst pcpu cache")
> > Reported-by: Stephen Smalley <sds@...ho.nsa.gov>
> > Tested-by: Stephen Smalley <sds@...ho.nsa.gov>
> > Signed-off-by: Florian Westphal <fw@...len.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@...l-moore.com>

Patch applied, thanks everyone!