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

[Joshua Brindle <brindle@quarksecurity.com>]

Paul Moore wrote:
> On Thu, May 21, 2015 at 9:35 AM, Joshua Brindle
> <brindle@quarksecurity.com>  wrote:
>> Paul Moore wrote:
>>> On Thu, May 21, 2015 at 12:17 AM, Jeffrey Vander Stoep wrote:
>>>> Thanks for all the feedback and suggestions. Agreed that raw numerical
>>>> values are confusing. I will fix up the commit message to set a better
>>>> precedent for intended use. I included them more to illustrate what is
>>>> happening under the hood. I like the idea of a qualifier for clarity.
>>>> The qualifier seems necessary for the suggested non-ioctl-specific
>>>> approach.
>>> Great, thank you.
>>>
>>>> Individual ioctl labels are only marginally better than raw numbers.
>>>> E.g. { TCSETSF TIOCGWINSZ TCGETA TCSETA TCSETAW TCSETAF TCSBRK TCXONC
>>>> TIOCMBIS }. More helpful...but not much.
>>>>
>>>> My plan was to group commonly used ioctl sets into macros.
>>>>
>>>> e.g. common_socket_ioc, priv_socket_ioc, tty_ioc, gpu_ioc, etc
>>>>
>>>> After monitoring ioctl use across five different devices I think this
>>>> is a good approach as just 10-20 macros would be adequate for a
>>>> targeted policy and would provide a clearer explanation of the
>>>> permissions given.
>>> Agreed.  We can use m4 to provide both the ioctl names and sets if needed.
>> m4 is never the answer....
>
> Except for the policy interfaces, permission sets, etc. ;)
>
> See Stephen's comments on this, specifically the idea that ioctls are
> not objects.  Further, the existing permission set shorthand is a very
> good precedence for this approach towards ioctl number handling; I see
> no reason *not* to use m4.
>

The reason *not* to use m4 is because we want some sort of meaningful 
identifiers preserved in the kernel policy for analysis. I know that 
isn't your use case but it is some of ours.

>> An attribute-like symbol that is maintained in the binary would make these
>> visible in the loaded policy and therefore more understandable/analyzable.
>
> I think the proposed approach is easily understood and/or analyzable.
>
>> It would also allow you to easily add them in specific devices in a more
>> readable way. For example, if the Android base policy allowed gpu_ioc to
>> surfaceflinger all a device specific variant would need to do is add its
>> ioctls to the attribute.
>
> Once again, I think the proposed approach ticks these boxes as well.
>

Not in the *kernel binary*