Skip to content

fix(helper): write logs and debug images as the user, not root - #196

Merged
phucvinh57 merged 1 commit into
mainfrom
fix/helper-symlink-chown
Nov 1, 2026
Merged

phucvinh57 merged 1 commit into
mainfrom
fix/helper-symlink-chown

Conversation

@phucvinh57

@phucvinh57 phucvinh57 commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

Summary

Security fix. With debug on, the root-run helper created the log file in a user-writable dir and then chown()ed it to the user. chown() follows symlinks, so a symlink placed in that dir could make root change ownership of a file outside it. Debug face images had the same problem, and dirs were created 0777. File writes into user dirs now run with the user's filesystem identity, so the kernel checks them with the user's permissions. Part of the stacked audit PRs starting at #193 (audit).

Closes: N/A / Discussion: N/A

Changes

  • auth/core: new ScopedFsUser (per-thread setfsuid/setfsgid). While held, the kernel drops root's file capabilities, so every path component is checked as the user and new files are owned by the user.
  • auth/core: setupConfig runs under it; fixOwnership removed; data dirs created 0700; unknown users are refused instead of falling back to $HOME or /.
  • auth/pam (helper): log sink opened under ScopedFsUser. Before auth the helper drops root's supplementary groups; if that fails it exits 1 (PAM_AUTH_ERR, fail closed).
  • auth/face: saveFailedFace writes debug images under ScopedFsUser.
  • auth/test: new fs_user test plants symlinks and checks writes are denied. Needs root: ctest skips it as non-root; CI runs it with sudo.

Verification

$ make build-auth
build OK
$ clang-format --dry-run --Werror <changed auth files>
OK
$ unshare --map-root-user --map-auto auth/build/test/fs_user/fs_user_test
16 checks PASS, including:
[PASS] write through symlink is denied
[PASS] victim content unchanged
[PASS] create inside symlinked root dir is denied
ALL PASS

Manual testing:

Distro / version Package (.deb/.rpm/AUR/source) DE / display manager Hardware (camera, IR, fingerprint) Flow tested (login, sudo, polkit, lock screen, app) Result

Not tested: real PAM login/sudo in a VM.

Review report

AI review: 0 important, 1 nit, 2 pre-existing. Nit (comment placement) fixed. Security review ran: no hole found in the fix (kernel cap drop, per-thread creds, spdlog fd lifetime, setgroups fail-closed checked). Pre-existing follow-ups (root still opens user config/faces/db/model files for reading; early readConfig warnings go to stdout) are handled in a later PR of this stack.

Risk

  • Low: docs, UI copy, styling, tests
  • Medium: app logic, Rust/Tauri commands, face/fingerprint pipeline
  • High: PAM module or helper, install/remove scripts, PAM config, packaging, CI/release workflows

What could break, and for whom: users of the PAM helper. A failing setgroups now fails auth closed. Upgrade note: a debugs/ dir left root-owned by very old releases makes debug image saving stop (debug feature only; login unaffected).

How to revert

git revert is enough (reintroduces the bug).

Checklist

  • I read the whole diff and can explain every change.
  • Verification above shows real output, not a claim.
  • All Important review findings are fixed or answered.
  • Docs are updated if behavior, install, packaging, or distro support changed.

@phucvinh57
phucvinh57 added this pull request to stack #199 October 31, 2026 18:58
@phucvinh57
phucvinh57 force-pushed the fix/helper-symlink-chown branch 2 times, most recently from 5c7eb97 to 2ebf206 Compare October 31, 2026 19:22
Base automatically changed from ci/cpp-tests to main October 31, 2026 19:26
@phucvinh57
phucvinh57 force-pushed the fix/helper-symlink-chown branch 2 times, most recently from 69c5325 to 673ca63 Compare October 31, 2026 19:26
@phucvinh57
phucvinh57 marked this pull request as ready for review October 31, 2026 19:39
With debug on, the root-run helper created ~/.local/share/.../logs/<day>.log
and then chown()ed it to the user. chown() follows symlinks, so a symlink
planted in the user-writable log or debug dir could make root hand
ownership of another file to the user. Debug face images had the same
problem, and mkdir used 0777.

Add ScopedFsUser (per-thread setfsuid/setfsgid). While it is held the
kernel drops root's file capabilities, so every path component is checked
with the user's permissions and new files are owned by the user. Use it
for setupConfig, the log sink and saveFailedFace; remove fixOwnership;
create data dirs 0700; refuse unknown users instead of falling back to
$HOME or /. The helper drops root's supplementary groups before auth so
they don't apply under the guard.

fs_user_test plants symlinks and checks writes are denied. It needs root
(CI runs it with sudo; locally: unshare --map-root-user --map-auto).
@phucvinh57
phucvinh57 force-pushed the fix/helper-symlink-chown branch from 673ca63 to 2ffdd64 Compare October 31, 2026 20:03
@phucvinh57
phucvinh57 merged commit 5cfd3bf into main Nov 1, 2026
2 checks passed
@phucvinh57
phucvinh57 deleted the fix/helper-symlink-chown branch November 1, 2026 03:42
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.

1 participant