kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/5] Add support for O_MAYEXEC
@ 2018-12-12  8:17 Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
                   ` (7 more replies)
  0 siblings, 8 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

Hi,

The goal of this patch series is to control script interpretation.  A
new O_MAYEXEC flag used by sys_open() is added to enable userland script
interpreter to delegate to the kernel (and thus the system security
policy) the permission to interpret scripts or other files containing
what can be seen as commands.

The security policy is the responsibility of an LSM.  A basic
system-wide policy is implemented with Yama and configurable through a
sysctl.

The initial idea come from CLIP OS and the original implementation has
been used for more than 10 years:
https://github.com/clipos-archive/clipos4_doc

An introduction to O_MAYEXEC was given at the Linux Security Summit
Europe 2018 - Linux Kernel Security Contributions by ANSSI:
https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
The "write xor execute" principle was explained at Kernel Recipes 2018 -
CLIP OS: a defense-in-depth OS:
https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s

This patch series can be applied on top of v4.20-rc6.  This can be
tested with CONFIG_SECURITY_YAMA.  I would really appreciate
constructive comments on this RFC.

Regards,

Mickaël Salaün (5):
  fs: Add support for an O_MAYEXEC flag on sys_open()
  fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie
  Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  selftest/yama: Add tests for O_MAYEXEC enforcing
  doc: Add documentation for Yama's open_mayexec_enforce

 Documentation/admin-guide/LSM/Yama.rst       |  41 +++
 MAINTAINERS                                  |   1 +
 fs/fcntl.c                                   |   2 +-
 fs/namei.c                                   |   2 +
 fs/open.c                                    |   4 +
 include/linux/fcntl.h                        |   2 +-
 include/linux/fs.h                           |   4 +
 include/uapi/asm-generic/fcntl.h             |   3 +
 security/yama/Kconfig                        |   3 +-
 security/yama/yama_lsm.c                     |  82 +++++-
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/yama/.gitignore      |   1 +
 tools/testing/selftests/yama/Makefile        |  19 ++
 tools/testing/selftests/yama/config          |   2 +
 tools/testing/selftests/yama/test_omayexec.c | 276 +++++++++++++++++++
 15 files changed, 439 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/yama/.gitignore
 create mode 100644 tools/testing/selftests/yama/Makefile
 create mode 100644 tools/testing/selftests/yama/config
 create mode 100644 tools/testing/selftests/yama/test_omayexec.c

-- 
2.20.0.rc2

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
@ 2018-12-12  8:17 ` Mickaël Salaün
  2018-12-12 14:43   ` Jan Kara
  2018-12-12  8:17 ` [RFC PATCH v1 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie Mickaël Salaün
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

When the O_MAYEXEC flag is passed, sys_open() may be subject to
additional restrictions depending on a security policy implemented by an
LSM through the inode_permission hook.

The underlying idea is to be able to restrict scripts interpretation
according to a policy defined by the system administrator.  For this to
be possible, script interpreters must use the O_MAYEXEC flag
appropriately.  To be fully effective, these interpreters also need to
handle the other ways to execute code (for which the kernel can't help):
command line parameters (e.g., option -e for Perl), module loading
(e.g., option -m for Python), stdin, file sourcing, environment
variables, configuration files...  According to the threat model, it may
be acceptable to allow some script interpreters (e.g. Bash) to interpret
commands from stdin, may it be a TTY or a pipe, because it may not be
enough to (directly) perform syscalls.

A simple security policy implementation is available in a following
patch for Yama.

This is an updated subset of the patch initially written by Vincent
Strubel for CLIP OS:
https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
This patch has been used for more than 10 years with customized script
interpreters.  Some examples can be found here:
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
---
 fs/fcntl.c                       | 2 +-
 fs/open.c                        | 4 ++++
 include/linux/fcntl.h            | 2 +-
 include/linux/fs.h               | 2 ++
 include/uapi/asm-generic/fcntl.h | 3 +++
 5 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 083185174c6d..6c85c4d0c006 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1031,7 +1031,7 @@ static int __init fcntl_init(void)
 	 * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
 	 * is defined as O_NONBLOCK on some platforms and not on others.
 	 */
-	BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+	BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
 		HWEIGHT32(
 			(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
 			__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/open.c b/fs/open.c
index 0285ce7dbd51..75479b79a58f 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
 	if (flags & O_APPEND)
 		acc_mode |= MAY_APPEND;
 
+	/* Check execution permissions on open. */
+	if (flags & O_MAYEXEC)
+		acc_mode |= MAY_OPENEXEC;
+
 	op->acc_mode = acc_mode;
 
 	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 27dc7a60693e..1fc00cabe9ab 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -9,7 +9,7 @@
 	(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
 	 O_APPEND | O_NDELAY | O_NONBLOCK | O_NDELAY | __O_SYNC | O_DSYNC | \
 	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
-	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_MAYEXEC)
 
 #ifndef force_o_largefile
 #define force_o_largefile() (BITS_PER_LONG != 32)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c95c0807471f..584c9329ad78 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -94,6 +94,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_CHDIR		0x00000040
 /* called from RCU mode, don't block */
 #define MAY_NOT_BLOCK		0x00000080
+/* the inode is opened with O_MAYEXEC */
+#define MAY_OPENEXEC		0x00000100
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6e..cbb9425d6e7c 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -97,6 +97,9 @@
 #define O_NDELAY	O_NONBLOCK
 #endif
 
+/* command execution from file is intended, check exec permissions */
+#define O_MAYEXEC	040000000
+
 #define F_DUPFD		0	/* dup */
 #define F_GETFD		1	/* get close_on_exec */
 #define F_SETFD		2	/* set/clear close_on_exec */
-- 
2.20.0.rc2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [RFC PATCH v1 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
@ 2018-12-12  8:17 ` Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC Mickaël Salaün
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

An LSM doesn't get path information related to an access request to open
an inode.  This new (internal) MAY_EXECMOUNT flag enables an LSM to
check if the underlying mount point of an inode is marked as executable.
This is useful to implement a security policy taking advantage of the
noexec mount option.

This flag is set according to path_noexec(), which checks if a mount
point is mounted with MNT_NOEXEC or if the underlying superblock is
SB_I_NOEXEC.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
---
 fs/namei.c         | 2 ++
 include/linux/fs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index 0cab6494978c..de4f33b3f464 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2970,6 +2970,8 @@ static int may_open(const struct path *path, int acc_mode, int flag)
 		break;
 	}
 
+	/* Pass the mount point executability. */
+	acc_mode |= path_noexec(path) ? 0 : MAY_EXECMOUNT;
 	error = inode_permission(inode, MAY_OPEN | acc_mode);
 	if (error)
 		return error;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 584c9329ad78..083a31b8068e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -96,6 +96,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_NOT_BLOCK		0x00000080
 /* the inode is opened with O_MAYEXEC */
 #define MAY_OPENEXEC		0x00000100
+/* the mount point is marked as executable */
+#define MAY_EXECMOUNT		0x00000200
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
-- 
2.20.0.rc2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie Mickaël Salaün
@ 2018-12-12  8:17 ` Mickaël Salaün
  2018-12-12 14:28   ` Mickaël Salaün
  2018-12-12 17:09   ` Jann Horn
  2018-12-12  8:17 ` [RFC PATCH v1 4/5] selftest/yama: Add tests for O_MAYEXEC enforcing Mickaël Salaün
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

Enable to either propagate the mount options from the underlying VFS
mount to prevent execution, or to propagate the file execute permission.
This may allow a script interpreter to check execution permissions
before reading commands from a file.

The main goal is to be able to protect the kernel by restricting
arbitrary syscalls that an attacker could perform with a crafted binary
or certain script languages.  It also improves multilevel isolation
by reducing the ability of an attacker to use side channels with
specific code.  These restrictions can natively be enforced for ELF
binaries (with the noexec mount option) but require this kernel
extension to properly handle scripts (e.g., Python, Perl).

Add a new sysctl kernel.yama.open_mayexec_enforce to control this
behavior.  A following patch adds documentation.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
---
 security/yama/Kconfig    |  3 +-
 security/yama/yama_lsm.c | 82 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/security/yama/Kconfig b/security/yama/Kconfig
index 96b27405558a..9457619fabd5 100644
--- a/security/yama/Kconfig
+++ b/security/yama/Kconfig
@@ -5,7 +5,8 @@ config SECURITY_YAMA
 	help
 	  This selects Yama, which extends DAC support with additional
 	  system-wide security settings beyond regular Linux discretionary
-	  access controls. Currently available is ptrace scope restriction.
+	  access controls. Currently available are ptrace scope restriction and
+	  enforcement of the O_MAYEXEC open flag.
 	  Like capabilities, this security module stacks with other LSMs.
 	  Further information can be found in
 	  Documentation/admin-guide/LSM/Yama.rst.
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index ffda91a4a1aa..120664e94ee5 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -1,10 +1,12 @@
 /*
  * Yama Linux Security Module
  *
- * Author: Kees Cook <keescook@chromium.org>
+ * Authors: Kees Cook <keescook@chromium.org>
+ *          Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
  *
  * Copyright (C) 2010 Canonical, Ltd.
  * Copyright (C) 2011 The Chromium OS Authors.
+ * Copyright (C) 2018 ANSSI
  *
  * 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
@@ -28,7 +30,14 @@
 #define YAMA_SCOPE_CAPABILITY	2
 #define YAMA_SCOPE_NO_ATTACH	3
 
+#define YAMA_OMAYEXEC_ENFORCE_NONE	0
+#define YAMA_OMAYEXEC_ENFORCE_MOUNT	(1 << 0)
+#define YAMA_OMAYEXEC_ENFORCE_FILE	(1 << 1)
+#define _YAMA_OMAYEXEC_LAST		YAMA_OMAYEXEC_ENFORCE_FILE
+#define _YAMA_OMAYEXEC_MASK		((_YAMA_OMAYEXEC_LAST << 1) - 1)
+
 static int ptrace_scope = YAMA_SCOPE_RELATIONAL;
+static int open_mayexec_enforce = YAMA_OMAYEXEC_ENFORCE_NONE;
 
 /* describe a ptrace relationship for potential exception */
 struct ptrace_relation {
@@ -423,7 +432,40 @@ int yama_ptrace_traceme(struct task_struct *parent)
 	return rc;
 }
 
+/**
+ * yama_inode_permission - check O_MAYEXEC permission before accessing an inode
+ * @inode: inode structure to check
+ * @mask: permission mask
+ *
+ * Return 0 if access is permitted, -EACCES otherwise.
+ */
+int yama_inode_permission(struct inode *inode, int mask)
+{
+	if (!(mask & MAY_OPENEXEC))
+		return 0;
+	/*
+	 * Match regular files and directories to make it easier to
+	 * modify script interpreters.
+	 */
+	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
+		return 0;
+
+	if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
+			!(mask & MAY_EXECMOUNT))
+		return -EACCES;
+
+	/*
+	 * May prefer acl_permission_check() instead of generic_permission(),
+	 * to not be bypassable with CAP_DAC_READ_SEARCH.
+	 */
+	if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
+		return generic_permission(inode, MAY_EXEC);
+
+	return 0;
+}
+
 static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
+	LSM_HOOK_INIT(inode_permission, yama_inode_permission),
 	LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
 	LSM_HOOK_INIT(task_prctl, yama_task_prctl),
@@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
 	return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
 }
 
+static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
+					  void __user *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	int error;
+
+	if (write) {
+		struct ctl_table table_copy;
+		int tmp_mayexec_enforce;
+
+		if (!capable(CAP_MAC_ADMIN))
+			return -EPERM;
+		tmp_mayexec_enforce = *((int *)table->data);
+		table_copy = *table;
+		/* do not erase open_mayexec_enforce */
+		table_copy.data = &tmp_mayexec_enforce;
+		error = proc_dointvec(&table_copy, write, buffer, lenp, ppos);
+		if (error)
+			return error;
+		if ((tmp_mayexec_enforce | _YAMA_OMAYEXEC_MASK) !=
+				_YAMA_OMAYEXEC_MASK)
+			return -EINVAL;
+		*((int *)table->data) = tmp_mayexec_enforce;
+	} else {
+		error = proc_dointvec(table, write, buffer, lenp, ppos);
+		if (error)
+			return error;
+	}
+	return 0;
+}
+
 static int zero;
 static int max_scope = YAMA_SCOPE_NO_ATTACH;
 
@@ -466,6 +539,13 @@ static struct ctl_table yama_sysctl_table[] = {
 		.extra1         = &zero,
 		.extra2         = &max_scope,
 	},
+	{
+		.procname       = "open_mayexec_enforce",
+		.data           = &open_mayexec_enforce,
+		.maxlen         = sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = yama_dointvec_bitmask_macadmin,
+	},
 	{ }
 };
 static void __init yama_init_sysctl(void)
-- 
2.20.0.rc2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [RFC PATCH v1 4/5] selftest/yama: Add tests for O_MAYEXEC enforcing
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
                   ` (2 preceding siblings ...)
  2018-12-12  8:17 ` [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC Mickaël Salaün
@ 2018-12-12  8:17 ` Mickaël Salaün
  2018-12-12  8:17 ` [RFC PATCH v1 5/5] doc: Add documentation for Yama's open_mayexec_enforce Mickaël Salaün
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

Test propagation of noexec mount points or file executability through
files open with or without O_MAYEXEC.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
Cc: Shuah Khan <shuah@kernel.org>
---
 MAINTAINERS                                  |   1 +
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/yama/.gitignore      |   1 +
 tools/testing/selftests/yama/Makefile        |  19 ++
 tools/testing/selftests/yama/config          |   2 +
 tools/testing/selftests/yama/test_omayexec.c | 276 +++++++++++++++++++
 6 files changed, 300 insertions(+)
 create mode 100644 tools/testing/selftests/yama/.gitignore
 create mode 100644 tools/testing/selftests/yama/Makefile
 create mode 100644 tools/testing/selftests/yama/config
 create mode 100644 tools/testing/selftests/yama/test_omayexec.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8119141a926f..a1d01a81b283 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16591,6 +16591,7 @@ M:	Kees Cook <keescook@chromium.org>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
 S:	Supported
 F:	security/yama/
+F:	tools/testing/selftests/yama/
 F:	Documentation/admin-guide/LSM/Yama.rst
 
 YEALINK PHONE DRIVER
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f0017c831e57..608f31167aa6 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -46,6 +46,7 @@ endif
 TARGETS += user
 TARGETS += vm
 TARGETS += x86
+TARGETS += yama
 TARGETS += zram
 #Please keep the TARGETS list alphabetically sorted
 # Run "make quicktest=1 run_tests" or
diff --git a/tools/testing/selftests/yama/.gitignore b/tools/testing/selftests/yama/.gitignore
new file mode 100644
index 000000000000..6e8d5cfb48d0
--- /dev/null
+++ b/tools/testing/selftests/yama/.gitignore
@@ -0,0 +1 @@
+/test_omayexec
diff --git a/tools/testing/selftests/yama/Makefile b/tools/testing/selftests/yama/Makefile
new file mode 100644
index 000000000000..d411f1615b60
--- /dev/null
+++ b/tools/testing/selftests/yama/Makefile
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0
+
+all:
+
+include ../lib.mk
+
+.PHONY: all clean
+
+BINARIES := test_omayexec
+CFLAGS += -Wl,-no-as-needed -Wall -Werror
+LDFLAGS += -lcap
+
+test_omayexec: test_omayexec.c ../kselftest_harness.h
+	$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+
+TEST_PROGS += $(BINARIES)
+EXTRA_CLEAN := $(BINARIES)
+
+all: $(BINARIES)
diff --git a/tools/testing/selftests/yama/config b/tools/testing/selftests/yama/config
new file mode 100644
index 000000000000..9d375bfc465b
--- /dev/null
+++ b/tools/testing/selftests/yama/config
@@ -0,0 +1,2 @@
+CONFIG_SECURITY=y
+CONFIG_SECURITY_YAMA=y
diff --git a/tools/testing/selftests/yama/test_omayexec.c b/tools/testing/selftests/yama/test_omayexec.c
new file mode 100644
index 000000000000..7d41097f0e89
--- /dev/null
+++ b/tools/testing/selftests/yama/test_omayexec.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Yama tests - O_MAYEXEC
+ *
+ * Copyright © 2018 ANSSI
+ *
+ * Author: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
+ */
+
+#include <errno.h>
+#include <fcntl.h> /* O_CLOEXEC */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h> /* strlen */
+#include <sys/capability.h>
+#include <sys/mount.h>
+#include <sys/stat.h> /* mkdir */
+#include <unistd.h> /* unlink, rmdir */
+
+#include "../kselftest_harness.h"
+
+#ifndef O_MAYEXEC
+#define O_MAYEXEC      040000000
+#endif
+
+#define SYSCTL_MAYEXEC	"/proc/sys/kernel/yama/open_mayexec_enforce"
+
+#define BIN_DIR		"./test-mount"
+#define BIN_PATH	BIN_DIR "/file"
+#define DIR_PATH	BIN_DIR "/directory"
+
+#define ALLOWED		1
+#define DENIED		0
+
+static void test_omx(struct __test_metadata *_metadata,
+		const char *const path, const int exec_allowed)
+{
+	int fd;
+
+	/* without O_MAYEXEC */
+	fd = open(path, O_RDONLY | O_CLOEXEC);
+	ASSERT_NE(-1, fd);
+	EXPECT_FALSE(close(fd));
+
+	/* with O_MAYEXEC */
+	fd = open(path, O_RDONLY | O_CLOEXEC | O_MAYEXEC);
+	if (exec_allowed) {
+		/* open should succeed */
+		ASSERT_NE(-1, fd);
+		EXPECT_FALSE(close(fd));
+	} else {
+		/* open should return EACCES */
+		ASSERT_EQ(-1, fd);
+		ASSERT_EQ(EACCES, errno);
+	}
+}
+
+static void ignore_dac(struct __test_metadata *_metadata, int override)
+{
+	cap_t caps;
+	const cap_value_t cap_val[2] = {
+		CAP_DAC_OVERRIDE,
+		CAP_DAC_READ_SEARCH,
+	};
+
+	caps = cap_get_proc();
+	ASSERT_TRUE(!!caps);
+	ASSERT_FALSE(cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_val,
+				override ? CAP_SET : CAP_CLEAR));
+	ASSERT_FALSE(cap_set_proc(caps));
+	EXPECT_FALSE(cap_free(caps));
+}
+
+static void test_dir_file(struct __test_metadata *_metadata,
+		const char *const dir_path, const char *const file_path,
+		const int exec_allowed, const int only_file_perm)
+{
+	if (only_file_perm) {
+		/* test as root */
+		ignore_dac(_metadata, 1);
+		/* always allowed because of generic_permission() use */
+		test_omx(_metadata, dir_path, ALLOWED);
+	}
+
+	/* without bypass */
+	ignore_dac(_metadata, 0);
+	test_omx(_metadata, dir_path, exec_allowed);
+	test_omx(_metadata, file_path, exec_allowed);
+}
+
+static void sysctl_write(struct __test_metadata *_metadata,
+		const char *path, const char *value)
+{
+	int fd;
+	size_t len_value;
+	ssize_t len_wrote;
+
+	fd = open(path, O_WRONLY | O_CLOEXEC);
+	ASSERT_NE(-1, fd);
+	len_value = strlen(value);
+	len_wrote = write(fd, value, len_value);
+	ASSERT_EQ(len_wrote, len_value);
+	EXPECT_FALSE(close(fd));
+}
+
+static void create_workspace(struct __test_metadata *_metadata,
+		int mount_exec, int file_exec)
+{
+	int fd;
+
+	/*
+	 * Cleanup previous workspace if any error previously happened (don't
+	 * check errors).
+	 */
+	umount(BIN_DIR);
+	rmdir(BIN_DIR);
+
+	/* create a clean mount point */
+	ASSERT_FALSE(mkdir(BIN_DIR, 00700));
+	ASSERT_FALSE(mount("test", BIN_DIR, "tmpfs",
+				MS_MGC_VAL | (mount_exec ? 0 : MS_NOEXEC),
+				"mode=0700,size=4k"));
+
+	/* create a test file */
+	fd = open(BIN_PATH, O_CREAT | O_RDONLY | O_CLOEXEC,
+			file_exec ? 00500 : 00400);
+	ASSERT_NE(-1, fd);
+	EXPECT_NE(-1, close(fd));
+
+	/* create a test directory */
+	ASSERT_FALSE(mkdir(DIR_PATH, file_exec ? 00500 : 00400));
+}
+
+static void delete_workspace(struct __test_metadata *_metadata)
+{
+	ignore_dac(_metadata, 1);
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "0");
+
+	/* no need to unlink BIN_PATH nor DIR_PATH */
+	ASSERT_FALSE(umount(BIN_DIR));
+	ASSERT_FALSE(rmdir(BIN_DIR));
+}
+
+FIXTURE_DATA(mount_exec_file_exec) { };
+
+FIXTURE_SETUP(mount_exec_file_exec)
+{
+	create_workspace(_metadata, 1, 1);
+}
+
+FIXTURE_TEARDOWN(mount_exec_file_exec)
+{
+	delete_workspace(_metadata);
+}
+
+TEST_F(mount_exec_file_exec, mount)
+{
+	/* enforce mount exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "1");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, ALLOWED, 0);
+}
+
+TEST_F(mount_exec_file_exec, file)
+{
+	/* enforce file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "2");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, ALLOWED, 0);
+}
+
+TEST_F(mount_exec_file_exec, mount_file)
+{
+	/* enforce mount and file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "3");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, ALLOWED, 0);
+}
+
+FIXTURE_DATA(mount_exec_file_noexec) { };
+
+FIXTURE_SETUP(mount_exec_file_noexec)
+{
+	create_workspace(_metadata, 1, 0);
+}
+
+FIXTURE_TEARDOWN(mount_exec_file_noexec)
+{
+	delete_workspace(_metadata);
+}
+
+TEST_F(mount_exec_file_noexec, mount)
+{
+	/* enforce mount exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "1");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, ALLOWED, 0);
+}
+
+TEST_F(mount_exec_file_noexec, file)
+{
+	/* enforce file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "2");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 1);
+}
+
+TEST_F(mount_exec_file_noexec, mount_file)
+{
+	/* enforce mount and file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "3");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 1);
+}
+
+FIXTURE_DATA(mount_noexec_file_exec) { };
+
+FIXTURE_SETUP(mount_noexec_file_exec)
+{
+	create_workspace(_metadata, 0, 1);
+}
+
+FIXTURE_TEARDOWN(mount_noexec_file_exec)
+{
+	delete_workspace(_metadata);
+}
+
+TEST_F(mount_noexec_file_exec, mount)
+{
+	/* enforce mount exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "1");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 0);
+}
+
+TEST_F(mount_noexec_file_exec, file)
+{
+	/* enforce file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "2");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, ALLOWED, 0);
+}
+
+TEST_F(mount_noexec_file_exec, mount_file)
+{
+	/* enforce mount and file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "3");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 0);
+}
+
+FIXTURE_DATA(mount_noexec_file_noexec) { };
+
+FIXTURE_SETUP(mount_noexec_file_noexec)
+{
+	create_workspace(_metadata, 0, 0);
+}
+
+FIXTURE_TEARDOWN(mount_noexec_file_noexec)
+{
+	delete_workspace(_metadata);
+}
+
+TEST_F(mount_noexec_file_noexec, mount)
+{
+	/* enforce mount exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "1");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 0);
+}
+
+TEST_F(mount_noexec_file_noexec, file)
+{
+	/* enforce file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "2");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 1);
+}
+
+TEST_F(mount_noexec_file_noexec, mount_file)
+{
+	/* enforce mount and file exec check */
+	sysctl_write(_metadata, SYSCTL_MAYEXEC, "3");
+	test_dir_file(_metadata, DIR_PATH, BIN_PATH, DENIED, 0);
+}
+
+TEST_HARNESS_MAIN
-- 
2.20.0.rc2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [RFC PATCH v1 5/5] doc: Add documentation for Yama's open_mayexec_enforce
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
                   ` (3 preceding siblings ...)
  2018-12-12  8:17 ` [RFC PATCH v1 4/5] selftest/yama: Add tests for O_MAYEXEC enforcing Mickaël Salaün
@ 2018-12-12  8:17 ` Mickaël Salaün
  2018-12-12 16:29 ` [RFC PATCH v1 0/5] Add support for O_MAYEXEC Jordan Glover
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12  8:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
---
 Documentation/admin-guide/LSM/Yama.rst | 41 ++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/Documentation/admin-guide/LSM/Yama.rst b/Documentation/admin-guide/LSM/Yama.rst
index d0a060de3973..a72c86a24b35 100644
--- a/Documentation/admin-guide/LSM/Yama.rst
+++ b/Documentation/admin-guide/LSM/Yama.rst
@@ -72,3 +72,44 @@ The sysctl settings (writable only with ``CAP_SYS_PTRACE``) are:
     ``PTRACE_TRACEME``. Once set, this sysctl value cannot be changed.
 
 The original children-only logic was based on the restrictions in grsecurity.
+
+open_mayexec_enforce
+====================
+
+The ``O_MAYEXEC`` flag can be passed to :manpage:`open(2)` to only open files
+(or directories) that are executable.  If the file is not identified as
+executable, then the syscall returns -EACCES.  This may allow a script
+interpreter to check executable permission before reading commands from a file.
+One interesting use case is to enforce a "write xor execute" policy through
+interpreters.
+
+Thanks to this flag, Yama enables to enforce the ``noexec`` mount option (i.e.
+the underlying mount point of the file is mounted with MNT_NOEXEC or its
+underlying superblock is SB_I_NOEXEC) not only on ELF binaries but also on
+scripts.  This may be possible thanks to script interpreters using the
+``O_MAYEXEC`` flag.  The executable permission is then checked before reading
+commands from a file, and thus can enforce the ``noexec`` at the interpreter
+level by propagating this security policy to the scripts.  To be fully
+effective, these interpreters also need to handle the other ways to execute
+code (for which the kernel can't help): command line parameters (e.g., option
+``-e`` for Perl), module loading (e.g., option ``-m`` for Python), stdin, file
+sourcing, environment variables, configuration files...  According to the
+threat model, it may be acceptable to allow some script interpreters (e.g.
+Bash) to interpret commands from stdin, may it be a TTY or a pipe, because it
+may not be enough to (directly) perform syscalls.
+
+Yama implements two complementary security policies to propagate the ``noexec``
+mount option or the executable file permission.  These policies are handled by
+the ``kernel.yama.open_mayexec_enforce`` sysctl (writable only with
+``CAP_MAC_ADMIN``) as a bitmask:
+
+1 - mount restriction:
+    check that the mount options for the underlying VFS mount do not prevent
+    execution.
+
+2 - file permission restriction:
+    check that the to-be-opened file is marked as executable for the current
+    process (e.g., POSIX permissions).
+
+Code samples can be found in tools/testing/selftests/yama/test_omayexec.c and
+https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC .
-- 
2.20.0.rc2

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2018-12-12  8:17 ` [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC Mickaël Salaün
@ 2018-12-12 14:28   ` Mickaël Salaün
  2018-12-12 17:09   ` Jann Horn
  1 sibling, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12 14:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Al Viro, James Morris, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel



Le 12/12/2018 à 09:17, Mickaël Salaün a écrit :
> Enable to either propagate the mount options from the underlying VFS
> mount to prevent execution, or to propagate the file execute permission.
> This may allow a script interpreter to check execution permissions
> before reading commands from a file.
> 
> The main goal is to be able to protect the kernel by restricting
> arbitrary syscalls that an attacker could perform with a crafted binary
> or certain script languages.  It also improves multilevel isolation
> by reducing the ability of an attacker to use side channels with
> specific code.  These restrictions can natively be enforced for ELF
> binaries (with the noexec mount option) but require this kernel
> extension to properly handle scripts (e.g., Python, Perl).
> 
> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
> behavior.  A following patch adds documentation.
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
> ---
>  security/yama/Kconfig    |  3 +-
>  security/yama/yama_lsm.c | 82 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 83 insertions(+), 2 deletions(-)
> 
> diff --git a/security/yama/Kconfig b/security/yama/Kconfig
> index 96b27405558a..9457619fabd5 100644
> --- a/security/yama/Kconfig
> +++ b/security/yama/Kconfig
> @@ -5,7 +5,8 @@ config SECURITY_YAMA
>  	help
>  	  This selects Yama, which extends DAC support with additional
>  	  system-wide security settings beyond regular Linux discretionary
> -	  access controls. Currently available is ptrace scope restriction.
> +	  access controls. Currently available are ptrace scope restriction and
> +	  enforcement of the O_MAYEXEC open flag.
>  	  Like capabilities, this security module stacks with other LSMs.
>  	  Further information can be found in
>  	  Documentation/admin-guide/LSM/Yama.rst.
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index ffda91a4a1aa..120664e94ee5 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -1,10 +1,12 @@
>  /*
>   * Yama Linux Security Module
>   *
> - * Author: Kees Cook <keescook@chromium.org>
> + * Authors: Kees Cook <keescook@chromium.org>
> + *          Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>   *
>   * Copyright (C) 2010 Canonical, Ltd.
>   * Copyright (C) 2011 The Chromium OS Authors.
> + * Copyright (C) 2018 ANSSI
>   *
>   * 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
> @@ -28,7 +30,14 @@
>  #define YAMA_SCOPE_CAPABILITY	2
>  #define YAMA_SCOPE_NO_ATTACH	3
>  
> +#define YAMA_OMAYEXEC_ENFORCE_NONE	0
> +#define YAMA_OMAYEXEC_ENFORCE_MOUNT	(1 << 0)
> +#define YAMA_OMAYEXEC_ENFORCE_FILE	(1 << 1)
> +#define _YAMA_OMAYEXEC_LAST		YAMA_OMAYEXEC_ENFORCE_FILE
> +#define _YAMA_OMAYEXEC_MASK		((_YAMA_OMAYEXEC_LAST << 1) - 1)
> +
>  static int ptrace_scope = YAMA_SCOPE_RELATIONAL;
> +static int open_mayexec_enforce = YAMA_OMAYEXEC_ENFORCE_NONE;
>  
>  /* describe a ptrace relationship for potential exception */
>  struct ptrace_relation {
> @@ -423,7 +432,40 @@ int yama_ptrace_traceme(struct task_struct *parent)
>  	return rc;
>  }
>  
> +/**
> + * yama_inode_permission - check O_MAYEXEC permission before accessing an inode
> + * @inode: inode structure to check
> + * @mask: permission mask
> + *
> + * Return 0 if access is permitted, -EACCES otherwise.
> + */
> +int yama_inode_permission(struct inode *inode, int mask)
> +{
> +	if (!(mask & MAY_OPENEXEC))
> +		return 0;
> +	/*
> +	 * Match regular files and directories to make it easier to
> +	 * modify script interpreters.
> +	 */
> +	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> +		return 0;

I forgot to mention that these checks do not handle fifos. This is
relevant in a threat model targeting persistent attacks (and with
additional protections/restrictions).

> +
> +	if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
> +			!(mask & MAY_EXECMOUNT))
> +		return -EACCES;
> +
> +	/*
> +	 * May prefer acl_permission_check() instead of generic_permission(),
> +	 * to not be bypassable with CAP_DAC_READ_SEARCH.
> +	 */
> +	if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
> +		return generic_permission(inode, MAY_EXEC);
> +
> +	return 0;
> +}
> +
>  static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
> +	LSM_HOOK_INIT(inode_permission, yama_inode_permission),
>  	LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>  	LSM_HOOK_INIT(task_prctl, yama_task_prctl),
> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
>  	return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>  }
>  
> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
> +					  void __user *buffer, size_t *lenp,
> +					  loff_t *ppos)
> +{
> +	int error;
> +
> +	if (write) {
> +		struct ctl_table table_copy;
> +		int tmp_mayexec_enforce;
> +
> +		if (!capable(CAP_MAC_ADMIN))
> +			return -EPERM;
> +		tmp_mayexec_enforce = *((int *)table->data);
> +		table_copy = *table;
> +		/* do not erase open_mayexec_enforce */
> +		table_copy.data = &tmp_mayexec_enforce;
> +		error = proc_dointvec(&table_copy, write, buffer, lenp, ppos);
> +		if (error)
> +			return error;
> +		if ((tmp_mayexec_enforce | _YAMA_OMAYEXEC_MASK) !=
> +				_YAMA_OMAYEXEC_MASK)
> +			return -EINVAL;
> +		*((int *)table->data) = tmp_mayexec_enforce;
> +	} else {
> +		error = proc_dointvec(table, write, buffer, lenp, ppos);
> +		if (error)
> +			return error;
> +	}
> +	return 0;
> +}
> +
>  static int zero;
>  static int max_scope = YAMA_SCOPE_NO_ATTACH;
>  
> @@ -466,6 +539,13 @@ static struct ctl_table yama_sysctl_table[] = {
>  		.extra1         = &zero,
>  		.extra2         = &max_scope,
>  	},
> +	{
> +		.procname       = "open_mayexec_enforce",
> +		.data           = &open_mayexec_enforce,
> +		.maxlen         = sizeof(int),
> +		.mode           = 0644,
> +		.proc_handler   = yama_dointvec_bitmask_macadmin,
> +	},
>  	{ }
>  };
>  static void __init yama_init_sysctl(void)
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12  8:17 ` [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
@ 2018-12-12 14:43   ` Jan Kara
  2018-12-12 17:09     ` Mickaël Salaün
                       ` (4 more replies)
  0 siblings, 5 replies; 43+ messages in thread
From: Jan Kara @ 2018-12-12 14:43 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel, sgrubb, Matthew Bobrowski

On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> When the O_MAYEXEC flag is passed, sys_open() may be subject to
> additional restrictions depending on a security policy implemented by an
> LSM through the inode_permission hook.
> 
> The underlying idea is to be able to restrict scripts interpretation
> according to a policy defined by the system administrator.  For this to
> be possible, script interpreters must use the O_MAYEXEC flag
> appropriately.  To be fully effective, these interpreters also need to
> handle the other ways to execute code (for which the kernel can't help):
> command line parameters (e.g., option -e for Perl), module loading
> (e.g., option -m for Python), stdin, file sourcing, environment
> variables, configuration files...  According to the threat model, it may
> be acceptable to allow some script interpreters (e.g. Bash) to interpret
> commands from stdin, may it be a TTY or a pipe, because it may not be
> enough to (directly) perform syscalls.
> 
> A simple security policy implementation is available in a following
> patch for Yama.
> 
> This is an updated subset of the patch initially written by Vincent
> Strubel for CLIP OS:
> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> This patch has been used for more than 10 years with customized script
> interpreters.  Some examples can be found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>

...

> diff --git a/fs/open.c b/fs/open.c
> index 0285ce7dbd51..75479b79a58f 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>  	if (flags & O_APPEND)
>  		acc_mode |= MAY_APPEND;
>  
> +	/* Check execution permissions on open. */
> +	if (flags & O_MAYEXEC)
> +		acc_mode |= MAY_OPENEXEC;
> +
>  	op->acc_mode = acc_mode;
>  
>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;

I don't feel experienced enough in security to tell whether we want this
functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
on the resulting struct file? That way also security_file_open() can be
used to arbitrate such executable opens and in particular
fanotify permission event FAN_OPEN_EXEC will get properly generated which I
guess is desirable (support for it is sitting in my tree waiting for the
merge window) - adding some audit people involved in FAN_OPEN_EXEC to
CC. Just an idea...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
                   ` (4 preceding siblings ...)
  2018-12-12  8:17 ` [RFC PATCH v1 5/5] doc: Add documentation for Yama's open_mayexec_enforce Mickaël Salaün
@ 2018-12-12 16:29 ` Jordan Glover
  2018-12-12 17:01   ` Mickaël Salaün
  2018-12-12 19:51 ` James Morris
  2018-12-13  3:02 ` Matthew Wilcox
  7 siblings, 1 reply; 43+ messages in thread
From: Jordan Glover @ 2018-12-12 16:29 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

On Wednesday, December 12, 2018 9:17 AM, Mickaël Salaün <mic@digikod.net> wrote:

> Hi,
>
> The goal of this patch series is to control script interpretation. A
> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> interpreter to delegate to the kernel (and thus the system security
> policy) the permission to interpret scripts or other files containing
> what can be seen as commands.
>
> The security policy is the responsibility of an LSM. A basic
> system-wide policy is implemented with Yama and configurable through a
> sysctl.
>
> The initial idea come from CLIP OS and the original implementation has
> been used for more than 10 years:
> https://github.com/clipos-archive/clipos4_doc
>
> An introduction to O_MAYEXEC was given at the Linux Security Summit
> Europe 2018 - Linux Kernel Security Contributions by ANSSI:
> https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
> The "write xor execute" principle was explained at Kernel Recipes 2018 -
> CLIP OS: a defense-in-depth OS:
> https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s
>
> This patch series can be applied on top of v4.20-rc6. This can be
> tested with CONFIG_SECURITY_YAMA. I would really appreciate
> constructive comments on this RFC.
>
> Regards,
>

Are various interpreters upstreams interested in adding support
for O_MAYEXEC if it land in kernel? Did you contacted them about this?

Jordan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12 16:29 ` [RFC PATCH v1 0/5] Add support for O_MAYEXEC Jordan Glover
@ 2018-12-12 17:01   ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12 17:01 UTC (permalink / raw)
  To: Jordan Glover, Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel


Le 12/12/2018 à 17:29, Jordan Glover a écrit :
> On Wednesday, December 12, 2018 9:17 AM, Mickaël Salaün <mic@digikod.net> wrote:
> 
>> Hi,
>>
>> The goal of this patch series is to control script interpretation. A
>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>> interpreter to delegate to the kernel (and thus the system security
>> policy) the permission to interpret scripts or other files containing
>> what can be seen as commands.
>>
>> The security policy is the responsibility of an LSM. A basic
>> system-wide policy is implemented with Yama and configurable through a
>> sysctl.
>>
>> The initial idea come from CLIP OS and the original implementation has
>> been used for more than 10 years:
>> https://github.com/clipos-archive/clipos4_doc
>>
>> An introduction to O_MAYEXEC was given at the Linux Security Summit
>> Europe 2018 - Linux Kernel Security Contributions by ANSSI:
>> https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
>> The "write xor execute" principle was explained at Kernel Recipes 2018 -
>> CLIP OS: a defense-in-depth OS:
>> https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s
>>
>> This patch series can be applied on top of v4.20-rc6. This can be
>> tested with CONFIG_SECURITY_YAMA. I would really appreciate
>> constructive comments on this RFC.
>>
>> Regards,
>>
> 
> Are various interpreters upstreams interested in adding support
> for O_MAYEXEC if it land in kernel? Did you contacted them about this?

I think the first step is to be OK on the kernel side. We will then be
able to help upstream interpreters implement this feature. It should be
OK because the behavior doesn't change by default, i.e. if the sysadmin
doesn't configure (and test) the whole system. Some examples of modified
interpreters can be found at
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
.

 Mickaël

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12 14:43   ` Jan Kara
@ 2018-12-12 17:09     ` Mickaël Salaün
  2018-12-12 20:42     ` Mimi Zohar
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-12 17:09 UTC (permalink / raw)
  To: Jan Kara, Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, sgrubb, Matthew Bobrowski


Le 12/12/2018 à 15:43, Jan Kara a écrit :
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>> additional restrictions depending on a security policy implemented by an
>> LSM through the inode_permission hook.
>>
>> The underlying idea is to be able to restrict scripts interpretation
>> according to a policy defined by the system administrator.  For this to
>> be possible, script interpreters must use the O_MAYEXEC flag
>> appropriately.  To be fully effective, these interpreters also need to
>> handle the other ways to execute code (for which the kernel can't help):
>> command line parameters (e.g., option -e for Perl), module loading
>> (e.g., option -m for Python), stdin, file sourcing, environment
>> variables, configuration files...  According to the threat model, it may
>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>> commands from stdin, may it be a TTY or a pipe, because it may not be
>> enough to (directly) perform syscalls.
>>
>> A simple security policy implementation is available in a following
>> patch for Yama.
>>
>> This is an updated subset of the patch initially written by Vincent
>> Strubel for CLIP OS:
>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>> This patch has been used for more than 10 years with customized script
>> interpreters.  Some examples can be found here:
>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
> 
> ...
> 
>> diff --git a/fs/open.c b/fs/open.c
>> index 0285ce7dbd51..75479b79a58f 100644
>> --- a/fs/open.c
>> +++ b/fs/open.c
>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>  	if (flags & O_APPEND)
>>  		acc_mode |= MAY_APPEND;
>>  
>> +	/* Check execution permissions on open. */
>> +	if (flags & O_MAYEXEC)
>> +		acc_mode |= MAY_OPENEXEC;
>> +
>>  	op->acc_mode = acc_mode;
>>  
>>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

Indeed, it may be useful for other LSM.

 Mickaël

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2018-12-12  8:17 ` [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC Mickaël Salaün
  2018-12-12 14:28   ` Mickaël Salaün
@ 2018-12-12 17:09   ` Jann Horn
  2018-12-13 14:49     ` Mickaël Salaün
  1 sibling, 1 reply; 43+ messages in thread
From: Jann Horn @ 2018-12-12 17:09 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: kernel list, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk-manpages, mickael.salaun, zohar,
	philippe.trebuchet, shuah, thibaut.sautereau, vincent.strubel,
	yves-alexis.perez, Kernel Hardening, Linux API,
	linux-security-module, linux-fsdevel

On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
> Enable to either propagate the mount options from the underlying VFS
> mount to prevent execution, or to propagate the file execute permission.
> This may allow a script interpreter to check execution permissions
> before reading commands from a file.
>
> The main goal is to be able to protect the kernel by restricting
> arbitrary syscalls that an attacker could perform with a crafted binary
> or certain script languages.  It also improves multilevel isolation
> by reducing the ability of an attacker to use side channels with
> specific code.  These restrictions can natively be enforced for ELF
> binaries (with the noexec mount option) but require this kernel
> extension to properly handle scripts (e.g., Python, Perl).
>
> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
> behavior.  A following patch adds documentation.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
> ---
[...]
> +/**
> + * yama_inode_permission - check O_MAYEXEC permission before accessing an inode
> + * @inode: inode structure to check
> + * @mask: permission mask
> + *
> + * Return 0 if access is permitted, -EACCES otherwise.
> + */
> +int yama_inode_permission(struct inode *inode, int mask)

This should be static, no?

> +{
> +       if (!(mask & MAY_OPENEXEC))
> +               return 0;
> +       /*
> +        * Match regular files and directories to make it easier to
> +        * modify script interpreters.
> +        */
> +       if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> +               return 0;

So files are subject to checks, but loading code from things like
sockets is always fine?

> +       if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
> +                       !(mask & MAY_EXECMOUNT))
> +               return -EACCES;
> +
> +       /*
> +        * May prefer acl_permission_check() instead of generic_permission(),
> +        * to not be bypassable with CAP_DAC_READ_SEARCH.
> +        */
> +       if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
> +               return generic_permission(inode, MAY_EXEC);
> +
> +       return 0;
> +}
> +
>  static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
> +       LSM_HOOK_INIT(inode_permission, yama_inode_permission),
>         LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>         LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>         LSM_HOOK_INIT(task_prctl, yama_task_prctl),
> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
>         return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>  }
>
> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
> +                                         void __user *buffer, size_t *lenp,
> +                                         loff_t *ppos)
> +{
> +       int error;
> +
> +       if (write) {
> +               struct ctl_table table_copy;
> +               int tmp_mayexec_enforce;
> +
> +               if (!capable(CAP_MAC_ADMIN))
> +                       return -EPERM;

Don't put capable() checks in sysctls, it doesn't work.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
                   ` (5 preceding siblings ...)
  2018-12-12 16:29 ` [RFC PATCH v1 0/5] Add support for O_MAYEXEC Jordan Glover
@ 2018-12-12 19:51 ` James Morris
  2018-12-12 20:13   ` Florian Weimer
  2018-12-13  3:02 ` Matthew Wilcox
  7 siblings, 1 reply; 43+ messages in thread
From: James Morris @ 2018-12-12 19:51 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Al Viro, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

On Wed, 12 Dec 2018, Mickaël Salaün wrote:

> Hi,
> 
> The goal of this patch series is to control script interpretation.  A
> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> interpreter to delegate to the kernel (and thus the system security
> policy) the permission to interpret scripts or other files containing
> what can be seen as commands.
> 
> The security policy is the responsibility of an LSM.  A basic
> system-wide policy is implemented with Yama and configurable through a
> sysctl.

If you're depending on the script interpreter to flag that the user may 
execute code, this seems to be equivalent in security terms to depending 
on the user.  e.g. what if the user uses ptrace and clears O_MAYEXEC?

 



-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12 19:51 ` James Morris
@ 2018-12-12 20:13   ` Florian Weimer
  2018-12-12 23:40     ` James Morris
  0 siblings, 1 reply; 43+ messages in thread
From: Florian Weimer @ 2018-12-12 20:13 UTC (permalink / raw)
  To: James Morris
  Cc: Mickaël Salaün, linux-kernel, Al Viro, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

* James Morris:

> If you're depending on the script interpreter to flag that the user may 
> execute code, this seems to be equivalent in security terms to depending 
> on the user.  e.g. what if the user uses ptrace and clears O_MAYEXEC?

The argument I've heard is this: Using ptrace (and adding the +x
attribute) are auditable events.

Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12 14:43   ` Jan Kara
  2018-12-12 17:09     ` Mickaël Salaün
@ 2018-12-12 20:42     ` Mimi Zohar
  2018-12-13  9:47     ` Matthew Bobrowski
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 43+ messages in thread
From: Mimi Zohar @ 2018-12-12 20:42 UTC (permalink / raw)
  To: Jan Kara, Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, sgrubb, Matthew Bobrowski

On Wed, 2018-12-12 at 15:43 +0100, Jan Kara wrote:


> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >  	if (flags & O_APPEND)
> >  		acc_mode |= MAY_APPEND;
> >  
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> >  	op->acc_mode = acc_mode;
> >  
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

Assuming the interpreters are properly modified (and signed),
MAY_OPENEXEC closes a major IMA measurement/appraisal gap.  The kernel
has no insight into the files that the interpreter is opening.  Having
the interpreter annotate the file open, allows IMA to differentiate
scripts opening data files from code.

IMA policy rules could then be written requiring code to be signed.

Example IMA policy rules:
measure func=FILE_CHECK mask=MAY_OPENEXEC
appraise func=FILE_CHECK mask=MAY_OPENEXEC appraise_type=imasig

Mimi

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12 20:13   ` Florian Weimer
@ 2018-12-12 23:40     ` James Morris
  2018-12-13  5:13       ` Florian Weimer
  0 siblings, 1 reply; 43+ messages in thread
From: James Morris @ 2018-12-12 23:40 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Mickaël Salaün, linux-kernel, Al Viro, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

On Wed, 12 Dec 2018, Florian Weimer wrote:

> * James Morris:
> 
> > If you're depending on the script interpreter to flag that the user may 
> > execute code, this seems to be equivalent in security terms to depending 
> > on the user.  e.g. what if the user uses ptrace and clears O_MAYEXEC?
> 
> The argument I've heard is this: Using ptrace (and adding the +x
> attribute) are auditable events.

I guess you could also preload a modified libc which strips the flag.


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
                   ` (6 preceding siblings ...)
  2018-12-12 19:51 ` James Morris
@ 2018-12-13  3:02 ` Matthew Wilcox
  2018-12-13  5:22   ` Florian Weimer
                     ` (2 more replies)
  7 siblings, 3 replies; 43+ messages in thread
From: Matthew Wilcox @ 2018-12-13  3:02 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
> The goal of this patch series is to control script interpretation.  A
> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> interpreter to delegate to the kernel (and thus the system security
> policy) the permission to interpret scripts or other files containing
> what can be seen as commands.

I don't have a problem with the concept, but we're running low on O_ bits.
Does this have to be done before the process gets a file descriptor,
or could we have a new syscall?  Since we're going to be changing the
interpreters anyway, it doesn't seem like too much of an imposition to
ask them to use:

	int verify_for_exec(int fd)

instead of adding an O_MAYEXEC.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-12 23:40     ` James Morris
@ 2018-12-13  5:13       ` Florian Weimer
  2018-12-13 14:57         ` Mickaël Salaün
  0 siblings, 1 reply; 43+ messages in thread
From: Florian Weimer @ 2018-12-13  5:13 UTC (permalink / raw)
  To: James Morris
  Cc: Mickaël Salaün, linux-kernel, Al Viro, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

* James Morris:

> On Wed, 12 Dec 2018, Florian Weimer wrote:
>
>> * James Morris:
>> 
>> > If you're depending on the script interpreter to flag that the user may 
>> > execute code, this seems to be equivalent in security terms to depending 
>> > on the user.  e.g. what if the user uses ptrace and clears O_MAYEXEC?
>> 
>> The argument I've heard is this: Using ptrace (and adding the +x
>> attribute) are auditable events.
>
> I guess you could also preload a modified libc which strips the flag.

My understanding is that this new libc would have to come somewhere, and
making it executable would be an auditable even as well.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13  3:02 ` Matthew Wilcox
@ 2018-12-13  5:22   ` Florian Weimer
  2018-12-13 11:04   ` Mimi Zohar
  2018-12-13 15:17   ` Mickaël Salaün
  2 siblings, 0 replies; 43+ messages in thread
From: Florian Weimer @ 2018-12-13  5:22 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

* Matthew Wilcox:

> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
>> The goal of this patch series is to control script interpretation.  A
>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>> interpreter to delegate to the kernel (and thus the system security
>> policy) the permission to interpret scripts or other files containing
>> what can be seen as commands.
>
> I don't have a problem with the concept, but we're running low on O_ bits.
> Does this have to be done before the process gets a file descriptor,
> or could we have a new syscall?  Since we're going to be changing the
> interpreters anyway, it doesn't seem like too much of an imposition to
> ask them to use:
>
> 	int verify_for_exec(int fd)
>
> instead of adding an O_MAYEXEC.

Will this work for auditing?

Maybe add an interface which explicitly upgrades O_PATH descriptors, and
give that a separate flag argument?  I suppose that would be more
friendly to auditing.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12 14:43   ` Jan Kara
  2018-12-12 17:09     ` Mickaël Salaün
  2018-12-12 20:42     ` Mimi Zohar
@ 2018-12-13  9:47     ` Matthew Bobrowski
  2018-12-13 14:23       ` Mickaël Salaün
  2019-04-15 18:47     ` Steve Grubb
  2019-08-04 23:55     ` Andy Lutomirski
  4 siblings, 1 reply; 43+ messages in thread
From: Matthew Bobrowski @ 2018-12-13  9:47 UTC (permalink / raw)
  To: Jan Kara
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, sgrubb

On Wed, Dec 12, 2018 at 03:43:06PM +0100, Jan Kara wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> > 
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> > 
> > A simple security policy implementation is available in a following
> > patch for Yama.
> > 
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > This patch has been used for more than 10 years with customized script
> > interpreters.  Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> > 
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> > Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> > Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
> 
> ...
> 
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >  	if (flags & O_APPEND)
> >  		acc_mode |= MAY_APPEND;
> >  
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> >  	op->acc_mode = acc_mode;
> >  
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

If I'm understanding this patch series correctly, without an enforced LSM
policy there's realistically no added benefit from a security perspective,
right? Also, I'm in agreement with what Jan has mentioned in regards to setting
the __FMODE_EXEC flag when O_MAYEXEC has been specified. This is something that
would work quite nicely in conjunction with some of the new file access
notification events.

Rather than setting it on the resulting struct file, couldn't they simply
incorporate it as part of op->open_flags when flags & O_MAYEXEC? Not entirely
sure whether this is what you actually meant or not though? Pretty much the
same as a call to exec(2)/execat(2) when it builds its open_flags.

-- 
Matthew Bobrowski

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13  3:02 ` Matthew Wilcox
  2018-12-13  5:22   ` Florian Weimer
@ 2018-12-13 11:04   ` Mimi Zohar
  2018-12-13 11:26     ` Florian Weimer
  2018-12-13 12:16     ` Matthew Wilcox
  2018-12-13 15:17   ` Mickaël Salaün
  2 siblings, 2 replies; 43+ messages in thread
From: Mimi Zohar @ 2018-12-13 11:04 UTC (permalink / raw)
  To: Matthew Wilcox, Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

On Wed, 2018-12-12 at 19:02 -0800, Matthew Wilcox wrote:
> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
> > The goal of this patch series is to control script interpretation.  A
> > new O_MAYEXEC flag used by sys_open() is added to enable userland script
> > interpreter to delegate to the kernel (and thus the system security
> > policy) the permission to interpret scripts or other files containing
> > what can be seen as commands.
> 
> I don't have a problem with the concept, but we're running low on O_ bits.
> Does this have to be done before the process gets a file descriptor,
> or could we have a new syscall?  Since we're going to be changing the
> interpreters anyway, it doesn't seem like too much of an imposition to
> ask them to use:
> 
> 	int verify_for_exec(int fd)
> 
> instead of adding an O_MAYEXEC.

The indication needs to be set during file open, before the open
returns to the caller.  This is the point where ima_file_check()
verifies the file's signature.  On failure, access to the file is
denied.

Mimi

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 11:04   ` Mimi Zohar
@ 2018-12-13 11:26     ` Florian Weimer
  2018-12-13 12:16       ` Mimi Zohar
  2018-12-13 12:16     ` Matthew Wilcox
  1 sibling, 1 reply; 43+ messages in thread
From: Florian Weimer @ 2018-12-13 11:26 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Matthew Wilcox, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mickaël Salaün,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel

* Mimi Zohar:

> The indication needs to be set during file open, before the open
> returns to the caller.  This is the point where ima_file_check()
> verifies the file's signature.  On failure, access to the file is
> denied.

Does this verification happen for open with O_PATH?

Thanks,
Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 11:04   ` Mimi Zohar
  2018-12-13 11:26     ` Florian Weimer
@ 2018-12-13 12:16     ` Matthew Wilcox
  1 sibling, 0 replies; 43+ messages in thread
From: Matthew Wilcox @ 2018-12-13 12:16 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

On Thu, Dec 13, 2018 at 06:04:20AM -0500, Mimi Zohar wrote:
> > I don't have a problem with the concept, but we're running low on O_ bits.
> > Does this have to be done before the process gets a file descriptor,
> > or could we have a new syscall?  Since we're going to be changing the
> > interpreters anyway, it doesn't seem like too much of an imposition to
> > ask them to use:
> > 
> > 	int verify_for_exec(int fd)
> > 
> > instead of adding an O_MAYEXEC.
> 
> The indication needs to be set during file open, before the open
> returns to the caller.  This is the point where ima_file_check()
> verifies the file's signature.  On failure, access to the file is
> denied.

I understand that's what happens today, but do we need to do it that way?
There's no harm in the interpreter having an fd to a file if it knows
not to execute it.  This is different from a program opening a file and
having the LSM deny access to it because it violates the security model.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 11:26     ` Florian Weimer
@ 2018-12-13 12:16       ` Mimi Zohar
  0 siblings, 0 replies; 43+ messages in thread
From: Mimi Zohar @ 2018-12-13 12:16 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Matthew Wilcox, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mickaël Salaün,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, linux-integrity

[Cc'ing linux-integrity]

On Thu, 2018-12-13 at 12:26 +0100, Florian Weimer wrote:
> * Mimi Zohar:
> 
> > The indication needs to be set during file open, before the open
> > returns to the caller.  This is the point where ima_file_check()
> > verifies the file's signature.  On failure, access to the file is
> > denied.
> 
> Does this verification happen for open with O_PATH?

Interesting!  According to the manpage, userspace cannot read/write to
the file.  It looks like do_o_path() intentionally skips do_last(),
with the call to ima_file_check().  If the file data isn't being
accessed, does the file's integrity need to be verified?

Mimi

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-13  9:47     ` Matthew Bobrowski
@ 2018-12-13 14:23       ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-13 14:23 UTC (permalink / raw)
  To: Matthew Bobrowski, Jan Kara
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel, sgrubb


On 13/12/2018 10:47, Matthew Bobrowski wrote:
> On Wed, Dec 12, 2018 at 03:43:06PM +0100, Jan Kara wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>>> This patch has been used for more than 10 years with customized script
>>> interpreters.  Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>>
>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>>> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
>>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>>  	if (flags & O_APPEND)
>>>  		acc_mode |= MAY_APPEND;
>>>  
>>> +	/* Check execution permissions on open. */
>>> +	if (flags & O_MAYEXEC)
>>> +		acc_mode |= MAY_OPENEXEC;
>>> +
>>>  	op->acc_mode = acc_mode;
>>>  
>>>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
>> CC. Just an idea...
> 
> If I'm understanding this patch series correctly, without an enforced LSM
> policy there's realistically no added benefit from a security perspective,
> right?

That's correct. The kernel knows the semantic but the enforcement is
delegated to an LSM and its policy.

> Also, I'm in agreement with what Jan has mentioned in regards to setting
> the __FMODE_EXEC flag when O_MAYEXEC has been specified. This is something that
> would work quite nicely in conjunction with some of the new file access
> notification events.

OK, I will add it in the next patch series (for the new FAN_OPEN_EXEC
support).

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2018-12-12 17:09   ` Jann Horn
@ 2018-12-13 14:49     ` Mickaël Salaün
  2019-01-03 11:17       ` Jann Horn
  0 siblings, 1 reply; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-13 14:49 UTC (permalink / raw)
  To: Jann Horn, Mickaël Salaün
  Cc: kernel list, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk-manpages, zohar,
	philippe.trebuchet, shuah, thibaut.sautereau, vincent.strubel,
	yves-alexis.perez, Kernel Hardening, Linux API,
	linux-security-module, linux-fsdevel


On 12/12/2018 18:09, Jann Horn wrote:
> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
>> Enable to either propagate the mount options from the underlying VFS
>> mount to prevent execution, or to propagate the file execute permission.
>> This may allow a script interpreter to check execution permissions
>> before reading commands from a file.
>>
>> The main goal is to be able to protect the kernel by restricting
>> arbitrary syscalls that an attacker could perform with a crafted binary
>> or certain script languages.  It also improves multilevel isolation
>> by reducing the ability of an attacker to use side channels with
>> specific code.  These restrictions can natively be enforced for ELF
>> binaries (with the noexec mount option) but require this kernel
>> extension to properly handle scripts (e.g., Python, Perl).
>>
>> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
>> behavior.  A following patch adds documentation.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>> ---
> [...]
>> +/**
>> + * yama_inode_permission - check O_MAYEXEC permission before accessing an inode
>> + * @inode: inode structure to check
>> + * @mask: permission mask
>> + *
>> + * Return 0 if access is permitted, -EACCES otherwise.
>> + */
>> +int yama_inode_permission(struct inode *inode, int mask)
> 
> This should be static, no?

Right, it will be in the next series. The previous function
(yama_ptrace_traceme) is not static though.

> 
>> +{
>> +       if (!(mask & MAY_OPENEXEC))
>> +               return 0;
>> +       /*
>> +        * Match regular files and directories to make it easier to
>> +        * modify script interpreters.
>> +        */
>> +       if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
>> +               return 0;
> 
> So files are subject to checks, but loading code from things like
> sockets is always fine?

As I said in a previous email, these checks do not handle fifo either.
This is relevant in a threat model targeting persistent attacks (and
with additional protections/restrictions). We may want to only whitelist
fifo, but I don't get how a socket is relevant here. Can you please clarify?

> 
>> +       if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
>> +                       !(mask & MAY_EXECMOUNT))
>> +               return -EACCES;
>> +
>> +       /*
>> +        * May prefer acl_permission_check() instead of generic_permission(),
>> +        * to not be bypassable with CAP_DAC_READ_SEARCH.
>> +        */
>> +       if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
>> +               return generic_permission(inode, MAY_EXEC);
>> +
>> +       return 0;
>> +}
>> +
>>  static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
>> +       LSM_HOOK_INIT(inode_permission, yama_inode_permission),
>>         LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>>         LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>>         LSM_HOOK_INIT(task_prctl, yama_task_prctl),
>> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
>>         return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>>  }
>>
>> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
>> +                                         void __user *buffer, size_t *lenp,
>> +                                         loff_t *ppos)
>> +{
>> +       int error;
>> +
>> +       if (write) {
>> +               struct ctl_table table_copy;
>> +               int tmp_mayexec_enforce;
>> +
>> +               if (!capable(CAP_MAC_ADMIN))
>> +                       return -EPERM;
> 
> Don't put capable() checks in sysctls, it doesn't work.
> 

I tested it and the root user can indeed open the file even if the
process doesn't have CAP_MAC_ADMIN, however writing in the sysctl file
is denied. Btw there is a similar check in the previous function
(yama_dointvec_minmax).

Thanks

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13  5:13       ` Florian Weimer
@ 2018-12-13 14:57         ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-13 14:57 UTC (permalink / raw)
  To: Florian Weimer, James Morris
  Cc: Mickaël Salaün, linux-kernel, Al Viro, Jonathan Corbet,
	Kees Cook, Matthew Garrett, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel


On 13/12/2018 06:13, Florian Weimer wrote:
> * James Morris:
> 
>> On Wed, 12 Dec 2018, Florian Weimer wrote:
>>
>>> * James Morris:
>>>
>>>> If you're depending on the script interpreter to flag that the user may 
>>>> execute code, this seems to be equivalent in security terms to depending 
>>>> on the user.  e.g. what if the user uses ptrace and clears O_MAYEXEC?

This security mechanism makes sense in an hardened system where the user
is not allowed to import and execute new file (write xor execute
policy). This can be enforced with appropriate mount points a more
advanced access control policy.

>>>
>>> The argument I've heard is this: Using ptrace (and adding the +x
>>> attribute) are auditable events.
>>
>> I guess you could also preload a modified libc which strips the flag.
> 
> My understanding is that this new libc would have to come somewhere, and
> making it executable would be an auditable even as well.

Auditing is a possible use case as well, but the W^X idea is to deny use
of libraries which are not in an executable mount point, i.e. only
execute trusted code.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13  3:02 ` Matthew Wilcox
  2018-12-13  5:22   ` Florian Weimer
  2018-12-13 11:04   ` Mimi Zohar
@ 2018-12-13 15:17   ` Mickaël Salaün
  2018-12-13 17:13     ` Matthew Wilcox
  2 siblings, 1 reply; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-13 15:17 UTC (permalink / raw)
  To: Matthew Wilcox, Mickaël Salaün
  Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel


On 13/12/2018 04:02, Matthew Wilcox wrote:
> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
>> The goal of this patch series is to control script interpretation.  A
>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>> interpreter to delegate to the kernel (and thus the system security
>> policy) the permission to interpret scripts or other files containing
>> what can be seen as commands.
> 
> I don't have a problem with the concept, but we're running low on O_ bits.
> Does this have to be done before the process gets a file descriptor,
> or could we have a new syscall?  Since we're going to be changing the
> interpreters anyway, it doesn't seem like too much of an imposition to
> ask them to use:
> 
> 	int verify_for_exec(int fd)
> 
> instead of adding an O_MAYEXEC.
> 

Adding a new syscall for this simple use case seems excessive. I think
that the open/openat syscall familly are the right place to do an atomic
open and permission check, the same way the kernel does for other file
access. Moreover, it will be easier to patch upstream interpreters
without the burden of handling a (new) syscall that may not exist on the
running system, whereas unknown open flags are ignored.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 15:17   ` Mickaël Salaün
@ 2018-12-13 17:13     ` Matthew Wilcox
  2018-12-13 17:36       ` Mickaël Salaün
  0 siblings, 1 reply; 43+ messages in thread
From: Matthew Wilcox @ 2018-12-13 17:13 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
> On 13/12/2018 04:02, Matthew Wilcox wrote:
> > On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
> >> The goal of this patch series is to control script interpretation.  A
> >> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> >> interpreter to delegate to the kernel (and thus the system security
> >> policy) the permission to interpret scripts or other files containing
> >> what can be seen as commands.
> > 
> > I don't have a problem with the concept, but we're running low on O_ bits.
> > Does this have to be done before the process gets a file descriptor,
> > or could we have a new syscall?  Since we're going to be changing the
> > interpreters anyway, it doesn't seem like too much of an imposition to
> > ask them to use:
> > 
> > 	int verify_for_exec(int fd)
> > 
> > instead of adding an O_MAYEXEC.
> 
> Adding a new syscall for this simple use case seems excessive. I think

We have somewhat less than 400 syscalls today.  We have 20 O_ bits defined.
Obviously there's a lower practical limit on syscalls, but in principle
we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.

> that the open/openat syscall familly are the right place to do an atomic
> open and permission check, the same way the kernel does for other file
> access. Moreover, it will be easier to patch upstream interpreters
> without the burden of handling a (new) syscall that may not exist on the
> running system, whereas unknown open flags are ignored.

Ah, but that's the problem.  The interpreter can see an -ENOSYS response
and handle it appropriately.  If the flag is silently ignored, the
interpreter has no idea whether it can do a racy check or whether to
skip even trying to do the check.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 17:13     ` Matthew Wilcox
@ 2018-12-13 17:36       ` Mickaël Salaün
  2018-12-13 17:44         ` Matthew Wilcox
  0 siblings, 1 reply; 43+ messages in thread
From: Mickaël Salaün @ 2018-12-13 17:36 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel


On 13/12/2018 18:13, Matthew Wilcox wrote:
> On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
>> On 13/12/2018 04:02, Matthew Wilcox wrote:
>>> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
>>>> The goal of this patch series is to control script interpretation.  A
>>>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>>>> interpreter to delegate to the kernel (and thus the system security
>>>> policy) the permission to interpret scripts or other files containing
>>>> what can be seen as commands.
>>>
>>> I don't have a problem with the concept, but we're running low on O_ bits.
>>> Does this have to be done before the process gets a file descriptor,
>>> or could we have a new syscall?  Since we're going to be changing the
>>> interpreters anyway, it doesn't seem like too much of an imposition to
>>> ask them to use:
>>>
>>> 	int verify_for_exec(int fd)
>>>
>>> instead of adding an O_MAYEXEC.
>>
>> Adding a new syscall for this simple use case seems excessive. I think
> 
> We have somewhat less than 400 syscalls today.  We have 20 O_ bits defined.
> Obviously there's a lower practical limit on syscalls, but in principle
> we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.
> 
>> that the open/openat syscall familly are the right place to do an atomic
>> open and permission check, the same way the kernel does for other file
>> access. Moreover, it will be easier to patch upstream interpreters
>> without the burden of handling a (new) syscall that may not exist on the
>> running system, whereas unknown open flags are ignored.
> 
> Ah, but that's the problem.  The interpreter can see an -ENOSYS response
> and handle it appropriately.  If the flag is silently ignored, the
> interpreter has no idea whether it can do a racy check or whether to
> skip even trying to do the check.

Right, but the interpreter should interpret the script if the open with
O_MAYEXEC succeed (but not otherwise): it may be because the flag is
known by the kernel and the system policy allow this call, or because
the (old) kernel doesn't known about this flag (which is fine and needed
for backward compatibility). The script interpretation must not failed
if the kernel doesn't support O_MAYEXEC, it is then useless for the
interpreter to do any additional check.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
  2018-12-13 17:36       ` Mickaël Salaün
@ 2018-12-13 17:44         ` Matthew Wilcox
  0 siblings, 0 replies; 43+ messages in thread
From: Matthew Wilcox @ 2018-12-13 17:44 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel

On Thu, Dec 13, 2018 at 06:36:15PM +0100, Mickaël Salaün wrote:
> On 13/12/2018 18:13, Matthew Wilcox wrote:
> > On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
> >> Adding a new syscall for this simple use case seems excessive. I think
> > 
> > We have somewhat less than 400 syscalls today.  We have 20 O_ bits defined.
> > Obviously there's a lower practical limit on syscalls, but in principle
> > we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.
> > 
> >> that the open/openat syscall familly are the right place to do an atomic
> >> open and permission check, the same way the kernel does for other file
> >> access. Moreover, it will be easier to patch upstream interpreters
> >> without the burden of handling a (new) syscall that may not exist on the
> >> running system, whereas unknown open flags are ignored.
> > 
> > Ah, but that's the problem.  The interpreter can see an -ENOSYS response
> > and handle it appropriately.  If the flag is silently ignored, the
> > interpreter has no idea whether it can do a racy check or whether to
> > skip even trying to do the check.
> 
> Right, but the interpreter should interpret the script if the open with
> O_MAYEXEC succeed (but not otherwise): it may be because the flag is
> known by the kernel and the system policy allow this call, or because
> the (old) kernel doesn't known about this flag (which is fine and needed
> for backward compatibility). The script interpretation must not failed
> if the kernel doesn't support O_MAYEXEC, it is then useless for the
> interpreter to do any additional check.

If that's the way interpreters want to work, then that's fine.  They
can just call the verify() syscall and ignore the -ENOSYS.  Done.

Or somebody who cares very, very deeply can change the interpreter to
decline to run any scripts if the kernel returns -ENOSYS.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2018-12-13 14:49     ` Mickaël Salaün
@ 2019-01-03 11:17       ` Jann Horn
  2019-01-08 13:29         ` Mickaël Salaün
  0 siblings, 1 reply; 43+ messages in thread
From: Jann Horn @ 2019-01-03 11:17 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Mickaël Salaün, kernel list, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk-manpages, zohar, philippe.trebuchet, shuah,
	thibaut.sautereau, vincent.strubel, yves-alexis.perez,
	Kernel Hardening, Linux API, linux-security-module,
	linux-fsdevel, Andy Lutomirski

On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
<mickael.salaun@ssi.gouv.fr> wrote:
> On 12/12/2018 18:09, Jann Horn wrote:
> > On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
> >> Enable to either propagate the mount options from the underlying VFS
> >> mount to prevent execution, or to propagate the file execute permission.
> >> This may allow a script interpreter to check execution permissions
> >> before reading commands from a file.
> >>
> >> The main goal is to be able to protect the kernel by restricting
> >> arbitrary syscalls that an attacker could perform with a crafted binary
> >> or certain script languages.  It also improves multilevel isolation
> >> by reducing the ability of an attacker to use side channels with
> >> specific code.  These restrictions can natively be enforced for ELF
> >> binaries (with the noexec mount option) but require this kernel
> >> extension to properly handle scripts (e.g., Python, Perl).
> >>
> >> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
> >> behavior.  A following patch adds documentation.
[...]
> >> +{
> >> +       if (!(mask & MAY_OPENEXEC))
> >> +               return 0;
> >> +       /*
> >> +        * Match regular files and directories to make it easier to
> >> +        * modify script interpreters.
> >> +        */
> >> +       if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> >> +               return 0;
> >
> > So files are subject to checks, but loading code from things like
> > sockets is always fine?
>
> As I said in a previous email, these checks do not handle fifo either.
> This is relevant in a threat model targeting persistent attacks (and
> with additional protections/restrictions). We may want to only whitelist
> fifo, but I don't get how a socket is relevant here. Can you please clarify?

I don't think that there's a security problem here. I just think it's
weird to have the extra check when it seems to me like it isn't really
necessary - nobody is going to want to execute a socket or fifo
anyway, right?

> >
> >> +       if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
> >> +                       !(mask & MAY_EXECMOUNT))
> >> +               return -EACCES;
> >> +
> >> +       /*
> >> +        * May prefer acl_permission_check() instead of generic_permission(),
> >> +        * to not be bypassable with CAP_DAC_READ_SEARCH.
> >> +        */
> >> +       if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
> >> +               return generic_permission(inode, MAY_EXEC);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >>  static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
> >> +       LSM_HOOK_INIT(inode_permission, yama_inode_permission),
> >>         LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
> >>         LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
> >>         LSM_HOOK_INIT(task_prctl, yama_task_prctl),
> >> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
> >>         return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
> >>  }
> >>
> >> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
> >> +                                         void __user *buffer, size_t *lenp,
> >> +                                         loff_t *ppos)
> >> +{
> >> +       int error;
> >> +
> >> +       if (write) {
> >> +               struct ctl_table table_copy;
> >> +               int tmp_mayexec_enforce;
> >> +
> >> +               if (!capable(CAP_MAC_ADMIN))
> >> +                       return -EPERM;
> >
> > Don't put capable() checks in sysctls, it doesn't work.
> >
>
> I tested it and the root user can indeed open the file even if the
> process doesn't have CAP_MAC_ADMIN, however writing in the sysctl file
> is denied. Btw there is a similar check in the previous function
> (yama_dointvec_minmax).

It's still wrong. If an attacker without CAP_MAC_ADMIN opens the
sysctl file, then passes the file descriptor to a setcap binary that
has CAP_MAC_ADMIN as stdout/stderr, and the setcap binary writes to
it, then the capable() check is bypassed. (But of course, to open the
sysctl file in the first place, you'd need to be root (uid 0), so the
check doesn't really matter.)

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2019-01-03 11:17       ` Jann Horn
@ 2019-01-08 13:29         ` Mickaël Salaün
  2019-01-08 23:30           ` Kees Cook
  0 siblings, 1 reply; 43+ messages in thread
From: Mickaël Salaün @ 2019-01-08 13:29 UTC (permalink / raw)
  To: Jann Horn
  Cc: Mickaël Salaün, kernel list, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk-manpages, zohar, philippe.trebuchet, shuah,
	thibaut.sautereau, vincent.strubel, yves-alexis.perez,
	Kernel Hardening, Linux API, linux-security-module,
	linux-fsdevel, Andy Lutomirski


On 03/01/2019 12:17, Jann Horn wrote:
> On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
> <mickael.salaun@ssi.gouv.fr> wrote:
>> On 12/12/2018 18:09, Jann Horn wrote:
>>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
>>>> Enable to either propagate the mount options from the underlying VFS
>>>> mount to prevent execution, or to propagate the file execute permission.
>>>> This may allow a script interpreter to check execution permissions
>>>> before reading commands from a file.
>>>>
>>>> The main goal is to be able to protect the kernel by restricting
>>>> arbitrary syscalls that an attacker could perform with a crafted binary
>>>> or certain script languages.  It also improves multilevel isolation
>>>> by reducing the ability of an attacker to use side channels with
>>>> specific code.  These restrictions can natively be enforced for ELF
>>>> binaries (with the noexec mount option) but require this kernel
>>>> extension to properly handle scripts (e.g., Python, Perl).
>>>>
>>>> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
>>>> behavior.  A following patch adds documentation.
> [...]
>>>> +{
>>>> +       if (!(mask & MAY_OPENEXEC))
>>>> +               return 0;
>>>> +       /*
>>>> +        * Match regular files and directories to make it easier to
>>>> +        * modify script interpreters.
>>>> +        */
>>>> +       if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
>>>> +               return 0;
>>>
>>> So files are subject to checks, but loading code from things like
>>> sockets is always fine?
>>
>> As I said in a previous email, these checks do not handle fifo either.
>> This is relevant in a threat model targeting persistent attacks (and
>> with additional protections/restrictions). We may want to only whitelist
>> fifo, but I don't get how a socket is relevant here. Can you please clarify?
> 
> I don't think that there's a security problem here. I just think it's
> weird to have the extra check when it seems to me like it isn't really
> necessary - nobody is going to want to execute a socket or fifo
> anyway, right?

Right, the fifo whitelisting should answer your concern then.

> 
>>>
>>>> +       if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
>>>> +                       !(mask & MAY_EXECMOUNT))
>>>> +               return -EACCES;
>>>> +
>>>> +       /*
>>>> +        * May prefer acl_permission_check() instead of generic_permission(),
>>>> +        * to not be bypassable with CAP_DAC_READ_SEARCH.
>>>> +        */
>>>> +       if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
>>>> +               return generic_permission(inode, MAY_EXEC);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>>  static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
>>>> +       LSM_HOOK_INIT(inode_permission, yama_inode_permission),
>>>>         LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>>>>         LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>>>>         LSM_HOOK_INIT(task_prctl, yama_task_prctl),
>>>> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
>>>>         return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>>>>  }
>>>>
>>>> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
>>>> +                                         void __user *buffer, size_t *lenp,
>>>> +                                         loff_t *ppos)
>>>> +{
>>>> +       int error;
>>>> +
>>>> +       if (write) {
>>>> +               struct ctl_table table_copy;
>>>> +               int tmp_mayexec_enforce;
>>>> +
>>>> +               if (!capable(CAP_MAC_ADMIN))
>>>> +                       return -EPERM;
>>>
>>> Don't put capable() checks in sysctls, it doesn't work.
>>>
>>
>> I tested it and the root user can indeed open the file even if the
>> process doesn't have CAP_MAC_ADMIN, however writing in the sysctl file
>> is denied. Btw there is a similar check in the previous function
>> (yama_dointvec_minmax).
> 
> It's still wrong. If an attacker without CAP_MAC_ADMIN opens the
> sysctl file, then passes the file descriptor to a setcap binary that
> has CAP_MAC_ADMIN as stdout/stderr, and the setcap binary writes to
> it, then the capable() check is bypassed. (But of course, to open the
> sysctl file in the first place, you'd need to be root (uid 0), so the
> check doesn't really matter.)

I agree with you that a confused deputy attack may uses file
descriptors, but I don't see how the current sysctl API may be used to
check the process capability at open time. Anyway, on a properly
configured system, especially one leveraging Linux capabilities (e.g.
CLIP OS), root processes may not have CAP_SYS_ADMIN. Moreover, SUID or
fcap binaries may not be available to an attacker (e.g. in a container).

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2019-01-08 13:29         ` Mickaël Salaün
@ 2019-01-08 23:30           ` Kees Cook
  2019-01-09 13:41             ` Mickaël Salaün
  0 siblings, 1 reply; 43+ messages in thread
From: Kees Cook @ 2019-01-08 23:30 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Jann Horn, Mickaël Salaün, kernel list, Al Viro,
	James Morris, Jonathan Corbet, Matthew Garrett,
	Michael Kerrisk-manpages, Mimi Zohar, philippe.trebuchet,
	Shuah Khan, thibaut.sautereau, vincent.strubel,
	Perez Yves-Alexis, Kernel Hardening, Linux API,
	linux-security-module, linux-fsdevel, Andy Lutomirski

On Tue, Jan 8, 2019 at 5:29 AM Mickaël Salaün
<mickael.salaun@ssi.gouv.fr> wrote:
>
>
> On 03/01/2019 12:17, Jann Horn wrote:
> > On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
> > <mickael.salaun@ssi.gouv.fr> wrote:
> >> On 12/12/2018 18:09, Jann Horn wrote:
> >>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
> >>>> Enable to either propagate the mount options from the underlying VFS
> >>>> mount to prevent execution, or to propagate the file execute permission.
> >>>> This may allow a script interpreter to check execution permissions
> >>>> before reading commands from a file.
> >>>>
> >>>> The main goal is to be able to protect the kernel by restricting
> >>>> arbitrary syscalls that an attacker could perform with a crafted binary
> >>>> or certain script languages.  It also improves multilevel isolation
> >>>> by reducing the ability of an attacker to use side channels with
> >>>> specific code.  These restrictions can natively be enforced for ELF
> >>>> binaries (with the noexec mount option) but require this kernel
> >>>> extension to properly handle scripts (e.g., Python, Perl).

I like this idea, but I think it shouldn't live in Yama (since it is
currently intended to be a ptrace-policy-only LSM). It was
_originally_ designed to do various DAC improvements, but the
agreement was that those should live directly in the VFS instead (i.e.
the symlink, hardlink and now fifo and regular file defenses).

This should likely go in similarly. (But if not, it could also be its own LSM.)

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
  2019-01-08 23:30           ` Kees Cook
@ 2019-01-09 13:41             ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2019-01-09 13:41 UTC (permalink / raw)
  To: Kees Cook, Al Viro, James Morris
  Cc: Jann Horn, Mickaël Salaün, kernel list,
	Jonathan Corbet, Matthew Garrett, Michael Kerrisk-manpages,
	Mimi Zohar, philippe.trebuchet, Shuah Khan, thibaut.sautereau,
	vincent.strubel, Perez Yves-Alexis, Kernel Hardening, Linux API,
	linux-security-module, linux-fsdevel, Andy Lutomirski


On 09/01/2019 00:30, Kees Cook wrote:
> On Tue, Jan 8, 2019 at 5:29 AM Mickaël Salaün
> <mickael.salaun@ssi.gouv.fr> wrote:
>>
>>
>> On 03/01/2019 12:17, Jann Horn wrote:
>>> On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
>>> <mickael.salaun@ssi.gouv.fr> wrote:
>>>> On 12/12/2018 18:09, Jann Horn wrote:
>>>>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
>>>>>> Enable to either propagate the mount options from the underlying VFS
>>>>>> mount to prevent execution, or to propagate the file execute permission.
>>>>>> This may allow a script interpreter to check execution permissions
>>>>>> before reading commands from a file.
>>>>>>
>>>>>> The main goal is to be able to protect the kernel by restricting
>>>>>> arbitrary syscalls that an attacker could perform with a crafted binary
>>>>>> or certain script languages.  It also improves multilevel isolation
>>>>>> by reducing the ability of an attacker to use side channels with
>>>>>> specific code.  These restrictions can natively be enforced for ELF
>>>>>> binaries (with the noexec mount option) but require this kernel
>>>>>> extension to properly handle scripts (e.g., Python, Perl).
> 
> I like this idea, but I think it shouldn't live in Yama (since it is
> currently intended to be a ptrace-policy-only LSM). It was
> _originally_ designed to do various DAC improvements, but the
> agreement was that those should live directly in the VFS instead (i.e.
> the symlink, hardlink and now fifo and regular file defenses).
> 
> This should likely go in similarly. (But if not, it could also be its own LSM.)
> 

I think that Yama is quite handy and make sense here, but I'm fine
putting this knob elsewhere. However, I was thinking, for a future patch
series, to add another sysctl to lock this choice, i.e. generalizing the
way Yama can lock the ptrace_scope.

What matter here is the ability for an LSM to use this O_MAYEXEC flag.
Yama is a good place to showcase this feature and I think it is cleaner
to leverage the LSM framework to put new (optional) security features. I
can easily create a new LSM but it would be pretty similar to Yama...
What do you think about it James and Al?

Side question: wouldn't it be better to use a 0600 mode (instead of
0644) for this kind of sysctl?

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12 14:43   ` Jan Kara
                       ` (2 preceding siblings ...)
  2018-12-13  9:47     ` Matthew Bobrowski
@ 2019-04-15 18:47     ` Steve Grubb
  2019-04-16 11:49       ` Florian Weimer
  2019-04-17 14:55       ` Mickaël Salaün
  2019-08-04 23:55     ` Andy Lutomirski
  4 siblings, 2 replies; 43+ messages in thread
From: Steve Grubb @ 2019-04-15 18:47 UTC (permalink / raw)
  To: Jan Kara
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, Matthew Bobrowski

Hello,

On Wednesday, December 12, 2018 9:43:06 AM EDT Jan Kara wrote:
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> > 
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> > 
> > A simple security policy implementation is available in a following
> > patch for Yama.
> > 
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d
> > 6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch This patch has
> > been used for more than 10 years with customized script interpreters. 
> > Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYE
> > XEC
> > 
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> > Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> > Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
> 
> ...
> 
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags,
> > umode_t mode, struct open_flags *o> 
> >  	if (flags & O_APPEND)
> >  	
> >  		acc_mode |= MAY_APPEND;
> > 
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> > 
> >  	op->acc_mode = acc_mode;
> >  	
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to CC.
> Just an idea...

Late in replying. But I think it's important to have a deep look into the 
issue.

TL;DR - This is a gentle man's handshake. It won't _really_ solve the 
problem.

This flag that is being proposed means that you would have to patch all 
interpreters to use it. If you are sure that upstreams will accept that, why 
not just change the policy to interpreters shouldn't execute anything unless 
the execute bit is set? That is simpler and doesn't need a kernel change. And 
setting the execute bit is an auditable event.

The bottom line is that any interpreter has to become a security policy 
enforcement point whether by indicating it wants to execute by setting a flag 
or by refusing to use a file without execute bit set. But this just moves the 
problem to one that is harder to fix. Why in the world does any programming 
language allow programs to be loaded via stdin? 

It is possible to wget a program and pipe it into python which subsequently 
pulls down an ELF shared object and runs it all without touching disk via 
memfd_create (e.g. SnakeEater). This is all direct to memory execution. And 
direct to memory bypasses anti-virus, selinux, IMA, application whitelisting, 
and other integrity schemes.

So, to fix this problem, you really need to not allow any programs to load via 
stdin so that everything that executes has to touch disk. This way you can 
get a fanotify event and see the application and vote yes/no on allowing it. 
And this will be particularly harder with the memfd_create fix for the runc 
container breakout. Prior to that, there were very few uses of that system 
call. Now it may be very common which means finding malicious use just got 
harder to spot.

But assuming these problems got fixed, then we have yet another place to look. 
Many interpreters allow you to specify a command to run via arguments. Some 
have a small buffer and some allow lengthy programs to be entered as an 
argument. One strategy might be that an attacker can bootstrap a lengthier 
program across the network. Python for example allows loading modules across 
a network. All you need to put in the commandline is the override for the 
module loader and a couple modules to import. It then loads the modules 
remotely. Getting rid of this hole will likely lead to some unhappy people - 
meaning it can't be fixed.

And even if we get that fixed, we have one last hole to plug. Shells. One can 
simply start a shell and paste their program into the shell and then execute 
it. You can easily do this with bash or python or any language that has a 
REPL (read–eval–print loop). To fix this means divorcing the notion of a 
language from a REPL. Production systems really do not need a Python shell, 
they need the interpreter. I doubt that this would be popular. But fixing each 
of these issues is what it would take to prevent unknown software from 
running. Not going this far leaves holes.

Best Regards,
-Steve

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-04-15 18:47     ` Steve Grubb
@ 2019-04-16 11:49       ` Florian Weimer
  2019-04-16 15:34         ` Steve Grubb
  2019-04-17 14:55       ` Mickaël Salaün
  1 sibling, 1 reply; 43+ messages in thread
From: Florian Weimer @ 2019-04-16 11:49 UTC (permalink / raw)
  To: Steve Grubb
  Cc: Jan Kara, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mickaël Salaün, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, Matthew Bobrowski

* Steve Grubb:

> This flag that is being proposed means that you would have to patch all 
> interpreters to use it. If you are sure that upstreams will accept that, why 
> not just change the policy to interpreters shouldn't execute anything unless 
> the execute bit is set? That is simpler and doesn't need a kernel change. And 
> setting the execute bit is an auditable event.

I think we need something like O_MAYEXEC so that security policies can
be enforced and noexec mounts can be detected.  I don't think it's a
good idea to do this in userspace, especially the latter.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-04-16 11:49       ` Florian Weimer
@ 2019-04-16 15:34         ` Steve Grubb
  2019-04-17 10:01           ` Florian Weimer
  0 siblings, 1 reply; 43+ messages in thread
From: Steve Grubb @ 2019-04-16 15:34 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Jan Kara, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mickaël Salaün, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, Matthew Bobrowski

On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
> * Steve Grubb:
> > This flag that is being proposed means that you would have to patch all
> > interpreters to use it. If you are sure that upstreams will accept that,
> > why not just change the policy to interpreters shouldn't execute
> > anything unless the execute bit is set? That is simpler and doesn't need
> > a kernel change. And setting the execute bit is an auditable event.
> 
> I think we need something like O_MAYEXEC so that security policies can
> be enforced and noexec mounts can be detected.

Application whitelisting can already today stop unknown software without 
needing O_MAYEXEC.

> I don't think it's a good idea to do this in userspace, especially the
> latter.

The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program. 
This does not require privs to do so.

But let's consider that this comes to pass and every interpreter is updated 
and IMA can see the O_MAYEXEC flag. Attackers now simply pivot to running 
programs via stdin. It never touches disk and therefore nothing enforces 
security policy. This already is among the most common ways that malware runs 
today to evade detection.

-Steve

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-04-16 15:34         ` Steve Grubb
@ 2019-04-17 10:01           ` Florian Weimer
  2019-04-17 15:04             ` Mickaël Salaün
  0 siblings, 1 reply; 43+ messages in thread
From: Florian Weimer @ 2019-04-17 10:01 UTC (permalink / raw)
  To: Steve Grubb
  Cc: Jan Kara, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mickaël Salaün, Mimi Zohar,
	Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
	Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
	linux-security-module, linux-fsdevel, Matthew Bobrowski

* Steve Grubb:

> On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
>> * Steve Grubb:
>> > This flag that is being proposed means that you would have to patch all
>> > interpreters to use it. If you are sure that upstreams will accept that,
>> > why not just change the policy to interpreters shouldn't execute
>> > anything unless the execute bit is set? That is simpler and doesn't need
>> > a kernel change. And setting the execute bit is an auditable event.
>> 
>> I think we need something like O_MAYEXEC so that security policies can
>> be enforced and noexec mounts can be detected.
>
> Application whitelisting can already today stop unknown software without 
> needing O_MAYEXEC.

I'm somewhat interested in using this to add a proper check for
executability to explicit dynamic loader invocations.  In other words,
this

  /lib64/ld-linux-x86-64.so.2 /path/to/noexec/fs/program

should refuse to run the program if the program is located on a file
system mounted with the noexec attribute.

> The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
> bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program. 
> This does not require privs to do so.

That doesn't really help with the above.

> But let's consider that this comes to pass and every interpreter is
> updated and IMA can see the O_MAYEXEC flag. Attackers now simply pivot
> to running programs via stdin. It never touches disk and therefore
> nothing enforces security policy. This already is among the most
> common ways that malware runs today to evade detection.

Are you referring to Windows malware using Powershell?

I'm not sure this is applicable to Linux.  We do not have much
behavioral monitoring anyway.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-04-15 18:47     ` Steve Grubb
  2019-04-16 11:49       ` Florian Weimer
@ 2019-04-17 14:55       ` Mickaël Salaün
  1 sibling, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2019-04-17 14:55 UTC (permalink / raw)
  To: Steve Grubb, Jan Kara
  Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel, Matthew Bobrowski, Florian Weimer


On 15/04/2019 20:47, Steve Grubb wrote:
> Hello,
>
> On Wednesday, December 12, 2018 9:43:06 AM EDT Jan Kara wrote:
>> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d
>>> 6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch This patch has
>>> been used for more than 10 years with customized script interpreters.
>>> Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYE
>>> XEC
>>>
>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>>> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
>>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags,
>>> umode_t mode, struct open_flags *o>
>>>     if (flags & O_APPEND)
>>>
>>>             acc_mode |= MAY_APPEND;
>>>
>>> +   /* Check execution permissions on open. */
>>> +   if (flags & O_MAYEXEC)
>>> +           acc_mode |= MAY_OPENEXEC;
>>> +
>>>
>>>     op->acc_mode = acc_mode;
>>>
>>>     op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to CC.
>> Just an idea...
>
> Late in replying. But I think it's important to have a deep look into the
> issue.
>
> TL;DR - This is a gentle man's handshake. It won't _really_ solve the
> problem.

Thanks for your comments. You should find most answers in this thread:
https://lore.kernel.org/lkml/20181212081712.32347-4-mic@digikod.net/

The threat model targets persistent attacks. This O_MAYEXEC flag is not
a silver bullet but it's a needed block to enforce a security policy on
a trusted system. This means that every component executable on the
system must be controlled, which means they may need some bit of
customization. Today no userspace application use this flag (except in
CLIP OS), but we need to first create a feature before it can be used.

It is very important to have in mind that a system security policy need
to have a (central) security manager, in this case the kernel thanks to
Yama's policy (but it could be SELinux, IMA or any other LSM). The goal
is not to give to the developer the job of defining a security policy
for the *system*; this job is for the system administrator (or the distro).

>
> This flag that is being proposed means that you would have to patch all
> interpreters to use it. If you are sure that upstreams will accept that, why
> not just change the policy to interpreters shouldn't execute anything unless
> the execute bit is set? That is simpler and doesn't need a kernel change. And
> setting the execute bit is an auditable event.

As said above, the definition of a the security policy is the job of the
system administrator. Moreover, the security policy may be defined by
the mount point restrictions (i.e. noexec) but it should be definable
with something else (e.g. a SELinux or IMA policy which may be agnostic
to the mount points).

>
> The bottom line is that any interpreter has to become a security policy
> enforcement point whether by indicating it wants to execute by setting a flag
> or by refusing to use a file without execute bit set. But this just moves the
> problem to one that is harder to fix. Why in the world does any programming
> language allow programs to be loaded via stdin?
>
> It is possible to wget a program and pipe it into python which subsequently
> pulls down an ELF shared object and runs it all without touching disk via
> memfd_create (e.g. SnakeEater). This is all direct to memory execution. And
> direct to memory bypasses anti-virus, selinux, IMA, application whitelisting,
> and other integrity schemes.
>
> So, to fix this problem, you really need to not allow any programs to load via
> stdin so that everything that executes has to touch disk. This way you can
> get a fanotify event and see the application and vote yes/no on allowing it.
> And this will be particularly harder with the memfd_create fix for the runc
> container breakout. Prior to that, there were very few uses of that system
> call. Now it may be very common which means finding malicious use just got
> harder to spot.

As said above, stdin must be restricted in some way. You may want to
take a look at the CLIP OS patches (which doesn't only add the O_MAYEXEC
flag but restrict other way to interpret code). It may be foolish to
block or restrict stdin for interpreters on a developer workstation, but
it may make sense for an embedded custom system.

The same apply for memfd_create. If you want to enforce a security
policy on this kind of *file descriptor*, you should ask to the proper
LSM to do so. The current Yama patch deal with this kind of FD if they
are accessed through /proc/*/fd because the procfs is mounted with
noexec. Anyway, the interpreter must *inform* the LSM that it wants to
execute/interpret something from this FD, which is done thanks to the
O_MAYEXEC flag.

>
> But assuming these problems got fixed, then we have yet another place to look.
> Many interpreters allow you to specify a command to run via arguments. Some
> have a small buffer and some allow lengthy programs to be entered as an
> argument. One strategy might be that an attacker can bootstrap a lengthier
> program across the network. Python for example allows loading modules across
> a network. All you need to put in the commandline is the override for the
> module loader and a couple modules to import. It then loads the modules
> remotely. Getting rid of this hole will likely lead to some unhappy people -
> meaning it can't be fixed.

Again, this depend on the threat model and the corresponding product. If
you want to handle everything on your system, then you may need some
adjustments.

>
> And even if we get that fixed, we have one last hole to plug. Shells. One can
> simply start a shell and paste their program into the shell and then execute
> it. You can easily do this with bash or python or any language that has a
> REPL (read–eval–print loop). To fix this means divorcing the notion of a
> language from a REPL. Production systems really do not need a Python shell,
> they need the interpreter. I doubt that this would be popular. But fixing each
> of these issues is what it would take to prevent unknown software from
> running. Not going this far leaves holes.

This is also covered by the threat model defined in the patch 3/5 (i.e.
protect the kernel by restricting arbitrary syscalls).

Regards,

--
Mickaël Salaün
ANSSI/SDE/ST/LAM

Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-04-17 10:01           ` Florian Weimer
@ 2019-04-17 15:04             ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2019-04-17 15:04 UTC (permalink / raw)
  To: Florian Weimer, Steve Grubb
  Cc: Jan Kara, Mickaël Salaün, linux-kernel, Al Viro,
	James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
	Michael Kerrisk, Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	kernel-hardening, linux-api, linux-security-module,
	linux-fsdevel, Matthew Bobrowski


On 17/04/2019 12:01, Florian Weimer wrote:
> * Steve Grubb:
>
>> On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
>>> * Steve Grubb:
>>>> This flag that is being proposed means that you would have to patch all
>>>> interpreters to use it. If you are sure that upstreams will accept that,
>>>> why not just change the policy to interpreters shouldn't execute
>>>> anything unless the execute bit is set? That is simpler and doesn't need
>>>> a kernel change. And setting the execute bit is an auditable event.
>>>
>>> I think we need something like O_MAYEXEC so that security policies can
>>> be enforced and noexec mounts can be detected.
>>
>> Application whitelisting can already today stop unknown software without
>> needing O_MAYEXEC.

Whitelisting may be a lot of thing (path/TPE, signed binaries…), but
being able to handle this with a global system configuration (instead of
app-specific hardcoded configuration) is a good idea. ;)

>
> I'm somewhat interested in using this to add a proper check for
> executability to explicit dynamic loader invocations.  In other words,
> this
>
>   /lib64/ld-linux-x86-64.so.2 /path/to/noexec/fs/program
>
> should refuse to run the program if the program is located on a file
> system mounted with the noexec attribute.

What if a sysadmin need to do this on an executable mount point? Being
able to enforce a security policy according to a configuration may fit
to much more use cases.

>
>> The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
>> bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program.
>> This does not require privs to do so.
>
> That doesn't really help with the above.

Right, ptrace/LD_PRELOAD and so on must be addressed by something else
than only O_MAYEXEC.

>
>> But let's consider that this comes to pass and every interpreter is
>> updated and IMA can see the O_MAYEXEC flag. Attackers now simply pivot
>> to running programs via stdin. It never touches disk and therefore
>> nothing enforces security policy. This already is among the most
>> common ways that malware runs today to evade detection.

As my previous reply, use cases like stdin may be restricted as well.

>
> Are you referring to Windows malware using Powershell?
>
> I'm not sure this is applicable to Linux.  We do not have much
> behavioral monitoring anyway.
>
> Thanks,
> Florian
>

--
Mickaël Salaün
ANSSI/SDE/ST/LAM

Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2018-12-12 14:43   ` Jan Kara
                       ` (3 preceding siblings ...)
  2019-04-15 18:47     ` Steve Grubb
@ 2019-08-04 23:55     ` Andy Lutomirski
  2019-08-06 16:40       ` Mickaël Salaün
  4 siblings, 1 reply; 43+ messages in thread
From: Andy Lutomirski @ 2019-08-04 23:55 UTC (permalink / raw)
  To: Jan Kara, Song Liu, Alexei Starovoitov, Daniel Borkmann
  Cc: Mickaël Salaün, LKML, Al Viro, James Morris,
	Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
	Mickaël Salaün, Mimi Zohar, Philippe Trébuchet,
	Shuah Khan, Thibaut Sautereau, Vincent Strubel,
	Yves-Alexis Perez, Kernel Hardening, Linux API, LSM List,
	Linux FS Devel, Steve Grubb, Matthew Bobrowski

On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> >
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> >
> > A simple security policy implementation is available in a following
> > patch for Yama.
> >
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > This patch has been used for more than 10 years with customized script
> > interpreters.  Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> >
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
> > Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
> > Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>
> ...
>
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >       if (flags & O_APPEND)
> >               acc_mode |= MAY_APPEND;
> >
> > +     /* Check execution permissions on open. */
> > +     if (flags & O_MAYEXEC)
> > +             acc_mode |= MAY_OPENEXEC;
> > +
> >       op->acc_mode = acc_mode;
> >
> >       op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...
>

I would really like to land this patch.  I'm fiddling with making
bpffs handle permissions intelligently, and the lack of a way to say
"hey, I want to open this bpf program so that I can run it" is
annoying.

--Andy

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
  2019-08-04 23:55     ` Andy Lutomirski
@ 2019-08-06 16:40       ` Mickaël Salaün
  0 siblings, 0 replies; 43+ messages in thread
From: Mickaël Salaün @ 2019-08-06 16:40 UTC (permalink / raw)
  To: Andy Lutomirski, Jan Kara, Song Liu, Alexei Starovoitov, Daniel Borkmann
  Cc: LKML, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
	Matthew Garrett, Michael Kerrisk, Mickaël Salaün,
	Mimi Zohar, Philippe Trébuchet, Shuah Khan,
	Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
	Kernel Hardening, Linux API, LSM List, Linux FS Devel,
	Steve Grubb, Matthew Bobrowski, Aleksa Sarai,
	Sean Christopherson


On 05/08/2019 01:55, Andy Lutomirski wrote:
> On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <jack@suse.cz> wrote:
>>
>> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>>> This patch has been used for more than 10 years with customized script
>>> interpreters.  Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>>
>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>>> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
>>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>>       if (flags & O_APPEND)
>>>               acc_mode |= MAY_APPEND;
>>>
>>> +     /* Check execution permissions on open. */
>>> +     if (flags & O_MAYEXEC)
>>> +             acc_mode |= MAY_OPENEXEC;
>>> +
>>>       op->acc_mode = acc_mode;
>>>
>>>       op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
>> CC. Just an idea...
>>
> 
> I would really like to land this patch.  I'm fiddling with making
> bpffs handle permissions intelligently, and the lack of a way to say
> "hey, I want to open this bpf program so that I can run it" is
> annoying.

Are you OK with this series? What about Aleksa's work on openat2(), and
Sean's work on SGX/noexec? Is it time to send a new patch series (with a
dedicated LSM instead of Yama)?

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2019-08-06 16:42 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-12  8:17 [RFC PATCH v1 0/5] Add support for O_MAYEXEC Mickaël Salaün
2018-12-12  8:17 ` [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
2018-12-12 14:43   ` Jan Kara
2018-12-12 17:09     ` Mickaël Salaün
2018-12-12 20:42     ` Mimi Zohar
2018-12-13  9:47     ` Matthew Bobrowski
2018-12-13 14:23       ` Mickaël Salaün
2019-04-15 18:47     ` Steve Grubb
2019-04-16 11:49       ` Florian Weimer
2019-04-16 15:34         ` Steve Grubb
2019-04-17 10:01           ` Florian Weimer
2019-04-17 15:04             ` Mickaël Salaün
2019-04-17 14:55       ` Mickaël Salaün
2019-08-04 23:55     ` Andy Lutomirski
2019-08-06 16:40       ` Mickaël Salaün
2018-12-12  8:17 ` [RFC PATCH v1 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie Mickaël Salaün
2018-12-12  8:17 ` [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC Mickaël Salaün
2018-12-12 14:28   ` Mickaël Salaün
2018-12-12 17:09   ` Jann Horn
2018-12-13 14:49     ` Mickaël Salaün
2019-01-03 11:17       ` Jann Horn
2019-01-08 13:29         ` Mickaël Salaün
2019-01-08 23:30           ` Kees Cook
2019-01-09 13:41             ` Mickaël Salaün
2018-12-12  8:17 ` [RFC PATCH v1 4/5] selftest/yama: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2018-12-12  8:17 ` [RFC PATCH v1 5/5] doc: Add documentation for Yama's open_mayexec_enforce Mickaël Salaün
2018-12-12 16:29 ` [RFC PATCH v1 0/5] Add support for O_MAYEXEC Jordan Glover
2018-12-12 17:01   ` Mickaël Salaün
2018-12-12 19:51 ` James Morris
2018-12-12 20:13   ` Florian Weimer
2018-12-12 23:40     ` James Morris
2018-12-13  5:13       ` Florian Weimer
2018-12-13 14:57         ` Mickaël Salaün
2018-12-13  3:02 ` Matthew Wilcox
2018-12-13  5:22   ` Florian Weimer
2018-12-13 11:04   ` Mimi Zohar
2018-12-13 11:26     ` Florian Weimer
2018-12-13 12:16       ` Mimi Zohar
2018-12-13 12:16     ` Matthew Wilcox
2018-12-13 15:17   ` Mickaël Salaün
2018-12-13 17:13     ` Matthew Wilcox
2018-12-13 17:36       ` Mickaël Salaün
2018-12-13 17:44         ` Matthew Wilcox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).