All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations
@ 2021-08-05 14:24 Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 1/7] selinux: add function name to audit data Christian Göttsche
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

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.)

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 1/7] selinux: add function name to audit data
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 2/7] selinux: improve debug log format Christian Göttsche
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

Include the systemd C function name in the audit message to improve the
debug ability on denials.
Similar like kernel denial messages include the syscall name.
---
 src/core/selinux-access.c | 18 ++++++++++++------
 src/core/selinux-access.h | 10 +++++++---
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index d077d5dea7..e8e73a5951 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -31,6 +31,7 @@ struct audit_info {
         sd_bus_creds *creds;
         const char *path;
         const char *cmdline;
+        const char *function;
 };
 
 /*
@@ -58,10 +59,11 @@ static int audit_callback(
                 xsprintf(gid_buf, GID_FMT, gid);
 
         snprintf(msgbuf, msgbufsize,
-                 "auid=%s uid=%s gid=%s%s%s%s%s%s%s",
+                 "auid=%s uid=%s gid=%s%s%s%s%s%s%s%s%s%s",
                  login_uid_buf, uid_buf, gid_buf,
                  audit->path ? " path=\"" : "", strempty(audit->path), audit->path ? "\"" : "",
-                 audit->cmdline ? " cmdline=\"" : "", strempty(audit->cmdline), audit->cmdline ? "\"" : "");
+                 audit->cmdline ? " cmdline=\"" : "", strempty(audit->cmdline), audit->cmdline ? "\"" : "",
+                 audit->function ? " function=\"" : "", strempty(audit->function), audit->function ? "\"" : "");
 
         return 0;
 }
@@ -179,7 +181,8 @@ int mac_selinux_generic_access_check(
                 sd_bus_message *message,
                 const char *path,
                 const char *permission,
-                sd_bus_error *error) {
+                sd_bus_error *error,
+                const char *func) {
 
         _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
         const char *tclass, *scon;
@@ -192,6 +195,7 @@ int mac_selinux_generic_access_check(
         assert(message);
         assert(permission);
         assert(error);
+        assert(func);
 
         r = access_init(error);
         if (r <= 0)
@@ -263,6 +267,7 @@ int mac_selinux_generic_access_check(
                 .creds = creds,
                 .path = path,
                 .cmdline = cl,
+                .function = func,
         };
 
         r = selinux_check_access(scon, fcon, tclass, permission, &audit_info);
@@ -274,8 +279,8 @@ int mac_selinux_generic_access_check(
         }
 
         log_full_errno_zerook(LOG_DEBUG, r,
-                              "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s path=%s cmdline=%s: %m",
-                              scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", path, cl);
+                              "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s func=%s path=%s cmdline=%s: %m",
+                              scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", func, path, cl);
         return enforce ? r : 0;
 }
 
@@ -285,7 +290,8 @@ int mac_selinux_generic_access_check(
                 sd_bus_message *message,
                 const char *path,
                 const char *permission,
-                sd_bus_error *error) {
+                sd_bus_error *error,
+                const char *func) {
 
         return 0;
 }
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index c6bfb32544..8931e998d0 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -5,10 +5,14 @@
 
 #include "manager.h"
 
-int mac_selinux_generic_access_check(sd_bus_message *message, const char *path, const char *permission, sd_bus_error *error);
+int mac_selinux_generic_access_check(sd_bus_message *message,
+                                     const char *path,
+                                     const char *permission,
+                                     sd_bus_error *error,
+                                     const char *func);
 
 #define mac_selinux_access_check(message, permission, error) \
-        mac_selinux_generic_access_check((message), NULL, (permission), (error))
+        mac_selinux_generic_access_check((message), NULL, (permission), (error), __func__)
 
 #define mac_selinux_unit_access_check(unit, message, permission, error) \
-        mac_selinux_generic_access_check((message), unit_label_path(unit), (permission), (error))
+        mac_selinux_generic_access_check((message), unit_label_path(unit), (permission), (error), __func__)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 2/7] selinux: improve debug log format
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 1/7] selinux: add function name to audit data Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 3/7] selinux: mark _mac_selinux_generic_access_check with leading underscore Christian Göttsche
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

path might be NULL when checking against the system permissions, so wrap
with strna().

The command line might not be available over D-Bus and thus cl might be
empty.  To improve readability quote the corresponding value.
---
 src/core/selinux-access.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index e8e73a5951..e40fd937a6 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -279,8 +279,8 @@ int mac_selinux_generic_access_check(
         }
 
         log_full_errno_zerook(LOG_DEBUG, r,
-                              "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s func=%s path=%s cmdline=%s: %m",
-                              scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", func, path, cl);
+                              "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s func=%s path=%s cmdline='%s': %m",
+                              scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", func, strna(path), cl);
         return enforce ? r : 0;
 }
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 3/7] selinux: mark _mac_selinux_generic_access_check with leading underscore
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 1/7] selinux: add function name to audit data Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 2/7] selinux: improve debug log format Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 4/7] core: add support for MAC checks on unit install operations Christian Göttsche
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

`mac_selinux_generic_access_check()` should not be called directly, only
via the wrapper macros `mac_selinux_access_check` and
`mac_selinux_unit_access_check`.
---
 src/core/selinux-access.c |  4 ++--
 src/core/selinux-access.h | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index e40fd937a6..180fe22773 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -177,7 +177,7 @@ static int access_init(sd_bus_error *error) {
    If the machine is in permissive mode it will return ok.  Audit messages will
    still be generated if the access would be denied in enforcing mode.
 */
