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

nagios daemon should create its PID file before dropping privileges #404

Closed
orlitzky opened this Issue Jul 28, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@orlitzky
Contributor

orlitzky commented Jul 28, 2017

Summary

The nagios daemon should create its PID file before dropping
privileges. This represents a minor security issue; additional factors
are needed to make it exploitable.

Description

The purpose of the PID file is to hold the PID of the running daemon,
so that later it can be stopped, restarted, or otherwise signalled
(many daemons reload their configurations in response to a SIGHUP).
To fulfill that purpose, the contents of the PID file need to be
trustworthy. If the PID file is writable by a non-root user, then he
can replace its contents with the PID of a root process. Afterwards,
any attempt to signal the PID contained in the PID file will instead
signal a root process chosen by the non-root user (a vulnerability).

This is commonly exploitable by init scripts that are run as root and
which blindly trust the contents of their PID files. If one daemon
flushes its cache in response to SIGUSR2 and another daemon drops all
connections in response to SIGUSR2, it is not hard to imagine a
denial-of-service by the user of the first daemon against the second.

Exploitation

There is only a risk of exploitation when some other user relies on
the data in the PID file. But you have to wonder, what's the point of
the PID file if not to provide the PID to other people? Any situation
where the PID file is used is therefore suspicious.

The init script daemon-init.in that ships with nagios itself relies on the PID file to
e.g. reload the nagios configuration.

@hedenface hedenface self-assigned this Jul 30, 2017

@hedenface hedenface added this to the 4.3.3 milestone Jul 30, 2017

@hedenface

This comment has been minimized.

Show comment
Hide comment
@hedenface

hedenface Jul 30, 2017

Member

My tests indicate this is fine: 1b19734

Can you clone, autoconf, make and test the maint branch to see?

Member

hedenface commented Jul 30, 2017

My tests indicate this is fine: 1b19734

Can you clone, autoconf, make and test the maint branch to see?

@hedenface hedenface closed this Jul 30, 2017

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Aug 1, 2017

Contributor

@hedenface Thanks for the fast response. I can push this patch live on Gentoo for testing, but first I see a problem with one of the ./configure defaults. Now that the PID file is owned by root, it won't be writable by the nagios user (good!). But in configure.ac, we still have

dnl User can override lock file location
AC_ARG_WITH(lockfile,
	AC_HELP_STRING([--with-lockfile=<path>],
		[sets path and file name for lock file]),
	lockfile=$withval,
	lockfile=$localstatedir/nagios.lock
)

I think the default should be /run instead, or at least something outside of $localstatedir. The problem with $localstatedir is that it needs to be owned by the nagios user, who can then delete the existing root-owned PID file and replace it with his own.

And now that the PID file will be created as root, there won't be a problem writing to e.g. /run/nagios.lock.

Contributor

orlitzky commented Aug 1, 2017

@hedenface Thanks for the fast response. I can push this patch live on Gentoo for testing, but first I see a problem with one of the ./configure defaults. Now that the PID file is owned by root, it won't be writable by the nagios user (good!). But in configure.ac, we still have

dnl User can override lock file location
AC_ARG_WITH(lockfile,
	AC_HELP_STRING([--with-lockfile=<path>],
		[sets path and file name for lock file]),
	lockfile=$withval,
	lockfile=$localstatedir/nagios.lock
)

I think the default should be /run instead, or at least something outside of $localstatedir. The problem with $localstatedir is that it needs to be owned by the nagios user, who can then delete the existing root-owned PID file and replace it with his own.

And now that the PID file will be created as root, there won't be a problem writing to e.g. /run/nagios.lock.

@orlitzky

This comment has been minimized.

Show comment
Hide comment
@orlitzky

orlitzky Aug 4, 2017

Contributor

Don't worry about the default lockfile path, I'll deal with it in issue #378.

Contributor

orlitzky commented Aug 4, 2017

Don't worry about the default lockfile path, I'll deal with it in issue #378.

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