* [PATCH 0/3] Taming execve, setuid, and LSMs @ 2010-03-26 13:38 Andy Lutomirski 2010-03-26 13:38 ` [PATCH 1/3] Add the execve_nosecurity syscall Andy Lutomirski ` (3 more replies) 0 siblings, 4 replies; 21+ messages in thread From: Andy Lutomirski @ 2010-03-26 13:38 UTC (permalink / raw) To: linux-kernel, linux-security-module Cc: Eric Biederman, Andrew G. Morgan, Andy Lutomirski Every now and then, someone wants to let unprivileged programs change something about their execution environment (think unsharing namespaces, changing capabilities, disabling networking, chrooting, mounting and unmounting filesystems). Whether or not any of these abilities are good ideas, there's a recurring problem that gets most of these patches shot down: setuid executables. The obvious solution is to allow a process to opt out of setuid semantics and require processes to do this before using these shiny new features. [1] [2] But there's a problem with this, too: with LSMs running, execve can do pretty much anything, and even unprivileged users running unprivileged programs can have crazy security implications. (Take a look at a default install of Fedora. If you can understand the security implications of disabling setuid, you get a cookie. If you can figure out which programs will result in a change of security label when exec'd, you get another cookie.) So here's another solution, based on the idea that in a sane world, execve should be a lot less magical than it is. Any unprivileged program can open an executable, parse its headers, map it, and run it, although getting all the details right is tedious at best (and there's no good way to get all of the threading semantics right from userspace). Patch 1 adds a new syscall execve_nosecurity. It does an exec, but without changing any security properties. This means no setuid, no setgid, no LSM credential hooks (e.g. no SELinux type transitions), and no ptrace restrictions. (You have to have read access to the program, because disabling security stuff could allow someone to ptrace a program that they couldn't otherwise ptrace.) This shouldn't be particularly scary -- any process could do much the same thing with open and mmap. (You can easily shoot yourself in the foot with this syscall -- think LD_PRELOAD or running some program with insufficient error checking that can get subverted when run in the wrong security context. So don't do that.) Patch 2 adds a prctl that irrevocably disables execve. Making execve do something different that could confuse LSMs is dangerous. Turning the whole thing off shouldn't be. (Of course, with execve disabled, you can still use execve_nosecurity. But any program that does that should take precautions not to shoot itself in the foot.) (In a future revision, this should probably be a new syscall.) Sadly, programs that have opted out of execve might want to use subprocesses that in turn run execve. This will fail. So patch 3 (which is ugly, but I don't see anything fundamentally wrong with it) allows processes to set a flag that turns execve into execve_nosecurity. This flag survives exec. Of course, this could be used to subvert setuid programs, so you can't set this flag unless you disable ordinary exec first. [1] Unprivileged: http://lkml.org/lkml/2009/12/30/265 [2] securebit approach: http://lwn.net/Articles/368600/ Andy Lutomirski (3): Add the execve_nosecurity syscall. Add PR_RESTRICT_ME to disable security-sensitive features for a process tree. Add PR_SET_FORCE_EXECVE_NOSECURITY to turn execve calls into execve_nosecurity. arch/x86/ia32/sys_ia32.c | 2 +- arch/x86/include/asm/unistd_64.h | 2 + arch/x86/kernel/entry_64.S | 16 +++++ arch/x86/kernel/process.c | 28 ++++++++- fs/binfmt_elf.c | 6 +- fs/binfmt_elf_fdpic.c | 6 +- fs/binfmt_em86.c | 2 +- fs/binfmt_flat.c | 7 ++- fs/binfmt_misc.c | 2 +- fs/binfmt_script.c | 2 +- fs/compat.c | 14 ++++- fs/exec.c | 126 ++++++++++++++++++++++++------------- include/linux/binfmts.h | 3 + include/linux/compat.h | 2 +- include/linux/fs.h | 2 +- include/linux/prctl.h | 11 +++ include/linux/sched.h | 5 +- kernel/fork.c | 3 + kernel/sys.c | 42 +++++++++++++ 19 files changed, 221 insertions(+), 60 deletions(-) The code below can be used to play with these new features. /* gcc -I /lib/modules/2.6.33/build/usr/include no_exec.c */ #include <asm/unistd.h> #include <linux/prctl.h> #include <stdio.h> int execve_nosecurity(const char *filename, char *const argv[], char *const envp[]) { return syscall(__NR_execve_nosecurity, filename, argv, envp); } int main(int argc, char **argv, char **envp) { if (argc < 3) { fprintf(stderr, "Usage: %s <force | noforce> program args...\n"); return 1; } int force_execve_nosecurity; if (!strcmp(argv[1], "force")) { force_execve_nosecurity = 1; } else if (!strcmp(argv[1], "noforce")) { force_execve_nosecurity = 0; } else { fprintf(stderr, "Invalid argument\n"); return 1; } if (prctl(PR_RESTRICT_ME, PR_RESTRICT_EXEC, 0, 0, 0) != 0) { perror("PR_RESTRICT_ME"); return 1; } if (prctl(PR_SET_FORCE_EXECVE_NOSECURITY, force_execve_nosecurity, 0, 0, 0) != 0) { perror("PR_SET_FORCE_EXECVE_NOSECURITY"); return 1; } execve_nosecurity(argv[2], argv + 2, envp); perror("execve_nosecurity"); return 1; } ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] Add the execve_nosecurity syscall. 2010-03-26 13:38 [PATCH 0/3] Taming execve, setuid, and LSMs Andy Lutomirski @ 2010-03-26 13:38 ` Andy Lutomirski 2010-03-26 13:38 ` [PATCH 2/3] Add PR_RESTRICT_ME to disable security-sensitive features for a process tree Andy Lutomirski ` (2 subsequent siblings) 3 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2010-03-26 13:38 UTC (permalink / raw) To: linux-kernel, linux-security-module Cc: Eric Biederman, Andrew G. Morgan, Andy Lutomirski execve_nosecurity does exactly what execve does, except for three differences: 1. execve_nosecurity does not let LSMs change anything. Anything includes: credentials, labels, and securebits. This means that you probably want to do prctl(PR_SET_KEEPCAPS, 0, ...) before using it. 2. execve_nosecurity requires both read and exec access. 3. execve_nosecurity does not change dumpability. Here's why: execve looks at setuid/setgid bits, the nosuid mount flag, and whatever else the LSM wants it to look at, all of which can change the security context of the process. To keep the system secure, this needs to interact with coredumps and ptrace. execve_nosecurity is a new syscall that does the same thing as execve but without security implications: it does not change credentials, security labels, ptrace status, or dumpability. Because admins might expect executable but non-readable programs not to leak their contents due to either setuid/setgid bits or LSM labels, execve_nosecurity requires both read and execute access. In principle (even though the mechanisms aren't all there), programs ought to be able to do what execve_nosecurity does without help from the kernel. Some LSMs (in particular commoncap) might take certain actions on every exec independently of preexisting credentials or labels and independently of the file's label. This is why SECURE_KEEP_CAPS gets cleared on every exec. It's a special case because SECURE_KEEP_CAPS is a strange API that's specifically meant to change the behavior of the very next exec call, and it has no effect when execve_nosecurity is called. Since I didn't want to add a brand new LSM hook just for execve_nosecurity (that hook shouldn't do anything at all in a sensible LSM) and since it's a new syscall and its users shouldn't be using SECURE_KEEP_CAPS anyway, I just leave that bit along. Signed-off-by: Andy Lutomirski <luto@mit.edu> --- arch/x86/ia32/sys_ia32.c | 2 +- arch/x86/include/asm/unistd_64.h | 2 + arch/x86/kernel/entry_64.S | 16 +++++ arch/x86/kernel/process.c | 28 +++++++++- fs/binfmt_elf.c | 6 +- fs/binfmt_elf_fdpic.c | 6 +- fs/binfmt_em86.c | 2 +- fs/binfmt_flat.c | 7 ++- fs/binfmt_misc.c | 2 +- fs/binfmt_script.c | 2 +- fs/compat.c | 6 +- fs/exec.c | 118 ++++++++++++++++++++++++-------------- include/linux/binfmts.h | 3 + include/linux/compat.h | 2 +- include/linux/fs.h | 2 +- include/linux/sched.h | 2 +- 16 files changed, 146 insertions(+), 60 deletions(-) diff --git a/arch/x86/ia32/sys_ia32.c b/arch/x86/ia32/sys_ia32.c index 422572c..e6f3dff 100644 --- a/arch/x86/ia32/sys_ia32.c +++ b/arch/x86/ia32/sys_ia32.c @@ -528,7 +528,7 @@ asmlinkage long sys32_execve(char __user *name, compat_uptr_t __user *argv, error = PTR_ERR(filename); if (IS_ERR(filename)) return error; - error = compat_do_execve(filename, argv, envp, regs); + error = compat_do_execve(filename, argv, envp, regs, true); putname(filename); return error; } diff --git a/arch/x86/include/asm/unistd_64.h b/arch/x86/include/asm/unistd_64.h index 4843f7b..d792a8c 100644 --- a/arch/x86/include/asm/unistd_64.h +++ b/arch/x86/include/asm/unistd_64.h @@ -663,6 +663,8 @@ __SYSCALL(__NR_rt_tgsigqueueinfo, sys_rt_tgsigqueueinfo) __SYSCALL(__NR_perf_event_open, sys_perf_event_open) #define __NR_recvmmsg 299 __SYSCALL(__NR_recvmmsg, sys_recvmmsg) +#define __NR_execve_nosecurity 300 +__SYSCALL(__NR_execve_nosecurity, stub_execve_nosecurity) #ifndef __NO_STUBS #define __ARCH_WANT_OLD_READDIR diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 0697ff1..77d0fd6 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -728,6 +728,22 @@ ENTRY(stub_execve) CFI_ENDPROC END(stub_execve) +ENTRY(stub_execve_nosecurity) + CFI_STARTPROC + popq %r11 + CFI_ADJUST_CFA_OFFSET -8 + CFI_REGISTER rip, r11 + SAVE_REST + FIXUP_TOP_OF_STACK %r11 + movq %rsp, %rcx + call sys_execve_nosecurity + RESTORE_TOP_OF_STACK %r11 + movq %rax,RAX(%rsp) + RESTORE_REST + jmp int_ret_from_sys_call + CFI_ENDPROC +END(stub_execve) + /* * sigreturn is special because it needs to restore all registers on return. * This cannot be done with SYSRET, so use the IRET return path instead. diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index c9b3522..90c153b 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -303,7 +303,33 @@ long sys_execve(char __user *name, char __user * __user *argv, error = PTR_ERR(filename); if (IS_ERR(filename)) return error; - error = do_execve(filename, argv, envp, regs); + error = do_execve(filename, argv, envp, regs, true); + +#ifdef CONFIG_X86_32 + if (error == 0) { + /* Make sure we don't return using sysenter.. */ + set_thread_flag(TIF_IRET); + } +#endif + + putname(filename); + return error; +} + +/* + * sys_execve_nosecurity() executes a new program without security changes. + */ +long sys_execve_nosecurity(char __user *name, char __user * __user *argv, + char __user * __user *envp, struct pt_regs *regs) +{ + long error; + char *filename; + + filename = getname(name); + error = PTR_ERR(filename); + if (IS_ERR(filename)) + return error; + error = do_execve(filename, argv, envp, regs, false); #ifdef CONFIG_X86_32 if (error == 0) { diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index fd5b2ea..6d3f0b6 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -231,7 +231,8 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, NEW_AUX_ENT(AT_EUID, cred->euid); NEW_AUX_ENT(AT_GID, cred->gid); NEW_AUX_ENT(AT_EGID, cred->egid); - NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm)); + NEW_AUX_ENT(AT_SECURE, + bprm->change_security ? security_bprm_secureexec(bprm) : 0); NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes); NEW_AUX_ENT(AT_EXECFN, bprm->exec); if (k_platform) { @@ -662,7 +663,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs) if (elf_interpreter[elf_ppnt->p_filesz - 1] != '\0') goto out_free_interp; - interpreter = open_exec(elf_interpreter); + interpreter = open_exec(elf_interpreter, + !bprm->change_security); retval = PTR_ERR(interpreter); if (IS_ERR(interpreter)) goto out_free_interp; diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 18d7729..b28b7a9 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -232,7 +232,8 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm, kdebug("Using ELF interpreter %s", interpreter_name); /* replace the program with the interpreter */ - interpreter = open_exec(interpreter_name); + interpreter = open_exec(interpreter_name, + !bprm->change_security); retval = PTR_ERR(interpreter); if (IS_ERR(interpreter)) { interpreter = NULL; @@ -636,7 +637,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm, NEW_AUX_ENT(AT_EUID, (elf_addr_t) cred->euid); NEW_AUX_ENT(AT_GID, (elf_addr_t) cred->gid); NEW_AUX_ENT(AT_EGID, (elf_addr_t) cred->egid); - NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm)); + NEW_AUX_ENT(AT_SECURE, + bprm->change_security ? security_bprm_secureexec(bprm) : 0); NEW_AUX_ENT(AT_EXECFN, bprm->exec); #ifdef ARCH_DLINFO diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index 32fb00b..debae80 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -81,7 +81,7 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs) * Note that we use open_exec() as the name is now in kernel * space, and we don't need to copy it. */ - file = open_exec(interp); + file = open_exec(interp, !bprm->change_security); if (IS_ERR(file)) return PTR_ERR(file); diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 42c6b4a..bafcee0 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -80,10 +80,12 @@ struct lib_info { unsigned long build_date; /* When this one was compiled */ short loaded; /* Has this library been loaded? */ } lib_list[MAX_SHARED_LIBS]; + + unsigned int change_security : 1; }; #ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); +static int load_flat_shared_library(int id, struct lib_info *p, bool); #endif static int load_flat_binary(struct linux_binprm *, struct pt_regs * regs); @@ -822,7 +824,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs) /* Open the file up */ bprm.filename = buf; - bprm.file = open_exec(bprm.filename); + bprm.file = open_exec(bprm.filename, !libs->change_security); res = PTR_ERR(bprm.file); if (IS_ERR(bprm.file)) return res; @@ -865,6 +867,7 @@ static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs) int i, j; memset(&libinfo, 0, sizeof(libinfo)); + libinfo.change_security = bprm->change_security; /* * We have to add the size of our arguments to our stack size * otherwise it's too easy for users to create stack overflows diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index c4e8353..7beb122 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -178,7 +178,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs) bprm->interp = iname; /* for binfmt_script */ - interp_file = open_exec (iname); + interp_file = open_exec (iname, !bprm->change_security); retval = PTR_ERR (interp_file); if (IS_ERR (interp_file)) goto _error; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 0834350..64b6d4d 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -87,7 +87,7 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs) /* * OK, now restart the process with the interpreter's dentry. */ - file = open_exec(interp); + file = open_exec(interp, !bprm->change_security); if (IS_ERR(file)) return PTR_ERR(file); diff --git a/fs/compat.c b/fs/compat.c index 00d90c2..585a2d7 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1459,7 +1459,8 @@ out: int compat_do_execve(char * filename, compat_uptr_t __user *argv, compat_uptr_t __user *envp, - struct pt_regs * regs) + struct pt_regs * regs, + bool change_security) { struct linux_binprm *bprm; struct file *file; @@ -1475,6 +1476,7 @@ int compat_do_execve(char * filename, bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); if (!bprm) goto out_files; + bprm->change_security = change_security; retval = prepare_bprm_creds(bprm); if (retval) @@ -1486,7 +1488,7 @@ int compat_do_execve(char * filename, clear_in_exec = retval; current->in_execve = 1; - file = open_exec(filename); + file = open_exec(filename, !change_security); retval = PTR_ERR(file); if (IS_ERR(file)) goto out_unmark; diff --git a/fs/exec.c b/fs/exec.c index cce6bbd..4067b65 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -346,6 +346,12 @@ int bprm_mm_init(struct linux_binprm *bprm) if (err) goto err; + /* If bprm->change_security is set, we'll override this later. If not, + then just keep the current settings. (Regenerating it from the + file flags or from (uid == euid) could have strange side effects.) + */ + set_dumpable(bprm->mm, current->mm ? get_dumpable(current->mm) : 1); + return 0; err: @@ -660,14 +666,18 @@ EXPORT_SYMBOL(setup_arg_pages); #endif /* CONFIG_MMU */ -struct file *open_exec(const char *name) +struct file *open_exec(const char *name, bool require_read_access) { struct file *file; int err; + int acc_mode = MAY_EXEC | MAY_OPEN; + + if (require_read_access) + acc_mode |= MAY_READ; file = do_filp_open(AT_FDCWD, name, O_LARGEFILE | O_RDONLY | FMODE_EXEC, 0, - MAY_EXEC | MAY_OPEN); + acc_mode); if (IS_ERR(file)) goto out; @@ -1000,10 +1010,13 @@ void setup_new_exec(struct linux_binprm * bprm) /* This is the point of no return */ current->sas_ss_sp = current->sas_ss_size = 0; - if (current_euid() == current_uid() && current_egid() == current_gid()) - set_dumpable(current->mm, 1); - else - set_dumpable(current->mm, suid_dumpable); + if (bprm->change_security) { + if (current_euid() == current_uid() && + current_egid() == current_gid()) + set_dumpable(current->mm, 1); + else + set_dumpable(current->mm, suid_dumpable); + } name = bprm->filename; @@ -1025,20 +1038,23 @@ void setup_new_exec(struct linux_binprm * bprm) current->mm->task_size = TASK_SIZE; /* install the new credentials */ - if (bprm->cred->uid != current_euid() || - bprm->cred->gid != current_egid()) { - current->pdeath_signal = 0; - } else if (file_permission(bprm->file, MAY_READ) || - bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP) { - set_dumpable(current->mm, suid_dumpable); + if (bprm->change_security) { + if (bprm->cred->uid != current_euid() || + bprm->cred->gid != current_egid()) { + current->pdeath_signal = 0; + } else if (file_permission(bprm->file, MAY_READ) || + bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP) { + set_dumpable(current->mm, suid_dumpable); + } + + /* + * Flush performance counters when crossing a + * security domain: + */ + if (!get_dumpable(current->mm)) + perf_event_exit_task(current); } - /* - * Flush performance counters when crossing a - * security domain: - */ - if (!get_dumpable(current->mm)) - perf_event_exit_task(current); /* An exec changes our domain. We are no longer part of the thread group */ @@ -1058,6 +1074,9 @@ EXPORT_SYMBOL(setup_new_exec); */ int prepare_bprm_creds(struct linux_binprm *bprm) { + if (!bprm->change_security) + return 0; + if (mutex_lock_interruptible(¤t->cred_guard_mutex)) return -ERESTARTNOINTR; @@ -1084,6 +1103,9 @@ void free_bprm(struct linux_binprm *bprm) */ void install_exec_creds(struct linux_binprm *bprm) { + if (!bprm->change_security) + return; + security_bprm_committing_creds(bprm); commit_creds(bprm->cred); @@ -1109,6 +1131,9 @@ int check_unsafe_exec(struct linux_binprm *bprm) unsigned n_fs; int res = 0; + if (!bprm->change_security) + return 0; + bprm->unsafe = tracehook_unsafe_exec(p); n_fs = 1; @@ -1150,35 +1175,37 @@ int prepare_binprm(struct linux_binprm *bprm) if (bprm->file->f_op == NULL) return -EACCES; - /* clear any previous set[ug]id data from a previous binary */ - bprm->cred->euid = current_euid(); - bprm->cred->egid = current_egid(); + if (bprm->change_security) { + /* clear any previous set[ug]id data from a previous binary */ + bprm->cred->euid = current_euid(); + bprm->cred->egid = current_egid(); - if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)) { - /* Set-uid? */ - if (mode & S_ISUID) { - bprm->per_clear |= PER_CLEAR_ON_SETID; - bprm->cred->euid = inode->i_uid; - } + if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)) { + /* Set-uid? */ + if (mode & S_ISUID) { + bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->cred->euid = inode->i_uid; + } - /* Set-gid? */ - /* - * If setgid is set but no group execute bit then this - * is a candidate for mandatory locking, not a setgid - * executable. - */ - if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { - bprm->per_clear |= PER_CLEAR_ON_SETID; - bprm->cred->egid = inode->i_gid; + /* Set-gid? */ + /* + * If setgid is set but no group execute bit then this + * is a candidate for mandatory locking, not a setgid + * executable. + */ + if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) { + bprm->per_clear |= PER_CLEAR_ON_SETID; + bprm->cred->egid = inode->i_gid; + } } + + /* fill in binprm security blob */ + retval = security_bprm_set_creds(bprm); + if (retval) + return retval; + bprm->cred_prepared = 1; } - /* fill in binprm security blob */ - retval = security_bprm_set_creds(bprm); - if (retval) - return retval; - bprm->cred_prepared = 1; - memset(bprm->buf, 0, BINPRM_BUF_SIZE); return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE); } @@ -1314,7 +1341,8 @@ EXPORT_SYMBOL(search_binary_handler); int do_execve(char * filename, char __user *__user *argv, char __user *__user *envp, - struct pt_regs * regs) + struct pt_regs * regs, + bool change_security) { struct linux_binprm *bprm; struct file *file; @@ -1330,6 +1358,7 @@ int do_execve(char * filename, bprm = kzalloc(sizeof(*bprm), GFP_KERNEL); if (!bprm) goto out_files; + bprm->change_security = change_security; retval = prepare_bprm_creds(bprm); if (retval) @@ -1338,10 +1367,11 @@ int do_execve(char * filename, retval = check_unsafe_exec(bprm); if (retval < 0) goto out_free; + clear_in_exec = retval; current->in_execve = 1; - file = open_exec(filename); + file = open_exec(filename, !change_security); retval = PTR_ERR(file); if (IS_ERR(file)) goto out_unmark; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 89c6249..019de0b 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -36,6 +36,9 @@ struct linux_binprm{ struct mm_struct *mm; unsigned long p; /* current top of mem */ unsigned int + change_security:1, /* true if this call to exec will + affect the security state of the + process. */ cred_prepared:1,/* true if creds already prepared (multiple * preps happen for interpreters) */ cap_effective:1;/* true if has elevated effective capabilities, diff --git a/include/linux/compat.h b/include/linux/compat.h index ef68119..45ccc3e 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -243,7 +243,7 @@ asmlinkage ssize_t compat_sys_pwritev(unsigned long fd, unsigned long vlen, u32 pos_low, u32 pos_high); int compat_do_execve(char * filename, compat_uptr_t __user *argv, - compat_uptr_t __user *envp, struct pt_regs * regs); + compat_uptr_t __user *envp, struct pt_regs * regs, bool); asmlinkage long compat_sys_select(int n, compat_ulong_t __user *inp, compat_ulong_t __user *outp, compat_ulong_t __user *exp, diff --git a/include/linux/fs.h b/include/linux/fs.h index ebb1cd5..b2094cc 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2128,7 +2128,7 @@ extern struct file *do_filp_open(int dfd, const char *pathname, extern int may_open(struct path *, int, int); extern int kernel_read(struct file *, loff_t, char *, unsigned long); -extern struct file * open_exec(const char *); +extern struct file * open_exec(const char *, bool); /* fs/dcache.c -- generic fs support functions */ extern int is_subdir(struct dentry *, struct dentry *); diff --git a/include/linux/sched.h b/include/linux/sched.h index 78efe7c..2d03069 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2175,7 +2175,7 @@ extern void daemonize(const char *, ...); extern int allow_signal(int); extern int disallow_signal(int); -extern int do_execve(char *, char __user * __user *, char __user * __user *, struct pt_regs *); +extern int do_execve(char *, char __user * __user *, char __user * __user *, struct pt_regs *, bool); extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned long, int __user *, int __user *); struct task_struct *fork_idle(int); -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] Add PR_RESTRICT_ME to disable security-sensitive features for a process tree. 2010-03-26 13:38 [PATCH 0/3] Taming execve, setuid, and LSMs Andy Lutomirski 2010-03-26 13:38 ` [PATCH 1/3] Add the execve_nosecurity syscall Andy Lutomirski @ 2010-03-26 13:38 ` Andy Lutomirski 2010-03-26 13:38 ` [PATCH 3/3] Add PR_SET_FORCE_EXECVE_NOSECURITY to turn execve calls into execve_nosecurity Andy Lutomirski 2010-04-19 17:26 ` [PATCH 0/3] Taming execve, setuid, and LSMs Serge E. Hallyn 3 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2010-03-26 13:38 UTC (permalink / raw) To: linux-kernel, linux-security-module Cc: Eric Biederman, Andrew G. Morgan, Andy Lutomirski This adds a prctl PR_RESTRICT_ME that enables restrictions that cannot be disabled and are inherited by children. There's a long history of dangerous patches that add similar restrictions that persist across execve. This is bad: execve can grant new privileges, and restrictions on exec'd programs can be used to subvert them. To avoid this issue, the very first PR_RESTRICT_ME restriction bit is PR_RESTRICT_EXEC, which simply disables exec. In the presence of execve_nosecurity, this can be used to shoot oneself in the foot, but it should not be possible to shoot other people in the foot with this patch. Any future PR_RESTRICT_ME bits should not be allowed to be set unless PR_RESTRICT_EXEC is also set. Signed-off-by: Andy Lutomirski <luto@mit.edu> --- fs/compat.c | 5 +++++ fs/exec.c | 5 +++++ include/linux/prctl.h | 6 ++++++ include/linux/sched.h | 2 ++ kernel/fork.c | 2 ++ kernel/sys.c | 29 +++++++++++++++++++++++++++++ 6 files changed, 49 insertions(+), 0 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index 585a2d7..a091da6 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1468,6 +1468,11 @@ int compat_do_execve(char * filename, bool clear_in_exec; int retval; + if (current->restrict_exec && change_security) { + retval = -EPERM; + goto out_ret; + } + retval = unshare_files(&displaced); if (retval) goto out_ret; diff --git a/fs/exec.c b/fs/exec.c index 4067b65..37fb5fa 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1350,6 +1350,11 @@ int do_execve(char * filename, bool clear_in_exec; int retval; + if (current->restrict_exec && change_security) { + retval = -EPERM; + goto out_ret; + } + retval = unshare_files(&displaced); if (retval) goto out_ret; diff --git a/include/linux/prctl.h b/include/linux/prctl.h index a3baeb2..b926055 100644 --- a/include/linux/prctl.h +++ b/include/linux/prctl.h @@ -102,4 +102,10 @@ #define PR_MCE_KILL_GET 34 +/* Get/set irrevocable restrictions. */ +#define PR_RESTRICT_ME 35 +# define PR_RESTRICT_EXEC 1 + +#define PR_GET_RESTRICT 36 + #endif /* _LINUX_PRCTL_H */ diff --git a/include/linux/sched.h b/include/linux/sched.h index 2d03069..d1956f7 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1302,6 +1302,8 @@ struct task_struct { /* Revert to default priority/policy when forking */ unsigned sched_reset_on_fork:1; + unsigned restrict_exec:1; /* Process may not call execve. */ + pid_t pid; pid_t tgid; diff --git a/kernel/fork.c b/kernel/fork.c index f88bd98..8f994e5 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1043,6 +1043,8 @@ static struct task_struct *copy_process(unsigned long clone_flags, if (retval < 0) goto bad_fork_free; + p->restrict_exec = current->restrict_exec; + /* * If multiple threads are within copy_process(), then this check * triggers too late. This doesn't hurt, the check is only there diff --git a/kernel/sys.c b/kernel/sys.c index 18bde97..3f4aa33 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -1580,6 +1580,35 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, else error = PR_MCE_KILL_DEFAULT; break; + case PR_RESTRICT_ME: + if (arg3 | arg4 | arg5) + return -EINVAL; + + /* This should become more sophisticated if + more restriction flags are added. */ + if (arg2 == PR_RESTRICT_EXEC) + current->restrict_exec = 1; + else if (arg2 != 0) + return -EINVAL; + break; + case PR_GET_RESTRICT: + if (arg2) { + unsigned long out2 = 0; + if (current->restrict_exec) + out2 |= PR_RESTRICT_EXEC; + error = put_user(out2, + (unsigned long __user *)arg2); + } + + /* In case some crazy person wants to add tons + of future flags, don't get in their way. */ + if (arg3 && !error) + error = put_user(0, (unsigned long __user *)arg3); + if (arg4 && !error) + error = put_user(0, (unsigned long __user *)arg4); + if (arg5 && !error) + error = put_user(0, (unsigned long __user *)arg5); + break; default: error = -EINVAL; break; -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] Add PR_SET_FORCE_EXECVE_NOSECURITY to turn execve calls into execve_nosecurity. 2010-03-26 13:38 [PATCH 0/3] Taming execve, setuid, and LSMs Andy Lutomirski 2010-03-26 13:38 ` [PATCH 1/3] Add the execve_nosecurity syscall Andy Lutomirski 2010-03-26 13:38 ` [PATCH 2/3] Add PR_RESTRICT_ME to disable security-sensitive features for a process tree Andy Lutomirski @ 2010-03-26 13:38 ` Andy Lutomirski 2010-04-19 17:26 ` [PATCH 0/3] Taming execve, setuid, and LSMs Serge E. Hallyn 3 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2010-03-26 13:38 UTC (permalink / raw) To: linux-kernel, linux-security-module Cc: Eric Biederman, Andrew G. Morgan, Andy Lutomirski This flag is preserved across execve_nosecurity. It's obviously dangerous, so we only allow it if PR_RESTRICT_EXEC is set. Signed-off-by: Andy Lutomirski <luto@mit.edu> --- fs/compat.c | 3 +++ fs/exec.c | 3 +++ include/linux/prctl.h | 5 +++++ include/linux/sched.h | 1 + kernel/fork.c | 1 + kernel/sys.c | 13 +++++++++++++ 6 files changed, 26 insertions(+), 0 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index a091da6..4b7f61f 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1468,6 +1468,9 @@ int compat_do_execve(char * filename, bool clear_in_exec; int retval; + if (current->force_execve_nosecurity) + change_security = false; + if (current->restrict_exec && change_security) { retval = -EPERM; goto out_ret; diff --git a/fs/exec.c b/fs/exec.c index 37fb5fa..0e045b8 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1350,6 +1350,9 @@ int do_execve(char * filename, bool clear_in_exec; int retval; + if (current->force_execve_nosecurity) + change_security = false; + if (current->restrict_exec && change_security) { retval = -EPERM; goto out_ret; diff --git a/include/linux/prctl.h b/include/linux/prctl.h index b926055..8465df3 100644 --- a/include/linux/prctl.h +++ b/include/linux/prctl.h @@ -108,4 +108,9 @@ #define PR_GET_RESTRICT 36 +/* Get/set execve -> execve_nosecurity remapping. */ +#define PR_SET_FORCE_EXECVE_NOSECURITY 37 +#define PR_GET_FORCE_EXECVE_NOSECURITY 38 + + #endif /* _LINUX_PRCTL_H */ diff --git a/include/linux/sched.h b/include/linux/sched.h index d1956f7..59f7bcd 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1303,6 +1303,7 @@ struct task_struct { unsigned sched_reset_on_fork:1; unsigned restrict_exec:1; /* Process may not call execve. */ + unsigned force_execve_nosecurity:1; /* execve means execve_nosecurity */ pid_t pid; pid_t tgid; diff --git a/kernel/fork.c b/kernel/fork.c index 8f994e5..d7e1688 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1044,6 +1044,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, goto bad_fork_free; p->restrict_exec = current->restrict_exec; + p->force_execve_nosecurity = current->force_execve_nosecurity; /* * If multiple threads are within copy_process(), then this check diff --git a/kernel/sys.c b/kernel/sys.c index 3f4aa33..d34daaa 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -1609,6 +1609,19 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, if (arg5 && !error) error = put_user(0, (unsigned long __user *)arg5); break; + case PR_SET_FORCE_EXECVE_NOSECURITY: + if (arg3 | arg4 | arg5) + return -EINVAL; + + /* Someone braver could remove this check. */ + if (!current->restrict_exec) + return -EPERM; + + current->force_execve_nosecurity = !!arg2; + break; + case PR_GET_FORCE_EXECVE_NOSECURITY: + error = current->force_execve_nosecurity; + break; default: error = -EINVAL; break; -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-03-26 13:38 [PATCH 0/3] Taming execve, setuid, and LSMs Andy Lutomirski ` (2 preceding siblings ...) 2010-03-26 13:38 ` [PATCH 3/3] Add PR_SET_FORCE_EXECVE_NOSECURITY to turn execve calls into execve_nosecurity Andy Lutomirski @ 2010-04-19 17:26 ` Serge E. Hallyn 2010-04-19 21:32 ` Andrew Lutomirski 3 siblings, 1 reply; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-19 17:26 UTC (permalink / raw) To: Andy Lutomirski Cc: linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andy Lutomirski (luto@MIT.EDU): > Every now and then, someone wants to let unprivileged programs change > something about their execution environment (think unsharing namespaces, > changing capabilities, disabling networking, chrooting, mounting and > unmounting filesystems). Whether or not any of these abilities are good > ideas, there's a recurring problem that gets most of these patches shot > down: setuid executables. > > The obvious solution is to allow a process to opt out of setuid > semantics and require processes to do this before using these shiny new > features. [1] [2] > > But there's a problem with this, too: with LSMs running, execve can do > pretty much anything, and even unprivileged users running unprivileged > programs can have crazy security implications. (Take a look at a > default install of Fedora. If you can understand the security > implications of disabling setuid, you get a cookie. If you can figure > out which programs will result in a change of security label when > exec'd, you get another cookie.) > > So here's another solution, based on the idea that in a sane world, > execve should be a lot less magical than it is. Any unprivileged > program can open an executable, parse its headers, map it, and run it, > although getting all the details right is tedious at best (and there's > no good way to get all of the threading semantics right from userspace). > > Patch 1 adds a new syscall execve_nosecurity. It does an exec, but > without changing any security properties. This means no setuid, no > setgid, no LSM credential hooks (e.g. no SELinux type transitions), and > no ptrace restrictions. (You have to have read access to the program, > because disabling security stuff could allow someone to ptrace a program > that they couldn't otherwise ptrace.) This shouldn't be particularly > scary -- any process could do much the same thing with open and mmap. > (You can easily shoot yourself in the foot with this syscall -- think > LD_PRELOAD or running some program with insufficient error checking that > can get subverted when run in the wrong security context. So don't do > that.) > > Patch 2 adds a prctl that irrevocably disables execve. Making execve do > something different that could confuse LSMs is dangerous. Turning the > whole thing off shouldn't be. (Of course, with execve disabled, you can > still use execve_nosecurity. But any program that does that should take > precautions not to shoot itself in the foot.) (In a future revision, > this should probably be a new syscall.) > > Sadly, programs that have opted out of execve might want to use > subprocesses that in turn run execve. This will fail. So patch 3 > (which is ugly, but I don't see anything fundamentally wrong with it) > allows processes to set a flag that turns execve into execve_nosecurity. > This flag survives exec. Of course, this could be used to subvert > setuid programs, so you can't set this flag unless you disable ordinary > exec first. > > [1] Unprivileged: http://lkml.org/lkml/2009/12/30/265 > [2] securebit approach: http://lwn.net/Articles/368600/ No responses for a month after this was sent. Really, thanks, I do appreciate the work at another approach. I'll be honest, I prefer option [1]. Though I think it's reasonable to require privilege for prctl(PR_SET_NOSUID). Make it a separate capability, and on most systems it should be safe to have a file sitting in /bin with cap_set_nosuid+pe. If OTOH you know you have legacy or poorly coded privileged programs which would not be safe bc they don't verify that they have the needed privs, you just don't provide the program to do prctl(PR_SET_NOSUID) for unprivileged users. ( I did like using new securebits as in [2], but I prefer the automatic not-raising-privs of [1] to simply -EPERM on uid/gid change and lack kof checking for privs raising of [2]. ) Really the trick will be finding a balance to satisfy those wanting this as a separate LSM, without traipsing into LSM stacking territory. I myself think this feature fits very nicely with established semantics, but not everyone agrees, so chances are my view is a bit tainted, and we should defer to those wanting this to be an LSM. Of course, another alternative is to skip this feature altogether and push toward targeted capabilties. The problem is that path amounts to playing whack-a-mole to catch all the places where privilege might leak to a parent namespace, whereas [1] simply, cleanly cuts them all off at the source. thanks, -serge ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-19 17:26 ` [PATCH 0/3] Taming execve, setuid, and LSMs Serge E. Hallyn @ 2010-04-19 21:32 ` Andrew Lutomirski 2010-04-19 21:39 ` Serge E. Hallyn 0 siblings, 1 reply; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-19 21:32 UTC (permalink / raw) To: Serge E. Hallyn Cc: linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Mon, Apr 19, 2010 at 1:26 PM, Serge E. Hallyn <serue@us.ibm.com> wrote: > Quoting Andy Lutomirski (luto@MIT.EDU): >> Every now and then, someone wants to let unprivileged programs change >> something about their execution environment (think unsharing namespaces, >> changing capabilities, disabling networking, chrooting, mounting and >> unmounting filesystems). Whether or not any of these abilities are good >> ideas, there's a recurring problem that gets most of these patches shot >> down: setuid executables. >> >> The obvious solution is to allow a process to opt out of setuid >> semantics and require processes to do this before using these shiny new >> features. [1] [2] >> >> But there's a problem with this, too: with LSMs running, execve can do >> pretty much anything, and even unprivileged users running unprivileged >> programs can have crazy security implications. (Take a look at a >> default install of Fedora. If you can understand the security >> implications of disabling setuid, you get a cookie. If you can figure >> out which programs will result in a change of security label when >> exec'd, you get another cookie.) >> >> So here's another solution, based on the idea that in a sane world, >> execve should be a lot less magical than it is. Any unprivileged >> program can open an executable, parse its headers, map it, and run it, >> although getting all the details right is tedious at best (and there's >> no good way to get all of the threading semantics right from userspace). >> >> Patch 1 adds a new syscall execve_nosecurity. It does an exec, but >> without changing any security properties. This means no setuid, no >> setgid, no LSM credential hooks (e.g. no SELinux type transitions), and >> no ptrace restrictions. (You have to have read access to the program, >> because disabling security stuff could allow someone to ptrace a program >> that they couldn't otherwise ptrace.) This shouldn't be particularly >> scary -- any process could do much the same thing with open and mmap. >> (You can easily shoot yourself in the foot with this syscall -- think >> LD_PRELOAD or running some program with insufficient error checking that >> can get subverted when run in the wrong security context. So don't do >> that.) >> >> Patch 2 adds a prctl that irrevocably disables execve. Making execve do >> something different that could confuse LSMs is dangerous. Turning the >> whole thing off shouldn't be. (Of course, with execve disabled, you can >> still use execve_nosecurity. But any program that does that should take >> precautions not to shoot itself in the foot.) (In a future revision, >> this should probably be a new syscall.) >> >> Sadly, programs that have opted out of execve might want to use >> subprocesses that in turn run execve. This will fail. So patch 3 >> (which is ugly, but I don't see anything fundamentally wrong with it) >> allows processes to set a flag that turns execve into execve_nosecurity. >> This flag survives exec. Of course, this could be used to subvert >> setuid programs, so you can't set this flag unless you disable ordinary >> exec first. >> >> [1] Unprivileged: http://lkml.org/lkml/2009/12/30/265 >> [2] securebit approach: http://lwn.net/Articles/368600/ > > No responses for a month after this was sent. Really, thanks, I do > appreciate the work at another approach. > > I'll be honest, I prefer option [1]. Though I think it's reasonable > to require privilege for prctl(PR_SET_NOSUID). Make it a separate > capability, and on most systems it should be safe to have a file > sitting in /bin with cap_set_nosuid+pe. If OTOH you know you have > legacy or poorly coded privileged programs which would not be safe > bc they don't verify that they have the needed privs, you just don't > provide the program to do prctl(PR_SET_NOSUID) for unprivileged users. Both approaches result in two kinds of exec: the normal kind that respects setuid, file capabilities, and LSMs, and the restricted kind that is supposed to be safe when programs have unshared namespaces and other crazy things. Eric's approach [1] adds a restricted kind of exec that ignores setuid but still (AFAICT) respects file capabilities and LSM transitions. I think this is a terrible idea for two reasons: 1. LSM transitions already scare me enough, and if anyone relies on them working in concert with setuid, then the mere act of separating them might break things, even if the "privileged" (by LSM) app in question is well-written. 2. File capabilities are just as dangerous as setuid, and I wouldn't even know how to write a program that's safe when it has extra capabilities granted by fE (or fP or whatever it is) and the caller has, say, an unshared fs namespace and the ability to rearrange the namespace arbitrarily. In short, I think that this nosuid exec is both dangerous in and of itself *and* doesn't actually solve the problem it was supposed to solve. I also don't like relying on the admin to decide that it's safe to allow PR_SET_NOSUID (or whatever you call it) and having to install a special privileged program to enable it. If sandbox-like features require explicit action by root, then they won't be as widely used as they should be. And how many admins will have any clue whether enabling this feature is safe? My approach introduces what I think is a much more obviously safe restricted exec, and I think it's so safe that no privilege or special configuration should be required to use it. As for what to call it (execve_nosecurity or PR_SET_NOSUID) or whether to have a special syscall so that programs that aren't restricted can use the restricted exec, I don't care all that much. I just think that the separate syscall might be useful in its own right and required almost no additional code, so I added it. > > ( I did like using new securebits as in [2], but I prefer the > automatic not-raising-privs of [1] to simply -EPERM on uid/gid > change and lack kof checking for privs raising of [2]. ) > > Really the trick will be finding a balance to satisfy those wanting > this as a separate LSM, without traipsing into LSM stacking territory. I think that making this an LSM is absurd. Containers (and anything else people want to do with namespaces or with other new features that interact badly with setuid) are features that people should be able to use easily, and system's choice of LSM shouldn't have anything to do with them. Not to mention that we're trying to *add* rights (e.g. unprivileged unshare), and LSM is about *removing* rights. > > I myself think this feature fits very nicely with established semantics, > but not everyone agrees, so chances are my view is a bit tainted, and > we should defer to those wanting this to be an LSM. > > Of course, another alternative is to skip this feature altogether and > push toward targeted capabilties. The problem is that path amounts > to playing whack-a-mole to catch all the places where privilege might > leak to a parent namespace, whereas [1] simply, cleanly cuts them all > off at the source. Agreed, that sounds painful. My secret goal is real userspace-controlled (by unprivileged users, no less) sandboxes, in which case in-kernel target capabilities are probably impossible. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-19 21:32 ` Andrew Lutomirski @ 2010-04-19 21:39 ` Serge E. Hallyn 2010-04-19 22:02 ` Andrew Lutomirski 2010-04-20 12:37 ` Stephen Smalley 0 siblings, 2 replies; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-19 21:39 UTC (permalink / raw) To: Andrew Lutomirski Cc: Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andrew Lutomirski (luto@mit.edu): > On Mon, Apr 19, 2010 at 1:26 PM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > Quoting Andy Lutomirski (luto@MIT.EDU): > >> Every now and then, someone wants to let unprivileged programs change > >> something about their execution environment (think unsharing namespaces, > >> changing capabilities, disabling networking, chrooting, mounting and > >> unmounting filesystems). ?Whether or not any of these abilities are good > >> ideas, there's a recurring problem that gets most of these patches shot > >> down: setuid executables. > >> > >> The obvious solution is to allow a process to opt out of setuid > >> semantics and require processes to do this before using these shiny new > >> features. [1] [2] > >> > >> But there's a problem with this, too: with LSMs running, execve can do > >> pretty much anything, and even unprivileged users running unprivileged > >> programs can have crazy security implications. ?(Take a look at a > >> default install of Fedora. ?If you can understand the security > >> implications of disabling setuid, you get a cookie. ?If you can figure > >> out which programs will result in a change of security label when > >> exec'd, you get another cookie.) > >> > >> So here's another solution, based on the idea that in a sane world, > >> execve should be a lot less magical than it is. ?Any unprivileged > >> program can open an executable, parse its headers, map it, and run it, > >> although getting all the details right is tedious at best (and there's > >> no good way to get all of the threading semantics right from userspace). > >> > >> Patch 1 adds a new syscall execve_nosecurity. ?It does an exec, but > >> without changing any security properties. ?This means no setuid, no > >> setgid, no LSM credential hooks (e.g. no SELinux type transitions), and > >> no ptrace restrictions. ?(You have to have read access to the program, > >> because disabling security stuff could allow someone to ptrace a program > >> that they couldn't otherwise ptrace.) ?This shouldn't be particularly > >> scary -- any process could do much the same thing with open and mmap. > >> (You can easily shoot yourself in the foot with this syscall -- think > >> LD_PRELOAD or running some program with insufficient error checking that > >> can get subverted when run in the wrong security context. ?So don't do > >> that.) > >> > >> Patch 2 adds a prctl that irrevocably disables execve. ?Making execve do > >> something different that could confuse LSMs is dangerous. ?Turning the > >> whole thing off shouldn't be. ?(Of course, with execve disabled, you can > >> still use execve_nosecurity. ?But any program that does that should take > >> precautions not to shoot itself in the foot.) ?(In a future revision, > >> this should probably be a new syscall.) > >> > >> Sadly, programs that have opted out of execve might want to use > >> subprocesses that in turn run execve. ?This will fail. ?So patch 3 > >> (which is ugly, but I don't see anything fundamentally wrong with it) > >> allows processes to set a flag that turns execve into execve_nosecurity. > >> This flag survives exec. ?Of course, this could be used to subvert > >> setuid programs, so you can't set this flag unless you disable ordinary > >> exec first. > >> > >> [1] Unprivileged: http://lkml.org/lkml/2009/12/30/265 > >> [2] securebit approach: http://lwn.net/Articles/368600/ > > > > No responses for a month after this was sent. ?Really, thanks, I do > > appreciate the work at another approach. > > > > I'll be honest, I prefer option [1]. ?Though I think it's reasonable > > to require privilege for prctl(PR_SET_NOSUID). ?Make it a separate > > capability, and on most systems it should be safe to have a file > > sitting in /bin with cap_set_nosuid+pe. ?If OTOH you know you have > > legacy or poorly coded privileged programs which would not be safe > > bc they don't verify that they have the needed privs, you just don't > > provide the program to do prctl(PR_SET_NOSUID) for unprivileged users. > > Both approaches result in two kinds of exec: the normal kind that > respects setuid, file capabilities, and LSMs, and the restricted kind > that is supposed to be safe when programs have unshared namespaces and > other crazy things. > > Eric's approach [1] adds a restricted kind of exec that ignores setuid > but still (AFAICT) respects file capabilities No, please see the rest of that thread - that was an oversight. > and LSM transitions. I > think this is a terrible idea for two reasons: > 1. LSM transitions already scare me enough, and if anyone relies on > them working in concert with setuid, then the mere act of separating > them might break things, even if the "privileged" (by LSM) app in > question is well-written. hmm... A good point. > 2. File capabilities are just as dangerous as setuid, and I wouldn't > even know how to write a program that's safe when it has extra > capabilities granted by fE (or fP or whatever it is) and the caller > has, say, an unshared fs namespace and the ability to rearrange the > namespace arbitrarily. Absolutely these should not be ignored, and Eric didn't mean to ignore them. > In short, I think that this nosuid exec is both dangerous in and of > itself *and* doesn't actually solve the problem it was supposed to > solve. > > I also don't like relying on the admin to decide that it's safe to > allow PR_SET_NOSUID (or whatever you call it) and having to install a > special privileged program to enable it. If sandbox-like features > require explicit action by root, then they won't be as widely used as > they should be. And how many admins will have any clue whether > enabling this feature is safe? I do not agree with deciding the admins are not competent to admin their system and therefore we should bypass them and let users decide. But it's moot, as I think you've convinced me with your point 1. above to take another look at your patches. > My approach introduces what I think is a much more obviously safe > restricted exec, and I think it's so safe that no privilege or special > configuration should be required to use it. > > As for what to call it (execve_nosecurity or PR_SET_NOSUID) or whether > to have a special syscall so that programs that aren't restricted can > use the restricted exec, I don't care all that much. I just think > that the separate syscall might be useful in its own right and > required almost no additional code, so I added it. > > > > > > ( I did like using new securebits as in [2], but I prefer the > > automatic not-raising-privs of [1] to simply -EPERM on uid/gid > > change and lack kof checking for privs raising of [2]. ) > > > > Really the trick will be finding a balance to satisfy those wanting > > this as a separate LSM, without traipsing into LSM stacking territory. > > I think that making this an LSM is absurd. Containers (and anything > else people want to do with namespaces or with other new features that > interact badly with setuid) are features that people should be able to Yes, but that's a reason to aim for targeted caps. Exec_nopriv or whatever is more a sandbox than a namespace feature. > use easily, and system's choice of LSM shouldn't have anything to do > with them. Not to mention that we're trying to *add* rights (e.g. > unprivileged unshare), and LSM is about *removing* rights. > > > > > I myself think this feature fits very nicely with established semantics, > > but not everyone agrees, so chances are my view is a bit tainted, and > > we should defer to those wanting this to be an LSM. > > > > Of course, another alternative is to skip this feature altogether and > > push toward targeted capabilties. ?The problem is that path amounts > > to playing whack-a-mole to catch all the places where privilege might > > leak to a parent namespace, whereas [1] simply, cleanly cuts them all > > off at the source. > > Agreed, that sounds painful. My secret goal is real > userspace-controlled (by unprivileged users, no less) sandboxes, in > which case in-kernel target capabilities are probably impossible. Not sure what you mean by that last part - inside the sandbox, you won't get capabilities, targeted or otherwise, but certainly targeted capabilities and a sandbox are not mutually exclusive. Thanks for responding, I'll take another look at your patchset in detail. thanks, -serge ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-19 21:39 ` Serge E. Hallyn @ 2010-04-19 22:02 ` Andrew Lutomirski 2010-04-19 22:25 ` Serge E. Hallyn 2010-04-20 12:37 ` Stephen Smalley 1 sibling, 1 reply; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-19 22:02 UTC (permalink / raw) To: Serge E. Hallyn Cc: Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Mon, Apr 19, 2010 at 5:39 PM, Serge E. Hallyn <serge@hallyn.com> wrote: > Quoting Andrew Lutomirski (luto@mit.edu): >> > >> > ( I did like using new securebits as in [2], but I prefer the >> > automatic not-raising-privs of [1] to simply -EPERM on uid/gid >> > change and lack kof checking for privs raising of [2]. ) >> > >> > Really the trick will be finding a balance to satisfy those wanting >> > this as a separate LSM, without traipsing into LSM stacking territory. >> >> I think that making this an LSM is absurd. Containers (and anything >> else people want to do with namespaces or with other new features that >> interact badly with setuid) are features that people should be able to > > Yes, but that's a reason to aim for targeted caps. Exec_nopriv or > whatever is more a sandbox than a namespace feature. > >> use easily, and system's choice of LSM shouldn't have anything to do >> with them. Not to mention that we're trying to *add* rights (e.g. >> unprivileged unshare), and LSM is about *removing* rights. Is a targeted cap something like "process A can call setdomainname, but only on one particular UTS namespace?" >> >> > >> > I myself think this feature fits very nicely with established semantics, >> > but not everyone agrees, so chances are my view is a bit tainted, and >> > we should defer to those wanting this to be an LSM. >> > >> > Of course, another alternative is to skip this feature altogether and >> > push toward targeted capabilties. ?The problem is that path amounts >> > to playing whack-a-mole to catch all the places where privilege might >> > leak to a parent namespace, whereas [1] simply, cleanly cuts them all >> > off at the source. >> >> Agreed, that sounds painful. My secret goal is real >> userspace-controlled (by unprivileged users, no less) sandboxes, in >> which case in-kernel target capabilities are probably impossible. > > Not sure what you mean by that last part - inside the sandbox, you won't > get capabilities, targeted or otherwise, but certainly targeted capabilities > and a sandbox are not mutually exclusive. Agreed. What I want is a syscall that says "make me a sandbox" and then for that program to be able to intercept and modify most (all?) syscalls issued from inside the sandbox. But programs in the sandbox probably need to call exec, and if the sandbox's owner can muck around with exec'd programs, then exec had better have no security effect. Hence a need for some kind of restricted exec. The sandbox owner would then make up own targeted capabilities if needed. But yes, targeted capabilities for kernel containers are probably orthogonal to sandboxes. > > Thanks for responding, I'll take another look at your patchset in detail. Thanks! --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-19 22:02 ` Andrew Lutomirski @ 2010-04-19 22:25 ` Serge E. Hallyn 0 siblings, 0 replies; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-19 22:25 UTC (permalink / raw) To: Andrew Lutomirski Cc: linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andrew Lutomirski (luto@mit.edu): > On Mon, Apr 19, 2010 at 5:39 PM, Serge E. Hallyn <serge@hallyn.com> wrote: > > Quoting Andrew Lutomirski (luto@mit.edu): > >> > > >> > ( I did like using new securebits as in [2], but I prefer the > >> > automatic not-raising-privs of [1] to simply -EPERM on uid/gid > >> > change and lack kof checking for privs raising of [2]. ) > >> > > >> > Really the trick will be finding a balance to satisfy those wanting > >> > this as a separate LSM, without traipsing into LSM stacking territory. > >> > >> I think that making this an LSM is absurd. Containers (and anything > >> else people want to do with namespaces or with other new features that > >> interact badly with setuid) are features that people should be able to > > > > Yes, but that's a reason to aim for targeted caps. Exec_nopriv or > > whatever is more a sandbox than a namespace feature. > > > >> use easily, and system's choice of LSM shouldn't have anything to do > >> with them. Not to mention that we're trying to *add* rights (e.g. > >> unprivileged unshare), and LSM is about *removing* rights. > > Is a targeted cap something like "process A can call setdomainname, > but only on one particular UTS namespace?" Right, only to the UTS ns in which you live. See for instance http://thread.gmane.org/gmane.linux.kernel.containers/15934 . It's how we express for instance that root in a child user_namespace has CAP_DAC_OVERRIDE over files in the container, but not over the host. -serge ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-19 21:39 ` Serge E. Hallyn 2010-04-19 22:02 ` Andrew Lutomirski @ 2010-04-20 12:37 ` Stephen Smalley 2010-04-20 14:23 ` Andrew Lutomirski 2010-04-21 1:37 ` Andrew Lutomirski 1 sibling, 2 replies; 21+ messages in thread From: Stephen Smalley @ 2010-04-20 12:37 UTC (permalink / raw) To: Serge E. Hallyn Cc: Andrew Lutomirski, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: > Quoting Andrew Lutomirski (luto@mit.edu): > > On Mon, Apr 19, 2010 at 1:26 PM, Serge E. Hallyn <serue@us.ibm.com> wrote: > > > Quoting Andy Lutomirski (luto@MIT.EDU): > > >> Every now and then, someone wants to let unprivileged programs change > > >> something about their execution environment (think unsharing namespaces, > > >> changing capabilities, disabling networking, chrooting, mounting and > > >> unmounting filesystems). ?Whether or not any of these abilities are good > > >> ideas, there's a recurring problem that gets most of these patches shot > > >> down: setuid executables. > > >> > > >> The obvious solution is to allow a process to opt out of setuid > > >> semantics and require processes to do this before using these shiny new > > >> features. [1] [2] > > >> > > >> But there's a problem with this, too: with LSMs running, execve can do > > >> pretty much anything, and even unprivileged users running unprivileged > > >> programs can have crazy security implications. ?(Take a look at a > > >> default install of Fedora. ?If you can understand the security > > >> implications of disabling setuid, you get a cookie. ?If you can figure > > >> out which programs will result in a change of security label when > > >> exec'd, you get another cookie.) > > >> > > >> So here's another solution, based on the idea that in a sane world, > > >> execve should be a lot less magical than it is. ?Any unprivileged > > >> program can open an executable, parse its headers, map it, and run it, > > >> although getting all the details right is tedious at best (and there's > > >> no good way to get all of the threading semantics right from userspace). > > >> > > >> Patch 1 adds a new syscall execve_nosecurity. ?It does an exec, but > > >> without changing any security properties. ?This means no setuid, no > > >> setgid, no LSM credential hooks (e.g. no SELinux type transitions), and > > >> no ptrace restrictions. ?(You have to have read access to the program, > > >> because disabling security stuff could allow someone to ptrace a program > > >> that they couldn't otherwise ptrace.) ?This shouldn't be particularly > > >> scary -- any process could do much the same thing with open and mmap. > > >> (You can easily shoot yourself in the foot with this syscall -- think > > >> LD_PRELOAD or running some program with insufficient error checking that > > >> can get subverted when run in the wrong security context. ?So don't do > > >> that.) > > >> > > >> Patch 2 adds a prctl that irrevocably disables execve. ?Making execve do > > >> something different that could confuse LSMs is dangerous. ?Turning the > > >> whole thing off shouldn't be. ?(Of course, with execve disabled, you can > > >> still use execve_nosecurity. ?But any program that does that should take > > >> precautions not to shoot itself in the foot.) ?(In a future revision, > > >> this should probably be a new syscall.) > > >> > > >> Sadly, programs that have opted out of execve might want to use > > >> subprocesses that in turn run execve. ?This will fail. ?So patch 3 > > >> (which is ugly, but I don't see anything fundamentally wrong with it) > > >> allows processes to set a flag that turns execve into execve_nosecurity. > > >> This flag survives exec. ?Of course, this could be used to subvert > > >> setuid programs, so you can't set this flag unless you disable ordinary > > >> exec first. > > >> > > >> [1] Unprivileged: http://lkml.org/lkml/2009/12/30/265 > > >> [2] securebit approach: http://lwn.net/Articles/368600/ > > > > > > No responses for a month after this was sent. ?Really, thanks, I do > > > appreciate the work at another approach. > > > > > > I'll be honest, I prefer option [1]. ?Though I think it's reasonable > > > to require privilege for prctl(PR_SET_NOSUID). ?Make it a separate > > > capability, and on most systems it should be safe to have a file > > > sitting in /bin with cap_set_nosuid+pe. ?If OTOH you know you have > > > legacy or poorly coded privileged programs which would not be safe > > > bc they don't verify that they have the needed privs, you just don't > > > provide the program to do prctl(PR_SET_NOSUID) for unprivileged users. > > > > Both approaches result in two kinds of exec: the normal kind that > > respects setuid, file capabilities, and LSMs, and the restricted kind > > that is supposed to be safe when programs have unshared namespaces and > > other crazy things. > > > > Eric's approach [1] adds a restricted kind of exec that ignores setuid > > but still (AFAICT) respects file capabilities > > No, please see the rest of that thread - that was an oversight. > > > and LSM transitions. I > > think this is a terrible idea for two reasons: > > 1. LSM transitions already scare me enough, and if anyone relies on > > them working in concert with setuid, then the mere act of separating > > them might break things, even if the "privileged" (by LSM) app in > > question is well-written. > > hmm... > > A good point. At least in the case of SELinux, context transitions upon execve are already disabled in the nosuid case, and Eric's patch updated the SELinux test accordingly. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 12:37 ` Stephen Smalley @ 2010-04-20 14:23 ` Andrew Lutomirski 2010-04-20 14:35 ` Serge E. Hallyn 2010-04-20 15:34 ` Stephen Smalley 2010-04-21 1:37 ` Andrew Lutomirski 1 sibling, 2 replies; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-20 14:23 UTC (permalink / raw) To: Stephen Smalley Cc: Serge E. Hallyn, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: >> Quoting Andrew Lutomirski (luto@mit.edu): >> > and LSM transitions. I >> > think this is a terrible idea for two reasons: >> > 1. LSM transitions already scare me enough, and if anyone relies on >> > them working in concert with setuid, then the mere act of separating >> > them might break things, even if the "privileged" (by LSM) app in >> > question is well-written. >> >> hmm... >> >> A good point. > > At least in the case of SELinux, context transitions upon execve are > already disabled in the nosuid case, and Eric's patch updated the > SELinux test accordingly. > True, but I think it's still asking for trouble -- other LSMs could (and almost certainly will, especially the out-of-tree ones) do something, and I think that any action at all that an LSM takes in the bprm_set_creds hook for a nosuid (or whatever it's called) process is wrong or at best misguided. Can you think of anything that an LSM should do (or even should be able to do) when a nosuid process calls exec, other than denying the request outright? With my patch, LSMs can still reject the open_exec call. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 14:23 ` Andrew Lutomirski @ 2010-04-20 14:35 ` Serge E. Hallyn 2010-04-20 15:11 ` Andrew Lutomirski 2010-04-21 21:15 ` Andrew Lutomirski 2010-04-20 15:34 ` Stephen Smalley 1 sibling, 2 replies; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-20 14:35 UTC (permalink / raw) To: Andrew Lutomirski Cc: Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andrew Lutomirski (luto@mit.edu): > On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: > >> Quoting Andrew Lutomirski (luto@mit.edu): > > >> > and LSM transitions. I > >> > think this is a terrible idea for two reasons: > >> > 1. LSM transitions already scare me enough, and if anyone relies on > >> > them working in concert with setuid, then the mere act of separating > >> > them might break things, even if the "privileged" (by LSM) app in > >> > question is well-written. > >> > >> hmm... > >> > >> A good point. > > > > At least in the case of SELinux, context transitions upon execve are > > already disabled in the nosuid case, and Eric's patch updated the > > SELinux test accordingly. > > > > True, but I think it's still asking for trouble -- other LSMs could > (and almost certainly will, especially the out-of-tree ones) do > something, and I think that any action at all that an LSM takes in the > bprm_set_creds hook for a nosuid (or whatever it's called) process is > wrong or at best misguided. I could be wrong, but I think the point is that your reasoning is correct, and that the same reasoning must apply if we're just executing a file out of an fs which has been mounted with '-o nosuid'. > Can you think of anything that an LSM should do (or even should be > able to do) when a nosuid process calls exec, other than denying the > request outright? With my patch, LSMs can still reject the open_exec > call. > > --Andy > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 14:35 ` Serge E. Hallyn @ 2010-04-20 15:11 ` Andrew Lutomirski 2010-04-21 21:15 ` Andrew Lutomirski 1 sibling, 0 replies; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-20 15:11 UTC (permalink / raw) To: Serge E. Hallyn Cc: Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, Apr 20, 2010 at 10:35 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: >> >> True, but I think it's still asking for trouble -- other LSMs could >> (and almost certainly will, especially the out-of-tree ones) do >> something, and I think that any action at all that an LSM takes in the >> bprm_set_creds hook for a nosuid (or whatever it's called) process is >> wrong or at best misguided. > > I could be wrong, but I think the point is that your reasoning is > correct, and that the same reasoning must apply if we're just > executing a file out of an fs which has been mounted with '-o nosuid'. I tend to agree, except that only root can set nosuid (presumably) and making that change will change existing behavior. Is that a problem? --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 14:35 ` Serge E. Hallyn 2010-04-20 15:11 ` Andrew Lutomirski @ 2010-04-21 21:15 ` Andrew Lutomirski 2010-04-21 22:30 ` Serge E. Hallyn 1 sibling, 1 reply; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-21 21:15 UTC (permalink / raw) To: Serge E. Hallyn Cc: Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, Apr 20, 2010 at 10:35 AM, Serge E. Hallyn <serue@us.ibm.com> wrote: > Quoting Andrew Lutomirski (luto@mit.edu): >> On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: >> >> Quoting Andrew Lutomirski (luto@mit.edu): >> >> >> > and LSM transitions. I >> >> > think this is a terrible idea for two reasons: >> >> > 1. LSM transitions already scare me enough, and if anyone relies on >> >> > them working in concert with setuid, then the mere act of separating >> >> > them might break things, even if the "privileged" (by LSM) app in >> >> > question is well-written. >> >> >> >> hmm... >> >> >> >> A good point. >> > >> > At least in the case of SELinux, context transitions upon execve are >> > already disabled in the nosuid case, and Eric's patch updated the >> > SELinux test accordingly. >> > >> >> True, but I think it's still asking for trouble -- other LSMs could >> (and almost certainly will, especially the out-of-tree ones) do >> something, and I think that any action at all that an LSM takes in the >> bprm_set_creds hook for a nosuid (or whatever it's called) process is >> wrong or at best misguided. > > I could be wrong, but I think the point is that your reasoning is > correct, and that the same reasoning must apply if we're just > executing a file out of an fs which has been mounted with '-o nosuid'. I think Stephen has just convinced me that MNT_NOSUID will never make sense -- there's odd legacy behavior in there and we'll probably never get anyone to change it. So if we give up on changing nosuid, there are a couple of things we might want to do: 1. A mode where execve acts like all filesystems are MNT_NOSUID. This sounds like a bad idea (if nothing else, it will cause apps that use selinux's exec_sid mechanism (runcon?) to silently malfunction). 2. A mode where execve (or a new syscall?) has no effect on credentials at all. This is conceptually simple and it would be great for new userspace code, especially code that wants to do something sandbox-like. For simplicity, even things like the effective and inherited capability sets should probably remain unchanged. In this mode, we'll have to disallow execing unreadable files. securebits are (almost) irrelevant. This is what my patch does. Dealing with AT_SECURE will be awkward at best, so programs that enter this mode should sanitize their own environments and should be very careful if they were setuid. (But they should do that anyway.) There are a couple of annoyances to deal with. First, there are LSM API issues, like this code in SELinux: new_tsec->osid = old_tsec->sid; /* Reset fs, key, and sock SIDs on execve. */ new_tsec->create_sid = 0; new_tsec->keycreate_sid = 0; new_tsec->sockcreate_sid = 0; and this code in commoncap: new->suid = new->fsuid = new->euid; new->sgid = new->fsgid = new->egid; I have no problem keeping these. The other annoyance is cap_effective. We could clear it on every exec (what commoncap does for non-legacy executables, I think), but that would completely break any legacy code running as root. We could set it to cap_permitted on every exec, which sounds like bad engineering even though I don't see any specific problem with it. We could also just leave it alone across exec, which might have odd side effects for programs which change their effective set and then call exec without thinking. (We could also emulate current behavior: in SECURE_NOROOT mode, clear effective, and otherwise set it depending on euid. This may be the best idea, since securebits already affects setuid(). This emulation should *not* extend to cap_permitted or cap_inheritable.) Empirically, my Fedora system is almost completely usable in this mode (with cap_effective just passing through unchanged). 3. Some intermediate mode meant for userspace code that wants to create containers or otherwise manipulate dangerous things but that still want to execute legacy code. Breaking out of containers on exec sounds like a really bad idea. Off the top of my head, I can think of a couple of possibilities: 3a. Treat all executables like they have some standard (safe) label. This could be: fP = 0, fI = everything, no setuid/setgid, and whatever LSM label makes sense (file_t or something new for selinux, perhaps). LSMs might want to add weird rules for what can exec what, but they *must not* ever increase permission. Decreasing permission (with selinux typebounds?) could be done, but I'm happy to leave that for new features that the LSM people could add if they want. 3b. Whatever the final version of Eric's patch was. Any thoughts on what we want to do? (2) seems most likely to survive bashing on LKML. --Andy P.S. Rather than targeted capabilities, why not have namespaces come with file descriptors that let you control them? sethostname and setdomainname could be ioctls on the UTS namespace fd, and a network namespace could come with two fds: one would be (or function as) a netlink socket and the other would either let you bind low-numbered ports just by possessing it or would have ioctls or something that replace bind. FS namespaces still seem scary. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-21 21:15 ` Andrew Lutomirski @ 2010-04-21 22:30 ` Serge E. Hallyn 2010-04-21 23:42 ` Andy Lutomirski 0 siblings, 1 reply; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-21 22:30 UTC (permalink / raw) To: Andrew Lutomirski Cc: Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andrew Lutomirski (luto@mit.edu): > So if we give up on changing nosuid, there are a couple of things we > might want to do: > > 1. A mode where execve acts like all filesystems are MNT_NOSUID. This > sounds like a bad idea (if nothing else, it will cause apps that use > selinux's exec_sid mechanism (runcon?) to silently malfunction). I think at this point we've lost track of exactly what we're trying to do. The goal, at least for myself and (I think) Eric, was to prevent certain changes in environment, initiated by an unprivileged user, from confusing setuid-root programs (initiated by the user). A concrete example was the proposed disablenet feature, with which an unprivileged task can remove its ability to open any new network connections. With that in mind, I think option 1 is actually the best option. I especially hate option 2 because of the resulting temptation to fudge with pE :) If you're going to fudge with pE, then IMO it MUST be done in a new securebits mode. Now actually, re-reading my msg, given our original goal, I dare say that Andrew Morgan's approach of simply returning -EPERM for any app which tries to setuid or change privileges on exec just might be the sanest way, at least to start with. -serge ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-21 22:30 ` Serge E. Hallyn @ 2010-04-21 23:42 ` Andy Lutomirski 0 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2010-04-21 23:42 UTC (permalink / raw) To: Serge E. Hallyn Cc: Andrew Lutomirski, Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Apr 21, 2010, at 6:30 PM, "Serge E. Hallyn" <serue@us.ibm.com> wrote: > Quoting Andrew Lutomirski (luto@mit.edu): >> So if we give up on changing nosuid, there are a couple of things we >> might want to do: >> >> 1. A mode where execve acts like all filesystems are MNT_NOSUID. >> This >> sounds like a bad idea (if nothing else, it will cause apps that use >> selinux's exec_sid mechanism (runcon?) to silently malfunction). > > I think at this point we've lost track of exactly what we're trying > to do. > > The goal, at least for myself and (I think) Eric, was to prevent > certain changes in environment, initiated by an unprivileged user, > from confusing setuid-root programs (initiated by the user). > > A concrete example was the proposed disablenet feature, with which > an unprivileged task can remove its ability to open any new network > connections. > > With that in mind, I think option 1 is actually the best option. I think the show-stopper for number 1 is the fact that nosuid has really strange semantics, and I'm a bit scared of making them more widespread. For example, selinux-aware apps can request a type change on exec, and nosuid causes that request to be silently ignored. This could silently break otherwise-working selinux sandboxes. Stephen doesn't want to change it... > I especially hate option 2 because of the resulting temptation to > fudge with pE :) If you're going to fudge with pE, then IMO it > MUST be done in a new securebits mode. I'll fight that fight later. (I wish the original rule had been pE' = pE except when setuid root, but it's way too late for that...) > > Now actually, re-reading my msg, given our original goal, I dare > say that Andrew Morgan's approach of simply returning -EPERM for > any app which tries to setuid or change privileges on exec just > might be the sanest way, at least to start with. > Fair enough. It'll annoy some selinux users, but maybe the selinux people will figure out how to fix it when enough users complain. I'll hack up and submit a patch series to add PR_EXEC_DISALLOW_PRIVS and allow CLONE_NEWNET when it's set. Then I'll argue with Alan Cox for a week or three, I suppose :) I think I'll arrange it so that PR_EXEC_DISALLOW_PRIVS & uid==0 && (pP != all) && !SECURE_ROOT will cause execve to always fail. nonoot && pP != 0 && !KEEPCAPS will fail as well, since it seems silly to add a special case (if you're nonroot and create an unprivileged container, drop the caps yourself). --Andy (My system has a setuid binary that does unshare(CLONE_NEWIPC), drops privs and execs it's argument. I'll be happy to get rid of it.) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 14:23 ` Andrew Lutomirski 2010-04-20 14:35 ` Serge E. Hallyn @ 2010-04-20 15:34 ` Stephen Smalley 2010-04-20 15:53 ` Andrew Lutomirski 1 sibling, 1 reply; 21+ messages in thread From: Stephen Smalley @ 2010-04-20 15:34 UTC (permalink / raw) To: Andrew Lutomirski Cc: Serge E. Hallyn, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, 2010-04-20 at 10:23 -0400, Andrew Lutomirski wrote: > On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: > >> Quoting Andrew Lutomirski (luto@mit.edu): > > >> > and LSM transitions. I > >> > think this is a terrible idea for two reasons: > >> > 1. LSM transitions already scare me enough, and if anyone relies on > >> > them working in concert with setuid, then the mere act of separating > >> > them might break things, even if the "privileged" (by LSM) app in > >> > question is well-written. > >> > >> hmm... > >> > >> A good point. > > > > At least in the case of SELinux, context transitions upon execve are > > already disabled in the nosuid case, and Eric's patch updated the > > SELinux test accordingly. > > > > True, but I think it's still asking for trouble -- other LSMs could > (and almost certainly will, especially the out-of-tree ones) do > something, and I think that any action at all that an LSM takes in the > bprm_set_creds hook for a nosuid (or whatever it's called) process is > wrong or at best misguided. > > Can you think of anything that an LSM should do (or even should be > able to do) when a nosuid process calls exec, other than denying the > request outright? With my patch, LSMs can still reject the open_exec > call. In the case where the context transition would shed permissions rather than gain permissions, it has been suggested that we shouldn't disable the transition even in the presence of nosuid. But automatically computing that for a domain transition is non-trivial, so we have the present behavior for SELinux. There also can be state updates even in the non-suid exec case, e.g. saved uids, clearing capabilities, etc. But as far as the access control goes, it should suffice to check read and execute access to the file, just as with the userland ELF loader scenario (which gets handled by the mmap hook). -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 15:34 ` Stephen Smalley @ 2010-04-20 15:53 ` Andrew Lutomirski 2010-04-21 12:34 ` Stephen Smalley 0 siblings, 1 reply; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-20 15:53 UTC (permalink / raw) To: Stephen Smalley Cc: Serge E. Hallyn, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, Apr 20, 2010 at 11:34 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> >> True, but I think it's still asking for trouble -- other LSMs could >> (and almost certainly will, especially the out-of-tree ones) do >> something, and I think that any action at all that an LSM takes in the >> bprm_set_creds hook for a nosuid (or whatever it's called) process is >> wrong or at best misguided. >> >> Can you think of anything that an LSM should do (or even should be >> able to do) when a nosuid process calls exec, other than denying the >> request outright? With my patch, LSMs can still reject the open_exec >> call. > > In the case where the context transition would shed permissions rather > than gain permissions, it has been suggested that we shouldn't disable > the transition even in the presence of nosuid. But automatically > computing that for a domain transition is non-trivial, so we have the > present behavior for SELinux. > > There also can be state updates even in the non-suid exec case, e.g. > saved uids, clearing capabilities, etc. Ah, right. In my patch, execve_nosecurity is (or will be, anyway) documented to skip all of this, and it's a new syscall, so nothing should need to be done. It doesn't allow anything that a userland ELF loader couldn't already do. (I'm not thrilled with changing the behavior of the original execve syscall, but one way or another, any nosuid mechanism will probably allow programs to exec other things without losing permissions that the admin might have expected. I don't see this is a real problem, though.) Is it even possible to purely drop permissions in SELinux? If your original type was orig_t and your new type is new_t, and if the rights granted to orig_t and new_t overlap nontrivially, then what are you supposed to do? Check both types for each hook? (Some annoying admin could even *change* the rights for orig_t or new_t after execve finishes.) > > But as far as the access control goes, it should suffice to check read > and execute access to the file, just as with the userland ELF loader > scenario (which gets handled by the mmap hook). > > -- > Stephen Smalley > National Security Agency > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 15:53 ` Andrew Lutomirski @ 2010-04-21 12:34 ` Stephen Smalley 0 siblings, 0 replies; 21+ messages in thread From: Stephen Smalley @ 2010-04-21 12:34 UTC (permalink / raw) To: Andrew Lutomirski Cc: Serge E. Hallyn, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, 2010-04-20 at 11:53 -0400, Andrew Lutomirski wrote: > On Tue, Apr 20, 2010 at 11:34 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> > >> True, but I think it's still asking for trouble -- other LSMs could > >> (and almost certainly will, especially the out-of-tree ones) do > >> something, and I think that any action at all that an LSM takes in the > >> bprm_set_creds hook for a nosuid (or whatever it's called) process is > >> wrong or at best misguided. > >> > >> Can you think of anything that an LSM should do (or even should be > >> able to do) when a nosuid process calls exec, other than denying the > >> request outright? With my patch, LSMs can still reject the open_exec > >> call. > > > > In the case where the context transition would shed permissions rather > > than gain permissions, it has been suggested that we shouldn't disable > > the transition even in the presence of nosuid. But automatically > > computing that for a domain transition is non-trivial, so we have the > > present behavior for SELinux. > > > > There also can be state updates even in the non-suid exec case, e.g. > > saved uids, clearing capabilities, etc. > > Ah, right. > > In my patch, execve_nosecurity is (or will be, anyway) documented to > skip all of this, and it's a new syscall, so nothing should need to be > done. It doesn't allow anything that a userland ELF loader couldn't > already do. (I'm not thrilled with changing the behavior of the > original execve syscall, but one way or another, any nosuid mechanism > will probably allow programs to exec other things without losing > permissions that the admin might have expected. I don't see this is a > real problem, though.) The further you deviate from existing execve semantics, the less likely your solution will work cleanly as a transparent replacement for execve for userland running in this nosuid state, and the less compelling the case for implementing execve_nosecurity in the kernel vs. just userspace emulation of it. > Is it even possible to purely drop permissions in SELinux? If your > original type was orig_t and your new type is new_t, and if the rights > granted to orig_t and new_t overlap nontrivially, then what are you > supposed to do? Check both types for each hook? (Some annoying admin > could even *change* the rights for orig_t or new_t after execve > finishes.) It has always been possible to configure policy such that one type is less privileged than its caller, and the typebounds construct introduced in more recent SELinux provides a kernel-enforced mechanism for ensuring that one type is strictly bounded by the permissions of another type. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-20 12:37 ` Stephen Smalley 2010-04-20 14:23 ` Andrew Lutomirski @ 2010-04-21 1:37 ` Andrew Lutomirski 2010-04-21 2:25 ` Serge E. Hallyn 1 sibling, 1 reply; 21+ messages in thread From: Andrew Lutomirski @ 2010-04-21 1:37 UTC (permalink / raw) To: Stephen Smalley Cc: Serge E. Hallyn, Serge E. Hallyn, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: >> Quoting Andrew Lutomirski (luto@mit.edu): >> > 1. LSM transitions already scare me enough, and if anyone relies on >> > them working in concert with setuid, then the mere act of separating >> > them might break things, even if the "privileged" (by LSM) app in >> > question is well-written. >> >> hmm... >> >> A good point. > > At least in the case of SELinux, context transitions upon execve are > already disabled in the nosuid case, and Eric's patch updated the > SELinux test accordingly. I don't see that code in current -linus, nor do I see where SELinux affects dumpability. What's supposed to happen? I'm writing a patch right now to clean this stuff up. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] Taming execve, setuid, and LSMs 2010-04-21 1:37 ` Andrew Lutomirski @ 2010-04-21 2:25 ` Serge E. Hallyn 0 siblings, 0 replies; 21+ messages in thread From: Serge E. Hallyn @ 2010-04-21 2:25 UTC (permalink / raw) To: Andrew Lutomirski Cc: Stephen Smalley, linux-kernel, linux-security-module, Eric Biederman, Andrew G. Morgan Quoting Andrew Lutomirski (luto@mit.edu): > On Tue, Apr 20, 2010 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On Mon, 2010-04-19 at 16:39 -0500, Serge E. Hallyn wrote: > >> Quoting Andrew Lutomirski (luto@mit.edu): > >> > 1. LSM transitions already scare me enough, and if anyone relies on > >> > them working in concert with setuid, then the mere act of separating > >> > them might break things, even if the "privileged" (by LSM) app in > >> > question is well-written. > >> > >> hmm... > >> > >> A good point. > > > > At least in the case of SELinux, context transitions upon execve are > > already disabled in the nosuid case, and Eric's patch updated the > > SELinux test accordingly. > > I don't see that code in current -linus, nor do I see where SELinux > affects dumpability. What's supposed to happen? I'm writing a patch > right now to clean this stuff up. check out security/selinux/hooks.c:selinux_bprm_set_creds() if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) new_tsec->sid = old_tsec->sid; I assume that's it? -serge ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2010-04-21 23:50 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-03-26 13:38 [PATCH 0/3] Taming execve, setuid, and LSMs Andy Lutomirski 2010-03-26 13:38 ` [PATCH 1/3] Add the execve_nosecurity syscall Andy Lutomirski 2010-03-26 13:38 ` [PATCH 2/3] Add PR_RESTRICT_ME to disable security-sensitive features for a process tree Andy Lutomirski 2010-03-26 13:38 ` [PATCH 3/3] Add PR_SET_FORCE_EXECVE_NOSECURITY to turn execve calls into execve_nosecurity Andy Lutomirski 2010-04-19 17:26 ` [PATCH 0/3] Taming execve, setuid, and LSMs Serge E. Hallyn 2010-04-19 21:32 ` Andrew Lutomirski 2010-04-19 21:39 ` Serge E. Hallyn 2010-04-19 22:02 ` Andrew Lutomirski 2010-04-19 22:25 ` Serge E. Hallyn 2010-04-20 12:37 ` Stephen Smalley 2010-04-20 14:23 ` Andrew Lutomirski 2010-04-20 14:35 ` Serge E. Hallyn 2010-04-20 15:11 ` Andrew Lutomirski 2010-04-21 21:15 ` Andrew Lutomirski 2010-04-21 22:30 ` Serge E. Hallyn 2010-04-21 23:42 ` Andy Lutomirski 2010-04-20 15:34 ` Stephen Smalley 2010-04-20 15:53 ` Andrew Lutomirski 2010-04-21 12:34 ` Stephen Smalley 2010-04-21 1:37 ` Andrew Lutomirski 2010-04-21 2:25 ` Serge E. Hallyn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).