[refpolicy] [PATCH v2] wm: update the window manager (wm) module and enable its role template

[guido@trentalancia.net (Guido Trentalancia)]

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 ?

> 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.?

Therefore, although theoretically one could distinguish between gnome-
shell and other window managers, there is very little benefit in
forking the policy.

> > Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> > ---
> > ?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.

> > 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.

> > ?	')
> > ?')
> > ?
> > @@ -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.

> > +
> > ?########################################
> > ?#
> > ?# 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.

> > +
> > ?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-shell needs to mmap some files

See above. I decided to let gnome-shell create and execute libffi files
in the home directory instead of tmp. This is much safer.

> > +
> > ?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