Message-ID: <20240615152010.GB44653@mail.hallyn.com>
Date: Sat, 15 Jun 2024 10:20:10 -0500
From: "Serge E. Hallyn" <serge@...lyn.com>
To: Jonathan Calmels <jcalmels@...0.net>
Cc: John Johansen <john.johansen@...onical.com>,
	Paul Moore <paul@...l-moore.com>, brauner@...nel.org,
	ebiederm@...ssion.com, Jonathan Corbet <corbet@....net>,
	James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>, KP Singh <kpsingh@...nel.org>,
	Matt Bobrowski <mattbobrowski@...gle.com>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
	Yonghong Song <yonghong.song@...ux.dev>,
	John Fastabend <john.fastabend@...il.com>,
	Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
	Jiri Olsa <jolsa@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>,
	Kees Cook <kees@...nel.org>, Joel Granados <j.granados@...sung.com>,
	David Howells <dhowells@...hat.com>,
	Jarkko Sakkinen <jarkko@...nel.org>,
	Stephen Smalley <stephen.smalley.work@...il.com>,
	Ondrej Mosnacek <omosnace@...hat.com>,
	Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
	containers@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-security-module@...r.kernel.org, bpf@...r.kernel.org,
	apparmor@...ts.ubuntu.com, keyrings@...r.kernel.org,
	selinux@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 4/4] bpf,lsm: Allow editing capabilities in BPF-LSM
 hooks

On Thu, Jun 13, 2024 at 01:50:29AM -0700, Jonathan Calmels wrote:
> On Wed, Jun 12, 2024 at 08:54:28PM GMT, John Johansen wrote:
> > On 6/12/24 10:29, Paul Moore wrote:
> > > On Wed, Jun 12, 2024 at 4:15 AM Jonathan Calmels <jcalmels@...0.net> wrote:
> > > > On Tue, Jun 11, 2024 at 06:38:31PM GMT, Paul Moore wrote:
> > > > > On Tue, Jun 11, 2024 at 6:15 PM Jonathan Calmels <jcalmels@...0.net> wrote:
> > > 
> > > ...
> > > 
> > > > > > Arguably, if we do want fine-grained userns policies, we need LSMs to
> > > > > > influence the userns capset at some point.
> > > > > 
> > > > > One could always use, or develop, a LSM that offers additional
> > > > > controls around exercising capabilities.  There are currently four
> > > > > in-tree LSMs, including the capabilities LSM, which supply a
> > > > > security_capable() hook that is used by the capability-based access
> > > > > controls in the kernel; all of these hook implementations work
> > > > > together within the LSM framework and provide an additional level of
> > > > > control/granularity beyond the existing capabilities.
> > > > 
> > > > Right, but the idea was to have a simple and easy way to reuse/trigger
> > > > as much of the commoncap one as possible from BPF. If we're saying we
> > > > need to reimplement and/or use a whole new framework, then there is
> > > > little value.
> > > 
> > > I can appreciate how allowing direct manipulation of capability bits
> > > from a BPF LSM looks attractive, but my hope is that our discussion
> > > here revealed that as you look deeper into making it work there are a
> > > number of pitfalls which prevent this from being a safe option for
> > > generalized systems.
> > > 
> > > > TBH, I don't feel strongly about this, which is why it is absent from
> > > > v1. However, as John pointed out, we should at least be able to modify
> > > > the blob if we want flexible userns caps policies down the road.
> > > 
> > > As discussed in this thread, there are existing ways to provide fine
> > > grained control over exercising capabilities that can be safely used
> > > within the LSM framework.  I don't want to speak to what John is
> > > envisioning, but he should be aware of these mechanisms, and if I
> > > recall he did voice a level of concern about the same worries I
> > > mentioned.
> > > 
> > 
> > sorry, I should have been more clear. I envision LSMs being able to
> > update their own state in the userns hook.
> > 
> > Basically the portion of the patch that removes const from the
> > userns hook.
> 
> Yes, pretty sure we'll need this regardless.
> 
> > An LSM updating the capset is worrysome for all the reasons you
> > pointed out, and I think a few more. I haven't had a chance to really
> > look at v2 yet, so I didn't want to speak directly on the bpf part of
> > the patch without first giving a good once over.
> > 
> > > I'm happy to discuss ways in which we can adjust the LSM hooks/layer
> > > to support different approaches to capability controls, but one LSM
> > > directly manipulating the state of another is going to be a no vote
> > > from me.
> > > 
> > I might not be as hard no as Paul here, I am always willing to listen
> > to arguments, but it would have to be a really good argument to
> > modify the capset, when there are multiple LSMs in play on a system.
> 
> The way I see it, it's more about enhancing the capability LSM with BPF
> hooks and have it modify its own state dynamically, not so much
> crosstalk between two distinct LSM frameworks (say one where the BPF
> LSM implements a lot of things like capable()).
> 
> In this context and with enough safeguards (say we only allow dropping
> caps) this could be a net positive. Sure, ordering could come into play
> in very specific scenarios, but at this point I would expect the
> admin/LSM author to be conscious about it.
> 
> If we think there is no way we can come up with something that's safe
> enough, and that the risks outweigh the benefits, fine by me, we can
> drop this patch from the series.

I think pursuing patches 1-3 now, and punting on 4 until later, would
be great.

Powered by blists - more mailing lists