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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Here is my personal todo list based on this conversation.

- change example policy in commit message to demonstrate intended use.
No raw ioctl values.
- Look into making logic more general, less ioctl specific
- Look at making the code clearer. I.e. address Paul's comments on
lack of clarity in struct/variable naming.

In the spirit of keeping this commit concise and as basic as possible
(it's already 800 LOC!) I will not address suggestions to propagate
additional policy information such as ioctl names and groups into the
kernel binary. I agree that would be useful, but I will leave as
future work.

Regarding comments on policy syntax, those will be addressed in a
separate non-kernel commit to the selinux project.

Thanks again for all the feedback!

On Thu, May 21, 2015 at 8:27 AM, Joshua Brindle
<brindle@xxxxxxxxxxxxxxxxx> wrote:
> Stephen Smalley wrote:
>>
>> On 05/21/2015 10:57 AM, Joshua Brindle wrote:
>>>
>>> William Roberts wrote:
>>>>
>>>> On May 21, 2015 7:53 AM, "Joshua Brindle"<brindle@xxxxxxxxxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>> William Roberts wrote:
>>>>>>
>>>>>> On Wed, May 20, 2015 at 9:17 PM, Jeffrey Vander
>>>>>> Stoep<jeffv@xxxxxxxxxx>
>>>>>> 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.
>>>>>>>
>>>>>>> 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
>>>>>>>
>>>>>> This is exactly what I had in mind, just use m4
>>>>>>
>>>>> Even outside of my analysis use case I think this is not a good idea.
>>>>>
>>>>> Consider the Android base policy defined gpu_op as a set of ioctls, and
>>>>
>>>> there were related rules for gpu users defined there. Then a device
>>>> variant
>>>> policy has additional gpu_op ioctl's. How does it add them? Does it
>>>> have to
>>>> re-add all of the related rules with the new ioctls?
>>>>
>>>> I could define a new macro my_device_GPU_ioctls and include the base
>>>> macro
>>>> in its expansion.
>>>
>>> And repeat every related rule (and keep them in sync as the base policy
>>> develops). How much easier is it just to add your ioctl to an ioctl
>>> attribute and be done?
>>
>>
>> Technically not required for that purpose, e.g. I can do this today:
>> $ cat device/lge/hammerhead/sepolicy/global_macros
>> define(`r_file_perms', `{ ' r_file_perms `write }')
>> and have it automatically add write everywhere we allow r_file_perms
>> even in the core policy, because the build process unions on a
>> file-by-file basis.
>>
>> Effectively your attribute would only be useful as inline documentation;
>> it would have no binding to the actual semantic meaning of the check.
>> If the policy writer allows it as gpu_ioc in policy but the driver is
>> actually something other than a gpu driver or chooses to interpret a
>> particular command value included in that attribute in some other way,
>> then it might have an entirely different effect.  So it might be helpful
>> but not sufficient for analysis.
>
>
> Unless the attributes were specific to the type in question:
>
> ioctl gpu_device gpu_ioc { 0x8910-0x8926 0x892A-0x8935 };
>
> allow surfaceflinger gpu_device : chr_file gpu_ioc;
>
> This would 1) enforce the policy authors intention, 2) force policy authors
> to be specific with what ioctls mean what with which devices and 3) allow
> more meaningful analysis.
>
> _______________________________________________
> Selinux mailing list
> Selinux@xxxxxxxxxxxxx
> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
> To get help, send an email containing "help" to
> Selinux-request@xxxxxxxxxxxxx.
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux