diff --git a/src/syscall/exec.c b/src/syscall/exec.c index 8f834599..fa3d40c1 100644 --- a/src/syscall/exec.c +++ b/src/syscall/exec.c @@ -35,6 +35,7 @@ #include "runtime/futex.h" #include "syscall/abi.h" +#include "syscall/chown-overlay.h" #include "syscall/exec.h" #include "syscall/fuse.h" #include "syscall/internal.h" @@ -369,6 +370,45 @@ static bool exec_matches_fakeroot_target(const struct stat *st) return marked_st.st_dev == st->st_dev && marked_st.st_ino == st->st_ino; } +/* Is ID something the guest can actually mean? + * + * A sysroot is an ordinary directory tree owned by whoever unpacked it, and + * elfuse maps no host IDs into the guest, so most files report the invoking + * macOS user (e.g. 501) -- an ID that exists nowhere in the guest's own + * /etc/passwd. Honouring setuid on such a file would leave the process at an + * effective ID that is neither root nor its own, failing both `euid == 0` + * privilege checks and ownership comparisons against guest IDs, and the + * granted ID would follow whoever happens to own the tree rather than anything + * the guest chose. + * + * Accept only root and the caller's own ID, and only when both views of + * ownership agree on it. + * + * physical is what the host reports; seen is what the guest's own stat reports, + * after the virtual chown overlay. Elevating needs both because each view alone + * fails in a different direction. Reading the set-id owner through the overlay + * would make root self-service, since the overlay takes any unprivileged + * guest's chown -- chown its own file to 0, set the bit, exec. Reading it only + * from the host would let a file the guest's own stat says belongs to someone + * else still elevate, because a physically root-owned file keeps elevating + * after the guest chowns it away. + * + * Requiring agreement grants nothing new: root still means a file the host + * really owns as root, and a guest chown can only ever withdraw an elevation, + * never create one. The caller's own ID makes the bit a no-op, as on Linux. + * Anything else leaves the ID untouched and the program simply runs + * unprivileged -- the outcome Linux gives for a setuid binary on a nosuid + * mount, and never an exec failure. + */ +static bool exec_id_may_elevate(uint32_t physical, + uint32_t seen, + uint32_t self_id) +{ + if (physical != seen) + return false; + return physical == 0 || physical == self_id; +} + static int check_exec_permission(const struct stat *st) { uint32_t uid = proc_get_euid(); @@ -549,8 +589,22 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + /* Judge access by the ownership the guest sees: fs-stat.c reports every + * guest stat through the virtual chown overlay, so the physical owner would + * refuse a file the guest's own stat says it may execute. Trusting a + * guest-writable overlay here -- chown_result records the intended owner on + * any EPERM, unchecked -- is a deliberate trade. A guest can chown a file + * into its own name to pass a check the physical mode refuses, and gains + * nothing: the host open must still succeed on the physical file. + * + * exec_st itself stays physical, and the set-id decision below takes both: + * it grants only where the two agree, so the overlay can withdraw an + * elevation but never conjure one. + */ + struct stat exec_st_seen = exec_st; + chown_overlay_apply(&exec_st_seen); - err = check_exec_permission(&exec_st); + err = check_exec_permission(&exec_st_seen); if (err < 0) { goto fail; } @@ -710,6 +764,7 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + chown_overlay_apply(&interp_st); err = check_exec_permission(&interp_st); if (err < 0) { goto fail; @@ -721,20 +776,32 @@ int64_t sys_execve(hv_vcpu_t vcpu, goto fail; } - /* Compute setuid/setgid from the directly-executed file, matching Linux - * kernel behaviour (fs/exec.c bprm_fill_uid). Scripts are - * deliberately excluded: the kernel ignores setuid/setgid on shebang - * scripts to prevent privilege escalation via interpreter manipulation. - * S_ISGID is only effective when the group-execute bit is also set, - * matching the kernel's mandatory-locking vs setgid distinction. + /* Compute setuid/setgid from the directly-executed file (fs/exec.c + * bprm_fill_uid). A set-id bit on the shebang script itself is ignored, as + * on Linux, so the interpreter named in the script cannot be manipulated + * into carrying the script's privilege. S_ISGID is only effective when the + * group-execute bit is also set, matching the kernel's mandatory-locking vs + * setgid distinction. + * + * Where this deliberately stops short of Linux: the kernel derives file + * creds from the file it finally executes, so a set-id *interpreter* named + * in a "#!" line does elevate there. exec_st is the directly-executed file + * and is never refreshed after the shebang loop, so it does not here. + * Elevating through an interpreter is unsupported, not accidentally lost. */ uint32_t new_euid = proc_get_euid(); uint32_t new_egid = proc_get_egid(); if (have_exec_st && !exec_is_script && S_ISREG(exec_st.st_mode)) { - if (exec_st.st_mode & S_ISUID) { + if ((exec_st.st_mode & S_ISUID) && + exec_id_may_elevate((uint32_t) exec_st.st_uid, + (uint32_t) exec_st_seen.st_uid, + proc_get_uid())) { new_euid = (uint32_t) exec_st.st_uid; } - if ((exec_st.st_mode & S_ISGID) && (exec_st.st_mode & S_IXGRP)) { + if ((exec_st.st_mode & S_ISGID) && (exec_st.st_mode & S_IXGRP) && + exec_id_may_elevate((uint32_t) exec_st.st_gid, + (uint32_t) exec_st_seen.st_gid, + proc_get_gid())) { new_egid = (uint32_t) exec_st.st_gid; } } @@ -844,6 +911,7 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + chown_overlay_apply(&interp_st); err = check_exec_permission(&interp_st); if (err < 0) { diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index 7fc9c10d..592a6d99 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -277,6 +277,8 @@ QEMU_SKIP=" test-credentials test-credentials-fakeroot test-fakeroot-exec + test-setuid-exec + test-setuid-exec-fakeroot test-sched-policy test-rseq test-tier-a @@ -331,6 +333,10 @@ QEMU_SKIP=" # test-fakeroot-exec: ELFUSE_FAKEROOT_EXEC is an elfuse-only escape hatch -- # a real kernel has no notion of an executable that turns on fakeroot, so # the marked exec simply stays unprivileged there. +# test-setuid-exec / -fakeroot: reasons about elfuse's virtual chown overlay +# and about host UIDs no sysroot maps into the guest, neither of which a +# real kernel has; the qemu reference lane also runs as genuine root, where +# a chown to any owner succeeds physically and setuid is honoured from it. # test-sched-policy: exercises elfuse's explicitly-a-stub scheduler policy # layer (see the file's own header) -- RT class changes are always # -EPERM'd regardless of privilege, whereas real root can set them. @@ -775,6 +781,10 @@ run_unit_tests() # interrupted run cannot leave it set for anything that follows. ELFUSE_FAKEROOT_EXEC="$bindir/test-fakeroot-exec" \ test_rc "$runner" "test-fakeroot-exec" 0 "$bindir/test-fakeroot-exec" + test_check "$runner" "test-setuid-exec" "all tests passed" \ + "$bindir/test-setuid-exec" + test_check "$runner" "test-setuid-exec-fakeroot" "all tests passed" \ + --fakeroot "$bindir/test-setuid-exec" printf "\nScheduler policy stub\n" test_rc "$runner" "test-sched-policy" 0 "$bindir/test-sched-policy" @@ -1242,7 +1252,7 @@ run_suite() # observed counts diverge. apple-unknown is the fallback row for SoC strings the # detector does not recognize yet. EXPECTED_BASELINES=( - "elfuse-aarch64|239|0" + "elfuse-aarch64|241|0" "qemu-aarch64|218|0" "elfuse-x86_64:apple-m1-m2|71|0" "elfuse-x86_64:apple-m3-plus|71|0" diff --git a/tests/test-setuid-exec.c b/tests/test-setuid-exec.c new file mode 100644 index 00000000..327f706f --- /dev/null +++ b/tests/test-setuid-exec.c @@ -0,0 +1,306 @@ +/* setuid-on-exec must grant an ID the guest can actually mean. + * + * A sysroot's files are owned by whoever unpacked them on the host, and no host + * IDs are mapped into the guest. Honouring setuid on such a file would leave + * the process at an effective ID that exists nowhere in the guest -- neither + * root nor its own -- so the bit has to be ignored there, the way Linux ignores + * it on a nosuid mount. + * + * Ownership the guest merely recorded is no better. The virtual chown overlay + * accepts any unprivileged guest's chown, so an owner of 0 that came from it + * proves nothing about privilege; only physical root ownership may elevate. + * That is the case this test cannot stage without being root already, so what + * it pins is the other half: a virtual owner, foreign or root, never elevates. + * + * Note which owner does the work. The set-id path reads the *physical* stat, + * and mkstemp already leaves the helper owned by the host user (501 or + * whatever unpacked the tree), which is foreign to the guest either way. The + * virtual chowns below stage the guest-visible half; the physical host owner + * is what the exec actually sees and refuses to honour. + * + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* An ID that is neither root nor any caller this test runs as, so the "owner + * the guest cannot mean" case is staged deterministically instead of relying on + * whichever host UID happens to own /tmp files. + */ +#define FOREIGN_UID 4242u + +/* Unique per invocation. A fixed name in the shared /tmp would let two runs + * truncate each other's helper, and the cleanup unlink would remove a file a + * concurrent run still needs. mkstemp creates it O_EXCL and mode 0600, so no + * other run can be holding the same path. + */ +static char helper_path[] = "/tmp/test-setuid-exec-XXXXXX"; + +/* Re-executed as the setuid helper: report whether euid matches expectations. + */ +static int child_main(const char *want_str) +{ + unsigned long want = strtoul(want_str, NULL, 10); + unsigned got = (unsigned) geteuid(); + if ((unsigned long) got != want) { + fprintf(stderr, " child: euid=%u expected=%lu\n", got, want); + return 2; + } + return 0; +} + +/* Copy this binary to a fresh helper_path and mark it setuid. The fd is closed + * before returning: exec'ing a file still open for writing is ETXTBSY. + */ +static int build_setuid_helper(void) +{ + char src[4096]; + ssize_t n = readlink("/proc/self/exe", src, sizeof(src) - 1); + if (n <= 0) + return -1; + src[n] = '\0'; + + int in = open(src, O_RDONLY); + if (in < 0) + return -1; + int out = mkstemp(helper_path); + if (out < 0) { + close(in); + return -1; + } + + /* Past mkstemp the file exists, so every failure below has to unlink it + * rather than leave a setuid-marked copy of this binary in the shared /tmp. + */ + int rc = 0; + char buf[65536]; + ssize_t got = 0; + while ((got = read(in, buf, sizeof(buf))) > 0) { + ssize_t off = 0; + while (off < got) { + ssize_t put = write(out, buf + off, (size_t) (got - off)); + if (put <= 0) { + rc = -1; + break; + } + off += put; + } + if (rc != 0) + break; + } + close(in); + /* Set the bit through the fd we own rather than re-opening by name. */ + if (rc == 0 && (got < 0 || fchmod(out, 04755) != 0)) + rc = -1; + if (close(out) != 0 && rc == 0) + rc = -1; + if (rc != 0) + (void) unlink(helper_path); + return rc; +} + +/* Run the helper and return its exit status, or -1 if it could not run. */ +static int run_helper(unsigned want_euid) +{ + char expect[32]; + snprintf(expect, sizeof(expect), "%u", want_euid); + + pid_t pid = fork(); + if (pid < 0) + return -1; + if (pid == 0) { + execl(helper_path, helper_path, "--child", expect, (char *) NULL); + _exit(127); + } + int status = 0; + if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status)) + return -1; + return WEXITSTATUS(status); +} + +/* Hand the helper to owner and re-arm the setuid bit, which a chown drops. + * Returns 1 when the guest-visible owner really is owner afterwards, 0 when the + * chown was refused, -1 when the staging itself broke. + */ +static int stage_owner(unsigned owner) +{ + errno = 0; + if (chown(helper_path, (uid_t) owner, (gid_t) -1) != 0) + return 0; + if (chmod(helper_path, 04755) != 0) + return -1; + struct stat st; + if (stat(helper_path, &st) != 0) + return -1; + /* exec reads ownership the same way stat does, so a stat that does not show + * the chown means the two would disagree about what was staged. + */ + if ((unsigned) st.st_uid != owner) { + fprintf(stderr, " stat reports owner=%u after chown to %u\n", + (unsigned) st.st_uid, owner); + return -1; + } + return 1; +} + +/* Whether the *guest* runs as root, which decides what a chown means to the + * emulation. It says nothing about the host: elfuse can be running as host root + * while the guest is uid 1000, so a chown the guest performs may still land + * physically. Checks that turn on that difference resolve it from the observed + * outcome rather than from this. + */ +static int caller_is_privileged(void) +{ + return geteuid() == 0; +} + +int main(int argc, char **argv) +{ + if (argc >= 3 && strcmp(argv[1], "--child") == 0) + return child_main(argv[2]); + + int failures = 0; + /* Checks that actually exercised an exec. Without this a run where every + * interesting case skipped -- a privileged caller, say -- would print the + * success line and exit 0 having proved nothing. + */ + int checks_run = 0; + unsigned self_uid = (unsigned) getuid(); + unsigned self_euid = (unsigned) geteuid(); + + printf("test-setuid-exec: 1. build setuid helper... "); + if (build_setuid_helper() != 0) { + printf("FAIL (could not build helper: %m)\n"); + return 1; + } + struct stat st; + if (stat(helper_path, &st) != 0) { + printf("FAIL (stat: %m)\n"); + failures++; + goto cleanup; + } + printf("PASS (owner=%u mode=%04o)\n", (unsigned) st.st_uid, + st.st_mode & 07777); + + /* 2. An owner the guest cannot mean must not become the effective ID. + * + * No skip: the physical owner staged by mkstemp is already foreign to the + * guest, whichever host user that is, so the case holds whether or not the + * virtual chown below is recorded. Attempting the chown on top only widens + * it -- the guest then sees a foreign owner too, and both views agree. + */ + printf("test-setuid-exec: 2. foreign owner does not elevate... "); + { + unsigned physical_owner = (unsigned) st.st_uid; + int staged = caller_is_privileged() ? 0 : stage_owner(FOREIGN_UID); + if (staged < 0) { + printf("FAIL (could not stage foreign owner)\n"); + failures++; + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0 || rc == 127) { + printf("FAIL (helper did not run)\n"); + failures++; + } else if (rc != 0) { + printf("FAIL (euid moved to an owner the guest cannot mean)\n"); + failures++; + } else if (staged == 1) { + printf("PASS (owner=%u physical, %u virtual, euid stayed %u)\n", + physical_owner, FOREIGN_UID, self_euid); + } else { + printf("PASS (physical owner=%u ignored, euid stayed %u)\n", + physical_owner, self_euid); + } + } + } + + /* 3. Nor may a *recorded* root owner. The chown overlay takes any guest's + * chown, so honouring the owner it reports would let an unprivileged + * process chown its own file to 0, set the bit, and exec its way to root. + * The regression guard for that escalation. + * + * A guest stat cannot tell a recorded chown from one the host really + * performed -- the overlay is invisible from in here, and both report + * owner 0. That distinction matters, because when elfuse itself runs as + * host root the chown below is physical, and a physically root-owned + * setuid file is *supposed* to elevate. Resolve it from the outcome + * instead: elevation to 0 only happens when the physical owner is 0, which + * only a real chown can produce. Staying put and landing on root are both + * correct, one per staging; anything else is the bug. + */ + printf("test-setuid-exec: 3. virtual root owner does not elevate... "); + { + int staged = stage_owner(0); + if (staged < 0) { + printf("FAIL (could not stage root owner)\n"); + failures++; + } else if (staged == 0) { + printf("SKIP (chown to root refused, as on a real kernel)\n"); + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0 || rc == 127) { + printf("FAIL (helper did not run)\n"); + failures++; + } else if (rc == 0) { + printf("PASS (stat says owner=0, euid stayed %u)\n", self_euid); + } else if (run_helper(0) == 0) { + printf("PASS (host chown was real, physical root elevated)\n"); + } else { + printf("FAIL (euid is neither %u nor 0; see the report)\n", + self_euid); + failures++; + } + } + } + + /* 4. The permission check must use the owner the guest sees, unlike the + * set-id owner above. A file the guest owns with owner-only permission has + * to run: judging it by the physical owner would fall through to the + * "other" bits and refuse an executable the guest's own stat says it owns. + */ + printf("test-setuid-exec: 4. guest-owned owner-only file runs... "); + if (chown(helper_path, self_uid, (gid_t) -1) != 0) { + printf("SKIP (cannot take ownership: %m)\n"); + } else if (chmod(helper_path, 0700) != 0) { + printf("FAIL (chmod: %m)\n"); + failures++; + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0) { + printf("FAIL (helper did not run at all)\n"); + failures++; + } else if (rc == 127) { + printf("FAIL (exec refused for a file the guest owns)\n"); + failures++; + } else if (rc != 0) { + printf("FAIL (helper exited %d)\n", rc); + failures++; + } else { + printf("PASS\n"); + } + } + +cleanup: + (void) unlink(helper_path); + + if (failures == 0 && checks_run == 0) { + printf("test-setuid-exec: no check exercised an exec -- FAIL\n"); + return 1; + } + if (failures == 0) + printf("test-setuid-exec: all tests passed -- PASS\n"); + else + printf("test-setuid-exec: %d failed\n", failures); + return failures != 0; +}