Re: [PATCH] Smack: SMACK_IOCLOADACCESS

Casey Schaufler
2011-08-26T16:28:42Z

On 8/26/2011 5:50 AM, Eric Paris wrote:
> On Fri, Aug 26, 2011 at 1:52 AM, Jarkko Sakkinen
> <jarkko.sakkinen@intel.com> wrote:
>> IOCTL call for /smack/load that takes access rule in
>> the same format as they are written into /smack/load.
>> Sets errno to zero if access is allowed and to EACCES
>> if not.
>>
>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>
> [SELinux maintainer here, but Casey knew to already take what I say
> with a grain of salt]

Indeed.

> I'm not telling you to do anything differently, just telling you what
> SELinux does, and why we do it. SELinux has a file in selinuxfs
> called 'access.' The file can be opened and one can write a rule into
> the file. One then calls read and gets back a structure which
> contains all of the permissions information allowed for the
> source/target/class.

Is that really how it works? The code reads as if the buffer
passed by write gets modified in place and the writer is expected
to use that. There is nothing in the code that looks like the
response is getting set aside for a subsequent read. If it works
the way you say it does it will be subject to races up the wazoo.
If it works the way I think the code says it does it is abusive
of the write interface.

> In SELinux we calculate all of the permissions
> for the tuple at once so providing all of the information at once can
> make a lot of sense. libselinux provides libraries that will cache
> these decisions in the userspace program and quickly answer the same
> (or similar) questions later.

I have played with similar code for Smack and it works just fine
if you're willing to ignore rule updates and schlep all the rule
lists around in user space on the off chance someone will make an
access check. Smack is intentionally kernel oriented (hence the
"k" and the end of the name) and the ioctl requires much less
user space support than the /selinux/accesss interface, should it
be a write+read interface or an abused write.

> http://userspace.selinuxproject.org/trac/browser/libselinux/src/compute_av.c
>
> Shows the userspace side of out "access" interface. Your interface is
> good in that it only takes 1 syscall and ours takes 2. Your interface
> is bad in that it is ioctl and we are told since birth that we must
> hate them no matter what (not that read/write is really any
> different). It isn't the same method the only other LSM I know about
> uses. It can only every return one value (ok, I know ioctl can be
> made to do anything at all)

Much better to use an ioctl for a query interface than to jump
through hoops with unnatural uses of other interfaces.

And just as a note, I was working with UNIX when ioctl was
introduced and I didn't like it then. I would not be considering
an ioctl interface if it where not the correct way to solve the
problem.

> Anyway, just food for thought....
>
> -Eric
>