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

[guido@trentalancia.net (Guido Trentalancia)]

Hello Jason,

you took the list off for this message, I think this is not
intentional, so I am bringing the reply back on the list...

On Wed, 14/12/2016 at 21.01 +0800, Jason Zaman wrote:
> 
> 
> On 14 Dec 2016 09:54, "Guido Trentalancia via refpolicy" <refpolicy@o
> ss.tresys.com> wrote:
> Enable the window manager role (wm contrib module) and update
> the module to work with gnome-shell.
> 
> This patch requires the following recently posted patch for the
> games module:
> 
> [PATCH v3 1/2] games: general update and improved pulseaudio
> integration
> http://oss.tresys.com/pipermail/refpolicy/2016-December/008679.html
> 
> This patch has received some testing with the following two
> configurations:
> - gnome-shell executing in normal mode (with display managers
> other than gdm, such as xdm from XOrg);
> - gnome-shell executing in gdm mode (with the Gnome Display
> Manager).
> 
> Patches 3/5, 4/5 and 5/5 are needed when gnome-shell is used
> in conjunction with gdm.
> 
> Since the window managers are not limited by gnome-shell, this latter
> version of the patch (along with part 2/5) uses separate optional
> conditionals for the gnome and wm role templates.
> 
> Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> ---
> ?policy/modules/contrib/colord.te? ?|? ? 5 ++
> ?policy/modules/contrib/dbus.te? ? ?|? ? 5 ++
> ?policy/modules/contrib/wm.if? ? ? ?|? ?43 +++++++++++++++++-
> ?policy/modules/contrib/wm.te? ? ? ?|? ?88
> ++++++++++++++++++++++++++++++++++++-
> ?policy/modules/roles/staff.te? ? ? |? ? 8 ++-
> ?policy/modules/roles/sysadm.te? ? ?|? ? 4 +
> ?policy/modules/roles/unprivuser.te |? ? 8 ++-
> ?7 files changed, 155 insertions(+), 6 deletions(-)
> 
> diff -pruN refpolicy-git-07122016-
> orig/policy/modules/contrib/colord.te refpolicy-git-
> 07122016/policy/modules/contrib/colord.te
> --- refpolicy-git-07122016-orig/policy/modules/contrib/colord.te? ? ?
> ? 2016-08-14 21:28:11.468519205 +0200
> +++ refpolicy-git-07122016/policy/modules/contrib/colord.te? ? ?2016-
> 12-14 02:45:54.815580399 +0100
> 
> First off, can you use git format patch and git send email because
> refpolicy has contrib as a submodule and the way you've sent it won't
> apply.?

Yes, I remember, you told me already another time about that, but to be
honest I have experienced difficulties in creating patches using git.

However, apart from the slightly different formatting, it applies
cleanly to the contrib submodule: just use "patch -p1 <
name_of_the_patch.patch"

