From: guido@trentalancia.net (Guido Trentalancia)
Date: Thu, 08 Dec 2016 22:51:21 +0100
Subject: [refpolicy] [PATCH v2] wm: update the window manager (wm)
module and enable its role template
In-Reply-To: <278b7093-a356-8aea-466e-c10aaddb8c64@gmail.com>
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>
<278b7093-a356-8aea-466e-c10aaddb8c64@gmail.com>
Message-ID: <1481233881.3170.19.camel@trentalancia.net>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com
On Thu, 08/12/2016 at 22.13 +0100, Dominick Grift via refpolicy wrote:
> 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.
gnome-shell does not need to connect to http ports and the Alt-F2 key
combination is not used to run programs.
Once again, the difference in terms of permissions compared to the
actual module is mostly general and harmless.
> Not to mention that gnome requires permissions that are specific to
> wayland compositors
The module is only tested with the X server and not wayland.
If and when support for wayland will be introduced, we'll decide
whether or not to fork new policy specifically for gnome-shell. As
already explained, at the moment, it definitely doesn't pay back.
> > > 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)
> > > > ?')
> > > > +
> > > > +#######################################
> > > > +##
> > > > +## Manage pulseaudio tmpfs files.
> > > > +##
> > > > +##
> > > > +##
> > > > +## Domain allowed access.
> > > > +##
> > > > +##
> > > > +#
> > > > +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/pul
> seaudio.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;
> > > > ?')
> > > > +
> > > > +########################################
> > > > +##
> > > > +## Do not audit attempts to execute
> > > > +## files in temporary directories.
> > > > +##
> > > > +##
> > > > +##
> > > > +## Domain to not audit.
> > > > +##
> > > > +##
> > > > +#
> > > > +interface(`wm_dontaudit_exec_tmp_files',`
> > > > + gen_require(`
> > > > + type wm_tmp_t;
> > > > + ')
> > > > +
> > > > + dontaudit $1 wm_tmp_t:file exec_file_perms;
> > > > +')
> > > > +
> > > > +########################################
> > > > +##
> > > > +## Do not audit attempts to execute
> > > > +## files in temporary filesystems.
> > > > +##
> > > > +##
> > > > +##
> > > > +## Domain to not audit.
> > > > +##
> > > > +##
> > > > +#
> > > > +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
No, it doesn't work like that.
And the home directory is the safest option.
> > > > +
> > > > ?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