[Mrigbozurike]

Why

Follow-up to #5 (refuse root). For people who must start rdc from a root context (a system unit, provisioning), this lets it bind and then become a named account, so it never serves as root.

What

• [serve].user / --user NAME: when started as root, the daemon binds the port, initgroups/setgid/setuid to the account, verifies setuid(0) now fails, and only then opens the audit log and accepts requests. A failed drop is fatal.
• No environment mutation. The earlier version called set_var("HOME") after the runtime had started worker threads; that is gone. Paths that depend on the account are derived from its passwd entry (Account::state_dir()), so the audit log lands in the target's ~/.local/state/rdc (or Library/Application Support/rdc on macOS) unless [serve.audit].path is set. The id change itself is applied to all threads by libc, so doing it after the runtime starts is fine.
• The decision table (plan_with) is pure and takes an injected passwd lookup, so every branch is unit-tested without root.
• Docs are explicit that a separate account can only see the desktop on X11, with a worked example, and that this path is untested on real hardware.

Testing

• Linux, as root: without --user it refuses; with --user rdctest the process shows as rdctest in ps, serves /v1/whoami, the audit file is created under /home/rdctest with mode 0600, and $HOME of the invoking shell is untouched.
• cargo clippy --all-targets -- -D warnings, cargo test on Linux and Windows.
• Not tested on an X11 desktop; happy for this to wait until someone can.

This PR includes the commit from #5 until that merges; the diff will shrink to just the drop once it does.

[bscott]

Thanks. The drop itself is done right: initgroups, setgid, setuid in order, each checked, setuid(0) verified to fail, bind before drop and the listener only served after. What blocks it is around the edges:

1. The documented setup does not work as written. The config is loaded and enforce_permissions() runs as root before the drop, so with sudo -E it refuses the desktop user's config (owned by uid 1000, not 0), and with plain sudo it reads /root/.config/rdc/config.toml, which the example never creates. The root-owned config in /root is actually the right model (the dropped account cannot edit its own allowlist); please document that and fix the example.
2. SECURITY.md still says rdc serve exits when started as root; it needs the drop path described.
3. rdc audit resolves the default path from the reader's state dir, so after a drop the log is only reachable with --path. Please say so in the docs.

Small: CStr::from_ptr(pwd.pw_dir) has no null check; gid 0 is not rejected; --log-file is created root-owned before the drop; on Windows the error should say "not supported" rather than "cannot switch users without root".

Honest take on scope: this is +285 lines for a case that only helps on X11, which nobody has tested yet, and #5 already makes root safe everywhere. I would rather merge #5 now and hold this until someone has an X11 setup to test on. If you want to keep it moving, fix 1 to 3 and rebase onto release/0.4.0 (see CONTRIBUTING for the release-branch flow).

[bscott]

0.4.0 is released and this PR's base is now release/0.5.0. Please rebase onto it (it is main as of v0.4.0).

[Mrigbozurike]

All three blockers fixed, plus the smaller ones. Rebased onto release/0.5.0 on top of #5, signed off.

1. The documented setup now works, and the root-owned config is the model. You're right that this is the right shape, so the docs say why: the config is read before the drop from the starting user's directory, so under plain sudo the allowlist lives at /root/.config/rdc/config.toml and the dropped account cannot edit its own allowlist. The guide now states explicitly that sudo -E is wrong (it keeps your HOME, so enforce_permissions sees a uid-1000-owned file under a uid-0 process and refuses), and the worked example creates the root-owned config, a log directory owned by the target account, xhost +SI:localuser:rdc, and sudo DISPLAY=:0 rdc serve. I ran that recipe end to end as root: daemon serves as rdctest, $HOME untouched, audit file 0600 under the target account.
2. SECURITY.md now describes the drop path rather than claiming rdc always exits, including that the config is read before the drop and therefore root-owned.
3. rdc audit after a drop is documented: the reader resolves the default path from its own state directory, so either pass --path, or - better, and now what the example does - set [serve.audit].path in the root-owned config so sudo rdc audit reads the same setting. Verified both: with no explicit path rdc audit as root fails on /root/.local/state/rdc/audit.jsonl; with the path set it prints the entries.

Smaller ones:

• pw_dir is NULL-checked (some NSS backends return one); an account with no home is a clear error at startup rather than a bad audit path.
• gid 0 is rejected alongside uid 0, in both plan and drop_to, with a test covering a uid-999/gid-0 target.
• --log-file is opened before the drop, so the file is chowned to the target account while we still can - the descriptor survives the drop, but the file would otherwise stay root-owned and break rotation. Verified: the file ends up rdctest:rdctest.
• The Windows message is now "not supported on this platform: dropping privileges needs POSIX user ids", pointing at the scheduled task's run level, with a test asserting the Unix wording does not appear there.

On scope: agreed, and no argument if you want to merge #5 and park this. It is still +285 lines for a case only X11 benefits from, and I have no X11 desktop to test on - everything above was verified as root on a headless box. Happy for it to wait for someone who can run it against a real display.

[bscott]

Thanks for the thorough follow-up. I'm holding this one for 0.6.0. #5 goes into 0.5.0 on its own, and this needs an X11 test before it can land. Two fixes first, both about not handing out root:

1. Only a real root start may drop. plan_with accepts real uid 1000 with effective uid 0, a setuid-root copy of the binary. That user controls --user, --log-file and their own config, so they could become any account. Refuse unless current.uid == Some(0). Setuid-root should be refused, as in serve: refuse to run as root #5.
2. Don't open or chown the log file as root. Root opens --log-file and then libc::chowns it, and both follow symlinks. In the documented setup the rdc account owns /var/log/rdc, so it can replace serve.log with a symlink to /etc/shadow, and the next root start hands that file over. The simplest fix is to open the log file after the drop, as the target account, and remove chown_to.

