mirror of
https://github.com/systemd/systemd.git
synced 2026-06-30 19:57:29 +00:00
analyze: don't treat user-scope services as running as root in security
`systemd-analyze security --user foo.service` currently flags units without `User=` as running as root. For user manager instances this is impossible: per systemd.exec(5), switching user identity is not permitted there, so the service always runs under the calling user's UID. Track the runtime scope inside SecurityInfo and short-circuit security_info_runs_privileged() and assess_user() for RUNTIME_SCOPE_USER, so that User=/DynamicUser=, SupplementaryGroups= and RemoveIPC= are no longer marked as if the service ran as root in both the bus-backed and --offline paths. Fixes #40292 Signed-off-by: Shihao Ren <renshihao.rsh@bytedance.com>
This commit is contained in:
@@ -108,6 +108,8 @@ typedef struct SecurityInfo {
|
||||
Set *system_call_filter;
|
||||
|
||||
mode_t _umask;
|
||||
|
||||
RuntimeScope runtime_scope;
|
||||
} SecurityInfo;
|
||||
|
||||
struct security_assessor {
|
||||
@@ -182,6 +184,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(SecurityInfo*, security_info_free);
|
||||
static bool security_info_runs_privileged(const SecurityInfo *i) {
|
||||
assert(i);
|
||||
|
||||
/* For the common --user case the manager runs as a regular user and cannot switch
|
||||
* UID, so user-scope services never end up privileged. We deliberately do not
|
||||
* handle root-owned user managers (user@0.service) here: in that narrow case the
|
||||
* service really is privileged and will be mis-classified. */
|
||||
if (i->runtime_scope == RUNTIME_SCOPE_USER)
|
||||
return false;
|
||||
|
||||
if (STRPTR_IN_SET(i->user, "0", "root"))
|
||||
return true;
|
||||
|
||||
@@ -232,6 +241,12 @@ static int assess_user(
|
||||
} else if (info->user && !STR_IN_SET(info->user, "0", "root", "")) {
|
||||
d = "Service runs under a static non-root user identity";
|
||||
b = 0;
|
||||
} else if (info->runtime_scope == RUNTIME_SCOPE_USER) {
|
||||
/* Same caveat as security_info_runs_privileged(): root-owned user
|
||||
* managers (user@0.service) will be reported as non-root here, which is
|
||||
* incorrect but matches the heuristic. */
|
||||
d = "Service runs under the calling user's identity";
|
||||
b = 0;
|
||||
} else {
|
||||
*ret_badness = 10;
|
||||
*ret_description = NULL;
|
||||
@@ -477,6 +492,12 @@ static int assess_remove_ipc(
|
||||
assert(ret_badness);
|
||||
assert(ret_description);
|
||||
|
||||
if (info->runtime_scope == RUNTIME_SCOPE_USER) {
|
||||
*ret_badness = UINT64_MAX;
|
||||
return strdup_to(ret_description,
|
||||
"RemoveIPC= has no effect on per-user services");
|
||||
}
|
||||
|
||||
if (security_info_runs_privileged(info))
|
||||
*ret_badness = UINT64_MAX;
|
||||
else
|
||||
@@ -2292,7 +2313,7 @@ static int property_read_device_allow(
|
||||
return sd_bus_message_exit_container(m);
|
||||
}
|
||||
|
||||
static int acquire_security_info(sd_bus *bus, const char *name, SecurityInfo *info, AnalyzeSecurityFlags flags) {
|
||||
static int acquire_security_info(sd_bus *bus, const char *name, SecurityInfo *info, RuntimeScope scope, AnalyzeSecurityFlags flags) {
|
||||
|
||||
static const struct bus_properties_map security_map[] = {
|
||||
{ "AmbientCapabilities", "t", NULL, offsetof(SecurityInfo, ambient_capabilities) },
|
||||
@@ -2413,12 +2434,15 @@ static int acquire_security_info(sd_bus *bus, const char *name, SecurityInfo *in
|
||||
info->capability_bounding_set &= ~((UINT64_C(1) << CAP_MKNOD) |
|
||||
(UINT64_C(1) << CAP_SYS_RAWIO));
|
||||
|
||||
info->runtime_scope = scope;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int analyze_security_one(sd_bus *bus,
|
||||
const char *name,
|
||||
Table *overview_table,
|
||||
RuntimeScope scope,
|
||||
AnalyzeSecurityFlags flags,
|
||||
unsigned threshold,
|
||||
sd_json_variant *policy,
|
||||
@@ -2434,7 +2458,7 @@ static int analyze_security_one(sd_bus *bus,
|
||||
assert(bus);
|
||||
assert(name);
|
||||
|
||||
r = acquire_security_info(bus, name, info, flags);
|
||||
r = acquire_security_info(bus, name, info, scope, flags);
|
||||
if (r == -EMEDIUMTYPE) /* Ignore this one because not loaded or Type is oneshot */
|
||||
return 0;
|
||||
if (r < 0)
|
||||
@@ -2448,13 +2472,15 @@ static int analyze_security_one(sd_bus *bus,
|
||||
}
|
||||
|
||||
/* Refactoring SecurityInfo so that it can make use of existing struct variables instead of reading from dbus */
|
||||
static int get_security_info(Unit *u, ExecContext *c, CGroupContext *g, SecurityInfo **ret_info) {
|
||||
static int get_security_info(Unit *u, ExecContext *c, CGroupContext *g, RuntimeScope scope, SecurityInfo **ret_info) {
|
||||
assert(ret_info);
|
||||
|
||||
_cleanup_(security_info_freep) SecurityInfo *info = security_info_new();
|
||||
if (!info)
|
||||
return log_oom();
|
||||
|
||||
info->runtime_scope = scope;
|
||||
|
||||
if (u) {
|
||||
if (u->id) {
|
||||
info->id = strdup(u->id);
|
||||
@@ -2654,6 +2680,7 @@ static int get_security_info(Unit *u, ExecContext *c, CGroupContext *g, Security
|
||||
}
|
||||
|
||||
static int offline_security_check(Unit *u,
|
||||
RuntimeScope scope,
|
||||
unsigned threshold,
|
||||
sd_json_variant *policy,
|
||||
PagerFlags pager_flags,
|
||||
@@ -2669,7 +2696,7 @@ static int offline_security_check(Unit *u,
|
||||
if (DEBUG_LOGGING)
|
||||
unit_dump(u, stdout, "\t");
|
||||
|
||||
r = get_security_info(u, unit_get_exec_context(u), unit_get_cgroup_context(u), &info);
|
||||
r = get_security_info(u, unit_get_exec_context(u), unit_get_cgroup_context(u), scope, &info);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -2775,7 +2802,7 @@ static int offline_security_checks(
|
||||
}
|
||||
|
||||
for (size_t i = 0; i < count; i++)
|
||||
RET_GATHER(r, offline_security_check(units[i], threshold, policy, pager_flags, json_format_flags));
|
||||
RET_GATHER(r, offline_security_check(units[i], scope, threshold, policy, pager_flags, json_format_flags));
|
||||
|
||||
return r;
|
||||
}
|
||||
@@ -2855,7 +2882,7 @@ static int analyze_security(sd_bus *bus,
|
||||
flags |= ANALYZE_SECURITY_SHORT|ANALYZE_SECURITY_ONLY_LOADED|ANALYZE_SECURITY_ONLY_LONG_RUNNING;
|
||||
|
||||
STRV_FOREACH(i, list) {
|
||||
r = analyze_security_one(bus, *i, overview_table, flags, threshold, policy, pager_flags, json_format_flags);
|
||||
r = analyze_security_one(bus, *i, overview_table, scope, flags, threshold, policy, pager_flags, json_format_flags);
|
||||
if (r < 0 && ret >= 0)
|
||||
ret = r;
|
||||
}
|
||||
@@ -2888,7 +2915,7 @@ static int analyze_security(sd_bus *bus,
|
||||
} else
|
||||
name = mangled;
|
||||
|
||||
r = analyze_security_one(bus, name, overview_table, flags, threshold, policy, pager_flags, json_format_flags);
|
||||
r = analyze_security_one(bus, name, overview_table, scope, flags, threshold, policy, pager_flags, json_format_flags);
|
||||
if (r < 0 && ret >= 0)
|
||||
ret = r;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user