From ddb4a1442def2a78b91a85b4251fb712ef23662b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 18 Jul 2017 15:25:23 -0700 Subject: exec: Rename bprm->cred_prepared to called_set_creds The cred_prepared bprm flag has a misleading name. It has nothing to do with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has been called. Rename this flag and improve its comment. Cc: David Howells Cc: Stephen Smalley Cc: Casey Schaufler Signed-off-by: Kees Cook Acked-by: John Johansen Acked-by: James Morris Acked-by: Paul Moore Acked-by: Serge Hallyn --- include/linux/binfmts.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 3ae9013eeaaa..9023e1d2d5cd 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -25,8 +25,12 @@ struct linux_binprm { struct mm_struct *mm; unsigned long p; /* current top of mem */ unsigned int - cred_prepared:1,/* true if creds already prepared (multiple - * preps happen for interpreters) */ + /* + * True after the bprm_set_creds hook has been called once + * (multiple calls can be made via prepare_binprm() for + * binfmt_script/misc). + */ + called_set_creds:1, cap_effective:1;/* true if has elevated effective capabilities, * false if not; except for init which inherits * its parent's caps anyway */ -- cgit From c425e189ffd7720c881fe9ccd7143cea577f6d03 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 18 Jul 2017 15:25:22 -0700 Subject: binfmt: Introduce secureexec flag The bprm_secureexec hook can be moved earlier. Right now, it is called during create_elf_tables(), via load_binary(), via search_binary_handler(), via exec_binprm(). Nearly all (see exception below) state used by bprm_secureexec is created during the bprm_set_creds hook, called from prepare_binprm(). For all LSMs (except commoncaps described next), only the first execution of bprm_set_creds takes any effect (they all check bprm->called_set_creds which prepare_binprm() sets after the first call to the bprm_set_creds hook). However, all these LSMs also only do anything with bprm_secureexec when they detected a secure state during their first run of bprm_set_creds. Therefore, it is functionally identical to move the detection into bprm_set_creds, since the results from secureexec here only need to be based on the first call to the LSM's bprm_set_creds hook. The single exception is that the commoncaps secureexec hook also examines euid/uid and egid/gid differences which are controlled by bprm_fill_uid(), via prepare_binprm(), which can be called multiple times (e.g. binfmt_script, binfmt_misc), and may clear the euid/egid for the final load (i.e. the script interpreter). However, while commoncaps specifically ignores bprm->cred_prepared, and runs its bprm_set_creds hook each time prepare_binprm() may get called, it needs to base the secureexec decision on the final call to bprm_set_creds. As a result, it will need special handling. To begin this refactoring, this adds the secureexec flag to the bprm struct, and calls the secureexec hook during setup_new_exec(). This is safe since all the cred work is finished (and past the point of no return). This explicit call will be removed in later patches once the hook has been removed. Cc: David Howells Signed-off-by: Kees Cook Reviewed-by: John Johansen Acked-by: Serge Hallyn Reviewed-by: James Morris --- include/linux/binfmts.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 9023e1d2d5cd..16838ba7ee75 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -31,9 +31,15 @@ struct linux_binprm { * binfmt_script/misc). */ called_set_creds:1, - cap_effective:1;/* true if has elevated effective capabilities, + cap_effective:1,/* true if has elevated effective capabilities, * false if not; except for init which inherits * its parent's caps anyway */ + /* + * Set by bprm_set_creds hook to indicate a privilege-gaining + * exec has happened. Used to sanitize execution environment + * and to set AT_SECURE auxv for glibc. + */ + secureexec:1; #ifdef __alpha__ unsigned int taso:1; #endif -- cgit From 46d98eb4e1d2bc225f661879e0e157a952107598 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 18 Jul 2017 15:25:27 -0700 Subject: commoncap: Refactor to remove bprm_secureexec hook The commoncap implementation of the bprm_secureexec hook is the only LSM that depends on the final call to its bprm_set_creds hook (since it may be called for multiple files, it ignores bprm->called_set_creds). As a result, it cannot safely _clear_ bprm->secureexec since other LSMs may have set it. Instead, remove the bprm_secureexec hook by introducing a new flag to bprm specific to commoncap: cap_elevated. This is similar to cap_effective, but that is used for a specific subset of elevated privileges, and exists solely to track state from bprm_set_creds to bprm_secureexec. As such, it will be removed in the next patch. Here, set the new bprm->cap_elevated flag when setuid/setgid has happened from bprm_fill_uid() or fscapabilities have been prepared. This temporarily moves the bprm_secureexec hook to a static inline. The helper will be removed in the next patch; this makes the step easier to review and bisect, since this does not introduce any changes to inputs nor outputs to the "elevated privileges" calculation. The new flag is merged with the bprm->secureexec flag in setup_new_exec() since this marks the end of any further prepare_binprm() calls. Cc: Andy Lutomirski Signed-off-by: Kees Cook Reviewed-by: Andy Lutomirski Acked-by: James Morris Acked-by: Serge Hallyn --- include/linux/binfmts.h | 7 +++++++ include/linux/security.h | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 16838ba7ee75..213c61fa3780 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -34,6 +34,13 @@ struct linux_binprm { cap_effective:1,/* true if has elevated effective capabilities, * false if not; except for init which inherits * its parent's caps anyway */ + /* + * True if most recent call to the commoncaps bprm_set_creds + * hook (due to multiple prepare_binprm() calls from the + * binfmt_script/misc handlers) resulted in elevated + * privileges. + */ + cap_elevated:1, /* * Set by bprm_set_creds hook to indicate a privilege-gaining * exec has happened. Used to sanitize execution environment diff --git a/include/linux/security.h b/include/linux/security.h index b6ea1dc9cc9d..f89832ccdf55 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -85,7 +85,6 @@ extern int cap_capset(struct cred *new, const struct cred *old, const kernel_cap_t *inheritable, const kernel_cap_t *permitted); extern int cap_bprm_set_creds(struct linux_binprm *bprm); -extern int cap_bprm_secureexec(struct linux_binprm *bprm); extern int cap_inode_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags); extern int cap_inode_removexattr(struct dentry *dentry, const char *name); @@ -543,7 +542,7 @@ static inline void security_bprm_committed_creds(struct linux_binprm *bprm) static inline int security_bprm_secureexec(struct linux_binprm *bprm) { - return cap_bprm_secureexec(bprm); + return 0; } static inline int security_sb_alloc(struct super_block *sb) -- cgit From ee67ae7ef6ff499137292ac8a9dfe86096796283 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 18 Jul 2017 15:25:28 -0700 Subject: commoncap: Move cap_elevated calculation into bprm_set_creds Instead of a separate function, open-code the cap_elevated test, which lets us entirely remove bprm->cap_effective (to use the local "effective" variable instead), and more accurately examine euid/egid changes via the existing local "is_setid". The following LTP tests were run to validate the changes: # ./runltp -f syscalls -s cap # ./runltp -f securebits # ./runltp -f cap_bounds # ./runltp -f filecaps All kernel selftests for capabilities and exec continue to pass as well. Signed-off-by: Kees Cook Reviewed-by: James Morris Acked-by: Serge Hallyn Reviewed-by: Andy Lutomirski --- include/linux/binfmts.h | 3 --- 1 file changed, 3 deletions(-) (limited to 'include') diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 213c61fa3780..fb44d6180ca0 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -31,9 +31,6 @@ struct linux_binprm { * binfmt_script/misc). */ called_set_creds:1, - cap_effective:1,/* true if has elevated effective capabilities, - * false if not; except for init which inherits - * its parent's caps anyway */ /* * True if most recent call to the commoncaps bprm_set_creds * hook (due to multiple prepare_binprm() calls from the -- cgit From 2af622802696e1dbe28d81c8ea6355dc30800396 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 18 Jul 2017 15:25:29 -0700 Subject: LSM: drop bprm_secureexec hook This removes the bprm_secureexec hook since the logic has been folded into the bprm_set_creds hook for all LSMs now. Cc: Eric W. Biederman Signed-off-by: Kees Cook Reviewed-by: John Johansen Acked-by: James Morris Acked-by: Serge Hallyn --- include/linux/lsm_hooks.h | 14 +++++--------- include/linux/security.h | 6 ------ 2 files changed, 5 insertions(+), 15 deletions(-) (limited to 'include') diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 3a90febadbe2..d1c7bef25691 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -43,7 +43,11 @@ * interpreters. The hook can tell whether it has already been called by * checking to see if @bprm->security is non-NULL. If so, then the hook * may decide either to retain the security information saved earlier or - * to replace it. + * to replace it. The hook must set @bprm->secureexec to 1 if a "secure + * exec" has happened as a result of this hook call. The flag is used to + * indicate the need for a sanitized execution environment, and is also + * passed in the ELF auxiliary table on the initial stack to indicate + * whether libc should enable secure mode. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. * @bprm_check_security: @@ -71,12 +75,6 @@ * linux_binprm structure. This hook is a good place to perform state * changes on the process such as clearing out non-inheritable signal * state. This is called immediately after commit_creds(). - * @bprm_secureexec: - * Return a boolean value (0 or 1) indicating whether a "secure exec" - * is required. The flag is passed in the auxiliary table - * on the initial stack to the ELF interpreter to indicate whether libc - * should enable secure mode. - * @bprm contains the linux_binprm structure. * * Security hooks for filesystem operations. * @@ -1388,7 +1386,6 @@ union security_list_options { int (*bprm_set_creds)(struct linux_binprm *bprm); int (*bprm_check_security)(struct linux_binprm *bprm); - int (*bprm_secureexec)(struct linux_binprm *bprm); void (*bprm_committing_creds)(struct linux_binprm *bprm); void (*bprm_committed_creds)(struct linux_binprm *bprm); @@ -1710,7 +1707,6 @@ struct security_hook_heads { struct list_head vm_enough_memory; struct list_head bprm_set_creds; struct list_head bprm_check_security; - struct list_head bprm_secureexec; struct list_head bprm_committing_creds; struct list_head bprm_committed_creds; struct list_head sb_alloc_security; diff --git a/include/linux/security.h b/include/linux/security.h index f89832ccdf55..974bb9b0996c 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -231,7 +231,6 @@ int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); -int security_bprm_secureexec(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_copy_data(char *orig, char *copy); @@ -540,11 +539,6 @@ static inline void security_bprm_committed_creds(struct linux_binprm *bprm) { } -static inline int security_bprm_secureexec(struct linux_binprm *bprm) -{ - return 0; -} - static inline int security_sb_alloc(struct super_block *sb) { return 0; -- cgit