Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 602596 - <sys-auth/munge-{0.5.10-r2,0.5.11-r1}: root privilege escalation
Summary: <sys-auth/munge-{0.5.10-r2,0.5.11-r1}: root privilege escalation
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Linux
: Normal major (vote)
Assignee: Gentoo Security Audit Team
URL:
Whiteboard: B1 [glsa]
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-14 04:24 UTC by Michael Orlitzky
Modified: 2017-06-06 06:27 UTC (History)
9 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Orlitzky gentoo-dev 2016-12-14 04:24:09 UTC
The munge ebuilds do,

  diropts -o munge -g munge -m700
  dodir /etc/munge || die

which leaves the munge user in total control of the directory /etc/munge. But, the init script somewhat trusts the contents of that directory. When starting the daemon, the following gets executed:

  [ -s "${KEYFILE}" ] && return 0
  ...
  chown munge:munge "${KEYFILE}" || return 1

where KEYFILE points to /etc/munge/munge.key by default.

The "-s" test is a strong mitigating factor -- it rules out symlinks and hard-links to nonempty files -- but the munge user can still hard-link /etc/munge/munge.key to any size-zero file on the system and have the init script call "chown" on it. On systems with no hard-link protection (vanilla-sources, among others), that can be used to gain root.

As a hypothetical scenario, it's not unheard of that /root/.bash_profile or /root/.bashrc be empty. Anyone who can take ownership of those gains root.
Comment 1 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2016-12-14 04:29:43 UTC
The -s test is meaningless, since there's a classic TOCTOU race condition here. The attacker can use inotify to block until just after the -s test has occurred, and then swap out the non-symlink for a symlink.

Seems like rather than chowning, the init script should just test the permissions and print an informative error to the admin if the permissions are wrong. But I don't know this package or have any idea what it does. I'll get the maintainer on this thread to discuss.
Comment 2 Jason A. Donenfeld archtester Gentoo Infrastructure gentoo-dev Security 2016-12-15 04:44:24 UTC
https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=38b6fafecf4a802d0c7d9f6b0a6ddf4c94056220

Maybe this fixes the issue.

Package maintainers -- your feedback is requested.
Comment 3 Justin Bronder (RETIRED) gentoo-dev 2016-12-22 19:35:47 UTC
Thanks for adding us back in!  Here's what I sent zx2c4 once I realized I couldn't comment anymore.

From: Justin Bronder <jsbronder@gentoo.org>
Date: Thu, 15 Dec 2016 09:36:16 -0500
To: zx2c4@gentoo.org
Subject: Re: sys-auth/munge: root privilege escalation
User-Agent: Mutt/1.5.24 (2015-08-30)

Package maintainers (who are probably just me as I introduced this due to
sys-cluster/torque requiring it) can't respond as the bug is restricted.

As best as I can recall, I just followed the install instructions from upstream,
https://github.com/dun/munge/wiki/Installation-Guide.  The changes in the linked
diff look quite reasonable to me.  May be worth bringing it up with upstream so
their docs can get updated.
Comment 4 Kristian Fiskerstrand gentoo-dev Security 2016-12-22 20:06:59 UTC
(In reply to Justin Bronder from comment #3)
> Thanks for adding us back in!  Here's what I sent zx2c4 once I realized I
> couldn't comment anymore.
> 
> From: Justin Bronder <jsbronder@gentoo.org>
> Date: Thu, 15 Dec 2016 09:36:16 -0500
> To: zx2c4@gentoo.org
> Subject: Re: sys-auth/munge: root privilege escalation
> User-Agent: Mutt/1.5.24 (2015-08-30)
> 
> Package maintainers (who are probably just me as I introduced this due to
> sys-cluster/torque requiring it) can't respond as the bug is restricted.
> 
> As best as I can recall, I just followed the install instructions from
> upstream,
> https://github.com/dun/munge/wiki/Installation-Guide.  The changes in the
> linked
> diff look quite reasonable to me.  May be worth bringing it up with upstream
> so
> their docs can get updated.

please also see also bug 540006 for similar hardening in
checkpath which mitigates this attack as well as includes discussion on the
kernel mitigation in gentoo-sources for this attack. Using checkpath in the init script should as such mitigate this issue.
Comment 5 Michael Orlitzky gentoo-dev 2016-12-23 14:06:34 UTC
I think upstream's init script is OK:

  https://github.com/dun/munge/blob/master/src/etc/munge.init.in

It only calls chown once, to set ownership of $VARRUNDIR when it didn't previously exist. I guess that's the best you can do in the absence of checkpath. It doesn't mess with the key file at all.

The installation instructions have you walking along a ledge, but they don't /technically/ say anything wrong. They suggest giving ownership of /etc/munge to the "munge" user, and then making it mode 700. You then have to create a (mode 400) key, which of course *implies* that you'll be calling chown on the key -- otherwise, the munge user couldn't read it from /etc/munge. Doing that once (say, in /root, and then moving it) is safe. It's automating the call to chown that creates a problem.

In hindsight, creating /etc/munge and the key might have been a candidate for pkg_config, but Jason's fix looks safe, works, and doesn't rock the boat too much.
Comment 6 Justin Bronder (RETIRED) gentoo-dev 2016-12-27 20:07:42 UTC
It looks like Jason already committed his fix.  Kristian, is that sufficient for you or should we add checkpath prior to "check_key()".  At least, I believe that's what you were suggesting.
Comment 7 Michael Orlitzky gentoo-dev 2016-12-27 21:11:39 UTC
(In reply to Justin Bronder from comment #6)
> It looks like Jason already committed his fix.  Kristian, is that sufficient
> for you or should we add checkpath prior to "check_key()".  At least, I
> believe that's what you were suggesting.

It's completely fixed -- the most important change was in the ebuild, where ownership of /etc/munge was taken away from the "munge" user. No checkpath is needed now.

Jason's fix included a straight-to-stable revision for the stable ebuild, so there's nothing left to do except the paperwork.
Comment 8 Thomas Deutschmann gentoo-dev Security 2017-01-08 22:26:42 UTC
New GLSA request filed.

Removing restriction because all done and already public since https://gitweb.gentoo.org/repo/gentoo.git/commit/sys-auth/munge?id=38b6fafecf4a802d0c7d9f6b0a6ddf4c94056220
Comment 9 Phil Tooley 2017-03-29 14:28:37 UTC
Unfortunately this is not a complete fix, as while the priviledge escalation bug is fixed, munged cannot now be started.

Currently munged checks that the keyfile is owned by the user munged is running as, and is readable/writeable only by the owner, it regards failing either one of those checks as critical error and quits.

To me the logical approach seems to be to set the /etc/munge perms in the ebuild as now and then just ignore the keyfile in the init script. On first run munge will create it if it does not exist or the sysadmin will have to copy it in for a cluster install and set the correct permissions. Then let munge do its own security checking and complain if things are wrong.
Comment 10 Thomas Deutschmann gentoo-dev Security 2017-06-03 11:45:35 UTC
(In reply to Phil Tooley from comment #9)
> Unfortunately this is not a complete fix, as while the priviledge escalation
> bug is fixed, munged cannot now be started.
> 
> [...]

Please file a new bug against the package if you still think this is an issue and should be addressed.
Comment 11 GLSAMaker/CVETool Bot gentoo-dev 2017-06-06 06:27:38 UTC
This issue was resolved and addressed in
 GLSA 201706-01 at https://security.gentoo.org/glsa/201706-01
by GLSA coordinator Yury German (BlueKnight).