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:   Thu, 5 Dec 2019 08:46:28 +0100
From:   Dominick Grift <dac.override@gmail.com>
To:     "Sugar, David" <dsugar@tresys.com>,
        "selinux-refpolicy@vger.kernel.org" 
        <selinux-refpolicy@vger.kernel.org>
Subject: Re: [PATCH] Allow systemd to getattr configfile
Message-ID: <20191205074628.GA1734091@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>
 <20191204174343.GC1321684@brutus.lan>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="ZPt4rx8FFjLCG7dd"
Content-Disposition: inline
In-Reply-To: <20191204174343.GC1321684@brutus.lan>
User-Agent: Every email client sucks, this one just sucks less.
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk


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

On Wed, Dec 04, 2019 at 06:43:43PM +0100, Dominick Grift wrote:
> 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 e=
xists to control starting a service.  But this requires getattr permissions=
 on the file.  This is generally for configuration files.  We are mostly se=
eing 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" in=
o=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 a=
dded 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 want=
s=20
> > to grant access to a particular type to getattr?
> >=20
> > So, for this case, I would need to modify chronyd.te and lvm.te to use=
=20
> > this interface?
> >=20
> > I think you are also suggesting that ConditionPathExists usage in a uni=
t=20
> > file could be trying to check for the existence of something other than=
=20
> > a configuration file.
> >=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?
> >=20
>=20
> $ man systemd.directives | grep -i conditionpath
>        ConditionPathExists=3D
>        ConditionPathExistsGlob=3D
>        ConditionPathIsDirectory=3D
>        ConditionPathIsMountPoint=3D
>        ConditionPathIsReadWrite=3D
>        ConditionPathIsSymbolicLink=3D

Don't get me wrong though. In DSSP2 standard I allow systemd to read all co=
nfig, which is also a less-than-optimal compromise.
I basically do this because systemd often needs to be able to read environm=
ent config files in /etc/sysconfig (EnvironmentFile=3D)
Ideally I would also narrow this down and maybe one day i will (or maybe no=
t)

The point i am trying to make wrt to this patch though, is that ConditionPa=
th* is not limited to config files

>=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/=
files.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/i=
nit.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/lv=
m.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
> --=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

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

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

iQGzBAEBCAAdFiEEujmXliIBLFTc2Y4AJXSOVTf5R2kFAl3otdAACgkQJXSOVTf5
R2nq+Av/fMd8GwC1S9nDVrHNGhaUA600svNe0jageSMRsnv9zhAorvg7bNN/j7F4
ENI0oLag1J4PbmuHjA6ErTsvDTo04/N628TbmezqATBNsg+UhjdCLmhYyUwvc5J2
Whheu04SjYKUpTEVo6Sswgb8fGm6/lm36DjW8GeqaJO1wbIRcU/5BGNLKeSaj9U9
gdW/InlUnC6khTjWRiBFkRi390Hnzq9hvN93ouyMjjHoFaI9PzZ7Wczg1BOm0dYg
lINhYkeGpMYup8PIz8LuoaAYXoKYXbVoYgUWcRPuVgQSzYsESeQwWpcj5HY4gJZw
2vxKMi8g3wGKHzUOhYk1u3jOvep1t0i5Tu5kFIfWDfk1Sklz5H4diM0Tn093DCaS
/yB2fEdWl3EqHbLd5PW1B6QCEOJowYvrKrc+J3HOEpnrIO2bFtr0KqdaLPxELr2J
sqty2Lr6JGW6YM08rX3FXkNGlYVnm6IsdcsVVqIkclUaCqdlzz0V/R3+69V3JhCJ
PYUD4/0H
=TdDQ
-----END PGP SIGNATURE-----

--ZPt4rx8FFjLCG7dd--