Message-ID: <720751180a9543cfa205cd527248df7c@ustx2ex-dag1mb5.msg.corp.akamai.com>
Date:   Sat, 15 Jun 2019 01:16:32 +0000
From:   "Lubashev, Igor" <ilubashe@...mai.com>
To:     James Morris <jmorris@...ei.org>
CC:     Serge Hallyn <serge@...lyn.com>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 0/1] security: add SECURE_KEEP_FSUID to preserve
 fsuid/fsgid across execve

> On Friday, June 14, 2019, James Morris wrote:
> On Thu, 13 Jun 2019, Igor Lubashev wrote:
> 
> > I've posted this in March but received no response. Reposting.
> >
> > This patch introduces SECURE_KEEP_FSUID to allow fsuid/fsgid to be
> > preserved across execve. It is currently impossible to execve a
> > program such that effective and filesystem uid differ.
> >
> > The need for this functionality arose from a desire to allow certain
> > non-privileged users to run perf. To do this, we install perf without
> > set-uid-root and have a set-uid-root wrapper decide who is allowed to
> > run perf (and with what arguments).
> >
> > The wrapper must execve perf with real and effective root uid, because
> > perf and KASLR require this. However, that presently resets fsuid to
> > root, giving the user ability to read and overwrite any file owned by
> > root (perf report -i, perf record -o). Also, perf record will create
> > perf.data that cannot be deleted by the user.
> >
> > We cannot reset /proc/sys/kernel/perf_event_paranoid to a permissive
> > level, since we must be selective which users have the permissions.
> >
> > Of course, we could fix our problem by a patch to perf to allow
> > passing a username on the command line and having perf execute
> > setfsuid before opening files. However, perf is not the only program
> > that uses kernel features that require root uid/euid, so a general
> > solution that does not involve updating all such programs seems
> > warranted.

> This seems like a very specific corner case, depending on fsuid!=0 for an
> euid=0 process, along with a whitelist policy for perf arguments. It would be a
> great way to escalate to root via a bug in an executed app or via a wrapper
> misconfiguration.

Any set-uid-root app is a hazard.  This wrapper's purpose is to reduce the risk inherent in running apps with elevated privs.
It removes all capabilities (CAT_SETUID, CAT_SETPCAP, CAP_DAC_OVERRIDE, etc.) except for the required ones before execve().  It has a list of users allowed run apps (and a per-user whitelist of arguments, and it manages resources and time used by apps).

The wrapper works great for things like tcpdump -- it is executed with the user's uid and just CAP_NET_CAP and CAP_NET_ADMIN set.

Unfortunately, perf is using uid==0 and euid==0 as a "capability bits".

In tools/perf/util/evsel.c:
	static bool perf_event_can_profile_kernel(void)
	{
		return geteuid() == 0 || perf_event_paranoid() == -1;
	}

In tools/perf/util/symbol.c:
	static bool symbol__read_kptr_restrict(void)
	{
	...
		value = ((geteuid() != 0) || (getuid() != 0)) ?
				(atoi(line) != 0) :
				(atoi(line) == 2);
	...
	}

> Have you considered the example security configuration in
> Documentation/admin-guide/perf-security.rst ?

Unfortunately, this configuration does not work, unless you reset /proc/sys/kernel/perf_event_paranoid to a permissive level (see code above). We have perf_event_paranoid set to 2. If it worked, we could had implemented the same capability-based policy in the wrapper.


> It also adds complexity to kernel credential handling -- it's yet another thing
> to consider when trying to reason about this.

I really wish that there were only two concepts that mattered: capability sets and fsuid/fsgid. The proposed patch allows one to switch to such mode -- a much simpler mode. Yes, the patch does add a "new feature", but what matters most for the complexity question is whether this feature is a move in the right direction.  I am leaning that way, but I am not 100% positive -- hence this RFC patch.


> What are some other examples of programs that could utilize this scheme?

That's everything, like our wrapper, that needs to allow non-root users to run apps (like perf) that use uid/euid as capabilities.  It is a required, if the apps could interact with a filesystem (and accessing root-owned files is not a desired effect).  It is a good idea from the security perspective even if those apps do not normally interact with a filesystem.

I do not have a clear view about a variety of Linux apps ever written, but I suspect that there are many apps that fall into "use uid/euid as capabilities" category.  There are at least two in the kernel's tools directory. There is also use of uid/eiud in the kernel itself, and anything that uses this functionality cannot be fixed w/o fixing the kernel. It may be a bit hard to find all such uses, but a good start is:

	grep -rE '(uid_eq|uid\(\)).*\b(GLOBAL_ROOT_ID|0)\b'

- Igor

Powered by blists - more mailing lists