[refpolicy] [PATCH 8/34]: patch to allow the devicekit module to work with dbus

[cpebenito@tresys.com (Christopher J. PeBenito)]

Forgot to reply to all...

On 02/24/11 15:58, Guido Trentalancia wrote:
> On Wed, 23/02/2011 at 09.16 -0500, Christopher J. PeBenito wrote: 
>> On 02/16/11 01:05, Guido Trentalancia wrote:
>>> This patch adds two new interfaces (one for the kernel and the
>>> other for mount). It then allows dbus chat between dbus and
>>> devicekit and between xdm and devicekit. It also adds some
>>> permissions needed to run devicekit.
>>>
>>> diff -pruN refpolicy-git-15022011-new-before-modification/policy/modules/kernel/kernel.if refpolicy-git-15022011-new-modified/policy/modules/kernel/kernel.if
>>> --- refpolicy-git-15022011-new-before-modification/policy/modules/kernel/kernel.if  2011-01-24 00:32:54.978503593 +0100
>>> +++ refpolicy-git-15022011-new-modified/policy/modules/kernel/kernel.if     2011-02-15 22:58:46.166838136 +0100
>>> @@ -1893,6 +1893,24 @@ interface(`kernel_rw_kernel_sysctl',`
>>>  
>>>  ########################################
>>>  ## <summary>
>>> +##      Allow caller to search filesystem sysctls.
>>> +## </summary>
>>> +## <param name="domain">
>>> +##      <summary>
>>> +##      Domain allowed access.
>>> +##      </summary>
>>> +## </param>
>>> +#
>>> +interface(`kernel_search_fs_sysctl',`
>>> +   gen_require(`
>>> +           type proc_t, sysctl_t, sysctl_fs_t;
>>> +   ')
>>> +
>>> +   search_dirs_pattern($1, { proc_t sysctl_t }, sysctl_fs_t)
>>> +')
>>> +
>>> +########################################
>>> +## <summary>
>>>  ## Read filesystem sysctls.
>>>  ## </summary>
>>>  ## <param name="domain">
>>> diff -pruN refpolicy-git-15022011-new-before-modification/policy/modules/services/dbus.te refpolicy-git-15022011-new-modified/policy/modules/services/dbus.te
>>> --- refpolicy-git-15022011-new-before-modification/policy/modules/services/dbus.te  2011-02-15 22:53:52.507511721 +0100
>>> +++ refpolicy-git-15022011-new-modified/policy/modules/services/dbus.te     2011-02-15 22:58:46.169838637 +0100
>>> @@ -145,6 +145,11 @@ optional_policy(`
>>>  ')
>>>  
>>>  optional_policy(`
>>> +   devicekit_dbus_chat_disk(system_dbusd_t)
>>> +   devicekit_dbus_chat_power(system_dbusd_t)
>>> +')
>>> +
>>> +optional_policy(`
>>>     policykit_dbus_chat(system_dbusd_t)
>>>     policykit_domtrans_auth(system_dbusd_t)
>>>     policykit_search_lib(system_dbusd_t)
>>> diff -pruN refpolicy-git-15022011-new-before-modification/policy/modules/services/devicekit.te refpolicy-git-15022011-new-modified/policy/modules/services/devicekit.te
>>> --- refpolicy-git-15022011-new-before-modification/policy/modules/services/devicekit.te     2011-01-08 19:07:21.241741196 +0100
>>> +++ refpolicy-git-15022011-new-modified/policy/modules/services/devicekit.te        2011-02-15 23:04:04.993242115 +0100
[cut]
>>>  kernel_search_debugfs(devicekit_power_t)
>>> +kernel_search_fs_sysctl(devicekit_power_t)
>>> +kernel_setsched(devicekit_power_t)
>>>  kernel_write_proc_files(devicekit_power_t)
>>>  
>>>  corecmd_exec_bin(devicekit_power_t)
>>> @@ -207,6 +212,7 @@ consoletype_exec(devicekit_power_t)
>>>  
>>>  domain_read_all_domains_state(devicekit_power_t)
>>>  
>>> +dev_getattr_apm_bios_dev(devicekit_power_t)
>>>  dev_read_input(devicekit_power_t)
>>>  dev_rw_generic_usb_dev(devicekit_power_t)
>>>  dev_rw_generic_chr_files(devicekit_power_t)
>>> @@ -216,8 +222,11 @@ dev_rw_sysfs(devicekit_power_t)
>>>  files_read_kernel_img(devicekit_power_t)
>>>  files_read_etc_files(devicekit_power_t)
>>>  files_read_usr_files(devicekit_power_t)
>>> +files_rw_etc_runtime_files(devicekit_power_t)
>>
>> Which files are being written?
> 
> I think it's /etc/mtab.
> 
> allow devicekit_power_t etc_runtime_t:file { read getattr ioctl write };
> 
> And there are also problems in the contexts because there might be
> files /etc/mtab~[0-9]{3,4} produced at runtime that don't get labeled
> etc_runtime_t but instead fall back to etc_t which creates problems for
> write:
> 
> type=AVC msg=audit(1298587357.352:19): avc:  denied  { link } for
> pid=2837 comm="mount" name="mtab~2837" dev=dm-1 ino=1216
> scontext=system_u:system_r:mount_t:s0-s0:c0.c1023
> tcontext=system_u:object_r:etc_t:s0 tclass=file
> type=AVC msg=audit(1298587357.358:20): avc:  denied  { unlink } for
> pid=2837 comm="mount" name="mtab~2837" dev=dm-1 ino=1216
> scontext=system_u:system_r:mount_t:s0-s0:c0.c1023
> tcontext=system_u:object_r:etc_t:s0 tclass=file
> 
> The above needs to get fixed in policy/modules/kernel/files.fc. Can you
> do some editing on the fly ?

I suspect this is due to the below /bin/mount incorrectly running in
devicekit_power_t.

>>> +fs_getattr_xattr_fs(devicekit_power_t)
>>>  fs_list_inotifyfs(devicekit_power_t)
>>> +fs_remount_xattr_fs(devicekit_power_t)
>>
>> Remounting filesystems?  Sounds suspect.
> 
> allow devicekit_power_t fs_t:filesystem remount;
> 
> type=AVC msg=audit(1298580943.963:9): avc:  denied  { remount } for
> pid=2679 comm="mount"
> scontext=system_u:system_r:devicekit_power_t:s0-s0:c0.c1023
> tcontext=system_u:object_r:fs_t:s0 tclass=filesystem
> type=SYSCALL msg=audit(1298580943.963:9): arch=40000003 syscall=21
> success=no exit=-13 a0=b797a6f0 a1=b797a718 a2=b797a728 a3=c0ed0020
> items=0 ppid=2678 pid=2679 auid=4294967295 uid=0 gid=0 euid=0 suid=0
> fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="mount"
> exe="/bin/mount" subj=system_u:system_r:devicekit_power_t:s0-s0:c0.c1023

The fact that this is actually /bin/mount suggests there should be a
domtrans to mount_t.

> key=(null)
> type=AVC msg=audit(1298580944.756:10): avc:  denied  { remount } for
> pid=2680 comm="mount"
> scontext=system_u:system_r:devicekit_power_t:s0-s0:c0.c1023
> tcontext=system_u:object_r:fs_t:s0 tclass=filesystem
> 
> The process does not exist anymore, maybe it's a script (it could
> be /sbin/mount.tmpfs that I took from Fedora which has got the proper
> label mount_exec_t). It happens after boot-up, after policykit has
> started but before a graphical login (or any other sort of login).
> 
> If it is problematic then perhaps it can be left out for the time being.
> Everything else even without it seems to be working fine.
> 
>>>  term_use_all_terms(devicekit_power_t)
>>>  
>>> @@ -230,6 +239,9 @@ sysnet_domtrans_ifconfig(devicekit_power
>>>  
>>>  userdom_read_all_users_state(devicekit_power_t)
>>>  
>>> +mount_exec(devicekit_power_t)
>>> +mount_getattr_executable_file(devicekit_power_t)
>>
>> This getattr rule is a subset of the exec rule.
> 
> allow devicekit_power_t mount_exec_t:file { getattr read open execute
> execute_no_trans };
> 
> If it was a subset then we could get rid of the redundant one. But it
> seems to me that mount_exec() hasn't got
> corecmd_search_bin(devicekit_power_t) and allow devicekit_power_t
> mount_exec_t:file getattr_file_perms. The two interfaces seem to deal
> with disjoint sets of permissions...
> 
> You were trying to improve style ? I suspect it cannot be done at this
> time, please double-check.

In that case, add the corecmd_search_bin() to the mount_exec() interface.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com