From: dac.override@gmail.com (Dominick Grift)
Date: Thu, 8 Dec 2016 22:13:26 +0100
Subject: [refpolicy] [PATCH v2] wm: update the window manager (wm)
 module and enable its role template
In-Reply-To: <1481230436.3170.13.camel@trentalancia.net>
References: <1481130053.3300.9.camel@trentalancia.net>
	<1481217618.20182.8.camel@trentalancia.net>
	<9c8eb718-b56e-f558-176d-c6623f005b9c@gmail.com>
	<1481230436.3170.13.camel@trentalancia.net>
Message-ID: <278b7093-a356-8aea-466e-c10aaddb8c64@gmail.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com


On 12/08/2016 09:53 PM, Guido Trentalancia via refpolicy wrote:
> On Thu, 08/12/2016 at 18.51 +0100, Dominick Grift via refpolicy wrote:
>> i
>> On 12/08/2016 06:20 PM, Guido Trentalancia via refpolicy wrote:
>>>
>>> Enable the window manager role (wm contrib module) and update
>>> the module to work with gnome-shell.
>>>
>>> This second version introduces better integration with common
>>> desktop applications and requires the following recently posted
>>> patch for the games module:
>>>
>>> [PATCH 1/2] games: general update and improved pulseaudio
>>> integration
>>> http://oss.tresys.com/pipermail/refpolicy/2016-December/008650.html
>>>
>>> This patch might need some more testing.
>>
>> Yes, i dont think this should be merged.
> 
> It needs to be tested with other window managers.
> 
> Have you found any actual limitation with the old gnome (gnome-
> panel/metacity), kde or others ?
> 

It is hard to explain but consider some of this stuff. gnome-shell
connects to http ports, gnome-shell needs to be able to run programs on
behalf of the calling user (ALT-f2), gnome-shell is a pulseaudio client,
gnome-shell needs comms with: bluetooth, accountsd, geoclue, upower,
logind: all things that a normal windows manager does not need.

Not to mention that gnome requires permissions that are specific to
wayland compositors

>> Also gnome-shell is a wm but its not every wm needs the permissions
>> that
>> gnome-shell needs. We should either give gnome-shell its own domain
>> or
>> add the gnome-shell specific rules to a tunable.
>>
>> For example: gnome-shell is a wayland compositor. but there are many
>> more things that gnome shell needs that a "normal" window manager
>> should
>> never need.
> 
> The permissions that have been added are very general and harmless. 

For now maybe because the policy is not comprehensive

>>>  policy/modules/contrib/pulseaudio.if |   19 +++++++++
>>>  policy/modules/contrib/wm.if         |   44 +++++++++++++++++++++
>>>  policy/modules/contrib/wm.te         |   73
>>> +++++++++++++++++++++++++++++++++++
>>>  policy/modules/roles/staff.te        |    1
>>>  policy/modules/roles/sysadm.te       |    1
>>>  policy/modules/roles/unprivuser.te  |    1
>>>  6 files changed, 139 insertions(+)
>>>
>>> diff -pruN refpolicy-git-07122016-
>>> orig/policy/modules/contrib/pulseaudio.if refpolicy-git-
>>> 07122016/policy/modules/contrib/pulseaudio.if
>>> --- refpolicy-git-07122016-
>>> orig/policy/modules/contrib/pulseaudio.if	2016-09-27
>>> 16:28:51.964139648 +0200
>>> +++ refpolicy-git-07122016/policy/modules/contrib/pulseaudio.if	
>>> 2016-12-08 15:47:40.117740787 +0100
>>> @@ -424,3 +424,22 @@ interface(`pulseaudio_rw_tmpfs_files',`
>>>  	fs_search_tmpfs($1)
>>>  	rw_files_pattern($1, pulseaudio_tmpfs_t,
>>> pulseaudio_tmpfs_t)
>>>  ')
>>> +
>>> +#######################################
>>> +## <summary>
>>> +##	Manage pulseaudio tmpfs files.
>>> +## </summary>
>>> +## <param name="domain">
>>> +##	<summary>
>>> +##	Domain allowed access.
>>> +##	</summary>
>>> +## </param>
>>> +#
>>> +interface(`pulseaudio_manage_tmpfs_files',`
>>> +	gen_require(`
>>> +		type pulseaudio_tmpfs_t;
>>> +	')
>>> +
>>> +	fs_search_tmpfs($1)
>>> +	manage_files_pattern($1, pulseaudio_tmpfs_t,
>>> pulseaudio_tmpfs_t)
>>> +')
>>
>> does not make sense, and i would like to know what prompted you to
>> add
>> this (preferably backed with avc denials)
> 
> It's needed to unlink them. Probably stale files from previous
> sessions.
> 
>> wm should is a pulseaudio_client and pulseaudio_client can already rw
>> and delete pulseaudio_tmpfs_files. It should not need more than that.
> 
> I decided to avoid using the pulseaudio_client attribute because it
> implies other unneeded permissions, including corenet permissions that
> are quite undesirable.
> 

Those permissions aren't unneeded. They should be conditional maybe but
there are configurations where clients may want to connect to pulseaudio
via the network.

>>> diff -pruN refpolicy-git-07122016-orig/policy/modules/contrib/wm.if 
>>> refpolicy-git-07122016/policy/modules/contrib/wm.if
>>> --- refpolicy-git-07122016-orig/policy/modules/contrib/wm.if	
>>> 2016-08-14 21:28:11.597521187 +0200
>>> +++ refpolicy-git-07122016/policy/modules/contrib/wm.if	2016
>>> -12-08 15:47:40.118740804 +0100
>>> @@ -47,6 +47,8 @@ template(`wm_role_template',`
>>>  	# Policy
>>>  	#
>>>  
>>> +	allow $3 $1_wm_t:fd use;
>>> +
>>>  	allow $1_wm_t $3:unix_stream_socket connectto;
>>>  	allow $3 $1_wm_t:unix_stream_socket connectto;
>>>  
>>> @@ -72,6 +74,7 @@ template(`wm_role_template',`
>>>  	xserver_manage_core_devices($1_wm_t)
>>>  
>>>  	optional_policy(`
>>> +		dbus_connect_spec_session_bus($1, $1_wm_t)
>>>  		dbus_spec_session_bus_client($1, $1_wm_t)
>>>  		dbus_system_bus_client($1_wm_t)
>>>  
>>> @@ -81,11 +84,14 @@ template(`wm_role_template',`
>>>  	')
>>>  
>>>  	optional_policy(`
>>> +		gnome_dbus_chat_gkeyringd($1, $1_wm_t)
>>>  		gnome_stream_connect_gkeyringd($1, $1_wm_t)
>>>  	')
>>>  
>>>  	optional_policy(`
>>>  		pulseaudio_run($1_wm_t, $2)
>>> +		pulseaudio_manage_tmpfs_files($1_wm_t)
>>> +		pulseaudio_use_fds($1_wm_t)
>>
>> these shouldnt be needed. what prompted you to add these? (preferably
>> show avc denials)
> 
> See above. Stale files need to be unlinked.

That is a bug here then:

https://github.com/TresysTechnology/refpolicy-contrib/blob/master/pulseaudio.te#L241

clients should then also be able to delete pulseaudio tmpfs files

> 
>>>  	')
>>>  ')
>>>  
>>> @@ -134,3 +140,41 @@ interface(`wm_dbus_chat',`
>>>  	allow $2 $1_wm_t:dbus send_msg;
>>>  	allow $1_wm_t $2:dbus send_msg;
>>>  ')
>>> +
>>> +########################################
>>> +## <summary>
>>> +##	Do not audit attempts to execute
>>> +##	files in temporary directories.
>>> +## </summary>
>>> +## <param name="domain">
>>> +##	<summary>
>>> +##	Domain to not audit.
>>> +##	</summary>
>>> +## </param>
>>> +#
>>> +interface(`wm_dontaudit_exec_tmp_files',`
>>> +	gen_require(`
>>> +		type wm_tmp_t;
>>> +	')
>>> +
>>> +	dontaudit $1 wm_tmp_t:file exec_file_perms;
>>> +')
>>> +
>>> +########################################
>>> +## <summary>
>>> +##	Do not audit attempts to execute
>>> +##	files in temporary filesystems.
>>> +## </summary>
>>> +## <param name="domain">
>>> +##	<summary>
>>> +##	Domain to not audit.
>>> +##	</summary>
>>> +## </param>
>>> +#
>>> +interface(`wm_dontaudit_exec_tmpfs_files',`
>>> +	gen_require(`
>>> +		type wm_tmpfs_t;
>>> +	')
>>> +
>>> +	dontaudit $1 wm_tmpfs_t:file exec_file_perms;
>>> +')
>>> diff -pruN refpolicy-git-07122016-orig/policy/modules/contrib/wm.te 
>>> refpolicy-git-07122016/policy/modules/contrib/wm.te
>>> --- refpolicy-git-07122016-orig/policy/modules/contrib/wm.te	
>>> 2016-10-29 16:29:19.762328008 +0200
>>> +++ refpolicy-git-07122016/policy/modules/contrib/wm.te	2016
>>> -12-08 17:57:40.843768477 +0100
>>> @@ -10,6 +10,14 @@ attribute wm_domain;
>>>  type wm_exec_t;
>>>  corecmd_executable_file(wm_exec_t)
>>>  
>>> +type wm_tmp_t;
>>> +typealias wm_tmp_t alias { user_wm_tmp_t staff_wm_tmp_t
>>> sysadm_wm_tmp_t };
>>> +userdom_user_tmp_file(wm_tmp_t)
>>> +
>>> +type wm_tmpfs_t;
>>> +typealias wm_tmpfs_t alias { user_wm_tmpfs_t staff_wm_tmpfs_t
>>> sysadm_wm_tmpfs_t };
>>> +userdom_user_tmpfs_file(wm_tmpfs_t)
>>
>> this is a pulseaudio_tmpfs_file()
> 
> I am not sure about that.
> 
> It works fine, so at the moment I can't see a reason to change it.
> 
> It is probably used to create libffi files and not pulseaudio files
> (that I expect to be created by the pulseaudio process). See below.
> 

All pulseaudio clients need to be able to read/write/unlink eachother
tmpfs files.

>>> +
>>>  ########################################
>>>  #
>>>  # Common wm domain local policy
>>> @@ -21,31 +29,60 @@ allow wm_domain self:netlink_kobject_uev
>>>  allow wm_domain self:shm create_shm_perms;
>>>  allow wm_domain self:unix_dgram_socket create_socket_perms;
>>>  
>>> +manage_dirs_pattern(wm_domain, wm_tmp_t, wm_tmp_t)
>>> +manage_files_pattern(wm_domain, wm_tmp_t, wm_tmp_t)
>>> +manage_lnk_files_pattern(wm_domain, wm_tmp_t, wm_tmp_t)
>>> +files_tmp_filetrans(wm_domain, wm_tmp_t, { dir file lnk_file })
>>> +
>>> +manage_dirs_pattern(wm_domain, wm_tmpfs_t, wm_tmpfs_t)
>>> +manage_files_pattern(wm_domain, wm_tmpfs_t, wm_tmpfs_t)
>>> +manage_lnk_files_pattern(wm_domain, wm_tmpfs_t, wm_tmpfs_t)
>>> +fs_tmpfs_filetrans(wm_domain, wm_tmpfs_t, { dir file lnk_file })
>>> +
>>> +can_exec(wm_domain, wm_exec_t)
>>> +
>>>  kernel_read_system_state(wm_domain)
>>>  
>>>  corecmd_getattr_all_executables(wm_domain)
>>>  
>>> +dev_read_rand(wm_domain)
>>>  dev_read_sound(wm_domain)
>>>  dev_read_sysfs(wm_domain)
>>>  dev_read_urand(wm_domain)
>>> +dev_rw_dri(wm_domain)
>>>  dev_rw_wireless(wm_domain)
>>>  dev_write_sound(wm_domain)
>>>  
>>> +files_read_etc_runtime_files(wm_domain)
>>>  files_read_usr_files(wm_domain)
>>>  
>>>  fs_getattr_all_fs(wm_domain)
>>>  
>>> +kernel_read_fs_sysctls(wm_domain)
>>> +kernel_read_proc_symlinks(wm_domain)
>>> +kernel_read_sysctl(wm_domain)
>>> +
>>>  miscfiles_read_fonts(wm_domain)
>>> +miscfiles_read_generic_certs(wm_domain)
>>>  miscfiles_read_localization(wm_domain)
>>>  
>>> +udev_read_pid_files(wm_domain)
>>> +
>>> +# this is needed by gnome-shell
>>> +userdom_exec_user_home_content_files(wm_domain)
>>
>> What exactly is it executing here?
> 
> I think it is executing files generated by libffi.

and how do those files end up in $HOME, maybe because you're blocking
access to tmp (XDG_RUNTIME_DIR)?

/run/user/UID is first choice

> 
>>> +
>>>  userdom_manage_user_tmp_sockets(wm_domain)
>>>  userdom_tmp_filetrans_user_tmp(wm_domain, sock_file)
>>>  userdom_user_runtime_filetrans_user_tmp(wm_domain, sock_file)
>>>  
>>>  userdom_manage_user_home_content_dirs(wm_domain)
>>>  userdom_manage_user_home_content_files(wm_domain)
>>> +
>>>  userdom_user_home_dir_filetrans_user_home_content(wm_domain, { dir
>>> file })
>>>  
>>> +wm_dontaudit_exec_tmp_files(wm_domain)
>>> +wm_dontaudit_exec_tmpfs_files(wm_domain)
>>
>> why dontaudit the above? gnome-sheM


These files are probably initially created in $CDG_RUNTIME_DIR which is
the best place. However if you block that then it falls back to ~/.cache
(which is second best)

> 
>>> +
>>>  optional_policy(`
>>>  	accountsd_dbus_chat(wm_domain)
>>>  ')
>>> @@ -55,10 +92,42 @@ optional_policy(`
>>>  ')		
>>>  
>>>  optional_policy(`
>>> +	consolekit_dbus_chat(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>>  	devicekit_dbus_chat_power(wm_domain)
>>>  ')
>>>  
>>>  optional_policy(`
>>> +	evolution_dbus_chat(wm_domain)
>>> +	evolution_alarm_dbus_chat(wm_domain)
>>> +	evolution_domtrans(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>> +	games_dbus_chat(wm_domain)
>>> +	games_domtrans(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>> +	java_domtrans(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>> +	mono_domtrans(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>> +	mozilla_domtrans(wm_domain)
>>> +	mozilla_dbus_chat(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>> +	mplayer_domtrans(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>>  	networkmanager_dbus_chat(wm_domain)
>>>  ')
>>>  
>>> @@ -71,5 +140,9 @@ optional_policy(`
>>>  ')
>>>  
>>>  optional_policy(`
>>> +	telepathy_mission_control_dbus_chat(wm_domain)
>>> +')
>>> +
>>> +optional_policy(`
>>>  	userhelper_exec_consolehelper(wm_domain)
>>>  ')
>>> diff -pruN refpolicy-git-07122016-
>>> orig/policy/modules/roles/staff.te refpolicy-git-
>>> 07122016/policy/modules/roles/staff.te
>>> --- refpolicy-git-07122016-orig/policy/modules/roles/staff.te	
>>> 2016-12-07 13:39:08.669449296 +0100
>>> +++ refpolicy-git-07122016/policy/modules/roles/staff.te	201
>>> 6-12-08 15:47:40.140741170 +0100
>>> @@ -85,6 +85,7 @@ ifndef(`distro_redhat',`
>>>  
>>>  		optional_policy(`
>>>  			gnome_role_template(staff, staff_r,
>>> staff_t)
>>> +			wm_role_template(staff, staff_r, staff_t)
>>>  		')
>>>  
>>>  		optional_policy(`
>>> diff -pruN refpolicy-git-07122016-
>>> orig/policy/modules/roles/sysadm.te refpolicy-git-
>>> 07122016/policy/modules/roles/sysadm.te
>>> --- refpolicy-git-07122016-orig/policy/modules/roles/sysadm.te	
>>> 2016-12-07 13:39:08.669449296 +0100
>>> +++ refpolicy-git-07122016/policy/modules/roles/sysadm.te	20
>>> 16-12-08 15:47:40.141741187 +0100
>>> @@ -1245,6 +1245,7 @@ ifndef(`distro_redhat',`
>>>  
>>>  		optional_policy(`
>>>  			gnome_role_template(sysadm, sysadm_r,
>>> sysadm_t)
>>> +			wm_role_template(sysadm, sysadm_r,
>>> sysadm_t)
>>>  		')
>>>  	')
>>>  
>>> diff -pruN refpolicy-git-07122016-
>>> orig/policy/modules/roles/unprivuser.te refpolicy-git-
>>> 07122016/policy/modules/roles/unprivuser.te
>>> --- refpolicy-git-07122016-orig/policy/modules/roles/unprivuser.te	
>>> 2016-12-07 13:39:08.669449296 +0100
>>> +++ refpolicy-git-07122016/policy/modules/roles/unprivuser.te	
>>> 2016-12-08 15:47:40.141741187 +0100
>>> @@ -54,6 +54,7 @@ ifndef(`distro_redhat',`
>>>  
>>>  		optional_policy(`
>>>  			gnome_role_template(user, user_r, user_t)
>>> +			wm_role_template(user, user_r, user_t)
>>>  		')
>>>  
>>>  		optional_policy(`
> 
> Guido
> 
> 
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
> 


-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 648 bytes
Desc: OpenPGP digital signature
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20161208/dd863caa/attachment-0001.bin