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 18:09:34 +0200
From:   Dominick Grift <dac.override@gmail.com>
To:     Chris PeBenito <pebenito@ieee.org>
Cc:     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: <20191012160934.GA3589@brutus.lan>
Mail-Followup-To: Chris PeBenito <pebenito@ieee.org>,
        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>
 <20191012075320.GA716332@brutus.lan>
 <f0b189cf-81d6-c567-2224-45ce70bcc796@ieee.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="6c2NcOVqGQ03X4Wi"
Content-Disposition: inline
In-Reply-To: <f0b189cf-81d6-c567-2224-45ce70bcc796@ieee.org>
User-Agent: Every email client sucks, this one just sucks less.
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk


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

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 <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 in=
to account and not the confined users (an alternative would be to call "use=
rdom_use_all_users_fds(colord_t)" instead. Which is arguable too broad as w=
ell 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 i=
s able 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=
 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 (comm=
on) users with colord access.
> >=20
> > Actually, I take this review back. I am not sure how to best deal with =
this fd.
>=20
> 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 th=
ose
> dbus rules plus the rules to address the fds together.
>=20
> 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 install=
s 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 pol=
icy 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 jus=
t 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 m=
ake 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.
>=20
> --=20
> Chris PeBenito

--=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

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

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

iQGzBAEBCAAdFiEEujmXliIBLFTc2Y4AJXSOVTf5R2kFAl2h+rkACgkQJXSOVTf5
R2nPXwv+Kkt0dHBySx84Ani1pzcEs0KYF5dUEyms2+eLFgJ8eyYztGQjlI5kpTUi
NeGb8YGiGefyEFOJnrCRsESZlRpCH+8Lj3Fln4x2GMxA3Uab1uBXuzyzlrL7dVgf
3Lz8HM0OQ+EtmmDbnCOVapuKdOMNdDCjxfNUy0VLdI5NKfN8lk/M0VoMxnmKqCNH
8rLbgSVdFi4d3PPKP8+KZqUhxYiWATYnxL59zJ/rD3uAyIqmARqH9vPkRB/vZaVa
ucX/SfKHAZtn1BdIhNynz+GEVNUN6bdMNR9QiaWdnzXn5Fo5LtMy+Tzp0ptII9gN
us88OqZcjJkios8c2hVQJbzfkmel7NHJls536SKFGUWYDYHSpJ4VDiJWgFknTBZ+
9bOCaA0X+ia6ksc3e3aa5ctnRjB4RVvt13Z1x7qAQjjSveA48oReU8GeI4UACyuq
d3IYKC1FpAotH0mCfl7rhKYfKFXjHNUwfusiCR2LSPvwDi7Y0KXcD2TIGYU54lXs
aA4MkYqT
=4Imb
-----END PGP SIGNATURE-----

--6c2NcOVqGQ03X4Wi--