linux-security-module July 2008 archive
Main Archive Page > Month Archives  > linux-security-module archives
linux-security-module: [PATCH 09/28] CRED: Neuter sys_capset() [

[PATCH 09/28] CRED: Neuter sys_capset() [ver #5]

From: David Howells <dhowells_at_nospam>
Date: Fri Jul 04 2008 - 15:24:05 GMT
To: viro@ftp.linux.org.uk, hch@infradead.org, sds@tycho.nsa.gov, casey@schaufler-ca.com, morgan@kernel.org, jmorris@namei.org


Take away the ability for sys_capset() to affect processes other than current.

This means that current will not need to lock its own credentials when reading them against interference by other processes.

This has effectively been the case for a while anyway, since:

 (1) Without LSM enabled, sys_capset() is disallowed.

 (2) With file-based capabilities, sys_capset() is neutered.

Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Serge Hallyn <serue@us.ibm.com> Acked-by: James Morris <jmorris@namei.org> --- fs/open.c | 12 -- include/linux/security.h | 60 +++++----- kernel/capability.c | 265 ++++++++-------------------------------------- security/commoncap.c | 25 +--- security/dummy.c | 12 +- security/security.c | 15 +-- security/selinux/hooks.c | 12 +- 7 files changed, 105 insertions(+), 296 deletions(-) diff --git a/fs/open.c b/fs/open.c index 24b4a11..6694803 100644 --- a/fs/open.c +++ b/fs/open.c
@@ -439,17 +439,7 @@ asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode)
current->fsgid = current->gid; if (!issecure(SECURE_NO_SETUID_FIXUP)) { - /* - * Clear the capabilities if we switch to a non-root user - */ -#ifndef CONFIG_SECURITY_FILE_CAPABILITIES - /* - * FIXME: There is a race here against sys_capset. The - * capabilities can change yet we will restore the old - * value below. We should hold task_capabilities_lock, - * but we cannot because user_path_walk can sleep. - */ -#endif /* ndef CONFIG_SECURITY_FILE_CAPABILITIES */ + /* Clear the capabilities if we switch to a non-root user */ if (current->uid) cap_set_effective(&__cap_empty_set, &old_cap); else diff --git a/include/linux/security.h b/include/linux/security.h index cfe2a6a..f5ff1f0 100644 --- a/include/linux/security.h +++ b/include/linux/security.h
@@ -49,8 +49,14 @@ extern int cap_settime(struct timespec *ts, struct timezone *tz);
extern int cap_ptrace_may_access(struct task_struct *child, unsigned int mode); extern int cap_ptrace_traceme(struct task_struct *parent); extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); -extern int cap_capset_check(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); -extern void cap_capset_set(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); +extern int cap_capset_check(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); +extern void cap_capset_set(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); extern int cap_bprm_set_security(struct linux_binprm *bprm); extern void cap_bprm_apply_creds(struct linux_binprm *bprm, int unsafe); extern int cap_bprm_secureexec(struct linux_binprm *bprm);
@@ -1193,23 +1199,15 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @capset_check: * Check permission before setting the @effective, @inheritable, and * @permitted capability sets for the @target process. - * Caveat: @target is also set to current if a set of processes is - * specified (i.e. all processes other than current and init or a - * particular process group). Hence, the capset_set hook may need to - * revalidate permission to the actual target process. - * @target contains the task_struct structure for target process. + * @target contains the task_struct structure for the current process. * @effective contains the effective capability set. * @inheritable contains the inheritable capability set. * @permitted contains the permitted capability set. * Return 0 if permission is granted. * @capset_set: * Set the @effective, @inheritable, and @permitted capability sets for - * the @target process. Since capset_check cannot always check permission - * to the real @target process, this hook may also perform permission - * checking to determine if the current process is allowed to set the - * capability sets of the @target process. However, this hook has no way - * of returning an error due to the structure of the sys_capset code. - * @target contains the task_struct structure for target process. + * the current process. + * @target contains the task_struct structure for current process. * @effective contains the effective capability set. * @inheritable contains the inheritable capability set. * @permitted contains the permitted capability set.
@@ -1310,13 +1308,13 @@ struct security_operations {
kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); int (*capset_check) (struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted); + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); void (*capset_set) (struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted); + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); int (*capable) (struct task_struct *tsk, int cap); int (*acct) (struct file *file); int (*sysctl) (struct ctl_table *table, int op);
@@ -1590,13 +1588,13 @@ int security_capget(struct task_struct *target,
kernel_cap_t *inheritable, kernel_cap_t *permitted); int security_capset_check(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted); + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); void security_capset_set(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted); + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted); int security_capable(struct task_struct *tsk, int cap); int security_acct(struct file *file); int security_sysctl(struct ctl_table *table, int op);
@@ -1786,17 +1784,17 @@ static inline int security_capget(struct task_struct *target,
} static inline int security_capset_check(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { return cap_capset_check(target, effective, inheritable, permitted); } static inline void security_capset_set(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { cap_capset_set(target, effective, inheritable, permitted); } diff --git a/kernel/capability.c b/kernel/capability.c index c930e7c..456cc6c 100644 --- a/kernel/capability.c +++ b/kernel/capability.c
@@ -115,160 +115,6 @@ static int cap_validate_magic(cap_user_header_t header, unsigned *tocopy)
return 0; } -#ifndef CONFIG_SECURITY_FILE_CAPABILITIES - -/* - * Without filesystem capability support, we nominally support one process - * setting the capabilities of another - */ -static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp, - kernel_cap_t *pIp, kernel_cap_t *pPp) -{ - struct task_struct *target; - int ret; - - spin_lock(&task_capability_lock); - read_lock(&tasklist_lock); - - if (pid && pid != task_pid_vnr(current)) { - target = find_task_by_vpid(pid); - if (!target) { - ret = -ESRCH; - goto out; - } - } else - target = current; - - ret = security_capget(target, pEp, pIp, pPp); - -out: - read_unlock(&tasklist_lock); - spin_unlock(&task_capability_lock); - - return ret; -} - -/* - * cap_set_pg - set capabilities for all processes in a given process - * group. We call this holding task_capability_lock and tasklist_lock. - */ -static inline int cap_set_pg(int pgrp_nr, kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) -{ - struct task_struct *g, *target; - int ret = -EPERM; - int found = 0; - struct pid *pgrp; - - spin_lock(&task_capability_lock); - read_lock(&tasklist_lock); - - pgrp = find_vpid(pgrp_nr); - do_each_pid_task(pgrp, PIDTYPE_PGID, g) { - target = g; - while_each_thread(g, target) { - if (!security_capset_check(target, effective, - inheritable, permitted)) { - security_capset_set(target, effective, - inheritable, permitted); - ret = 0; - } - found = 1; - } - } while_each_pid_task(pgrp, PIDTYPE_PGID, g); - - read_unlock(&tasklist_lock); - spin_unlock(&task_capability_lock); - - if (!found) - ret = 0; - return ret; -} - -/* - * cap_set_all - set capabilities for all processes other than init - * and self. We call this holding task_capability_lock and tasklist_lock. - */ -static inline int cap_set_all(kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) -{ - struct task_struct *g, *target; - int ret = -EPERM; - int found = 0; - - spin_lock(&task_capability_lock); - read_lock(&tasklist_lock); - - do_each_thread(g, target) { - if (target == current - || is_container_init(target->group_leader)) - continue; - found = 1; - if (security_capset_check(target, effective, inheritable, - permitted)) - continue; - ret = 0; - security_capset_set(target, effective, inheritable, permitted); - } while_each_thread(g, target); - - read_unlock(&tasklist_lock); - spin_unlock(&task_capability_lock); - - if (!found) - ret = 0; - - return ret; -} - -/* - * Given the target pid does not refer to the current process we - * need more elaborate support... (This support is not present when - * filesystem capabilities are configured.) - */ -static inline int do_sys_capset_other_tasks(pid_t pid, kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) -{ - struct task_struct *target; - int ret; - - if (!capable(CAP_SETPCAP)) - return -EPERM; - - if (pid == -1) /* all procs other than current and init */ - return cap_set_all(effective, inheritable, permitted); - - else if (pid < 0) /* all procs in process group */ - return cap_set_pg(-pid, effective, inheritable, permitted); - - /* target != current */ - spin_lock(&task_capability_lock); - read_lock(&tasklist_lock); - - target = find_task_by_vpid(pid); - if (!target) - ret = -ESRCH; - else { - ret = security_capset_check(target, effective, inheritable, - permitted); - - /* having verified that the proposed changes are legal, - we now put them into effect. */ - if (!ret) - security_capset_set(target, effective, inheritable, - permitted); - } - - read_unlock(&tasklist_lock); - spin_unlock(&task_capability_lock); - - return ret; -} - -#else /* ie., def CONFIG_SECURITY_FILE_CAPABILITIES */ - /* * If we have configured with filesystem capability support, then the * only thing that can change the capabilities of the current process
@@ -302,41 +148,6 @@ static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
return ret; } -/* - * With filesystem capability support configured, the kernel does not - * permit the changing of capabilities in one process by another - * process. (CAP_SETPCAP has much less broad semantics when configured - * this way.) - */ -static inline int do_sys_capset_other_tasks(pid_t pid, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) -{ - return -EPERM; -} - -#endif /* ie., ndef CONFIG_SECURITY_FILE_CAPABILITIES */ - -/* - * Atomically modify the effective capabilities returning the original - * value. No permission check is performed here - it is assumed that the - * caller is permitted to set the desired effective capabilities. - */ -void cap_set_effective(const kernel_cap_t *pE_new, - kernel_cap_t *_pE_old) -{ - spin_lock(&task_capability_lock); - - if (_pE_old) - *_pE_old = current->cap_effective; - current->cap_effective = *pE_new; - - spin_unlock(&task_capability_lock); -} - -EXPORT_SYMBOL(cap_set_effective); - /** * sys_capget - get the capabilities of a given process. * @header: pointer to struct that contains capability version and
@@ -410,16 +221,14 @@ asmlinkage long sys_capget(cap_user_header_t header, cap_user_data_t dataptr)
* @data: pointer to struct that contains the effective, permitted, * and inheritable capabilities * - * Set capabilities for a given process, all processes, or all - * processes in a given process group. + * Set capabilities for the current process only. The ability to any other + * process(es) has been deprecated and removed. * * The restrictions on setting capabilities are specified as: * - * [pid is for the 'target' task. 'current' is the calling task.] - * - * I: any raised capabilities must be a subset of the (old current) permitted - * P: any raised capabilities must be a subset of the (old current) permitted - * E: must be set to a subset of (new target) permitted + * I: any raised capabilities must be a subset of the old permitted + * P: any raised capabilities must be a subset of the old permitted + * E: must be set to a subset of new permitted * * Returns 0 on success and < 0 on error. */
@@ -428,6 +237,7 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
struct __user_cap_data_struct kdata[_KERNEL_CAPABILITY_U32S]; unsigned i, tocopy; kernel_cap_t inheritable, permitted, effective; + struct task_struct *target = current; int ret; pid_t pid;
@@ -438,6 +248,17 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
if (get_user(pid, &header->pid)) return -EFAULT; + /* may only affect current now */ + if (pid != 0) { + read_lock(&tasklist_lock); + ret = 0; + if (pid != task_pid_vnr(target)) + ret = -EPERM; + read_unlock(&tasklist_lock); + if (ret < 0) + return ret; + } + if (copy_from_user(&kdata, data, tocopy * sizeof(struct __user_cap_data_struct))) { return -EFAULT;
@@ -455,35 +276,39 @@ asmlinkage long sys_capset(cap_user_header_t header, const cap_user_data_t data)
i++; } - if (pid && (pid != task_pid_vnr(current))) - ret = do_sys_capset_other_tasks(pid, &effective, &inheritable, - &permitted); - else { - /* - * This lock is required even when filesystem - * capability support is configured - it protects the - * sys_capget() call from returning incorrect data in - * the case that the targeted process is not the - * current one. - */ - spin_lock(&task_capability_lock); - - ret = security_capset_check(current, &effective, &inheritable, - &permitted); - /* - * Having verified that the proposed changes are - * legal, we now put them into effect. - */ - if (!ret) - security_capset_set(current, &effective, &inheritable, - &permitted); - spin_unlock(&task_capability_lock); - } + spin_lock(&task_capability_lock); + read_lock(&tasklist_lock); + ret = security_capset_check(target, + &effective, &inheritable, &permitted); + if (!ret) + security_capset_set(target, + &effective, &inheritable, &permitted); + read_unlock(&tasklist_lock); + spin_unlock(&task_capability_lock); return ret; } +/* + * Atomically modify the effective capabilities returning the original + * value. No permission check is performed here - it is assumed that the + * caller is permitted to set the desired effective capabilities. + */ +void cap_set_effective(const kernel_cap_t *pE_new, + kernel_cap_t *_pE_old) +{ + spin_lock(&task_capability_lock); + + if (_pE_old) + *_pE_old = current->cap_effective; + current->cap_effective = *pE_new; + + spin_unlock(&task_capability_lock); +} + +EXPORT_SYMBOL(cap_set_effective); + /** * capable - Determine if the current task has a superior capability in effect * @cap: The capability to be tested for diff --git a/security/commoncap.c b/security/commoncap.c index aae8eb8..9dcee2f 100644 --- a/security/commoncap.c +++ b/security/commoncap.c
@@ -95,15 +95,6 @@ int cap_capget (struct task_struct *target, kernel_cap_t *effective,
#ifdef CONFIG_SECURITY_FILE_CAPABILITIES -static inline int cap_block_setpcap(struct task_struct *target) -{ - /* - * No support for remote process capability manipulation with - * filesystem capability support. - */ - return (target != current); -} - static inline int cap_inh_is_capped(void) { /*
@@ -118,7 +109,6 @@ static inline int cap_limit_ptraced_target(void) { return 1; }
#else /* ie., ndef CONFIG_SECURITY_FILE_CAPABILITIES */ -static inline int cap_block_setpcap(struct task_struct *t) { return 0; } static inline int cap_inh_is_capped(void) { return 1; } static inline int cap_limit_ptraced_target(void) {
@@ -127,12 +117,11 @@ static inline int cap_limit_ptraced_target(void)
#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */ -int cap_capset_check (struct task_struct *target, kernel_cap_t *effective, - kernel_cap_t *inheritable, kernel_cap_t *permitted) +int cap_capset_check(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { - if (cap_block_setpcap(target)) { - return -EPERM; - } if (cap_inh_is_capped() && !cap_issubset(*inheritable, cap_combine(target->cap_inheritable,
@@ -162,8 +151,10 @@ int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
return 0; } -void cap_capset_set (struct task_struct *target, kernel_cap_t *effective, - kernel_cap_t *inheritable, kernel_cap_t *permitted) +void cap_capset_set(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { target->cap_effective = *effective; target->cap_inheritable = *inheritable; diff --git a/security/dummy.c b/security/dummy.c index 9be5003..ba81756 100644 --- a/security/dummy.c +++ b/security/dummy.c
@@ -61,17 +61,17 @@ static int dummy_capget (struct task_struct *target, kernel_cap_t * effective,
} static int dummy_capset_check (struct task_struct *target, - kernel_cap_t * effective, - kernel_cap_t * inheritable, - kernel_cap_t * permitted) + const kernel_cap_t * effective, + const kernel_cap_t * inheritable, + const kernel_cap_t * permitted) { return -EPERM; } static void dummy_capset_set (struct task_struct *target, - kernel_cap_t * effective, - kernel_cap_t * inheritable, - kernel_cap_t * permitted) + const kernel_cap_t * effective, + const kernel_cap_t * inheritable, + const kernel_cap_t * permitted) { return; } diff --git a/security/security.c b/security/security.c index d0e2342..29268ed 100644 --- a/security/security.c +++ b/security/security.c
@@ -180,17 +180,18 @@ int security_capget(struct task_struct *target,
} int security_capset_check(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { - return security_ops->capset_check(target, effective, inheritable, permitted); + return security_ops->capset_check(target, + effective, inheritable, permitted); } void security_capset_set(struct task_struct *target, - kernel_cap_t *effective, - kernel_cap_t *inheritable, - kernel_cap_t *permitted) + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { security_ops->capset_set(target, effective, inheritable, permitted); } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f21904a..5f06e6b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c
@@ -1738,8 +1738,10 @@ static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
return secondary_ops->capget(target, effective, inheritable, permitted); } -static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effective, - kernel_cap_t *inheritable, kernel_cap_t *permitted) +static int selinux_capset_check(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { int error;
@@ -1750,8 +1752,10 @@ static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effect
return task_has_perm(current, target, PROCESS__SETCAP); } -static void selinux_capset_set(struct task_struct *target, kernel_cap_t *effective, - kernel_cap_t *inheritable, kernel_cap_t *permitted) +static void selinux_capset_set(struct task_struct *target, + const kernel_cap_t *effective, + const kernel_cap_t *inheritable, + const kernel_cap_t *permitted) { secondary_ops->capset_set(target, effective, inheritable, permitted); } -- 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