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, 28 Dec 2019 16:59:21 +0100
From:   Dominick Grift <dac.override@gmail.com>
To:     Jason Zaman <jason@perfinion.com>
Cc:     Chris PeBenito <pebenito@ieee.org>,
        selinux-refpolicy@vger.kernel.org
Subject: Re: [PATCH 1/9] systemd: Add elogind support
Message-ID: <20191228155921.GA2360617@brutus.lan>
Mail-Followup-To: Jason Zaman <jason@perfinion.com>,
        Chris PeBenito <pebenito@ieee.org>,
        selinux-refpolicy@vger.kernel.org
References: <20191224101043.58122-1-jason@perfinion.com>
 <c5960eb5-acb6-19b5-2690-f9956dd174a0@ieee.org>
 <20191228043504.GA38088@baraddur.perfinion.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="azLHFNyN32YCQGCU"
Content-Disposition: inline
In-Reply-To: <20191228043504.GA38088@baraddur.perfinion.com>
User-Agent: Every email client sucks, this one just sucks less.
Sender: selinux-refpolicy-owner@vger.kernel.org
Precedence: bulk


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

On Sat, Dec 28, 2019 at 12:35:04PM +0800, Jason Zaman wrote:
> On Thu, Dec 26, 2019 at 12:03:32PM -0500, Chris PeBenito wrote:
> > On 12/24/19 5:10 AM, Jason Zaman wrote:
> > > Elogind is based off systemd-logind extracted to stand alone.
> >=20
> > I'm not a fan of this.  Systemd is already a big mess of permissions by=
=20
> > itself, and I'm relctant to add even more to it to support something el=
se.
>=20
> I'm not super happy about it either. I tried to make elogind_t
> standalone originally. it didnt end up working that well cuz it really
> *is* systemd-logind, just without systemd as pid1. The problem is all
> the paths are the same, everything in /run and /var and all that gets
> used exactly the same, so the fcontexts would conflict. A lot of the
> perms I ended up adding seem like things that systemd-logind should be
> able to do anyway too (like purging tmp to clean up /run/user when
> people logout, or sending audit logs) or do these things end up done by
> pid1 instead if its systemd?
>=20
> It's a similar issue to how tmpfiles works on gentoo. We made a policy
> for opentmpfiles (originally in openrc) then later the systemd policy in
> upstream refpol added systemd-tmpfiles. I've had to ifndef init_systemd
> around those fcontexts and it kind of works but its pretty awkward and
> makes switching between openrc/systemd more annoying than it should be.
>=20
> I'd be up for modularizing systemd.te if it'd make things easier but I'm
> not completely sure how. I see a few different parts that need to be
> handled carefully: 1) the paths on disk, these should ideally be the
> same for all the implementations of things. 2) the daemons themselves,
> these could be the same or different domains makes little difference. 3)
> how other programs interact with the daemons. I'm not really sure
> duplicating perms in every other policy is the right way to go? like
> everything would have to call both systemd_logind_foo() and
> elogind_foo()?
>=20
> If you have better ideas how to approach this, I'm all ears :)

I guess there are two options here. Either make your elogind module depend =
on whatever module has the types declared that need to be used by both logi=
nd and elogind (less optimal but less intrusive), or strip the "shared" typ=
es from the module that currently has it declared and declare it in a separ=
ate "shared" module so that both logind and elogind can tap into that (woul=
d require some refactoring but should be doable and be more optimal i suspe=
ct i suspect).

The same would apply to tmpfiles i gather.

