From: Steffen Klassert <steffen.klassert@secunet•com>
To: Paul Moore <paul@paul-moore•com>
Cc: Florian Westphal <fw@strlen•de>, <netdev@vger•kernel.org>,
Stephen Smalley <sds@tycho•nsa.gov>
Subject: Re: [PATCH ipsec] xfrm: do unconditional template resolution before pcpu cache check
Date: Fri, 3 Nov 2017 10:27:40 +0100 [thread overview]
Message-ID: <20171103092740.GQ11292@secunet.com> (raw)
In-Reply-To: <CAHC9VhSikfCPf6_c-ts6FLsw56k_tpBGGMDD4909zP=V_Va+tQ@mail.gmail.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!
next prev parent reply other threads:[~2017-11-03 9:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-02 15:46 [PATCH ipsec] xfrm: do unconditional template resolution before pcpu cache check Florian Westphal
2017-11-02 22:57 ` Paul Moore
2017-11-03 9:27 ` Steffen Klassert [this message]
2017-11-14 20:46 ` [regression, 4.14] xfrm: Fix stack-out-of-bounds read in xfrm_state_find breaks selinux-testsuite Stephen Smalley
2017-11-15 5:40 ` Steffen Klassert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171103092740.GQ11292@secunet.com \
--to=steffen.klassert@secunet$(echo .)com \
--cc=fw@strlen$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=paul@paul-moore$(echo .)com \
--cc=sds@tycho$(echo .)nsa.gov \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox