diff mbox

[v1,1/1] Add Trusted Path Execution as a stackable LSM

Message ID 20170603055351.16080-1-matt@nmatt.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Brown June 3, 2017, 5:53 a.m. UTC
This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature in Grsecurity and also incorporates logging ideas from
cormander's tpe-lkm.

Modifications from the Grsecurity implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, denial messages were improved by including the full path of the
disallowed program. (This idea was taken from cormander's tpe-lkm)

Trusted Path Execution is not a new idea:

http://phrack.org/issues/52/6.html#article

| A trusted path is one that is inside is a root owned directory that
| is not group or world writable.  /bin, /usr/bin, /usr/local/bin, are
| (under normal circumstances) considered trusted.  Any non-root
| users home directory is not trusted, nor is /tmp.

This Trusted Path Execution implementation introduces the following
Kconfig options and sysctls. These config behaviors are taken straight
from Grsecurity's implementation.

CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)

This option enables Trusted Path Execution. TPE blocks *untrusted*
users from executing files that meet the following conditions:

* file is not in a root-owned directory
* file is writable by a user other than root

NOTE: root is never restricted by TPE

CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)

This option defines a group id that, by default, is the untrusted group.
If a user is untrusted then it has the checks described in
CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
checks are not applied. Since root is never restricted by TPE, you can
effectively remove the concept of a trusted or untrusted group by
setting this value to 0.

CONFIG_SECURITY_TPE_ALL (sysctl=kernel.tpe.restrict_all)

This option applies another set of restrictions to all non-root users
even if they are trusted. This only allows execution under the
following conditions:

* file is in a directory owned by the user that is not group or
  world-writable
* file is in a directory owned by root and writable only by root

CONFIG_SECURITY_TPE_INVERT (sysctl=kernel.tpe.gid_invert)

This option reverses the trust logic of the gid option and makes
kernel.tpe.gid into the trusted group. This means that all other groups
become untrusted. This sysctl is helpful when you want TPE restrictions
to apply to most of the users on the system.

Threat Models:

1. Attacker on system executing exploit on system vulnerability

*  If attacker uses a binary as a part of their system exploit, TPE can
   frustrate their efforts

*  Issues:
   *  Can be bypassed by interpreted languages such as python. You can run
      malicious code by doing: python -c 'evil code'

2. Attacker on system replaces binary used by a privileged user with a
   malicious one

*  This situation arises when administrator of a system leaves a binary
   as world writable.

*  TPE is very effective against this threat model

Signed-off-by: Matt Brown <matt@nmatt.com>
---
 MAINTAINERS               |   5 ++
 include/linux/lsm_hooks.h |   5 ++
 security/Kconfig          |   1 +
 security/Makefile         |   2 +
 security/security.c       |   1 +
 security/tpe/Kconfig      |  57 +++++++++++++++
 security/tpe/Makefile     |   3 +
 security/tpe/tpe_lsm.c    | 175 ++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 249 insertions(+)
 create mode 100644 security/tpe/Kconfig
 create mode 100644 security/tpe/Makefile
 create mode 100644 security/tpe/tpe_lsm.c

Comments

Al Viro June 3, 2017, 6:33 a.m. UTC | #1
On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:

> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> +{
> +	struct file *file = bprm->file;
> +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
> +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);

Bloody wonderful.  Do tell, what *does* prevent a race with rename(2) here,
somehow making sure that your 'inode' won't get freed right under you?
Jann Horn June 3, 2017, 10:39 a.m. UTC | #2
On Sat, Jun 3, 2017 at 7:53 AM, Matt Brown <matt@nmatt.com> wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature in Grsecurity and also incorporates logging ideas from
> cormander's tpe-lkm.
>
> Modifications from the Grsecurity implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, denial messages were improved by including the full path of the
> disallowed program. (This idea was taken from cormander's tpe-lkm)
[...]
> Threat Models:
[...]
> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
>
> *  This situation arises when administrator of a system leaves a binary
>    as world writable.
>
> *  TPE is very effective against this threat model

How do you end up with world-writable binaries in $PATH?
Solar Designer June 3, 2017, 3:47 p.m. UTC | #3
Matt,

On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature in Grsecurity and also incorporates logging ideas from
> cormander's tpe-lkm.
> 
> Modifications from the Grsecurity implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, denial messages were improved by including the full path of the
> disallowed program. (This idea was taken from cormander's tpe-lkm)
> 
> Trusted Path Execution is not a new idea:
> 
> http://phrack.org/issues/52/6.html#article

FWIW, I think this is mostly a misfeature, which I deliberately didn't
merge into -ow patches back then.

> This option enables Trusted Path Execution. TPE blocks *untrusted*
> users from executing files that meet the following conditions:
> 
> * file is not in a root-owned directory
> * file is writable by a user other than root
> 
> NOTE: root is never restricted by TPE

> Threat Models:
> 
> 1. Attacker on system executing exploit on system vulnerability
> 
> *  If attacker uses a binary as a part of their system exploit, TPE can
>    frustrate their efforts
> 
> *  Issues:
>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'

Yes, and not only that: a local attacker may also bypass TPE by what
would have been non-security bugs in many other programs - e.g., a
buffer overflow by a command-line argument in any of the allowed
programs suddenly becomes security-relevant.

A variation of your threat model 1, which makes more sense to me than
what's traditionally implied, is when the attacker does not yet use an
interactive shell.  TPE can in fact frustrate automated remote exploits
that don't specifically target (nor support) TPE-enabled systems.

> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
> 
> *  This situation arises when administrator of a system leaves a binary
>    as world writable.
> 
> *  TPE is very effective against this threat model

This makes more sense to me.  It's known-bypassable policy enforcement
by the sysadmin against one's own human error and such.  However, for
this to be effective the exception "NOTE: root is never restricted by
TPE" should be removed or made an option.

In a similar spirit, it'd also make sense to have a policy disallowing
at least root to write into directories writable by other users without
setting the O_EXCL flag.  I actually had this in a kernel module, which
I used to find issues of this kind in Postfix in 2006 or so (Wietse
promptly patched those), but I didn't try running this in production.

Summary: I see little value in TPE, but I don't intend to argue against
it any further (and I deliberately dropped the CC list on this reply,
not to hamper your efforts much).  If you choose to proceed with trying
to upstream it anyway, I suggest the above changes to the description of
the threat models and the tiny code change to allow for restricting root
as well.

Thanks,

Alexander
Greg KH June 3, 2017, 3:59 p.m. UTC | #4
On Sat, Jun 03, 2017 at 05:47:07PM +0200, Solar Designer wrote:
> On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> > This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> > feature in Grsecurity and also incorporates logging ideas from
> > cormander's tpe-lkm.
> > 
> > Modifications from the Grsecurity implementation of TPE were made to
> > turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> > Also, denial messages were improved by including the full path of the
> > disallowed program. (This idea was taken from cormander's tpe-lkm)
> > 
> > Trusted Path Execution is not a new idea:
> > 
> > http://phrack.org/issues/52/6.html#article
> 
> FWIW, I think this is mostly a misfeature, which I deliberately didn't
> merge into -ow patches back then.

I agree with this :)

> > This option enables Trusted Path Execution. TPE blocks *untrusted*
> > users from executing files that meet the following conditions:
> > 
> > * file is not in a root-owned directory
> > * file is writable by a user other than root
> > 
> > NOTE: root is never restricted by TPE
> 
> > Threat Models:
> > 
> > 1. Attacker on system executing exploit on system vulnerability
> > 
> > *  If attacker uses a binary as a part of their system exploit, TPE can
> >    frustrate their efforts
> > 
> > *  Issues:
> >    *  Can be bypassed by interpreted languages such as python. You can run
> >       malicious code by doing: python -c 'evil code'
> 
> Yes, and not only that: a local attacker may also bypass TPE by what
> would have been non-security bugs in many other programs - e.g., a
> buffer overflow by a command-line argument in any of the allowed
> programs suddenly becomes security-relevant.
> 
> A variation of your threat model 1, which makes more sense to me than
> what's traditionally implied, is when the attacker does not yet use an
> interactive shell.  TPE can in fact frustrate automated remote exploits
> that don't specifically target (nor support) TPE-enabled systems.

But for those systems, and this feature as well, can't a "simple"
apparmor policy do the exact same thing?  Also, I'm sure the SELinux can
do this as well, but I don't know the config language there as well.

So I think this is already a feature that is supported, it just takes a
bit more configuration work on the admin.

thanks,

greg k-h
Solar Designer June 3, 2017, 4:13 p.m. UTC | #5
Matt,

On Sat, Jun 03, 2017 at 05:47:07PM +0200, Solar Designer wrote:
> On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> > 2. Attacker on system replaces binary used by a privileged user with a
> >    malicious one
> > 
> > *  This situation arises when administrator of a system leaves a binary
> >    as world writable.
> > 
> > *  TPE is very effective against this threat model
> 
> This makes more sense to me.  It's known-bypassable policy enforcement
> by the sysadmin against one's own human error and such.  However, for
> this to be effective the exception "NOTE: root is never restricted by
> TPE" should be removed or made an option.
> 
> In a similar spirit, it'd also make sense to have a policy disallowing
> at least root to write into directories writable by other users without
> setting the O_EXCL flag.  I actually had this in a kernel module, which
> I used to find issues of this kind in Postfix in 2006 or so (Wietse
> promptly patched those), but I didn't try running this in production.

I misrecalled - that was in 2001, from Postfix's change log:

20011217
[...]
        Security: subtle hardening of the Postfix chroot jail,
        Postfix queue file permissions and access methods, in case
        someone compromises the postfix account.  Michael Tokarev,
        who received the insights from Solar Designer, who tested
        Postfix with a kernel module that is paranoid about open()
        calls.  Files:  master/master_wakeup.c, util/fifo_trigger.c,
        postfix-script.

2006 is when I finally met Wietse in real life, and then bothered to
forward-port the kernel module IIRC from 2.2 to 2.4 kernels and rerun it
on newer Postfix, which found no new issues (suggesting that no bugs of
this type were introduced into Postfix between 2001 and 2006, at least
for whatever coverage my little testing of it achieved).  This kind of
testing could be performed in userspace as well, such as with a revision
of strace or with a preloadable library, even if in an inherently racy
fashion (that's OK for non-production testing).

None of this is directly related to the TPE feature and patch indeed,
but it's something you may look into as well or refocus on.  This finds
real issues (errors in upstream software, distros, sysadmin practices),
and being bypassable is OK when it's not a policy against adversaries
but rather against unintentional errors.

> Summary: I see little value in TPE, but I don't intend to argue against
> it any further (and I deliberately dropped the CC list on this reply,
> not to hamper your efforts much).  If you choose to proceed with trying
> to upstream it anyway, I suggest the above changes to the description of
> the threat models and the tiny code change to allow for restricting root
> as well.

Alexander
Solar Designer June 3, 2017, 4:22 p.m. UTC | #6
On Sat, Jun 03, 2017 at 05:59:20PM +0200, Greg KH wrote:
> But for those systems, and this feature as well, can't a "simple"
> apparmor policy do the exact same thing?  Also, I'm sure the SELinux can
> do this as well, but I don't know the config language there as well.
> 
> So I think this is already a feature that is supported, it just takes a
> bit more configuration work on the admin.

Yes, that's "a bit" more effort up to the point where almost(?) no one
would bother.  Sometimes simple features can reasonably co-exist with
more general frameworks that could also be used to achieve the effect.
So I don't view this as a sufficiently good argument against TPE as a
feature on its own.

Alexander
Daniel Micay June 3, 2017, 5:16 p.m. UTC | #7
On Sat, 2017-06-03 at 18:22 +0200, Solar Designer wrote:
> On Sat, Jun 03, 2017 at 05:59:20PM +0200, Greg KH wrote:
> > But for those systems, and this feature as well, can't a "simple"
> > apparmor policy do the exact same thing?  Also, I'm sure the SELinux
> > can
> > do this as well, but I don't know the config language there as well.
> > 
> > So I think this is already a feature that is supported, it just
> > takes a
> > bit more configuration work on the admin.
> 
> Yes, that's "a bit" more effort up to the point where almost(?) no one
> would bother.  Sometimes simple features can reasonably co-exist with
> more general frameworks that could also be used to achieve the effect.
> So I don't view this as a sufficiently good argument against TPE as a
> feature on its own.
> 
> Alexander

Some thoughts on this, veering a bit into some loosely related topics:

SELinux can cover the same kind of stuff as PaX MPROTECT (execmem,
execheap, execstack, execmod) and grsecurity TPE (execute and
execute_no_trans). Android uses it to enforce comparable rules for most
of the SELinux domains in the base system, but the Android Runtime JIT
and Chromium JIT (particularly in the WebView) block the equivalent to
MPROTECT for much of the app layer and Play Services violates rules
comparable to TPE in the app layer too. In CopperheadOS, it's taken to
completion because the WebView sandbox is force enabled and the Android
Runtime is set to use full ahead-of-time compilation without the JIT.
Nothing violates the equivalent to the TPE rules in the Android Open
Source Project. The domain for the Chromium sandbox is the only one with
execmem and only the privileged base system really violates the TPE
style guarantees (system_server/installd can install an app and run it
in one of the app domains at the very least).

Beyond these features, there's also a distinction between code from
verified/trusted partitions (i.e. covered by dm-verity) and from
partitions that hold persistent state rather than being pristine
verified OS partitions. There's more to "trust" than just whether only
root or root / current user can write there. Android *mostly* forbids
the base system from executing code from outside of those verified
partitions too, but it falls a bit short and that's something else
that's finished off in CopperheadOS. I wouldn't go as far as to say it
makes verified boot work well for anything beyond the usual guarantees
for factory resets though. There's still a lot of trust in persistent
state, just not code executed from it by the base system. The SELinux
label usage also needs to be audited to make sure that an attacker can't
persist unverified SELinux labels that are not replaced. Verified boot
is really hard to do in a meaningful way. ChromeOS weakened it via
having an Android userspace in a container since the guarantees dropped
to those of Android there.

I see why people want lightweight, independent implementations of these
features but I don't have a use for it anymore. I'm used to having great
full system SELinux policies as a baseline from the Android Open Source
Project and being able to tweak those existing policies to accomplish
targeted goals like these. It provides much finer-grained control so it
can do a lot more and compatibility is easier. The one thing it isn't
capable of dealing with on Android is a dynamic toggle because Android
uses a static SELinux policy. It's not possible to have a toggle for
things like debugging features exposed via toggles over ADB that doesn't
at least require respawning processes to get them into a different debug
domain. That's why perf_event_paranoid=3 is needed for Android even if
an LSM hook is added there and why ptrace_scope=2 in CopperheadOS does
something that SELinux (on Android) and seccomp-bpf can't accomplish. On
the other hand, I don't really have any use for TPE since doing it with
SELinux doesn't interfere with debugging so a toggle isn't needed.

There's always the option of making a stub SELinux policy used for
little more than covering similar ground to MPROTECT and TPE if that is
really all someone wants. The policy would be tiny. There can be one
domain for nearly everything and one for an exception from the rules.
The labelling would be simple too. I'm curious how minimal that could
be, but focusing on only those specific micro issues is probably not
something people are going to do in practice.
Corey Henderson June 3, 2017, 9:55 p.m. UTC | #8
On 6/3/2017 10:22 AM, Solar Designer wrote:
> On Sat, Jun 03, 2017 at 05:59:20PM +0200, Greg KH wrote:
>> But for those systems, and this feature as well, can't a "simple"
>> apparmor policy do the exact same thing?  Also, I'm sure the SELinux can
>> do this as well, but I don't know the config language there as well.
>>
>> So I think this is already a feature that is supported, it just takes a
>> bit more configuration work on the admin.
> 
> Yes, that's "a bit" more effort up to the point where almost(?) no one
> would bother.  Sometimes simple features can reasonably co-exist with
> more general frameworks that could also be used to achieve the effect.
> So I don't view this as a sufficiently good argument against TPE as a
> feature on its own.
> 
> Alexander
> 

For what it's worth, the whole reason I wrote Trusted Path Execution 
(tpe-lkm) as an out-of-tree module was for those who use a targeted 
SELinux policy (or none at all) but still wanted access to some basic 
protection from their own authorized users, without having to maintain 
their own kernel or a written LSM policy.

https://github.com/cormander/tpe-lkm

It uses ftrace to enter the kernel, but the code should port pretty 
easily to a stackable LSM built in-tree.

--
Corey
Nicolas Belouin June 3, 2017, 10:02 p.m. UTC | #9
Well,
I think it would be interesting to have such option if you don't​ want to go with a full blown SELinux, but want such feature as TPE. The fact that you don't configure it from userspace is good, as you can keep a very minimal system and force an attacker to only use the provided tools to attack the system, without being able to alter nor disable the protection at any time.


On June 3, 2017 7:16:40 PM GMT+02:00, Daniel Micay <danielmicay@gmail.com> wrote:
>On Sat, 2017-06-03 at 18:22 +0200, Solar Designer wrote:
>> On Sat, Jun 03, 2017 at 05:59:20PM +0200, Greg KH wrote:
>> > But for those systems, and this feature as well, can't a "simple"
>> > apparmor policy do the exact same thing?  Also, I'm sure the
>SELinux
>> > can
>> > do this as well, but I don't know the config language there as
>well.
>> > 
>> > So I think this is already a feature that is supported, it just
>> > takes a
>> > bit more configuration work on the admin.
>> 
>> Yes, that's "a bit" more effort up to the point where almost(?) no
>one
>> would bother.  Sometimes simple features can reasonably co-exist with
>> more general frameworks that could also be used to achieve the
>effect.
>> So I don't view this as a sufficiently good argument against TPE as a
>> feature on its own.
>> 
>> Alexander
>
>Some thoughts on this, veering a bit into some loosely related topics:
>
>SELinux can cover the same kind of stuff as PaX MPROTECT (execmem,
>execheap, execstack, execmod) and grsecurity TPE (execute and
>execute_no_trans). Android uses it to enforce comparable rules for most
>of the SELinux domains in the base system, but the Android Runtime JIT
>and Chromium JIT (particularly in the WebView) block the equivalent to
>MPROTECT for much of the app layer and Play Services violates rules
>comparable to TPE in the app layer too. In CopperheadOS, it's taken to
>completion because the WebView sandbox is force enabled and the Android
>Runtime is set to use full ahead-of-time compilation without the JIT.
>Nothing violates the equivalent to the TPE rules in the Android Open
>Source Project. The domain for the Chromium sandbox is the only one
>with
>execmem and only the privileged base system really violates the TPE
>style guarantees (system_server/installd can install an app and run it
>in one of the app domains at the very least).
>
>Beyond these features, there's also a distinction between code from
>verified/trusted partitions (i.e. covered by dm-verity) and from
>partitions that hold persistent state rather than being pristine
>verified OS partitions. There's more to "trust" than just whether only
>root or root / current user can write there. Android *mostly* forbids
>the base system from executing code from outside of those verified
>partitions too, but it falls a bit short and that's something else
>that's finished off in CopperheadOS. I wouldn't go as far as to say it
>makes verified boot work well for anything beyond the usual guarantees
>for factory resets though. There's still a lot of trust in persistent
>state, just not code executed from it by the base system. The SELinux
>label usage also needs to be audited to make sure that an attacker
>can't
>persist unverified SELinux labels that are not replaced. Verified boot
>is really hard to do in a meaningful way. ChromeOS weakened it via
>having an Android userspace in a container since the guarantees dropped
>to those of Android there.
>
>I see why people want lightweight, independent implementations of these
>features but I don't have a use for it anymore. I'm used to having
>great
>full system SELinux policies as a baseline from the Android Open Source
>Project and being able to tweak those existing policies to accomplish
>targeted goals like these. It provides much finer-grained control so it
>can do a lot more and compatibility is easier. The one thing it isn't
>capable of dealing with on Android is a dynamic toggle because Android
>uses a static SELinux policy. It's not possible to have a toggle for
>things like debugging features exposed via toggles over ADB that
>doesn't
>at least require respawning processes to get them into a different
>debug
>domain. That's why perf_event_paranoid=3 is needed for Android even if
>an LSM hook is added there and why ptrace_scope=2 in CopperheadOS does
>something that SELinux (on Android) and seccomp-bpf can't accomplish.
>On
>the other hand, I don't really have any use for TPE since doing it with
>SELinux doesn't interfere with debugging so a toggle isn't needed.
>
>There's always the option of making a stub SELinux policy used for
>little more than covering similar ground to MPROTECT and TPE if that is
>really all someone wants. The policy would be tiny. There can be one
>domain for nearly everything and one for an exception from the rules.
>The labelling would be simple too. I'm curious how minimal that could
>be, but focusing on only those specific micro issues is probably not
>something people are going to do in practice.

Nicolas
Matt Brown June 3, 2017, 10:30 p.m. UTC | #10
On 06/03/2017 06:39 AM, Jann Horn wrote:
> On Sat, Jun 3, 2017 at 7:53 AM, Matt Brown <matt@nmatt.com> wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature in Grsecurity and also incorporates logging ideas from
>> cormander's tpe-lkm.
>>
>> Modifications from the Grsecurity implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, denial messages were improved by including the full path of the
>> disallowed program. (This idea was taken from cormander's tpe-lkm)
> [...]
>> Threat Models:
> [...]
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when administrator of a system leaves a binary
>>    as world writable.
>>
>> *  TPE is very effective against this threat model
>
> How do you end up with world-writable binaries in $PATH?
>

Sys Admin screw up. It also protects against world-writable binaries
anywhere on the system, not just in $PATH.
Matt Brown June 4, 2017, 5:24 a.m. UTC | #11
On 06/03/2017 02:33 AM, Al Viro wrote:
> On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
>
>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>> +{
>> +	struct file *file = bprm->file;
>> +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
>> +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
>
> Bloody wonderful.  Do tell, what *does* prevent a race with rename(2) here,
> somehow making sure that your 'inode' won't get freed right under you?
>

Good catch. How does this look:

spin_lock(&inode->i_lock);
spin_lock(&file_inode->i_lock);
if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
	reason1 = "directory not owned by user";
else if (inode->i_mode & 0002)
	reason1 = "file in world-writable directory";
else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
	reason1 = "file in group-writable directory";
else if (file_inode->i_mode & 0002)
	reason1 = "file is world-writable";
spin_unlock(&inode->i_lock);
spin_unlock(&file_inode->i_lock);

and likewise for other places in the code?
Eric Biggers June 4, 2017, 5:47 a.m. UTC | #12
On Sun, Jun 04, 2017 at 01:24:13AM -0400, Matt Brown wrote:
> On 06/03/2017 02:33 AM, Al Viro wrote:
> > On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> > 
> > > +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> > > +{
> > > +	struct file *file = bprm->file;
> > > +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
> > > +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
> > 
> > Bloody wonderful.  Do tell, what *does* prevent a race with rename(2) here,
> > somehow making sure that your 'inode' won't get freed right under you?
> > 
> 
> Good catch. How does this look:
> 
> spin_lock(&inode->i_lock);
> spin_lock(&file_inode->i_lock);
> if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
> 	reason1 = "directory not owned by user";
> else if (inode->i_mode & 0002)
> 	reason1 = "file in world-writable directory";
> else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
> 	reason1 = "file in group-writable directory";
> else if (file_inode->i_mode & 0002)
> 	reason1 = "file is world-writable";
> spin_unlock(&inode->i_lock);
> spin_unlock(&file_inode->i_lock);
> 
> and likewise for other places in the code?

No, it needs to take a reference on the parent dentry before using it, using
dget_parent(), I think, and then dropping it later with dput().  Taking i_lock
isn't needed.

Eric
Al Viro June 4, 2017, 6:51 a.m. UTC | #13
On Sun, Jun 04, 2017 at 01:24:13AM -0400, Matt Brown wrote:
> On 06/03/2017 02:33 AM, Al Viro wrote:
> > On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> > 
> > > +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> > > +{
> > > +	struct file *file = bprm->file;
> > > +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
> > > +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
> > 
> > Bloody wonderful.  Do tell, what *does* prevent a race with rename(2) here,
> > somehow making sure that your 'inode' won't get freed right under you?
> > 
> 
> Good catch. How does this look:
> 
> spin_lock(&inode->i_lock);
> spin_lock(&file_inode->i_lock);
> if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
> 	reason1 = "directory not owned by user";
> else if (inode->i_mode & 0002)
> 	reason1 = "file in world-writable directory";
> else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
> 	reason1 = "file in group-writable directory";
> else if (file_inode->i_mode & 0002)
> 	reason1 = "file is world-writable";
> spin_unlock(&inode->i_lock);
> spin_unlock(&file_inode->i_lock);
> 
> and likewise for other places in the code?

Er...  You have a pointer to object that might get freed by a thread
running on another CPU.  So you attempt to take a spinlock sitting
inside that object.  How exactly is that supposed to help anything?
Matt Brown June 4, 2017, 12:43 p.m. UTC | #14
On 06/04/2017 01:47 AM, Eric Biggers wrote:
> On Sun, Jun 04, 2017 at 01:24:13AM -0400, Matt Brown wrote:
>> On 06/03/2017 02:33 AM, Al Viro wrote:
>>> On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
>>>
>>>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>>>> +{
>>>> +	struct file *file = bprm->file;
>>>> +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
>>>> +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
>>>
>>> Bloody wonderful.  Do tell, what *does* prevent a race with rename(2) here,
>>> somehow making sure that your 'inode' won't get freed right under you?
>>>
>>
>> Good catch. How does this look:
>>
>> spin_lock(&inode->i_lock);
>> spin_lock(&file_inode->i_lock);
>> if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
>> 	reason1 = "directory not owned by user";
>> else if (inode->i_mode & 0002)
>> 	reason1 = "file in world-writable directory";
>> else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
>> 	reason1 = "file in group-writable directory";
>> else if (file_inode->i_mode & 0002)
>> 	reason1 = "file is world-writable";
>> spin_unlock(&inode->i_lock);
>> spin_unlock(&file_inode->i_lock);
>>
>> and likewise for other places in the code?
>
> No, it needs to take a reference on the parent dentry before using it, using
> dget_parent(), I think, and then dropping it later with dput().  Taking i_lock
> isn't needed.
>
> Eric
>

Got it. Thank you!

Matt Brown
Mickaël Salaün June 4, 2017, 4:43 p.m. UTC | #15
Hi,

If you want to get some information about the history of TPE in
grsecurity, take a look at
https://github.com/linux-scraping/linux-grsecurity/ and run git log
grsecurity/grsec_tpe.c

Here are some links about TPE (before grsecurity used it):
* http://phrack.org/issues/52/6.html#article
* http://phrack.org/issues/53/8.html#article
* https://lwn.net/Articles/32087/
*
https://www.usenix.org/legacy/event/usenix04/tech/freenix/full_papers/rahimi/rahimi_html/

You may want to adjust the credits.

A more flexible way to configure TPE options (sysctl) may be considered too.

Regards,
 Mickaël

On 03/06/2017 07:53, Matt Brown wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature in Grsecurity and also incorporates logging ideas from
> cormander's tpe-lkm.
> 
> Modifications from the Grsecurity implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, denial messages were improved by including the full path of the
> disallowed program. (This idea was taken from cormander's tpe-lkm)
> 
> Trusted Path Execution is not a new idea:
> 
> http://phrack.org/issues/52/6.html#article
> 
> | A trusted path is one that is inside is a root owned directory that
> | is not group or world writable.  /bin, /usr/bin, /usr/local/bin, are
> | (under normal circumstances) considered trusted.  Any non-root
> | users home directory is not trusted, nor is /tmp.
> 
> This Trusted Path Execution implementation introduces the following
> Kconfig options and sysctls. These config behaviors are taken straight
> from Grsecurity's implementation.
> 
> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
> 
> This option enables Trusted Path Execution. TPE blocks *untrusted*
> users from executing files that meet the following conditions:
> 
> * file is not in a root-owned directory
> * file is writable by a user other than root
> 
> NOTE: root is never restricted by TPE
> 
> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
> 
> This option defines a group id that, by default, is the untrusted group.
> If a user is untrusted then it has the checks described in
> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
> checks are not applied. Since root is never restricted by TPE, you can
> effectively remove the concept of a trusted or untrusted group by
> setting this value to 0.
> 
> CONFIG_SECURITY_TPE_ALL (sysctl=kernel.tpe.restrict_all)
> 
> This option applies another set of restrictions to all non-root users
> even if they are trusted. This only allows execution under the
> following conditions:
> 
> * file is in a directory owned by the user that is not group or
>   world-writable
> * file is in a directory owned by root and writable only by root
> 
> CONFIG_SECURITY_TPE_INVERT (sysctl=kernel.tpe.gid_invert)
> 
> This option reverses the trust logic of the gid option and makes
> kernel.tpe.gid into the trusted group. This means that all other groups
> become untrusted. This sysctl is helpful when you want TPE restrictions
> to apply to most of the users on the system.
> 
> Threat Models:
> 
> 1. Attacker on system executing exploit on system vulnerability
> 
> *  If attacker uses a binary as a part of their system exploit, TPE can
>    frustrate their efforts
> 
> *  Issues:
>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'
> 
> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
> 
> *  This situation arises when administrator of a system leaves a binary
>    as world writable.
> 
> *  TPE is very effective against this threat model
> 
> Signed-off-by: Matt Brown <matt@nmatt.com>
> ---
>  MAINTAINERS               |   5 ++
>  include/linux/lsm_hooks.h |   5 ++
>  security/Kconfig          |   1 +
>  security/Makefile         |   2 +
>  security/security.c       |   1 +
>  security/tpe/Kconfig      |  57 +++++++++++++++
>  security/tpe/Makefile     |   3 +
>  security/tpe/tpe_lsm.c    | 175 ++++++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 249 insertions(+)
>  create mode 100644 security/tpe/Kconfig
>  create mode 100644 security/tpe/Makefile
>  create mode 100644 security/tpe/tpe_lsm.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 38d3e4e..1952bd6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11357,6 +11357,11 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
>  S:	Supported
>  F:	security/yama/
>  
> +TPE SECURITY MODULE
> +M:	Matt Brown <matt@nmatt.com>
> +S:	Supported
> +F:	security/tpe/
> +
>  SENSABLE PHANTOM
>  M:	Jiri Slaby <jirislaby@gmail.com>
>  S:	Maintained
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index e29d4c6..d017f49 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1920,5 +1920,10 @@ void __init loadpin_add_hooks(void);
>  #else
>  static inline void loadpin_add_hooks(void) { };
>  #endif
> +#ifdef CONFIG_SECURITY_TPE
> +void __init tpe_add_hooks(void);
> +#else
> +static inline void tpe_add_hooks(void) { };
> +#endif
>  
>  #endif /* ! __LINUX_LSM_HOOKS_H */
> diff --git a/security/Kconfig b/security/Kconfig
> index 34fb609..30e60cd 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -245,6 +245,7 @@ source security/tomoyo/Kconfig
>  source security/apparmor/Kconfig
>  source security/loadpin/Kconfig
>  source security/yama/Kconfig
> +source security/tpe/Kconfig
>  
>  source security/integrity/Kconfig
>  
> diff --git a/security/Makefile b/security/Makefile
> index f2d71cd..f8b5197 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -9,6 +9,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
>  subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
>  subdir-$(CONFIG_SECURITY_YAMA)		+= yama
>  subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
> +subdir-$(CONFIG_SECURITY_TPE)		+= tpe
>  
>  # always enable default capabilities
>  obj-y					+= commoncap.o
> @@ -24,6 +25,7 @@ obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
>  obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
>  obj-$(CONFIG_SECURITY_YAMA)		+= yama/
>  obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
> +obj-$(CONFIG_SECURITY_TPE)		+= tpe/
>  obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
>  
>  # Object integrity file lists
> diff --git a/security/security.c b/security/security.c
> index d0e07f2..ab0dc26 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -62,6 +62,7 @@ int __init security_init(void)
>  	capability_add_hooks();
>  	yama_add_hooks();
>  	loadpin_add_hooks();
> +	tpe_add_hooks();
>  
>  	/*
>  	 * Load all the remaining security modules.
> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
> new file mode 100644
> index 0000000..84fe1b7
> --- /dev/null
> +++ b/security/tpe/Kconfig
> @@ -0,0 +1,57 @@
> +config SECURITY_TPE
> +	bool "Trusted Path Execution (TPE)"
> +	default n
> +	help
> +	  If you say Y here, you will be able to choose a gid to add to the
> +	  supplementary groups of users you want to mark as "untrusted."
> +	  These users will not be able to execute any files that are not in
> +	  root-owned directories writable only by root.  If the sysctl option
> +	  is enabled, a sysctl option with name "tpe" is created.
> +
> +config SECURITY_TPE_ALL
> +	bool "Partially restrict all non-root users"
> +	depends on SECURITY_TPE
> +	help
> +	  If you say Y here, all non-root users will be covered under
> +	  a weaker TPE restriction.  This is separate from, and in addition to,
> +	  the main TPE options that you have selected elsewhere.  Thus, if a
> +	  "trusted" GID is chosen, this restriction applies to even that GID.
> +	  Under this restriction, all non-root users will only be allowed to
> +	  execute files in directories they own that are not group or
> +	  world-writable, or in directories owned by root and writable only by
> +	  root.  If the sysctl option is enabled, a sysctl option with name
> +	  "tpe_restrict_all" is created.
> +
> +config SECURITY_TPE_INVERT
> +	bool "Invert GID option"
> +	depends on SECURITY_TPE
> +	help
> +	  If you say Y here, the group you specify in the TPE configuration will
> +	  decide what group TPE restrictions will be *disabled* for.  This
> +	  option is useful if you want TPE restrictions to be applied to most
> +	  users on the system.  If the sysctl option is enabled, a sysctl option
> +	  with name "tpe_invert" is created.  Unlike other sysctl options, this
> +	  entry will default to on for backward-compatibility.
> +
> +config SECURITY_TPE_GID
> +	int
> +	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT)
> +	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT)
> +
> +config SECURITY_TPE_UNTRUSTED_GID
> +	int "GID for TPE-untrusted users"
> +	depends on SECURITY_TPE && !SECURITY_TPE_INVERT
> +	default 1005
> +	help
> +	  Setting this GID determines what group TPE restrictions will be
> +	  *enabled* for.  If the sysctl option is enabled, a sysctl option
> +	  with name "tpe_gid" is created.
> +
> +config SECURITY_TPE_TRUSTED_GID
> +	int "GID for TPE-trusted users"
> +	depends on SECURITY_TPE && SECURITY_TPE_INVERT
> +	default 1005
> +	help
> +	  Setting this GID determines what group TPE restrictions will be
> +	  *disabled* for.  If the sysctl option is enabled, a sysctl option
> +	  with name "tpe_gid" is created.
> diff --git a/security/tpe/Makefile b/security/tpe/Makefile
> new file mode 100644
> index 0000000..e1bd8ef
> --- /dev/null
> +++ b/security/tpe/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_SECURITY_TPE) := tpe_lsm.o
> +
> +tpe-y := tpe_lsm.o
> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
> new file mode 100644
> index 0000000..075ca02
> --- /dev/null
> +++ b/security/tpe/tpe_lsm.c
> @@ -0,0 +1,175 @@
> +/*
> + * Trusted Path Execution Security Module
> + *
> + * Copyright 2017 Matt Brown
> + *
> + * Author: Matt Brown <matt@nmatt.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/kernel.h>
> +#include <linux/uidgid.h>
> +#include <linux/ratelimit.h>
> +#include <linux/limits.h>
> +#include <linux/cred.h>
> +#include <linux/slab.h>
> +#include <linux/lsm_hooks.h>
> +#include <linux/sysctl.h>
> +#include <linux/binfmts.h>
> +#include <linux/string_helpers.h>
> +
> +#define TPE_GLOBAL_UID(x) from_kuid_munged(&init_user_ns, (x))
> +#define TPE_GLOBAL_GID(x) from_kgid_munged(&init_user_ns, (x))
> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
> +#define global_nonroot_gid(x) (!gid_eq((x), GLOBAL_ROOT_GID))
> +
> +static int tpe_enabled __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE);
> +static kgid_t tpe_gid __read_mostly = KGIDT_INIT(CONFIG_SECURITY_TPE_GID);
> +static int tpe_all __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_ALL);
> +static int tpe_invert __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_INVERT);
> +
> +int print_tpe_error(struct file *file, char *reason1, char *reason2)
> +{
> +	char *filepath;
> +
> +	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
> +
> +	if (!filepath)
> +		return -ENOMEM;
> +
> +	pr_warn_ratelimited("TPE: Denied execution of %s Reason: %s%s%s\n",
> +		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
> +		reason1, reason2 ? " and " : "", reason2 ?: "");
> +	kfree(filepath);
> +	return -EPERM;
> +}
> +
> +/*
> + * Return 0 if the hook is successful and permission is granted.
> + * Otherwise return the proper error message
> + *
> + */
> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> +{
> +	struct file *file = bprm->file;
> +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
> +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
> +	const struct cred *cred = current_cred();
> +	char *reason1 = NULL;
> +	char *reason2 = NULL;
> +
> +	if (!tpe_enabled)
> +		return 0;
> +
> +	/* never restrict root */
> +	if (global_root(cred->uid))
> +		return 0;
> +
> +	if (!tpe_all)
> +		goto general_tpe_check;
> +
> +	/* TPE_ALL: restrictions enforced even if the gid is trusted */
> +	if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
> +		reason1 = "directory not owned by user";
> +	else if (inode->i_mode & 0002)
> +		reason1 = "file in world-writable directory";
> +	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
> +		reason1 = "file in group-writable directory";
> +	else if (file_inode->i_mode & 0002)
> +		reason1 = "file is world-writable";
> +
> +	if (reason1)
> +		goto end;
> +
> +general_tpe_check:
> +	/* determine if group is trusted */
> +	if (tpe_invert && !in_group_p(tpe_gid))
> +		reason2 = "not in trusted group";
> +	else if (!tpe_invert && in_group_p(tpe_gid))
> +		reason2 = "in untrusted group";
> +	else
> +		return 0;
> +
> +	/* main TPE checks */
> +	if (global_nonroot(inode->i_uid))
> +		reason1 = "file in non-root-owned directory";
> +	else if (inode->i_mode & 0002)
> +		reason1 = "file in world-writable directory";
> +	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
> +		reason1 = "file in group-writable directory";
> +	else if (file_inode->i_mode & 0002)
> +		reason1 = "file is world-writable";
> +
> +end:
> +	if (reason1)
> +		return print_tpe_error(file, reason1, reason2);
> +	else
> +		return 0;
> +}
> +
> +static struct security_hook_list tpe_hooks[] = {
> +	LSM_HOOK_INIT(bprm_set_creds, tpe_bprm_set_creds),
> +};
> +
> +#ifdef CONFIG_SYSCTL
> +struct ctl_path tpe_sysctl_path[] = {
> +	{ .procname = "kernel", },
> +	{ .procname = "tpe", },
> +	{ }
> +};
> +
> +static struct ctl_table tpe_sysctl_table[] = {
> +	{
> +		.procname	= "enabled",
> +		.data		= &tpe_enabled,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0600,
> +		.proc_handler	= proc_dointvec,
> +	},
> +	{
> +		.procname	= "gid",
> +		.data		= &tpe_gid,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0600,
> +		.proc_handler	= proc_dointvec,
> +	},
> +	{
> +		.procname	= "gid_invert",
> +		.data		= &tpe_invert,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0600,
> +		.proc_handler	= proc_dointvec,
> +	},
> +	{
> +		.procname	= "restrict_all",
> +		.data		= &tpe_all,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0600,
> +		.proc_handler	= proc_dointvec,
> +	},
> +	{ }
> +};
> +static void __init tpe_init_sysctl(void)
> +{
> +	if (!register_sysctl_paths(tpe_sysctl_path, tpe_sysctl_table))
> +		panic("TPE: sysctl registration failed.\n");
> +}
> +#else
> +static inline void tpe_init_sysctl(void) { }
> +#endif /* CONFIG_SYSCTL */
> +
> +
> +void __init tpe_add_hooks(void)
> +{
> +	pr_info("TPE: securing systems like it's 1998\n");
> +	security_add_hooks(tpe_hooks, ARRAY_SIZE(tpe_hooks), "tpe");
> +	tpe_init_sysctl();
> +}
>
Mickaël Salaün June 4, 2017, 10:07 p.m. UTC | #16
As was pointed out to me, the first grsecurity's implementation of TPE
date back to earlier days (before Git was used for Linux):
https://github.com/linux-scraping/grsecurity-patches/blob/master/grsec-2.4.5/grsecurity-1.4-LIDS-2.4.5.patch

There seem to be multiple implementations inspired by the Phrack
articles. This may be worth to take a look at this different approaches.

 Mickaël

On 04/06/2017 18:43, Mickaël Salaün wrote:
> Hi,
> 
> If you want to get some information about the history of TPE in
> grsecurity, take a look at
> https://github.com/linux-scraping/linux-grsecurity/ and run git log
> grsecurity/grsec_tpe.c
> 
> Here are some links about TPE (before grsecurity used it):
> * http://phrack.org/issues/52/6.html#article
> * http://phrack.org/issues/53/8.html#article
> * https://lwn.net/Articles/32087/
> *
> https://www.usenix.org/legacy/event/usenix04/tech/freenix/full_papers/rahimi/rahimi_html/
> 
> You may want to adjust the credits.
> 
> A more flexible way to configure TPE options (sysctl) may be considered too.
> 
> Regards,
>  Mickaël
> 
> On 03/06/2017 07:53, Matt Brown wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature in Grsecurity and also incorporates logging ideas from
>> cormander's tpe-lkm.
>>
>> Modifications from the Grsecurity implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, denial messages were improved by including the full path of the
>> disallowed program. (This idea was taken from cormander's tpe-lkm)
>>
>> Trusted Path Execution is not a new idea:
>>
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside is a root owned directory that
>> | is not group or world writable.  /bin, /usr/bin, /usr/local/bin, are
>> | (under normal circumstances) considered trusted.  Any non-root
>> | users home directory is not trusted, nor is /tmp.
>>
>> This Trusted Path Execution implementation introduces the following
>> Kconfig options and sysctls. These config behaviors are taken straight
>> from Grsecurity's implementation.
>>
>> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>>
>> This option enables Trusted Path Execution. TPE blocks *untrusted*
>> users from executing files that meet the following conditions:
>>
>> * file is not in a root-owned directory
>> * file is writable by a user other than root
>>
>> NOTE: root is never restricted by TPE
>>
>> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>>
>> This option defines a group id that, by default, is the untrusted group.
>> If a user is untrusted then it has the checks described in
>> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
>> checks are not applied. Since root is never restricted by TPE, you can
>> effectively remove the concept of a trusted or untrusted group by
>> setting this value to 0.
>>
>> CONFIG_SECURITY_TPE_ALL (sysctl=kernel.tpe.restrict_all)
>>
>> This option applies another set of restrictions to all non-root users
>> even if they are trusted. This only allows execution under the
>> following conditions:
>>
>> * file is in a directory owned by the user that is not group or
>>   world-writable
>> * file is in a directory owned by root and writable only by root
>>
>> CONFIG_SECURITY_TPE_INVERT (sysctl=kernel.tpe.gid_invert)
>>
>> This option reverses the trust logic of the gid option and makes
>> kernel.tpe.gid into the trusted group. This means that all other groups
>> become untrusted. This sysctl is helpful when you want TPE restrictions
>> to apply to most of the users on the system.
>>
>> Threat Models:
>>
>> 1. Attacker on system executing exploit on system vulnerability
>>
>> *  If attacker uses a binary as a part of their system exploit, TPE can
>>    frustrate their efforts
>>
>> *  Issues:
>>    *  Can be bypassed by interpreted languages such as python. You can run
>>       malicious code by doing: python -c 'evil code'
>>
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when administrator of a system leaves a binary
>>    as world writable.
>>
>> *  TPE is very effective against this threat model
>>
>> Signed-off-by: Matt Brown <matt@nmatt.com>
>> ---
>>  MAINTAINERS               |   5 ++
>>  include/linux/lsm_hooks.h |   5 ++
>>  security/Kconfig          |   1 +
>>  security/Makefile         |   2 +
>>  security/security.c       |   1 +
>>  security/tpe/Kconfig      |  57 +++++++++++++++
>>  security/tpe/Makefile     |   3 +
>>  security/tpe/tpe_lsm.c    | 175 ++++++++++++++++++++++++++++++++++++++++++++++
>>  8 files changed, 249 insertions(+)
>>  create mode 100644 security/tpe/Kconfig
>>  create mode 100644 security/tpe/Makefile
>>  create mode 100644 security/tpe/tpe_lsm.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 38d3e4e..1952bd6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11357,6 +11357,11 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
>>  S:	Supported
>>  F:	security/yama/
>>  
>> +TPE SECURITY MODULE
>> +M:	Matt Brown <matt@nmatt.com>
>> +S:	Supported
>> +F:	security/tpe/
>> +
>>  SENSABLE PHANTOM
>>  M:	Jiri Slaby <jirislaby@gmail.com>
>>  S:	Maintained
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index e29d4c6..d017f49 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1920,5 +1920,10 @@ void __init loadpin_add_hooks(void);
>>  #else
>>  static inline void loadpin_add_hooks(void) { };
>>  #endif
>> +#ifdef CONFIG_SECURITY_TPE
>> +void __init tpe_add_hooks(void);
>> +#else
>> +static inline void tpe_add_hooks(void) { };
>> +#endif
>>  
>>  #endif /* ! __LINUX_LSM_HOOKS_H */
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 34fb609..30e60cd 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -245,6 +245,7 @@ source security/tomoyo/Kconfig
>>  source security/apparmor/Kconfig
>>  source security/loadpin/Kconfig
>>  source security/yama/Kconfig
>> +source security/tpe/Kconfig
>>  
>>  source security/integrity/Kconfig
>>  
>> diff --git a/security/Makefile b/security/Makefile
>> index f2d71cd..f8b5197 100644
>> --- a/security/Makefile
>> +++ b/security/Makefile
>> @@ -9,6 +9,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
>>  subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
>>  subdir-$(CONFIG_SECURITY_YAMA)		+= yama
>>  subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
>> +subdir-$(CONFIG_SECURITY_TPE)		+= tpe
>>  
>>  # always enable default capabilities
>>  obj-y					+= commoncap.o
>> @@ -24,6 +25,7 @@ obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
>>  obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
>>  obj-$(CONFIG_SECURITY_YAMA)		+= yama/
>>  obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
>> +obj-$(CONFIG_SECURITY_TPE)		+= tpe/
>>  obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
>>  
>>  # Object integrity file lists
>> diff --git a/security/security.c b/security/security.c
>> index d0e07f2..ab0dc26 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -62,6 +62,7 @@ int __init security_init(void)
>>  	capability_add_hooks();
>>  	yama_add_hooks();
>>  	loadpin_add_hooks();
>> +	tpe_add_hooks();
>>  
>>  	/*
>>  	 * Load all the remaining security modules.
>> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
>> new file mode 100644
>> index 0000000..84fe1b7
>> --- /dev/null
>> +++ b/security/tpe/Kconfig
>> @@ -0,0 +1,57 @@
>> +config SECURITY_TPE
>> +	bool "Trusted Path Execution (TPE)"
>> +	default n
>> +	help
>> +	  If you say Y here, you will be able to choose a gid to add to the
>> +	  supplementary groups of users you want to mark as "untrusted."
>> +	  These users will not be able to execute any files that are not in
>> +	  root-owned directories writable only by root.  If the sysctl option
>> +	  is enabled, a sysctl option with name "tpe" is created.
>> +
>> +config SECURITY_TPE_ALL
>> +	bool "Partially restrict all non-root users"
>> +	depends on SECURITY_TPE
>> +	help
>> +	  If you say Y here, all non-root users will be covered under
>> +	  a weaker TPE restriction.  This is separate from, and in addition to,
>> +	  the main TPE options that you have selected elsewhere.  Thus, if a
>> +	  "trusted" GID is chosen, this restriction applies to even that GID.
>> +	  Under this restriction, all non-root users will only be allowed to
>> +	  execute files in directories they own that are not group or
>> +	  world-writable, or in directories owned by root and writable only by
>> +	  root.  If the sysctl option is enabled, a sysctl option with name
>> +	  "tpe_restrict_all" is created.
>> +
>> +config SECURITY_TPE_INVERT
>> +	bool "Invert GID option"
>> +	depends on SECURITY_TPE
>> +	help
>> +	  If you say Y here, the group you specify in the TPE configuration will
>> +	  decide what group TPE restrictions will be *disabled* for.  This
>> +	  option is useful if you want TPE restrictions to be applied to most
>> +	  users on the system.  If the sysctl option is enabled, a sysctl option
>> +	  with name "tpe_invert" is created.  Unlike other sysctl options, this
>> +	  entry will default to on for backward-compatibility.
>> +
>> +config SECURITY_TPE_GID
>> +	int
>> +	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT)
>> +	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT)
>> +
>> +config SECURITY_TPE_UNTRUSTED_GID
>> +	int "GID for TPE-untrusted users"
>> +	depends on SECURITY_TPE && !SECURITY_TPE_INVERT
>> +	default 1005
>> +	help
>> +	  Setting this GID determines what group TPE restrictions will be
>> +	  *enabled* for.  If the sysctl option is enabled, a sysctl option
>> +	  with name "tpe_gid" is created.
>> +
>> +config SECURITY_TPE_TRUSTED_GID
>> +	int "GID for TPE-trusted users"
>> +	depends on SECURITY_TPE && SECURITY_TPE_INVERT
>> +	default 1005
>> +	help
>> +	  Setting this GID determines what group TPE restrictions will be
>> +	  *disabled* for.  If the sysctl option is enabled, a sysctl option
>> +	  with name "tpe_gid" is created.
>> diff --git a/security/tpe/Makefile b/security/tpe/Makefile
>> new file mode 100644
>> index 0000000..e1bd8ef
>> --- /dev/null
>> +++ b/security/tpe/Makefile
>> @@ -0,0 +1,3 @@
>> +obj-$(CONFIG_SECURITY_TPE) := tpe_lsm.o
>> +
>> +tpe-y := tpe_lsm.o
>> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
>> new file mode 100644
>> index 0000000..075ca02
>> --- /dev/null
>> +++ b/security/tpe/tpe_lsm.c
>> @@ -0,0 +1,175 @@
>> +/*
>> + * Trusted Path Execution Security Module
>> + *
>> + * Copyright 2017 Matt Brown
>> + *
>> + * Author: Matt Brown <matt@nmatt.com>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/uidgid.h>
>> +#include <linux/ratelimit.h>
>> +#include <linux/limits.h>
>> +#include <linux/cred.h>
>> +#include <linux/slab.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <linux/sysctl.h>
>> +#include <linux/binfmts.h>
>> +#include <linux/string_helpers.h>
>> +
>> +#define TPE_GLOBAL_UID(x) from_kuid_munged(&init_user_ns, (x))
>> +#define TPE_GLOBAL_GID(x) from_kgid_munged(&init_user_ns, (x))
>> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
>> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
>> +#define global_nonroot_gid(x) (!gid_eq((x), GLOBAL_ROOT_GID))
>> +
>> +static int tpe_enabled __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE);
>> +static kgid_t tpe_gid __read_mostly = KGIDT_INIT(CONFIG_SECURITY_TPE_GID);
>> +static int tpe_all __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_ALL);
>> +static int tpe_invert __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_INVERT);
>> +
>> +int print_tpe_error(struct file *file, char *reason1, char *reason2)
>> +{
>> +	char *filepath;
>> +
>> +	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
>> +
>> +	if (!filepath)
>> +		return -ENOMEM;
>> +
>> +	pr_warn_ratelimited("TPE: Denied execution of %s Reason: %s%s%s\n",
>> +		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
>> +		reason1, reason2 ? " and " : "", reason2 ?: "");
>> +	kfree(filepath);
>> +	return -EPERM;
>> +}
>> +
>> +/*
>> + * Return 0 if the hook is successful and permission is granted.
>> + * Otherwise return the proper error message
>> + *
>> + */
>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>> +{
>> +	struct file *file = bprm->file;
>> +	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
>> +	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
>> +	const struct cred *cred = current_cred();
>> +	char *reason1 = NULL;
>> +	char *reason2 = NULL;
>> +
>> +	if (!tpe_enabled)
>> +		return 0;
>> +
>> +	/* never restrict root */
>> +	if (global_root(cred->uid))
>> +		return 0;
>> +
>> +	if (!tpe_all)
>> +		goto general_tpe_check;
>> +
>> +	/* TPE_ALL: restrictions enforced even if the gid is trusted */
>> +	if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
>> +		reason1 = "directory not owned by user";
>> +	else if (inode->i_mode & 0002)
>> +		reason1 = "file in world-writable directory";
>> +	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
>> +		reason1 = "file in group-writable directory";
>> +	else if (file_inode->i_mode & 0002)
>> +		reason1 = "file is world-writable";
>> +
>> +	if (reason1)
>> +		goto end;
>> +
>> +general_tpe_check:
>> +	/* determine if group is trusted */
>> +	if (tpe_invert && !in_group_p(tpe_gid))
>> +		reason2 = "not in trusted group";
>> +	else if (!tpe_invert && in_group_p(tpe_gid))
>> +		reason2 = "in untrusted group";
>> +	else
>> +		return 0;
>> +
>> +	/* main TPE checks */
>> +	if (global_nonroot(inode->i_uid))
>> +		reason1 = "file in non-root-owned directory";
>> +	else if (inode->i_mode & 0002)
>> +		reason1 = "file in world-writable directory";
>> +	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
>> +		reason1 = "file in group-writable directory";
>> +	else if (file_inode->i_mode & 0002)
>> +		reason1 = "file is world-writable";
>> +
>> +end:
>> +	if (reason1)
>> +		return print_tpe_error(file, reason1, reason2);
>> +	else
>> +		return 0;
>> +}
>> +
>> +static struct security_hook_list tpe_hooks[] = {
>> +	LSM_HOOK_INIT(bprm_set_creds, tpe_bprm_set_creds),
>> +};
>> +
>> +#ifdef CONFIG_SYSCTL
>> +struct ctl_path tpe_sysctl_path[] = {
>> +	{ .procname = "kernel", },
>> +	{ .procname = "tpe", },
>> +	{ }
>> +};
>> +
>> +static struct ctl_table tpe_sysctl_table[] = {
>> +	{
>> +		.procname	= "enabled",
>> +		.data		= &tpe_enabled,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0600,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +	{
>> +		.procname	= "gid",
>> +		.data		= &tpe_gid,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0600,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +	{
>> +		.procname	= "gid_invert",
>> +		.data		= &tpe_invert,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0600,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +	{
>> +		.procname	= "restrict_all",
>> +		.data		= &tpe_all,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0600,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +	{ }
>> +};
>> +static void __init tpe_init_sysctl(void)
>> +{
>> +	if (!register_sysctl_paths(tpe_sysctl_path, tpe_sysctl_table))
>> +		panic("TPE: sysctl registration failed.\n");
>> +}
>> +#else
>> +static inline void tpe_init_sysctl(void) { }
>> +#endif /* CONFIG_SYSCTL */
>> +
>> +
>> +void __init tpe_add_hooks(void)
>> +{
>> +	pr_info("TPE: securing systems like it's 1998\n");
>> +	security_add_hooks(tpe_hooks, ARRAY_SIZE(tpe_hooks), "tpe");
>> +	tpe_init_sysctl();
>> +}
>>
>
Matt Brown June 5, 2017, 2:41 p.m. UTC | #17
On 6/3/17 11:47 AM, Solar Designer wrote:
> Matt,
> 
> On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature in Grsecurity and also incorporates logging ideas from
>> cormander's tpe-lkm.
>>
>> Modifications from the Grsecurity implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, denial messages were improved by including the full path of the
>> disallowed program. (This idea was taken from cormander's tpe-lkm)
>>
>> Trusted Path Execution is not a new idea:
>>
>> http://phrack.org/issues/52/6.html#article
> 
> FWIW, I think this is mostly a misfeature, which I deliberately didn't
> merge into -ow patches back then.
> 
>> This option enables Trusted Path Execution. TPE blocks *untrusted*
>> users from executing files that meet the following conditions:
>>
>> * file is not in a root-owned directory
>> * file is writable by a user other than root
>>
>> NOTE: root is never restricted by TPE
> 
>> Threat Models:
>>
>> 1. Attacker on system executing exploit on system vulnerability
>>
>> *  If attacker uses a binary as a part of their system exploit, TPE can
>>    frustrate their efforts
>>
>> *  Issues:
>>    *  Can be bypassed by interpreted languages such as python. You can run
>>       malicious code by doing: python -c 'evil code'
> 
> Yes, and not only that: a local attacker may also bypass TPE by what
> would have been non-security bugs in many other programs - e.g., a
> buffer overflow by a command-line argument in any of the allowed
> programs suddenly becomes security-relevant.
> 
> A variation of your threat model 1, which makes more sense to me than
> what's traditionally implied, is when the attacker does not yet use an
> interactive shell.  TPE can in fact frustrate automated remote exploits
> that don't specifically target (nor support) TPE-enabled systems.
> 
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when administrator of a system leaves a binary
>>    as world writable.
>>
>> *  TPE is very effective against this threat model
> 
> This makes more sense to me.  It's known-bypassable policy enforcement
> by the sysadmin against one's own human error and such.  However, for
> this to be effective the exception "NOTE: root is never restricted by
> TPE" should be removed or made an option.

OK so a sysctl, kernel.tpe.restrict_root, that toggles if root is also
restricted? With this in mind I might rename restrict_all since that
might be confusing.

> 
> In a similar spirit, it'd also make sense to have a policy disallowing
> at least root to write into directories writable by other users without
> setting the O_EXCL flag.  I actually had this in a kernel module, which
> I used to find issues of this kind in Postfix in 2006 or so (Wietse
> promptly patched those), but I didn't try running this in production.
> 

Could you expand on what you think the threat model is for root not
writing into directories writable by other users? symlink issues?

> Summary: I see little value in TPE, but I don't intend to argue against
> it any further (and I deliberately dropped the CC list on this reply,
> not to hamper your efforts much).  If you choose to proceed with trying
> to upstream it anyway, I suggest the above changes to the description of
> the threat models and the tiny code change to allow for restricting root
> as well.
> 
> Thanks,
> 
> Alexander
>
Solar Designer June 5, 2017, 3:23 p.m. UTC | #18
On Mon, Jun 05, 2017 at 10:41:30AM -0400, Matt Brown wrote:
> On 6/3/17 11:47 AM, Solar Designer wrote:
> > On Sat, Jun 03, 2017 at 01:53:51AM -0400, Matt Brown wrote:
> >> 2. Attacker on system replaces binary used by a privileged user with a
> >>    malicious one
> >>
> >> *  This situation arises when administrator of a system leaves a binary
> >>    as world writable.
> >>
> >> *  TPE is very effective against this threat model
> > 
> > This makes more sense to me.  It's known-bypassable policy enforcement
> > by the sysadmin against one's own human error and such.  However, for
> > this to be effective the exception "NOTE: root is never restricted by
> > TPE" should be removed or made an option.
> 
> OK so a sysctl, kernel.tpe.restrict_root, that toggles if root is also
> restricted?

Maybe.  Up to you and others.  I really don't intend to discuss this in
detail.  I merely pointed out what I knew and had thought of before wrt
TPE and related functionality.

> > In a similar spirit, it'd also make sense to have a policy disallowing
> > at least root to write into directories writable by other users without
> > setting the O_EXCL flag.  I actually had this in a kernel module, which
> > I used to find issues of this kind in Postfix in 2006 or so (Wietse
> > promptly patched those), but I didn't try running this in production.
> 
> Could you expand on what you think the threat model is for root not
> writing into directories writable by other users? symlink issues?

Mostly yes, but also hard links, and attacks on logic of the programs
themselves via FIFOs and even pre-created regular files (e.g., create a
mode 666 file where the program would write some of its own data and
then read it back - e.g., a queued message - and then you can modify
that data even if the file's permissions as normally created by the
program wouldn't have allowed that and e.g. the directory's sticky bit
wouldn't have allowed replacing the file).  Some of this overlaps with
the usual symlink-in-+t and hard-link-to-non-owned-files restrictions,
some of it is separate/additional (e.g., mostly not limited to +t
directories and mostly intended to find and fix policy violations rather
than to provide hardening on production systems).  It's non-obvious
whether this is more closely related to TPE (certainly not as it relates
to its more usual threat model 1 from your original message) or possibly
to the link restrictions, or is separate (as I had it for my testing).

> > Summary: I see little value in TPE, but I don't intend to argue against
> > it any further (and I deliberately dropped the CC list on this reply,
> > not to hamper your efforts much).  If you choose to proceed with trying
> > to upstream it anyway, I suggest the above changes to the description of
> > the threat models and the tiny code change to allow for restricting root
> > as well.

Alexander
Alan Cox June 5, 2017, 3:30 p.m. UTC | #19
> | A trusted path is one that is inside is a root owned directory that
> | is not group or world writable.  /bin, /usr/bin, /usr/local/bin, are
> | (under normal circumstances) considered trusted.  Any non-root
> | users home directory is not trusted, nor is /tmp.

You need the entire path to be root owned and root write only from /, and
to assume that the attacker can't get CAP_SYS_DAC. Compare that with
labelling and labelling looks rather better.

It's the same story as ever - paths are not very meaningful for security,
content is. You label content not paths. The existing LSMs can provide
both content label based execution protection and even path based.
SELinux can do it right already, AppArmor appears to be able to do it
compatibly wrong with your proposal already.

> NOTE: root is never restricted by TPE

Why are we talking about "root" not capabilities ? We stopped tying stuff
to "root" 20 years ago.

>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'

Or using ld.so since ELF binaries are dynamically loaded and will even
helpfully trying and dynamically load libraries not on your trust path
completely configurably via environment variables.

So all your funky protection goes to poop the moment you hit the most
wannabe n00b attacker who knows how to set LD_PRELOAD.

The unix x bit was never designed as a security feature. It got slightly
tweaked for one for directory path walking but the purpose of the 'x' bit
on a file is solely to stop the user accidentally executing garbage.

> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
> 
> *  This situation arises when administrator of a system leaves a binary
>    as world writable.
> 
> *  TPE is very effective against this threat model

Keeping the executables root is allowed to use in a root owned, only root
reachable space with no setuid bits works even better. Especially as you
can mount it r/o almost all of the time.

Pleae explain how your filesystem mode checks interact with existing file
systems that implement other security semantics eg OpenAFS, or for that
matter DOS ?

[Not an idle question as most distributions keep their EFI system
partition mounted even though it seems a rather silly thing to do]

Alan
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 38d3e4e..1952bd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11357,6 +11357,11 @@  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
 S:	Supported
 F:	security/yama/
 
+TPE SECURITY MODULE
+M:	Matt Brown <matt@nmatt.com>
+S:	Supported
+F:	security/tpe/
+
 SENSABLE PHANTOM
 M:	Jiri Slaby <jirislaby@gmail.com>
 S:	Maintained
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index e29d4c6..d017f49 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1920,5 +1920,10 @@  void __init loadpin_add_hooks(void);
 #else
 static inline void loadpin_add_hooks(void) { };
 #endif
+#ifdef CONFIG_SECURITY_TPE
+void __init tpe_add_hooks(void);
+#else
+static inline void tpe_add_hooks(void) { };
+#endif
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index 34fb609..30e60cd 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -245,6 +245,7 @@  source security/tomoyo/Kconfig
 source security/apparmor/Kconfig
 source security/loadpin/Kconfig
 source security/yama/Kconfig
+source security/tpe/Kconfig
 
 source security/integrity/Kconfig
 
diff --git a/security/Makefile b/security/Makefile
index f2d71cd..f8b5197 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -9,6 +9,7 @@  subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
 subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
 subdir-$(CONFIG_SECURITY_YAMA)		+= yama
 subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
+subdir-$(CONFIG_SECURITY_TPE)		+= tpe
 
 # always enable default capabilities
 obj-y					+= commoncap.o
@@ -24,6 +25,7 @@  obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
 obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
 obj-$(CONFIG_SECURITY_YAMA)		+= yama/
 obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
+obj-$(CONFIG_SECURITY_TPE)		+= tpe/
 obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
 
 # Object integrity file lists
diff --git a/security/security.c b/security/security.c
index d0e07f2..ab0dc26 100644
--- a/security/security.c
+++ b/security/security.c
@@ -62,6 +62,7 @@  int __init security_init(void)
 	capability_add_hooks();
 	yama_add_hooks();
 	loadpin_add_hooks();
+	tpe_add_hooks();
 
 	/*
 	 * Load all the remaining security modules.
diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
new file mode 100644
index 0000000..84fe1b7
--- /dev/null
+++ b/security/tpe/Kconfig
@@ -0,0 +1,57 @@ 
+config SECURITY_TPE
+	bool "Trusted Path Execution (TPE)"
+	default n
+	help
+	  If you say Y here, you will be able to choose a gid to add to the
+	  supplementary groups of users you want to mark as "untrusted."
+	  These users will not be able to execute any files that are not in
+	  root-owned directories writable only by root.  If the sysctl option
+	  is enabled, a sysctl option with name "tpe" is created.
+
+config SECURITY_TPE_ALL
+	bool "Partially restrict all non-root users"
+	depends on SECURITY_TPE
+	help
+	  If you say Y here, all non-root users will be covered under
+	  a weaker TPE restriction.  This is separate from, and in addition to,
+	  the main TPE options that you have selected elsewhere.  Thus, if a
+	  "trusted" GID is chosen, this restriction applies to even that GID.
+	  Under this restriction, all non-root users will only be allowed to
+	  execute files in directories they own that are not group or
+	  world-writable, or in directories owned by root and writable only by
+	  root.  If the sysctl option is enabled, a sysctl option with name
+	  "tpe_restrict_all" is created.
+
+config SECURITY_TPE_INVERT
+	bool "Invert GID option"
+	depends on SECURITY_TPE
+	help
+	  If you say Y here, the group you specify in the TPE configuration will
+	  decide what group TPE restrictions will be *disabled* for.  This
+	  option is useful if you want TPE restrictions to be applied to most
+	  users on the system.  If the sysctl option is enabled, a sysctl option
+	  with name "tpe_invert" is created.  Unlike other sysctl options, this
+	  entry will default to on for backward-compatibility.
+
+config SECURITY_TPE_GID
+	int
+	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT)
+	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT)
+
+config SECURITY_TPE_UNTRUSTED_GID
+	int "GID for TPE-untrusted users"
+	depends on SECURITY_TPE && !SECURITY_TPE_INVERT
+	default 1005
+	help
+	  Setting this GID determines what group TPE restrictions will be
+	  *enabled* for.  If the sysctl option is enabled, a sysctl option
+	  with name "tpe_gid" is created.
+
+config SECURITY_TPE_TRUSTED_GID
+	int "GID for TPE-trusted users"
+	depends on SECURITY_TPE && SECURITY_TPE_INVERT
+	default 1005
+	help
+	  Setting this GID determines what group TPE restrictions will be
+	  *disabled* for.  If the sysctl option is enabled, a sysctl option
+	  with name "tpe_gid" is created.
diff --git a/security/tpe/Makefile b/security/tpe/Makefile
new file mode 100644
index 0000000..e1bd8ef
--- /dev/null
+++ b/security/tpe/Makefile
@@ -0,0 +1,3 @@ 
+obj-$(CONFIG_SECURITY_TPE) := tpe_lsm.o
+
+tpe-y := tpe_lsm.o
diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
new file mode 100644
index 0000000..075ca02
--- /dev/null
+++ b/security/tpe/tpe_lsm.c
@@ -0,0 +1,175 @@ 
+/*
+ * Trusted Path Execution Security Module
+ *
+ * Copyright 2017 Matt Brown
+ *
+ * Author: Matt Brown <matt@nmatt.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/uidgid.h>
+#include <linux/ratelimit.h>
+#include <linux/limits.h>
+#include <linux/cred.h>
+#include <linux/slab.h>
+#include <linux/lsm_hooks.h>
+#include <linux/sysctl.h>
+#include <linux/binfmts.h>
+#include <linux/string_helpers.h>
+
+#define TPE_GLOBAL_UID(x) from_kuid_munged(&init_user_ns, (x))
+#define TPE_GLOBAL_GID(x) from_kgid_munged(&init_user_ns, (x))
+#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
+#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
+#define global_nonroot_gid(x) (!gid_eq((x), GLOBAL_ROOT_GID))
+
+static int tpe_enabled __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE);
+static kgid_t tpe_gid __read_mostly = KGIDT_INIT(CONFIG_SECURITY_TPE_GID);
+static int tpe_all __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_ALL);
+static int tpe_invert __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_INVERT);
+
+int print_tpe_error(struct file *file, char *reason1, char *reason2)
+{
+	char *filepath;
+
+	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
+
+	if (!filepath)
+		return -ENOMEM;
+
+	pr_warn_ratelimited("TPE: Denied execution of %s Reason: %s%s%s\n",
+		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
+		reason1, reason2 ? " and " : "", reason2 ?: "");
+	kfree(filepath);
+	return -EPERM;
+}
+
+/*
+ * Return 0 if the hook is successful and permission is granted.
+ * Otherwise return the proper error message
+ *
+ */
+static int tpe_bprm_set_creds(struct linux_binprm *bprm)
+{
+	struct file *file = bprm->file;
+	struct inode *inode = d_backing_inode(file->f_path.dentry->d_parent);
+	struct inode *file_inode = d_backing_inode(file->f_path.dentry);
+	const struct cred *cred = current_cred();
+	char *reason1 = NULL;
+	char *reason2 = NULL;
+
+	if (!tpe_enabled)
+		return 0;
+
+	/* never restrict root */
+	if (global_root(cred->uid))
+		return 0;
+
+	if (!tpe_all)
+		goto general_tpe_check;
+
+	/* TPE_ALL: restrictions enforced even if the gid is trusted */
+	if (global_nonroot(inode->i_uid) && !uid_eq(inode->i_uid, cred->uid))
+		reason1 = "directory not owned by user";
+	else if (inode->i_mode & 0002)
+		reason1 = "file in world-writable directory";
+	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
+		reason1 = "file in group-writable directory";
+	else if (file_inode->i_mode & 0002)
+		reason1 = "file is world-writable";
+
+	if (reason1)
+		goto end;
+
+general_tpe_check:
+	/* determine if group is trusted */
+	if (tpe_invert && !in_group_p(tpe_gid))
+		reason2 = "not in trusted group";
+	else if (!tpe_invert && in_group_p(tpe_gid))
+		reason2 = "in untrusted group";
+	else
+		return 0;
+
+	/* main TPE checks */
+	if (global_nonroot(inode->i_uid))
+		reason1 = "file in non-root-owned directory";
+	else if (inode->i_mode & 0002)
+		reason1 = "file in world-writable directory";
+	else if ((inode->i_mode & 0020) && global_nonroot_gid(inode->i_gid))
+		reason1 = "file in group-writable directory";
+	else if (file_inode->i_mode & 0002)
+		reason1 = "file is world-writable";
+
+end:
+	if (reason1)
+		return print_tpe_error(file, reason1, reason2);
+	else
+		return 0;
+}
+
+static struct security_hook_list tpe_hooks[] = {
+	LSM_HOOK_INIT(bprm_set_creds, tpe_bprm_set_creds),
+};
+
+#ifdef CONFIG_SYSCTL
+struct ctl_path tpe_sysctl_path[] = {
+	{ .procname = "kernel", },
+	{ .procname = "tpe", },
+	{ }
+};
+
+static struct ctl_table tpe_sysctl_table[] = {
+	{
+		.procname	= "enabled",
+		.data		= &tpe_enabled,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "gid",
+		.data		= &tpe_gid,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "gid_invert",
+		.data		= &tpe_invert,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "restrict_all",
+		.data		= &tpe_all,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{ }
+};
+static void __init tpe_init_sysctl(void)
+{
+	if (!register_sysctl_paths(tpe_sysctl_path, tpe_sysctl_table))
+		panic("TPE: sysctl registration failed.\n");
+}
+#else
+static inline void tpe_init_sysctl(void) { }
+#endif /* CONFIG_SYSCTL */
+
+
+void __init tpe_add_hooks(void)
+{
+	pr_info("TPE: securing systems like it's 1998\n");
+	security_add_hooks(tpe_hooks, ARRAY_SIZE(tpe_hooks), "tpe");
+	tpe_init_sysctl();
+}