>=20
> -- Jason
>=20
>=20
> >=20
> >=20
> > > Signed-off-by: Jason Zaman <jason@perfinion.com>
> > > ---
> > >   policy/modules/admin/sudo.if       |  2 ++
> > >   policy/modules/system/authlogin.if |  5 +++++
> > >   policy/modules/system/systemd.fc   |  5 +++++
> > >   policy/modules/system/systemd.te   | 27 ++++++++++++++++++++++++++-
> > >   4 files changed, 38 insertions(+), 1 deletion(-)
> > >=20
> > > diff --git a/policy/modules/admin/sudo.if b/policy/modules/admin/sudo=
=2Eif
> > > index c1459364..4f08af28 100644
> > > --- a/policy/modules/admin/sudo.if
> > > +++ b/policy/modules/admin/sudo.if
> > > @@ -159,6 +159,8 @@ template(`sudo_role_template',`
> > >  =20
> > >   	optional_policy(`
> > >   		dbus_system_bus_client($1_sudo_t)
> > > +		systemd_dbus_chat_logind($1_sudo_t)
> > > +		systemd_write_inherited_logind_sessions_pipes($1_sudo_t)
> > >  =20
> > >   		ifdef(`init_systemd',`
> > >   			init_dbus_chat($1_sudo_t)
> > > diff --git a/policy/modules/system/authlogin.if b/policy/modules/syst=
em/authlogin.if
> > > index c16748f2..83837458 100644
> > > --- a/policy/modules/system/authlogin.if
> > > +++ b/policy/modules/system/authlogin.if
> > > @@ -71,6 +71,11 @@ interface(`auth_use_pam',`
> > >   		optional_policy(`
> > >   			fprintd_dbus_chat($1)
> > >   		')
> > > +
> > > +		optional_policy(`
> > > +			systemd_dbus_chat_logind($1)
> > > +			systemd_write_inherited_logind_sessions_pipes($1)
> > > +		')
> > >   	')
> > >  =20
> > >   	optional_policy(`
> > > diff --git a/policy/modules/system/systemd.fc b/policy/modules/system=
/systemd.fc
> > > index 607b1d88..e6831465 100644
> > > --- a/policy/modules/system/systemd.fc
> > > +++ b/policy/modules/system/systemd.fc
> > > @@ -16,6 +16,10 @@
> > >   /usr/bin/systemd-tty-ask-password-agent	--	gen_context(system_u:obj=
ect_r:systemd_passwd_agent_exec_t,s0)
> > >   /usr/bin/systemd-notify			--	gen_context(system_u:object_r:systemd_=
notify_exec_t,s0)
> > >  =20
> > > +/usr/lib/elogind/elogind		--	gen_context(system_u:object_r:systemd_l=
ogind_exec_t,s0)
> > > +/usr/lib/elogind/elogind-cgroups-agent	--	gen_context(system_u:objec=
t_r:systemd_logind_exec_t,s0)
> > > +/usr/lib/elogind/elogind-uaccess-command	--	gen_context(system_u:obj=
ect_r:systemd_logind_exec_t,s0)
> > > +
> > >   # Systemd generators
> > >   /usr/lib/systemd/system-generators/systemd-gpt-auto-generator	    -=
-	    gen_context(system_u:object_r:systemd_gpt_generator_exec_t,s0)
> > >  =20
> > > @@ -56,6 +60,7 @@
> > >   /var/lib/systemd/rfkill(/.*)?	gen_context(system_u:object_r:systemd=
_rfkill_var_lib_t,s0)
> > >  =20
> > >   /run/\.nologin[^/]*	--	gen_context(system_u:object_r:systemd_sessio=
ns_runtime_t,s0)
> > > +/run/elogind\.pid	--	gen_context(system_u:object_r:systemd_logind_ru=
ntime_t,s0)
> > >   /run/nologin	--	gen_context(system_u:object_r:systemd_sessions_runt=
ime_t,s0)
> > >  =20
> > >   /run/systemd/ask-password(/.*)?	gen_context(system_u:object_r:syste=
md_passwd_runtime_t,s0)
> > > diff --git a/policy/modules/system/systemd.te b/policy/modules/system=
/systemd.te
> > > index 1422d8e2..f13b7252 100644
> > > --- a/policy/modules/system/systemd.te
> > > +++ b/policy/modules/system/systemd.te
> > > @@ -99,6 +99,7 @@ init_system_domain(systemd_locale_t, systemd_locale=
_exec_t)
> > >  =20
> > >   type systemd_logind_t;
> > >   type systemd_logind_exec_t;
> > > +dbus_system_domain(systemd_logind_t, systemd_logind_exec_t)
> > >   init_daemon_domain(systemd_logind_t, systemd_logind_exec_t)
> > >   init_named_socket_activation(systemd_logind_t, systemd_logind_runti=
me_t)
> > >  =20
> > > @@ -108,6 +109,7 @@ files_pid_file(systemd_logind_inhibit_runtime_t)
> > >   type systemd_logind_runtime_t alias systemd_logind_var_run_t;
> > >   files_pid_file(systemd_logind_runtime_t)
> > >   init_daemon_pid_file(systemd_logind_runtime_t, dir, "systemd_logind=
")
> > > +init_daemon_pid_file(systemd_logind_runtime_t, file, "elogind")
> > >  =20
> > >   type systemd_logind_var_lib_t;
> > >   files_type(systemd_logind_var_lib_t)
> > > @@ -427,7 +429,7 @@ logging_send_syslog_msg(systemd_log_parse_env_typ=
e)
> > >   # Logind local policy
> > >   #
> > >  =20
> > > -allow systemd_logind_t self:capability { chown dac_override dac_read=
_search fowner sys_admin sys_tty_config };
> > > +allow systemd_logind_t self:capability { chown dac_override dac_read=
_search fowner fsetid sys_admin sys_resource sys_tty_config };
> > >   allow systemd_logind_t self:process { getcap setfscreate };
> > >   allow systemd_logind_t self:netlink_kobject_uevent_socket create_so=
cket_perms;
> > >   allow systemd_logind_t self:unix_dgram_socket create_socket_perms;
> > > @@ -439,6 +441,9 @@ init_var_lib_filetrans(systemd_logind_t, systemd_=
logind_var_lib_t, dir)
> > >   manage_fifo_files_pattern(systemd_logind_t, systemd_logind_runtime_=
t, systemd_logind_runtime_t)
> > >   manage_files_pattern(systemd_logind_t, systemd_logind_runtime_t, sy=
stemd_logind_runtime_t)
> > >   allow systemd_logind_t systemd_logind_runtime_t:dir manage_dir_perm=
s;
> > > +files_pid_filetrans(systemd_logind_t, systemd_logind_runtime_t, file)
> > > +
> > > +create_dirs_pattern(systemd_logind_t, systemd_machined_runtime_t, sy=
stemd_machined_runtime_t)
> > >  =20
> > >   manage_dirs_pattern(systemd_logind_t, systemd_logind_inhibit_runtim=
e_t, systemd_logind_inhibit_runtime_t)
> > >   manage_files_pattern(systemd_logind_t, systemd_logind_inhibit_runti=
me_t, systemd_logind_inhibit_runtime_t)
> > > @@ -451,6 +456,8 @@ allow systemd_logind_t systemd_sessions_runtime_t=
:fifo_file manage_fifo_file_per
> > >  =20
> > >   kernel_read_kernel_sysctls(systemd_logind_t)
> > >  =20
> > > +auth_write_login_records(systemd_logind_t)
> > > +
> > >   dev_getattr_dri_dev(systemd_logind_t)
> > >   dev_getattr_generic_usb_dev(systemd_logind_t)
> > >   dev_getattr_kvm_dev(systemd_logind_t)
> > > @@ -470,10 +477,13 @@ dev_setattr_video_dev(systemd_logind_t)
> > >  =20
> > >   domain_obj_id_change_exemption(systemd_logind_t)
> > >  =20
> > > +files_purge_tmp(systemd_logind_t)
> > >   files_read_etc_files(systemd_logind_t)
> > >   files_search_pids(systemd_logind_t)
> > >  =20
> > >   fs_getattr_cgroup(systemd_logind_t)
> > > +fs_manage_cgroup_dirs(systemd_logind_t)
> > > +fs_manage_cgroup_files(systemd_logind_t)
> > >   fs_getattr_tmpfs(systemd_logind_t)
> > >   fs_getattr_tmpfs_dirs(systemd_logind_t)
> > >   fs_list_tmpfs(systemd_logind_t)
> > > @@ -483,6 +493,8 @@ fs_read_efivarfs_files(systemd_logind_t)
> > >   fs_relabelfrom_tmpfs_dirs(systemd_logind_t)
> > >   fs_unmount_tmpfs(systemd_logind_t)
> > >  =20
> > > +logging_send_audit_msgs(systemd_logind_t)
> > > +
> > >   selinux_get_enforce_mode(systemd_logind_t)
> > >  =20
> > >   storage_getattr_removable_dev(systemd_logind_t)
> > > @@ -495,6 +507,7 @@ term_use_unallocated_ttys(systemd_logind_t)
> > >  =20
> > >   auth_manage_faillog(systemd_logind_t)
> > >  =20
> > > +init_create_runtime_dirs(systemd_logind_t)
> > >   init_dbus_send_script(systemd_logind_t)
> > >   init_get_all_units_status(systemd_logind_t)
> > >   init_get_system_status(systemd_logind_t)
> > > @@ -537,6 +550,14 @@ userdom_relabelto_user_runtime_dirs(systemd_logi=
nd_t)
> > >   userdom_setattr_user_ttys(systemd_logind_t)
> > >   userdom_use_user_ttys(systemd_logind_t)
> > >  =20
> > > +tunable_policy(`use_nfs_home_dirs',`
> > > +       fs_read_nfs_files(systemd_logind_t)
> > > +')
> > > +
> > > +tunable_policy(`use_samba_home_dirs',`
> > > +       fs_read_cifs_files(systemd_logind_t)
> > > +')
> > > +
> > >   # Needed to work around patch not yet merged into the systemd-login=
d supported on RHEL 7.x
> > >   # The change in systemd by Nicolas Iooss on 02-Feb-2016 with hash 4=
b51966cf6c06250036e428608da92f8640beb96
> > >   # should fix the problem where user directories in /run/user/$UID/ =
are not getting the proper context
> > > @@ -568,6 +589,10 @@ optional_policy(`
> > >   	policykit_dbus_chat(systemd_logind_t)
> > >   ')
> > >  =20
> > > +optional_policy(`
> > > +	shutdown_domtrans(systemd_logind_t)
> > > +')
> > > +
> > >   optional_policy(`
> > >   	xserver_read_state(systemd_logind_t)
> > >   	xserver_dbus_chat(systemd_logind_t)
> > >=20
> >=20
> >=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

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

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

iQGzBAEBCAAdFiEEujmXliIBLFTc2Y4AJXSOVTf5R2kFAl4He9UACgkQJXSOVTf5
R2mpPgwAqy+Rp5V2ppSAMF5V9zR11EGelwD3Euho6rpxumbKnK9at/Ps/Y73xaLQ
8H3R/vdxa3luU3wPFZ8WdVClhgr76KrYQLxq/volxKQSXzoNTmgrwW03oWy7KEr1
it34zxs1B8YcJpoWY14IVfAEZNeb2stXf5O6kIIMIX+iqH0rU+4T0VJP30Fxs1cj
4qliiMqYHkbmBsNrz6cVWyNgNd0mu+lkvJQOU1BS/Z5jTaJ+o2q3Aw2D/+wAoZwA
GKEr51ZAtkr2IKTXvhFo6jqCsxQW+8WOFn396QVaNerB2Ll9GYImCDTWn89MxH8E
zShGWC+k61vDhfqMJppFtJyJUlkEi0hWYc8kUyQKxDYKbRguECZ5CNGPqBR+Sp/k
Dp1hRlwxMmdNKGny0tLfgha8GL6Ym0PeduZbSpg54lY29GJcY4GRyWFiLtvoed8i
HCuisauDwpZN/gAlnQrJLJNIkfOsGtBkoqPJwGco2ArpWwn8ivSyczyNSx8fSc2h
3mVu0RFg
=wII4
-----END PGP SIGNATURE-----

--azLHFNyN32YCQGCU--