-int mac_selinux_generic_access_check(
+int _mac_selinux_generic_access_check(
                 sd_bus_message *message,
                 const char *path,
                 const char *permission,
@@ -286,7 +286,7 @@ int mac_selinux_generic_access_check(
 
 #else /* HAVE_SELINUX */
 
-int mac_selinux_generic_access_check(
+int _mac_selinux_generic_access_check(
                 sd_bus_message *message,
                 const char *path,
                 const char *permission,
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index 8931e998d0..8a5582997a 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -5,14 +5,14 @@
 
 #include "manager.h"
 
-int mac_selinux_generic_access_check(sd_bus_message *message,
-                                     const char *path,
-                                     const char *permission,
-                                     sd_bus_error *error,
-                                     const char *func);
+int _mac_selinux_generic_access_check(sd_bus_message *message,
+                                      const char *path,
+                                      const char *permission,
+                                      sd_bus_error *error,
+                                      const char *func);
 
 #define mac_selinux_access_check(message, permission, error) \
-        mac_selinux_generic_access_check((message), NULL, (permission), (error), __func__)
+        _mac_selinux_generic_access_check((message), NULL, (permission), (error), __func__)
 
 #define mac_selinux_unit_access_check(unit, message, permission, error) \
-        mac_selinux_generic_access_check((message), unit_label_path(unit), (permission), (error), __func__)
+        _mac_selinux_generic_access_check((message), unit_label_path(unit), (permission), (error), __func__)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 4/7] core: add support for MAC checks on unit install operations
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
                   ` (2 preceding siblings ...)
  2021-08-05 14:24 ` [PATCH SYSTEMD 3/7] selinux: mark _mac_selinux_generic_access_check with leading underscore Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 5/7] core: implement the sd-bus generic callback for SELinux Christian Göttsche
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

shared:

Add a MAC callback call to unit_file_* functions to check unit
operations, like enable or disable.  The MAC callback is supplied with
the unit name as first argument.  The second argument is a blob used by
the back-end MAC specific check.  If no check should be performed, pass
NULL pointers.

core:

Implement a generic MAC callback for sd-bus and register it for D-Bus
exposed methods.

systemctl:

Adopt to the new callback by passing NULL.  No callback check is needed
when called on the client side, cause regular kernel MAC checks (like
write or unlink) on the unit file will be performed.
---
 src/core/dbus-callbackdata.h             |   5 ++
 src/core/dbus-manager.c                  |  39 ++++++---
 src/core/manager.c                       |   2 +-
 src/shared/install.c                     | 100 +++++++++++++++++++----
 src/shared/install.h                     |  42 +++++++---
 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 ++++-----
 11 files changed, 225 insertions(+), 111 deletions(-)
 create mode 100644 src/core/dbus-callbackdata.h

diff --git a/src/core/dbus-callbackdata.h b/src/core/dbus-callbackdata.h
new file mode 100644
index 0000000000..7dedb680c0
--- /dev/null
+++ b/src/core/dbus-callbackdata.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+typedef struct MacUnitCallbackUserdata {
+} MacUnitCallbackUserdata;
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 1f2ac8152c..eec358cb6b 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -11,6 +11,7 @@
 #include "bus-common-errors.h"
 #include "bus-get-properties.h"
 #include "bus-log-control-api.h"
+#include "dbus-callbackdata.h"
 #include "data-fd-util.h"
 #include "dbus-cgroup.h"
 #include "dbus-execute.h"
@@ -2184,10 +2185,19 @@ fail:
         return r;
 }
 
+static int mac_unit_callback_check(const char *unit_name, void *userdata) {
+        MacUnitCallbackUserdata *ud = userdata;
+
+        assert(unit_name);
+        assert(ud);
+
+        return 0;
+}
+
 static int method_enable_unit_files_generic(
                 sd_bus_message *message,
                 Manager *m,
-                int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes),
+                int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata),
                 bool carries_install_info,
                 sd_bus_error *error) {
 
@@ -2196,6 +2206,7 @@ static int method_enable_unit_files_generic(
         size_t n_changes = 0;
         UnitFileFlags flags;
         int r;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2228,7 +2239,7 @@ static int method_enable_unit_files_generic(
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = call(m->unit_file_scope, flags, NULL, l, &changes, &n_changes);
+        r = call(m->unit_file_scope, flags, NULL, l, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2251,8 +2262,8 @@ static int method_link_unit_files(sd_bus_message *message, void *userdata, sd_bu
         return method_enable_unit_files_generic(message, userdata, unit_file_link, false, error);
 }
 
-static int unit_file_preset_without_mode(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char **files, UnitFileChange **changes, size_t *n_changes) {
-        return unit_file_preset(scope, flags, root_dir, files, UNIT_FILE_PRESET_FULL, changes, n_changes);
+static int unit_file_preset_without_mode(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char **files, UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata) {
+        return unit_file_preset(scope, flags, root_dir, files, UNIT_FILE_PRESET_FULL, changes, n_changes, mac_check, userdata);
 }
 
 static int method_preset_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2273,6 +2284,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
         int runtime, force, r;
         UnitFileFlags flags;
         const char *mode;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2301,7 +2313,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = unit_file_preset(m->unit_file_scope, flags, NULL, l, preset_mode, &changes, &n_changes);
+        r = unit_file_preset(m->unit_file_scope, flags, NULL, l, preset_mode, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2311,7 +2323,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
 static int method_disable_unit_files_generic(
                 sd_bus_message *message,
                 Manager *m,
-                int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes),
+                int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata),
                 sd_bus_error *error) {
 
         _cleanup_strv_free_ char **l = NULL;
@@ -2319,6 +2331,7 @@ static int method_disable_unit_files_generic(
         UnitFileFlags flags;
         size_t n_changes = 0;
         int r;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2352,7 +2365,7 @@ static int method_disable_unit_files_generic(
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = call(m->unit_file_scope, flags, NULL, l, &changes, &n_changes);
+        r = call(m->unit_file_scope, flags, NULL, l, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2377,6 +2390,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
         size_t n_changes = 0;
         Manager *m = userdata;
         int r;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2391,7 +2405,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = unit_file_revert(m->unit_file_scope, NULL, l, &changes, &n_changes);
+        r = unit_file_revert(m->unit_file_scope, NULL, l, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2437,6 +2451,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
         const char *mode;
         UnitFileFlags flags;
         int force, runtime, r;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2465,7 +2480,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = unit_file_preset_all(m->unit_file_scope, flags, NULL, preset_mode, &changes, &n_changes);
+        r = unit_file_preset_all(m->unit_file_scope, flags, NULL, preset_mode, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2481,6 +2496,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
         char *target, *type;
         UnitDependency dep;
         UnitFileFlags flags;
+        MacUnitCallbackUserdata mcud = {};
 
         assert(message);
         assert(m);
@@ -2505,7 +2521,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
         if (dep < 0)
                 return -EINVAL;
 
-        r = unit_file_add_dependency(m->unit_file_scope, flags, NULL, l, target, dep, &changes, &n_changes);
+        r = unit_file_add_dependency(m->unit_file_scope, flags, NULL, l, target, dep, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
@@ -2520,6 +2536,7 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
         const char *name;
         char **p;
         int runtime, r;
+        MacUnitCallbackUserdata mcud = {};
 
         r = sd_bus_message_read(message, "sb", &name, &runtime);
         if (r < 0)
@@ -2537,7 +2554,7 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
         flags = UNIT_FILE_DRY_RUN |
                 (runtime ? UNIT_FILE_RUNTIME : 0);
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, flags, NULL, p, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, flags, NULL, p, &changes, &n_changes, mac_unit_callback_check, &mcud);
         if (r < 0)
                 return log_error_errno(r, "Failed to get file links for %s: %m", name);
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 24dfe9fc06..952bc57036 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1691,7 +1691,7 @@ static void manager_preset_all(Manager *m) {
                 return;
 
         /* If this is the first boot, and we are in the host system, then preset everything */
-        r = unit_file_preset_all(UNIT_FILE_SYSTEM, 0, NULL, UNIT_FILE_PRESET_ENABLE_ONLY, NULL, 0);
+        r = unit_file_preset_all(UNIT_FILE_SYSTEM, 0, NULL, UNIT_FILE_PRESET_ENABLE_ONLY, NULL, 0, NULL, NULL);
         if (r < 0)
                 log_full_errno(r == -EEXIST ? LOG_NOTICE : LOG_WARNING, r,
                                "Failed to populate /etc with preset unit settings, ignoring: %m");
diff --git a/src/shared/install.c b/src/shared/install.c
index a348f0c572..f1e603fecf 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -2158,13 +2158,26 @@ static int install_context_mark_for_removal(
         return 0;
 }
 
+static int do_mac_check(
+                mac_callback_t mac_check,
+                const char *unit_name,
+                void *userdata) {
+
+        if (!mac_check)
+                return 0;
+
+        return mac_check(unit_name, userdata);
+}
+
 int unit_file_mask(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         const char *config_path;
@@ -2192,6 +2205,10 @@ int unit_file_mask(
                         continue;
                 }
 
+                q = do_mac_check(mac_check, *i, userdata);
+                if (q < 0)
+                        return q;
+
                 path = path_make_absolute(*i, config_path);
                 if (!path)
                         return -ENOMEM;
@@ -2210,7 +2227,9 @@ int unit_file_unmask(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
@@ -2240,6 +2259,10 @@ int unit_file_unmask(
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
+                r = do_mac_check(mac_check, *i, userdata);
+                if (r < 0)
+                        return r;
+
                 path = path_make_absolute(*i, config_path);
                 if (!path)
                         return -ENOMEM;
@@ -2304,7 +2327,9 @@ int unit_file_link(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_strv_free_ char **todo = NULL;
@@ -2336,6 +2361,10 @@ int unit_file_link(
                 if (!unit_name_is_valid(fn, UNIT_NAME_ANY))
                         return -EINVAL;
 
+                r = do_mac_check(mac_check, fn, userdata);
+                if (r < 0)
+                        return r;
+
                 full = path_join(paths.root_dir, *i);
                 if (!full)
                         return -ENOMEM;
@@ -2404,7 +2433,9 @@ int unit_file_revert(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
@@ -2435,6 +2466,10 @@ int unit_file_revert(
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
+                r = do_mac_check(mac_check, *i, userdata);
+                if (r < 0)
+                        return r;
+
                 STRV_FOREACH(p, paths.search_path) {
                         _cleanup_free_ char *path = NULL, *dropin = NULL;
                         struct stat st;
@@ -2561,7 +2596,9 @@ int unit_file_add_dependency(
                 const char *target,
                 UnitDependency dep,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(install_context_done) InstallContext c = {};
@@ -2595,6 +2632,10 @@ int unit_file_add_dependency(
 
         assert(target_info->type == UNIT_FILE_TYPE_REGULAR);
 
+        r = do_mac_check(mac_check, target_info->name, userdata);
+        if (r < 0)
+                return r;
+
         STRV_FOREACH(f, files) {
                 char ***l;
 
@@ -2605,6 +2646,10 @@ int unit_file_add_dependency(
 
                 assert(i->type == UNIT_FILE_TYPE_REGULAR);
 
+                r = do_mac_check(mac_check, i->name, userdata);
+                if (r < 0)
+                        return r;
+
                 /* We didn't actually load anything from the unit
                  * file, but instead just add in our new symlink to
                  * create. */
@@ -2630,7 +2675,9 @@ int unit_file_enable(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(install_context_done) InstallContext c = {};
@@ -2657,6 +2704,10 @@ int unit_file_enable(
                         return r;
 
                 assert(i->type == UNIT_FILE_TYPE_REGULAR);
+
+                r = do_mac_check(mac_check, i->name, userdata);
+                if (r < 0)
+                        return r;
         }
 
         /* This will return the number of symlink rules that were
@@ -2673,7 +2724,9 @@ int unit_file_disable(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(install_context_done) InstallContext c = {};
@@ -2697,6 +2750,10 @@ int unit_file_disable(
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
+                r = do_mac_check(mac_check, *i, userdata);
+                if (r < 0)
+                        return r;
+
                 r = install_info_add(&c, *i, NULL, paths.root_dir, /* auxiliary= */ false, NULL);
                 if (r < 0)
                         return r;
@@ -2715,7 +2772,9 @@ int unit_file_reenable(
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         char **n;
         int r;
@@ -2728,12 +2787,12 @@ int unit_file_reenable(
                 n[i] = basename(files[i]);
         n[i] = NULL;
 
-        r = unit_file_disable(scope, flags, root_dir, n, changes, n_changes);
+        r = unit_file_disable(scope, flags, root_dir, n, changes, n_changes, mac_check, userdata);
         if (r < 0)
                 return r;
 
         /* But the enable command with the full name */
-        return unit_file_enable(scope, flags, root_dir, files, changes, n_changes);
+        return unit_file_enable(scope, flags, root_dir, files, changes, n_changes, mac_check, userdata);
 }
 
 int unit_file_set_default(
@@ -3248,7 +3307,9 @@ static int preset_prepare_one(
                 const char *name,
                 const UnitFilePresets *presets,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(install_context_done) InstallContext tmp = {};
         _cleanup_strv_free_ char **instance_name_list = NULL;
@@ -3262,6 +3323,11 @@ static int preset_prepare_one(
                                   &i, changes, n_changes);
         if (r < 0)
                 return r;
+
+        r = do_mac_check(mac_check, i->name, userdata);
+        if (r < 0)
+                return r;
+
         if (!streq(name, i->name)) {
                 log_debug("Skipping %s because it is an alias for %s.", name, i->name);
                 return 0;
@@ -3301,7 +3367,9 @@ int unit_file_preset(
                 char **files,
                 UnitFilePresetMode mode,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
@@ -3327,7 +3395,7 @@ int unit_file_preset(
                 return r;
 
         STRV_FOREACH(i, files) {
-                r = preset_prepare_one(scope, &plus, &minus, &paths, *i, &presets, changes, n_changes);
+                r = preset_prepare_one(scope, &plus, &minus, &paths, *i, &presets, changes, n_changes, mac_check, userdata);
                 if (r < 0)
                         return r;
         }
@@ -3341,7 +3409,9 @@ int unit_file_preset_all(
                 const char *root_dir,
                 UnitFilePresetMode mode,
                 UnitFileChange **changes,
-                size_t *n_changes) {
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata) {
 
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
@@ -3386,7 +3456,7 @@ int unit_file_preset_all(
                         if (!IN_SET(de->d_type, DT_LNK, DT_REG))
                                 continue;
 
-                        r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, &presets, changes, n_changes);
+                        r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, &presets, changes, n_changes, mac_check, userdata);
                         if (r < 0 &&
                             !IN_SET(r, -EEXIST, -ERFKILL, -EADDRNOTAVAIL, -EIDRM, -EUCLEAN, -ELOOP, -ENOENT))
                                 /* Ignore generated/transient/missing/invalid units when applying preset, propagate other errors.
diff --git a/src/shared/install.h b/src/shared/install.h
index cdc5435035..96896f0548 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -93,27 +93,35 @@ struct UnitFileInstallInfo {
         bool auxiliary;
 };
 
+typedef int (*mac_callback_t)(const char *unit_name, void *userdata);
+
 int unit_file_enable(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_disable(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_reenable(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_preset(
                 UnitFileScope scope,
                 UnitFileFlags flags,
@@ -121,41 +129,53 @@ int unit_file_preset(
                 char **files,
                 UnitFilePresetMode mode,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_preset_all(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 UnitFilePresetMode mode,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_mask(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_unmask(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_link(
                 UnitFileScope scope,
                 UnitFileFlags flags,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_revert(
                 UnitFileScope scope,
                 const char *root_dir,
                 char **files,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 int unit_file_set_default(
                 UnitFileScope scope,
                 UnitFileFlags flags,
@@ -175,7 +195,9 @@ int unit_file_add_dependency(
                 const char *target,
                 UnitDependency dep,
                 UnitFileChange **changes,
-                size_t *n_changes);
+                size_t *n_changes,
+                mac_callback_t mac_check,
+                void *userdata);
 
 int unit_file_lookup_state(
                 UnitFileScope scope,
diff --git a/src/systemctl/systemctl-add-dependency.c b/src/systemctl/systemctl-add-dependency.c
index ba385ea2a2..90b6c28aa0 100644
--- a/src/systemctl/systemctl-add-dependency.c
+++ b/src/systemctl/systemctl-add-dependency.c
@@ -37,7 +37,7 @@ int add_dependency(int argc, char *argv[], void *userdata) {
                 assert_not_reached();
 
         if (install_client_side()) {
-                r = unit_file_add_dependency(arg_scope, unit_file_flags_from_args(), arg_root, names, target, dep, &changes, &n_changes);
+                r = unit_file_add_dependency(arg_scope, unit_file_flags_from_args(), arg_root, names, target, dep, &changes, &n_changes, NULL, NULL);
                 unit_file_dump_changes(r, "add dependency on", changes, n_changes, arg_quiet);
 
                 if (r > 0)
diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c
index dcbe2c7302..5cfdcd73ca 100644
--- a/src/systemctl/systemctl-enable.c
+++ b/src/systemctl/systemctl-enable.c
@@ -106,23 +106,23 @@ int enable_unit(int argc, char *argv[], void *userdata) {
 
                 flags = unit_file_flags_from_args();
                 if (streq(verb, "enable")) {
-                        r = unit_file_enable(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_enable(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                         carries_install_info = r;
                 } else if (streq(verb, "disable"))
-                        r = unit_file_disable(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_disable(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                 else if (streq(verb, "reenable")) {
-                        r = unit_file_reenable(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_reenable(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                         carries_install_info = r;
                 } else if (streq(verb, "link"))
-                        r = unit_file_link(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_link(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                 else if (streq(verb, "preset")) {
-                        r = unit_file_preset(arg_scope, flags, arg_root, names, arg_preset_mode, &changes, &n_changes);
+                        r = unit_file_preset(arg_scope, flags, arg_root, names, arg_preset_mode, &changes, &n_changes, NULL, NULL);
                 } else if (streq(verb, "mask"))
-                        r = unit_file_mask(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_mask(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                 else if (streq(verb, "unmask"))
-                        r = unit_file_unmask(arg_scope, flags, arg_root, names, &changes, &n_changes);
+                        r = unit_file_unmask(arg_scope, flags, arg_root, names, &changes, &n_changes, NULL, NULL);
                 else if (streq(verb, "revert"))
-                        r = unit_file_revert(arg_scope, arg_root, names, &changes, &n_changes);
+                        r = unit_file_revert(arg_scope, arg_root, names, &changes, &n_changes, NULL, NULL);
                 else
                         assert_not_reached();
 
diff --git a/src/systemctl/systemctl-is-enabled.c b/src/systemctl/systemctl-is-enabled.c
index e33dffaf29..08e1e03619 100644
--- a/src/systemctl/systemctl-is-enabled.c
+++ b/src/systemctl/systemctl-is-enabled.c
@@ -18,7 +18,7 @@ static int show_installation_targets_client_side(const char *name) {
         flags = UNIT_FILE_DRY_RUN |
                 (arg_runtime ? UNIT_FILE_RUNTIME : 0);
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, flags, NULL, p, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, flags, NULL, p, &changes, &n_changes, NULL, NULL);
         if (r < 0)
                 return log_error_errno(r, "Failed to get file links for %s: %m", name);
 
diff --git a/src/systemctl/systemctl-preset-all.c b/src/systemctl/systemctl-preset-all.c
index b5eb199f4a..c325606b43 100644
--- a/src/systemctl/systemctl-preset-all.c
+++ b/src/systemctl/systemctl-preset-all.c
@@ -13,7 +13,7 @@ int preset_all(int argc, char *argv[], void *userdata) {
         int r;
 
         if (install_client_side()) {
-                r = unit_file_preset_all(arg_scope, unit_file_flags_from_args(), arg_root, arg_preset_mode, &changes, &n_changes);
+                r = unit_file_preset_all(arg_scope, unit_file_flags_from_args(), arg_root, arg_preset_mode, &changes, &n_changes, NULL, NULL);
                 unit_file_dump_changes(r, "preset", changes, n_changes, arg_quiet);
 
                 if (r > 0)
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 30494f7fa1..e7b0cc3d6e 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -54,7 +54,7 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", NULL) >= 0);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
 
-        assert_se(unit_file_mask(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_mask(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/dev/null"));
@@ -70,11 +70,11 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_MASKED);
 
         /* Enabling a masked unit should fail! */
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) == -ERFKILL);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) == -ERFKILL);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_unmask(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_unmask(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/a.service");
@@ -82,7 +82,7 @@ static void test_basic_mask_and_enable(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
@@ -97,12 +97,12 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
 
         /* Enabling it again should succeed but be a NOP */
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 0);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/a.service");
@@ -116,13 +116,13 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
 
         /* Disabling a disabled unit must succeed but be a NOP */
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 0);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
         /* Let's enable this indirectly via a symlink */
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("d.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("d.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
@@ -138,7 +138,7 @@ static void test_basic_mask_and_enable(const char *root) {
 
         /* Let's try to reenable */
 
-        assert_se(unit_file_reenable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("b.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_reenable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("b.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/a.service");
@@ -180,7 +180,7 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", NULL) >= 0);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("f.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("f.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/f.service"));
@@ -248,7 +248,7 @@ static void test_linked_units(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "linked3.service", &state) >= 0 && state == UNIT_FILE_LINKED);
 
         /* First, let's link the unit into the search path */
-        assert_se(unit_file_link(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("/opt/linked.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_link(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("/opt/linked.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/opt/linked.service"));
@@ -260,7 +260,7 @@ static void test_linked_units(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "linked.service", &state) >= 0 && state == UNIT_FILE_LINKED);
 
         /* Let's unlink it from the search path again */
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
@@ -271,7 +271,7 @@ static void test_linked_units(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "linked.service", NULL) == -ENOENT);
 
         /* Now, let's not just link it, but also enable it */
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("/opt/linked.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("/opt/linked.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 2);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/linked.service");
         q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
@@ -293,7 +293,7 @@ static void test_linked_units(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "linked.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
 
         /* And let's unlink it again */
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 2);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/linked.service");
         q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
@@ -313,7 +313,7 @@ static void test_linked_units(const char *root) {
 
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "linked.service", NULL) == -ENOENT);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked2.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked2.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 2);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/linked2.service");
         q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked2.service");
@@ -332,7 +332,7 @@ static void test_linked_units(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked3.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("linked3.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(startswith(changes[0].path, root));
@@ -395,7 +395,7 @@ static void test_add_dependency(const char *root) {
         p = strjoina(root, "/usr/lib/systemd/system/add-dependency-test-service.service");
         assert_se(symlink("real-add-dependency-test-service.service", p) >= 0);
 
-        assert_se(unit_file_add_dependency(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("add-dependency-test-service.service"), "add-dependency-test-target.target", UNIT_WANTS, &changes, &n_changes) >= 0);
+        assert_se(unit_file_add_dependency(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("add-dependency-test-service.service"), "add-dependency-test-target.target", UNIT_WANTS, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/real-add-dependency-test-service.service"));
@@ -436,7 +436,7 @@ static void test_template_enable(const char *root) {
 
         log_info("== %s with template@.service enabled ==", __func__);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
@@ -452,7 +452,7 @@ static void test_template_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         assert_se(streq(changes[0].path, p));
@@ -468,7 +468,7 @@ static void test_template_enable(const char *root) {
 
         log_info("== %s with template@foo.service enabled ==", __func__);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/template@foo.service");
@@ -483,7 +483,7 @@ static void test_template_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         assert_se(streq(changes[0].path, p));
@@ -501,7 +501,7 @@ static void test_template_enable(const char *root) {
 
         log_info("== %s with template-symlink@quux.service enabled ==", __func__);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template-symlink@quux.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template-symlink@quux.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/template@quux.service");
@@ -546,7 +546,7 @@ static void test_indirect(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectb.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/indirectb.service"));
@@ -559,7 +559,7 @@ static void test_indirect(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectb.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/indirectb.service");
@@ -598,7 +598,7 @@ static void test_preset_and_list(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-yes.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-yes.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-yes.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/preset-yes.service"));
@@ -610,7 +610,7 @@ static void test_preset_and_list(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-yes.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-yes.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-yes.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/preset-yes.service");
@@ -621,7 +621,7 @@ static void test_preset_and_list(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-yes.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-no.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-no.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 0);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
@@ -629,7 +629,7 @@ static void test_preset_and_list(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-yes.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "preset-no.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset_all(UNIT_FILE_SYSTEM, 0, root, UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset_all(UNIT_FILE_SYSTEM, 0, root, UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
 
         assert_se(n_changes > 0);
 
@@ -693,7 +693,7 @@ static void test_revert(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "xx.service", &state) >= 0 && state == UNIT_FILE_STATIC);
 
         /* Initially there's nothing to revert */
-        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 0);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
@@ -702,7 +702,7 @@ static void test_revert(const char *root) {
         assert_se(write_string_file(p, "# Empty override\n", WRITE_STRING_FILE_CREATE) >= 0);
 
         /* Revert the override file */
-        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         assert_se(streq(changes[0].path, p));
@@ -714,7 +714,7 @@ static void test_revert(const char *root) {
         assert_se(write_string_file(p, "# Empty dropin\n", WRITE_STRING_FILE_CREATE) >= 0);
 
         /* Revert the dropin file */
-        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_revert(UNIT_FILE_SYSTEM, root, STRV_MAKE("xx.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         assert_se(streq(changes[0].path, p));
@@ -754,7 +754,7 @@ static void test_preset_order(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("prefix-1.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("prefix-1.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/prefix-1.service"));
@@ -766,7 +766,7 @@ static void test_preset_order(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("prefix-2.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("prefix-2.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 0);
 
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
@@ -866,7 +866,7 @@ static void test_with_dropin(const char *root) {
 
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-4b.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-1.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-1.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
@@ -879,7 +879,7 @@ static void test_with_dropin(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-2.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
@@ -893,7 +893,7 @@ static void test_with_dropin(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-3.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-3.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-3.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
@@ -907,7 +907,7 @@ static void test_with_dropin(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-4a.service"), &changes, &n_changes) == 2);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-4a.service"), &changes, &n_changes, NULL, NULL) == 2);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-3.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
@@ -978,7 +978,7 @@ static void test_with_dropin_template(const char *root) {
 
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-3@.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-1@instance-1.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-1@instance-1.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
@@ -991,7 +991,7 @@ static void test_with_dropin_template(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2@instance-1.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2@instance-1.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
@@ -1004,7 +1004,7 @@ static void test_with_dropin_template(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2@instance-2.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2@instance-2.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-2@.service"));
@@ -1013,7 +1013,7 @@ static void test_with_dropin_template(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-3@.service"), &changes, &n_changes) == 1);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-3@.service"), &changes, &n_changes, NULL, NULL) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-3@.service"));
@@ -1053,7 +1053,7 @@ static void test_preset_multiple_instances(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar0.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
         /* Preset a single instantiated unit specified in the list */
-        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar0.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
@@ -1062,7 +1062,7 @@ static void test_preset_multiple_instances(const char *root) {
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), &changes, &n_changes) >= 0);
+        assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_UNLINK);
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/foo@bar0.service");
@@ -1075,7 +1075,7 @@ static void test_preset_multiple_instances(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar1.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bartest.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 
-        assert_se(unit_file_preset_all(UNIT_FILE_SYSTEM, 0, root, UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+        assert_se(unit_file_preset_all(UNIT_FILE_SYSTEM, 0, root, UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL) >= 0);
         assert_se(n_changes > 0);
 
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
diff --git a/src/test/test-install.c b/src/test/test-install.c
index 7a0beb2d24..523aaf8220 100644
--- a/src/test/test-install.c
+++ b/src/test/test-install.c
@@ -52,12 +52,12 @@ int main(int argc, char* argv[]) {
 
         log_info("/*** enable **/");
 
-        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         log_info("/*** enable2 **/");
 
-        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -71,7 +71,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -85,10 +85,10 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
         log_info("/*** mask2 ***/");
-        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -102,10 +102,10 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
         log_info("/*** unmask2 ***/");
-        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -119,7 +119,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_mask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -133,10 +133,10 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
         log_info("/*** disable2 ***/");
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -150,7 +150,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes);
+        r = unit_file_unmask(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -164,7 +164,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_enable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -178,7 +178,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -191,7 +191,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_link(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_link(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -205,7 +205,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -218,7 +218,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_link(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_link(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -232,7 +232,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_reenable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_reenable(UNIT_FILE_SYSTEM, 0, NULL, (char**) files2, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -246,7 +246,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, 0, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
@@ -258,7 +258,7 @@ int main(int argc, char* argv[]) {
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_preset(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, UNIT_FILE_PRESET_FULL, &changes, &n_changes);
+        r = unit_file_preset(UNIT_FILE_SYSTEM, 0, NULL, (char**) files, UNIT_FILE_PRESET_FULL, &changes, &n_changes, NULL, NULL);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 5/7] core: implement the sd-bus generic callback for SELinux
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
                   ` (3 preceding siblings ...)
  2021-08-05 14:24 ` [PATCH SYSTEMD 4/7] core: add support for MAC checks on unit install operations Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 6/7] core: avoid bypasses in D-BUS SELinux filter Christian Göttsche
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

---
 src/core/dbus-callbackdata.h | 10 ++++++
 src/core/dbus-manager.c      | 45 +++++++++++++++++----------
 src/core/manager.c           |  7 +++++
 src/core/manager.h           |  1 +
 src/core/selinux-access.c    | 53 ++++++++++++++++++++++++++++++-
 src/core/selinux-access.h    |  7 +++++
 src/shared/install.c         | 60 ++++++++++++++++++++++++++++++++++++
 src/shared/install.h         |  2 ++
 8 files changed, 168 insertions(+), 17 deletions(-)

diff --git a/src/core/dbus-callbackdata.h b/src/core/dbus-callbackdata.h
index 7dedb680c0..400c3bc481 100644
--- a/src/core/dbus-callbackdata.h
+++ b/src/core/dbus-callbackdata.h
@@ -1,5 +1,15 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include "sd-bus.h"
+
+#include "manager.h"
+
 typedef struct MacUnitCallbackUserdata {
+        Manager *manager;
+        sd_bus_message *message;
+        sd_bus_error *error;
+        const char *func;
+
+        const char *selinux_permission;
 } MacUnitCallbackUserdata;
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index eec358cb6b..66cc276f8f 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -2187,10 +2187,15 @@ fail:
 
 static int mac_unit_callback_check(const char *unit_name, void *userdata) {
         MacUnitCallbackUserdata *ud = userdata;
+        int r;
 
         assert(unit_name);
         assert(ud);
 
+        r = mac_selinux_unit_callback_check(unit_name, ud);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 
@@ -2199,6 +2204,8 @@ static int method_enable_unit_files_generic(
                 Manager *m,
                 int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata),
                 bool carries_install_info,
+                const char *mac_selinux_permission,
+                const char *func,
                 sd_bus_error *error) {
 
         _cleanup_strv_free_ char **l = NULL;
@@ -2206,7 +2213,7 @@ static int method_enable_unit_files_generic(
         size_t n_changes = 0;
         UnitFileFlags flags;
         int r;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, func, mac_selinux_permission };
 
         assert(message);
         assert(m);
@@ -2247,19 +2254,19 @@ static int method_enable_unit_files_generic(
 }
 
 static int method_enable_unit_files_with_flags(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_enable, true, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_enable, true, "enable", __func__, error);
 }
 
 static int method_enable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_enable, true, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_enable, true, "enable", __func__, error);
 }
 
 static int method_reenable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_reenable, true, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_reenable, true, "enable", __func__, error);
 }
 
 static int method_link_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_link, false, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_link, false, "enable", __func__, error);
 }
 
 static int unit_file_preset_without_mode(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char **files, UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata) {
@@ -2267,11 +2274,11 @@ static int unit_file_preset_without_mode(UnitFileScope scope, UnitFileFlags flag
 }
 
 static int method_preset_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_preset_without_mode, true, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_preset_without_mode, true, "reload", __func__, error);
 }
 
 static int method_mask_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_enable_unit_files_generic(message, userdata, unit_file_mask, false, error);
+        return method_enable_unit_files_generic(message, userdata, unit_file_mask, false, "disable", __func__, error);
 }
 
 static int method_preset_unit_files_with_mode(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2284,7 +2291,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
         int runtime, force, r;
         UnitFileFlags flags;
         const char *mode;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, __func__, "reload" };
 
         assert(message);
         assert(m);
@@ -2324,6 +2331,8 @@ static int method_disable_unit_files_generic(
                 sd_bus_message *message,
                 Manager *m,
                 int (*call)(UnitFileScope scope, UnitFileFlags flags, const char *root_dir, char *files[], UnitFileChange **changes, size_t *n_changes, mac_callback_t mac_check, void *userdata),
+                const char *mac_selinux_permissions,
+                const char *func,
                 sd_bus_error *error) {
 
         _cleanup_strv_free_ char **l = NULL;
@@ -2331,7 +2340,7 @@ static int method_disable_unit_files_generic(
         UnitFileFlags flags;
         size_t n_changes = 0;
         int r;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, func, mac_selinux_permissions };
 
         assert(message);
         assert(m);
@@ -2373,15 +2382,15 @@ static int method_disable_unit_files_generic(
 }
 
 static int method_disable_unit_files_with_flags(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_disable_unit_files_generic(message, userdata, unit_file_disable, error);
+        return method_disable_unit_files_generic(message, userdata, unit_file_disable, "disable", __func__, error);
 }
 
 static int method_disable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_disable_unit_files_generic(message, userdata, unit_file_disable, error);
+        return method_disable_unit_files_generic(message, userdata, unit_file_disable, "disable", __func__, error);
 }
 
 static int method_unmask_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        return method_disable_unit_files_generic(message, userdata, unit_file_unmask, error);
+        return method_disable_unit_files_generic(message, userdata, unit_file_unmask, "enable", __func__, error);
 }
 
 static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2390,7 +2399,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
         size_t n_changes = 0;
         Manager *m = userdata;
         int r;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = {  m, message, error, __func__, "reload" };
 
         assert(message);
         assert(m);
@@ -2451,7 +2460,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
         const char *mode;
         UnitFileFlags flags;
         int force, runtime, r;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, __func__, "reload" };
 
         assert(message);
         assert(m);
@@ -2496,7 +2505,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
         char *target, *type;
         UnitDependency dep;
         UnitFileFlags flags;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, __func__, "reload" };
 
         assert(message);
         assert(m);
@@ -2529,6 +2538,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
 }
 
 static int method_get_unit_file_links(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        Manager *m = userdata;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
         UnitFileChange *changes = NULL;
         size_t n_changes = 0, i;
@@ -2536,7 +2546,10 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
         const char *name;
         char **p;
         int runtime, r;
-        MacUnitCallbackUserdata mcud = {};
+        MacUnitCallbackUserdata mcud = { m, message, error, __func__, "status" };
+
+        assert(message);
+        assert(m);
 
         r = sd_bus_message_read(message, "sb", &name, &runtime);
         if (r < 0)
diff --git a/src/core/manager.c b/src/core/manager.c
index 952bc57036..f9bd2636fa 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1977,6 +1977,13 @@ Unit *manager_get_unit(Manager *m, const char *name) {
         return hashmap_get(m->units, name);
 }
 
+const char *manager_lookup_unit_label_path(Manager *m, const char *name) {
+        assert(m);
+        assert(name);
+
+        return hashmap_get(m->unit_id_map, name);
+}
+
 static int manager_dispatch_target_deps_queue(Manager *m) {
         Unit *u;
         int r = 0;
diff --git a/src/core/manager.h b/src/core/manager.h
index 284ea42a9d..42a345c9a7 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -474,6 +474,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds);
 
 Job *manager_get_job(Manager *m, uint32_t id);
 Unit *manager_get_unit(Manager *m, const char *name);
+const char *manager_lookup_unit_label_path(Manager *m, const char *name);
 
 int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j);
 
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 180fe22773..82a0feffb4 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -16,11 +16,14 @@
 #include "alloc-util.h"
 #include "audit-fd.h"
 #include "bus-util.h"
+#include "dbus-callbackdata.h"
 #include "errno-util.h"
 #include "format-util.h"
+#include "install.h"
 #include "log.h"
 #include "path-util.h"
 #include "selinux-util.h"
+#include "stat-util.h"
 #include "stdio-util.h"
 #include "strv.h"
 #include "util.h"
@@ -284,6 +287,47 @@ int _mac_selinux_generic_access_check(
         return enforce ? r : 0;
 }
 
+int mac_selinux_unit_callback_check(
+        const char *unit_name,
+        const MacUnitCallbackUserdata *userdata) {
+
+        const Unit *u;
+        const char *path = NULL;
+        _cleanup_free_ char *path_lookup = NULL;
+
+        assert(unit_name);
+        assert(userdata);
+        assert(userdata->manager);
+        assert(userdata->message);
+        assert(userdata->error);
+        assert(userdata->func);
+
+        if (!mac_selinux_use())
+                return 0;
+
+        if (!userdata->selinux_permission)
+                return 0;
+
+        u = manager_get_unit(userdata->manager, unit_name);
+        if (u)
+                path = unit_label_path(u);
+
+        /* maybe the unit is not loaded, e.g. a disabled user session unit */
+        if (!path)
+                path = manager_lookup_unit_label_path(userdata->manager, unit_name);
+
+        /* try to search for a unit path and do not base the check on a masked path, e.g. /dev/null */
+        if (!path || null_or_empty_path(path) > 0)
+                (void) unit_file_label_path(userdata->manager->unit_file_scope, NULL, unit_name, &path_lookup);
+
+        return _mac_selinux_generic_access_check(
+                userdata->message,
+                path_lookup ?: path,
+                userdata->selinux_permission,
+                userdata->error,
+                userdata->func);
+}
+
 #else /* HAVE_SELINUX */
 
 int _mac_selinux_generic_access_check(
@@ -296,4 +340,11 @@ int _mac_selinux_generic_access_check(
         return 0;
 }
 
-#endif /* HAVE_SELINUX */
+int mac_selinux_unit_callback_check(
+                const char *unit_name,
+                const MacUnitCallbackUserdata *userdata) {
+
+        return 0;
+}
+
+#endif
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index 8a5582997a..65eb643462 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -5,6 +5,9 @@
 
 #include "manager.h"
 
+/* forward declaration */
+typedef struct MacUnitCallbackUserdata MacUnitCallbackUserdata;
+
 int _mac_selinux_generic_access_check(sd_bus_message *message,
                                       const char *path,
                                       const char *permission,
@@ -16,3 +19,7 @@ int _mac_selinux_generic_access_check(sd_bus_message *message,
 
 #define mac_selinux_unit_access_check(unit, message, permission, error) \
         _mac_selinux_generic_access_check((message), unit_label_path(unit), (permission), (error), __func__)
+
+int mac_selinux_unit_callback_check(
+                const char *unit_name,
+                const MacUnitCallbackUserdata *userdata);
diff --git a/src/shared/install.c b/src/shared/install.c
index f1e603fecf..0eab706724 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -3559,6 +3559,66 @@ int unit_file_get_list(
         return 0;
 }
 
+int unit_file_label_path(
+        UnitFileScope scope,
+        const char *root_dir,
+        const char *name,
+        char **ret_path) {
+
+        _cleanup_(lookup_paths_free) LookupPaths paths = {};
+        char **dirname;
+        int r;
+
+        assert(scope >= 0);
+        assert(scope < _UNIT_FILE_SCOPE_MAX);
+        assert(name);
+        assert_return(unit_name_is_valid(name, UNIT_NAME_ANY), -EINVAL);
+        assert(ret_path);
+
+        r = lookup_paths_init(&paths, scope, 0, root_dir);
+        if (r < 0)
+                return r;
+
+        STRV_FOREACH(dirname, paths.search_path) {
+                _cleanup_closedir_ DIR *d = NULL;
+                const struct dirent *de;
+
+                d = opendir(*dirname);
+                if (!d) {
+                        if (errno == ENOENT)
+                                continue;
+                        if (IN_SET(errno, ENOTDIR, EACCES)) {
+                                log_debug_errno(errno, "Failed to open \"%s\": %m", *dirname);
+                                continue;
+                        }
+
+                        return -errno;
+                }
+
+                FOREACH_DIRENT(de, d, return -errno) {
+                        _cleanup_free_ char *p = NULL;
+
+                        if (!IN_SET(de->d_type, DT_LNK, DT_REG))
+                                continue;
+
+                        if (!streq(de->d_name, name))
+                                continue;
+
+                        p = path_join(*dirname, de->d_name);
+                        if (!p)
+                                return -ENOMEM;
+
+                        if (null_or_empty_path(p))
+                                continue;
+
+                        *ret_path = TAKE_PTR(p);
+                        return 0;
+                }
+        }
+
+        return -ENOENT;
+}
+
 static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = {
         [UNIT_FILE_ENABLED]         = "enabled",
         [UNIT_FILE_ENABLED_RUNTIME] = "enabled-runtime",
diff --git a/src/shared/install.h b/src/shared/install.h
index 96896f0548..e03422c65a 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -208,6 +208,8 @@ int unit_file_lookup_state(
 int unit_file_get_state(UnitFileScope scope, const char *root_dir, const char *filename, UnitFileState *ret);
 int unit_file_exists(UnitFileScope scope, const LookupPaths *paths, const char *name);
 
+int unit_file_label_path(UnitFileScope scope, const char *root_dir, const char *name, char **ret_path);
+
 int unit_file_get_list(UnitFileScope scope, const char *root_dir, Hashmap *h, char **states, char **patterns);
 Hashmap* unit_file_list_free(Hashmap *h);
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 6/7] core: avoid bypasses in D-BUS SELinux filter
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
                   ` (4 preceding siblings ...)
  2021-08-05 14:24 ` [PATCH SYSTEMD 5/7] core: implement the sd-bus generic callback for SELinux Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 14:24 ` [PATCH SYSTEMD 7/7] core: tweak job_type_to_access_method SELinux permissions Christian Göttsche
  2021-08-05 15:08 ` [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Dominick Grift
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

Do not allow access if D-Bus credentials are unavailable.
Perform system check if job or unit not found.
---
 src/core/dbus.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/core/dbus.c b/src/core/dbus.c
index f876433c00..576dfa0b11 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -214,13 +214,12 @@ static int mac_selinux_filter(sd_bus_message *message, void *userdata, sd_bus_er
         Manager *m = userdata;
         const char *verb, *path;
         Unit *u = NULL;
-        Job *j;
         int r;
 
         assert(message);
 
         /* Our own method calls are all protected individually with
-         * selinux checks, but the built-in interfaces need to be
+         * SELinux checks, but the built-in interfaces need to be
          * protected too. */
 
         if (sd_bus_message_is_method_call(message, "org.freedesktop.DBus.Properties", "Set"))
@@ -235,42 +234,37 @@ static int mac_selinux_filter(sd_bus_message *message, void *userdata, sd_bus_er
 
         path = sd_bus_message_get_path(message);
 
-        if (object_path_startswith("/org/freedesktop/systemd1", path)) {
-                r = mac_selinux_access_check(message, verb, error);
-                if (r < 0)
-                        return r;
+        if (streq_ptr(path, "/org/freedesktop/systemd1"))
+                return mac_selinux_access_check(message, verb, error);
 
-                return 0;
-        }
-
-        if (streq_ptr(path, "/org/freedesktop/systemd1/unit/self")) {
+        if (object_path_startswith(path, "/org/freedesktop/systemd1/job")) {
+                Job *j;
+                r = manager_get_job_from_dbus_path(m, path, &j);
+                if (r >= 0)
+                        u = j->unit;
+        } else if (streq_ptr(path, "/org/freedesktop/systemd1/unit/self")) {
                 _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
                 pid_t pid;
 
                 r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_PID, &creds);
                 if (r < 0)
-                        return 0;
+                        return r;
 
                 r = sd_bus_creds_get_pid(creds, &pid);
                 if (r < 0)
-                        return 0;
+                        return r;
 
                 u = manager_get_unit_by_pid(m, pid);
-        } else {
-                r = manager_get_job_from_dbus_path(m, path, &j);
-                if (r >= 0)
-                        u = j->unit;
-                else
-                        manager_load_unit_from_dbus_path(m, path, NULL, &u);
-        }
-        if (!u)
-                return 0;
+        } else if (object_path_startswith(path, "/org/freedesktop/systemd1/unit")) {
+                manager_load_unit_from_dbus_path(m, path, NULL, &u);
+        } else
+                log_warning("Unexpected object path '%s' in SELinux D-Bus filter", path);
 
-        r = mac_selinux_unit_access_check(u, message, verb, error);
-        if (r < 0)
-                return r;
+        if (!u)
+                /* Fall back to main instance check, e.g. if job or unit not found. */
+                return mac_selinux_access_check(message, verb, error);
 
-        return 0;
+        return mac_selinux_unit_access_check(u, message, verb, error);
 }
 #endif
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH SYSTEMD 7/7] core: tweak job_type_to_access_method SELinux permissions
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
                   ` (5 preceding siblings ...)
  2021-08-05 14:24 ` [PATCH SYSTEMD 6/7] core: avoid bypasses in D-BUS SELinux filter Christian Göttsche
@ 2021-08-05 14:24 ` Christian Göttsche
  2021-08-05 15:08 ` [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Dominick Grift
  7 siblings, 0 replies; 9+ messages in thread
From: Christian Göttsche @ 2021-08-05 14:24 UTC (permalink / raw)
  To: selinux

JOB_RELOAD_OR_START might collapse into JOB_START and JOB_VERIFY_ACTIVE
is similar to JOB_START.  Use the more heavy and appropriate permission
"start" instead of "reload".
---
 src/core/job.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/core/job.c b/src/core/job.c
index dd16a0b280..0b2e0373c6 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -1557,12 +1557,20 @@ const char* job_type_to_access_method(JobType t) {
         assert(t >= 0);
         assert(t < _JOB_TYPE_MAX);
 
-        if (IN_SET(t, JOB_START, JOB_RESTART, JOB_TRY_RESTART))
+        switch (t) {
+        case JOB_START:
+        case JOB_RESTART:
+        case JOB_TRY_RESTART:
+        case JOB_RELOAD_OR_START:
+        case JOB_VERIFY_ACTIVE:
                 return "start";
-        else if (t == JOB_STOP)
+        case JOB_STOP:
                 return "stop";
-        else
+        case JOB_RELOAD:
+        case JOB_TRY_RELOAD:
+        default:
                 return "reload";
+        }
 }
 
 /*
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations
  2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
                   ` (6 preceding siblings ...)
  2021-08-05 14:24 ` [PATCH SYSTEMD 7/7] core: tweak job_type_to_access_method SELinux permissions Christian Göttsche
@ 2021-08-05 15:08 ` Dominick Grift
  7 siblings, 0 replies; 9+ messages in thread
From: Dominick Grift @ 2021-08-05 15:08 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: selinux

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-05 15:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 14:24 [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 1/7] selinux: add function name to audit data Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 2/7] selinux: improve debug log format Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 3/7] selinux: mark _mac_selinux_generic_access_check with leading underscore Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 4/7] core: add support for MAC checks on unit install operations Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 5/7] core: implement the sd-bus generic callback for SELinux Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 6/7] core: avoid bypasses in D-BUS SELinux filter Christian Göttsche
2021-08-05 14:24 ` [PATCH SYSTEMD 7/7] core: tweak job_type_to_access_method SELinux permissions Christian Göttsche
2021-08-05 15:08 ` [PATCH SYSTEMD 0/7] Re-add SELinux checks for unit install operations Dominick Grift

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.