Re: [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations

From: Dominick Grift <dominick.grift@defensec.nl>
To: "Christian Göttsche" <cgzones@googlemail.com>
Cc: selinux@vger.kernel.org
Subject: Re: [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations
Date: Thu, 05 Aug 2021 17:08:09 +0200	[thread overview]
Message-ID: <87zgtw53qe.fsf@defensec.nl> (raw)
In-Reply-To: <20210805142445.61725-1-cgzones@googlemail.com> ("Christian =?utf-8?Q?G=C3=B6ttsche=22's?= message of "Thu, 5 Aug 2021 16:24:38 +0200")

Christian Göttsche <cgzones@googlemail.com> writes:

> The checks (permission verbs) in question are enable for the operations
> enable, reenable, link and unmask and disable for the operations disable
> and mask; those SELinux permissions exist in the the reference and fedora
> SELinux policy.
> These checks were dropped with v225 (see [1]) due to incomplete and
> missing infrastructure in the unit handling code.
>
> In addition the operations preset and revert are checked with the (also
> already existing) SELinux permission reload.
> (In the future I'd like to separate them into a new permission modify?
> together with calls to the standard D-Bus interfaces at
> org.freedesktop.DBus.Properties.Set.)

Please consider that any policy leveraging these permissions would
potentially have to deal with compatibility. We don't want to be forced
into a situation similiar to that situation we were led in when systemd
permissions were associated with the system Linux object class.

Also it distracts from the main topic which is to re-do properly
what was reverted earlier.

If at all possible then please address any "additions" such as preset
and revert elsewhere.

Thanks for picking this up again.

>
> Job actions JOB_RELOAD_OR_START and JOB_VERIFY_ACTIVE are now checked with
> the permission start instead of reload.
>
> The D-Bus filter now falls back to an instance check in case no unit can
> be decoded (e.g. the job has finished or the unit does not exist).
>
> Reduced proposal of [2]/[3]
> Closes: [4]
>
> [1]: https://github.com/systemd/systemd/pull/1044
> [2]: https://github.com/systemd/systemd/pull/10023
> [3]: https://lore.kernel.org/selinux/20191218142808.30433-1-cgzones@googlemail.com/
> [4]: https://github.com/systemd/systemd/issues/1050
>
> Christian Göttsche (7):
>   selinux: add function name to audit data
>   selinux: improve debug log format
>   selinux: mark _mac_selinux_generic_access_check with leading
>     underscore
>   core: add support for MAC checks on unit install operations
>   core: implement the sd-bus generic callback for SELinux
>   core: avoid bypasses in D-BUS SELinux filter
>   core: tweak job_type_to_access_method SELinux permissions
>
>  src/core/dbus-callbackdata.h             |  15 +++
>  src/core/dbus-manager.c                  |  70 +++++++---
>  src/core/dbus.c                          |  44 +++----
>  src/core/job.c                           |  14 +-
>  src/core/manager.c                       |   9 +-
>  src/core/manager.h                       |   1 +
>  src/core/selinux-access.c                |  75 +++++++++--
>  src/core/selinux-access.h                |  17 ++-
>  src/shared/install.c                     | 160 ++++++++++++++++++++---
>  src/shared/install.h                     |  44 +++++--
>  src/systemctl/systemctl-add-dependency.c |   2 +-
>  src/systemctl/systemctl-enable.c         |  16 +--
>  src/systemctl/systemctl-is-enabled.c     |   2 +-
>  src/systemctl/systemctl-preset-all.c     |   2 +-
>  src/test/test-install-root.c             |  88 ++++++-------
>  src/test/test-install.c                  |  38 +++---
>  16 files changed, 437 insertions(+), 160 deletions(-)
>  create mode 100644 src/core/dbus-callbackdata.h
>
> --
> 2.32.0
>

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift