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

service pidfile handling is probably broken and insecure #647

Open
eli-schwartz opened this issue Aug 30, 2023 · 3 comments
Open

service pidfile handling is probably broken and insecure #647

eli-schwartz opened this issue Aug 30, 2023 · 3 comments

Comments

@eli-schwartz
Copy link

eli-schwartz commented Aug 30, 2023

tl;dr

Killing a service via pidfile should use the command_user privileges.

Background

I was looking around in the documentation on writing openrc services due to a discussion about a znc service dying where someone mentioned "i bet its bc openrc wants to run it as root". I figured this is wrong, because surely openrc allows marking a service as needing to be started as non-root.

It turns out that openrc does support this: https://github.com/OpenRC/openrc/blob/master/service-script-guide.md#dont-write-your-own-startstop-functions
You're supposed to set command_user="user:group".

However, it is later on dis-recommended and warned against as being a security vulnerability to start services as non-root: https://github.com/OpenRC/openrc/blob/master/service-script-guide.md#pid-files-should-be-writable-only-by-root

PID files must be writable only by root, which means additionally that they must live in a root-owned directory. This directory is normally /run under Linux and /var/run under other operating systems.

Some daemons run as an unprivileged user account, and create their PID files (as the unprivileged user) in a path like /var/run/foo/foo.pid. That can usually be exploited by the unprivileged user to kill root processes, since when a service is stopped, root usually sends a SIGTERM to the contents of the PID file (which are controlled by the unprivileged user). The main warning sign for that problem is using checkpath to set ownership on the directory containing the PID file. For example,

Why is this a problem?

What does this even mean? It's bad to start services as an unprivileged user that cannot do arbitrary things to root, because the service manager grants absolute trust to the pidfile and allow it to DoS any process it likes, so instead we grant absolute trust to the daemon itself and believe in its pinky promise to drop privileges later?

This documentation appears to have been written by the author of http://michael.orlitzky.com/cves/cve-2017-18284.xhtml which I was linked to as supporting rationale for this documentation warning.

IMO this is totally wrong and indicative of an underlying flaw in openrc itself. If daemons are insecure because they write a pidfile as non-root, owned by non-root, that root then kills...

... the solution is not to start all daemons as root, the solution is to perform killing of that pid after dropping privileges to the command_user.


/cc @thesamesam @orlitzky

@orlitzky
Copy link
Contributor

... the solution is not to start all daemons as root, the solution is to perform killing of that pid after dropping privileges to the command_user.

I don't think anyone ever said that the solution is to start all daemons as root. There are essentially two types of daemons:

  1. Traditional UNIX daemons that are supposed to be started as root, that write their own PID files, and then fork/background themselves.
  2. "Simple" daemons that are basically just regular programs that run in the foreground. The service manager (OpenRC, systemd) is supposed to push them into the background and handle the PID file.

The mistake is launching the first type as a non-root user and then letting it create a PID file in an unprivileged location. OpenRC doesn't necessarily even know what user such a daemon will run as. For example, the user account is often specified in the daemon's configuration file, e.g.

# grep User /etc/clamd.conf 
User clamav

and arbitrary configuration formats aren't parseable in POSIX shell. That design isn't spectacular but it makes sense when you consider the context -- it had to work with a very dumb service manager like sysvinit that doesn't maintain any state between start/stop. And it still works pretty well so long as you don't mistake it for the second type.

There's no problem with the second type because OpenRC handles the PID file (securely) for you. And almost all daemons are of the second type (or provide a compatible option) today because it's what works best with systemd.

To summarize, the PID handling is secure so long as the service script writer chooses the best way to launch the daemon. There are insane corner cases with goofball software obviously but in general you probably just want to launch everything in the foreground and let OpenRC background it these days.

As a side note, it sounds unbelievable, but AFAIK there is no POSIX way to drop privileges. Unrelated to PID files, there's a race condition in checkpath that would benefit greatly from one.

@orlitzky
Copy link
Contributor

there is no POSIX way to drop privileges

in the shell

of course the C daemons know how to do it

@navi-desu
Copy link
Member

services started by start-stop-daemon get stopped by it, in c code, so we can (and very likely should) drop privileges before sending stop signals

even if "properly" set up services won't have a problem, there's no real reason to leave the door open to mistakes. services that start as root and drop privs themselves will still have root kill them, but now services that don't will have a sane safety guard, makes the process less foot-gunny

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

No branches or pull requests

3 participants