From 119d332d9c2cf1974b235c8d9e4e3ad821cf436a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 12 Sep 2025 19:59:26 +0100 Subject: [PATCH 1/4] machine: do not allow unprivileged users to register other users' processes as machines Registering a process as a machine means a caller can get machined to send sigterm to it, and more. If an unpriv user is registering, ensure the registered process is actually owned by the user. Follow-up for adaff8eb35d9c471af81fddaa4403bc5843a256f --- src/machine/machine-varlink.c | 10 ++++++++++ src/machine/machined-dbus.c | 10 ++++++++++ test/units/TEST-13-NSPAWN.unpriv.sh | 19 +++++++++++++++++-- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/machine/machine-varlink.c b/src/machine/machine-varlink.c index 776654b51d..b2b2f1f2db 100644 --- a/src/machine/machine-varlink.c +++ b/src/machine/machine-varlink.c @@ -15,6 +15,7 @@ #include "machine-varlink.h" #include "machined.h" #include "mount-util.h" +#include "namespace-util.h" #include "operation.h" #include "pidref.h" #include "socket-util.h" @@ -186,6 +187,15 @@ int vl_method_register(sd_varlink *link, sd_json_variant *parameters, sd_varlink if (r < 0) return r; + /* Ensure an unprivileged user cannot claim any process they don't control as their own machine */ + if (machine->uid != 0) { + r = process_is_owned_by_uid(&machine->leader, machine->uid); + if (r < 0) + return r; + if (r == 0) + return sd_varlink_error(link, SD_VARLINK_ERROR_PERMISSION_DENIED, NULL); + } + r = machine_link(manager, machine); if (r == -EEXIST) return sd_varlink_error(link, VARLINK_ERROR_MACHINE_EXISTS, NULL); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 883c8c1338..91e5e21869 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -29,6 +29,7 @@ #include "machine-dbus.h" #include "machine-pool.h" #include "machined.h" +#include "namespace-util.h" #include "operation.h" #include "os-util.h" #include "path-util.h" @@ -321,6 +322,15 @@ static int method_create_or_register_machine( if (r < 0) return r; + /* Ensure an unprivileged user cannot claim any process they don't control as their own machine */ + if (uid != 0) { + r = process_is_owned_by_uid(&leader_pidref, uid); + if (r < 0) + return r; + if (r == 0) + return sd_bus_error_set(error, SD_BUS_ERROR_ACCESS_DENIED, "Only root may register machines for other users"); + } + const char *details[] = { "name", name, "class", machine_class_to_string(c), diff --git a/test/units/TEST-13-NSPAWN.unpriv.sh b/test/units/TEST-13-NSPAWN.unpriv.sh index 03af7ebc9c..17ccb19c8e 100755 --- a/test/units/TEST-13-NSPAWN.unpriv.sh +++ b/test/units/TEST-13-NSPAWN.unpriv.sh @@ -15,7 +15,7 @@ fi at_exit() { rm -rf /home/testuser/.local/state/machines/zurps ||: machinectl terminate zurps ||: - rm -f /usr/share/polkit-1/rules.d/registermachinetest.rules + rm -f /etc/polkit-1/rules.d/registermachinetest.rules } trap at_exit EXIT @@ -33,7 +33,8 @@ systemd-dissect --shift /home/testuser/.local/state/machines/zurps foreign # Install a PK rule that allows 'testuser' user to register a machine even # though they are not on an fg console, just for testing -cat >/usr/share/polkit-1/rules.d/registermachinetest.rules <<'EOF' +mkdir -p /etc/polkit-1/rules.d +cat >/etc/polkit-1/rules.d/registermachinetest.rules <<'EOF' polkit.addRule(function(action, subject) { if (action.id == "org.freedesktop.machine1.register-machine" && subject.user == "testuser") { @@ -54,4 +55,18 @@ machinectl terminate zurps (! run0 -u testuser systemctl is-active --user systemd-nspawn@zurps.service) +(! run0 -u testuser \ + busctl call \ + org.freedesktop.machine1 \ + /org/freedesktop/machine1 \ + org.freedesktop.machine1.Manager \ + RegisterMachine \ + 'sayssus' \ + shouldnotwork1 \ + 16 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 \ + "" \ + container \ + "$(systemctl show -p MainPID --value systemd-logind.service)" \ + "$PWD") + loginctl disable-linger testuser From 44e3c4c8bc031706a236acf9a8d6e5e7c5e2fd0a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Sat, 13 Sep 2025 01:28:24 +0100 Subject: [PATCH 2/4] machine: validate root directory over varlink Use strict validation to reject invalid directories as the D-Bus API already does Follow-up for 5b44c81ff868a4d1b78a74e4770f7a8b2f1d0f91 --- src/machine/machine-varlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/machine/machine-varlink.c b/src/machine/machine-varlink.c index b2b2f1f2db..5a2f67129e 100644 --- a/src/machine/machine-varlink.c +++ b/src/machine/machine-varlink.c @@ -137,7 +137,7 @@ int vl_method_register(sd_varlink *link, sd_json_variant *parameters, sd_varlink { "leaderProcessId", SD_JSON_VARIANT_OBJECT, machine_pidref, offsetof(Machine, leader), SD_JSON_STRICT }, { "supervisor", _SD_JSON_VARIANT_TYPE_INVALID, machine_pidref, offsetof(Machine, supervisor), SD_JSON_STRICT }, { "supervisorProcessId", SD_JSON_VARIANT_OBJECT, machine_pidref, offsetof(Machine, supervisor), SD_JSON_STRICT }, - { "rootDirectory", SD_JSON_VARIANT_STRING, json_dispatch_path, offsetof(Machine, root_directory), 0 }, + { "rootDirectory", SD_JSON_VARIANT_STRING, json_dispatch_path, offsetof(Machine, root_directory), SD_JSON_STRICT }, { "ifIndices", SD_JSON_VARIANT_ARRAY, machine_ifindices, 0, 0 }, { "vSockCid", _SD_JSON_VARIANT_TYPE_INVALID, machine_cid, offsetof(Machine, vsock_cid), 0 }, { "sshAddress", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Machine, ssh_address), SD_JSON_STRICT }, From e80394e19303add17091ec0ce44c34a94645e8cf Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 16 Sep 2025 15:49:26 +0100 Subject: [PATCH 3/4] man: clarify that machined RootDirectory parameter is informational only It's basically just a label, it is not used for any purpose --- man/org.freedesktop.machine1.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/man/org.freedesktop.machine1.xml b/man/org.freedesktop.machine1.xml index ccd81be659..fcabd6da2b 100644 --- a/man/org.freedesktop.machine1.xml +++ b/man/org.freedesktop.machine1.xml @@ -336,7 +336,8 @@ node /org/freedesktop/machine1 { be either container or vm indicating whether the machine to register is of the respective class. The leader PID should be the host PID of the init process of the container or the encapsulating process of the VM. If the root directory of the container is known and - available in the host's hierarchy, it should be passed. Otherwise, pass the empty string instead. Finally, the + available in the host's hierarchy, it should be passed (note that this is for informational purposes + only, and will not be used otherwise). Otherwise, pass the empty string instead. Finally, the scope properties are passed as array in the same way as to PID1's StartTransientUnit() method. Calling this method will internally register a transient scope unit for the calling client (utilizing the passed scope_properties) and move the leader PID into From 8324f9351c5d22fa49fa59d1ec3f71afa9408143 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 16 Sep 2025 15:51:08 +0100 Subject: [PATCH 4/4] machine: add a comment to clarify that root_directory is informational only To avoid any possible mistakes in the future, add a comment in the object declaration --- src/machine/machine.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/machine/machine.h b/src/machine/machine.h index 8f1a044f7a..4bdb049c5f 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -45,6 +45,8 @@ typedef struct Machine { char *state_file; char *service; + /* Note that the root directory is accepted as-is from the caller, including unprivileged users, so + * do not use it for anything but informational purposes. */ char *root_directory; char *unit;