* SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
@ 2011-01-21 19:30 ` Eric Paris
0 siblings, 0 replies; 8+ messages in thread
From: Eric Paris @ 2011-01-21 19:30 UTC (permalink / raw)
To: linux-kernel, linux-security-module, selinux
Cc: sds, jmorris, casey, john.johansen, penguin-kernel, takedakn
[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....
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
@ 2011-01-21 19:30 ` Eric Paris
0 siblings, 0 replies; 8+ messages in thread
From: Eric Paris @ 2011-01-21 19:30 UTC (permalink / raw)
To: linux-kernel, linux-security-module, selinux
Cc: sds, jmorris, casey, john.johansen, penguin-kernel, takedakn
[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....
-Eric
--
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
2011-01-21 19:30 ` Eric Paris
@ 2011-01-21 19:50 ` Casey Schaufler
-1 siblings, 0 replies; 8+ messages in thread
From: Casey Schaufler @ 2011-01-21 19:50 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, linux-security-module, selinux, sds, jmorris,
john.johansen, penguin-kernel, takedakn, Casey Schaufler
On 1/21/2011 11:30 AM, 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.
Bugger bugger bugger ... (you get the idea) ... bugger.
This means calling out each and every ioctl separately,
unless someone has a magic access behavior detector for
device drivers. You can't even assume that two devices
that share an ioctl are going to do the same thing with
it. In the old Orange Book days almost half the work on
describing the access control implementation went into
iotcls and fcntls. Bugger.
What strategy is SELinux planning to take on getting the
ioctls right, and more importantly, keeping them correct
going forward?
> 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!
Grrrrr.
> 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....
Yeah. Good fun, that. Any chance we could share strategic thinking?
I hate the notion of brute forcing each ioctl, having done it in the
past for a system with lots fewer ioctls than Linux is supporting.
> -Eric
Thank you for the heads up on this. Bugger ... bugger.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
@ 2011-01-21 19:50 ` Casey Schaufler
0 siblings, 0 replies; 8+ messages in thread
From: Casey Schaufler @ 2011-01-21 19:50 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, linux-security-module, selinux, sds, jmorris,
john.johansen, penguin-kernel, takedakn, Casey Schaufler
On 1/21/2011 11:30 AM, 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.
Bugger bugger bugger ... (you get the idea) ... bugger.
This means calling out each and every ioctl separately,
unless someone has a magic access behavior detector for
device drivers. You can't even assume that two devices
that share an ioctl are going to do the same thing with
it. In the old Orange Book days almost half the work on
describing the access control implementation went into
iotcls and fcntls. Bugger.
What strategy is SELinux planning to take on getting the
ioctls right, and more importantly, keeping them correct
going forward?
> 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!
Grrrrr.
> 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....
Yeah. Good fun, that. Any chance we could share strategic thinking?
I hate the notion of brute forcing each ioctl, having done it in the
past for a system with lots fewer ioctls than Linux is supporting.
> -Eric
Thank you for the heads up on this. Bugger ... bugger.
--
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
2011-01-21 19:30 ` Eric Paris
@ 2011-01-21 21:37 ` Stephen Smalley
-1 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2011-01-21 21:37 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, linux-security-module, selinux, jmorris, casey,
john.johansen, penguin-kernel, takedakn
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong and nonsensicle
@ 2011-01-21 21:37 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2011-01-21 21:37 UTC (permalink / raw)
To: Eric Paris
Cc: linux-kernel, linux-security-module, selinux, jmorris, casey,
john.johansen, penguin-kernel, takedakn
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong andnonsensicle
2011-01-21 19:30 ` Eric Paris
` (2 preceding siblings ...)
(?)
@ 2011-01-22 2:01 ` Tetsuo Handa
2011-01-22 2:15 ` Eric Paris
-1 siblings, 1 reply; 8+ messages in thread
From: Tetsuo Handa @ 2011-01-22 2:01 UTC (permalink / raw)
To: eparis; +Cc: linux-kernel, linux-security-module
Eric Paris wrote:
> 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....
Since the mapping of ioctl cmd number and what the kernel does with that number
is unknown for LSM modules, TOMOYO does not use permission bits.
TOMOYO simply checks ioctl cmd number value passed to ioctl() requests.
For example,
file ioctl /dev/tty0 0x4B4E
file ioctl /dev/console 0x5402
file ioctl /dev/snd/controlC0 0x80045500
file ioctl socket:[family=2:type=1:protocol=6] 0x8942
file ioctl socket:[family=2:type=2:protocol=17] 0x8913
http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/centos5.5/domain_policy.conf?v=policy-sample
So, I think nothing to change for TOMOYO.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong andnonsensicle
2011-01-22 2:01 ` SELinux/SMACK/TOMOYO: ioctl permissions handling is wrong andnonsensicle Tetsuo Handa
@ 2011-01-22 2:15 ` Eric Paris
0 siblings, 0 replies; 8+ messages in thread
From: Eric Paris @ 2011-01-22 2:15 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-kernel, linux-security-module
On Sat, 2011-01-22 at 11:01 +0900, Tetsuo Handa wrote:
> Eric Paris wrote:
> > 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....
>
> Since the mapping of ioctl cmd number and what the kernel does with that number
> is unknown for LSM modules, TOMOYO does not use permission bits.
> TOMOYO simply checks ioctl cmd number value passed to ioctl() requests.
> For example,
>
> file ioctl /dev/tty0 0x4B4E
> file ioctl /dev/console 0x5402
> file ioctl /dev/snd/controlC0 0x80045500
> file ioctl socket:[family=2:type=1:protocol=6] 0x8942
> file ioctl socket:[family=2:type=2:protocol=17] 0x8913
>
> http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/centos5.5/domain_policy.conf?v=policy-sample
>
> So, I think nothing to change for TOMOYO.
You are correct, I thought I saw you guys doing something similar, but
that is clearly not the case. It's just SELinux and SMACK that are
doing it wrong.
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-01-22 2:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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.