> @@ -137,3 +137,8 @@ optional_policy(`
> ? ? ? ? udev_read_db(colord_t)
> ? ? ? ? udev_read_pid_files(colord_t)
> ?')
> +
> +optional_policy(`
> +? ? ? ?xserver_read_xdm_lib_files(colord_t)
> +? ? ? ?xserver_use_xdm_fds(colord_t)
> +')
> diff -pruN refpolicy-git-07122016-orig/policy/modules/contrib/dbus.te 
> refpolicy-git-07122016/policy/modules/contrib/dbus.te
> --- refpolicy-git-07122016-orig/policy/modules/contrib/dbus.te? 2016-
> 08-14 21:28:11.477519343 +0200
> +++ refpolicy-git-07122016/policy/modules/contrib/dbus.te? ? ? ?2016-
> 12-14 02:24:00.796768671 +0100
> @@ -159,6 +159,11 @@ optional_policy(`
> ? ? ? ? udev_read_db(system_dbusd_t)
> ?')
> 
> +optional_policy(`
> +? ? ? ?xserver_read_xdm_lib_files(system_dbusd_t)
> +? ? ? ?xserver_use_xdm_fds(system_dbusd_t)
> +')
> +
> ?########################################
> ?#
> ?# Common session bus local policy
> 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-
> 12-14 02:24:53.377000472 +0100
> +++ refpolicy-git-07122016/policy/modules/contrib/wm.if 2016-12-13
> 22:53:54.445212825 +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,7 +84,7 @@ template(`wm_role_template',`
> ? ? ? ? ')
> 
> ? ? ? ? optional_policy(`
> -? ? ? ? ? ? ? ?gnome_stream_connect_gkeyringd($1, $1_wm_t)
> +? ? ? ? ? ? ? ?gnome_stream_connect_all_gkeyringd($1_wm_t)
> ? ? ? ? ')
> 
> ? ? ? ? optional_policy(`
> @@ -134,3 +137,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-
> 12-14 02:24:53.396000918 +0100
> +++ refpolicy-git-07122016/policy/modules/contrib/wm.te 2016-12-13
> 00:34:34.876856837 +0100
> @@ -10,6 +10,18 @@ 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)
> +
> +optional_policy(`
> +? ? ? ?pulseaudio_tmpfs_content(wm_tmpfs_t)
> +')
> +
> ?########################################
> ?#
> ?# Common wm domain local policy
> @@ -21,31 +33,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)
> 
> I'm pretty sure this is not required. This perm depends on your type
> of graphics card and driver. OpenGL on my machine needs it but other
> people I've talked to don't require it. I think I am going to add
> this as a Boolean to allow rw_dri(xdomain) so people with those kinds
> of cards can enable it everywhere instead of forcing it for people
> who don't need it.?

It is harmless and it benefits the DRI driver. If you haven't got a
graphic card with DRI or if you have DRI disabled, it just does
nothing.

It is simply a permission to write to devices in /dev/dri/*.

A boolean would just make things complicate without any benefit for
anyone.

It is not forcing anything, it is just supporting the natural behaviour
of an application carrying out a licit operation.

> ?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)
> +
> ?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)
> +
> ?optional_policy(`
> ? ? ? ? accountsd_dbus_chat(wm_domain)
> ?')
> @@ -55,10 +96,51 @@ optional_policy(`
> ?')
> 
> ?optional_policy(`
> +? ? ? ?consolekit_dbus_chat(wm_domain)
> +')
> +
> +optional_policy(`
> ? ? ? ? devicekit_dbus_chat_power(wm_domain)
> ?')
> 
> ?optional_policy(`
> +? ? ? ?evolution_domtrans(wm_domain)
> +
> +? ? ? ?optional_policy(`
> +? ? ? ? ? ? ? ?evolution_dbus_chat(wm_domain)
> +? ? ? ? ? ? ? ?evolution_alarm_dbus_chat(wm_domain)
> +? ? ? ?')
> +')
> +
> +optional_policy(`
> +? ? ? ?games_domtrans(wm_domain)
> +
> +? ? ? ?optional_policy(`
> +? ? ? ? ? ? ? ?games_dbus_chat(wm_domain)
> +? ? ? ?')
> +')
> +
> +optional_policy(`
> +? ? ? ?java_domtrans(wm_domain)
> +')
> +
> +optional_policy(`
> +? ? ? ?mono_domtrans(wm_domain)
> +')
> +
> +optional_policy(`
> +? ? ? ?mozilla_domtrans(wm_domain)
> +
> +? ? ? ?optional_policy(`
> +? ? ? ? ? ? ? ?mozilla_dbus_chat(wm_domain)
> +? ? ? ?')
> +')
> +
> +optional_policy(`
> +? ? ? ?mplayer_domtrans(wm_domain)
> +')
> +
> +optional_policy(`
> ? ? ? ? networkmanager_dbus_chat(wm_domain)
> ?')
> 
> Whoa are we going to have to add every single application to
> wm_domain to be able to run it? That will get annoying super fast.
> Isn't there an application_domain attribute we can use? If there
> isn't we might want to reverse this so X application types instead
> declare that wm can run them (something like the application_type
> interface)

I don't understand what you mean.

> @@ -67,9 +149,13 @@ optional_policy(`
> ?')
> 
> ?optional_policy(`
> -? ? ? ?pulseaudio_stream_connect(wm_domain)
> +? ? ? ?telepathy_mission_control_dbus_chat(wm_domain)
> ?')
> 
> ?optional_policy(`
> ? ? ? ? userhelper_exec_consolehelper(wm_domain)
> ?')
> +
> +optional_policy(`
> +? ? ? ?xserver_dbus_chat_xdm(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-14 02:24:53.397000941 +0100
> +++ refpolicy-git-07122016/policy/modules/roles/staff.te? ? ? ? 2016-
> 12-13 22:45:02.857851229 +0100
> @@ -88,11 +88,11 @@ ifndef(`distro_redhat',`
> ? ? ? ? ? ? ? ? ')
> 
> ? ? ? ? ? ? ? ? optional_policy(`
> -? ? ? ? ? ? ? ? ? ? ? ?pulseaudio_role(staff_r, staff_t)
> +? ? ? ? ? ? ? ? ? ? ? ?telepathy_role_template(staff, staff_r,
> staff_t)
> ? ? ? ? ? ? ? ? ')
> 
> ? ? ? ? ? ? ? ? optional_policy(`
> -? ? ? ? ? ? ? ? ? ? ? ?telepathy_role_template(staff, staff_r,
> staff_t)
> +? ? ? ? ? ? ? ? ? ? ? ?wm_role_template(staff, staff_r, staff_t)
> ? ? ? ? ? ? ? ? ')
> ? ? ? ? ')
> 
> @@ -145,6 +145,10 @@ ifndef(`distro_redhat',`
> ? ? ? ? ')
> 
> ? ? ? ? optional_policy(`
> +? ? ? ? ? ? ? ?pulseaudio_role(staff_r, staff_t)
> +? ? ? ?')
> +
> 
> This has nothing to do with window managers. Pulse stuff should be in
> a separate patch.?

Yes, it has nothing to do with window managers.

It is a general bug in the current policy, because pulseaudio does not
depend on dbus.

I took a chance to fix it, which is probably better than not fixing it.

> +? ? ? ?optional_policy(`
> ? ? ? ? ? ? ? ? pyzor_role(staff_r, staff_t)
> ? ? ? ? ')
> 
> 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-14 02:24:53.397000941 +0100
> +++ refpolicy-git-07122016/policy/modules/roles/sysadm.te? ? ? ?2016-
> 12-13 22:45:25.577422292 +0100
> @@ -1246,6 +1246,10 @@ ifndef(`distro_redhat',`
> ? ? ? ? ? ? ? ? optional_policy(`
> ? ? ? ? ? ? ? ? ? ? ? ? gnome_role_template(sysadm, sysadm_r,
> sysadm_t)
> ? ? ? ? ? ? ? ? ')
> +
> +? ? ? ? ? ? ? ?optional_policy(`
> +? ? ? ? ? ? ? ? ? ? ? ?wm_role_template(sysadm, sysadm_r, sysadm_t)
> +? ? ? ? ? ? ? ?')
> ? ? ? ? ')
> 
> ? ? ? ? optional_policy(`
> 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-14 02:24:53.398000965 +0100
> +++ refpolicy-git-07122016/policy/modules/roles/unprivuser.te? ?2016-
> 12-13 22:44:50.493540449 +0100
> @@ -57,11 +57,11 @@ ifndef(`distro_redhat',`
> ? ? ? ? ? ? ? ? ')
> 
> ? ? ? ? ? ? ? ? optional_policy(`
> -? ? ? ? ? ? ? ? ? ? ? ?pulseaudio_role(user_r, user_t)
> +? ? ? ? ? ? ? ? ? ? ? ?telepathy_role_template(user, user_r, user_t)
> ? ? ? ? ? ? ? ? ')
> 
> ? ? ? ? ? ? ? ? optional_policy(`
> -? ? ? ? ? ? ? ? ? ? ? ?telepathy_role_template(user, user_r, user_t)
> +? ? ? ? ? ? ? ? ? ? ? ?wm_role_template(user, user_r, user_t)
> ? ? ? ? ? ? ? ? ')
> ? ? ? ? ')
> 
> @@ -122,6 +122,10 @@ ifndef(`distro_redhat',`
> ? ? ? ? ')
> 
> ? ? ? ? optional_policy(`
> +? ? ? ? ? ? ? ?pulseaudio_role(user_r, user_t)
> +? ? ? ?')
> +
> 
> What's up with the random rearranging of the lines here? It makes the
> patch seem like it's doing more than it is.?
> 
> +? ? ? ?optional_policy(`
> ? ? ? ? ? ? ? ? pyzor_role(user_r, user_t)
> ? ? ? ? ')

I have tested the patchset with and without gdm and it works fine.

I recommend to apply it as it brings benefits to all Reference Policy
users.

Guido