Ensure that direct file execution (e.g. ./script.sh) and indirect file execution (e.g. sh script.sh) lead to the same result to support script protection.
Gu Bowen (1): IMA support bprm_creds_for_exec
Kees Cook (1): exec: Check __FMODE_EXEC instead of in_execve for LSMs
Linus Torvalds (1): execve: open the executable file before doing anything else
Mickaël Salaün (1): exec: Add a new AT_CHECK flag to execveat(2)
fs/exec.c | 87 ++++++++++++++++++------------- include/linux/binfmts.h | 7 ++- include/linux/ima.h | 1 + include/uapi/linux/fcntl.h | 31 +++++++++++ kernel/audit.h | 1 + kernel/auditsc.c | 1 + security/apparmor/lsm.c | 4 +- security/integrity/ima/ima_main.c | 11 ++++ security/security.c | 17 +++++- security/tomoyo/tomoyo.c | 3 +- 10 files changed, 123 insertions(+), 40 deletions(-)
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/12726 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/L...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/12726 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/L...
From: Linus Torvalds torvalds@linux-foundation.org
mainline inclusion from mainline-v6.8-rc1 commit 978ffcbf00d82b03b79e64b5c8249589b50e7463 category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IAZ996 CVE: NA
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
No point in allocating a new mm, counting arguments and environment variables etc if we're just going to return ENOENT.
This patch does expose the fact that 'do_filp_open()' that execve() uses is still unnecessarily expensive in the failure case, because it allocates the 'struct file *' early, even if the path lookup (which is heavily optimized) fails.
So that remains an unnecessary cost in the "no such executable" case, but it's a separate issue. Regardless, I do not want to do _both_ a filename_lookup() and a later do_filp_open() like the origin patch by Josh Triplett did in [1].
Reported-by: Josh Triplett josh@joshtriplett.org Cc: Kees Cook keescook@chromium.org Cc: Mateusz Guzik mjguzik@gmail.com Cc: Al Viro viro@zeniv.linux.org.uk Link: https://lore.kernel.org/lkml/5c7333ea4bec2fad1b47a8fa2db7c31e4ffc4f14.166333... [1] Link: https://lore.kernel.org/lkml/202209161637.9EDAF6B18@keescook/ Link: https://lore.kernel.org/lkml/CAHk-=wgznerM-xs+x+krDfE7eVBiy_HOam35rbsFMMOwvY... Link: https://lore.kernel.org/lkml/CAHk-=whf9qLO8ipps4QhmS0BkM8mtWJhvnuDSdtw5gFjhz... Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Gu Bowen gubowen5@huawei.com --- fs/exec.c | 69 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c index a4725461a443..c77504e1dadf 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1493,12 +1493,24 @@ static void free_bprm(struct linux_binprm *bprm) kfree(bprm); }
-static struct linux_binprm *alloc_bprm(int fd, struct filename *filename) +static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags) { - struct linux_binprm *bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); + struct linux_binprm *bprm; + struct file *file; int retval = -ENOMEM; - if (!bprm) - goto out; + + file = do_open_execat(fd, filename, flags); + if (IS_ERR(file)) + return ERR_CAST(file); + + bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); + if (!bprm) { + allow_write_access(file); + fput(file); + return ERR_PTR(-ENOMEM); + } + + bprm->file = file;
if (fd == AT_FDCWD || filename->name[0] == '/') { bprm->filename = filename->name; @@ -1511,18 +1523,28 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename) if (!bprm->fdpath) goto out_free;
+ /* + * Record that a name derived from an O_CLOEXEC fd will be + * inaccessible after exec. This allows the code in exec to + * choose to fail when the executable is not mmaped into the + * interpreter and an open file descriptor is not passed to + * the interpreter. This makes for a better user experience + * than having the interpreter start and then immediately fail + * when it finds the executable is inaccessible. + */ + if (get_close_on_exec(fd)) + bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE; + bprm->filename = bprm->fdpath; } bprm->interp = bprm->filename;
retval = bprm_mm_init(bprm); - if (retval) - goto out_free; - return bprm; + if (!retval) + return bprm;
out_free: free_bprm(bprm); -out: return ERR_PTR(retval); }
@@ -1799,10 +1821,8 @@ static int exec_binprm(struct linux_binprm *bprm) /* * sys_execve() executes a new program. */ -static int bprm_execve(struct linux_binprm *bprm, - int fd, struct filename *filename, int flags) +static int bprm_execve(struct linux_binprm *bprm) { - struct file *file; int retval;
retval = prepare_bprm_creds(bprm); @@ -1818,26 +1838,8 @@ static int bprm_execve(struct linux_binprm *bprm, current->in_execve = 1; sched_mm_cid_before_execve(current);
- file = do_open_execat(fd, filename, flags); - retval = PTR_ERR(file); - if (IS_ERR(file)) - goto out_unmark; - sched_exec();
- bprm->file = file; - /* - * Record that a name derived from an O_CLOEXEC fd will be - * inaccessible after exec. This allows the code in exec to - * choose to fail when the executable is not mmaped into the - * interpreter and an open file descriptor is not passed to - * the interpreter. This makes for a better user experience - * than having the interpreter start and then immediately fail - * when it finds the executable is inaccessible. - */ - if (bprm->fdpath && get_close_on_exec(fd)) - bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE; - /* Set the unchanging part of bprm->cred */ retval = security_bprm_creds_for_exec(bprm); if (retval) @@ -1867,7 +1869,6 @@ static int bprm_execve(struct linux_binprm *bprm, if (bprm->point_of_no_return && !fatal_signal_pending(current)) force_fatal_sig(SIGSEGV);
-out_unmark: sched_mm_cid_after_execve(current); current->fs->in_exec = 0; current->in_execve = 0; @@ -1902,7 +1903,7 @@ static int do_execveat_common(int fd, struct filename *filename, * further execve() calls fail. */ current->flags &= ~PF_NPROC_EXCEEDED;
- bprm = alloc_bprm(fd, filename); + bprm = alloc_bprm(fd, filename, flags); if (IS_ERR(bprm)) { retval = PTR_ERR(bprm); goto out_ret; @@ -1951,7 +1952,7 @@ static int do_execveat_common(int fd, struct filename *filename, bprm->argc = 1; }
- retval = bprm_execve(bprm, fd, filename, flags); + retval = bprm_execve(bprm); out_free: free_bprm(bprm);
@@ -1976,7 +1977,7 @@ int kernel_execve(const char *kernel_filename, if (IS_ERR(filename)) return PTR_ERR(filename);
- bprm = alloc_bprm(fd, filename); + bprm = alloc_bprm(fd, filename, 0); if (IS_ERR(bprm)) { retval = PTR_ERR(bprm); goto out_ret; @@ -2011,7 +2012,7 @@ int kernel_execve(const char *kernel_filename, if (retval < 0) goto out_free;
- retval = bprm_execve(bprm, fd, filename, 0); + retval = bprm_execve(bprm); out_free: free_bprm(bprm); out_ret:
From: Kees Cook keescook@chromium.org
mainline inclusion from mainline-v6.8-rc2 commit 4759ff71f23e1a9cba001009abab68cde6dc327a category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IAZ996 CVE: NA
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
After commit 978ffcbf00d8 ("execve: open the executable file before doing anything else"), current->in_execve was no longer in sync with the open(). This broke AppArmor and TOMOYO which depend on this flag to distinguish "open" operations from being "exec" operations.
Instead of moving around in_execve, switch to using __FMODE_EXEC, which is where the "is this an exec?" intent is stored. Note that TOMOYO still uses in_execve around cred handling.
Reported-by: Kevin Locke kevin@kevinlocke.name Closes: https://lore.kernel.org/all/ZbE4qn9_h14OqADK@kevinlocke.name Suggested-by: Linus Torvalds torvalds@linux-foundation.org Fixes: 978ffcbf00d8 ("execve: open the executable file before doing anything else") Cc: Josh Triplett josh@joshtriplett.org Cc: John Johansen john.johansen@canonical.com Cc: Paul Moore paul@paul-moore.com Cc: James Morris jmorris@namei.org Cc: Serge E. Hallyn serge@hallyn.com Cc: Kentaro Takeda takedakn@nttdata.co.jp Cc: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: Jan Kara jack@suse.cz Cc: Eric Biederman ebiederm@xmission.com Cc: Andrew Morton akpm@linux-foundation.org Cc: Sebastian Andrzej Siewior bigeasy@linutronix.de Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Cc: apparmor@lists.ubuntu.com Cc: linux-security-module@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Gu Bowen gubowen5@huawei.com --- security/apparmor/lsm.c | 4 +++- security/tomoyo/tomoyo.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 5303a51eff9c..56cd03fa8086 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -461,8 +461,10 @@ static int apparmor_file_open(struct file *file) * Cache permissions granted by the previous exec check, with * implicit read and executable mmap which are required to * actually execute the image. + * + * Illogically, FMODE_EXEC is in f_flags, not f_mode. */ - if (current->in_execve) { + if (file->f_flags & __FMODE_EXEC) { fctx->allow = MAY_EXEC | MAY_READ | AA_EXEC_MMAP; return 0; } diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index 298d182759c2..db1fefbd66de 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -327,7 +327,8 @@ static int tomoyo_file_fcntl(struct file *file, unsigned int cmd, static int tomoyo_file_open(struct file *f) { /* Don't check read permission here if called from execve(). */ - if (current->in_execve) + /* Illogically, FMODE_EXEC is in f_flags, not f_mode. */ + if (f->f_flags & __FMODE_EXEC) return 0; return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path, f->f_flags);
From: Mickaël Salaün mic@digikod.net
maillist inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IAZ996 CVE: NA
Reference: https://lore.kernel.org/all/20241011184422.977903-2-mic@digikod.net/
--------------------------------
Add a new AT_CHECK flag to execveat(2) to check if a file would be allowed for execution. The main use case is for script interpreters and dynamic linkers to check execution permission according to the kernel's security policy. Another use case is to add context to access logs e.g., which script (instead of interpreter) accessed a file. As any executable code, scripts could also use this check [1].
This is different from faccessat(2) + X_OK which only checks a subset of access rights (i.e. inode permission and mount options for regular files), but not the full context (e.g. all LSM access checks). The main use case for access(2) is for SUID processes to (partially) check access on behalf of their caller. The main use case for execveat(2) + AT_CHECK is to check if a script execution would be allowed, according to all the different restrictions in place. Because the use of AT_CHECK follows the exact kernel semantic as for a real execution, user space gets the same error codes.
An interesting point of using execveat(2) instead of openat2(2) is that it decouples the check from the enforcement. Indeed, the security check can be logged (e.g. with audit) without blocking an execution environment not yet ready to enforce a strict security policy.
LSMs can control or log execution requests with security_bprm_creds_for_exec(). However, to enforce a consistent and complete access control (e.g. on binary's dependencies) LSMs should restrict file executability, or mesure executed files, with security_file_open() by checking file->f_flags & __FMODE_EXEC.
Because AT_CHECK is dedicated to user space interpreters, it doesn't make sense for the kernel to parse the checked files, look for interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC if the format is unknown. Because of that, security_bprm_check() is never called when AT_CHECK is used.
It should be noted that script interpreters cannot directly use execveat(2) (without this new AT_CHECK flag) because this could lead to unexpected behaviors e.g., `python script.sh` could lead to Bash being executed to interpret the script. Unlike the kernel, script interpreters may just interpret the shebang as a simple comment, which should not change for backward compatibility reasons.
Because scripts or libraries files might not currently have the executable permission set, or because we might want specific users to be allowed to run arbitrary scripts, the following patch provides a dynamic configuration mechanism with the SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE securebits.
This is a redesign of the CLIP OS 4's O_MAYEXEC: https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b6... This patch has been used for more than a decade with customized script interpreters. Some examples can be found here: https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
Cc: Al Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: Kees Cook keescook@chromium.org Cc: Paul Moore paul@paul-moore.com Cc: Serge Hallyn serge@hallyn.com Link: https://docs.python.org/3/library/io.html#io.open_code [1] Signed-off-by: Mickaël Salaün mic@digikod.net Link: https://lore.kernel.org/r/20241011184422.977903-2-mic@digikod.net Conflicts: include/uapi/linux/fcntl.h [Context conflicts] Signed-off-by: Gu Bowen gubowen5@huawei.com --- fs/exec.c | 18 ++++++++++++++++-- include/linux/binfmts.h | 7 ++++++- include/uapi/linux/fcntl.h | 31 +++++++++++++++++++++++++++++++ kernel/audit.h | 1 + kernel/auditsc.c | 1 + security/security.c | 10 ++++++++++ 6 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c index c77504e1dadf..67272e29a0b2 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -905,7 +905,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) .lookup_flags = LOOKUP_FOLLOW, };
- if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0) + if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0) return ERR_PTR(-EINVAL); if (flags & AT_SYMLINK_NOFOLLOW) open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW; @@ -1539,6 +1539,20 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl } bprm->interp = bprm->filename;
+ /* + * At this point, security_file_open() has already been called (with + * __FMODE_EXEC) and access control checks for AT_CHECK will stop just + * after the security_bprm_creds_for_exec() call in bprm_execve(). + * Indeed, the kernel should not try to parse the content of the file + * with exec_binprm() nor change the calling thread, which means that + * the following security functions will be not called: + * - security_bprm_check() + * - security_bprm_creds_from_file() + * - security_bprm_committing_creds() + * - security_bprm_committed_creds() + */ + bprm->is_check = !!(flags & AT_CHECK); + retval = bprm_mm_init(bprm); if (!retval) return bprm; @@ -1842,7 +1856,7 @@ static int bprm_execve(struct linux_binprm *bprm)
/* Set the unchanging part of bprm->cred */ retval = security_bprm_creds_for_exec(bprm); - if (retval) + if (retval || bprm->is_check) goto out;
retval = exec_binprm(bprm); diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 8d51f69f9f5e..330b194b66e7 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -42,7 +42,12 @@ struct linux_binprm { * Set when errors can no longer be returned to the * original userspace. */ - point_of_no_return:1; + point_of_no_return:1, + /* + * Set by user space to check executability according to the + * caller's environment. + */ + is_check:1; struct file *executable; /* Executable to pass to the interpreter */ struct file *interpreter; struct file *file; diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h index 282e90aeb163..f368112f343b 100644 --- a/include/uapi/linux/fcntl.h +++ b/include/uapi/linux/fcntl.h @@ -116,6 +116,37 @@ #define AT_HANDLE_FID AT_REMOVEDIR /* file handle is needed to compare object identity and may not be usable to open_by_handle_at(2) */ +/* + * AT_CHECK only performs a check on a regular file and returns 0 if execution + * of this file would be allowed, ignoring the file format and then the related + * interpreter dependencies (e.g. ELF libraries, script's shebang). + * + * Programs should always perform this check to apply kernel-level checks + * against files that are not directly executed by the kernel but passed to a + * user space interpreter instead. All files that contain executable code, + * from the point of view of the interpreter, should be checked. However the + * result of this check should only be enforced according to + * SECBIT_EXEC_RESTRICT_FILE or SECBIT_EXEC_DENY_INTERACTIVE. See securebits.h + * documentation and the samples/check-exec/inc.c example. + * + * The main purpose of this flag is to improve the security and consistency of + * an execution environment to ensure that direct file execution (e.g. + * `./script.sh`) and indirect file execution (e.g. `sh script.sh`) lead to the + * same result. For instance, this can be used to check if a file is + * trustworthy according to the caller's environment. + * + * In a secure environment, libraries and any executable dependencies should + * also be checked. For instance, dynamic linking should make sure that all + * libraries are allowed for execution to avoid trivial bypass (e.g. using + * LD_PRELOAD). For such secure execution environment to make sense, only + * trusted code should be executable, which also requires integrity guarantees. + * + * To avoid race conditions leading to time-of-check to time-of-use issues, + * AT_CHECK should be used with AT_EMPTY_PATH to check against a file + * descriptor instead of a path. + */ +#define AT_CHECK 0x10000 + #if defined(__KERNEL__) #define AT_GETATTR_NOSEC 0x80000000 #endif diff --git a/kernel/audit.h b/kernel/audit.h index 457ca207e08a..798b8d4e3589 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -198,6 +198,7 @@ struct audit_context { struct open_how openat2; struct { int argc; + bool is_check; } execve; struct { char *name; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 6f0d6fb6523f..b6316e284342 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2662,6 +2662,7 @@ void __audit_bprm(struct linux_binprm *bprm)
context->type = AUDIT_EXECVE; context->execve.argc = bprm->argc; + context->execve.is_check = bprm->is_check; }
diff --git a/security/security.c b/security/security.c index 3aa2d295464e..df5977b9ac9a 100644 --- a/security/security.c +++ b/security/security.c @@ -1053,6 +1053,12 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) * to 1 if AT_SECURE should be set to request libc enable secure mode. @bprm * contains the linux_binprm structure. * + * If execveat(2) is called with the AT_CHECK flag, bprm->is_check is set. The + * result must be the same as without this flag even if the execution will + * never really happen and @bprm will always be dropped. + * + * This hook must not change current->cred, only @bprm->cred. + * * Return: Returns 0 if the hook is successful and permission is granted. */ int security_bprm_creds_for_exec(struct linux_binprm *bprm) @@ -2845,6 +2851,10 @@ int security_file_receive(struct file *file) * Save open-time permission checking state for later use upon file_permission, * and recheck access if anything has changed since inode_permission. * + * We can check if a file is opened for execution (e.g. execve(2) call), either + * directly or indirectly (e.g. ELF's ld.so) by checking file->f_flags & + * __FMODE_EXEC . + * * Return: Returns 0 if permission is granted. */ int security_file_open(struct file *file)
hulk inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IAZ996 CVE: NA
Reference: https://lore.kernel.org/linux-integrity/9e3df65c2bf060b5833558e9f8d82dcd2fe9...
----------------------------------------------------------------------
Support ima measure and appraise the indirect script calls. If script exec check is enabled, it will call ima_bprm_check() through security_bprm_creds_for_exec().
Signed-off-by: Huaxin Lu luhuaxin1@huawei.com Signed-off-by: Gu Bowen gubowen5@huawei.com --- include/linux/ima.h | 1 + security/integrity/ima/ima_main.c | 11 +++++++++++ security/security.c | 7 ++++++- 3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h index 86b57757c7b1..76d0f71d7955 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -17,6 +17,7 @@ struct linux_binprm; #ifdef CONFIG_IMA extern enum hash_algo ima_get_current_hash_algo(void); extern int ima_bprm_check(struct linux_binprm *bprm); +extern int ima_bprm_creds_for_exec(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask); extern void ima_post_create_tmpfile(struct mnt_idmap *idmap, struct inode *inode); diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 5358ca2d8aaf..b167b649ba31 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -638,6 +638,17 @@ int ima_bprm_check(struct linux_binprm *bprm) MAY_EXEC, CREDS_CHECK); }
+/** + * ima_bprm_creds_for_exec - ima support exec check. + */ +int ima_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + if (!bprm->is_check) + return 0; + + return ima_bprm_check(bprm); +} + /** * ima_file_check - based on policy, collect/store measurement. * @file: pointer to the file to be measured diff --git a/security/security.c b/security/security.c index df5977b9ac9a..603e6260d63d 100644 --- a/security/security.c +++ b/security/security.c @@ -1063,7 +1063,12 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) */ int security_bprm_creds_for_exec(struct linux_binprm *bprm) { - return call_int_hook(bprm_creds_for_exec, 0, bprm); + int ret; + + ret = call_int_hook(bprm_creds_for_exec, 0, bprm); + if (ret) + return ret; + return ima_bprm_creds_for_exec(bprm); }
/**