From: Laurent Bigonville <[email protected]>
Recent versions of dbus are started as Type=notify
type=AVC msg=audit(03/10/19 15:32:40.347:64) : avc: denied { write } for pid=809 comm=dbus-daemon name=notify dev="tmpfs" ino=1751 scontext=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 tcontext=system_u:object_r:init_runtime_t:s0 tclass=sock_file permissive=1
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/services/dbus.te | 3 +++
1 file changed, 3 insertions(+)
diff --git a/policy/modules/services/dbus.te b/policy/modules/services/dbus.te
index bb3dac7d..58b03757 100644
--- a/policy/modules/services/dbus.te
+++ b/policy/modules/services/dbus.te
@@ -159,6 +159,9 @@ ifdef(`init_systemd', `
# for /run/systemd/dynamic-uid/
init_list_pids(system_dbusd_t)
init_read_runtime_symlinks(system_dbusd_t)
+
+ # Recent versions of dbus are started as Type=notify
+ init_write_runtime_socket(system_dbusd_t)
')
optional_policy(`
--
2.23.0
From: Laurent Bigonville <[email protected]>
colord reads the color profiles files that are stored in
~/.local/share/icc/, The file descriptor to that file is passed over
D-Bus so it needs to be inherited
----
time->Sat Oct 5 11:35:54 2019
type=AVC msg=audit(1570268154.991:223): avc: denied { read } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
type=AVC msg=audit(1570268154.991:223): avc: denied { use } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=fd permissive=1
----
time->Sat Oct 5 11:35:55 2019
type=AVC msg=audit(1570268155.007:225): avc: denied { getattr } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
----
time->Sat Oct 5 11:35:55 2019
type=AVC msg=audit(1570268155.007:226): avc: denied { map } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
----
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/services/colord.te | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/policy/modules/services/colord.te b/policy/modules/services/colord.te
index fada3fb8..2fbb1835 100644
--- a/policy/modules/services/colord.te
+++ b/policy/modules/services/colord.te
@@ -141,6 +141,13 @@ optional_policy(`
udev_read_pid_files(colord_t)
')
+# colord reads the color profiles files that are stored in ~/.local/share/icc/,
+# The file descriptor to that file is passed over D-Bus so it needs to be inherited
+optional_policy(`
+ unconfined_use_fds(colord_t)
+ xdg_read_data_files(colord_t)
+')
+
optional_policy(`
xserver_read_xdm_lib_files(colord_t)
xserver_use_xdm_fds(colord_t)
--
2.23.0
From: Laurent Bigonville <[email protected]>
----
time->Sun Oct 6 15:05:29 2019
type=AVC msg=audit(1570367129.524:673): avc: denied { write } for pid=9609 comm="systemd-localed" name="notify" dev="tmpfs" ino=18551 scontext=system_u:system_r:systemd_locale_t:s0 tcontext=system_u:object_r:init_runtime_t:s0 tclass=sock_file permissive=1
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/system/systemd.te | 2 ++
1 file changed, 2 insertions(+)
diff --git a/policy/modules/system/systemd.te b/policy/modules/system/systemd.te
index 1422d8e2..3eeb8c64 100644
--- a/policy/modules/system/systemd.te
+++ b/policy/modules/system/systemd.te
@@ -395,6 +395,8 @@ kernel_read_kernel_sysctls(systemd_locale_t)
files_read_etc_files(systemd_locale_t)
+init_write_runtime_socket(systemd_locale_t)
+
seutil_read_file_contexts(systemd_locale_t)
systemd_log_parse_environment(systemd_locale_t)
--
2.23.0
From: Laurent Bigonville <[email protected]>
colord-sane daemon indirectly links against libsnmp, the library tries
to read files in /var/lib/snmp
----
type=AVC msg=audit(06/10/19 12:58:49.639:83) : avc: denied { getattr } for pid=873 comm=colord-sane path=/var/lib/snmp dev="dm-1" ino=399773 scontext=system_u:system_r:colord_t:s0 tcontext=system_u:object_r:snmpd_var_lib_t:s0 tclass=dir permissive=1
----
type=AVC msg=audit(06/10/19 12:58:49.639:84) : avc: denied { search } for pid=873 comm=colord-sane name=snmp dev="dm-1" ino=399773 scontext=system_u:system_r:colord_t:s0 tcontext=system_u:object_r:snmpd_var_lib_t:s0 tclass=dir permissive=1
----
type=AVC msg=audit(06/10/19 12:58:49.647:85) : avc: denied { search } for pid=873 comm=colord-sane name=snmp dev="dm-1" ino=399773 scontext=system_u:system_r:colord_t:s0 tcontext=system_u:object_r:snmpd_var_lib_t:s0 tclass=dir permissive=1
----
type=AVC msg=audit(06/10/19 12:58:49.647:86) : avc: denied { getattr } for pid=873 comm=colord-sane path=/var/lib/snmp dev="dm-1" ino=399773 scontext=system_u:system_r:colord_t:s0 tcontext=system_u:object_r:snmpd_var_lib_t:s0 tclass=dir permissive=1
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/services/colord.te | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/policy/modules/services/colord.te b/policy/modules/services/colord.te
index 2fbb1835..ec03244b 100644
--- a/policy/modules/services/colord.te
+++ b/policy/modules/services/colord.te
@@ -132,6 +132,10 @@ optional_policy(`
policykit_read_reload(colord_t)
')
+optional_policy(`
+ snmp_read_snmp_var_lib_files(colord_t)
+')
+
optional_policy(`
sysnet_exec_ifconfig(colord_t)
')
--
2.23.0
From: Laurent Bigonville <[email protected]>
----
time->Thu Oct 3 17:16:40 2019
type=AVC msg=audit(1570115800.136:513): avc: denied { create } for pid=1384 comm="geoclue" scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:system_r:geoclue_t:s0 tclass=unix_dgram_socket permissive=1
----
time->Thu Oct 3 17:16:40 2019
type=AVC msg=audit(1570115800.136:514): avc: denied { sendto } for pid=1384 comm="geoclue" path="/run/systemd/journal/socket" scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:system_r:syslogd_t:s0 tc
lass=unix_dgram_socket permissive=1
type=AVC msg=audit(1570115800.136:514): avc: denied { write } for pid=1384 comm="geoclue" name="socket" dev="tmpfs" ino=1781 scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:object_r:devlog_t:s0 tcla
ss=sock_file permissive=1
type=AVC msg=audit(1570115800.136:514): avc: denied { search } for pid=1384 comm="geoclue" name="journal" dev="tmpfs" ino=1777 scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:object_r:syslogd_runtim
e_t:s0 tclass=dir permissive=1
type=AVC msg=audit(1570115800.136:514): avc: denied { search } for pid=1384 comm="geoclue" name="systemd" dev="tmpfs" ino=11001 scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:object_r:init_runtime_
t:s0 tclass=dir permissive=1
type=AVC msg=audit(1570115800.136:514): avc: denied { write } for pid=1384 comm="geoclue" scontext=system_u:system_r:geoclue_t:s0 tcontext=system_u:system_r:geoclue_t:s0 tclass=unix_dgram_socket permissive=1
----
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/services/geoclue.te | 2 ++
1 file changed, 2 insertions(+)
diff --git a/policy/modules/services/geoclue.te b/policy/modules/services/geoclue.te
index c6e66408..a36bcb80 100644
--- a/policy/modules/services/geoclue.te
+++ b/policy/modules/services/geoclue.te
@@ -30,6 +30,8 @@ dev_read_urand(geoclue_t)
auth_use_nsswitch(geoclue_t)
+logging_send_syslog_msg(geoclue_t)
+
miscfiles_read_generic_certs(geoclue_t)
miscfiles_read_localization(geoclue_t)
--
2.23.0
From: Laurent Bigonville <[email protected]>
The pid file is deleted by NetworkManager itself when the vpnc process
exits
NetworkManager call vpnc the following way:
system_u:system_r:NetworkManager_t:s0 root 11692 0.0 0.0 166272 9472 ? Sl 12:58 0:00 /usr/lib/NetworkManager/nm-vpnc-service --bus-name org.freedesktop.NetworkManager.vpnc.Connection_21
system_u:system_r:vpnc_t:s0 root 11703 0.1 0.0 9900 4896 ? SL 12:58 0:00 /usr/sbin/vpnc --no-detach --pid-file /var/run/NetworkManager/nm-vpnc-fa482929-93ee-4c64-bfba-4ee31d70f35f.pid -
----
type=AVC msg=audit(07/10/19 10:38:35.131:931) : avc: denied { write } for pid=8653 comm=vpnc path=/run/NetworkManager/nm-vpnc-fa482929-93ee-4c64-bfba-4ee31d70f35f.pid dev="tmpfs" ino=112390 scontext=system_u:system_r:vpnc_t:s0 tcontext=system_u:object_r:NetworkManager_runtime_t:s0 tclass=file permissive=1
type=AVC msg=audit(07/10/19 10:38:35.131:931) : avc: denied { create } for pid=8653 comm=vpnc name=nm-vpnc-fa482929-93ee-4c64-bfba-4ee31d70f35f.pid scontext=system_u:system_r:vpnc_t:s0 tcontext=system_u:object_r:NetworkManager_runtime_t:s0 tclass=file permissive=1
type=AVC msg=audit(07/10/19 10:38:35.131:931) : avc: denied { add_name } for pid=8653 comm=vpnc name=nm-vpnc-fa482929-93ee-4c64-bfba-4ee31d70f35f.pid scontext=system_u:system_r:vpnc_t:s0 tcontext=system_u:object_r:NetworkManager_runtime_t:s0 tclass=dir permissive=1
type=AVC msg=audit(07/10/19 10:38:35.131:931) : avc: denied { write } for pid=8653 comm=vpnc name=NetworkManager dev="tmpfs" ino=30783 scontext=system_u:system_r:vpnc_t:s0 tcontext=system_u:object_r:NetworkManager_runtime_t:s0 tclass=dir permissive=1
This commit also adds the needed interfaces
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/admin/vpn.te | 2 ++
policy/modules/services/networkmanager.if | 38 +++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/policy/modules/admin/vpn.te b/policy/modules/admin/vpn.te
index 01fd8340..6b2ad24c 100644
--- a/policy/modules/admin/vpn.te
+++ b/policy/modules/admin/vpn.te
@@ -124,6 +124,8 @@ optional_policy(`
optional_policy(`
networkmanager_attach_tun_iface(vpnc_t)
+ networkmanager_create_pid_files(vpnc_t)
+ networkmanager_rw_pid_files(vpnc_t)
')
optional_policy(`
diff --git a/policy/modules/services/networkmanager.if b/policy/modules/services/networkmanager.if
index 4c6dd342..fb89f210 100644
--- a/policy/modules/services/networkmanager.if
+++ b/policy/modules/services/networkmanager.if
@@ -253,6 +253,25 @@ interface(`networkmanager_append_log_files',`
append_files_pattern($1, NetworkManager_log_t, NetworkManager_log_t)
')
+########################################
+## <summary>
+## Create networkmanager pid files.
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+#
+interface(`networkmanager_create_pid_files',`
+ gen_require(`
+ type NetworkManager_runtime_t;
+ ')
+
+ files_search_pids($1)
+ create_files_pattern($1, NetworkManager_runtime_t, NetworkManager_runtime_t)
+')
+
########################################
## <summary>
## Read networkmanager pid files.
@@ -273,6 +292,25 @@ interface(`networkmanager_read_pid_files',`
allow $1 NetworkManager_runtime_t:file read_file_perms;
')
+########################################
+## <summary>
+## Read/Write networkmanager pid files.
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+#
+interface(`networkmanager_rw_pid_files',`
+ gen_require(`
+ type NetworkManager_runtime_t;
+ ')
+
+ files_search_pids($1)
+ rw_files_pattern($1, NetworkManager_runtime_t, NetworkManager_runtime_t)
+')
+
####################################
## <summary>
## Connect to networkmanager over
--
2.23.0
From: Laurent Bigonville <[email protected]>
When alsactl is started as a daemon, it creates a pidfile
(/run/alsactl.pid), that needs to be allowed
----
time->Sun Oct 6 10:59:09 2019
type=AVC msg=audit(1570352349.743:45): avc: denied { write open } for pid=804 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
type=AVC msg=audit(1570352349.743:45): avc: denied { create } for pid=804 comm="alsactl" name="alsactl.pid" scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
----
time->Sun Oct 6 11:54:38 2019
type=AVC msg=audit(1570355678.226:657): avc: denied { open } for pid=9186 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
type=AVC msg=audit(1570355678.226:657): avc: denied { read } for pid=9186 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
----
time->Sun Oct 6 11:54:38 2019
type=AVC msg=audit(1570355678.230:659): avc: denied { unlink } for pid=804 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/admin/alsa.fc | 1 +
policy/modules/admin/alsa.te | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/policy/modules/admin/alsa.fc b/policy/modules/admin/alsa.fc
index 75ea9ebf..16ccb7ca 100644
--- a/policy/modules/admin/alsa.fc
+++ b/policy/modules/admin/alsa.fc
@@ -4,6 +4,7 @@ HOME_DIR/\.asoundrc -- gen_context(system_u:object_r:alsa_home_t,s0)
/etc/asound\.conf -- gen_context(system_u:object_r:alsa_etc_t,s0)
/run/alsa(/.*)? gen_context(system_u:object_r:alsa_runtime_t,s0)
+/run/alsactl.pid -- gen_context(system_u:object_r:alsa_runtime_t,s0)
/usr/bin/ainit -- gen_context(system_u:object_r:alsa_exec_t,s0)
/usr/bin/alsactl -- gen_context(system_u:object_r:alsa_exec_t,s0)
diff --git a/policy/modules/admin/alsa.te b/policy/modules/admin/alsa.te
index e567dd32..9d053c4d 100644
--- a/policy/modules/admin/alsa.te
+++ b/policy/modules/admin/alsa.te
@@ -58,8 +58,9 @@ allow alsa_t alsa_etc_t:file map;
can_exec(alsa_t, alsa_exec_t)
allow alsa_t alsa_runtime_t:dir manage_dir_perms;
+allow alsa_t alsa_runtime_t:file manage_file_perms;
allow alsa_t alsa_runtime_t:lnk_file manage_lnk_file_perms;
-files_pid_filetrans(alsa_t, alsa_runtime_t, dir)
+files_pid_filetrans(alsa_t, alsa_runtime_t, { dir file })
manage_dirs_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
manage_files_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
--
2.23.0
From: Laurent Bigonville <[email protected]>
When alsactl is running as a daemon with systemd, it sets its process
priority to be nice to other processes. When stopping the service, it's
signaling to itself that it needs to exit.
----
time->Sun Oct 6 11:59:59 2019
type=AVC msg=audit(1570355999.755:43): avc: denied { setsched } for pid=794 comm="alsactl" scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:system_r:alsa_t:s0 tclass=process permissive=1
----
time->Sun Oct 6 11:59:59 2019
type=AVC msg=audit(1570355999.755:44): avc: denied { getsched } for pid=794 comm="alsactl" scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:system_r:alsa_t:s0 tclass=process permissive=1
----
time->Sun Oct 6 12:07:26 2019
type=AVC msg=audit(1570356446.747:292): avc: denied { signal } for pid=3585 comm="alsactl" scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:system_r:alsa_t:s0 tclass=process permissive=1
Signed-off-by: Laurent Bigonville <[email protected]>
---
policy/modules/admin/alsa.te | 1 +
1 file changed, 1 insertion(+)
diff --git a/policy/modules/admin/alsa.te b/policy/modules/admin/alsa.te
index 9d053c4d..a2287485 100644
--- a/policy/modules/admin/alsa.te
+++ b/policy/modules/admin/alsa.te
@@ -44,6 +44,7 @@ files_lock_file(alsa_var_lock_t)
allow alsa_t self:capability { dac_override dac_read_search ipc_owner setgid setuid };
# kill : kill pulseaudio
dontaudit alsa_t self:capability { kill sys_admin };
+allow alsa_t self:process { getsched setsched signal };
allow alsa_t self:sem create_sem_perms;
allow alsa_t self:shm create_shm_perms;
allow alsa_t self:unix_stream_socket { accept listen };
--
2.23.0
On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
> From: Laurent Bigonville <[email protected]>
>
> colord reads the color profiles files that are stored in
> ~/.local/share/icc/, The file descriptor to that file is passed over
> D-Bus so it needs to be inherited
This patch is cutting corners a little. It only takes unconfined_t into account and not the confined users (an alternative would be to call "userdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as well but closest you can get to "common users" without surgery.
Secondly xdg_read_data_files() is a little broad.
Also if this patch implies that whatever maintains XDG_DATA_DIR/icc is able to maintain generic xdg data files, which is arguable broad as well.
The second and third argument are subject to how far you want to take things, and so I won't object if that is not addressed.
The fd use issue, in my view, should be addressed for all login (common) users with colord access.
>
> ----
> time->Sat Oct 5 11:35:54 2019
> type=AVC msg=audit(1570268154.991:223): avc: denied { read } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> type=AVC msg=audit(1570268154.991:223): avc: denied { use } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=fd permissive=1
> ----
> time->Sat Oct 5 11:35:55 2019
> type=AVC msg=audit(1570268155.007:225): avc: denied { getattr } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> ----
> time->Sat Oct 5 11:35:55 2019
> type=AVC msg=audit(1570268155.007:226): avc: denied { map } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> ----
>
> Signed-off-by: Laurent Bigonville <[email protected]>
> ---
> policy/modules/services/colord.te | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/policy/modules/services/colord.te b/policy/modules/services/colord.te
> index fada3fb8..2fbb1835 100644
> --- a/policy/modules/services/colord.te
> +++ b/policy/modules/services/colord.te
> @@ -141,6 +141,13 @@ optional_policy(`
> udev_read_pid_files(colord_t)
> ')
>
> +# colord reads the color profiles files that are stored in ~/.local/share/icc/,
> +# The file descriptor to that file is passed over D-Bus so it needs to be inherited
> +optional_policy(`
> + unconfined_use_fds(colord_t)
> + xdg_read_data_files(colord_t)
> +')
> +
> optional_policy(`
> xserver_read_xdm_lib_files(colord_t)
> xserver_use_xdm_fds(colord_t)
> --
> 2.23.0
>
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
On Fri, Oct 11, 2019 at 02:54:23PM +0200, Dominick Grift wrote:
> On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
> > From: Laurent Bigonville <[email protected]>
> >
> > colord reads the color profiles files that are stored in
> > ~/.local/share/icc/, The file descriptor to that file is passed over
> > D-Bus so it needs to be inherited
>
> This patch is cutting corners a little. It only takes unconfined_t into account and not the confined users (an alternative would be to call "userdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as well but closest you can get to "common users" without surgery.
> Secondly xdg_read_data_files() is a little broad.
> Also if this patch implies that whatever maintains XDG_DATA_DIR/icc is able to maintain generic xdg data files, which is arguable broad as well.
>
> The second and third argument are subject to how far you want to take things, and so I won't object if that is not addressed.
> The fd use issue, in my view, should be addressed for all login (common) users with colord access.
Actually, I take this review back. I am not sure how to best deal with this fd.
>
> >
> > ----
> > time->Sat Oct 5 11:35:54 2019
> > type=AVC msg=audit(1570268154.991:223): avc: denied { read } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> > type=AVC msg=audit(1570268154.991:223): avc: denied { use } for pid=852 comm="gdbus" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=fd permissive=1
> > ----
> > time->Sat Oct 5 11:35:55 2019
> > type=AVC msg=audit(1570268155.007:225): avc: denied { getattr } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> > ----
> > time->Sat Oct 5 11:35:55 2019
> > type=AVC msg=audit(1570268155.007:226): avc: denied { map } for pid=852 comm="colord" path="/home/bigon/.local/share/icc/edid-fcd2cc06dec015794261e6b7756cbcec.icc" dev="dm-3" ino=413402 scontext=system_u:system_r:colord_t:s0 tcontext=unconfined_u:object_r:xdg_data_t:s0 tclass=file permissive=1
> > ----
> >
> > Signed-off-by: Laurent Bigonville <[email protected]>
> > ---
> > policy/modules/services/colord.te | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/policy/modules/services/colord.te b/policy/modules/services/colord.te
> > index fada3fb8..2fbb1835 100644
> > --- a/policy/modules/services/colord.te
> > +++ b/policy/modules/services/colord.te
> > @@ -141,6 +141,13 @@ optional_policy(`
> > udev_read_pid_files(colord_t)
> > ')
> >
> > +# colord reads the color profiles files that are stored in ~/.local/share/icc/,
> > +# The file descriptor to that file is passed over D-Bus so it needs to be inherited
> > +optional_policy(`
> > + unconfined_use_fds(colord_t)
> > + xdg_read_data_files(colord_t)
> > +')
> > +
> > optional_policy(`
> > xserver_read_xdm_lib_files(colord_t)
> > xserver_use_xdm_fds(colord_t)
> > --
> > 2.23.0
> >
>
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
On 10/12/19 3:53 AM, Dominick Grift wrote:
> On Fri, Oct 11, 2019 at 02:54:23PM +0200, Dominick Grift wrote:
>> On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
>>> From: Laurent Bigonville <[email protected]>
>>>
>>> colord reads the color profiles files that are stored in
>>> ~/.local/share/icc/, The file descriptor to that file is passed over
>>> D-Bus so it needs to be inherited
>>
>> This patch is cutting corners a little. It only takes unconfined_t into account and not the confined users (an alternative would be to call "userdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as well but closest you can get to "common users" without surgery.
>> Secondly xdg_read_data_files() is a little broad.
>> Also if this patch implies that whatever maintains XDG_DATA_DIR/icc is able to maintain generic xdg data files, which is arguable broad as well.
>>
>> The second and third argument are subject to how far you want to take things, and so I won't object if that is not addressed.
>> The fd use issue, in my view, should be addressed for all login (common) users with colord access.
>
> Actually, I take this review back. I am not sure how to best deal with this fd.
It seems that going to a colord_role() would be the way to go. There
already is a colord_dbus_chat($1_t) in userdomain.if, so you could put
those dbus rules plus the rules to address the fds together.
I agree the xdg_read_data_files() is somewhat broad, but it seems like
xdg_data_t files aren't sensitive. Maybe that's just how it is on
system? I don't feel strongly on this.
--
Chris PeBenito
On 10/11/19 8:24 AM, Laurent Bigonville wrote:
> From: Laurent Bigonville <[email protected]>
>
> When alsactl is started as a daemon, it creates a pidfile
> (/run/alsactl.pid), that needs to be allowed
>
> ----
> time->Sun Oct 6 10:59:09 2019
> type=AVC msg=audit(1570352349.743:45): avc: denied { write open } for pid=804 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
> type=AVC msg=audit(1570352349.743:45): avc: denied { create } for pid=804 comm="alsactl" name="alsactl.pid" scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
> ----
> time->Sun Oct 6 11:54:38 2019
> type=AVC msg=audit(1570355678.226:657): avc: denied { open } for pid=9186 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
> type=AVC msg=audit(1570355678.226:657): avc: denied { read } for pid=9186 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
> ----
> time->Sun Oct 6 11:54:38 2019
> type=AVC msg=audit(1570355678.230:659): avc: denied { unlink } for pid=804 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882 scontext=system_u:system_r:alsa_t:s0 tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>
> Signed-off-by: Laurent Bigonville <[email protected]>
> ---
> policy/modules/admin/alsa.fc | 1 +
> policy/modules/admin/alsa.te | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/policy/modules/admin/alsa.fc b/policy/modules/admin/alsa.fc
> index 75ea9ebf..16ccb7ca 100644
> --- a/policy/modules/admin/alsa.fc
> +++ b/policy/modules/admin/alsa.fc
> @@ -4,6 +4,7 @@ HOME_DIR/\.asoundrc -- gen_context(system_u:object_r:alsa_home_t,s0)
> /etc/asound\.conf -- gen_context(system_u:object_r:alsa_etc_t,s0)
>
> /run/alsa(/.*)? gen_context(system_u:object_r:alsa_runtime_t,s0)
> +/run/alsactl.pid -- gen_context(system_u:object_r:alsa_runtime_t,s0)
Needs escaping ( \.pid )
>
> /usr/bin/ainit -- gen_context(system_u:object_r:alsa_exec_t,s0)
> /usr/bin/alsactl -- gen_context(system_u:object_r:alsa_exec_t,s0)
> diff --git a/policy/modules/admin/alsa.te b/policy/modules/admin/alsa.te
> index e567dd32..9d053c4d 100644
> --- a/policy/modules/admin/alsa.te
> +++ b/policy/modules/admin/alsa.te
> @@ -58,8 +58,9 @@ allow alsa_t alsa_etc_t:file map;
> can_exec(alsa_t, alsa_exec_t)
>
> allow alsa_t alsa_runtime_t:dir manage_dir_perms;
> +allow alsa_t alsa_runtime_t:file manage_file_perms;
> allow alsa_t alsa_runtime_t:lnk_file manage_lnk_file_perms;
> -files_pid_filetrans(alsa_t, alsa_runtime_t, dir)
> +files_pid_filetrans(alsa_t, alsa_runtime_t, { dir file })
>
> manage_dirs_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
> manage_files_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
>
--
Chris PeBenito
On Sat, Oct 12, 2019 at 11:51:43AM -0400, Chris PeBenito wrote:
> On 10/12/19 3:53 AM, Dominick Grift wrote:
> > On Fri, Oct 11, 2019 at 02:54:23PM +0200, Dominick Grift wrote:
> > > On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
> > > > From: Laurent Bigonville <[email protected]>
> > > >
> > > > colord reads the color profiles files that are stored in
> > > > ~/.local/share/icc/, The file descriptor to that file is passed over
> > > > D-Bus so it needs to be inherited
> > >
> > > This patch is cutting corners a little. It only takes unconfined_t into account and not the confined users (an alternative would be to call "userdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as well but closest you can get to "common users" without surgery.
> > > Secondly xdg_read_data_files() is a little broad.
> > > Also if this patch implies that whatever maintains XDG_DATA_DIR/icc is able to maintain generic xdg data files, which is arguable broad as well.
> > >
> > > The second and third argument are subject to how far you want to take things, and so I won't object if that is not addressed.
> > > The fd use issue, in my view, should be addressed for all login (common) users with colord access.
> >
> > Actually, I take this review back. I am not sure how to best deal with this fd.
>
> It seems that going to a colord_role() would be the way to go. There
> already is a colord_dbus_chat($1_t) in userdomain.if, so you could put those
> dbus rules plus the rules to address the fds together.
>
> I agree the xdg_read_data_files() is somewhat broad, but it seems like
> xdg_data_t files aren't sensitive. Maybe that's just how it is on system?
> I don't feel strongly on this.
Yes it depends i guess. The thing is that like /usr theres really all kinds of things below ~/.local, like bin, lib, doc etc (pip for example installs to ~/.local/{bin,lib}).
So I would surely at least consider that beforehand
ls -aZ ~/.local/
wheel.id:wheel.role:users.generic_home_data.home_data_file:s0 . wheel.id:wheel.role:users.home_libraries.home_file:s0 lib
wheel.id:wheel.role:users.home_dir.file:s0 .. wheel.id:wheel.role:users.generic_home_data.home_data_file:s0 share
wheel.id:wheel.role:users.home_commands.home_file:s0 bin
There's also other gotchas, take for example your personal libvirt pool in ~/.local, this content may potentially also be need to be accessible by the qemu user.
I guess what i am saying is that not everything below /usr is always just "data"
I dont have enough experience with colord to give advice, looking at my policy there's also a colord --user instance, it seems also heavily integrated with gnome-settings-daemon.
I think this patch is probably alright as is for now (maybe its best to just ignore confined users in this stage) as for further partitioning ~/.local, i suppose we can alway's revisit these changes later as this only applies to ~/.local/share/icc anyway.
If this change is one of the few controversial changes that are needed to make gnome work on debian with unconfined, then i think it might be worth it to just accept this and make a note about it to address this properly when someone wants to work on the confined support for this aspect.
>
> --
> Chris PeBenito
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
Le 12/10/19 à 17:52, Chris PeBenito a écrit :
> On 10/11/19 8:24 AM, Laurent Bigonville wrote:
>> From: Laurent Bigonville <[email protected]>
>>
>> When alsactl is started as a daemon, it creates a pidfile
>> (/run/alsactl.pid), that needs to be allowed
>>
>> ----
>> time->Sun Oct 6 10:59:09 2019
>> type=AVC msg=audit(1570352349.743:45): avc: denied { write open }
>> for pid=804 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs"
>> ino=25882 scontext=system_u:system_r:alsa_t:s0
>> tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>> type=AVC msg=audit(1570352349.743:45): avc: denied { create } for
>> pid=804 comm="alsactl" name="alsactl.pid"
>> scontext=system_u:system_r:alsa_t:s0
>> tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>> ----
>> time->Sun Oct 6 11:54:38 2019
>> type=AVC msg=audit(1570355678.226:657): avc: denied { open } for
>> pid=9186 comm="alsactl" path="/run/alsactl.pid" dev="tmpfs" ino=25882
>> scontext=system_u:system_r:alsa_t:s0
>> tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>> type=AVC msg=audit(1570355678.226:657): avc: denied { read } for
>> pid=9186 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882
>> scontext=system_u:system_r:alsa_t:s0
>> tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>> ----
>> time->Sun Oct 6 11:54:38 2019
>> type=AVC msg=audit(1570355678.230:659): avc: denied { unlink } for
>> pid=804 comm="alsactl" name="alsactl.pid" dev="tmpfs" ino=25882
>> scontext=system_u:system_r:alsa_t:s0
>> tcontext=system_u:object_r:var_run_t:s0 tclass=file permissive=1
>>
>> Signed-off-by: Laurent Bigonville <[email protected]>
>> ---
>> policy/modules/admin/alsa.fc | 1 +
>> policy/modules/admin/alsa.te | 3 ++-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/policy/modules/admin/alsa.fc b/policy/modules/admin/alsa.fc
>> index 75ea9ebf..16ccb7ca 100644
>> --- a/policy/modules/admin/alsa.fc
>> +++ b/policy/modules/admin/alsa.fc
>> @@ -4,6 +4,7 @@ HOME_DIR/\.asoundrc --
>> gen_context(system_u:object_r:alsa_home_t,s0)
>> /etc/asound\.conf --
>> gen_context(system_u:object_r:alsa_etc_t,s0)
>> /run/alsa(/.*)? gen_context(system_u:object_r:alsa_runtime_t,s0)
>> +/run/alsactl.pid --
>> gen_context(system_u:object_r:alsa_runtime_t,s0)
>
> Needs escaping ( \.pid )
It's fixed in the github merge request
Do you think that you could already merge all the patches of the series
except the colord one?
>
>
>> /usr/bin/ainit -- gen_context(system_u:object_r:alsa_exec_t,s0)
>> /usr/bin/alsactl --
>> gen_context(system_u:object_r:alsa_exec_t,s0)
>> diff --git a/policy/modules/admin/alsa.te b/policy/modules/admin/alsa.te
>> index e567dd32..9d053c4d 100644
>> --- a/policy/modules/admin/alsa.te
>> +++ b/policy/modules/admin/alsa.te
>> @@ -58,8 +58,9 @@ allow alsa_t alsa_etc_t:file map;
>> can_exec(alsa_t, alsa_exec_t)
>> allow alsa_t alsa_runtime_t:dir manage_dir_perms;
>> +allow alsa_t alsa_runtime_t:file manage_file_perms;
>> allow alsa_t alsa_runtime_t:lnk_file manage_lnk_file_perms;
>> -files_pid_filetrans(alsa_t, alsa_runtime_t, dir)
>> +files_pid_filetrans(alsa_t, alsa_runtime_t, { dir file })
>> manage_dirs_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
>> manage_files_pattern(alsa_t, alsa_tmp_t, alsa_tmp_t)
>>
>
>
Le 12/10/19 ? 18:09, Dominick Grift a ?crit?:
> On Sat, Oct 12, 2019 at 11:51:43AM -0400, Chris PeBenito wrote:
>> On 10/12/19 3:53 AM, Dominick Grift wrote:
>>> On Fri, Oct 11, 2019 at 02:54:23PM +0200, Dominick Grift wrote:
>>>> On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
>>>>> From: Laurent Bigonville <[email protected]>
>>>>>
>>>>> colord reads the color profiles files that are stored in
>>>>> ~/.local/share/icc/, The file descriptor to that file is passed over
>>>>> D-Bus so it needs to be inherited
>>>> This patch is cutting corners a little. It only takes unconfined_t into account and not the confined users (an alternative would be to call "userdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as well but closest you can get to "common users" without surgery.
>>>> Secondly xdg_read_data_files() is a little broad.
>>>> Also if this patch implies that whatever maintains XDG_DATA_DIR/icc is able to maintain generic xdg data files, which is arguable broad as well.
>>>>
>>>> The second and third argument are subject to how far you want to take things, and so I won't object if that is not addressed.
>>>> The fd use issue, in my view, should be addressed for all login (common) users with colord access.
>>> Actually, I take this review back. I am not sure how to best deal with this fd.
>> It seems that going to a colord_role() would be the way to go. There
>> already is a colord_dbus_chat($1_t) in userdomain.if, so you could put those
>> dbus rules plus the rules to address the fds together.
>>
>> I agree the xdg_read_data_files() is somewhat broad, but it seems like
>> xdg_data_t files aren't sensitive. Maybe that's just how it is on system?
>> I don't feel strongly on this.
> Yes it depends i guess. The thing is that like /usr theres really all kinds of things below ~/.local, like bin, lib, doc etc (pip for example installs to ~/.local/{bin,lib}).
> So I would surely at least consider that beforehand
>
> ls -aZ ~/.local/
> wheel.id:wheel.role:users.generic_home_data.home_data_file:s0 . wheel.id:wheel.role:users.home_libraries.home_file:s0 lib
> wheel.id:wheel.role:users.home_dir.file:s0 .. wheel.id:wheel.role:users.generic_home_data.home_data_file:s0 share
> wheel.id:wheel.role:users.home_commands.home_file:s0 bin
>
> There's also other gotchas, take for example your personal libvirt pool in ~/.local, this content may potentially also be need to be accessible by the qemu user.
>
> I guess what i am saying is that not everything below /usr is always just "data"
>
> I dont have enough experience with colord to give advice, looking at my policy there's also a colord --user instance, it seems also heavily integrated with gnome-settings-daemon.
>
> I think this patch is probably alright as is for now (maybe its best to just ignore confined users in this stage) as for further partitioning ~/.local, i suppose we can alway's revisit these changes later as this only applies to ~/.local/share/icc anyway.
>
> If this change is one of the few controversial changes that are needed to make gnome work on debian with unconfined, then i think it might be worth it to just accept this and make a note about it to address this properly when someone wants to work on the confined support for this aspect.
This patch is not "the last patch to make GNOME work", there are still
other stuff to fix, so we might want to take some time to do that properly.