Re: [PATCH 0/2] selinux: add targeted whitelisting of ioctl commands.

[Stephen Smalley <sds@tycho.nsa.gov>]

On 05/20/2015 08:39 PM, William Roberts wrote:
> 
> 
> On Wed, May 20, 2015 at 1:04 PM, Paul Moore <paul@paul-moore.com
> <mailto:paul@paul-moore.com>> wrote:
> 
>     First off, my apologies for such a long delay in providing
>     feedback.  The
>     delay wasn't due to any fault of yours, rather just a backlog on my
>     todo list.
> 
>     Comments below ...
> 
>     On Thursday, April 09, 2015 02:48:50 PM Jeff Vander Stoep wrote:
>     > ---- motivation ----
>     > Ioctls provide many of the operations necessary for device
>     control. The
>     > typical driver supports a device specific set of operations
>     accessible by
>     > the ioctl system call and specified by the command argument. SELinux
>     > provides per operation access control to many system operations
>     e.g. chown,
>     > kill, setuid, ipc_lock, etc. Ioclts on the other hand are granted
>     on a per
>     > file descriptor basis using the ioctl permission, meaning that the
>     set of
>     > operations provided by the driver are granted on an all-or-nothing
>     basis.
> 
>     ...
> 
>     > ---- Design constraints ----
>     > Policy: Support existing policy, targeted whitelisting. The ioctls
>     commands
>     > used on a system may not be completely known to a
>     sysadmin/policywriter. It
>     > is not reasonable to enforce that all needed commands be known in
>     order to
>     > use this feature in a targeted manner. Existing policy using the ioctl
>     > permission will continue to work as-is. Policy targeting a specific
>     > source/target/class will only be enforced on that particular
>     > source/target/class. E.g. continue to allow init to access all ioctls
>     > provided by a driver, but only allow the browser to access a subset.
>     >
>     > Performance: Many ioctl calls are performance sensitive, and some
>     ioctls
>     > have a large command set (e.g. sockets support hundreds of commands).
>     > Execution time on a filtered ioctl should be similar to execution
>     time on
>     > an unfiltered one and not related to the number of commands being
>     filtered.
>     > Performance numbers will be included in a separate document.
>     >
>     > Command space: The ioctl command is a 32 bit number comprised of four
>     > fields, number - sequence number of the command. 8 bits
>     > type - magic number assigned to the driver. 8 bits
>     > size - size of the user data involved. typically 14 bits (arch
>     dependent)
>     > direction - The direction of data transfer. typically 2 bits (arch
>     > dependent) The command is uniquely identified by the type and
>     number, size
>     > and direction are considered to be arguments and are not
>     considered for
>     > SELinux whitelisting.
>     >
>     > ---- Policy format ----
>     > allow <source> <target>:<class> { 0x8910-0x8926 0x892A-0x8935 }
>     > auditallow <source> <target>:<class> 0x892A
> 
>     I agree with only specifying the lower 16 bits (command,type) when
>     specifying
>     the individual ioctls, I even like the '-' shortcut, but I'm a little
>     concerned about specifying a number directly in the permission field
>     without
>     any sort of qualifier.  Specifically I'm worried that it hurts the
>     readability
>     of the policy and could pose problems with future work.
> 
>     I'd be much happier if we could add some sort of syntax which would
>     qualify
>     the numbers as ioctls, for example:
> 
>       allow <source> <target>:<class> { ioctl(0x8910-0x8926) ioctl(0x892A) }
> 
> 
> If you want additional syntax couldn't we move that burden to m4 rather
> then making it a part of the compiler core?

It has to be handled by checkpolicy and encoded in the kernel binary
policy as an additional field if we want to support other uses of these
operation structures for purposes other than just ioctl.