Received-SPF: pass (google.com: domain of linux-kernel+bounces-2296-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231213143813.6818-1-michael.weiss@aisec.fraunhofer.de>
 <20231213143813.6818-4-michael.weiss@aisec.fraunhofer.de> <20231215-golfanlage-beirren-f304f9dafaca@brauner>
 <61b39199-022d-4fd8-a7bf-158ee37b3c08@aisec.fraunhofer.de>
 <20231215-kubikmeter-aufsagen-62bf8d4e3d75@brauner> <CAADnVQKeUmV88OfQOfiX04HjKbXq7Wfcv+N3O=5kdL4vic6qrw@mail.gmail.com>
 <20231216-vorrecht-anrief-b096fa50b3f7@brauner>
In-Reply-To: <20231216-vorrecht-anrief-b096fa50b3f7@brauner>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Sat, 16 Dec 2023 09:41:10 -0800
Message-ID: <CAADnVQK7MDUZTUxcqCH=unrrGExCjaagfJFqFPhVSLUisJVk_Q@mail.gmail.com>
Subject: Re: [RFC PATCH v3 3/3] devguard: added device guard for mknod in
 non-initial userns
To: Christian Brauner <brauner@kernel.org>
Cc: =?UTF-8?Q?Michael_Wei=C3=9F?= <michael.weiss@aisec.fraunhofer.de>, 
	Alexander Mikhalitsyn <alexander@mihalicyn.com>, Alexei Starovoitov <ast@kernel.org>, 
	Paul Moore <paul@paul-moore.com>, Daniel Borkmann <daniel@iogearbox.net>, 
	Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, 
	Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, 
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, 
	Quentin Monnet <quentin@isovalent.com>, Alexander Viro <viro@zeniv.linux.org.uk>, 
	Miklos Szeredi <miklos@szeredi.hu>, Amir Goldstein <amir73il@gmail.com>, 
	"Serge E. Hallyn" <serge@hallyn.com>, bpf <bpf@vger.kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, 
	Linux-Fsdevel <linux-fsdevel@vger.kernel.org>, 
	LSM List <linux-security-module@vger.kernel.org>, gyroidos@aisec.fraunhofer.de
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Dec 16, 2023 at 2:38=E2=80=AFAM Christian Brauner <brauner@kernel.o=
rg> wrote:
>
> On Fri, Dec 15, 2023 at 10:08:08AM -0800, Alexei Starovoitov wrote:
> > On Fri, Dec 15, 2023 at 6:15=E2=80=AFAM Christian Brauner <brauner@kern=
el.org> wrote:
> > >
> > > On Fri, Dec 15, 2023 at 02:26:53PM +0100, Michael Wei=C3=9F wrote:
> > > > On 15.12.23 13:31, Christian Brauner wrote:
> > > > > On Wed, Dec 13, 2023 at 03:38:13PM +0100, Michael Wei=C3=9F wrote=
:
> > > > >> devguard is a simple LSM to allow CAP_MKNOD in non-initial user
> > > > >> namespace in cooperation of an attached cgroup device program. W=
e
> > > > >> just need to implement the security_inode_mknod() hook for this.
> > > > >> In the hook, we check if the current task is guarded by a device
> > > > >> cgroup using the lately introduced cgroup_bpf_current_enabled()
> > > > >> helper. If so, we strip out SB_I_NODEV from the super block.
> > > > >>
> > > > >> Access decisions to those device nodes are then guarded by exist=
ing
> > > > >> device cgroups mechanism.
> > > > >>
> > > > >> Signed-off-by: Michael Wei=C3=9F <michael.weiss@aisec.fraunhofer=
.de>
> > > > >> ---
> > > > >
> > > > > I think you misunderstood me... My point was that I believe you d=
on't
> > > > > need an additional LSM at all and no additional LSM hook. But I m=
ight be
> > > > > wrong. Only a POC would show.
> > > >
> > > > Yeah sorry, I got your point now.
> > >
> > > I think I might have had a misconception about how this works.
> > > A bpf LSM program can't easily alter a kernel object such as struct
> > > super_block I've been told.
> >
> > Right. bpf cannot change arbitrary kernel objects,
> > but we can add a kfunc that will change a specific bit in a specific
> > data structure.
> > Adding a new lsm hook that does:
> >     rc =3D call_int_hook(sb_device_access, 0, sb);
> >     switch (rc) {
> >     case 0: do X
> >     case 1: do Y
> >
> > is the same thing, but uglier, since return code will be used
> > to do this action.
> > The 'do X' can be one kfunc
> > and 'do Y' can be another.
> > If later we find out that 'do X' is not a good idea we can remove
> > that kfunc.
>
> The reason I moved the SB_I_MANAGED_DEVICES here is that I want a single
> central place where that is done for any possible LSM that wants to
> implement device management. So we don't have to go chasing where that
> bit is set for each LSM. I also don't want to have LSMs raise bits in
> sb->s_iflags directly as that's VFS property.

a kfunc that sets a bit in sb->s_iflags will be the same central place.
It will be somewhere in the fs/ directory and vfs maintainers can do what t=
hey
wish with it, including removal.
For traditional LSM one would need to do an accurate code review to make
sure that they don't mess with sb->s_iflags while for bpf_lsm it
will be done automatically. That kfunc will be that only one central place.