Tests to add. The patch below adds a regression test for each problem. Both fail on your current head and should pass once fixed. The symlink test only catches the bug when run as root, so please run it under sudo once. If you remove chown_to (fix 2), delete that test and add one showing the log file is opened after the drop.

tests.patch (apply with git apply tests.patch)

diff --git a/src/privdrop.rs b/src/privdrop.rs
index cb16d8c..2dee00f 100644
--- a/src/privdrop.rs
+++ b/src/privdrop.rs
@@ -312,12 +312,37 @@ mod tests {
     }
 
     #[test]
-    fn setuid_root_can_still_drop() {
-        // Real uid 1000, effective 0: root privileges, so a drop is both possible and required.
+    fn setuid_root_may_not_drop() {
+        // Real uid 1000, effective 0: a setuid-root copy run by an ordinary user. That user
+        // controls --user, --log-file and their own config, so honouring --user would let them
+        // become any account (and, through --log-file, own any file). Only a real root start
+        // may drop; setuid-root is refused like any other root.
         let c = Current { uid: Some(1000), euid: Some(0), name: "alice".into() };
         assert!(c.is_root());
         assert!(plan_with(&c, None, fake).is_err());
-        assert!(matches!(plan_with(&c, Some("rdc"), fake).unwrap(), Plan::DropTo(a) if a.uid == 999));
+        assert!(plan_with(&c, Some("rdc"), fake).is_err(), "setuid-root must not be able to pick an account");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn log_file_handover_does_not_follow_symlinks() {
+        use std::os::unix::fs::MetadataExt;
+        // The log directory is owned by the target account in the documented setup, so that
+        // account can swap the log file for a symlink to a root-owned file before the next
+        // root start. Handing the log over must never give it the symlink's target.
+        let dir = std::env::temp_dir().join(format!("rdc-privdrop-test-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        let precious = dir.join("shadow");
+        std::fs::write(&precious, "root:x:0:0\n").unwrap();
+        let link = dir.join("serve.log");
+        std::os::unix::fs::symlink(&precious, &link).unwrap();
+        let owner_before = std::fs::metadata(&precious).unwrap().uid();
+        let acct = Account { name: "nobody".into(), uid: 65534, gid: 65534, home: "/nonexistent".into() };
+        let _ = chown_to(&link, &acct);
+        let owner_after = std::fs::metadata(&precious).unwrap().uid();
+        let _ = std::fs::remove_dir_all(&dir);
+        assert_eq!(owner_before, owner_after, "chown_to followed a symlink and gave away its target");
     }
 
     #[test]

Once #5 is merged, please rebase onto release/0.5.0 so this diff shrinks to just the drop. I'll retarget it to release/0.6.0 when that branch exists.

---

Generated by Claude Code

[Mrigbozurike]

Both fixed; understood on 0.6.0 and the X11 test. Signed off.

1. Only a real root start may drop. plan_with now refuses unless current.uid == Some(0), so a setuid-root copy cannot pick an account. Your setuid_root_may_not_drop replaces my old test.

2. The log file is no longer opened or chowned as root. chown_to is gone. --log-file is now opened after the drop by the target account, via a small logging::Sink that logs to stderr until then and swaps to the file once privileges are dropped; if that account cannot write the path, rdc says so and exits.

   I ran your attack against the fixed build — rdc account owns /var/log/rdc, serve.log is a symlink to a root-owned file:

   before: owner=root
   INFO rdc::privdrop::unix: dropped privileges to rdctest (uid 1001, gid 1002)
   Error: opening the log file after dropping privileges (the target account needs to be able
   to write /var/log/rdc/serve.log)
   Caused by: Permission denied (os error 13)
   after:  owner=root
   

   The target is untouched and the daemon refuses to start. On the normal path the file is created rdctest:rdctest mode 0600 and holds only post-drop lines. Per your note I dropped the symlink test with chown_to and added three in logging: the deferred file is not created before the drop, early lines do not land in it, and an undeferred file still opens immediately.

Docs and SECURITY.md updated for both. This still sits on top of #5; once that merges the diff is just the drop.

[Mrigbozurike]

Rebased onto release/0.5.0 now that #5 is in, so this is down to the drop alone: one commit, 10 files, +599/−70 (was two commits and 12 files). Signed off.

Two conflicts came out of the rebase, both in the docs rather than the code:

• CHANGELOG.md — the [serve].user entry moved to Unreleased, and the root-refusal bullet it carried is gone, since that shipped in 0.5.0's Changed section.
• docs/setup-linux.md — kept both the sandbox tables from systemd: sandbox the user unit #3 and the "Running as a dedicated user" section; they sit next to each other.

cargo fmt --check, cargo clippy --all-targets -- -D warnings and cargo test are clean (77 tests) on Linux and Windows. I re-ran the drop end to end as root against the rebased build to be sure the 0.5.0 changes to main.rs and server/mod.rs didn't disturb it:

WARN  rdc::server: started with root privileges; will drop to rdctest after binding
INFO  rdc::privdrop::unix: dropped privileges to rdctest (uid 1001, gid 1002)

-rw------- 1 rdctest rdctest  serve.log                              # --log-file, opened after the drop
-rw------- 1 rdctest rdctest  /home/rdctest/.local/state/rdc/audit.jsonl

The invoking root account's state directory is never created.

Still parked for 0.6.0 and still untested on a real X11 desktop — everything above is a headless box. Happy for it to wait for someone with a display; retarget whenever release/0.6.0 exists.

[bscott]

@Mrigbozurike Doing a final review and testing then I'll merge