[PATCH] Properly split capset_check+capset_set

From: Serge E. Hallyn
Date: Wed Dec 15 2004 - 14:56:33 EST


In kernel/capability.c, capset_check verifies whether or not the
destination process is permitted to undergo the capability change,
while capset_set (1) checks the permission of current to set the
destination process' capability and (2) performs the change.

As Stephen Smalley pointed out, the cap_capset_check code is redundant
with what is hardcoded in kernel/capability.c:sys_capset(). On the
other hand, because the security_capset_set hook is responsible for
doing both an authorization check and doing the actual change,
(particularly, in the case of a cap_set_all or cap_set_pg), when
stacking security modules, the first module may complete the
capset_set before the second module refuses permission.

The attached patch (against 2.6.10-rc3-mm1 w/ ioctl patch) removes the
redundant cap_capset_check hook and moves the security_capset_check
call to just before security_capset_set. The selinux_capset_set hook
now simply sets the capability (through its secondary), while
selinux_capset_check checks the authorization permission.

thanks,
-serge

Signed-off-by: Serge Hallyn <serue@xxxxxxxxxx>

Index: linux-2.6.10-rc3-mm1/include/linux/security.h
===================================================================
--- linux-2.6.10-rc3-mm1.orig/include/linux/security.h 2004-12-13 12:17:35.000000000 -0600
+++ linux-2.6.10-rc3-mm1/include/linux/security.h 2004-12-13 17:57:41.053436296 -0600
@@ -41,7 +41,6 @@ extern int cap_capable (struct task_stru
extern int cap_settime (struct timespec *ts, struct timezone *tz);
extern int cap_ptrace (struct task_struct *parent, struct task_struct *child);
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_bprm_set_security (struct linux_binprm *bprm);
extern void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe);
@@ -1943,7 +1942,7 @@ static inline int security_capset_check
kernel_cap_t *inheritable,
kernel_cap_t *permitted)
{
- return cap_capset_check (target, effective, inheritable, permitted);
+ return 0;
}

static inline void security_capset_set (struct task_struct *target,
Index: linux-2.6.10-rc3-mm1/kernel/capability.c
===================================================================
--- linux-2.6.10-rc3-mm1.orig/kernel/capability.c 2004-12-13 12:17:36.000000000 -0600
+++ linux-2.6.10-rc3-mm1/kernel/capability.c 2004-12-13 12:18:40.000000000 -0600
@@ -93,8 +93,12 @@ static inline void cap_set_pg(int pgrp,

do_each_task_pid(pgrp, PIDTYPE_PGID, g) {
target = g;
- while_each_thread(g, target)
- security_capset_set(target, effective, inheritable, permitted);
+ while_each_thread(g, target) {
+ if (!security_capset_check(target, effective,
+ inheritable, permitted))
+ security_capset_set(target, effective,
+ inheritable, permitted);
+ }
} while_each_task_pid(pgrp, PIDTYPE_PGID, g);
}

@@ -111,6 +115,9 @@ static inline void cap_set_all(kernel_ca
do_each_thread(g, target) {
if (target == current || target->pid == 1)
continue;
+ if (security_capset_check(target, effective, inheritable,
+ permitted))
+ continue;
security_capset_set(target, effective, inheritable, permitted);
} while_each_thread(g, target);
}
@@ -169,9 +176,6 @@ asmlinkage long sys_capset(cap_user_head

ret = -EPERM;

- if (security_capset_check(target, &effective, &inheritable, &permitted))
- goto out;
-
if (!cap_issubset(inheritable, cap_combine(target->cap_inheritable,
current->cap_permitted)))
goto out;
@@ -196,7 +200,10 @@ asmlinkage long sys_capset(cap_user_head
else /* all procs in process group */
cap_set_pg(-pid, &effective, &inheritable, &permitted);
} else {
- security_capset_set(target, &effective, &inheritable, &permitted);
+ if (!security_capset_check(target, &effective, &inheritable,
+ &permitted))
+ security_capset_set(target, &effective, &inheritable,
+ &permitted);
}

out:
Index: linux-2.6.10-rc3-mm1/security/capability.c
===================================================================
--- linux-2.6.10-rc3-mm1.orig/security/capability.c 2004-12-13 15:56:16.000000000 -0600
+++ linux-2.6.10-rc3-mm1/security/capability.c 2004-12-13 17:57:41.084431584 -0600
@@ -27,7 +27,6 @@
static struct security_operations capability_ops = {
.ptrace = cap_ptrace,
.capget = cap_capget,
- .capset_check = cap_capset_check,
.capset_set = cap_capset_set,
.capable = cap_capable,
.settime = cap_settime,
Index: linux-2.6.10-rc3-mm1/security/commoncap.c
===================================================================
--- linux-2.6.10-rc3-mm1.orig/security/commoncap.c 2004-12-13 12:17:36.000000000 -0600
+++ linux-2.6.10-rc3-mm1/security/commoncap.c 2004-12-13 12:18:40.000000000 -0600
@@ -75,32 +75,6 @@ int cap_capget (struct task_struct *targ
return 0;
}

-int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
- kernel_cap_t *inheritable, kernel_cap_t *permitted)
-{
- /* Derived from kernel/capability.c:sys_capset. */
- /* verify restrictions on target's new Inheritable set */
- if (!cap_issubset (*inheritable,
- cap_combine (target->cap_inheritable,
- current->cap_permitted))) {
- return -EPERM;
- }
-
- /* verify restrictions on target's new Permitted set */
- if (!cap_issubset (*permitted,
- cap_combine (target->cap_permitted,
- current->cap_permitted))) {
- return -EPERM;
- }
-
- /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
- if (!cap_issubset (*effective, *permitted)) {
- return -EPERM;
- }
-
- return 0;
-}
-
void cap_capset_set (struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
@@ -406,7 +380,6 @@ EXPORT_SYMBOL(cap_capable);
EXPORT_SYMBOL(cap_settime);
EXPORT_SYMBOL(cap_ptrace);
EXPORT_SYMBOL(cap_capget);
-EXPORT_SYMBOL(cap_capset_check);
EXPORT_SYMBOL(cap_capset_set);
EXPORT_SYMBOL(cap_bprm_set_security);
EXPORT_SYMBOL(cap_bprm_apply_creds);
Index: linux-2.6.10-rc3-mm1/security/root_plug.c
===================================================================
--- linux-2.6.10-rc3-mm1.orig/security/root_plug.c 2004-10-18 16:55:28.000000000 -0500
+++ linux-2.6.10-rc3-mm1/security/root_plug.c 2004-12-13 17:57:41.085431432 -0600
@@ -86,7 +86,6 @@ static struct security_operations rootpl
/* Use the capability functions for some of the hooks */
.ptrace = cap_ptrace,
.capget = cap_capget,
- .capset_check = cap_capset_check,
.capset_set = cap_capset_set,
.capable = cap_capable,

Index: linux-2.6.10-rc3-mm1/security/selinux/hooks.c
===================================================================
--- linux-2.6.10-rc3-mm1.orig/security/selinux/hooks.c 2004-12-13 12:17:36.000000000 -0600
+++ linux-2.6.10-rc3-mm1/security/selinux/hooks.c 2004-12-13 17:57:38.456831040 -0600
@@ -1393,24 +1393,12 @@ static int selinux_capget(struct task_st
static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
- int error;
-
- error = secondary_ops->capset_check(target, effective, inheritable, permitted);
- if (error)
- return error;
-
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)
{
- int error;
-
- error = task_has_perm(current, target, PROCESS__SETCAP);
- if (error)
- return;
-
secondary_ops->capset_set(target, effective, inheritable, permitted);
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/