Re: [PATCH 0/9] Devices accessibility control group (v4)

Greg KH
2008-03-07T06:08:36Z

On Thu, Mar 06, 2008 at 11:36:22AM +0300, Pavel Emelyanov wrote:
> Greg KH wrote:
> > On Wed, Mar 05, 2008 at 09:15:25PM -0600, Serge E. Hallyn wrote:
> >> Quoting Greg KH (greg@kroah.com):
> >>> On Wed, Mar 05, 2008 at 08:23:35PM +0300, Pavel Emelyanov wrote:
> >>>> Changes from v3:
> >>>> * Ported on 2.6.25-rc3-mm1;
> >>>> * Re-splitted into smaller pieces;
> >>>> * Added more comments to tricky places.
> >>>>
> >>>> This controller allows to tune the devices accessibility by tasks,
> >>>> i.e. grant full access for /dev/null, /dev/zero etc, grant read-only
> >>>> access to IDE devices and completely hide SCSI disks.
> >>> From within the kernel itself? The kernel should not be keeping track
> >>> of the mode of devices, that's what the filesystem holding /dev is for.
> >>> Those modes change all the time depending on the device plugged in, and
> >>> the user using the "console". Why should the kernel need to worry about
> >>> any of this?
> >> These are distinct from the permissions on device files. No matter what
> >> the permissions on the device files, a task in a devcg cgroup which
> >> isn't allowed write to chardev 4:64 will not be able to write to
> >> /dev/ttyS0.
> >
> > Then why not do that from userspace with a different /dev, or with a
> > LSM?
>
> Different dev is not suitable, since task may still call mknod to
> create device it needs and use it. This is not about comfortable
> use, this is about security.

LSM can control mknod quite well. And it is _all_ about security, don't
try to add "security" to some other portion of the kernel that it not
expecting it :)

> LSM approach was proposed, but that required some API to configure
> the permissions.

Much like the API you already created to configure the permissions...

> This API done via control groups, so there were no
> difference between this approach and that. Except for this one doesn't
> create one more level of filtering at the top of kobject lookup and
> thus is simpler and faster.

How can it be "faster"? There is no speed problems here.

Please use LSM for this, that is what it is explicitly there for.

> >> The purpose is to prevent a root task from granting itself access to
> >> certain devices. Without this, the only option currently is to take
> >> CAP_MKNOD out of the capability bounding set for a container and make
> >> sure that /dev is set up right (and enforce nodev for mounts). In
> >> itself that doesn't sound so bad and it was my preference at first,
> >
> > that would be my preference as well.
>
> That approach relies on a proper user space setup inside a container,
> but this creates security holes, since container user may ignore all
> these "requirements".

Ok then, use LSM to enforce those requirements, again, that is what it
is there for.

> >> but the argument is that things like udev should be able to run in a
> >> container, and will object about not being able to create devices.
> >
> > No reason you can't modify udev to do something like this. At the
>
> Sure we _can_ modify udev, but the problem is that users of virtualisation
> solutions often (very often) use old software (e.g. set up some out-dated
> distribution inside a container), so trick with modified udev simply won't
> work in many cases.

So the kernel's job is to enforce policy that an old userspace can not
seem to get straight? That bit of logic doesn't go over so well...

Andrew, please drop these patches from your tree, this all should be
done with a simple LSM interface, which would be much smaller and much
less intrusive than this patchset.

thanks,

greg k-h