Received-SPF: pass (google.com: best guess record for domain of selinux-refpolicy-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 12 Oct 2019 09:53:20 +0200
From:   Dominick Grift <dac.override@gmail.com>
To:     Laurent Bigonville <bigon@debian.org>,
        selinux-refpolicy@vger.kernel.org
Subject: Re: [PATCH 05/10] Allow colord_t to read the color profile stored in
 ~/.local/share/icc/
Message-ID: <20191012075320.GA716332@brutus.lan>
Mail-Followup-To: Laurent Bigonville <bigon@debian.org>,
        selinux-refpolicy@vger.kernel.org
References: <20191011122416.14651-1-bigon@debian.org>
 <20191011122416.14651-5-bigon@debian.org>
 <20191011125423.GA279944@brutus.lan>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="zhXaljGHf11kAtnf"
Content-Disposition: inline
In-Reply-To: <20191011125423.GA279944@brutus.lan>
User-Agent: Every email client sucks, this one just sucks less.
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk


--zhXaljGHf11kAtnf
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 <bigon@bigon.be>
> >=20
> > 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
>=20
> This patch is cutting corners a little. It only takes unconfined_t into a=
ccount 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 ab=
le to maintain generic xdg data files, which is arguable broad as well.
>=20
> The second and third argument are subject to how far you want to take thi=
ngs, 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.

>=20
> >=20
> > ----
> > time->Sat Oct  5 11:35:54 2019
> > type=3DAVC msg=3Daudit(1570268154.991:223): avc:  denied  { read } for =
 pid=3D852 comm=3D"gdbus" path=3D"/home/bigon/.local/share/icc/edid-fcd2cc0=
6dec015794261e6b7756cbcec.icc" dev=3D"dm-3" ino=3D413402 scontext=3Dsystem_=
u:system_r:colord_t:s0 tcontext=3Dunconfined_u:object_r:xdg_data_t:s0 tclas=
s=3Dfile permissive=3D1
> > type=3DAVC msg=3Daudit(1570268154.991:223): avc:  denied  { use } for  =
pid=3D852 comm=3D"gdbus" path=3D"/home/bigon/.local/share/icc/edid-fcd2cc06=
dec015794261e6b7756cbcec.icc" dev=3D"dm-3" ino=3D413402 scontext=3Dsystem_u=
:system_r:colord_t:s0 tcontext=3Dunconfined_u:unconfined_r:unconfined_t:s0-=
s0:c0.c1023 tclass=3Dfd permissive=3D1
> > ----
> > time->Sat Oct  5 11:35:55 2019
> > type=3DAVC msg=3Daudit(1570268155.007:225): avc:  denied  { getattr } f=
or  pid=3D852 comm=3D"colord" path=3D"/home/bigon/.local/share/icc/edid-fcd=
2cc06dec015794261e6b7756cbcec.icc" dev=3D"dm-3" ino=3D413402 scontext=3Dsys=
tem_u:system_r:colord_t:s0 tcontext=3Dunconfined_u:object_r:xdg_data_t:s0 t=
class=3Dfile permissive=3D1
> > ----
> > time->Sat Oct  5 11:35:55 2019
> > type=3DAVC msg=3Daudit(1570268155.007:226): avc:  denied  { map } for  =
pid=3D852 comm=3D"colord" path=3D"/home/bigon/.local/share/icc/edid-fcd2cc0=
6dec015794261e6b7756cbcec.icc" dev=3D"dm-3" ino=3D413402 scontext=3Dsystem_=
u:system_r:colord_t:s0 tcontext=3Dunconfined_u:object_r:xdg_data_t:s0 tclas=
s=3Dfile permissive=3D1
> > ----
> >=20
> > Signed-off-by: Laurent Bigonville <bigon@bigon.be>
> > ---
> >  policy/modules/services/colord.te | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >=20
> > diff --git a/policy/modules/services/colord.te b/policy/modules/service=
s/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)
> >  ')
> > =20
> > +# colord reads the color profiles files that are stored in ~/.local/sh=
are/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)
> > --=20
> > 2.23.0
> >=20
>=20
> --=20
> Key fingerprint =3D 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=3Dget&search=3D0x3B6C5F1D2C7B6B02
> Dominick Grift


--=20
Key fingerprint =3D 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=3Dget&search=3D0x3B6C5F1D2C7B6B02
Dominick Grift

--zhXaljGHf11kAtnf
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQGzBAEBCAAdFiEEujmXliIBLFTc2Y4AJXSOVTf5R2kFAl2hhmsACgkQJXSOVTf5
R2lsCAv/UoBUcf5z94UGFXSxks+w6BxzTc0xns/50fI5xd9sXDMw0SeOWtWVoA2v
vhRMrgUy1Q/S/ix7jjkSEN/3LMxYERcVmB7493MqsYXJhLsALqd7tYC3E/PGdQz6
TOQsscsNa664OELcYGSiCn3Hjg691Cc5tbWK2sndi+607QCqyFrP//VsOlztOidB
oEDLHYna0rKV69LkfJQRmjfrW7VyhQ7GV1RUut4fB82qLqSpaYW9EROnDiTdjn2Q
CoW+bJAAehqhZFflSKZA2wv3EPeEA+9eMdMsoTvZZPc2xR5OOrLftgkHfW2RtmNU
zycq68N+dSP8YdQb8MVSKxJWh8LMtThZM5buLK4ADV0tHMmQIioXTd7Q5bR4j6hp
enrsfxC+PVO8D3jM++gf2ikaDCY+A+aPmCkQba63mIP6oHQ5/x6EIbWJUA4GKr09
xyjgh1guJGLyCdxWWrz2MKehkWFo1Mlzvf6bYN0tWbk+9560yXPJ/sQIYwGNCoLN
EqfQlpCH
=HViU
-----END PGP SIGNATURE-----

--zhXaljGHf11kAtnf--