Skip to content

serve: refuse to run as root - #5

Merged
bscott merged 1 commit into
bscott:release/0.5.0from
Mrigbozurike:drop-privileges
Oct 7, 2026
Merged

bscott merged 1 commit into
bscott:release/0.5.0from
Mrigbozurike:drop-privileges

Conversation

@Mrigbozurike

@Mrigbozurike Mrigbozurike commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

rdc never needs root (the port is unprivileged, everything else happens in the desktop session), but nothing stopped it being started under sudo or from a system unit, and then the whole remote-control surface ran as root.

What

  • rdc serve exits with a clear message if it is root. The decision is a pure function with tests.
  • rdc doctor reports the process user and flags root.
  • SECURITY.md and the README state the root policy and that nothing persists unless rdc service install is run explicitly.

Split out of the original #5 as requested; the [serve].user privilege drop is now its own PR built on top of this one. Verified on Linux (as root it refuses; as a user it serves) and Windows (builds, tests). Signed off.

@Mrigbozurike
Mrigbozurike force-pushed the drop-privileges branch 2 times, most recently from 0afbf43 to ad012bd Compare September 10, 2026 09:48
@bscott

bscott commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Thanks. Refusing to run as root is right and I'd like to merge that part quickly.

The privilege-drop half is a lot of code for a case the PR itself says is only useful on X11, which nobody has tested yet, and set_var after the tokio runtime has started worker threads is undefined behaviour in Rust.

Could you split this into two PRs?

  1. Refuse to run as root, with the doctor line. I'll merge that as soon as it's up.
  2. The [serve].user drop as a separate PR, so it can be tested on an X11 box first.

Also, please sign off the commits (git commit -s).

@Mrigbozurike Mrigbozurike changed the title serve: refuse to run as root, drop to [serve].user after bind serve: refuse to run as root Sep 12, 2026
@Mrigbozurike

Copy link
Copy Markdown
Contributor Author

Split as requested. This PR is now just the root refusal + doctor line (317dbff, signed off). The [serve].user drop is in #13, rebuilt without set_var.

@bscott

bscott commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Thanks for the split, this is the right shape. One fix and a branch change:

  1. It checks getuid() only. A setuid-root binary run by a normal user has euid 0 and passes. Please carry both uids, treat either being 0 as root, and add the two mixed-uid cases to the tests.

Small: the non-Unix current() invents uid 1000, which doctor then prints on Windows; print just the name there. And SECURITY.md's "rdc serve alone leaves nothing behind" is not quite true (it writes the audit log); "installs no service" is accurate.

Branch: we moved to release branches (see CONTRIBUTING). This goes into 0.5.0; please rebase onto release/0.4.0 (main.rs and CHANGELOG conflict, both trivial) and I will retarget the PR when release/0.5.0 exists. I would like to merge this one first.

@bscott
bscott changed the base branch from main to release/0.5.0 September 14, 2026 02:57
@bscott
bscott self-requested a review as a code owner September 14, 2026 02:57
@bscott

bscott commented Sep 14, 2026

Copy link
Copy Markdown
Owner

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).

Signed-off-by: mrigbozurike <drbillionbook@gmail.com>
@Mrigbozurike

Copy link
Copy Markdown
Contributor Author

Fixed and rebased onto release/0.5.0, signed off.

  1. Both uids. Current now carries uid and euid, and is_root() is true if either is 0 -- a setuid-root binary run by an ordinary user (uid 1000, euid 0) has every root privilege, and real-uid-0-with-dropped-euid can take it back with seteuid. Both mixed cases are tests (setuid_root_is_refused, root_with_dropped_euid_is_refused), along with the ordinary and both-zero cases.
  2. No invented uid off Unix. The ids are Options, None on platforms that have no such concept, so is_root() is false there by construction rather than by guessing 1000. describe() prints alice (uid 1000), alice (uid 1000, euid 0) when they differ, or just alice where there are no uids -- which is what doctor shows on Windows. Covered by a test.
  3. SECURITY.md corrected. It now says rdc serve installs no service, and that it does write the audit log and any --log-file under the user's state directory -- the previous "leaves nothing behind" was wrong.

Verified on Linux both ways: as root it refuses with the id in the message, and under setpriv --reuid=1000 doctor prints [ok ] process user: ubuntu (uid 1000).

@bscott
bscott merged commit 87e5d49 into bscott:release/0.5.0 Oct 7, 2026
4 checks passed
bscott pushed a commit that referenced this pull request Oct 7, 2026
Bump the version to 0.5.0 and move Unreleased under a dated 0.5.0 section. The root refusal from #5 is listed under Changed as a breaking change; the window-title entry from #4 states that titles are on by default and reach audit streams.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6FWzFP6qZXMn5TXipJPRw
Signed-off-by: Brian Scott <claudeai@bscott.email>
This was referenced Oct 7, 2026
Mrigbozurike pushed a commit to Mrigbozurike/rdc-least-privilege-runtime that referenced this pull request Oct 8, 2026
Merges bscott#4 (audit: record focused window title and screenshot hash) at 7e07e04. The only conflict is CHANGELOG.md, where bscott#4 and bscott#5 both add an entry under Unreleased; both are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6FWzFP6qZXMn5TXipJPRw
Signed-off-by: Brian Scott <claudeai@bscott.email>
Mrigbozurike pushed a commit to Mrigbozurike/rdc-least-privilege-runtime that referenced this pull request Oct 8, 2026
Merges bscott#3 (systemd: sandbox the user unit) at fc66dbf. Conflicts: CHANGELOG.md (each PR adds an entry under Unreleased; all are kept) and docs/setup-linux.md (bscott#3 and bscott#5 add sections at the same place; both are kept, the unit's sandbox section first under Running, then Persistence is opt-in).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6FWzFP6qZXMn5TXipJPRw
Signed-off-by: Brian Scott <claudeai@bscott.email>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants