2008-08-26 18:58:06

by Andreas Gruenbacher

[permalink] [raw]
Subject: [patch] file capabilities: Add no_file_caps switch

Hello,

here is a patch allowing to disable file capabilities via a kernel command
line option (once compiled in with CONFIG_SECURITY_FILE_CAPABILITIES).

We would like to ship our next round of products with file capabilities
compiled in, yet we feel that too many system utilities are still file
capabilitiy unaware, and so we would like to turn them off by default
initially. File capabilities can be used to grant privileges to binaries
which otherwise look "harmless", which is a security risk until utilities
like rpm have learned how to install and verify capabilities, etc.

Any objections?
Thanks.

Signed-off-by: Andreas Gruenbacher <[email protected]>

---
include/linux/init_task.h | 13 ------
kernel/capability.c | 79 +++++++++++++--------------------------
kernel/sched.c | 9 ++++
security/commoncap.c | 93 +++++++++++++++++++++++-----------------------
4 files changed, 85 insertions(+), 109 deletions(-)

Index: b/include/linux/init_task.h
===================================================================
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -102,17 +102,6 @@ extern struct group_info init_groups;
#define INIT_IDS
#endif

-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-/*
- * Because of the reduced scope of CAP_SETPCAP when filesystem
- * capabilities are in effect, it is safe to allow CAP_SETPCAP to
- * be available in the default configuration.
- */
-# define CAP_INIT_BSET CAP_FULL_SET
-#else
-# define CAP_INIT_BSET CAP_INIT_EFF_SET
-#endif
-
/*
* INIT_TASK is used to set up the first task table, touch at
* your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -151,7 +140,7 @@ extern struct group_info init_groups;
.cap_effective = CAP_INIT_EFF_SET, \
.cap_inheritable = CAP_INIT_INH_SET, \
.cap_permitted = CAP_FULL_SET, \
- .cap_bset = CAP_INIT_BSET, \
+ .cap_bset = CAP_INIT_EFF_SET, /* also see sched_init() */ \
.securebits = SECUREBITS_DEFAULT, \
.user = INIT_USER, \
.comm = "swapper", \
Index: b/kernel/capability.c
===================================================================
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -33,6 +33,19 @@ EXPORT_SYMBOL(__cap_empty_set);
EXPORT_SYMBOL(__cap_full_set);
EXPORT_SYMBOL(__cap_init_eff_set);

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+int file_caps_enabled = 1;
+
+static int __init file_caps_disable(char *str)
+{
+ file_caps_enabled = 0;
+ return 1;
+}
+__setup("no_file_caps", file_caps_disable);
+#else
+static const int file_caps_enabled = 0;
+#endif
+
/*
* More recent versions of libcap are available from:
*
@@ -115,39 +128,6 @@ static int cap_validate_magic(cap_user_h
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.
@@ -234,6 +214,17 @@ static inline int do_sys_capset_other_ta
struct task_struct *target;
int ret;

+ if (file_caps_enabled) {
+ /*
+ * 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.)
+ */
+
+ return -EPERM;
+ }
+
if (!capable(CAP_SETPCAP))
return -EPERM;

@@ -267,8 +258,6 @@ static inline int do_sys_capset_other_ta
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
@@ -296,6 +285,10 @@ static inline int cap_get_target_pid(pid

read_unlock(&tasklist_lock);
spin_unlock(&task_capability_lock);
+ } else if (!file_caps_enabled) {
+ spin_lock(&task_capability_lock);
+ ret = security_capget(current, pEp, pIp, pPp);
+ spin_unlock(&task_capability_lock);
} else
ret = security_capget(current, pEp, pIp, pPp);

@@ -303,22 +296,6 @@ static inline int cap_get_target_pid(pid
}

