Fixes for yeswehack reports (#40840)
This commit is contained in:
+13
-15
@@ -2115,12 +2115,13 @@ static int unit_update_cgroup(
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int unit_attach_pid_to_cgroup_via_bus(Unit *u, pid_t pid, const char *suffix_path) {
|
||||
static int unit_attach_pid_to_cgroup_via_bus(Unit *u, const char *cgroup_path, pid_t pid) {
|
||||
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
|
||||
char *pp;
|
||||
int r;
|
||||
|
||||
assert(u);
|
||||
assert(cgroup_path);
|
||||
assert(pid_is_valid(pid));
|
||||
|
||||
if (MANAGER_IS_SYSTEM(u->manager))
|
||||
return -EINVAL;
|
||||
@@ -2128,18 +2129,13 @@ static int unit_attach_pid_to_cgroup_via_bus(Unit *u, pid_t pid, const char *suf
|
||||
if (!u->manager->system_bus)
|
||||
return -EIO;
|
||||
|
||||
CGroupRuntime *crt = unit_get_cgroup_runtime(u);
|
||||
if (!crt || !crt->cgroup_path)
|
||||
return -EOWNERDEAD;
|
||||
|
||||
/* Determine this unit's cgroup path relative to our cgroup root */
|
||||
pp = path_startswith(crt->cgroup_path, u->manager->cgroup_root);
|
||||
const char *pp = path_startswith_full(cgroup_path,
|
||||
u->manager->cgroup_root,
|
||||
PATH_STARTSWITH_RETURN_LEADING_SLASH|PATH_STARTSWITH_REFUSE_DOT_DOT);
|
||||
if (!pp)
|
||||
return -EINVAL;
|
||||
|
||||
pp = strjoina("/", pp, suffix_path);
|
||||
path_simplify(pp);
|
||||
|
||||
r = bus_call_method(u->manager->system_bus,
|
||||
bus_systemd_mgr,
|
||||
"AttachProcessesToUnit",
|
||||
@@ -2172,8 +2168,10 @@ int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
|
||||
CGroupRuntime *crt = ASSERT_PTR(unit_get_cgroup_runtime(u));
|
||||
|
||||
if (isempty(suffix_path))
|
||||
p = crt->cgroup_path;
|
||||
p = empty_to_root(crt->cgroup_path);
|
||||
else {
|
||||
assert(path_is_absolute(suffix_path));
|
||||
|
||||
joined = path_join(crt->cgroup_path, suffix_path);
|
||||
if (!joined)
|
||||
return -ENOMEM;
|
||||
@@ -2189,7 +2187,7 @@ int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
|
||||
* before we use it */
|
||||
r = pidref_verify(pid);
|
||||
if (r < 0) {
|
||||
log_unit_info_errno(u, r, "PID " PID_FMT " vanished before we could move it to target cgroup '%s', skipping: %m", pid->pid, empty_to_root(p));
|
||||
log_unit_info_errno(u, r, "PID " PID_FMT " vanished before we could move it to target cgroup '%s', skipping: %m", pid->pid, p);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -2199,7 +2197,7 @@ int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
|
||||
|
||||
log_unit_full_errno(u, again ? LOG_DEBUG : LOG_INFO, r,
|
||||
"Couldn't move process "PID_FMT" to%s requested cgroup '%s': %m",
|
||||
pid->pid, again ? " directly" : "", empty_to_root(p));
|
||||
pid->pid, again ? " directly" : "", p);
|
||||
|
||||
if (again) {
|
||||
int z;
|
||||
@@ -2209,11 +2207,11 @@ int unit_attach_pids_to_cgroup(Unit *u, Set *pids, const char *suffix_path) {
|
||||
* Since it's more privileged it might be able to move the process across the
|
||||
* leaves of a subtree whose top node is not owned by us. */
|
||||
|
||||
z = unit_attach_pid_to_cgroup_via_bus(u, pid->pid, suffix_path);
|
||||
z = unit_attach_pid_to_cgroup_via_bus(u, p, pid->pid);
|
||||
if (z >= 0)
|
||||
goto success;
|
||||
|
||||
log_unit_info_errno(u, z, "Couldn't move process "PID_FMT" to requested cgroup '%s' (directly or via the system bus): %m", pid->pid, empty_to_root(p));
|
||||
log_unit_info_errno(u, z, "Couldn't move process "PID_FMT" to requested cgroup '%s' (directly or via the system bus): %m", pid->pid, p);
|
||||
}
|
||||
|
||||
RET_GATHER(ret, r);
|
||||
|
||||
@@ -652,6 +652,12 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
if (!path_is_absolute(cgroup))
|
||||
return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Control group path is not absolute: %s", cgroup);
|
||||
|
||||
if (!path_is_normalized(cgroup))
|
||||
return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Control group path is not normalized: %s", cgroup);
|
||||
|
||||
u = manager_get_unit_by_cgroup(m, cgroup);
|
||||
if (!u)
|
||||
return sd_bus_error_setf(reterr_error, BUS_ERROR_NO_SUCH_UNIT,
|
||||
|
||||
+13
-11
@@ -1559,9 +1559,9 @@ static int property_get_effective_limit(
|
||||
}
|
||||
|
||||
int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd_bus_error *reterr_error) {
|
||||
Unit *u = ASSERT_PTR(userdata);
|
||||
_cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
|
||||
_cleanup_set_free_ Set *pids = NULL;
|
||||
Unit *u = userdata;
|
||||
const char *path;
|
||||
int r;
|
||||
|
||||
@@ -1599,12 +1599,22 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
/* Let's query the sender's UID, so that we can make our security decisions */
|
||||
uid_t sender_uid;
|
||||
r = sd_bus_creds_get_euid(creds, &sender_uid);
|
||||
if (r < 0)
|
||||
return r;
|
||||
bool validate_ownership = sender_uid != 0 && sender_uid != getuid();
|
||||
|
||||
if (validate_ownership && !uid_is_valid(u->ref_uid)) /* process_is_owned_by_uid() requires a valid uid */
|
||||
return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_ACCESS_DENIED,
|
||||
"Refusing to attach processes to unit with unknown user credentials.");
|
||||
|
||||
r = sd_bus_message_enter_container(message, 'a', "u");
|
||||
if (r < 0)
|
||||
return r;
|
||||
for (;;) {
|
||||
_cleanup_(pidref_freep) PidRef *pidref = NULL;
|
||||
uid_t sender_uid;
|
||||
uint32_t upid;
|
||||
|
||||
r = sd_bus_message_read(message, "u", &upid);
|
||||
@@ -1634,25 +1644,17 @@ int bus_unit_method_attach_processes(sd_bus_message *message, void *userdata, sd
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
/* Let's query the sender's UID, so that we can make our security decisions */
|
||||
r = sd_bus_creds_get_euid(creds, &sender_uid);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
/* Let's validate security: if the sender is root or the owner of the service manager, then
|
||||
* all is OK. If the sender is any other user, then the process in question must be owned by
|
||||
* both the sender and the target unit's UID. Note that ownership here means either direct
|
||||
* ownership, or indirect via a userns that is owned by the right UID. */
|
||||
if (sender_uid != 0 && sender_uid != getuid()) {
|
||||
if (validate_ownership) {
|
||||
r = process_is_owned_by_uid(pidref, sender_uid);
|
||||
if (r < 0)
|
||||
return sd_bus_error_set_errnof(reterr_error, r, "Failed to check if process " PID_FMT " is owned by client's UID: %m", pidref->pid);
|
||||
if (r == 0)
|
||||
return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_ACCESS_DENIED, "Process " PID_FMT " not owned by client's UID. Refusing.", pidref->pid);
|
||||
|
||||
if (!uid_is_valid(u->ref_uid)) /* process_is_owned_by_uid() requires a valid uid */
|
||||
return sd_bus_error_setf(reterr_error, SD_BUS_ERROR_ACCESS_DENIED, "Unit does not have a valid ref_uid, refusing to attach process " PID_FMT ".", pidref->pid);
|
||||
|
||||
r = process_is_owned_by_uid(pidref, u->ref_uid);
|
||||
if (r < 0)
|
||||
return sd_bus_error_set_errnof(reterr_error, r, "Failed to check if process " PID_FMT " is owned by target unit's UID: %m", pidref->pid);
|
||||
|
||||
@@ -457,7 +457,7 @@ static int lookup_unit_by_parameters(
|
||||
}
|
||||
|
||||
if (p->cgroup) {
|
||||
if (!path_is_safe(p->cgroup))
|
||||
if (!path_is_absolute(p->cgroup) || !path_is_normalized(p->cgroup))
|
||||
return sd_varlink_error_invalid_parameter_name(link, "cgroup");
|
||||
|
||||
Unit *cgroup_unit = manager_get_unit_by_cgroup(manager, p->cgroup);
|
||||
|
||||
@@ -2235,7 +2235,7 @@ _public_ int sd_json_variant_append_array(sd_json_variant **v, sd_json_variant *
|
||||
|
||||
if (old != *v)
|
||||
/* Readjust the parent pointers to the new address */
|
||||
for (size_t i = 1; i < size; i++)
|
||||
for (size_t i = 0; i < size; i++)
|
||||
(*v)[1 + i].parent = *v;
|
||||
|
||||
return json_variant_array_put_element(*v, element);
|
||||
|
||||
Reference in New Issue
Block a user