* [RFC PATCH 0/8] systemd: improve SELinux support
@ 2019-12-18 14:28 Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 1/8] selinux-util: increase log severity Christian Göttsche
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Improve the SELinux support in systemd, especially re-adding checks for
unit file operations, like enable, mask...
The original pull request can be found at https://github.com/systemd/systemd/pull/10023
Patch 1 and 2 improve logging on failures in permissive mode.
Patch 3 adds the ability to obtain the context for a masked unit.
Patch 4 and 5 change several system und service checks. For better
distinction two new permissions are introduced: modify and listdynusers.
Patch 6 and 7 re-introduce checking unit file install operations.
They were dropped in 8faae625dc9b6322db452937f54176e56e65265a .
For consistency in the unexpected case while perforimg a service access
check no path can be gathered, now the check will still be executed on
the service security class (currently it switches to the system security
class).
Patch 8 adds some notes for adding future D-Bus interfaces.
Christian Göttsche (8):
selinux-util: increase log severity
selinux-access: log warning on context acquisition failure
core: bookkeeping withdrawal path of masked units
core: add missing SELinux checks for dbus methods
core: make SELinux access permissions more distinct
core: add support for MAC checks on unit install operations
core: implement the sd-bus generic callback for SELinux
core: add notes to D-Bus interfaces about adding SELinux checks
src/analyze/analyze.c | 11 ++-
src/basic/selinux-util.c | 4 +-
src/core/dbus-automount.c | 3 +
src/core/dbus-cgroup.c | 3 +
src/core/dbus-device.c | 3 +
src/core/dbus-execute.c | 3 +
src/core/dbus-job.c | 7 ++
src/core/dbus-kill.c | 3 +
src/core/dbus-manager.c | 164 ++++++++++++++++++++++++++++-------
src/core/dbus-mount.c | 3 +
src/core/dbus-path.c | 3 +
src/core/dbus-scope.c | 3 +
src/core/dbus-service.c | 3 +
src/core/dbus-slice.c | 3 +
src/core/dbus-socket.c | 3 +
src/core/dbus-swap.c | 3 +
src/core/dbus-target.c | 3 +
src/core/dbus-timer.c | 3 +
src/core/dbus-unit.c | 14 ++-
src/core/load-fragment.c | 10 +++
src/core/manager.c | 10 ++-
src/core/manager.h | 2 +
src/core/selinux-access.c | 44 ++++++++--
src/core/selinux-access.h | 28 +++++-
src/core/unit.c | 13 ++-
src/core/unit.h | 3 +-
src/shared/install.c | 101 +++++++++++++++++----
src/shared/install.h | 42 ++++++---
src/shared/unit-file.c | 52 ++++++++---
src/shared/unit-file.h | 1 +
src/systemctl/systemctl.c | 28 +++---
src/test/test-install-root.c | 86 +++++++++---------
src/test/test-install.c | 38 ++++----
src/test/test-unit-file.c | 8 +-
34 files changed, 543 insertions(+), 165 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 1/8] selinux-util: increase log severity
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 2/8] selinux-access: log warning on context acquisition failure Christian Göttsche
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
`log_enforcing()` and `log_enforcing_errno()` are only used for important messages, which describe failures in enforced mode.
They are guarded either by `!mac_selinux_use()` or `!label_hnd` checks, where the latter is itself guarded by the former.
Only SELinux enabled systems print these logs.
This helps to configure a system in permissive mode, without getting surprising failures when switching to enforced mode.
---
src/basic/selinux-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c
index 2c6d407295..1d209d03d5 100644
--- a/src/basic/selinux-util.c
+++ b/src/basic/selinux-util.c
@@ -37,8 +37,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(context_t, context_free);
static int cached_use = -1;
static struct selabel_handle *label_hnd = NULL;
-#define log_enforcing(...) log_full(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, __VA_ARGS__)
-#define log_enforcing_errno(r, ...) log_full_errno(security_getenforce() == 1 ? LOG_ERR : LOG_DEBUG, r, __VA_ARGS__)
+#define log_enforcing(...) log_full(security_getenforce() == 1 ? LOG_ERR : LOG_WARNING, __VA_ARGS__)
+#define log_enforcing_errno(r, ...) log_full_errno(security_getenforce() == 1 ? LOG_ERR : LOG_WARNING, r, __VA_ARGS__)
#endif
bool mac_selinux_use(void) {
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 2/8] selinux-access: log warning on context acquisition failure
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 1/8] selinux-util: increase log severity Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 3/8] core: bookkeeping withdrawal path of masked units Christian Göttsche
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Relevant when testing in permissive mode, where the function does not return a failure.
This helps to configure a system in permissive mode, without getting surprising failures when switching to enforced mode.
---
src/core/selinux-access.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index 9fd3099fea..4500e4452f 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -223,6 +223,7 @@ int mac_selinux_generic_access_check(
r = getfilecon_raw(path, &fcon);
if (r < 0) {
+ log_warning_errno(errno, "SELinux getfilecon_raw on '%s' failed: %m (tclass=%s perm=%s)", path, tclass, permission);
r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path);
goto finish;
}
@@ -231,6 +232,7 @@ int mac_selinux_generic_access_check(
} else {
r = getcon_raw(&fcon);
if (r < 0) {
+ log_warning_errno(errno, "SELinux getcon_raw failed: %m (tclass=%s perm=%s)", tclass, permission);
r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context.");
goto finish;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 3/8] core: bookkeeping withdrawal path of masked units
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 1/8] selinux-util: increase log severity Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 2/8] selinux-access: log warning on context acquisition failure Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods Christian Göttsche
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Make it possible to retrieve the MAC target context for an operation on a masked unit beforehand the execution.
E.g. if unit foo is installed at `/lib/systemd/system/foo.service` with label `foo_unit_t`, but masked via a symlink `/etc/systemd/system/foo.service` -> `/dev/null`,
this storing enables to check the unmask operation on the desired context `foo_unit_t`.
---
src/analyze/analyze.c | 11 ++++++++-
src/core/load-fragment.c | 10 ++++++++
src/core/manager.c | 1 +
src/core/manager.h | 1 +
src/core/unit.c | 9 +++++--
src/core/unit.h | 1 +
src/shared/unit-file.c | 52 +++++++++++++++++++++++++++++----------
src/shared/unit-file.h | 1 +
src/systemctl/systemctl.c | 2 +-
src/test/test-unit-file.c | 8 ++++--
10 files changed, 77 insertions(+), 19 deletions(-)
diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c
index 991e61de7e..8510603185 100644
--- a/src/analyze/analyze.c
+++ b/src/analyze/analyze.c
@@ -1558,6 +1558,7 @@ static bool strv_fnmatch_strv_or_empty(char* const* patterns, char **strv, int f
static int do_unit_files(int argc, char *argv[], void *userdata) {
_cleanup_(lookup_paths_free) LookupPaths lp = {};
_cleanup_hashmap_free_ Hashmap *unit_ids = NULL;
+ _cleanup_hashmap_free_ Hashmap *unit_withdrawals = NULL;
_cleanup_hashmap_free_ Hashmap *unit_names = NULL;
char **patterns = strv_skip(argv, 1);
Iterator i;
@@ -1569,7 +1570,7 @@ static int do_unit_files(int argc, char *argv[], void *userdata) {
if (r < 0)
return log_error_errno(r, "lookup_paths_init() failed: %m");
- r = unit_file_build_name_map(&lp, NULL, &unit_ids, &unit_names, NULL);
+ r = unit_file_build_name_map(&lp, NULL, &unit_ids, &unit_withdrawals, &unit_names, NULL);
if (r < 0)
return log_error_errno(r, "unit_file_build_name_map() failed: %m");
@@ -1581,6 +1582,14 @@ static int do_unit_files(int argc, char *argv[], void *userdata) {
printf("ids: %s → %s\n", k, dst);
}
+ HASHMAP_FOREACH_KEY(dst, k, unit_withdrawals, i) {
+ if (!strv_fnmatch_or_empty(patterns, k, FNM_NOESCAPE) &&
+ !strv_fnmatch_or_empty(patterns, dst, FNM_NOESCAPE))
+ continue;
+
+ printf("withdrawals: %s → %s\n", k, dst);
+ }
+
HASHMAP_FOREACH_KEY(v, k, unit_names, i) {
if (!strv_fnmatch_or_empty(patterns, k, FNM_NOESCAPE) &&
!strv_fnmatch_strv_or_empty(patterns, v, FNM_NOESCAPE))
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 1679e047dd..338bf84996 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4670,6 +4670,7 @@ int unit_load_fragment(Unit *u) {
r = unit_file_build_name_map(&u->manager->lookup_paths,
&u->manager->unit_cache_mtime,
&u->manager->unit_id_map,
+ &u->manager->unit_withdrawal_map,
&u->manager->unit_name_map,
&u->manager->unit_path_cache);
if (r < 0)
@@ -4702,8 +4703,17 @@ int unit_load_fragment(Unit *u) {
return r;
if (null_or_empty(&st)) {
+ const char *wpath;
+
u->load_state = UNIT_MASKED;
u->fragment_mtime = 0;
+
+ wpath = hashmap_get(u->manager->unit_withdrawal_map, u->id);
+ if (wpath) {
+ r = free_and_strdup(&u->withdrawal_path, wpath);
+ if (r < 0)
+ return r;
+ }
} else {
u->load_state = UNIT_LOADED;
u->fragment_mtime = timespec_load(&st.st_mtim);
diff --git a/src/core/manager.c b/src/core/manager.c
index 13a6b49a8f..7d8015e211 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -691,6 +691,7 @@ static int manager_setup_prefix(Manager *m) {
static void manager_free_unit_name_maps(Manager *m) {
m->unit_id_map = hashmap_free(m->unit_id_map);
+ m->unit_withdrawal_map = hashmap_free(m->unit_withdrawal_map);
m->unit_name_map = hashmap_free(m->unit_name_map);
m->unit_path_cache = set_free_free(m->unit_path_cache);
m->unit_cache_mtime = 0;
diff --git a/src/core/manager.h b/src/core/manager.h
index 51df7f8cd9..fb1a29d88c 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -224,6 +224,7 @@ struct Manager {
UnitFileScope unit_file_scope;
LookupPaths lookup_paths;
Hashmap *unit_id_map;
+ Hashmap *unit_withdrawal_map;
Hashmap *unit_name_map;
Set *unit_path_cache;
usec_t unit_cache_mtime;
diff --git a/src/core/unit.c b/src/core/unit.c
index c406bb7ab2..f9b299c8a4 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -708,6 +708,7 @@ void unit_free(Unit *u) {
strv_free(u->documentation);
free(u->fragment_path);
free(u->source_path);
+ free(u->withdrawal_path);
strv_free(u->dropin_paths);
free(u->instance);
@@ -1252,6 +1253,9 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
if (u->source_path)
fprintf(f, "%s\tSource Path: %s\n", prefix, u->source_path);
+ if (u->withdrawal_path)
+ fprintf(f, "%s\tWithdrawal Path: %s\n", prefix, u->withdrawal_path);
+
STRV_FOREACH(j, u->dropin_paths)
fprintf(f, "%s\tDropIn Path: %s\n", prefix, *j);
@@ -5709,9 +5713,10 @@ const char *unit_label_path(Unit *u) {
if (!p)
return NULL;
- /* If a unit is masked, then don't read the SELinux label of /dev/null, as that really makes no sense */
+ /* If a unit is masked, then don't read the SELinux label of /dev/null, as that really makes no sense.
+ * Instead try withdrawal path (which can be NULL) */
if (path_equal(p, "/dev/null"))
- return NULL;
+ return u->withdrawal_path;
return p;
}
diff --git a/src/core/unit.h b/src/core/unit.h
index c5d8170c92..5dac251d74 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -130,6 +130,7 @@ typedef struct Unit {
char *fragment_path; /* if loaded from a config file this is the primary path to it */
char *source_path; /* if converted, the source file */
+ char *withdrawal_path; /* if masked, path to withdrawal unit file (if existent) */
char **dropin_paths;
usec_t fragment_mtime;
diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index 28cd3c8600..2bf302650e 100644
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -195,20 +195,23 @@ int unit_file_build_name_map(
const LookupPaths *lp,
usec_t *cache_mtime,
Hashmap **ret_unit_ids_map,
+ Hashmap **ret_unit_withdrawal_map,
Hashmap **ret_unit_names_map,
Set **ret_path_cache) {
- /* Build two mappings: any name → main unit (i.e. the end result of symlink resolution), unit name →
- * all aliases (i.e. the entry for a given key is a a list of all names which point to this key). The
- * key is included in the value iff we saw a file or symlink with that name. In other words, if we
- * have a key, but it is not present in the value for itself, there was an alias pointing to it, but
- * the unit itself is not loadable.
+ /* Build three mappings:
+ * - any name → main unit (i.e. the end result of symlink resolution)
+ * - any name → withdrawal unit file path (in case the unit is maksed)
+ * - unit name → all aliases (i.e. the entry for a given key is a a list of all names which point to this key).
+ * The key is included in the value iff we saw a file or symlink with that name. In other words, if we
+ * have a key, but it is not present in the value for itself, there was an alias pointing to it, but
+ * the unit itself is not loadable.
*
* At the same, build a cache of paths where to find units.
*/
- _cleanup_hashmap_free_ Hashmap *ids = NULL, *names = NULL;
- _cleanup_set_free_free_ Set *paths = NULL;
+ _cleanup_hashmap_free_ Hashmap *ids = NULL, *withdrawals = NULL, *names = NULL;
+ _cleanup_set_free_free_ Set *paths = NULL, *masked_ids = NULL;
char **dir;
int r;
usec_t mtime = 0;
@@ -224,6 +227,10 @@ int unit_file_build_name_map(
return log_oom();
}
+ masked_ids = set_new(&string_hash_ops);
+ if (!masked_ids)
+ return log_oom();
+
STRV_FOREACH(dir, (char**) lp->search_path) {
struct dirent *de;
_cleanup_closedir_ DIR *d = NULL;
@@ -248,6 +255,7 @@ int unit_file_build_name_map(
_cleanup_free_ char *_filename_free = NULL, *simplified = NULL;
const char *suffix, *dst = NULL;
bool valid_unit_name;
+ bool revisit_masked;
valid_unit_name = unit_name_is_valid(de->d_name, UNIT_NAME_ANY);
@@ -276,8 +284,8 @@ int unit_file_build_name_map(
/* search_path is ordered by priority (highest first). If the name is already mapped
* to something (incl. itself), it means that we have already seen it, and we should
- * ignore it here. */
- if (hashmap_contains(ids, de->d_name))
+ * ignore it here. Except the unit is masked, then try to get the withdrawal path */
+ if ((revisit_masked = hashmap_contains(ids, de->d_name)) && !set_contains(masked_ids, de->d_name))
continue;
dirent_ensure_type(d, de);
@@ -349,10 +357,27 @@ int unit_file_build_name_map(
log_debug("%s: normal unit file: %s", __func__, dst);
}
- r = hashmap_put_strdup(&ids, de->d_name, dst);
- if (r < 0)
- return log_warning_errno(r, "Failed to add entry to hashmap (%s→%s): %m",
- de->d_name, dst);
+ if (revisit_masked) {
+ if (!path_equal(dst, "/dev/null")) {
+ r = hashmap_put_strdup(&withdrawals, de->d_name, dst);
+ if (r < 0)
+ return log_warning_errno(r, "Failed to add entry to hashmap (%s→%s): %m",
+ de->d_name, dst);
+
+ set_remove(masked_ids, de->d_name);
+ }
+ } else {
+ r = hashmap_put_strdup(&ids, de->d_name, dst);
+ if (r < 0)
+ return log_warning_errno(r, "Failed to add entry to hashmap (%s→%s): %m",
+ de->d_name, dst);
+
+ if (path_equal(dst, "/dev/null")) {
+ r = set_put_strdup(masked_ids, de->d_name);
+ if (r < 0)
+ return log_oom();
+ }
+ }
}
}
@@ -383,6 +408,7 @@ int unit_file_build_name_map(
if (cache_mtime)
*cache_mtime = mtime;
*ret_unit_ids_map = TAKE_PTR(ids);
+ *ret_unit_withdrawal_map = TAKE_PTR(withdrawals);
*ret_unit_names_map = TAKE_PTR(names);
if (ret_path_cache)
*ret_path_cache = TAKE_PTR(paths);
diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h
index 98ba677f3f..e116487e65 100644
--- a/src/shared/unit-file.h
+++ b/src/shared/unit-file.h
@@ -45,6 +45,7 @@ int unit_file_build_name_map(
const LookupPaths *lp,
usec_t *ret_time,
Hashmap **ret_unit_ids_map,
+ Hashmap **ret_unit_withdrawal_map,
Hashmap **ret_unit_names_map,
Set **ret_path_cache);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 20e0d453d2..497cca2ff8 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -2660,7 +2660,7 @@ static int unit_find_paths(
_cleanup_set_free_free_ Set *names = NULL;
if (!cached_name_map) {
- r = unit_file_build_name_map(lp, NULL, &cached_id_map, &cached_name_map, NULL);
+ r = unit_file_build_name_map(lp, NULL, &cached_id_map, NULL, &cached_name_map, NULL);
if (r < 0)
return r;
}
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index 6021164739..0671b75074 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -29,6 +29,7 @@ static void test_unit_validate_alias_symlink_and_warn(void) {
static void test_unit_file_build_name_map(char **ids) {
_cleanup_(lookup_paths_free) LookupPaths lp = {};
_cleanup_hashmap_free_ Hashmap *unit_ids = NULL;
+ _cleanup_hashmap_free_ Hashmap *unit_withdrawals = NULL;
_cleanup_hashmap_free_ Hashmap *unit_names = NULL;
Iterator i;
const char *k, *dst;
@@ -38,11 +39,14 @@ static void test_unit_file_build_name_map(char **ids) {
assert_se(lookup_paths_init(&lp, UNIT_FILE_SYSTEM, 0, NULL) >= 0);
- assert_se(unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL) == 1);
+ assert_se(unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_withdrawals, &unit_names, NULL) == 1);
HASHMAP_FOREACH_KEY(dst, k, unit_ids, i)
log_info("ids: %s → %s", k, dst);
+ HASHMAP_FOREACH_KEY(dst, k, unit_withdrawals, i)
+ log_info("withdrawals: %s → %s", k, dst);
+
HASHMAP_FOREACH_KEY(v, k, unit_names, i) {
_cleanup_free_ char *j = strv_join(v, ", ");
log_info("aliases: %s ← %s", k, j);
@@ -51,7 +55,7 @@ static void test_unit_file_build_name_map(char **ids) {
char buf[FORMAT_TIMESTAMP_MAX];
log_debug("Last modification time: %s", format_timestamp(buf, sizeof buf, mtime));
- r = unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL);
+ r = unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_withdrawals, &unit_names, NULL);
assert_se(IN_SET(r, 0, 1));
if (r == 0)
log_debug("Cache rebuild skipped based on mtime.");
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (2 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 3/8] core: bookkeeping withdrawal path of masked units Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 20:05 ` Stephen Smalley
2019-12-18 14:28 ` [RFC PATCH 5/8] core: make SELinux access permissions more distinct Christian Göttsche
` (4 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Add new SELinux permissions `modify` and `listdynusers` to the class `service`.
- modfiy checks altering operations, like `systemctl log-level debug` or `systemctl add-wants foo bar`.
- listdynusers checks obtaining dynamic users, which is very common on systems using nss-systemd.
Add a new permission to avoid undermining the `status` permission.
Perform SELinux access checks for the following D-Bus exposed methods:
D-Bus interface | c function name | SELinux permission verb
GetAfter / GetBefore | bus_job_method_get_waiting_jobs | status
LogTarget | property_set_log_target | modify
LogLevel | property_set_log_level | modify
RuntimeWatchdocUSec | property_set_runtime_watchdog | modify
ServiceWatchdogs | bus_property_set_bool_wrapper | modify
GetUnit | method_get_unit | status
GetUnitByPID | method_get_unit_by_pid | status
GetUnitByControlGroup | method_get_unit_by_control_group | status
LoadUnit | method_load_unit | status
ListUnitsByNames | method_list_units_by_names | status
LookupDynamicUserByName | method_lookup_dynamic_user_by_name | listdynusers
LookupDynamicUserByUID | method_lookup_dynamic_user_by_uid | listdynusers
GetDynamicUsers | method_get_dynamic_users | listdynusers
AddDependencyUnitFiles | method_add_dependency_unit_files | modify
GetUnitFileLinks | method_get_unit_file_links | status
Unref | bus_unit_method_unref | modify
---
src/core/dbus-job.c | 4 ++
src/core/dbus-manager.c | 89 ++++++++++++++++++++++++++++++++++++-----
src/core/dbus-unit.c | 4 ++
3 files changed, 88 insertions(+), 9 deletions(-)
diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
index a7d342257b..7b0b093757 100644
--- a/src/core/dbus-job.c
+++ b/src/core/dbus-job.c
@@ -71,6 +71,10 @@ int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_
Job *j = userdata;
int r, i, n;
+ r = mac_selinux_unit_access_check(j->unit, message, "status", error);
+ if (r < 0)
+ return r;
+
if (strstr(sd_bus_message_get_member(message), "After"))
n = job_get_after(j, &list);
else
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index c751e84253..d2db6e4a61 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -121,6 +121,10 @@ static int property_set_log_target(
assert(bus);
assert(value);
+ r = mac_selinux_access_check(value, "modify", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read(value, "s", &t);
if (r < 0)
return r;
@@ -178,6 +182,10 @@ static int property_set_log_level(
assert(bus);
assert(value);
+ r = mac_selinux_access_check(value, "modify", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read(value, "s", &t);
if (r < 0)
return r;
@@ -282,6 +290,10 @@ static int property_set_runtime_watchdog(
assert_cc(sizeof(usec_t) == sizeof(uint64_t));
+ r = mac_selinux_access_check(value, "modify", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read(value, "t", t);
if (r < 0)
return r;
@@ -289,6 +301,24 @@ static int property_set_runtime_watchdog(
return watchdog_set_timeout(t);
}
+static int bus_property_set_bool_wrapper(
+ sd_bus *bus,
+ const char *path,
+ const char *interface,
+ const char *property,
+ sd_bus_message *value,
+ void *userdata,
+ sd_bus_error *error) {
+
+ int r;
+
+ r = mac_selinux_access_check(value, "modify", error);
+ if (r < 0)
+ return r;
+
+ return bus_property_set_bool(bus, path, interface, property, value, userdata, error);
+}
+
static int bus_get_unit_by_name(Manager *m, sd_bus_message *message, const char *name, Unit **ret_unit, sd_bus_error *error) {
Unit *u;
int r;
@@ -375,6 +405,10 @@ static int method_get_unit(sd_bus_message *message, void *userdata, sd_bus_error
if (r < 0)
return r;
+ r = mac_selinux_unit_access_check(u, message, "status", error);
+ if (r < 0)
+ return r;
+
return reply_unit_path(u, message, error);
}
@@ -413,6 +447,10 @@ static int method_get_unit_by_pid(sd_bus_message *message, void *userdata, sd_bu
if (!u)
return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pid);
+ r = mac_selinux_unit_access_check(u, message, "status", error);
+ if (r < 0)
+ return r;
+
return reply_unit_path(u, message, error);
}
@@ -488,6 +526,10 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd
if (!u)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Control group '%s' is not valid or not managed by this instance", cgroup);
+ r = mac_selinux_unit_access_check(u, message, "status", error);
+ if (r < 0)
+ return r;
+
return reply_unit_path(u, message, error);
}
@@ -510,6 +552,10 @@ static int method_load_unit(sd_bus_message *message, void *userdata, sd_bus_erro
if (r < 0)
return r;
+ r = mac_selinux_unit_access_check(u, message, "status", error);
+ if (r < 0)
+ return r;
+
return reply_unit_path(u, message, error);
}
@@ -529,6 +575,7 @@ static int method_start_unit_generic(sd_bus_message *message, Manager *m, JobTyp
if (r < 0)
return r;
+ /* bus_unit_method_start_generic() includes a mac_selinux check */
return bus_unit_method_start_generic(message, u, job_type, reload_if_possible, error);
}
@@ -703,6 +750,10 @@ static int method_list_units_by_names(sd_bus_message *message, void *userdata, s
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "status", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read_strv(message, &units);
if (r < 0)
return r;
@@ -1263,11 +1314,11 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *
assert(message);
assert(m);
- r = verify_run_space("Refusing to reload", error);
+ r = mac_selinux_access_check(message, "reload", error);
if (r < 0)
return r;
- r = mac_selinux_access_check(message, "reload", error);
+ r = verify_run_space("Refusing to reload", error);
if (r < 0)
return r;
@@ -1299,11 +1350,11 @@ static int method_reexecute(sd_bus_message *message, void *userdata, sd_bus_erro
assert(message);
assert(m);
- r = verify_run_space("Refusing to reexecute", error);
+ r = mac_selinux_access_check(message, "reload", error);
if (r < 0)
return r;
- r = mac_selinux_access_check(message, "reload", error);
+ r = verify_run_space("Refusing to reexecute", error);
if (r < 0)
return r;
@@ -1428,6 +1479,10 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "reboot", error);
+ if (r < 0)
+ return r;
+
if (statvfs("/run/systemd", &svfs) < 0)
return sd_bus_error_set_errnof(error, errno, "Failed to statvfs(/run/systemd): %m");
@@ -1441,10 +1496,6 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
format_bytes(fb_need, sizeof(fb_need), RELOAD_DISK_SPACE_MIN));
}
- r = mac_selinux_access_check(message, "reboot", error);
- if (r < 0)
- return r;
-
if (!MANAGER_IS_SYSTEM(m))
return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Root switching is only supported by system manager.");
@@ -1636,6 +1687,10 @@ static int method_lookup_dynamic_user_by_name(sd_bus_message *message, void *use
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "listdynusers", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read_basic(message, 's', &name);
if (r < 0)
return r;
@@ -1663,6 +1718,10 @@ static int method_lookup_dynamic_user_by_uid(sd_bus_message *message, void *user
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "listdynusers", error);
+ if (r < 0)
+ return r;
+
assert_cc(sizeof(uid) == sizeof(uint32_t));
r = sd_bus_message_read_basic(message, 'u', &uid);
if (r < 0)
@@ -1692,6 +1751,10 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "listdynusers", error);
+ if (r < 0)
+ return r;
+
assert_cc(sizeof(uid_t) == sizeof(uint32_t));
if (!MANAGER_IS_SYSTEM(m))
@@ -2264,6 +2327,10 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
assert(message);
assert(m);
+ r = mac_selinux_access_check(message, "modify", error);
+ if (r < 0)
+ return r;
+
r = bus_verify_manage_unit_files_async(m, message, error);
if (r < 0)
return r;
@@ -2300,6 +2367,10 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
char **p;
int runtime, r;
+ r = mac_selinux_access_check(message, "status", error);
+ if (r < 0)
+ return r;
+
r = sd_bus_message_read(message, "sb", &name, &runtime);
if (r < 0)
return r;
@@ -2422,7 +2493,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
/* The following item is an obsolete alias */
SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, reboot_watchdog), SD_BUS_VTABLE_HIDDEN),
SD_BUS_WRITABLE_PROPERTY("KExecWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, kexec_watchdog), 0),
- SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool, offsetof(Manager, service_watchdogs), 0),
+ SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool_wrapper, offsetof(Manager, service_watchdogs), 0),
SD_BUS_PROPERTY("ControlGroup", "s", NULL, offsetof(Manager, cgroup_root), 0),
SD_BUS_PROPERTY("SystemState", "s", property_get_system_state, 0, 0),
SD_BUS_PROPERTY("ExitCode", "y", bus_property_get_unsigned, offsetof(Manager, return_value), 0),
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 9477c47140..f86efaac3a 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -645,6 +645,10 @@ int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error
assert(message);
assert(u);
+ r = mac_selinux_unit_access_check(u, message, "modify", error);
+ if (r < 0)
+ return r;
+
r = bus_unit_track_remove_sender(u, message);
if (r == -EUNATCH)
return sd_bus_error_setf(error, BUS_ERROR_NOT_REFERENCED, "Unit has not been referenced yet.");
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 5/8] core: make SELinux access permissions more distinct
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (3 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 6/8] core: add support for MAC checks on unit install operations Christian Göttsche
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Make access permissions more distinct by using the new introduced permission `modify`:
- method_set_environment : reload -> modify
- method_unset_environment : reload -> modify
- method_unset_and_set_environment : reload -> modify
- method_set_exit_code : exit -> modify
- bus_unit_method_set_properties : start -> modify
- bus_unit_method_ref : start -> modify
---
src/core/dbus-manager.c | 8 ++++----
src/core/dbus-unit.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index d2db6e4a61..cd66871d48 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1561,7 +1561,7 @@ static int method_set_environment(sd_bus_message *message, void *userdata, sd_bu
assert(message);
assert(m);
- r = mac_selinux_access_check(message, "reload", error);
+ r = mac_selinux_access_check(message, "modify", error);
if (r < 0)
return r;
@@ -1592,7 +1592,7 @@ static int method_unset_environment(sd_bus_message *message, void *userdata, sd_
assert(message);
assert(m);
- r = mac_selinux_access_check(message, "reload", error);
+ r = mac_selinux_access_check(message, "modify", error);
if (r < 0)
return r;
@@ -1624,7 +1624,7 @@ static int method_unset_and_set_environment(sd_bus_message *message, void *userd
assert(message);
assert(m);
- r = mac_selinux_access_check(message, "reload", error);
+ r = mac_selinux_access_check(message, "modify", error);
if (r < 0)
return r;
@@ -1662,7 +1662,7 @@ static int method_set_exit_code(sd_bus_message *message, void *userdata, sd_bus_
assert(message);
assert(m);
- r = mac_selinux_access_check(message, "exit", error);
+ r = mac_selinux_access_check(message, "modify", error);
if (r < 0)
return r;
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index f86efaac3a..37b5decf52 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -579,7 +579,7 @@ int bus_unit_method_set_properties(sd_bus_message *message, void *userdata, sd_b
assert(message);
assert(u);
- r = mac_selinux_unit_access_check(u, message, "start", error);
+ r = mac_selinux_unit_access_check(u, message, "modify", error);
if (r < 0)
return r;
@@ -614,7 +614,7 @@ int bus_unit_method_ref(sd_bus_message *message, void *userdata, sd_bus_error *e
assert(message);
assert(u);
- r = mac_selinux_unit_access_check(u, message, "start", error);
+ r = mac_selinux_unit_access_check(u, message, "modify", error);
if (r < 0)
return r;
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 6/8] core: add support for MAC checks on unit install operations
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (4 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 5/8] core: make SELinux access permissions more distinct Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 7/8] core: implement the sd-bus generic callback for SELinux Christian Göttsche
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
shared:
Add a MAC callback call to unit_file_* functions to check unit operations, like enable, 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 a NULL pointer.
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 MAC checks (like write, unlink...) on the installed unit file (in /lib/systemd/system) or temporary one (in /etc/systemd/system) will be performed.
---
src/core/dbus-manager.c | 41 ++++++++++----
src/core/manager.c | 2 +-
src/shared/install.c | 101 +++++++++++++++++++++++++++++------
src/shared/install.h | 42 +++++++++++----
src/systemctl/systemctl.c | 26 ++++-----
src/test/test-install-root.c | 86 ++++++++++++++---------------
src/test/test-install.c | 38 ++++++-------
7 files changed, 224 insertions(+), 112 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index cd66871d48..b866fd58b6 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -2062,10 +2062,22 @@ fail:
return r;
}
+struct mac_callback_userdata {
+};
+
+static int mac_callback_check(const char *name, void *userdata) {
+ struct mac_callback_userdata *ud = userdata;
+
+ assert(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) {
@@ -2074,6 +2086,7 @@ static int method_enable_unit_files_generic(
size_t n_changes = 0;
UnitFileFlags flags;
int runtime, force, r;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2094,7 +2107,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_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2113,8 +2126,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) {
@@ -2135,6 +2148,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
int runtime, force, r;
UnitFileFlags flags;
const char *mode;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2163,7 +2177,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, mm, &changes, &n_changes);
+ r = unit_file_preset(m->unit_file_scope, flags, NULL, l, mm, &changes, &n_changes, mac_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2173,13 +2187,14 @@ 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;
UnitFileChange *changes = NULL;
size_t n_changes = 0;
int r, runtime;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2198,7 +2213,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, runtime ? UNIT_FILE_RUNTIME : 0, NULL, l, &changes, &n_changes);
+ r = call(m->unit_file_scope, runtime ? UNIT_FILE_RUNTIME : 0, NULL, l, &changes, &n_changes, mac_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2219,6 +2234,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
size_t n_changes = 0;
Manager *m = userdata;
int r;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2233,7 +2249,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_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2279,6 +2295,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
const char *mode;
UnitFileFlags flags;
int force, runtime, r;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2307,7 +2324,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, mm, &changes, &n_changes);
+ r = unit_file_preset_all(m->unit_file_scope, flags, NULL, mm, &changes, &n_changes, mac_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2323,6 +2340,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
char *target, *type;
UnitDependency dep;
UnitFileFlags flags;
+ struct mac_callback_userdata mcud = {};
assert(message);
assert(m);
@@ -2351,7 +2369,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_callback_check, &mcud);
if (r < 0)
return install_error(error, r, changes, n_changes);
@@ -2366,6 +2384,7 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
const char *name;
char **p;
int runtime, r;
+ struct mac_callback_userdata mcud = {};
r = mac_selinux_access_check(message, "status", error);
if (r < 0)
@@ -2387,7 +2406,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_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 7d8015e211..0b6ef09924 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1569,7 +1569,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 0e673b3358..3a2d89b9f4 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -2010,13 +2010,26 @@ static int install_context_mark_for_removal(
return 0;
}
+static int do_mac_check(
+ mac_callback_t mac_check,
+ const char *name,
+ void *userdata) {
+
+ if (!mac_check)
+ return 0;
+
+ return mac_check(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;
@@ -2044,6 +2057,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;
@@ -2062,7 +2079,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;
@@ -2092,6 +2111,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;
@@ -2156,7 +2179,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;
@@ -2188,6 +2213,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;
@@ -2256,7 +2285,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 = {};
@@ -2287,6 +2318,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;
@@ -2413,7 +2448,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 = {};
@@ -2447,6 +2484,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;
@@ -2457,6 +2498,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. */
@@ -2481,7 +2526,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 = {};
@@ -2508,6 +2555,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
@@ -2524,7 +2575,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 = {};
@@ -2548,6 +2601,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, false, NULL);
if (r < 0)
return r;
@@ -2566,7 +2623,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;
@@ -2579,12 +2638,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(
@@ -3090,7 +3149,9 @@ static int preset_prepare_one(
const char *name,
Presets 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;
@@ -3104,6 +3165,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;
@@ -3143,7 +3209,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 = {};
@@ -3169,7 +3237,8 @@ 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;
}
@@ -3183,7 +3252,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 = {};
@@ -3231,7 +3302,7 @@ int unit_file_preset_all(
continue;
/* we don't pass changes[] in, because we want to handle errors on our own */
- r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, presets, NULL, 0);
+ r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, presets, NULL, 0, mac_check, userdata);
if (r == -ERFKILL)
r = unit_file_changes_add(changes, n_changes,
UNIT_FILE_IS_MASKED, de->d_name, NULL);
diff --git a/src/shared/install.h b/src/shared/install.h
index b2ad9c4a71..4ad60c5dd8 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -87,27 +87,35 @@ struct UnitFileInstallInfo {
bool auxiliary;
};
+typedef int (*mac_callback_t)(const char *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,
@@ -115,41 +123,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,
@@ -169,7 +189,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.c b/src/systemctl/systemctl.c
index 497cca2ff8..1304ad1f25 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -6899,23 +6899,23 @@ static int enable_unit(int argc, char *argv[], void *userdata) {
flags = args_to_flags();
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);
- else if (streq(verb, "preset")) {
- r = unit_file_preset(arg_scope, flags, arg_root, names, arg_preset_mode, &changes, &n_changes);
- } else if (streq(verb, "mask"))
- r = unit_file_mask(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, NULL, NULL);
+ else if (streq(verb, "mask"))
+ 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("Unknown verb");
@@ -7112,7 +7112,7 @@ static int add_dependency(int argc, char *argv[], void *userdata) {
assert_not_reached("Unknown verb");
if (install_client_side()) {
- r = unit_file_add_dependency(arg_scope, args_to_flags(), arg_root, names, target, dep, &changes, &n_changes);
+ r = unit_file_add_dependency(arg_scope, args_to_flags(), 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)
@@ -7174,7 +7174,7 @@ static int preset_all(int argc, char *argv[], void *userdata) {
int r;
if (install_client_side()) {
- r = unit_file_preset_all(arg_scope, args_to_flags(), arg_root, arg_preset_mode, &changes, &n_changes);
+ r = unit_file_preset_all(arg_scope, args_to_flags(), arg_root, arg_preset_mode, &changes, &n_changes, NULL, NULL);
unit_file_dump_changes(r, "preset", changes, n_changes, arg_quiet);
if (r > 0)
@@ -7234,7 +7234,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/test/test-install-root.c b/src/test/test-install-root.c
index 323e1124ba..7f8d534106 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -51,7 +51,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_DISABLED);
- 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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/dev/null"));
@@ -67,11 +67,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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/a.service");
@@ -79,7 +79,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
@@ -94,12 +94,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_ENABLED);
/* 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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/a.service");
@@ -113,13 +113,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_DISABLED);
/* 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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
@@ -135,7 +135,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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/a.service");
@@ -204,7 +204,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/opt/linked.service"));
@@ -216,7 +216,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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/linked.service");
@@ -227,7 +227,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_PATH"/multi-user.target.wants/linked.service");
q = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/linked.service");
@@ -249,7 +249,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_PATH"/multi-user.target.wants/linked.service");
q = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/linked.service");
@@ -269,7 +269,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_PATH"/multi-user.target.wants/linked2.service");
q = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/linked2.service");
@@ -288,7 +288,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 == UNIT_FILE_SYMLINK);
assert_se(startswith(changes[0].path, root));
@@ -351,7 +351,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/real-add-dependency-test-service.service"));
@@ -392,7 +392,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
@@ -408,7 +408,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 == UNIT_FILE_UNLINK);
assert_se(streq(changes[0].path, p));
@@ -424,7 +424,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/template@foo.service");
@@ -439,7 +439,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 == UNIT_FILE_UNLINK);
assert_se(streq(changes[0].path, p));
@@ -457,7 +457,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/template@quux.service");
@@ -502,7 +502,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_INDIRECT);
- 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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/indirectb.service"));
@@ -515,7 +515,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_INDIRECT);
- 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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/indirectb.service");
@@ -555,7 +555,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/preset-yes.service"));
@@ -567,7 +567,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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/preset-yes.service");
@@ -578,7 +578,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;
@@ -586,7 +586,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);
@@ -650,7 +650,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;
@@ -659,7 +659,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 == UNIT_FILE_UNLINK);
assert_se(streq(changes[0].path, p));
@@ -671,7 +671,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 == UNIT_FILE_UNLINK);
assert_se(streq(changes[0].path, p));
@@ -711,7 +711,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/prefix-1.service"));
@@ -723,7 +723,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);
@@ -823,7 +823,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 == UNIT_FILE_SYMLINK);
assert_se(changes[1].type == UNIT_FILE_SYMLINK);
@@ -836,7 +836,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 == UNIT_FILE_SYMLINK);
@@ -850,7 +850,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 == UNIT_FILE_SYMLINK);
@@ -864,7 +864,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 == UNIT_FILE_SYMLINK);
@@ -935,7 +935,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 == UNIT_FILE_SYMLINK);
assert_se(changes[1].type == UNIT_FILE_SYMLINK);
@@ -948,7 +948,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 == UNIT_FILE_SYMLINK);
assert_se(changes[1].type == UNIT_FILE_SYMLINK);
@@ -961,7 +961,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-2@.service"));
@@ -970,7 +970,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 == UNIT_FILE_SYMLINK);
assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-3@.service"));
@@ -1010,7 +1010,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 == UNIT_FILE_SYMLINK);
@@ -1019,7 +1019,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 == UNIT_FILE_UNLINK);
p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/foo@bar0.service");
@@ -1032,7 +1032,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 62daaccd62..5c09a428f0 100644
--- a/src/test/test-install.c
+++ b/src/test/test-install.c
@@ -53,12 +53,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);
@@ -72,7 +72,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);
@@ -86,10 +86,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);
@@ -103,10 +103,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);
@@ -120,7 +120,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);
@@ -134,10 +134,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);
@@ -151,7 +151,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);
@@ -165,7 +165,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);
@@ -179,7 +179,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);
@@ -192,7 +192,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);
@@ -206,7 +206,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);
@@ -219,7 +219,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);
@@ -233,7 +233,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);
@@ -247,7 +247,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);
@@ -259,7 +259,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.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 7/8] core: implement the sd-bus generic callback for SELinux
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (5 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 6/8] core: add support for MAC checks on unit install operations Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 8/8] core: add notes to D-Bus interfaces about adding SELinux checks Christian Göttsche
2019-12-19 20:24 ` [RFC PATCH 0/8] systemd: improve SELinux support Chris PeBenito
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
Also keep the SELinux class `service` in `mac_selinux_generic_access_check()` in the case no path is given.
Previously this could mainly happen on a mask unit, which is fixed now.
Changing the class, but keeping the permission, is unpleasant as the class `system` must otherwise define always all permissions of `service` and a unit operation resulting in a system class-based check is odd.
The fallback target context remains the process context of systemd-pid1.
---
src/core/dbus-manager.c | 37 +++++++++++++++++++++-------------
src/core/manager.c | 7 +++++++
src/core/manager.h | 1 +
src/core/selinux-access.c | 42 ++++++++++++++++++++++++++++++++++-----
src/core/selinux-access.h | 28 +++++++++++++++++++++++---
src/core/unit.c | 4 +++-
src/core/unit.h | 2 +-
7 files changed, 97 insertions(+), 24 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index b866fd58b6..14085ba1a1 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -2063,14 +2063,20 @@ fail:
}
struct mac_callback_userdata {
+ struct mac_selinux_callback_userdata selinux;
};
static int mac_callback_check(const char *name, void *userdata) {
struct mac_callback_userdata *ud = userdata;
+ int r;
assert(name);
assert(ud);
+ r = mac_selinux_callback_check(name, &(ud->selinux));
+ if (r < 0)
+ return r;
+
return 0;
}
@@ -2079,6 +2085,7 @@ 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_verb,
sd_bus_error *error) {
_cleanup_strv_free_ char **l = NULL;
@@ -2086,7 +2093,7 @@ static int method_enable_unit_files_generic(
size_t n_changes = 0;
UnitFileFlags flags;
int runtime, force, r;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, mac_selinux_verb } };
assert(message);
assert(m);
@@ -2115,15 +2122,15 @@ static int method_enable_unit_files_generic(
}
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, MAC_SELINUX_UNIT_PERM_ENABLE, 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, MAC_SELINUX_UNIT_PERM_REENABLE, 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, MAC_SELINUX_UNIT_PERM_LINK, 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) {
@@ -2131,11 +2138,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, MAC_SELINUX_UNIT_PERM_PRESET, 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, MAC_SELINUX_UNIT_PERM_MASK, error);
}
static int method_preset_unit_files_with_mode(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2148,7 +2155,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
int runtime, force, r;
UnitFileFlags flags;
const char *mode;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, MAC_SELINUX_UNIT_PERM_PRESET } };
assert(message);
assert(m);
@@ -2188,13 +2195,14 @@ 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_verb,
sd_bus_error *error) {
_cleanup_strv_free_ char **l = NULL;
UnitFileChange *changes = NULL;
size_t n_changes = 0;
int r, runtime;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, mac_selinux_verb } };
assert(message);
assert(m);
@@ -2221,11 +2229,11 @@ static int method_disable_unit_files_generic(
}
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, MAC_SELINUX_UNIT_PERM_DISABLE, 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, MAC_SELINUX_UNIT_PERM_UNMASK, error);
}
static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2234,7 +2242,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
size_t n_changes = 0;
Manager *m = userdata;
int r;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, MAC_SELINUX_UNIT_PERM_REVERT } };
assert(message);
assert(m);
@@ -2295,7 +2303,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
const char *mode;
UnitFileFlags flags;
int force, runtime, r;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, MAC_SELINUX_UNIT_PERM_PRESET } };
assert(message);
assert(m);
@@ -2340,7 +2348,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
char *target, *type;
UnitDependency dep;
UnitFileFlags flags;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, MAC_SELINUX_UNIT_PERM_ADDDEPENDENCY } };
assert(message);
assert(m);
@@ -2377,6 +2385,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;
@@ -2384,7 +2393,7 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
const char *name;
char **p;
int runtime, r;
- struct mac_callback_userdata mcud = {};
+ struct mac_callback_userdata mcud = { .selinux = { m, message, error, MAC_SELINUX_UNIT_PERM_STATUS } };
r = mac_selinux_access_check(message, "status", error);
if (r < 0)
diff --git a/src/core/manager.c b/src/core/manager.c
index 0b6ef09924..0f69ba12fd 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1868,6 +1868,13 @@ Unit *manager_get_unit(Manager *m, const char *name) {
return hashmap_get(m->units, name);
}
+const char *manager_get_unitpath(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;
unsigned k;
diff --git a/src/core/manager.h b/src/core/manager.h
index fb1a29d88c..acd3762744 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -452,6 +452,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_get_unitpath(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 4500e4452f..d3b89c1d7a 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -168,6 +168,33 @@ static int access_init(sd_bus_error *error) {
return 1;
}
+int mac_selinux_callback_check(
+ const char *name,
+ struct mac_selinux_callback_userdata *userdata) {
+ const Unit *u;
+ const char *path = NULL;
+
+ assert(name);
+ assert(userdata);
+ assert(userdata->m);
+ assert(userdata->message);
+ assert(userdata->error);
+ assert(userdata->permission);
+
+ if (!mac_selinux_use())
+ return 0;
+
+ u = manager_get_unit(userdata->m, 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_get_unitpath(userdata->m, name);
+
+ return mac_selinux_generic_access_check(userdata->message, path, MAC_SELINUX_CLASS_UNIT, userdata->permission, userdata->error);
+}
+
/*
This function communicates with the kernel to check whether or not it should
allow the access.
@@ -177,11 +204,12 @@ static int access_init(sd_bus_error *error) {
int mac_selinux_generic_access_check(
sd_bus_message *message,
const char *path,
+ const char *tclass,
const char *permission,
sd_bus_error *error) {
_cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
- const char *tclass = NULL, *scon = NULL;
+ const char *scon = NULL;
struct audit_info audit_info = {};
_cleanup_free_ char *cl = NULL;
char *fcon = NULL;
@@ -227,8 +255,6 @@ int mac_selinux_generic_access_check(
r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get file context on %s.", path);
goto finish;
}
-
- tclass = "service";
} else {
r = getcon_raw(&fcon);
if (r < 0) {
@@ -236,8 +262,6 @@ int mac_selinux_generic_access_check(
r = sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to get current context.");
goto finish;
}
-
- tclass = "system";
}
sd_bus_creds_get_cmdline(creds, &cmdline);
@@ -269,10 +293,18 @@ finish:
int mac_selinux_generic_access_check(
sd_bus_message *message,
const char *path,
+ const char *tclass,
const char *permission,
sd_bus_error *error) {
return 0;
}
+int mac_selinux_callback_check(
+ const char *name,
+ struct mac_selinux_callback_userdata *userdata) {
+
+ return 0;
+}
+
#endif
diff --git a/src/core/selinux-access.h b/src/core/selinux-access.h
index 1e75930f57..cfdeded59b 100644
--- a/src/core/selinux-access.h
+++ b/src/core/selinux-access.h
@@ -6,15 +6,37 @@
#include "bus-util.h"
#include "manager.h"
-int mac_selinux_generic_access_check(sd_bus_message *message, const char *path, const char *permission, sd_bus_error *error);
+#define MAC_SELINUX_CLASS_SYSTEM "system"
+#define MAC_SELINUX_CLASS_UNIT "service"
+
+#define MAC_SELINUX_UNIT_PERM_ADDDEPENDENCY "modify"
+#define MAC_SELINUX_UNIT_PERM_DISABLE "disable"
+#define MAC_SELINUX_UNIT_PERM_ENABLE "enable"
+#define MAC_SELINUX_UNIT_PERM_LINK "enable"
+#define MAC_SELINUX_UNIT_PERM_MASK "disable"
+#define MAC_SELINUX_UNIT_PERM_PRESET "modify"
+#define MAC_SELINUX_UNIT_PERM_REENABLE "enable"
+#define MAC_SELINUX_UNIT_PERM_REVERT "modify"
+#define MAC_SELINUX_UNIT_PERM_STATUS "status"
+#define MAC_SELINUX_UNIT_PERM_UNMASK "enable"
+
+struct mac_selinux_callback_userdata {
+ Manager *m;
+ sd_bus_message *message;
+ sd_bus_error *error;
+ const char *permission;
+};
+int mac_selinux_callback_check(const char *name, struct mac_selinux_callback_userdata *userdata);
+
+int mac_selinux_generic_access_check(sd_bus_message *message, const char *path, const char *tclass, const char *permission, sd_bus_error *error);
#if HAVE_SELINUX
#define mac_selinux_access_check(message, permission, error) \
- mac_selinux_generic_access_check((message), NULL, (permission), (error))
+ mac_selinux_generic_access_check((message), NULL, MAC_SELINUX_CLASS_SYSTEM, (permission), (error))
#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), MAC_SELINUX_CLASS_UNIT, (permission), (error))
#else
diff --git a/src/core/unit.c b/src/core/unit.c
index f9b299c8a4..1d5c702020 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5703,9 +5703,11 @@ bool unit_needs_console(Unit *u) {
return exec_context_may_touch_console(ec);
}
-const char *unit_label_path(Unit *u) {
+const char *unit_label_path(const Unit *u) {
const char *p;
+ assert(u);
+
/* Returns the file system path to use for MAC access decisions, i.e. the file to read the SELinux label off
* when validating access checks. */
diff --git a/src/core/unit.h b/src/core/unit.h
index 5dac251d74..821c7b3c84 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -839,7 +839,7 @@ int unit_warn_leftover_processes(Unit *u);
bool unit_needs_console(Unit *u);
-const char *unit_label_path(Unit *u);
+const char *unit_label_path(const Unit *u);
int unit_pid_attachable(Unit *unit, pid_t pid, sd_bus_error *error);
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC PATCH 8/8] core: add notes to D-Bus interfaces about adding SELinux checks
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (6 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 7/8] core: implement the sd-bus generic callback for SELinux Christian Göttsche
@ 2019-12-18 14:28 ` Christian Göttsche
2019-12-19 20:24 ` [RFC PATCH 0/8] systemd: improve SELinux support Chris PeBenito
8 siblings, 0 replies; 16+ messages in thread
From: Christian Göttsche @ 2019-12-18 14:28 UTC (permalink / raw)
To: selinux
---
src/core/dbus-automount.c | 3 +++
src/core/dbus-cgroup.c | 3 +++
src/core/dbus-device.c | 3 +++
src/core/dbus-execute.c | 3 +++
src/core/dbus-job.c | 3 +++
src/core/dbus-kill.c | 3 +++
src/core/dbus-manager.c | 3 +++
src/core/dbus-mount.c | 3 +++
src/core/dbus-path.c | 3 +++
src/core/dbus-scope.c | 3 +++
src/core/dbus-service.c | 3 +++
src/core/dbus-slice.c | 3 +++
src/core/dbus-socket.c | 3 +++
src/core/dbus-swap.c | 3 +++
src/core/dbus-target.c | 3 +++
src/core/dbus-timer.c | 3 +++
src/core/dbus-unit.c | 6 ++++++
17 files changed, 54 insertions(+)
diff --git a/src/core/dbus-automount.c b/src/core/dbus-automount.c
index bd6e6a9dde..129ef98df1 100644
--- a/src/core/dbus-automount.c
+++ b/src/core/dbus-automount.c
@@ -8,6 +8,9 @@
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, automount_result, AutomountResult);
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_automount_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Where", "s", NULL, offsetof(Automount, where), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c
index 27dc9e43c3..ad277e94c9 100644
--- a/src/core/dbus-cgroup.c
+++ b/src/core/dbus-cgroup.c
@@ -344,6 +344,9 @@ static int property_get_ip_address_access(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_cgroup_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Delegate", "b", bus_property_get_bool, offsetof(CGroupContext, delegate), 0),
diff --git a/src/core/dbus-device.c b/src/core/dbus-device.c
index 6cf7f58e02..c2566c274b 100644
--- a/src/core/dbus-device.c
+++ b/src/core/dbus-device.c
@@ -4,6 +4,9 @@
#include "device.h"
#include "unit.h"
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_device_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("SysFSPath", "s", NULL, offsetof(Device, sysfs), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 1d0bc1ede3..67bc91c3c6 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -692,6 +692,9 @@ static int property_get_log_extra_fields(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_exec_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Environment", "as", NULL, offsetof(ExecContext, environment), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
index 7b0b093757..ca531570ba 100644
--- a/src/core/dbus-job.c
+++ b/src/core/dbus-job.c
@@ -119,6 +119,9 @@ int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_
return sd_bus_send(NULL, reply, NULL);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_job_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_METHOD("Cancel", NULL, NULL, bus_job_method_cancel, SD_BUS_VTABLE_UNPRIVILEGED),
diff --git a/src/core/dbus-kill.c b/src/core/dbus-kill.c
index 30597e86f0..e7d0d2b16c 100644
--- a/src/core/dbus-kill.c
+++ b/src/core/dbus-kill.c
@@ -25,6 +25,9 @@ static int property_get_restart_kill_signal(
return sd_bus_message_append_basic(reply, 'i', &s);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_kill_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("KillMode", "s", property_get_kill_mode, offsetof(KillContext, kill_mode), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 14085ba1a1..721ebeeaa8 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -2476,6 +2476,9 @@ static int method_abandon_scope(sd_bus_message *message, void *userdata, sd_bus_
return bus_scope_method_abandon(message, u, error);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_manager_vtable[] = {
SD_BUS_VTABLE_START(0),
diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c
index b6d61627eb..1fface5532 100644
--- a/src/core/dbus-mount.c
+++ b/src/core/dbus-mount.c
@@ -39,6 +39,9 @@ static BUS_DEFINE_PROPERTY_GET(property_get_options, "s", Mount, mount_get_optio
static BUS_DEFINE_PROPERTY_GET(property_get_type, "s", Mount, mount_get_fstype);
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, mount_result, MountResult);
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_mount_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Where", "s", NULL, offsetof(Mount, where), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-path.c b/src/core/dbus-path.c
index 1a97d62486..9f53c4bd1e 100644
--- a/src/core/dbus-path.c
+++ b/src/core/dbus-path.c
@@ -42,6 +42,9 @@ static int property_get_paths(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_path_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Unit", "s", bus_property_get_triggered_unit, 0, SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-scope.c b/src/core/dbus-scope.c
index 84d91dcfa3..740a09874d 100644
--- a/src/core/dbus-scope.c
+++ b/src/core/dbus-scope.c
@@ -42,6 +42,9 @@ int bus_scope_method_abandon(sd_bus_message *message, void *userdata, sd_bus_err
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, scope_result, ScopeResult);
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_scope_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Controller", "s", NULL, offsetof(Scope, controller), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 5cf9b21890..ef912577ab 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -91,6 +91,9 @@ static int property_get_exit_status_set(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_service_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Type", "s", property_get_type, offsetof(Service, type), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-slice.c b/src/core/dbus-slice.c
index effd5fa5d7..db49933ae3 100644
--- a/src/core/dbus-slice.c
+++ b/src/core/dbus-slice.c
@@ -5,6 +5,9 @@
#include "slice.h"
#include "unit.h"
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_slice_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_VTABLE_END
diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c
index 25d3d71391..15ba47091d 100644
--- a/src/core/dbus-socket.c
+++ b/src/core/dbus-socket.c
@@ -74,6 +74,9 @@ static int property_get_listen(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_socket_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("BindIPv6Only", "s", property_get_bind_ipv6_only, offsetof(Socket, bind_ipv6_only), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c
index 353fa20132..de4a0b0e55 100644
--- a/src/core/dbus-swap.c
+++ b/src/core/dbus-swap.c
@@ -29,6 +29,9 @@ static BUS_DEFINE_PROPERTY_GET(property_get_priority, "i", Swap, swap_get_priori
static BUS_DEFINE_PROPERTY_GET(property_get_options, "s", Swap, swap_get_options);
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, swap_result, SwapResult);
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_swap_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("What", "s", NULL, offsetof(Swap, what), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
diff --git a/src/core/dbus-target.c b/src/core/dbus-target.c
index ba50113641..7c61f7d6fd 100644
--- a/src/core/dbus-target.c
+++ b/src/core/dbus-target.c
@@ -3,6 +3,9 @@
#include "dbus-target.h"
#include "unit.h"
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_target_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_VTABLE_END
diff --git a/src/core/dbus-timer.c b/src/core/dbus-timer.c
index 439c276fac..354336e4a3 100644
--- a/src/core/dbus-timer.c
+++ b/src/core/dbus-timer.c
@@ -118,6 +118,9 @@ static int property_get_next_elapse_monotonic(
TIMER_MONOTONIC_CLOCK(t), CLOCK_MONOTONIC));
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_timer_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Unit", "s", bus_property_get_triggered_unit, 0, SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 37b5decf52..184405d8be 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -764,6 +764,9 @@ static int property_get_refs(
return sd_bus_message_close_container(reply);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_unit_vtable[] = {
SD_BUS_VTABLE_START(0),
@@ -1350,6 +1353,9 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd
return sd_bus_reply_method_return(message, NULL);
}
+/* Note: when adding a SD_BUS_WRITABLE_PROPERTY or SD_BUS_METHOD add a TODO(selinux),
+ * so the SELinux people can add a permission check.
+ */
const sd_bus_vtable bus_unit_cgroup_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("Slice", "s", property_get_slice, 0, 0),
--
2.24.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods
2019-12-18 14:28 ` [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods Christian Göttsche
@ 2019-12-18 20:05 ` Stephen Smalley
2019-12-19 14:49 ` Christian Göttsche
0 siblings, 1 reply; 16+ messages in thread
From: Stephen Smalley @ 2019-12-18 20:05 UTC (permalink / raw)
To: Christian Göttsche, selinux
On 12/18/19 9:28 AM, Christian Göttsche wrote:
> Add new SELinux permissions `modify` and `listdynusers` to the class `service`.
> - modfiy checks altering operations, like `systemctl log-level debug` or `systemctl add-wants foo bar`.
> - listdynusers checks obtaining dynamic users, which is very common on systems using nss-systemd.
> Add a new permission to avoid undermining the `status` permission.
>
> Perform SELinux access checks for the following D-Bus exposed methods:
>
> D-Bus interface | c function name | SELinux permission verb
>
> GetAfter / GetBefore | bus_job_method_get_waiting_jobs | status
> LogTarget | property_set_log_target | modify
> LogLevel | property_set_log_level | modify
> RuntimeWatchdocUSec | property_set_runtime_watchdog | modify
> ServiceWatchdogs | bus_property_set_bool_wrapper | modify
> GetUnit | method_get_unit | status
> GetUnitByPID | method_get_unit_by_pid | status
> GetUnitByControlGroup | method_get_unit_by_control_group | status
> LoadUnit | method_load_unit | status
> ListUnitsByNames | method_list_units_by_names | status
> LookupDynamicUserByName | method_lookup_dynamic_user_by_name | listdynusers
> LookupDynamicUserByUID | method_lookup_dynamic_user_by_uid | listdynusers
> GetDynamicUsers | method_get_dynamic_users | listdynusers
> AddDependencyUnitFiles | method_add_dependency_unit_files | modify
> GetUnitFileLinks | method_get_unit_file_links | status
> Unref | bus_unit_method_unref | modify
If we are going to introduce new permissions or change existing ones, we
may want to consider just defining a separate permission for every
logical interface. Then we can let the policy writer decide what
matters to them and at what granularity they want to distinguish things,
using macros/interfaces as appropriate to avoid needing to specify them
all individually.
Also, you may want to leverage the policy capability mechanism in
userspace to permit compatible evolution of permission checks in the
same manner it is presently used in the kernel for
extended_socket_class, network_peer_controls, open_perms,
nnp_nosuid_transition, etc.
> ---
> src/core/dbus-job.c | 4 ++
> src/core/dbus-manager.c | 89 ++++++++++++++++++++++++++++++++++++-----
> src/core/dbus-unit.c | 4 ++
> 3 files changed, 88 insertions(+), 9 deletions(-)
>
> diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
> index a7d342257b..7b0b093757 100644
> --- a/src/core/dbus-job.c
> +++ b/src/core/dbus-job.c
> @@ -71,6 +71,10 @@ int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_
> Job *j = userdata;
> int r, i, n;
>
> + r = mac_selinux_unit_access_check(j->unit, message, "status", error);
> + if (r < 0)
> + return r;
> +
> if (strstr(sd_bus_message_get_member(message), "After"))
> n = job_get_after(j, &list);
> else
> diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
> index c751e84253..d2db6e4a61 100644
> --- a/src/core/dbus-manager.c
> +++ b/src/core/dbus-manager.c
> @@ -121,6 +121,10 @@ static int property_set_log_target(
> assert(bus);
> assert(value);
>
> + r = mac_selinux_access_check(value, "modify", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read(value, "s", &t);
> if (r < 0)
> return r;
> @@ -178,6 +182,10 @@ static int property_set_log_level(
> assert(bus);
> assert(value);
>
> + r = mac_selinux_access_check(value, "modify", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read(value, "s", &t);
> if (r < 0)
> return r;
> @@ -282,6 +290,10 @@ static int property_set_runtime_watchdog(
>
> assert_cc(sizeof(usec_t) == sizeof(uint64_t));
>
> + r = mac_selinux_access_check(value, "modify", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read(value, "t", t);
> if (r < 0)
> return r;
> @@ -289,6 +301,24 @@ static int property_set_runtime_watchdog(
> return watchdog_set_timeout(t);
> }
>
> +static int bus_property_set_bool_wrapper(
> + sd_bus *bus,
> + const char *path,
> + const char *interface,
> + const char *property,
> + sd_bus_message *value,
> + void *userdata,
> + sd_bus_error *error) {
> +
> + int r;
> +
> + r = mac_selinux_access_check(value, "modify", error);
> + if (r < 0)
> + return r;
> +
> + return bus_property_set_bool(bus, path, interface, property, value, userdata, error);
> +}
> +
> static int bus_get_unit_by_name(Manager *m, sd_bus_message *message, const char *name, Unit **ret_unit, sd_bus_error *error) {
> Unit *u;
> int r;
> @@ -375,6 +405,10 @@ static int method_get_unit(sd_bus_message *message, void *userdata, sd_bus_error
> if (r < 0)
> return r;
>
> + r = mac_selinux_unit_access_check(u, message, "status", error);
> + if (r < 0)
> + return r;
> +
> return reply_unit_path(u, message, error);
> }
>
> @@ -413,6 +447,10 @@ static int method_get_unit_by_pid(sd_bus_message *message, void *userdata, sd_bu
> if (!u)
> return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pid);
>
> + r = mac_selinux_unit_access_check(u, message, "status", error);
> + if (r < 0)
> + return r;
> +
> return reply_unit_path(u, message, error);
> }
>
> @@ -488,6 +526,10 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd
> if (!u)
> return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Control group '%s' is not valid or not managed by this instance", cgroup);
>
> + r = mac_selinux_unit_access_check(u, message, "status", error);
> + if (r < 0)
> + return r;
> +
> return reply_unit_path(u, message, error);
> }
>
> @@ -510,6 +552,10 @@ static int method_load_unit(sd_bus_message *message, void *userdata, sd_bus_erro
> if (r < 0)
> return r;
>
> + r = mac_selinux_unit_access_check(u, message, "status", error);
> + if (r < 0)
> + return r;
> +
> return reply_unit_path(u, message, error);
> }
>
> @@ -529,6 +575,7 @@ static int method_start_unit_generic(sd_bus_message *message, Manager *m, JobTyp
> if (r < 0)
> return r;
>
> + /* bus_unit_method_start_generic() includes a mac_selinux check */
> return bus_unit_method_start_generic(message, u, job_type, reload_if_possible, error);
> }
>
> @@ -703,6 +750,10 @@ static int method_list_units_by_names(sd_bus_message *message, void *userdata, s
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "status", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read_strv(message, &units);
> if (r < 0)
> return r;
> @@ -1263,11 +1314,11 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *
> assert(message);
> assert(m);
>
> - r = verify_run_space("Refusing to reload", error);
> + r = mac_selinux_access_check(message, "reload", error);
> if (r < 0)
> return r;
>
> - r = mac_selinux_access_check(message, "reload", error);
> + r = verify_run_space("Refusing to reload", error);
> if (r < 0)
> return r;
>
> @@ -1299,11 +1350,11 @@ static int method_reexecute(sd_bus_message *message, void *userdata, sd_bus_erro
> assert(message);
> assert(m);
>
> - r = verify_run_space("Refusing to reexecute", error);
> + r = mac_selinux_access_check(message, "reload", error);
> if (r < 0)
> return r;
>
> - r = mac_selinux_access_check(message, "reload", error);
> + r = verify_run_space("Refusing to reexecute", error);
> if (r < 0)
> return r;
>
> @@ -1428,6 +1479,10 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "reboot", error);
> + if (r < 0)
> + return r;
> +
> if (statvfs("/run/systemd", &svfs) < 0)
> return sd_bus_error_set_errnof(error, errno, "Failed to statvfs(/run/systemd): %m");
>
> @@ -1441,10 +1496,6 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
> format_bytes(fb_need, sizeof(fb_need), RELOAD_DISK_SPACE_MIN));
> }
>
> - r = mac_selinux_access_check(message, "reboot", error);
> - if (r < 0)
> - return r;
> -
> if (!MANAGER_IS_SYSTEM(m))
> return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Root switching is only supported by system manager.");
>
> @@ -1636,6 +1687,10 @@ static int method_lookup_dynamic_user_by_name(sd_bus_message *message, void *use
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "listdynusers", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read_basic(message, 's', &name);
> if (r < 0)
> return r;
> @@ -1663,6 +1718,10 @@ static int method_lookup_dynamic_user_by_uid(sd_bus_message *message, void *user
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "listdynusers", error);
> + if (r < 0)
> + return r;
> +
> assert_cc(sizeof(uid) == sizeof(uint32_t));
> r = sd_bus_message_read_basic(message, 'u', &uid);
> if (r < 0)
> @@ -1692,6 +1751,10 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "listdynusers", error);
> + if (r < 0)
> + return r;
> +
> assert_cc(sizeof(uid_t) == sizeof(uint32_t));
>
> if (!MANAGER_IS_SYSTEM(m))
> @@ -2264,6 +2327,10 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
> assert(message);
> assert(m);
>
> + r = mac_selinux_access_check(message, "modify", error);
> + if (r < 0)
> + return r;
> +
> r = bus_verify_manage_unit_files_async(m, message, error);
> if (r < 0)
> return r;
> @@ -2300,6 +2367,10 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
> char **p;
> int runtime, r;
>
> + r = mac_selinux_access_check(message, "status", error);
> + if (r < 0)
> + return r;
> +
> r = sd_bus_message_read(message, "sb", &name, &runtime);
> if (r < 0)
> return r;
> @@ -2422,7 +2493,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
> /* The following item is an obsolete alias */
> SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, reboot_watchdog), SD_BUS_VTABLE_HIDDEN),
> SD_BUS_WRITABLE_PROPERTY("KExecWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, kexec_watchdog), 0),
> - SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool, offsetof(Manager, service_watchdogs), 0),
> + SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool_wrapper, offsetof(Manager, service_watchdogs), 0),
> SD_BUS_PROPERTY("ControlGroup", "s", NULL, offsetof(Manager, cgroup_root), 0),
> SD_BUS_PROPERTY("SystemState", "s", property_get_system_state, 0, 0),
> SD_BUS_PROPERTY("ExitCode", "y", bus_property_get_unsigned, offsetof(Manager, return_value), 0),
> diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
> index 9477c47140..f86efaac3a 100644
> --- a/src/core/dbus-unit.c
> +++ b/src/core/dbus-unit.c
> @@ -645,6 +645,10 @@ int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error
> assert(message);
> assert(u);
>
> + r = mac_selinux_unit_access_check(u, message, "modify", error);
> + if (r < 0)
> + return r;
> +
> r = bus_unit_track_remove_sender(u, message);
> if (r == -EUNATCH)
> return sd_bus_error_setf(error, BUS_ERROR_NOT_REFERENCED, "Unit has not been referenced yet.");
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods
2019-12-18 20:05 ` Stephen Smalley
@ 2019-12-19 14:49 ` Christian Göttsche
2019-12-19 15:13 ` Stephen Smalley
0 siblings, 1 reply; 16+ messages in thread
From: Christian Göttsche @ 2019-12-19 14:49 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux
Am Mi., 18. Dez. 2019 um 21:05 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
>
> On 12/18/19 9:28 AM, Christian Göttsche wrote:
> > Add new SELinux permissions `modify` and `listdynusers` to the class `service`.
> > - modfiy checks altering operations, like `systemctl log-level debug` or `systemctl add-wants foo bar`.
> > - listdynusers checks obtaining dynamic users, which is very common on systems using nss-systemd.
> > Add a new permission to avoid undermining the `status` permission.
> >
> > Perform SELinux access checks for the following D-Bus exposed methods:
> >
> > D-Bus interface | c function name | SELinux permission verb
> >
> > GetAfter / GetBefore | bus_job_method_get_waiting_jobs | status
> > LogTarget | property_set_log_target | modify
> > LogLevel | property_set_log_level | modify
> > RuntimeWatchdocUSec | property_set_runtime_watchdog | modify
> > ServiceWatchdogs | bus_property_set_bool_wrapper | modify
> > GetUnit | method_get_unit | status
> > GetUnitByPID | method_get_unit_by_pid | status
> > GetUnitByControlGroup | method_get_unit_by_control_group | status
> > LoadUnit | method_load_unit | status
> > ListUnitsByNames | method_list_units_by_names | status
> > LookupDynamicUserByName | method_lookup_dynamic_user_by_name | listdynusers
> > LookupDynamicUserByUID | method_lookup_dynamic_user_by_uid | listdynusers
> > GetDynamicUsers | method_get_dynamic_users | listdynusers
> > AddDependencyUnitFiles | method_add_dependency_unit_files | modify
> > GetUnitFileLinks | method_get_unit_file_links | status
> > Unref | bus_unit_method_unref | modify
>
> If we are going to introduce new permissions or change existing ones, we
> may want to consider just defining a separate permission for every
> logical interface. Then we can let the policy writer decide what
> matters to them and at what granularity they want to distinguish things,
> using macros/interfaces as appropriate to avoid needing to specify them
> all individually.
>
> Also, you may want to leverage the policy capability mechanism in
> userspace to permit compatible evolution of permission checks in the
> same manner it is presently used in the kernel for
> extended_socket_class, network_peer_controls, open_perms,
> nnp_nosuid_transition, etc.
>
This might be an idea, to preserve full backward compatibility.
While on it one could untangle the system security class.
Afaik this would require a SELinux userland and kernel update, to
introduce a new policy capability identifier?
>
> > ---
> > src/core/dbus-job.c | 4 ++
> > src/core/dbus-manager.c | 89 ++++++++++++++++++++++++++++++++++++-----
> > src/core/dbus-unit.c | 4 ++
> > 3 files changed, 88 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/core/dbus-job.c b/src/core/dbus-job.c
> > index a7d342257b..7b0b093757 100644
> > --- a/src/core/dbus-job.c
> > +++ b/src/core/dbus-job.c
> > @@ -71,6 +71,10 @@ int bus_job_method_get_waiting_jobs(sd_bus_message *message, void *userdata, sd_
> > Job *j = userdata;
> > int r, i, n;
> >
> > + r = mac_selinux_unit_access_check(j->unit, message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > if (strstr(sd_bus_message_get_member(message), "After"))
> > n = job_get_after(j, &list);
> > else
> > diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
> > index c751e84253..d2db6e4a61 100644
> > --- a/src/core/dbus-manager.c
> > +++ b/src/core/dbus-manager.c
> > @@ -121,6 +121,10 @@ static int property_set_log_target(
> > assert(bus);
> > assert(value);
> >
> > + r = mac_selinux_access_check(value, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read(value, "s", &t);
> > if (r < 0)
> > return r;
> > @@ -178,6 +182,10 @@ static int property_set_log_level(
> > assert(bus);
> > assert(value);
> >
> > + r = mac_selinux_access_check(value, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read(value, "s", &t);
> > if (r < 0)
> > return r;
> > @@ -282,6 +290,10 @@ static int property_set_runtime_watchdog(
> >
> > assert_cc(sizeof(usec_t) == sizeof(uint64_t));
> >
> > + r = mac_selinux_access_check(value, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read(value, "t", t);
> > if (r < 0)
> > return r;
> > @@ -289,6 +301,24 @@ static int property_set_runtime_watchdog(
> > return watchdog_set_timeout(t);
> > }
> >
> > +static int bus_property_set_bool_wrapper(
> > + sd_bus *bus,
> > + const char *path,
> > + const char *interface,
> > + const char *property,
> > + sd_bus_message *value,
> > + void *userdata,
> > + sd_bus_error *error) {
> > +
> > + int r;
> > +
> > + r = mac_selinux_access_check(value, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > + return bus_property_set_bool(bus, path, interface, property, value, userdata, error);
> > +}
> > +
> > static int bus_get_unit_by_name(Manager *m, sd_bus_message *message, const char *name, Unit **ret_unit, sd_bus_error *error) {
> > Unit *u;
> > int r;
> > @@ -375,6 +405,10 @@ static int method_get_unit(sd_bus_message *message, void *userdata, sd_bus_error
> > if (r < 0)
> > return r;
> >
> > + r = mac_selinux_unit_access_check(u, message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > return reply_unit_path(u, message, error);
> > }
> >
> > @@ -413,6 +447,10 @@ static int method_get_unit_by_pid(sd_bus_message *message, void *userdata, sd_bu
> > if (!u)
> > return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pid);
> >
> > + r = mac_selinux_unit_access_check(u, message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > return reply_unit_path(u, message, error);
> > }
> >
> > @@ -488,6 +526,10 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd
> > if (!u)
> > return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Control group '%s' is not valid or not managed by this instance", cgroup);
> >
> > + r = mac_selinux_unit_access_check(u, message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > return reply_unit_path(u, message, error);
> > }
> >
> > @@ -510,6 +552,10 @@ static int method_load_unit(sd_bus_message *message, void *userdata, sd_bus_erro
> > if (r < 0)
> > return r;
> >
> > + r = mac_selinux_unit_access_check(u, message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > return reply_unit_path(u, message, error);
> > }
> >
> > @@ -529,6 +575,7 @@ static int method_start_unit_generic(sd_bus_message *message, Manager *m, JobTyp
> > if (r < 0)
> > return r;
> >
> > + /* bus_unit_method_start_generic() includes a mac_selinux check */
> > return bus_unit_method_start_generic(message, u, job_type, reload_if_possible, error);
> > }
> >
> > @@ -703,6 +750,10 @@ static int method_list_units_by_names(sd_bus_message *message, void *userdata, s
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read_strv(message, &units);
> > if (r < 0)
> > return r;
> > @@ -1263,11 +1314,11 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *
> > assert(message);
> > assert(m);
> >
> > - r = verify_run_space("Refusing to reload", error);
> > + r = mac_selinux_access_check(message, "reload", error);
> > if (r < 0)
> > return r;
> >
> > - r = mac_selinux_access_check(message, "reload", error);
> > + r = verify_run_space("Refusing to reload", error);
> > if (r < 0)
> > return r;
> >
> > @@ -1299,11 +1350,11 @@ static int method_reexecute(sd_bus_message *message, void *userdata, sd_bus_erro
> > assert(message);
> > assert(m);
> >
> > - r = verify_run_space("Refusing to reexecute", error);
> > + r = mac_selinux_access_check(message, "reload", error);
> > if (r < 0)
> > return r;
> >
> > - r = mac_selinux_access_check(message, "reload", error);
> > + r = verify_run_space("Refusing to reexecute", error);
> > if (r < 0)
> > return r;
> >
> > @@ -1428,6 +1479,10 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "reboot", error);
> > + if (r < 0)
> > + return r;
> > +
> > if (statvfs("/run/systemd", &svfs) < 0)
> > return sd_bus_error_set_errnof(error, errno, "Failed to statvfs(/run/systemd): %m");
> >
> > @@ -1441,10 +1496,6 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er
> > format_bytes(fb_need, sizeof(fb_need), RELOAD_DISK_SPACE_MIN));
> > }
> >
> > - r = mac_selinux_access_check(message, "reboot", error);
> > - if (r < 0)
> > - return r;
> > -
> > if (!MANAGER_IS_SYSTEM(m))
> > return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Root switching is only supported by system manager.");
> >
> > @@ -1636,6 +1687,10 @@ static int method_lookup_dynamic_user_by_name(sd_bus_message *message, void *use
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "listdynusers", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read_basic(message, 's', &name);
> > if (r < 0)
> > return r;
> > @@ -1663,6 +1718,10 @@ static int method_lookup_dynamic_user_by_uid(sd_bus_message *message, void *user
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "listdynusers", error);
> > + if (r < 0)
> > + return r;
> > +
> > assert_cc(sizeof(uid) == sizeof(uint32_t));
> > r = sd_bus_message_read_basic(message, 'u', &uid);
> > if (r < 0)
> > @@ -1692,6 +1751,10 @@ static int method_get_dynamic_users(sd_bus_message *message, void *userdata, sd_
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "listdynusers", error);
> > + if (r < 0)
> > + return r;
> > +
> > assert_cc(sizeof(uid_t) == sizeof(uint32_t));
> >
> > if (!MANAGER_IS_SYSTEM(m))
> > @@ -2264,6 +2327,10 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
> > assert(message);
> > assert(m);
> >
> > + r = mac_selinux_access_check(message, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = bus_verify_manage_unit_files_async(m, message, error);
> > if (r < 0)
> > return r;
> > @@ -2300,6 +2367,10 @@ static int method_get_unit_file_links(sd_bus_message *message, void *userdata, s
> > char **p;
> > int runtime, r;
> >
> > + r = mac_selinux_access_check(message, "status", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = sd_bus_message_read(message, "sb", &name, &runtime);
> > if (r < 0)
> > return r;
> > @@ -2422,7 +2493,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
> > /* The following item is an obsolete alias */
> > SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, reboot_watchdog), SD_BUS_VTABLE_HIDDEN),
> > SD_BUS_WRITABLE_PROPERTY("KExecWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, kexec_watchdog), 0),
> > - SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool, offsetof(Manager, service_watchdogs), 0),
> > + SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool_wrapper, offsetof(Manager, service_watchdogs), 0),
> > SD_BUS_PROPERTY("ControlGroup", "s", NULL, offsetof(Manager, cgroup_root), 0),
> > SD_BUS_PROPERTY("SystemState", "s", property_get_system_state, 0, 0),
> > SD_BUS_PROPERTY("ExitCode", "y", bus_property_get_unsigned, offsetof(Manager, return_value), 0),
> > diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
> > index 9477c47140..f86efaac3a 100644
> > --- a/src/core/dbus-unit.c
> > +++ b/src/core/dbus-unit.c
> > @@ -645,6 +645,10 @@ int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error
> > assert(message);
> > assert(u);
> >
> > + r = mac_selinux_unit_access_check(u, message, "modify", error);
> > + if (r < 0)
> > + return r;
> > +
> > r = bus_unit_track_remove_sender(u, message);
> > if (r == -EUNATCH)
> > return sd_bus_error_setf(error, BUS_ERROR_NOT_REFERENCED, "Unit has not been referenced yet.");
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods
2019-12-19 14:49 ` Christian Göttsche
@ 2019-12-19 15:13 ` Stephen Smalley
0 siblings, 0 replies; 16+ messages in thread
From: Stephen Smalley @ 2019-12-19 15:13 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux, Paul Moore, Ondrej Mosnacek
On 12/19/19 9:49 AM, Christian Göttsche wrote:
> Am Mi., 18. Dez. 2019 um 21:05 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
>>
>> On 12/18/19 9:28 AM, Christian Göttsche wrote:
>>> Add new SELinux permissions `modify` and `listdynusers` to the class `service`.
>>> - modfiy checks altering operations, like `systemctl log-level debug` or `systemctl add-wants foo bar`.
>>> - listdynusers checks obtaining dynamic users, which is very common on systems using nss-systemd.
>>> Add a new permission to avoid undermining the `status` permission.
>>>
>>> Perform SELinux access checks for the following D-Bus exposed methods:
>>>
>>> D-Bus interface | c function name | SELinux permission verb
>>>
>>> GetAfter / GetBefore | bus_job_method_get_waiting_jobs | status
>>> LogTarget | property_set_log_target | modify
>>> LogLevel | property_set_log_level | modify
>>> RuntimeWatchdocUSec | property_set_runtime_watchdog | modify
>>> ServiceWatchdogs | bus_property_set_bool_wrapper | modify
>>> GetUnit | method_get_unit | status
>>> GetUnitByPID | method_get_unit_by_pid | status
>>> GetUnitByControlGroup | method_get_unit_by_control_group | status
>>> LoadUnit | method_load_unit | status
>>> ListUnitsByNames | method_list_units_by_names | status
>>> LookupDynamicUserByName | method_lookup_dynamic_user_by_name | listdynusers
>>> LookupDynamicUserByUID | method_lookup_dynamic_user_by_uid | listdynusers
>>> GetDynamicUsers | method_get_dynamic_users | listdynusers
>>> AddDependencyUnitFiles | method_add_dependency_unit_files | modify
>>> GetUnitFileLinks | method_get_unit_file_links | status
>>> Unref | bus_unit_method_unref | modify
>>
>> If we are going to introduce new permissions or change existing ones, we
>> may want to consider just defining a separate permission for every
>> logical interface. Then we can let the policy writer decide what
>> matters to them and at what granularity they want to distinguish things,
>> using macros/interfaces as appropriate to avoid needing to specify them
>> all individually.
>>
>> Also, you may want to leverage the policy capability mechanism in
>> userspace to permit compatible evolution of permission checks in the
>> same manner it is presently used in the kernel for
>> extended_socket_class, network_peer_controls, open_perms,
>> nnp_nosuid_transition, etc.
>>
>
> This might be an idea, to preserve full backward compatibility.
> While on it one could untangle the system security class.
>
> Afaik this would require a SELinux userland and kernel update, to
> introduce a new policy capability identifier?
Correct. I'd like to relax that requirement in the future (see
https://github.com/SELinuxProject/selinux/issues/55) but for now you
need a patched libsepol and a patched kernel. This would be the first
policy capability used by userspace instead of the kernel, so we'd also
need/want libselinux functions for testing whether a given policy
capability is enabled via selinuxfs. People with existing kernels or
policies would continue to operate with the old permission checks; it
would require an upgrade of libsepol, kernel, and a policy that
explicitly enables the new policy capability to cause the new permission
checks to take effect.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/8] systemd: improve SELinux support
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
` (7 preceding siblings ...)
2019-12-18 14:28 ` [RFC PATCH 8/8] core: add notes to D-Bus interfaces about adding SELinux checks Christian Göttsche
@ 2019-12-19 20:24 ` Chris PeBenito
2019-12-20 10:54 ` Dominick Grift
8 siblings, 1 reply; 16+ messages in thread
From: Chris PeBenito @ 2019-12-19 20:24 UTC (permalink / raw)
To: Christian Göttsche, selinux
On 12/18/19 9:28 AM, Christian Göttsche wrote:
> Improve the SELinux support in systemd, especially re-adding checks for
> unit file operations, like enable, mask...
>
> The original pull request can be found at https://github.com/systemd/systemd/pull/10023
>
> Patch 1 and 2 improve logging on failures in permissive mode.
>
> Patch 3 adds the ability to obtain the context for a masked unit.
>
> Patch 4 and 5 change several system und service checks. For better
> distinction two new permissions are introduced: modify and listdynusers.
>
> Patch 6 and 7 re-introduce checking unit file install operations.
> They were dropped in 8faae625dc9b6322db452937f54176e56e65265a .
> For consistency in the unexpected case while perforimg a service access
> check no path can be gathered, now the check will still be executed on
> the service security class (currently it switches to the system security
> class).
>
> Patch 8 adds some notes for adding future D-Bus interfaces.
Thanks for working on this. Just to make sure I didn't miss anything
while reading the patches, there are no new permissions being added to
the system class, correct?
--
Chris PeBenito
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/8] systemd: improve SELinux support
2019-12-19 20:24 ` [RFC PATCH 0/8] systemd: improve SELinux support Chris PeBenito
@ 2019-12-20 10:54 ` Dominick Grift
2019-12-20 14:43 ` Chris PeBenito
0 siblings, 1 reply; 16+ messages in thread
From: Dominick Grift @ 2019-12-20 10:54 UTC (permalink / raw)
To: Chris PeBenito; +Cc: Christian Göttsche, selinux
[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]
On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote:
> On 12/18/19 9:28 AM, Christian Göttsche wrote:
> > Improve the SELinux support in systemd, especially re-adding checks for
> > unit file operations, like enable, mask...
> >
> > The original pull request can be found at https://github.com/systemd/systemd/pull/10023
> >
> > Patch 1 and 2 improve logging on failures in permissive mode.
> >
> > Patch 3 adds the ability to obtain the context for a masked unit.
> >
> > Patch 4 and 5 change several system und service checks. For better
> > distinction two new permissions are introduced: modify and listdynusers.
> >
> > Patch 6 and 7 re-introduce checking unit file install operations.
> > They were dropped in 8faae625dc9b6322db452937f54176e56e65265a .
> > For consistency in the unexpected case while perforimg a service access
> > check no path can be gathered, now the check will still be executed on
> > the service security class (currently it switches to the system security
> > class).
> >
> > Patch 8 adds some notes for adding future D-Bus interfaces.
>
> Thanks for working on this. Just to make sure I didn't miss anything while
> reading the patches, there are no new permissions being added to the system
> class, correct?
This is what i understand:
Systemd first wants the regression fixed (re-add enable disable permission checks)
This phase should not add anything to the system class. It merely addresses a previous regression.
When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?).
If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd.
One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation.
>
> --
> Chris PeBenito
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/8] systemd: improve SELinux support
2019-12-20 10:54 ` Dominick Grift
@ 2019-12-20 14:43 ` Chris PeBenito
2019-12-20 14:53 ` Dominick Grift
0 siblings, 1 reply; 16+ messages in thread
From: Chris PeBenito @ 2019-12-20 14:43 UTC (permalink / raw)
To: Christian Göttsche, selinux
On 12/20/19 5:54 AM, Dominick Grift wrote:
> On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote:
>> On 12/18/19 9:28 AM, Christian Göttsche wrote:
>>> Improve the SELinux support in systemd, especially re-adding checks for
>>> unit file operations, like enable, mask...
>>>
>>> The original pull request can be found at https://github.com/systemd/systemd/pull/10023
>>>
>>> Patch 1 and 2 improve logging on failures in permissive mode.
>>>
>>> Patch 3 adds the ability to obtain the context for a masked unit.
>>>
>>> Patch 4 and 5 change several system und service checks. For better
>>> distinction two new permissions are introduced: modify and listdynusers.
>>>
>>> Patch 6 and 7 re-introduce checking unit file install operations.
>>> They were dropped in 8faae625dc9b6322db452937f54176e56e65265a .
>>> For consistency in the unexpected case while perforimg a service access
>>> check no path can be gathered, now the check will still be executed on
>>> the service security class (currently it switches to the system security
>>> class).
>>>
>>> Patch 8 adds some notes for adding future D-Bus interfaces.
>>
>> Thanks for working on this. Just to make sure I didn't miss anything while
>> reading the patches, there are no new permissions being added to the system
>> class, correct?
>
> This is what i understand:
>
> Systemd first wants the regression fixed (re-add enable disable permission checks)
> This phase should not add anything to the system class. It merely addresses a previous regression.
I didn't look at all of the implementation details, but as long as there
are no new permissions in the system class, I'm ok with the changes.
> When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?).
> If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd.
> One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation.
The correct thing to do with future changes is to remove the userspace
permissions and put them in a new userspace object class (with
appropriate compat, etc). If you're adding new permissions at the same
time, a disabled policy capability would mean not checking the new
permission; it wouldn't mean adding the new permission to the system class.
--
Chris PeBenito
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 0/8] systemd: improve SELinux support
2019-12-20 14:43 ` Chris PeBenito
@ 2019-12-20 14:53 ` Dominick Grift
0 siblings, 0 replies; 16+ messages in thread
From: Dominick Grift @ 2019-12-20 14:53 UTC (permalink / raw)
To: Chris PeBenito; +Cc: Christian Göttsche, selinux
[-- Attachment #1: Type: text/plain, Size: 3243 bytes --]
On Fri, Dec 20, 2019 at 09:43:09AM -0500, Chris PeBenito wrote:
> On 12/20/19 5:54 AM, Dominick Grift wrote:
> > On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote:
> > > On 12/18/19 9:28 AM, Christian Göttsche wrote:
> > > > Improve the SELinux support in systemd, especially re-adding checks for
> > > > unit file operations, like enable, mask...
> > > >
> > > > The original pull request can be found at https://github.com/systemd/systemd/pull/10023
> > > >
> > > > Patch 1 and 2 improve logging on failures in permissive mode.
> > > >
> > > > Patch 3 adds the ability to obtain the context for a masked unit.
> > > >
> > > > Patch 4 and 5 change several system und service checks. For better
> > > > distinction two new permissions are introduced: modify and listdynusers.
> > > >
> > > > Patch 6 and 7 re-introduce checking unit file install operations.
> > > > They were dropped in 8faae625dc9b6322db452937f54176e56e65265a .
> > > > For consistency in the unexpected case while perforimg a service access
> > > > check no path can be gathered, now the check will still be executed on
> > > > the service security class (currently it switches to the system security
> > > > class).
> > > >
> > > > Patch 8 adds some notes for adding future D-Bus interfaces.
> > >
> > > Thanks for working on this. Just to make sure I didn't miss anything while
> > > reading the patches, there are no new permissions being added to the system
> > > class, correct?
> >
> > This is what i understand:
> >
> > Systemd first wants the regression fixed (re-add enable disable permission checks)
> > This phase should not add anything to the system class. It merely addresses a previous regression.
>
>
> I didn't look at all of the implementation details, but as long as there are
> no new permissions in the system class, I'm ok with the changes.
This patch series has already been rejected by systemd in the mean time, the reason being that they first want to only address the existing regression.
Addding anything new requires a blessing from the various (distribution) maintainers (good luck with that).
>
>
> > When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?).
> > If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd.
> > One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation.
> The correct thing to do with future changes is to remove the userspace
> permissions and put them in a new userspace object class (with appropriate
> compat, etc). If you're adding new permissions at the same time, a disabled
> policy capability would mean not checking the new permission; it wouldn't
> mean adding the new permission to the system class.
>
> --
> Chris PeBenito
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-12-20 14:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 14:28 [RFC PATCH 0/8] systemd: improve SELinux support Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 1/8] selinux-util: increase log severity Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 2/8] selinux-access: log warning on context acquisition failure Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 3/8] core: bookkeeping withdrawal path of masked units Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 4/8] core: add missing SELinux checks for dbus methods Christian Göttsche
2019-12-18 20:05 ` Stephen Smalley
2019-12-19 14:49 ` Christian Göttsche
2019-12-19 15:13 ` Stephen Smalley
2019-12-18 14:28 ` [RFC PATCH 5/8] core: make SELinux access permissions more distinct Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 6/8] core: add support for MAC checks on unit install operations Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 7/8] core: implement the sd-bus generic callback for SELinux Christian Göttsche
2019-12-18 14:28 ` [RFC PATCH 8/8] core: add notes to D-Bus interfaces about adding SELinux checks Christian Göttsche
2019-12-19 20:24 ` [RFC PATCH 0/8] systemd: improve SELinux support Chris PeBenito
2019-12-20 10:54 ` Dominick Grift
2019-12-20 14:43 ` Chris PeBenito
2019-12-20 14:53 ` 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.