Re: [PATCH 05/10] Allow colord_t to read the color profile stored in ~/.local/share/icc/

From: Dominick Grift <dac.override@gmail.com>
To: Laurent Bigonville <bigon@debian.org>
Cc: selinux-refpolicy@vger.kernel.org
Subject: Re: [PATCH 05/10] Allow colord_t to read the color profile stored in ~/.local/share/icc/
Date: Fri, 11 Oct 2019 14:54:23 +0200	[thread overview]
Message-ID: <20191011125423.GA279944@brutus.lan> (raw)
In-Reply-To: <20191011122416.14651-5-bigon@debian.org>

[-- Attachment #1: Type: text/plain, Size: 3371 bytes --]

On Fri, Oct 11, 2019 at 02:24:11PM +0200, Laurent Bigonville wrote:
> From: Laurent Bigonville <bigon@bigon.be>
> 
> 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 <bigon@bigon.be>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]