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 diff --git a/src/machine/machine-varlink.c b/src/machine/machine-varlink.c index 776654b51d..5a2f67129e 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" @@ -136,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 }, @@ -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/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; 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