[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <79e4d55f-a80b-5361-e18d-d8fa67ae640b@digikod.net>
Date: Thu, 13 Aug 2020 16:49:11 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: linux-kernel@...r.kernel.org, Aleksa Sarai <cyphar@...har.com>,
Alexei Starovoitov <ast@...nel.org>, Al Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>, Andy Lutomirski
<luto@...nel.org>, Christian Brauner <christian.brauner@...ntu.com>,
Christian Heimes <christian@...hon.org>,
Daniel Borkmann <daniel@...earbox.net>,
Deven Bowers <deven.desai@...ux.microsoft.com>,
Dmitry Vyukov <dvyukov@...gle.com>, Eric Biggers <ebiggers@...nel.org>,
Eric Chiang <ericchiang@...gle.com>, Florian Weimer <fweimer@...hat.com>,
James Morris <jmorris@...ei.org>, Jan Kara <jack@...e.cz>,
Jann Horn <jannh@...gle.com>, Jonathan Corbet <corbet@....net>,
Kees Cook <keescook@...omium.org>,
Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
Matthew Garrett <mjg59@...gle.com>, Matthew Wilcox <willy@...radead.org>,
Michael Kerrisk <mtk.manpages@...il.com>, Mimi Zohar <zohar@...ux.ibm.com>,
Philippe Trébuchet <philippe.trebuchet@....gouv.fr>,
Scott Shell <scottsh@...rosoft.com>,
Sean Christopherson <sean.j.christopherson@...el.com>,
Shuah Khan <shuah@...nel.org>, Steve Dower <steve.dower@...hon.org>,
Steve Grubb <sgrubb@...hat.com>,
Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
Thibaut Sautereau <thibaut.sautereau@...p-os.org>,
Vincent Strubel <vincent.strubel@....gouv.fr>,
kernel-hardening@...ts.openwall.com, linux-api@...r.kernel.org,
linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-fsdevel@...r.kernel.org,
Thibaut Sautereau <thibaut.sautereau@....gouv.fr>,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v7 5/7] fs,doc: Enable to enforce noexec mounts or file
exec through O_MAYEXEC
On 11/08/2020 21:58, Eric W. Biederman wrote:
> Mickaël Salaün <mic@...ikod.net> writes:
>
>> Allow for the enforcement of the O_MAYEXEC openat2(2) flag. Thanks to
>> the noexec option from the underlying VFS mount, or to the file execute
>> permission, userspace can enforce these execution policies. This may
>> allow script interpreters to check execution permission before reading
>> commands from a file, or dynamic linkers to allow shared object
>> loading.
>
> Ick!!!!!
>
> This feels like being so open minded your brains fall out.
Really? :)
>
> I can see having a sysctl that allows the new open flag to be ignored
> so that the existing lack of enforcement when the flag is passed
> continues.
And that could break distros (i.e. multiple interpreters, with or
without O_MAYEXEC, different versions, and different kernels).
>
> But having the sysctl be fine grained seems like way too much rope.
This follow the same rational: file permissions and mount options can
change but they are controled by the sysadmin, how also configure sysctl
values.
>
> I don't think the code needs to do more than enforce or not enforce this
> logic.
I think this is the more viable behavior for an eclectic set of distros
(and different use cases).
>
> You can test the sysctl once when you process O_MAYEXEC. But code such
> as may_open should not have the conditional behavior. It should get an
> appropriate set of flags that are always enforced. With the madness of
> what to do left at the edge of userspace.
The problem is that this userspace is not in charge of the system-wide
policy, the sysadmin is. As pointed out by the commit message, please
take a look at the rational:
https://lore.kernel.org/lkml/1477d3d7-4b36-afad-7077-a38f42322238@digikod.net/
>
> Anything else appears to be madness, overengineering, and a failure to
> separate concerns.
Oh! What a conclusion! :)
I'd say it is a pragmatic approach.
>
> Eric
>
>
>> Add a new sysctl fs.open_mayexec_enforce to enable system administrators
>> to enforce two complementary security policies according to the
>> installed system: enforce the noexec mount option, and enforce
>> executable file permission. Indeed, because of compatibility with
>> installed systems, only system administrators are able to check that
>> this new enforcement is in line with the system mount points and file
>> permissions. A following patch adds documentation.
>>
>> Being able to restrict execution also enables 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). To get a
>> consistent execution policy, additional memory restrictions should also
>> be enforced (e.g. thanks to SELinux).
>>
>> Because the O_MAYEXEC flag is a meant to enforce a system-wide security
>> policy (but not application-centric policies), it does not make sense
>> for userland to check the sysctl value. Indeed, this new flag only
>> enables to extend the system ability to enforce a policy thanks to (some
>> trusted) userland collaboration. Moreover, additional security policies
>> could be managed by LSMs. This is a best-effort approach from the
>> application developer point of view:
>> https://lore.kernel.org/lkml/1477d3d7-4b36-afad-7077-a38f42322238@digikod.net/
>>
>> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
>> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@....gouv.fr>
>> Cc: Aleksa Sarai <cyphar@...har.com>
>> Cc: Al Viro <viro@...iv.linux.org.uk>
>> Cc: Jonathan Corbet <corbet@....net>
>> Cc: Kees Cook <keescook@...omium.org>
>> Cc: Randy Dunlap <rdunlap@...radead.org>
>> ---
>>
>> Changes since v6:
>> * Allow opening pipes, block devices and character devices with
>> O_MAYEXEC when there is no enforced policy, but forbid any non-regular
>> file opened with O_MAYEXEC otherwise (i.e. for any enforced policy).
>> * Add a paragraph about the non-regular files policy.
>> * Move path_noexec() calls out of the fast-path (suggested by Kees
>> Cook).
>>
>> Changes since v5:
>> * Remove the static enforcement configuration through Kconfig because it
>> makes the code more simple like this, and because the current sysctl
>> configuration can only be set with CAP_SYS_ADMIN, the same way mount
>> options (i.e. noexec) can be set. If an harden distro wants to
>> enforce a configuration, it should restrict capabilities or sysctl
>> configuration. Furthermore, an LSM can easily leverage O_MAYEXEC to
>> fit its need.
>> * Move checks from inode_permission() to may_open() and make the error
>> codes more consistent according to file types (in line with a previous
>> commit): opening a directory with O_MAYEXEC returns EISDIR and other
>> non-regular file types may return EACCES.
>> * In may_open(), when OMAYEXEC_ENFORCE_FILE is set, replace explicit
>> call to generic_permission() with an artificial MAY_EXEC to avoid
>> double calls. This makes sense especially when an LSM policy forbids
>> execution of a file.
>> * Replace the custom proc_omayexec() with
>> proc_dointvec_minmax_sysadmin(), and then replace the CAP_MAC_ADMIN
>> check with a CAP_SYS_ADMIN one (suggested by Kees Cook and Stephen
>> Smalley).
>> * Use BIT() (suggested by Kees Cook).
>> * Rename variables (suggested by Kees Cook).
>> * Reword the kconfig help.
>> * Import the documentation patch (suggested by Kees Cook):
>> https://lore.kernel.org/lkml/20200505153156.925111-6-mic@digikod.net/
>> * Update documentation and add LWN.net article.
>>
>> Changes since v4:
>> * Add kernel configuration options to enforce O_MAYEXEC at build time,
>> and disable the sysctl in such case (requested by James Morris).
>> * Reword commit message.
>>
>> Changes since v3:
>> * Update comment with O_MAYEXEC.
>>
>> Changes since v2:
>> * Cosmetic changes.
>>
>> Changes since v1:
>> * Move code from Yama to the FS subsystem (suggested by Kees Cook).
>> * Make omayexec_inode_permission() static (suggested by Jann Horn).
>> * Use mode 0600 for the sysctl.
>> * Only match regular files (not directories nor other types), which
>> follows the same semantic as commit 73601ea5b7b1 ("fs/open.c: allow
>> opening only regular files during execve()").
>> ---
>> Documentation/admin-guide/sysctl/fs.rst | 49 +++++++++++++++++++++++++
>> fs/namei.c | 24 ++++++++++++
>> include/linux/fs.h | 1 +
>> kernel/sysctl.c | 12 +++++-
>> 4 files changed, 84 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst
>> index 2a45119e3331..ce6e2081d3a9 100644
>> --- a/Documentation/admin-guide/sysctl/fs.rst
>> +++ b/Documentation/admin-guide/sysctl/fs.rst
>> @@ -37,6 +37,7 @@ Currently, these files are in /proc/sys/fs:
>> - inode-nr
>> - inode-state
>> - nr_open
>> +- open_mayexec_enforce
>> - overflowuid
>> - overflowgid
>> - pipe-user-pages-hard
>> @@ -165,6 +166,54 @@ system needs to prune the inode list instead of allocating
>> more.
>>
>>
>> +open_mayexec_enforce
>> +--------------------
>> +
>> +While being ignored by :manpage:`open(2)` and :manpage:`openat(2)`, the
>> +``O_MAYEXEC`` flag can be passed to :manpage:`openat2(2)` to only open regular
>> +files that are expected to be 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,
>> +or a dynamic linker to only load executable shared objects. One interesting
>> +use case is to enforce a "write xor execute" policy through interpreters.
>> +
>> +The ability to restrict code execution must be thought as a system-wide policy,
>> +which first starts by restricting mount points with the ``noexec`` option.
>> +This option is also automatically applied to special filesystems such as /proc .
>> +This prevents files on such mount points to be directly executed by the kernel
>> +or mapped as executable memory (e.g. libraries). With script interpreters
>> +using the ``O_MAYEXEC`` flag, the executable permission can then be checked
>> +before reading commands from files. This makes it possible to enforce the
>> +``noexec`` at the interpreter level, and thus propagates this security policy
>> +to scripts. To be fully effective, these interpreters also need to handle the
>> +other ways to execute code: command line parameters (e.g., option ``-e`` for
>> +Perl), module loading (e.g., option ``-m`` for Python), stdin, file sourcing,
>> +environment variables, configuration files, etc. 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.
>> +
>> +There are two complementary security policies: enforce the ``noexec`` mount
>> +option, and enforce executable file permission. These policies are handled by
>> +the ``fs.open_mayexec_enforce`` sysctl (writable only with ``CAP_SYS_ADMIN``)
>> +as a bitmask:
>> +
>> +1 - Mount restriction: checks that the mount options for the underlying VFS
>> + mount do not prevent execution.
>> +
>> +2 - File permission restriction: checks that the to-be-opened file is marked as
>> + executable for the current process (e.g., POSIX permissions).
>> +
>> +Note that as long as a policy is enforced, opening any non-regular file with
>> +``O_MAYEXEC`` is denied (e.g. TTYs, pipe), even when such a file is marked as
>> +executable or is on an executable mount point.
>> +
>> +Code samples can be found in tools/testing/selftests/openat2/omayexec_test.c
>> +and interpreter patches (for the original O_MAYEXEC version) may be found at
>> +https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC .
>> +See also an overview article: https://lwn.net/Articles/820000/ .
>> +
>> +
>> overflowgid & overflowuid
>> -------------------------
>>
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 3f074ec77390..8ec13c7fd403 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -39,6 +39,7 @@
>> #include <linux/bitops.h>
>> #include <linux/init_task.h>
>> #include <linux/uaccess.h>
>> +#include <linux/sysctl.h>
>>
>> #include "internal.h"
>> #include "mount.h"
>> @@ -425,6 +426,11 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>> return 0;
>> }
>>
>> +#define OPEN_MAYEXEC_ENFORCE_MOUNT BIT(0)
>> +#define OPEN_MAYEXEC_ENFORCE_FILE BIT(1)
>> +
>> +int sysctl_open_mayexec_enforce __read_mostly;
>> +
>> /**
>> * inode_permission - Check for access rights to a given inode
>> * @inode: Inode to check permission on
>> @@ -2861,11 +2867,29 @@ static int may_open(const struct path *path, int acc_mode, int flag)
>> case S_IFSOCK:
>> if (acc_mode & MAY_EXEC)
>> return -EACCES;
>> + /*
>> + * Opening devices (e.g. TTYs) or pipes with O_MAYEXEC may be
>> + * legitimate when there is no enforced policy.
>> + */
>> + if ((acc_mode & MAY_OPENEXEC) && sysctl_open_mayexec_enforce)
>> + return -EACCES;
>> flag &= ~O_TRUNC;
>> break;
>> case S_IFREG:
>> if ((acc_mode & MAY_EXEC) && path_noexec(path))
>> return -EACCES;
>> + if (acc_mode & MAY_OPENEXEC) {
>> + if ((sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_MOUNT)
>> + && path_noexec(path))
>> + return -EACCES;
>> + if (sysctl_open_mayexec_enforce & OPEN_MAYEXEC_ENFORCE_FILE)
>> + /*
>> + * Because acc_mode may change here, the next and only
>> + * use of acc_mode should then be by the following call
>> + * to inode_permission().
>> + */
>> + acc_mode |= MAY_EXEC;
>> + }
>> break;
>> }
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 56f835c9a87a..071f37707ccc 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -83,6 +83,7 @@ extern int sysctl_protected_symlinks;
>> extern int sysctl_protected_hardlinks;
>> extern int sysctl_protected_fifos;
>> extern int sysctl_protected_regular;
>> +extern int sysctl_open_mayexec_enforce;
>>
>> typedef __kernel_rwf_t rwf_t;
>>
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index db1ce7af2563..5008a2566e79 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -113,6 +113,7 @@ static int sixty = 60;
>>
>> static int __maybe_unused neg_one = -1;
>> static int __maybe_unused two = 2;
>> +static int __maybe_unused three = 3;
>> static int __maybe_unused four = 4;
>> static unsigned long zero_ul;
>> static unsigned long one_ul = 1;
>> @@ -888,7 +889,6 @@ static int proc_taint(struct ctl_table *table, int write,
>> return err;
>> }
>>
>> -#ifdef CONFIG_PRINTK
>> static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>> void *buffer, size_t *lenp, loff_t *ppos)
>> {
>> @@ -897,7 +897,6 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>>
>> return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>> }
>> -#endif
>>
>> /**
>> * struct do_proc_dointvec_minmax_conv_param - proc_dointvec_minmax() range checking structure
>> @@ -3264,6 +3263,15 @@ static struct ctl_table fs_table[] = {
>> .extra1 = SYSCTL_ZERO,
>> .extra2 = &two,
>> },
>> + {
>> + .procname = "open_mayexec_enforce",
>> + .data = &sysctl_open_mayexec_enforce,
>> + .maxlen = sizeof(int),
>> + .mode = 0600,
>> + .proc_handler = proc_dointvec_minmax_sysadmin,
>> + .extra1 = SYSCTL_ZERO,
>> + .extra2 = &three,
>> + },
>> #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE)
>> {
>> .procname = "binfmt_misc",
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
