User: Password:
|
|
Subscribe / Log in / New account

Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle

From:  Stephen Smalley <sds-+05T5uksL2qpZYMLLGbcSA-AT-public.gmane.org>
To:  Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA-AT-public.gmane.org>
Subject:  Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
Date:  Fri, 21 Jan 2011 16:37:05 -0500
Message-ID:  <1295645825.10909.5.camel@moss-pluto>
Cc:  linux-kernel-u79uwXL29TY76Z2rM5mHXA-AT-public.gmane.org, linux-security-module-u79uwXL29TY76Z2rM5mHXA-AT-public.gmane.org, selinux-+05T5uksL2qpZYMLLGbcSA-AT-public.gmane.org, jmorris-gx6/JNMH7DfYtjvyW6yDsg-AT-public.gmane.org, casey-iSGtlc1asvQWG2LlvL+J4A-AT-public.gmane.org, john.johansen-Z7WLFzj8eWMS+FvcfC7Uqw-AT-public.gmane.org, penguin-kernel-JPay3/Yim36HaxMnTkn67Xf5DAMn2ifp-AT-public.gmane.org, takedakn-3MafRgGXt7BL9jVzuh4AOg-AT-public.gmane.org
Archive-link:  Article

On Fri, 2011-01-21 at 14:30 -0500, Eric Paris wrote:
> [I've included an AA person as well in case you ever decide to try to
> mediate ioctl operations]
> 
> SELinux used to recognize certain individual ioctls and check
> permissions based on the knowledge of the individual ioctl.  In commit
> 242631c49d4cf396 the SELinux code stopped trying to understand
> individual ioctls and to instead looked at the ioctl access bits to
> determine in we should check read or write for that operation.  This
> same suggestion was made to SMACK (and I believe copied into TOMOYO).
> But this suggestion is total rubbish.  The ioctl access bits are
> actually the access requirements for the structure being passed into the
> ioctl, and are completely unrelated to the operation of the ioctl or the
> object the ioctl is being performed upon.
> 
> Take FS_IOC_FIEMAP as an example.  FS_IOC_FIEMAP is defined as:
> 
> FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
> 
> So it has access bits R and W.  What this really means is that the
> kernel is going to both read and write to the struct fiemap.  It has
> nothing at all to do with the operations that this ioctl might perform
> on the file itself!
> 
> If anything, our logic is exactly backwards, since an ioctl which writes
> to userspace would be 'reading' something from the file and an ioctl
> which reads from userspace would be 'writing' something to the file...
> 
> I'm planning to revert this SELinux commit, but I want other LSM authors
> to realize that (assuming I'm not completely off in the woods somewhere)
> you should take a look at your ioctl permissions checking as well....

That's unfortunate.  Prior attempt to address ioctl was here:
http://marc.info/?l=linux-security-module&m=113088357...

Which led to the approach based on _IOC_DIR.

We could revisit that approach, or just give up and always check
FILE__IOCTL unconditionally. I don't think we want to go back to
interpreting ioctl commands in the hook, as it is a layering violation
and same ioctl command value could mean different things for different
underlying objects.

-- 
Stephen Smalley
National Security Agency




(Log in to post comments)


Copyright © 2011, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds