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:   Wed, 4 Dec 2019 18:31:34 +0100
From:   Dominick Grift <dac.override@gmail.com>
To:     "Sugar, David" <dsugar@tresys.com>
Cc:     "selinux-refpolicy@vger.kernel.org" 
        <selinux-refpolicy@vger.kernel.org>
Subject: Re: [PATCH] Allow systemd to getattr configfile
Message-ID: <20191204173134.GB1321684@brutus.lan>
Mail-Followup-To: "Sugar, David" <dsugar@tresys.com>,
        "selinux-refpolicy@vger.kernel.org" <selinux-refpolicy@vger.kernel.org>
References: <20191204163306.16545-1-dsugar@tresys.com>
 <20191204163306.16545-2-dsugar@tresys.com>
 <20191204165614.GA1321684@brutus.lan>
 <e3d4c752-899f-5454-37f5-851378426e8a@tresys.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="7iMSBzlTiPOCCT2k"
Content-Disposition: inline
In-Reply-To: <e3d4c752-899f-5454-37f5-851378426e8a@tresys.com>
User-Agent: Every email client sucks, this one just sucks less.
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk


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

On Wed, Dec 04, 2019 at 05:22:55PM +0000, Sugar, David wrote:
>=20
>=20
> On 12/4/19 11:56 AM, Dominick Grift wrote:
> > On Wed, Dec 04, 2019 at 04:33:20PM +0000, Sugar, David wrote:
> >> Systemd has ConditionalPathExists which is used to check if a path exi=
sts to control starting a service.  But this requires getattr permissions o=
n the file.  This is generally for configuration files.  We are mostly seei=
ng this is in our own policy.  But this lvm denial also fits the example.
> >>
> >> type=3DAVC msg=3Daudit(1575427946.229:1624): avc:  denied  { getattr }=
 for  pid=3D1 comm=3D"systemd" path=3D"/etc/lvm/lvm.conf" dev=3D"dm-0" ino=
=3D51799 scontext=3Dsystem_u:system_r:init_t:s0 tcontext=3Dsystem_u:object_=
r:lvm_etc_t:s0 tclass=3Dfile permissive=3D0
> >>
> >> This second example is from chronyd, but it is happening becuase I add=
ed the conditional in a drop-in file. Note that chronyd_conf_t is already a=
 'configfile'.
> >>
> >> type=3DAVC msg=3Daudit(1575427959.882:1901): avc:  denied  { getattr }=
 for  pid=3D1 comm=3D"systemd" path=3D"/etc/chrony.conf" dev=3D"dm-0" ino=
=3D53824 scontext=3Dsystem_u:system_r:init_t:s0 tcontext=3Dsystem_u:object_=
r:chronyd_conf_t:s0 tclass=3Dfile permissive=3D1
> >=20
> > how about something a little more general?
> >=20
> > systemd_ConditionPath(`,'
> > 	allow init_t $1:dir search_dir_perms;
> > 	allow init_t $1:lnk_file read_lnk_file_perms;
> > 	allow init_t $1:fifo_file getattr_fifo_file_perms;
> > 	allow init_t $1:sock_file getattr_sock_file_perms;
> > 	allow init_t $1:file getattr_file_perms;
> > 	allow init_t $1:blk_file getattr_blk_file_perms;
> > 	allow init_t $1:chr_file getattr_chr_file_perms;
> > ')
> >=20
> I think you are suggesting an interface 'systemd_conditionpath' that=20
> would exist in init.if and then need to be used by any module that wants=
=20
> to grant access to a particular type to getattr?

Yes

>=20
> So, for this case, I would need to modify chronyd.te and lvm.te to use=20
> this interface?

Yes

>=20
> I think you are also suggesting that ConditionPathExists usage in a unit=
=20
> file could be trying to check for the existence of something other than=
=20
> a configuration file.

Yes, and on top of that there are other "conditions" but generally it boils=
 down to systemd "statting" the target

>=20
> Taking it to the extreme, a unit file could be checking for the=20
> existence of a file that is in a different SELinux domain.  Does it=20
> instead make sense to use the 'files_getattr_all_files',=20
> 'files_getattr_all_sockets', 'files_getattr_all_pipes', etc... in init.te?

I would argue that this would be too broad/generic, not to mention that it =
could also apply to a device node (basicallu anything)

>=20
>=20
> >>
> >> Signed-off-by: Dave Sugar <dsugar@tresys.com>
> >> ---
> >>   policy/modules/kernel/files.if | 20 ++++++++++++++++++++
> >>   policy/modules/system/init.te  |  1 +
> >>   policy/modules/system/lvm.te   |  2 +-
> >>   3 files changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/policy/modules/kernel/files.if b/policy/modules/kernel/fi=
les.if
> >> index f1c94411..87be07ae 100644
> >> --- a/policy/modules/kernel/files.if
> >> +++ b/policy/modules/kernel/files.if
> >> @@ -1562,6 +1562,26 @@ interface(`files_relabel_config_dirs',`
> >>   	relabel_dirs_pattern($1, configfile, configfile)
> >>   ')
> >>  =20
> >> +########################################
> >> +## <summary>
> >> +##	Getattr config files in /etc.
> >> +## </summary>
> >> +## <param name=3D"domain">
> >> +##	<summary>
> >> +##	Domain allowed access.
> >> +##	</summary>
> >> +## </param>
> >> +#
> >> +interface(`files_getattr_config_files',`
> >> +	gen_require(`
> >> +		attribute configfile;
> >> +	')
> >> +
> >> +	allow $1 configfile:dir list_dir_perms;
> >> +	getattr_files_pattern($1, configfile, configfile)
> >> +	read_lnk_files_pattern($1, configfile, configfile)
> >> +')
> >> +
> >>   ########################################
> >>   ## <summary>
> >>   ##	Read config files in /etc.
> >> diff --git a/policy/modules/system/init.te b/policy/modules/system/ini=
t.te
> >> index 8973a622..747b696e 100644
> >> --- a/policy/modules/system/init.te
> >> +++ b/policy/modules/system/init.te
> >> @@ -320,6 +320,7 @@ ifdef(`init_systemd',`
> >>   	domain_subj_id_change_exemption(init_t)
> >>   	domain_role_change_exemption(init_t)
> >>  =20
> >> +	files_getattr_config_files(init_t)
> >>   	files_read_all_pids(init_t)
> >>   	files_list_usr(init_t)
> >>   	files_list_var(init_t)
> >> diff --git a/policy/modules/system/lvm.te b/policy/modules/system/lvm.=
te
> >> index ad4eb579..c05344e0 100644
> >> --- a/policy/modules/system/lvm.te
> >> +++ b/policy/modules/system/lvm.te
> >> @@ -25,7 +25,7 @@ domain_obj_id_change_exemption(lvm_t)
> >>   role system_r types lvm_t;
> >>  =20
> >>   type lvm_etc_t;
> >> -files_type(lvm_etc_t)
> >> +files_config_file(lvm_etc_t)
> >>  =20
> >>   type lvm_lock_t;
> >>   files_lock_file(lvm_lock_t)
> >> --=20
> >> 2.21.0
> >>
> >=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

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

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

iQGzBAEBCAAdFiEEujmXliIBLFTc2Y4AJXSOVTf5R2kFAl3n7XIACgkQJXSOVTf5
R2lNjAwAojbEYGkenm50P96z+mBwm17eIJuR0Qfi5TiKqkga9yXmi1//jmOQ/DQp
GUXrwAhzeW/sEFzleVEklmhY5lPAPwH0H/ispwh6rIklsoSc9VHVdOYD7WzzaG3M
7BX/uCQNQZK5f6dogtIevRgcAkfZsCTFD+8A2P0//sfQedeR6K8guzXFc1+n3CvB
jjyzU/VK98tmACeP06XC1wsNwv61q5oYr77EM0pVydXpu1qbZjtR/NJfElQAk3fr
/4nkBbK3x+hJvsmK0YaFSigh5XHxbwOCa/Q2JEXxanrjzN/rTpdAkKTpHQGVXVZL
tJPLVQHGeGmbEHfX1hE9fyWIKfPhTn8LW9yT5jUyANhK7vzd8NE7Dzje9PX3PF2d
qk7gZz8pCR0RYRAn5L5+hoqEyF0XVm2iv720Eb3MKuseH/pY29jcqPmcuBL3RZoO
53chD6Jj2X2icrzW3CsTD8Xop9Tp4I4hxGpQ6zNF0GvdJxziJXK9acn75raHixCJ
KnbPNJ8v
=VH3v
-----END PGP SIGNATURE-----

--7iMSBzlTiPOCCT2k--