Skip to content

Unit-path audit reads mode bits only, so a foreign-owned ancestor reads as safe #943

Description

@alexeyzimarev

DirectoryExposure.GrantingWrite decides whether a directory on the path to a service unit lets someone else replace what it holds. It reads mode bits, and mode bits are only half the property: the owner of a directory can always rename its entries, whatever the group and other bits say.

Two consequences, both raised on #938:

  • An ancestor owned by another unprivileged account at 0755 is reported clean. /srv/alice/bob-home holding Bob's unit directory, with /srv/alice owned by Alice, is the shape — Alice's ordinary owner-write lets her replace bob-home, and the audit sees nothing.
  • The sticky exemption has the same hole. Sticky restricts rename and unlink to an entry's own owner or the directory's owner, so a foreign-owned sticky directory is exempted although its owner retains exactly the power the exemption assumes away. /tmp is safe because root owns it, not because it is sticky.

Root-owned ancestors must stay exempt or nothing installs anywhere — /, /home and /Users are root-owned on every machine, and root can do this regardless of what we check.

Why it was not done in #938

The check needs each ancestor's st_uid compared against the effective uid. No BCL API exposes a file's owner, so this means a stat P/Invoke, and struct stat differs by platform, architecture and libc version — glibc historically routed it through __xstat, which is why .NET resolves it in its own native shim instead of marshalling the struct. Getting that subtly wrong inside a security check is worse than not having the check, so it wants its own change with real coverage on each target rather than riding along with a path-walk fix.

A no-op chmod to the current mode is not a substitute: it separates "mine" from "not mine", but not root-owned from Alice-owned, and everything has root-owned ancestors.

Scope

  • Read st_uid per ancestor on Linux and macOS, compared with geteuid().
  • Report an ancestor owned by a non-root account other than the installing user, and drop the sticky exemption for those.
  • Decide gate vs advisory. install refuses world-write today and daemon doctor advises on group-write; foreign ownership is closer to the gate, but only once the uid read is trustworthy on every target.
  • The README currently says the audit is mode-bits-only. That sentence comes out when this lands.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions