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
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 <[email protected]>
Reviewed-by: Philippe Trébuchet <[email protected]>
Reviewed-by: Thibaut Sautereau <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mickaël Salaün <[email protected]>
---
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
Signed-off-by: Mickaël Salaün <[email protected]>
Reviewed-by: Philippe Trébuchet <[email protected]>
Reviewed-by: Thibaut Sautereau <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mickaël Salaün <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Thibaut Sautereau <[email protected]>
Signed-off-by: Vincent Strubel <[email protected]>
Reviewed-by: Philippe Trébuchet <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mickaël Salaün <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Philippe Trébuchet <[email protected]>
Reviewed-by: Thibaut Sautereau <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mickaël Salaün <[email protected]>
---
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 <[email protected]>
+ * Authors: Kees Cook <[email protected]>
+ * Mickaël Salaün <[email protected]>
*
* 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
Test propagation of noexec mount points or file executability through
files open with or without O_MAYEXEC.
Signed-off-by: Mickaël Salaün <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Mickaël Salaün <[email protected]>
Cc: Shuah Khan <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
+ */
+
+#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
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 <[email protected]>
> Reviewed-by: Philippe Trébuchet <[email protected]>
> Reviewed-by: Thibaut Sautereau <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Mickaël Salaün <[email protected]>
> ---
> 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 <[email protected]>
> + * Authors: Kees Cook <[email protected]>
> + * Mickaël Salaün <[email protected]>
> *
> * 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)
>
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 <[email protected]>
> Signed-off-by: Thibaut Sautereau <[email protected]>
> Signed-off-by: Vincent Strubel <[email protected]>
> Reviewed-by: Philippe Tr?buchet <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Micka?l Sala?n <[email protected]>
...
> 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 <[email protected]>
SUSE Labs, CR
On Wednesday, December 12, 2018 9:17 AM, Mickaël Salaün <[email protected]> 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
Le 12/12/2018 à 17:29, Jordan Glover a écrit :
> On Wednesday, December 12, 2018 9:17 AM, Mickaël Salaün <[email protected]> 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
On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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 <[email protected]>
> Reviewed-by: Philippe Trébuchet <[email protected]>
> Reviewed-by: Thibaut Sautereau <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Mickaël Salaün <[email protected]>
> ---
[...]
> +/**
> + * 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.
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 <[email protected]>
>> Signed-off-by: Thibaut Sautereau <[email protected]>
>> Signed-off-by: Vincent Strubel <[email protected]>
>> Reviewed-by: Philippe Trébuchet <[email protected]>
>> Cc: Al Viro <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Mickaël Salaün <[email protected]>
>
> ...
>
>> 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
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
<[email protected]>
* 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
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
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
<[email protected]>
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.
* 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
* 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
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 <[email protected]>
> > Signed-off-by: Thibaut Sautereau <[email protected]>
> > Signed-off-by: Vincent Strubel <[email protected]>
> > Reviewed-by: Philippe Tr?buchet <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Micka?l Sala?n <[email protected]>
>
> ...
>
> > 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
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
* 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
[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
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.
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 <[email protected]>
>>> Signed-off-by: Thibaut Sautereau <[email protected]>
>>> Signed-off-by: Vincent Strubel <[email protected]>
>>> Reviewed-by: Philippe Trébuchet <[email protected]>
>>> Cc: Al Viro <[email protected]>
>>> Cc: Kees Cook <[email protected]>
>>> Cc: Mickaël Salaün <[email protected]>
>>
>> ...
>>
>>> 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).
On 12/12/2018 18:09, Jann Horn wrote:
> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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 <[email protected]>
>> Reviewed-by: Philippe Trébuchet <[email protected]>
>> Reviewed-by: Thibaut Sautereau <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Mickaël Salaün <[email protected]>
>> ---
> [...]
>> +/**
>> + * 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
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.
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.
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.
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.
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.
On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
<[email protected]> wrote:
> On 12/12/2018 18:09, Jann Horn wrote:
> > On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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.)
On 03/01/2019 12:17, Jann Horn wrote:
> On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
> <[email protected]> wrote:
>> On 12/12/2018 18:09, Jann Horn wrote:
>>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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).
On Tue, Jan 8, 2019 at 5:29 AM Mickaël Salaün
<[email protected]> wrote:
>
>
> On 03/01/2019 12:17, Jann Horn wrote:
> > On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
> > <[email protected]> wrote:
> >> On 12/12/2018 18:09, Jann Horn wrote:
> >>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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
On 09/01/2019 00:30, Kees Cook wrote:
> On Tue, Jan 8, 2019 at 5:29 AM Mickaël Salaün
> <[email protected]> wrote:
>>
>>
>> On 03/01/2019 12:17, Jann Horn wrote:
>>> On Thu, Dec 13, 2018 at 3:49 PM Mickaël Salaün
>>> <[email protected]> wrote:
>>>> On 12/12/2018 18:09, Jann Horn wrote:
>>>>> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <[email protected]> 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?
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 <[email protected]>
> > Signed-off-by: Thibaut Sautereau <[email protected]>
> > Signed-off-by: Vincent Strubel <[email protected]>
> > Reviewed-by: Philippe Trébuchet <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Mickaël Salaün <[email protected]>
>
> ...
>
> > 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
* 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
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
* 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
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 <[email protected]>
>>> Signed-off-by: Thibaut Sautereau <[email protected]>
>>> Signed-off-by: Vincent Strubel <[email protected]>
>>> Reviewed-by: Philippe Trébuchet <[email protected]>
>>> Cc: Al Viro <[email protected]>
>>> Cc: Kees Cook <[email protected]>
>>> Cc: Mickaël Salaün <[email protected]>
>>
>> ...
>>
>>> 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/[email protected]/
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 [email protected]. 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: [email protected]. If you have received this message in error, we thank you for informing the sender and destroying the message.
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 [email protected]. 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: [email protected]. If you have received this message in error, we thank you for informing the sender and destroying the message.
On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <[email protected]> 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 <[email protected]>
> > Signed-off-by: Thibaut Sautereau <[email protected]>
> > Signed-off-by: Vincent Strubel <[email protected]>
> > Reviewed-by: Philippe Trébuchet <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Mickaël Salaün <[email protected]>
>
> ...
>
> > 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
On 05/08/2019 01:55, Andy Lutomirski wrote:
> On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <[email protected]> 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 <[email protected]>
>>> Signed-off-by: Thibaut Sautereau <[email protected]>
>>> Signed-off-by: Vincent Strubel <[email protected]>
>>> Reviewed-by: Philippe Trébuchet <[email protected]>
>>> Cc: Al Viro <[email protected]>
>>> Cc: Kees Cook <[email protected]>
>>> Cc: Mickaël Salaün <[email protected]>
>>
>> ...
>>
>>> 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)?