From: Stephen Smalley <sds@tycho.nsa.gov>
To: Eric Paris <eparis@redhat.com>
Cc: linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov,
jmorris@namei.org, casey@schaufler-ca.com,
john.johansen@canonical.com, penguin-kernel@I-love.SAKURA.ne.jp,
takedakn@nttdata.co.jp
Subject: Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
Date: Fri, 21 Jan 2011 16:37:05 -0500 [thread overview]
Message-ID: <1295645825.10909.5.camel@moss-pluto> (raw)
In-Reply-To: <1295638239.3403.15.camel@localhost.localdomain>
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=113088357020104&w=2
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
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Smalley <sds@tycho.nsa.gov>
To: Eric Paris <eparis@redhat.com>
Cc: linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov,
jmorris@namei.org, casey@schaufler-ca.com,
john.johansen@canonical.com, penguin-kernel@I-love.SAKURA.ne.jp,
takedakn@nttdata.co.jp
Subject: Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
Date: Fri, 21 Jan 2011 16:37:05 -0500 [thread overview]
Message-ID: <1295645825.10909.5.camel@moss-pluto> (raw)
In-Reply-To: <1295638239.3403.15.camel@localhost.localdomain>
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=113088357020104&w=2
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
next prev parent reply other threads:[~2011-01-21 21:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-21 19:30 SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle Eric Paris
2011-01-21 19:30 ` Eric Paris
2011-01-21 19:50 ` Casey Schaufler
2011-01-21 19:50 ` Casey Schaufler
2011-01-21 21:37 ` Stephen Smalley [this message]
2011-01-21 21:37 ` Stephen Smalley
2011-01-22 2:01 ` SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong andnonsensicle Tetsuo Handa
2011-01-22 2:15 ` Eric Paris
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1295645825.10909.5.camel@moss-pluto \
--to=sds@tycho.nsa.gov \
--cc=casey@schaufler-ca.com \
--cc=eparis@redhat.com \
--cc=jmorris@namei.org \
--cc=john.johansen@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=selinux@tycho.nsa.gov \
--cc=takedakn@nttdata.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.