Re: [RFC PATCH v3 3/3] devguard: added device guard for mknod in non-initial userns

[Michael Weiß]

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@xxxxxxxxxxxxxxxxxxx>
>> ---
> 
> 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.