Re: [PATCH 5/9] Make use of permissions, returned by kobj_lookup

[Pavel Emelyanov <xemul@openvz.org>]

[snip]

>> I have basically three objections against LSM.
>>
>> 1. LSM is not stackable, so loading this tiny module with devices
>>    access rights will block other security modules;
> 
> Do you really want to run other LSMs within a containerd kernel?  Is
> that a requirement?  It would seem to run counter to the main goal of
> containers to me.

_I_ do not want to run LSM with containers, but this was a requirement
from some people when we decided which approach to chose - LSM of mine.

Nevertheless, this is not very important, indeed.

>> 2. Turning CONFIG_SECURITY on immediately causes all the other hooks
>>    to get called. This affects performance on critical paths, like
>>    process creation/destruction, network flow and so on. This impact
>>    is small, but noticeable;
> 
> The last time this was brought up, it was not noticable, except for some
> network paths.  And even then, the number was lost in the noise from
> what I saw.  I think with a containered machine, you have bigger things
> to be worried about :)

The namespace-based virtualization implies no performance overhead for 
a container, actually. This is not xen or vmware where performance 
loss is OK.

Besides, I've measured some things - the lat_syscall test for open from 
lmbench test suite and the nptl perf test. Here are the results:

        sec     nosec
open    3.0980s  3.0709s
nptl    2.7746s  2.7710s

So we have 0.88% loss in open and ~0.15% with nptl. I know, this is not that
much, but it is noticeable. Besides, this is only two tests, digging deeper
may reveal more.

Let alone the fact that simply turning the CONFIG_SECURITY to 'y' puts +8Kb 
to the vmlinux...

I think, I finally agree with you and Al Viro, that the kobj mapper is 
not the right place to put the filtering, but taking the above numbers 
into account, can we put the "hooks" into the #else /* CONFIG_SECURITY */
versions of security_inode_permission/security_file_permission/etc?

>> 3. With LSM turned on we'll have to "virtualize" it, i.e. make its
>>    work safe in a container. I don't presume to judge how much work
>>    will have to be done in this area, so the result patch would be
>>    even larger and maybe will duplicate functionality, which is currently
>>    in cgroups. OTOH, cgroups already provide the ways to correctly 
>>    delegate proper rights to containers.
> 
> No, your lsm would be your "virtualize" policy.  I don't think you would
> have to do any additional work here, but could be wrong.  Would like to
> see the code to prove it.
> 
>>> Opening a dev node is not on any "fast path" that you need to be
>>> concerned about a few extra calls within the kernel.
>>>
>>> And, I think in the end your patch would be much smaller and easier to
>>> understand and review and maintain overall.
>> Hardly - the largest part of my patch is cgroup manipulations. The part
>> that makes the char and block layers switch to new map ac check the 
>> permissions is 10-20 lines of new code.
>>
>> But with LSM I will still need this API.
> 
> Yes, but your LSM hooks will be smaller than the code modifications to
> the map logic :)
> 
> Again, I object to this as you are driving a new security policy
> infrastructure into the device node logic where it does not belong as we
> already have this functionality in the LSM interface today.  Please use
> that one instead and don't clutter up the kernel with "one-off" security
> changes like this one.
> 
> Please try the LSM interface and see what happens.  If, after you have
> created a patch, you still have objections, please post it for review
> and I will be glad to revisit my opinion at that time.
> 
> thanks,
> 
> greg k-h
>