Quoting Stephen Smalley (sds@private):
...
> capset_set than for other hooks. Possibly reflects a more basic problem
> in the capset code and hooks; capset_check is supposed to handle
> security checking while capset_set is supposed to just update state, but
> we often don't have the specific target task in capset_check, so
> capset_set has to apply a check too for completeness. Also, looking at
> the core kernel, it looks like capset_check isn't serving a useful
> purpose anymore, as the core kernel is applying the same checks
> directly. capget may not be a real issue, as it is only getting the
> bits into a kernel data structure and will not copy to userspace if we
> returned an error. Possibly need two separate hooks at the same points
> where capset_set is presently being invoked to cleanly separate
> permission checking hooks from state update.
So something like the following patch might be appropriate? It removes
the redundant capset_check code, moving capset_check to right before each
call to capset_set, and moves the selinux_capset_set authentication code
into capset_check.
thanks,
-serge
Index: linux-2.6.10-rc3-bk2/include/linux/security.h
===================================================================
--- linux-2.6.10-rc3-bk2.orig/include/linux/security.h 2004-12-07 19:35:52.249237440 -0600
+++ linux-2.6.10-rc3-bk2/include/linux/security.h 2004-12-07 19:42:44.489567384 -0600
@@ -41,7 +41,6 @@
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);
@@ -1932,7 +1931,7 @@
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-bk2/kernel/capability.c
===================================================================
--- linux-2.6.10-rc3-bk2.orig/kernel/capability.c 2004-12-07 19:35:52.659175120 -0600
+++ linux-2.6.10-rc3-bk2/kernel/capability.c 2004-12-07 20:30:19.482542480 -0600
@@ -93,8 +93,12 @@
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 @@
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 @@
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 @@
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-bk2/security/commoncap.c
===================================================================
--- linux-2.6.10-rc3-bk2.orig/security/commoncap.c 2004-12-07 19:35:54.010969616 -0600
+++ linux-2.6.10-rc3-bk2/security/commoncap.c 2004-12-07 19:42:13.580266312 -0600
@@ -75,32 +75,6 @@
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)
{
@@ -398,7 +372,6 @@
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-bk2/security/root_plug.c
===================================================================
--- linux-2.6.10-rc3-bk2.orig/security/root_plug.c 2004-10-18 16:55:28.000000000 -0500
+++ linux-2.6.10-rc3-bk2/security/root_plug.c 2004-12-07 19:45:22.286578592 -0600
@@ -86,7 +86,6 @@
/* 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-bk2/security/selinux/hooks.c
===================================================================
--- linux-2.6.10-rc3-bk2.orig/security/selinux/hooks.c 2004-12-07 19:35:54.051963384 -0600
+++ linux-2.6.10-rc3-bk2/security/selinux/hooks.c 2004-12-07 19:44:41.366799344 -0600
@@ -1391,24 +1391,12 @@
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);
}
This archive was generated by hypermail 2.1.3 : Wed Dec 08 2004 - 05:27:34 PST