Date: Fri, 15 Dec 2023 14:26:53 +0100
From: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
To: Christian Brauner <brauner@...nel.org>
CC: Alexander Mikhalitsyn <alexander@...alicyn.com>, Alexei Starovoitov
	<ast@...nel.org>, Paul Moore <paul@...l-moore.com>, Daniel Borkmann
	<daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
	<martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song
	<yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh
	<kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo
	<haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Quentin Monnet
	<quentin@...valent.com>, Alexander Viro <viro@...iv.linux.org.uk>, Miklos
 Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>, "Serge E.
 Hallyn" <serge@...lyn.com>, <bpf@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
	<linux-security-module@...r.kernel.org>, <gyroidos@...ec.fraunhofer.de>
Subject: Re: [RFC PATCH v3 3/3] devguard: added device guard for mknod in
 non-initial userns

On 15.12.23 13:31, Christian Brauner wrote:
> On Wed, Dec 13, 2023 at 03:38:13PM +0100, Michael Weiß wrote:
>> devguard is a simple LSM to allow CAP_MKNOD in non-initial user
>> namespace in cooperation of an attached cgroup device program. We
>> 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 existing
>> device cgroups mechanism.
>>
>> Signed-off-by: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
>> ---
> 
> I think you misunderstood me... My point was that I believe you don't
> need an additional LSM at all and no additional LSM hook. But I might be
> wrong. Only a POC would show.

Yeah sorry, I got your point now.

> 
> Just write a bpf lsm program that strips SB_I_NODEV in the existing
> security_sb_set_mnt_opts() call which is guranteed to be called when a
> new superblock is created.

This does not work since SB_I_NODEV is a required_iflag in
mount_too_revealing(). This I have already tested when writing the
simple LSM here. So maybe we need to drop SB_I_NODEV from required_flags
there, too. Would that be safe?

> 
> Store your device access rules in a bpf map or in the sb->s_security
> blob (This is where I'm fuzzy and could use a bpf LSM expert's input.).
> 
> Then make that bpf lsm program kick in everytime a
> security_inode_mknod() and security_file_open() is called and do device
> access management in there. Actually, you might need to add one hook
> when the actual device that's about to be opened is know. 
> This should be where today the device access hooks are called.
> 
> And then you should already be done with this. The only thing that you
> need is the capable check patch.
> 
> You don't need that cgroup_bpf_current_enabled() per se. Device
> management could now be done per superblock, and not per task. IOW, you
> allowlist a bunch of devices that can be created and opened. Any task
> that passes basic permission checks and that passes the bpf lsm program
> may create device nodes.
> 
> That's a way more natural device management model than making this a per
> cgroup thing. Though that could be implemented as well with this.
> 
> I would try to write a bpf lsm program that does device access
> management with your capable() sysctl patch applied and see how far I
> get.
> 
> I don't have the time otherwise I'd do it.
I'll give it a try but no promises how fast this will go.

Powered by blists - more mailing lists