/*
- * 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.
Index: b/kernel/sched.c
===================================================================
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -8149,6 +8149,15 @@ void __init sched_init(void)
plist_head_init(&init_task.pi_waiters, &init_task.pi_lock);
#endif

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+ /*
+ * Because of the reduced scope of CAP_SETPCAP when filesystem
+ * capabilities are in effect, it is safe to allow CAP_SETPCAP to
+ * be available in the default configuration.
+ */
+ init_task.cap_bset = CAP_FULL_SET;
+#endif
+
/*
* The boot idle thread does lazy MMU switching as well:
*/
Index: b/security/commoncap.c
===================================================================
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -27,6 +27,12 @@
#include <linux/prctl.h>
#include <linux/securebits.h>

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+extern int file_caps_enabled;
+#else
+static const int file_caps_enabled = 0;
+#endif
+
int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
{
NETLINK_CB(skb).eff_cap = current->cap_effective;
@@ -93,10 +99,11 @@ int cap_capget (struct task_struct *targ
return 0;
}

-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-
static inline int cap_block_setpcap(struct task_struct *target)
{
+ if (!file_caps_enabled)
+ return 0;
+
/*
* No support for remote process capability manipulation with
* filesystem capability support.
@@ -106,6 +113,9 @@ static inline int cap_block_setpcap(stru

static inline int cap_inh_is_capped(void)
{
+ if (!file_caps_enabled)
+ return 1;
+
/*
* Return 1 if changes to the inheritable set are limited
* to the old permitted set. That is, if the current task
@@ -114,18 +124,13 @@ static inline int cap_inh_is_capped(void
return (cap_capable(current, CAP_SETPCAP) != 0);
}

-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)
{
- return !capable(CAP_SETPCAP);
-}
+ if (!file_caps_enabled)
+ return !capable(CAP_SETPCAP);

-#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
+ return 1;
+}

int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
@@ -176,13 +181,14 @@ static inline void bprm_clear_caps(struc
bprm->cap_effective = false;
}

-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-
int cap_inode_need_killpriv(struct dentry *dentry)
{
struct inode *inode = dentry->d_inode;
int error;

+ if (!file_caps_enabled)
+ return 0;
+
if (!inode->i_op || !inode->i_op->getxattr)
return 0;

@@ -196,6 +202,9 @@ int cap_inode_killpriv(struct dentry *de
{
struct inode *inode = dentry->d_inode;

+ if (!file_caps_enabled)
+ return 0;
+
if (!inode->i_op || !inode->i_op->removexattr)
return 0;

@@ -279,6 +288,11 @@ static int get_file_caps(struct linux_bi
struct vfs_cap_data vcaps;
struct inode *inode;

+ if (!file_caps_enabled) {
+ bprm_clear_caps(bprm);
+ return 0;
+ }
+
if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
bprm_clear_caps(bprm);
return 0;
@@ -312,24 +326,6 @@ out:
return rc;
}

-#else
-int cap_inode_need_killpriv(struct dentry *dentry)
-{
- return 0;
-}
-
-int cap_inode_killpriv(struct dentry *dentry)
-{
- return 0;
-}
-
-static inline int get_file_caps(struct linux_binprm *bprm)
-{
- bprm_clear_caps(bprm);
- return 0;
-}
-#endif
-
int cap_bprm_set_security (struct linux_binprm *bprm)
{
int ret;
@@ -530,7 +526,6 @@ int cap_task_post_setuid (uid_t old_ruid
return 0;
}

-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
/*
* Rationale: code calling task_setscheduler, task_setioprio, and
* task_setnice, assumes that
@@ -552,19 +547,30 @@ static inline int cap_safe_nice(struct t
int cap_task_setscheduler (struct task_struct *p, int policy,
struct sched_param *lp)
{
+ if (!file_caps_enabled)
+ return 0;
+
return cap_safe_nice(p);
}

int cap_task_setioprio (struct task_struct *p, int ioprio)
{
+ if (!file_caps_enabled)
+ return 0;
+
return cap_safe_nice(p);
}

int cap_task_setnice (struct task_struct *p, int nice)
{
+ if (!file_caps_enabled)
+ return 0;
+
return cap_safe_nice(p);
}

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+
/*
* called from kernel/sys.c for prctl(PR_CABSET_DROP)
* done without task_capability_lock() because it introduces
@@ -582,20 +588,6 @@ static long cap_prctl_drop(unsigned long
return 0;
}

-#else
-int cap_task_setscheduler (struct task_struct *p, int policy,
- struct sched_param *lp)
-{
- return 0;
-}
-int cap_task_setioprio (struct task_struct *p, int ioprio)
-{
- return 0;
-}
-int cap_task_setnice (struct task_struct *p, int nice)
-{
- return 0;
-}
#endif

int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
@@ -612,6 +604,9 @@ int cap_task_prctl(int option, unsigned
break;
#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
case PR_CAPBSET_DROP:
+ if (!file_caps_enabled)
+ return 0;
+
error = cap_prctl_drop(arg2);
break;

@@ -635,6 +630,9 @@ int cap_task_prctl(int option, unsigned
* capability-based-privilege environment.
*/
case PR_SET_SECUREBITS:
+ if (!file_caps_enabled)
+ return 0;
+
if ((((current->securebits & SECURE_ALL_LOCKS) >> 1)
& (current->securebits ^ arg2)) /*[1]*/
|| ((current->securebits & SECURE_ALL_LOCKS
@@ -654,6 +652,9 @@ int cap_task_prctl(int option, unsigned
}
break;
case PR_GET_SECUREBITS:
+ if (!file_caps_enabled)
+ return 0;
+
error = current->securebits;
break;


2008-08-26 20:29:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch


* Andreas Gruenbacher <[email protected]> wrote:

> +++ b/kernel/sched.c
> @@ -8149,6 +8149,15 @@ void __init sched_init(void)
> plist_head_init(&init_task.pi_waiters, &init_task.pi_lock);
> #endif
>
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> + /*
> + * Because of the reduced scope of CAP_SETPCAP when filesystem
> + * capabilities are in effect, it is safe to allow CAP_SETPCAP to
> + * be available in the default configuration.
> + */
> + init_task.cap_bset = CAP_FULL_SET;
> +#endif

hm, this doesnt belong into sched.c. You need your own (ifdef-less) init
function in init/main.c or so.

Ingo

2008-08-27 14:04:43

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Quoting Andreas Gruenbacher ([email protected]):
> Hello,
>
> here is a patch allowing to disable file capabilities via a kernel command
> line option (once compiled in with CONFIG_SECURITY_FILE_CAPABILITIES).
>
> We would like to ship our next round of products with file capabilities
> compiled in, yet we feel that too many system utilities are still file
> capabilitiy unaware, and so we would like to turn them off by default
> initially. File capabilities can be used to grant privileges to binaries
> which otherwise look "harmless", which is a security risk until utilities
> like rpm have learned how to install and verify capabilities, etc.
>
> Any objections?

Hi Andreas,

No objections in general - if it makes you more comfortable shipping
kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
However, can you elaborate on your concerns?

In particular, if as you say above the concern is really just that a
file might have capabilities accidentally (or maliciously) enabled, then
we should be able to just check for file_caps_enabled() at
get_file_caps(), refusing to fill in the file capabilities.

The other changes which you are canceling out confuscate the code but
actually make no difference. In particular, the change in behavior
wrt CAP_SETPCAP is as follows: With
CONFIG_SECURITY_FILE_CAPABILITIES=n, CAP_SETPCAP means that you
can give your capabilities to another task. With
CONFIG_SECURITY_FILE_CAPABILITIES=y, CAP_SETPCAP *only* means that
you can add bits to your pI. But pI is always masked with fI, so
if we refuse to fill in fI at get_file_caps(), then that is ok :)

Do you want me to send a such a patch?

thanks,
-serge

> Thanks.
>
> Signed-off-by: Andreas Gruenbacher <[email protected]>
>
> ---
> include/linux/init_task.h | 13 ------
> kernel/capability.c | 79 +++++++++++++--------------------------
> kernel/sched.c | 9 ++++
> security/commoncap.c | 93 +++++++++++++++++++++++-----------------------
> 4 files changed, 85 insertions(+), 109 deletions(-)
>
> Index: b/include/linux/init_task.h
> ===================================================================
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -102,17 +102,6 @@ extern struct group_info init_groups;
> #define INIT_IDS
> #endif
>
> -#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> -/*
> - * Because of the reduced scope of CAP_SETPCAP when filesystem
> - * capabilities are in effect, it is safe to allow CAP_SETPCAP to
> - * be available in the default configuration.
> - */
> -# define CAP_INIT_BSET CAP_FULL_SET
> -#else
> -# define CAP_INIT_BSET CAP_INIT_EFF_SET
> -#endif
> -
> /*
> * INIT_TASK is used to set up the first task table, touch at
> * your own risk!. Base=0, limit=0x1fffff (=2MB)
> @@ -151,7 +140,7 @@ extern struct group_info init_groups;
> .cap_effective = CAP_INIT_EFF_SET, \
> .cap_inheritable = CAP_INIT_INH_SET, \
> .cap_permitted = CAP_FULL_SET, \
> - .cap_bset = CAP_INIT_BSET, \
> + .cap_bset = CAP_INIT_EFF_SET, /* also see sched_init() */ \
> .securebits = SECUREBITS_DEFAULT, \
> .user = INIT_USER, \
> .comm = "swapper", \
> Index: b/kernel/capability.c
> ===================================================================
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -33,6 +33,19 @@ EXPORT_SYMBOL(__cap_empty_set);
> EXPORT_SYMBOL(__cap_full_set);
> EXPORT_SYMBOL(__cap_init_eff_set);
>
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> +int file_caps_enabled = 1;
> +
> +static int __init file_caps_disable(char *str)
> +{
> + file_caps_enabled = 0;
> + return 1;
> +}
> +__setup("no_file_caps", file_caps_disable);
> +#else
> +static const int file_caps_enabled = 0;
> +#endif
> +
> /*
> * More recent versions of libcap are available from:
> *
> @@ -115,39 +128,6 @@ static int cap_validate_magic(cap_user_h
> 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.
> @@ -234,6 +214,17 @@ static inline int do_sys_capset_other_ta
> struct task_struct *target;
> int ret;
>
> + if (file_caps_enabled) {
> + /*
> + * 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.)
> + */
> +
> + return -EPERM;
> + }
> +
> if (!capable(CAP_SETPCAP))
> return -EPERM;
>
> @@ -267,8 +258,6 @@ static inline int do_sys_capset_other_ta
> 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
> @@ -296,6 +285,10 @@ static inline int cap_get_target_pid(pid
>
> read_unlock(&tasklist_lock);
> spin_unlock(&task_capability_lock);
> + } else if (!file_caps_enabled) {
> + spin_lock(&task_capability_lock);
> + ret = security_capget(current, pEp, pIp, pPp);
> + spin_unlock(&task_capability_lock);
> } else
> ret = security_capget(current, pEp, pIp, pPp);
>
> @@ -303,22 +296,6 @@ static inline int cap_get_target_pid(pid
> }
>
> /*
> - * 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.
> Index: b/kernel/sched.c
> ===================================================================
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8149,6 +8149,15 @@ void __init sched_init(void)
> plist_head_init(&init_task.pi_waiters, &init_task.pi_lock);
> #endif
>
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> + /*
> + * Because of the reduced scope of CAP_SETPCAP when filesystem
> + * capabilities are in effect, it is safe to allow CAP_SETPCAP to
> + * be available in the default configuration.
> + */
> + init_task.cap_bset = CAP_FULL_SET;
> +#endif
> +
> /*
> * The boot idle thread does lazy MMU switching as well:
> */
> Index: b/security/commoncap.c
> ===================================================================
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -27,6 +27,12 @@
> #include <linux/prctl.h>
> #include <linux/securebits.h>
>
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> +extern int file_caps_enabled;
> +#else
> +static const int file_caps_enabled = 0;
> +#endif
> +
> int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
> {
> NETLINK_CB(skb).eff_cap = current->cap_effective;
> @@ -93,10 +99,11 @@ int cap_capget (struct task_struct *targ
> return 0;
> }
>
> -#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> -
> static inline int cap_block_setpcap(struct task_struct *target)
> {
> + if (!file_caps_enabled)
> + return 0;
> +
> /*
> * No support for remote process capability manipulation with
> * filesystem capability support.
> @@ -106,6 +113,9 @@ static inline int cap_block_setpcap(stru
>
> static inline int cap_inh_is_capped(void)
> {
> + if (!file_caps_enabled)
> + return 1;
> +
> /*
> * Return 1 if changes to the inheritable set are limited
> * to the old permitted set. That is, if the current task
> @@ -114,18 +124,13 @@ static inline int cap_inh_is_capped(void
> return (cap_capable(current, CAP_SETPCAP) != 0);
> }
>
> -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)
> {
> - return !capable(CAP_SETPCAP);
> -}
> + if (!file_caps_enabled)
> + return !capable(CAP_SETPCAP);
>
> -#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
> + return 1;
> +}
>
> int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
> kernel_cap_t *inheritable, kernel_cap_t *permitted)
> @@ -176,13 +181,14 @@ static inline void bprm_clear_caps(struc
> bprm->cap_effective = false;
> }
>
> -#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> -
> int cap_inode_need_killpriv(struct dentry *dentry)
> {
> struct inode *inode = dentry->d_inode;
> int error;
>
> + if (!file_caps_enabled)
> + return 0;
> +
> if (!inode->i_op || !inode->i_op->getxattr)
> return 0;
>
> @@ -196,6 +202,9 @@ int cap_inode_killpriv(struct dentry *de
> {
> struct inode *inode = dentry->d_inode;
>
> + if (!file_caps_enabled)
> + return 0;
> +
> if (!inode->i_op || !inode->i_op->removexattr)
> return 0;
>
> @@ -279,6 +288,11 @@ static int get_file_caps(struct linux_bi
> struct vfs_cap_data vcaps;
> struct inode *inode;
>
> + if (!file_caps_enabled) {
> + bprm_clear_caps(bprm);
> + return 0;
> + }
> +
> if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
> bprm_clear_caps(bprm);
> return 0;
> @@ -312,24 +326,6 @@ out:
> return rc;
> }
>
> -#else
> -int cap_inode_need_killpriv(struct dentry *dentry)
> -{
> - return 0;
> -}
> -
> -int cap_inode_killpriv(struct dentry *dentry)
> -{
> - return 0;
> -}
> -
> -static inline int get_file_caps(struct linux_binprm *bprm)
> -{
> - bprm_clear_caps(bprm);
> - return 0;
> -}
> -#endif
> -
> int cap_bprm_set_security (struct linux_binprm *bprm)
> {
> int ret;
> @@ -530,7 +526,6 @@ int cap_task_post_setuid (uid_t old_ruid
> return 0;
> }
>
> -#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> /*
> * Rationale: code calling task_setscheduler, task_setioprio, and
> * task_setnice, assumes that
> @@ -552,19 +547,30 @@ static inline int cap_safe_nice(struct t
> int cap_task_setscheduler (struct task_struct *p, int policy,
> struct sched_param *lp)
> {
> + if (!file_caps_enabled)
> + return 0;
> +
> return cap_safe_nice(p);
> }
>
> int cap_task_setioprio (struct task_struct *p, int ioprio)
> {
> + if (!file_caps_enabled)
> + return 0;
> +
> return cap_safe_nice(p);
> }
>
> int cap_task_setnice (struct task_struct *p, int nice)
> {
> + if (!file_caps_enabled)
> + return 0;
> +
> return cap_safe_nice(p);
> }
>
> +#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> +
> /*
> * called from kernel/sys.c for prctl(PR_CABSET_DROP)
> * done without task_capability_lock() because it introduces
> @@ -582,20 +588,6 @@ static long cap_prctl_drop(unsigned long
> return 0;
> }
>
> -#else
> -int cap_task_setscheduler (struct task_struct *p, int policy,
> - struct sched_param *lp)
> -{
> - return 0;
> -}
> -int cap_task_setioprio (struct task_struct *p, int ioprio)
> -{
> - return 0;
> -}
> -int cap_task_setnice (struct task_struct *p, int nice)
> -{
> - return 0;
> -}
> #endif
>
> int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> @@ -612,6 +604,9 @@ int cap_task_prctl(int option, unsigned
> break;
> #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
> case PR_CAPBSET_DROP:
> + if (!file_caps_enabled)
> + return 0;
> +
> error = cap_prctl_drop(arg2);
> break;
>
> @@ -635,6 +630,9 @@ int cap_task_prctl(int option, unsigned
> * capability-based-privilege environment.
> */
> case PR_SET_SECUREBITS:
> + if (!file_caps_enabled)
> + return 0;
> +
> if ((((current->securebits & SECURE_ALL_LOCKS) >> 1)
> & (current->securebits ^ arg2)) /*[1]*/
> || ((current->securebits & SECURE_ALL_LOCKS
> @@ -654,6 +652,9 @@ int cap_task_prctl(int option, unsigned
> }
> break;
> case PR_GET_SECUREBITS:
> + if (!file_caps_enabled)
> + return 0;
> +
> error = current->securebits;
> break;

2008-08-27 15:29:36

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wednesday, 27 August 2008 15:52:06 Serge E. Hallyn wrote:
> Quoting Andreas Gruenbacher ([email protected]):
> > Hello,
> >
> > here is a patch allowing to disable file capabilities via a kernel
> > command line option (once compiled in with
> > CONFIG_SECURITY_FILE_CAPABILITIES).
> >
> > We would like to ship our next round of products with file capabilities
> > compiled in, yet we feel that too many system utilities are still file
> > capabilitiy unaware, and so we would like to turn them off by default
> > initially. File capabilities can be used to grant privileges to binaries
> > which otherwise look "harmless", which is a security risk until utilities
> > like rpm have learned how to install and verify capabilities, etc.
> >
> > Any objections?
>
> Hi Andreas,
>
> No objections in general - if it makes you more comfortable shipping
> kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
> However, can you elaborate on your concerns?

We don't have the time left for developing the few missing pieces and properly
integrating file capabilities into our products (use in various packages,
support in rpm, system management, manuals, release notes), and so I would
like to have a way to turn them off by default for now.

> In particular, if as you say above the concern is really just that a
> file might have capabilities accidentally (or maliciously) enabled, then
> we should be able to just check for file_caps_enabled() at
> get_file_caps(), refusing to fill in the file capabilities.

My main concern is accidental granting of capabilities because of admin
unawareness / lack of tool support. This could be taken care of by not
loading the capabilities from disk.

> The other changes which you are canceling out confuscate the code but
> actually make no difference.

Well, the other difference is that with CONFIG_SECURITY_FILE_CAPABILITIES=y
you currently lose the ability to pass on capabilities to other processes. Do
you have good arguments why this feature is unnecessary? I don' think that
having this feature was a good idea in the first place, but taking it away
now after several years may break current users which may not have gotten
converted, yet.

Thanks,
Andreas

2008-08-27 16:04:50

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Quoting Andreas Gruenbacher ([email protected]):
> On Wednesday, 27 August 2008 15:52:06 Serge E. Hallyn wrote:
> > Quoting Andreas Gruenbacher ([email protected]):
> > > Hello,
> > >
> > > here is a patch allowing to disable file capabilities via a kernel
> > > command line option (once compiled in with
> > > CONFIG_SECURITY_FILE_CAPABILITIES).
> > >
> > > We would like to ship our next round of products with file capabilities
> > > compiled in, yet we feel that too many system utilities are still file
> > > capabilitiy unaware, and so we would like to turn them off by default
> > > initially. File capabilities can be used to grant privileges to binaries
> > > which otherwise look "harmless", which is a security risk until utilities
> > > like rpm have learned how to install and verify capabilities, etc.
> > >
> > > Any objections?
> >
> > Hi Andreas,
> >
> > No objections in general - if it makes you more comfortable shipping
> > kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
> > However, can you elaborate on your concerns?
>
> We don't have the time left for developing the few missing pieces and properly
> integrating file capabilities into our products (use in various packages,
> support in rpm, system management, manuals, release notes), and so I would
> like to have a way to turn them off by default for now.
>
> > In particular, if as you say above the concern is really just that a
> > file might have capabilities accidentally (or maliciously) enabled, then
> > we should be able to just check for file_caps_enabled() at
> > get_file_caps(), refusing to fill in the file capabilities.
>
> My main concern is accidental granting of capabilities because of admin
> unawareness / lack of tool support. This could be taken care of by not
> loading the capabilities from disk.
>
> > The other changes which you are canceling out confuscate the code but
> > actually make no difference.
>
> Well, the other difference is that with CONFIG_SECURITY_FILE_CAPABILITIES=y
> you currently lose the ability to pass on capabilities to other processes. Do
> you have good arguments why this feature is unnecessary?

Yes, mainly that you don't actually have that ability anyway, because
when CONFIG_SECURITY_FILE_CAPABILITIES=n, then CAP_SETPCAP is not in the
system-wide capability bounding set, and without CAP_SETPCAP you cannot
pass capabilities to another process.

You can do it if you have a custom initrd that adds CAP_SETPCAP to the
bounding set early enough, but it has to be done by pid=1. As far as I
have seen there are 0 users of the feature. I would expect most people
would find it easier to recompile a tweaked kernel changing the #define
of CAP_INIT_BSET.

If, however, you really do have such users, then we must go with a
version of your patch. We may then want to consider altogether
replacing the CONFIG_SECURITY_FILE_CAPABILITIES boolean with a default
value for file_caps_enabled. That may actually end up cleaner than
the current code by removing all of the #ifdefs. Does that sound ok
to you? Andrew Morgan, would you be ok with that?

(Also note that if you have such users, you'll want to ask David
Howells not to push the patch he has floated removing the ability to
pass caps to another task altogether when CONFIG_SECURITY_FILE_CAPABILITIES=n :)

> I don' think that
> having this feature was a good idea in the first place, but taking it away
> now after several years may break current users which may not have gotten
> converted, yet.

thanks,
-serge

2008-08-27 16:18:44

by David Howells

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Serge E. Hallyn <[email protected]> wrote:

> (Also note that if you have such users, you'll want to ask David Howells not
> to push the patch he has floated removing the ability to pass caps to
> another task altogether when CONFIG_SECURITY_FILE_CAPABILITIES=n :)

Ugh. My patch removes the ability to pass caps to another task under all
circumstances because to do otherwise means that I have to make the kernel use
RCU locking for a task to access its own creds. If you want this, I'll have
to redo all my later patches.

David

2008-08-27 16:52:17

by Alan

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wed, 27 Aug 2008 17:13:24 +0100
David Howells <[email protected]> wrote:

> Serge E. Hallyn <[email protected]> wrote:
>
> > (Also note that if you have such users, you'll want to ask David Howells not
> > to push the patch he has floated removing the ability to pass caps to
> > another task altogether when CONFIG_SECURITY_FILE_CAPABILITIES=n :)
>
> Ugh. My patch removes the ability to pass caps to another task under all
> circumstances because to do otherwise means that I have to make the kernel use
> RCU locking for a task to access its own creds. If you want this, I'll have
> to redo all my later patches.

That gets foul in another way - bounding the worst case RCU
memory utilisation if someone is sitting doing things like while(1)
change_credentials();

2008-08-27 16:58:09

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wednesday, 27 August 2008 18:04:39 Serge E. Hallyn wrote:
> Quoting Andreas Gruenbacher ([email protected]):
> > On Wednesday, 27 August 2008 15:52:06 Serge E. Hallyn wrote:
> > > Quoting Andreas Gruenbacher ([email protected]):
> > > > Hello,
> > > >
> > > > here is a patch allowing to disable file capabilities via a kernel
> > > > command line option (once compiled in with
> > > > CONFIG_SECURITY_FILE_CAPABILITIES).
> > > >
> > > > We would like to ship our next round of products with file
> > > > capabilities compiled in, yet we feel that too many system utilities
> > > > are still file capabilitiy unaware, and so we would like to turn them
> > > > off by default initially. File capabilities can be used to grant
> > > > privileges to binaries which otherwise look "harmless", which is a
> > > > security risk until utilities like rpm have learned how to install
> > > > and verify capabilities, etc.
> > > >
> > > > Any objections?
> > >
> > > Hi Andreas,
> > >
> > > No objections in general - if it makes you more comfortable shipping
> > > kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
> > > However, can you elaborate on your concerns?
> >
> > We don't have the time left for developing the few missing pieces and
> > properly integrating file capabilities into our products (use in various
> > packages, support in rpm, system management, manuals, release notes), and
> > so I would like to have a way to turn them off by default for now.
> >
> > > In particular, if as you say above the concern is really just that a
> > > file might have capabilities accidentally (or maliciously) enabled,
> > > then we should be able to just check for file_caps_enabled() at
> > > get_file_caps(), refusing to fill in the file capabilities.
> >
> > My main concern is accidental granting of capabilities because of admin
> > unawareness / lack of tool support. This could be taken care of by not
> > loading the capabilities from disk.
> >
> > > The other changes which you are canceling out confuscate the code but
> > > actually make no difference.
> >
> > Well, the other difference is that with
> > CONFIG_SECURITY_FILE_CAPABILITIES=y you currently lose the ability to
> > pass on capabilities to other processes. Do you have good arguments why
> > this feature is unnecessary?
>
> Yes, mainly that you don't actually have that ability anyway, because
> when CONFIG_SECURITY_FILE_CAPABILITIES=n, then CAP_SETPCAP is not in the
> system-wide capability bounding set, and without CAP_SETPCAP you cannot
> pass capabilities to another process.
>
> You can do it if you have a custom initrd that adds CAP_SETPCAP to the
> bounding set early enough, but it has to be done by pid=1. As far as I
> have seen there are 0 users of the feature.

Alright, this should suffice and we won't have to care about this case then.

What remains is a way to disable the loading of capabilities from the kernel
command line, but this is a rather trivial patch. Would you like to write
that? Shall i send a patch?

> If, however, you really do have such users, then we must go with a
> version of your patch. We may then want to consider altogether
> replacing the CONFIG_SECURITY_FILE_CAPABILITIES boolean with a default
> value for file_caps_enabled. That may actually end up cleaner than
> the current code by removing all of the #ifdefs.

Most ifdefs would go away by adding a file_caps_enabled variable / #define in
capability.h and using that. I would suggest to make this on-by-default as
the common case will eventually be on, and that way, we won't have to carry
the kernel command-line option code forever.

> (Also note that if you have such users, you'll want to ask David
> Howells not to push the patch he has floated removing the ability to
> pass caps to another task altogether when
> CONFIG_SECURITY_FILE_CAPABILITIES=n :)

I was actually about to ask for making this behavior change pertinent instead
of having it depend on CONFIG_SECURITY_FILE_CAPABILITIES :)

Thanks,
Andreas

2008-08-27 17:03:39

by David Howells

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Alan Cox <[email protected]> wrote:

> That gets foul in another way - bounding the worst case RCU
> memory utilisation if someone is sitting doing things like while(1)
> change_credentials();

Is it worth calling synchronize_rcu() in commit_creds() to make sure that the
old memory will have been freed if possible? Admittedly that'll slow setuid()
and co. down, but it should avoid that problem.

David

2008-08-27 17:04:46

by David Howells

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Andreas Gruenbacher <[email protected]> wrote:

> > (Also note that if you have such users, you'll want to ask David
> > Howells not to push the patch he has floated removing the ability to
> > pass caps to another task altogether when
> > CONFIG_SECURITY_FILE_CAPABILITIES=n :)
>
> I was actually about to ask for making this behavior change pertinent instead
> of having it depend on CONFIG_SECURITY_FILE_CAPABILITIES :)

Do you mean 'permanent' rather than 'pertinent'?

David

2008-08-27 18:58:59

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Quoting Andreas Gruenbacher ([email protected]):
> On Wednesday, 27 August 2008 18:04:39 Serge E. Hallyn wrote:
> > Quoting Andreas Gruenbacher ([email protected]):
> > > On Wednesday, 27 August 2008 15:52:06 Serge E. Hallyn wrote:
> > > > Quoting Andreas Gruenbacher ([email protected]):
> > > > > Hello,
> > > > >
> > > > > here is a patch allowing to disable file capabilities via a kernel
> > > > > command line option (once compiled in with
> > > > > CONFIG_SECURITY_FILE_CAPABILITIES).
> > > > >
> > > > > We would like to ship our next round of products with file
> > > > > capabilities compiled in, yet we feel that too many system utilities
> > > > > are still file capabilitiy unaware, and so we would like to turn them
> > > > > off by default initially. File capabilities can be used to grant
> > > > > privileges to binaries which otherwise look "harmless", which is a
> > > > > security risk until utilities like rpm have learned how to install
> > > > > and verify capabilities, etc.
> > > > >
> > > > > Any objections?
> > > >
> > > > Hi Andreas,
> > > >
> > > > No objections in general - if it makes you more comfortable shipping
> > > > kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
> > > > However, can you elaborate on your concerns?
> > >
> > > We don't have the time left for developing the few missing pieces and
> > > properly integrating file capabilities into our products (use in various
> > > packages, support in rpm, system management, manuals, release notes), and
> > > so I would like to have a way to turn them off by default for now.
> > >
> > > > In particular, if as you say above the concern is really just that a
> > > > file might have capabilities accidentally (or maliciously) enabled,
> > > > then we should be able to just check for file_caps_enabled() at
> > > > get_file_caps(), refusing to fill in the file capabilities.
> > >
> > > My main concern is accidental granting of capabilities because of admin
> > > unawareness / lack of tool support. This could be taken care of by not
> > > loading the capabilities from disk.
> > >
> > > > The other changes which you are canceling out confuscate the code but
> > > > actually make no difference.
> > >
> > > Well, the other difference is that with
> > > CONFIG_SECURITY_FILE_CAPABILITIES=y you currently lose the ability to
> > > pass on capabilities to other processes. Do you have good arguments why
> > > this feature is unnecessary?
> >
> > Yes, mainly that you don't actually have that ability anyway, because
> > when CONFIG_SECURITY_FILE_CAPABILITIES=n, then CAP_SETPCAP is not in the
> > system-wide capability bounding set, and without CAP_SETPCAP you cannot
> > pass capabilities to another process.
> >
> > You can do it if you have a custom initrd that adds CAP_SETPCAP to the
> > bounding set early enough, but it has to be done by pid=1. As far as I
> > have seen there are 0 users of the feature.
>
> Alright, this should suffice and we won't have to care about this case then.
>
> What remains is a way to disable the loading of capabilities from the kernel
> command line, but this is a rather trivial patch. Would you like to write
> that? Shall i send a patch?

The below patch seems to be working for me. Please take a look.

> > If, however, you really do have such users, then we must go with a
> > version of your patch. We may then want to consider altogether
> > replacing the CONFIG_SECURITY_FILE_CAPABILITIES boolean with a default
> > value for file_caps_enabled. That may actually end up cleaner than
> > the current code by removing all of the #ifdefs.
>
> Most ifdefs would go away by adding a file_caps_enabled variable / #define in
> capability.h and using that. I would suggest to make this on-by-default as
> the common case will eventually be on, and that way, we won't have to carry
> the kernel command-line option code forever.
>
> > (Also note that if you have such users, you'll want to ask David
> > Howells not to push the patch he has floated removing the ability to
> > pass caps to another task altogether when
> > CONFIG_SECURITY_FILE_CAPABILITIES=n :)
>
> I was actually about to ask for making this behavior change pertinent instead
> of having it depend on CONFIG_SECURITY_FILE_CAPABILITIES :)

Ok, cool. In that case - David I was a little surprised to find that
patch not applied in Linus' tree. You sent it quite some time ago,
didn't you? Were you planning on trying again to push it soon?

In any case, how about the following patch?

If it is ok with everyone, then I'll rewrite the intro and send it
out separately.

thanks,
-serge

>From 0b69a9e882b07e33814aa15a2f5cdf90b92c7ec8 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <[email protected]>
Date: Wed, 27 Aug 2008 13:33:37 -0500
Subject: [PATCH 1/1] file capabilities: add no_file_caps switch (v2)

Hi,

here is a simplification of Andreas' patch from yesterday. We
appear to be agreed that it suffices to prevent file capabilities
from being actually read from disk, so this version only does that.
This means that booting with the no_file_caps boot option will
not be the same as booting a kernel with file capabilities
compiled out - in particular a task with CAP_SETPCAP will not
have any chance of passing capabilities to another task (which
isn't "really" possible anyway, and which may soon by killed
altogether by David Howells in any case), and it will instead
be able to put new capabilities in its pI. However since fI
will always be empty and pI is masked with fI, it gains the
task nothing.

We also support the extra prctl options, setting securebits and
dropping capabilities from the per-process bounding set.

The other remaining difference is that killpriv, task_setscheduler,
setioprio, and setnice will continue to be hooked. That will
be noticable in the case where a root task changed its uid
while keeping some caps, and another task owned by the new uid
tries to change settings for the more privileged task.

Signed-off-by: Serge Hallyn <[email protected]>
---
kernel/capability.c | 13 +++++++++++++
security/commoncap.c | 11 +++++++++++
2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/kernel/capability.c b/kernel/capability.c
index 33e51e7..5d034ec 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -33,6 +33,19 @@ EXPORT_SYMBOL(__cap_empty_set);
EXPORT_SYMBOL(__cap_full_set);
EXPORT_SYMBOL(__cap_init_eff_set);

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+int file_caps_enabled = 1;
+
+static int __init file_caps_disable(char *str)
+{
+ file_caps_enabled = 0;
+ return 1;
+}
+__setup("no_file_caps", file_caps_disable);
+#else
+static const int file_caps_enabled = 0;
+#endif
+
/*
* More recent versions of libcap are available from:
*
diff --git a/security/commoncap.c b/security/commoncap.c
index e4c4b3f..8c66d34 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -27,6 +27,12 @@
#include <linux/prctl.h>
#include <linux/securebits.h>

+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+extern int file_caps_enabled;
+#else
+static const int file_caps_enabled = 0;
+#endif
+
int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
{
NETLINK_CB(skb).eff_cap = current->cap_effective;
@@ -279,6 +285,11 @@ static int get_file_caps(struct linux_binprm *bprm)
struct vfs_cap_data vcaps;
struct inode *inode;

+ if (!file_caps_enabled) {
+ bprm_clear_caps(bprm);
+ return 0;
+ }
+
if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
bprm_clear_caps(bprm);
return 0;
--
1.5.4.3

2008-08-27 21:22:49

by David Howells

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Serge E. Hallyn <[email protected]> wrote:

> Ok, cool. In that case - David I was a little surprised to find that
> patch not applied in Linus' tree. You sent it quite some time ago,
> didn't you? Were you planning on trying again to push it soon?

I think you were getting my patches confused.

I had two patches of note:

(1) Fix PF_SUPERPRIV that I was pushing to get upstream, and is now upstream.

(2) Neuter sys_capset(). I've been holding this off for the next merge
window as it isn't a bugfix, unlike (1). Perhaps I should ask James to
push it to Linus. James?

David

2008-08-28 00:08:54

by James Morris

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wed, 27 Aug 2008, David Howells wrote:

> (2) Neuter sys_capset(). I've been holding this off for the next merge
> window as it isn't a bugfix, unlike (1). Perhaps I should ask James to
> push it to Linus. James?

Linus only pulled the PF_SUPERPRIV fix once the sys_capset change was
removed from the patch. It really does need to be a bugfix at this stage.


- James
--
James Morris
<[email protected]>

2008-08-28 00:48:20

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Quoting James Morris ([email protected]):
> On Wed, 27 Aug 2008, David Howells wrote:
>
> > (2) Neuter sys_capset(). I've been holding this off for the next merge
> > window as it isn't a bugfix, unlike (1). Perhaps I should ask James to
> > push it to Linus. James?
>
> Linus only pulled the PF_SUPERPRIV fix once the sys_capset change was
> removed from the patch. It really does need to be a bugfix at this stage.

Ok, sorry, of course that makes sense. I was just confused about where
the patch was originally heading.

Would it be appropriate to put the capset neutering patch in your
security-testing tree, James, or does that feed straight into
linux-next?

thanks,
-serge

2008-08-28 01:58:12

by James Morris

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wed, 27 Aug 2008, Serge E. Hallyn wrote:

> Quoting James Morris ([email protected]):
> > On Wed, 27 Aug 2008, David Howells wrote:
> >
> > > (2) Neuter sys_capset(). I've been holding this off for the next merge
> > > window as it isn't a bugfix, unlike (1). Perhaps I should ask James to
> > > push it to Linus. James?
> >
> > Linus only pulled the PF_SUPERPRIV fix once the sys_capset change was
> > removed from the patch. It really does need to be a bugfix at this stage.
>
> Ok, sorry, of course that makes sense. I was just confused about where
> the patch was originally heading.
>
> Would it be appropriate to put the capset neutering patch in your
> security-testing tree, James, or does that feed straight into
> linux-next?

It's already in the next-creds branch, but it could be added to the next
branch (which will be pushed to Linus in the next merge window). Both
branches are in linux-next.


--
James Morris
<[email protected]>

2008-08-28 15:36:03

by Andrew G. Morgan

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sorry to get to this late.

I very much prefer this patch over the resuscitation of the cap_set_pg()
code.

[I would also like to advocate we combine it with making
CONFIG_SECURITY_FILE_CAPABILITIES=y the default config option.]

Cheers

Andrew

Serge E. Hallyn wrote:
| In any case, how about the following patch?
|
| If it is ok with everyone, then I'll rewrite the intro and send it
| out separately.
|
| thanks,
| -serge
|
|>From 0b69a9e882b07e33814aa15a2f5cdf90b92c7ec8 Mon Sep 17 00:00:00 2001
| From: Serge Hallyn <[email protected]>
| Date: Wed, 27 Aug 2008 13:33:37 -0500
| Subject: [PATCH 1/1] file capabilities: add no_file_caps switch (v2)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFItsWw+bHCR3gb8jsRAg6RAKCJsG3GajbfGqEX6QnN2vdJlyAEXgCdGmdS
d/7ZcozkYSn4RvX/saISXBM=
=bnGm
-----END PGP SIGNATURE-----

2008-08-28 16:27:52

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

On Wednesday, 27 August 2008 20:58:45 Serge E. Hallyn wrote:
> The below patch seems to be working for me. Please take a look.
>
> If it is ok with everyone, then I'll rewrite the intro and send it
> out separately.

Yes, looks good.

Thanks,
Andreas

2008-08-28 17:09:50

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [patch] file capabilities: Add no_file_caps switch

Quoting Andrew G. Morgan ([email protected]):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Sorry to get to this late.
>
> I very much prefer this patch over the resuscitation of the cap_set_pg()
> code.
>
> [I would also like to advocate we combine it with making
> CONFIG_SECURITY_FILE_CAPABILITIES=y the default config option.]

I certainly don't mind that, but I would expect complaints about this
unless the default boot option were 'n'.

Do you think it's worth sending as a separate trivial patch? Then if
people object to one both changes won't be held up.

(Anyway that's what I'll do right now)

-serge