Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polkit-agent-helper-1 is setuid root and runnable by ordinary users, does it need to be? #169

Open
polkit-github-migration-bot opened this issue Feb 2, 2022 · 3 comments · May be fixed by #501

Comments

@polkit-github-migration-bot
Copy link
Collaborator

In gitlab.freedesktop.org by smcv on Feb 2, 2022, 23:29

Link to the original issue: https://gitlab.freedesktop.org/polkit/polkit/-/issues/168
I've been thinking about ways to reduce setuid attack surface to avoid things like CVE-2021-4034 happening in the future.

polkit currently installs polkit-agent-helper-1, which is used by polkit agents to re-authenticate a user. This is typically done by running a PAM stack, which is required to be done as root and with privileges. The -shadow implementation bypasses PAM and reads /etc/shadow directly, which similarly requires privileges.

setuid executables run in a hostile environment: a potential attacker controls every aspect of their execution environment (environment variables, argv, open fds, controlling terminal, signal dispositions...), and if setuid executable makes a security-sensitive assumption about any of those process parameters, an attacker might be able to exploit that.

One possibility to harden this would be to use systemd socket-activation to launch one instance of something functionally equivalent to the current polkit-agent-helper-1 (except not setuid, and with the user to authenticate written to its stdin instead of being passed in argv) per connection to a socket. libpolkit-agent-1 would run a separate non-setuid program, reusing the current polkit-agent-helper-1 filename for backwards compatibility, which would just connect to the socket and proxy the PAM conversation. Non-systemd systems would have to either continue to build and install a setuid polkit-agent-helper-1 (no improvement, but no regression either), or implement socket-activation with inetd or something.

Another possible route would be for the program that is run by libpolkit-agent-1 to be unprivileged, and just make a D-Bus call to polkitd, fd-passing a pair of pipes (or maybe a single AF_UNIX socket). polkitd could execute a setuid program (directly equivalent to the current helper) installed with rwsr-x--- permissions and owned by root:polkitd (this would need a new polkitd group, obviously), so that users other than polkitd cannot possibly exploit it via creative choices of execution environment. This is similar to the way dbus-daemon-launch-helper is setuid root, but only the system bus uid (typically dbus or messagebus) can run it.

(In old versions of polkit with less privilege-separation, this would ironically have been easier, because polkitd ran as root and so no setuid would have been required.)

This would also allow rate-limiting authentication attempts, if that's desirable: if using systemd socket activation, this would be systemd configuration, or if using D-Bus calls to polkitd, it would be up to polkitd.

@polkit-github-migration-bot
Copy link
Collaborator Author

In gitlab.freedesktop.org by smcv on Feb 3, 2022, 01:45

Limitations of both the routes I suggested:

  • The PAM (or equivalent) code obviously still needs to run with privileges, so if there are exploitable bugs in the code that parses strings passed over stdin/stdout and converts them into PAM operations, those are not mitigated. However, those (and possibly argv) would become the only untrusted inputs, which still seems like a win.

  • If there are PAM modules in polkit's stack that expect to be invoked from a setuid process (effective uid 0, real uid = their caller) and make decisions involving the real uid, they will be unable to do so.

  • If there are PAM modules in polkit's stack that expect to be invoked from a setuid process and make decisions involving the (untrusted!) environment variables in its execution environment, they will be unable to do so. (Arguably this is a feature rather than a limitation: it stops those PAM modules from being compromised via attacker-chosen environment variables.)

  • polkitd, running as the polkitd uid, is still able to authorize root-equivalent actions (that's sort of the point!), so the privilege separation between root and the polkitd uid is never going to be amazingly strong.

@polkit-github-migration-bot
Copy link
Collaborator Author

In gitlab.freedesktop.org by clx814727823 on Jul 19, 2022, 08:39

The PAM (or equivalent) code obviously still needs to run with privileges

Why? I mean that, in fact, pam_unix.so will fork and exec unix_chkpwd, which have suid to check /etc/shadow.

check https://github.com/linux-pam/linux-pam/blob/b872b6e68a60ae351ca4c7eea6dfe95cd8f8d130/modules/pam_unix/support.c#L469

So why we need a suid helper at all?

bluca added a commit to bluca/polkit that referenced this issue Sep 16, 2024
…t SETUID

SETUID binaries are considered harmful, as te execution context is
under the control of unprivileged attackers.

Enhance the polkit pam agent helper with a new mode: when running
under systemd, add a socket-activated service that the helper will
run under, as root. The agent talks to this service via AF_UNIX
instead of spawning it, and STDIN/STDOUT are connected as before.
The helper can make use of PID FDs and SO_PEERCRED to reliably
identify the caller. In order to do this, a third version of the
auth D-Bus method is added, that also takes a subject, built using
the PID FD.
If the AF_UNIX socket is not present, the agent will fork the
helper as before, with no changes.

Fixes polkit-org#169
@bluca
Copy link
Member

bluca commented Sep 16, 2024

I've opted for a slight variation: the agent will talk directory to the socket-activated privileged helper via AF_UNIX, instead of going through a third intermediate binary. The privileged socket-activated helper will identify the caller via PID FD so that it is safe against reuse attacks.

bluca added a commit to bluca/polkit that referenced this issue Sep 17, 2024
…t SETUID

SETUID binaries are considered harmful, as te execution context is
under the control of unprivileged attackers.

Enhance the polkit pam agent helper with a new mode: when running
under systemd, add a socket-activated service that the helper will
run under, as root. The agent talks to this service via AF_UNIX
instead of spawning it, and STDIN/STDOUT are connected as before.
The helper can make use of PID FDs and SO_PEERCRED to reliably
identify the caller. In order to do this, a third version of the
auth D-Bus method is added, that also takes a subject, built using
the PID FD.
If the AF_UNIX socket is not present, the agent will fork the
helper as before, with no changes.

Fixes polkit-org#169
bluca added a commit to bluca/polkit that referenced this issue Sep 17, 2024
…t SETUID

SETUID binaries are considered harmful, as te execution context is
under the control of unprivileged attackers.

Enhance the polkit pam agent helper with a new mode: when running
under systemd, add a socket-activated service that the helper will
run under, as root. The agent talks to this service via AF_UNIX
instead of spawning it, and STDIN/STDOUT are connected as before.
The helper can make use of PID FDs and SO_PEERCRED to reliably
identify the caller. In order to do this, a third version of the
auth D-Bus method is added, that also takes a subject, built using
the PID FD.
If the AF_UNIX socket is not present, the agent will fork the
helper as before, with no changes.

Fixes polkit-org#169
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 a pull request may close this issue.

2 participants