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

Partial privilege escalation via PID file manipulation #5991

Closed
orlitzky opened this Issue Jan 16, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@orlitzky

orlitzky commented Jan 16, 2018

The icinga2 daemon creates its PID file after dropping privileges to the ICINGA2_USER. That may be exploited through the init script (or other management tools) by the ICINGA2_USER to kill root processes, since when the daemon is stopped, root sends a "kill" signal to the contents of the PID file (which are under the control of the runtime user).

For example,

 $ sudo ls -l /run/icinga2/
total 4
drwxr-s--- 2 icinga icingacmd 40 Jan 16 13:16 cmd
-rw-r--r-- 1 icinga icingacmd  5 Jan 16 13:43 icinga2.pid

However, the init script (icinga2.init.d.cmake) is executed as root, and does,

stop() {
    ...
    pid=`cat $ICINGA2_PID_FILE` # untrustworthy!
    ...
    kill -KILL $pid

If the daemon is in any way compromised, the ICINGA2_USER can write a PID of his choice into his own PID file. The next time the icinga2 service is stopped, his desired PID will be killed. That can be abused to kill off sshd, to reboot the machine, etc.

The "easy" way to work around this problem is to create the PID file as root, before dropping privileges. That comes with its own set of problems, in particular that you won't be able to modify or delete the PID file in the forked (non-root) process. It does however allow a very dumb, portable, init script to handle the PID file safely.

@Crunsher

This comment has been minimized.

Member

Crunsher commented Jan 17, 2018

Not as simple as one might hope, we have to be able to modify the PID file in our forked instances.
A workaround would be using USR2 (related #2287) which will be used to signal icinga to shut down because there is a reload and a new process would like to take over.

@gunnarbeutner

This comment has been minimized.

Contributor

gunnarbeutner commented Jan 17, 2018

That won't work because we can't rely on how other processes handle SIGUSR2.

Crunsher added a commit that referenced this issue Jan 17, 2018

Crunsher added a commit that referenced this issue Jan 17, 2018

Crunsher added a commit that referenced this issue Jan 26, 2018

Crunsher added a commit that referenced this issue Jan 30, 2018

Crunsher added a commit that referenced this issue Jan 30, 2018

@Crunsher

This comment has been minimized.

@Crunsher

This comment has been minimized.

Member

Crunsher commented Feb 15, 2018

We do the same with SIGHUP, will need to extend internal kill to make it safe.

Crunsher added a commit that referenced this issue Feb 16, 2018

Crunsher added a commit that referenced this issue Feb 20, 2018

Crunsher added a commit that referenced this issue Feb 20, 2018

@Crunsher Crunsher closed this in 6ae376b Feb 21, 2018

Crunsher added a commit that referenced this issue Feb 23, 2018

@Crunsher Crunsher added this to the 2.8.2 milestone Feb 23, 2018

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