[PATCH] capabilities: Ambient capability set V2

[PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

V1->V2:
 - Fix up the processing of the caps bits after discussions
   with Any and Serge. Make patch less intrusive.

Ambient caps are something like restricted root privileges.
A process has a set of additional capabilities and those
are inherited without have to set capabilites in other
binaries involved. This allow the partial use of root
like features in a controlled way. It is often useful
to do this for user space device drivers or software that
needs increased priviledges for networking or to control
its own scheduling. Ambient caps allow one to avoid
having to run these with full root priviledges.

Control over this feature is avaialable via a new
prctl option called PR_CAP_AMBIENT. The second argument to prctl
is a the capability number and the third the desired state.
0 for off. Otherwise on.

Ambient bits are enabled regardless of the inheritance
mask of the target binary. They are only restricted
by the bounding set.

History:

Linux capabilities have suffered from the problem that they are not
inheritable like unregular process characteristics under Unix. This is
behavior that is counter intuitive to the expected behavior of processes
in Unix.

In particular there has been recently software that controls NICs from user
space and provides IP stack like behavior also in user space (DPDK and RDMA
kernel API based implementations). Those typically need either capabilities
to allow raw network access or have to be run setsuid. There is scripting and
LD_PREFLOAD etc involved, arbitrary binaries may be run from those scripts
including those setting additional capabilites or requiring root access.

That does not go well with having file capabilities set that would enable
the capabilities. Maybe it would work if one would setup capabilities on
all executables but that would also defeat a secure design since these
binaries may only need those caps for certain situations. Ok setting the
inheritable flags on everything may also get one there (if there would not
be the issues with LD_PRELOAD, debugging etc etc).

The easy solution is to allow some capabilities be inherited like setsuid
is. We really prefer to use capabilities instead of setsuid (we want to
limit what damage someone can do after all!). Therefore we have been
running a patch like this in production for the last 6 years. At some
point it becomes tedious to run your own custom kernel so we would like
to have this functionality upstream.

See some of the earlier related discussions on the problems with capability
inheritance:

0. Recent surprise:
                https://lkml.org/lkml/2014/1/21/175

1. Attempt to revise caps
                http://www.madore.org/~david/linux/newcaps/

2. Problems of passing caps through exec
                http://unix.stackexchange.com/questions/128394/passing-capabilities-through-exec

3. Problems of binding to privileged ports
                http://stackoverflow.com/questions/413807/is-there-a-way-for-non-root-processes-to-bind-to-privileged-ports-1024-on-l

4. Reviving capabilities
                http://lwn.net/Articles/199004/

There does not seem to be an alternative on the horizon. Some involved
in security development under Linux have even stated that they want to
rip out the whole thing and replace it. Its been a couple of years now
and we are still suffering from the capabilities mess. Let us just
fix it. Others have already done implementations like this like Nokia
for the N900.


This patch does not change the default behavior but it allows to set up
a list of capabilities via prctl that will enable regular
unix inheritance only for the selected group of capabilities.

With that it is then possible to do something trivial like setting
CAP_NET_RAW on an executable that can then allow that capability to
be inherited by others.

Lets have a look at a coding example of a wrapper that enables
a couple of capabilities:

------------------------------ ambient_test.c
/*
 * Test program for the ambient capabilities
 *
 *
 * Compile using:
 *	gcc -o ambient_test ambient_test.o
 *
 * This program must have the following capabilities to run properly:
 * CAP_SETPCAP, CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_NICE
 *
 * A command to equip this with the right caps is:
 *
 *	setcap cap_setpcap,cap_net_raw,cap_net_admin,cap_sys_nice+eip ambient_test
 *
 * To get a shell with additional caps that can be inherited do:
 *
 * ./ambient_test /bin/bash
 *
 */

#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <sys/prctl.h>
#include <linux/capability.h>

/* Defintion to be updated in the user space include files */
#define PR_CAP_AMBIENT 45

int main(int argc, char **argv)
{
	int rc;

	if (prctl(PR_CAP_AMBIENT, CAP_NET_RAW))
		perror("Cannot set CAP_NET_RAW");

	if (prctl(PR_CAP_AMBIENT, CAP_NET_ADMIN))
		perror("Cannot set CAP_NET_ADMIN");

	if (prctl(PR_CAP_AMBIENT, CAP_SYS_NICE))
		perror("Cannot set CAP_SYS_NICE");

	printf("Ambient_test forking shell\n");
	if (execv(argv[1], argv + 1))
		perror("Cannot exec");

	return 0;
}
-------------------------------- ambient_test.c

Allows the inheritance of CAP_SYS_NICE, CAP_NET_RAW and CAP_NET_ADMIN.
With that device raw access is possible and also real time priorities
can be set from user space. This is a frequently needed set of
priviledged operations in HPC and HFT applications. User space
processes need to be able to directly access devices as well as
have full control over scheduling.

Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c	2015-02-25 13:43:06.929973954 -0600
+++ linux/security/commoncap.c	2015-02-26 16:10:02.347913397 -0600
@@ -347,15 +347,17 @@ static inline int bprm_caps_from_vfs_cap
 		*has_cap = true;

 	CAP_FOR_EACH_U32(i) {
+		__u32 ambient = current_cred()->cap_ambient.cap[i];
 		__u32 permitted = caps->permitted.cap[i];
 		__u32 inheritable = caps->inheritable.cap[i];

 		/*
-		 * pP' = (X & fP) | (pI & fI)
+		 * pP' = (X & fP) | (pI & (fI | pA))
 		 */
 		new->cap_permitted.cap[i] =
 			(new->cap_bset.cap[i] & permitted) |
-			(new->cap_inheritable.cap[i] & inheritable);
+			(new->cap_inheritable.cap[i] &
+					(inheritable | ambient));

 		if (permitted & ~new->cap_permitted.cap[i])
 			/* insufficient to execute correctly */
@@ -453,8 +455,18 @@ static int get_file_caps(struct linux_bi
 		if (rc == -EINVAL)
 			printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
 				__func__, rc, bprm->filename);
-		else if (rc == -ENODATA)
+		else if (rc == -ENODATA) {
 			rc = 0;
+			if (!cap_isclear(current_cred()->cap_ambient)) {
+				/*
+				 * The ambient caps are permitted for
+				 * files that have no caps
+				 */
+				bprm->cred->cap_permitted =
+					current_cred()->cap_ambient;
+				*effective = true;
+			}
+		}
 		goto out;
 	}

@@ -549,9 +561,20 @@ skip:
 	new->sgid = new->fsgid = new->egid;

 	if (effective)
+		/*
+		 * pE' = pP' & (fE | pA)
+		 *
+		 * fE is implicity all set if effective == true.
+		 * Therefore the above reduces to
+		 *
+		 * pE' = pP'
+		 */
 		new->cap_effective = new->cap_permitted;
 	else
 		cap_clear(new->cap_effective);
+
+	/* pA' = pA */
+	new->cap_ambient = old->cap_ambient;
 	bprm->cap_effective = effective;

 	/*
@@ -566,7 +589,7 @@ skip:
 	 * Number 1 above might fail if you don't have a full bset, but I think
 	 * that is interesting information to audit.
 	 */
-	if (!cap_isclear(new->cap_effective)) {
+	if (!cap_issubset(new->cap_effective, new->cap_ambient)) {
 		if (!cap_issubset(CAP_FULL_SET, new->cap_effective) ||
 		    !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) ||
 		    issecure(SECURE_NOROOT)) {
@@ -598,7 +621,7 @@ int cap_bprm_secureexec(struct linux_bin
 	if (!uid_eq(cred->uid, root_uid)) {
 		if (bprm->cap_effective)
 			return 1;
-		if (!cap_isclear(cred->cap_permitted))
+		if (!cap_issubset(cred->cap_permitted, cred->cap_ambient))
 			return 1;
 	}

@@ -933,6 +956,23 @@ int cap_task_prctl(int option, unsigned
 			new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
 		return commit_creds(new);

+	case PR_CAP_AMBIENT:
+		if (!ns_capable(current_user_ns(), CAP_SETPCAP))
+			return -EPERM;
+
+		if (!cap_valid(arg2))
+			return -EINVAL;
+
+		if (!ns_capable(current_user_ns(), arg2))
+			return -EPERM;
+
+		new = prepare_creds();
+		if (arg3 == 0)
+			cap_lower(new->cap_ambient, arg2);
+		else
+			cap_raise(new->cap_ambient, arg2);
+		return commit_creds(new);
+
 	default:
 		/* No functionality available - continue with default */
 		return -ENOSYS;
Index: linux/include/linux/cred.h
===================================================================
--- linux.orig/include/linux/cred.h	2015-02-25 13:43:06.929973954 -0600
+++ linux/include/linux/cred.h	2015-02-25 13:43:06.925972078 -0600
@@ -122,6 +122,7 @@ struct cred {
 	kernel_cap_t	cap_permitted;	/* caps we're permitted */
 	kernel_cap_t	cap_effective;	/* caps we can actually use */
 	kernel_cap_t	cap_bset;	/* capability bounding set */
+	kernel_cap_t	cap_ambient;	/* Ambient capability set */
 #ifdef CONFIG_KEYS
 	unsigned char	jit_keyring;	/* default keyring to attach requested
 					 * keys to */
Index: linux/include/uapi/linux/prctl.h
===================================================================
--- linux.orig/include/uapi/linux/prctl.h	2015-02-25 13:43:06.929973954 -0600
+++ linux/include/uapi/linux/prctl.h	2015-02-25 13:43:06.925972078 -0600
@@ -185,4 +185,7 @@ struct prctl_mm_map {
 #define PR_MPX_ENABLE_MANAGEMENT  43
 #define PR_MPX_DISABLE_MANAGEMENT 44

+/* Control the ambient capability set */
+#define PR_CAP_AMBIENT 45
+
 #endif /* _LINUX_PRCTL_H */
Index: linux/fs/proc/array.c
===================================================================
--- linux.orig/fs/proc/array.c	2015-02-25 13:43:06.929973954 -0600
+++ linux/fs/proc/array.c	2015-02-25 13:43:06.925972078 -0600
@@ -302,7 +302,8 @@ static void render_cap_t(struct seq_file
 static inline void task_cap(struct seq_file *m, struct task_struct *p)
 {
 	const struct cred *cred;
-	kernel_cap_t cap_inheritable, cap_permitted, cap_effective, cap_bset;
+	kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
+			cap_bset, cap_ambient;

 	rcu_read_lock();
 	cred = __task_cred(p);
@@ -310,12 +311,14 @@ static inline void task_cap(struct seq_f
 	cap_permitted	= cred->cap_permitted;
 	cap_effective	= cred->cap_effective;
 	cap_bset	= cred->cap_bset;
+	cap_ambient	= cred->cap_ambient;
 	rcu_read_unlock();

 	render_cap_t(m, "CapInh:\t", &cap_inheritable);
 	render_cap_t(m, "CapPrm:\t", &cap_permitted);
 	render_cap_t(m, "CapEff:\t", &cap_effective);
 	render_cap_t(m, "CapBnd:\t", &cap_bset);
+	render_cap_t(m, "CapAmb:\t", &cap_ambient);
 }

 static inline void task_seccomp(struct seq_file *m, struct task_struct *p)

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 04:14:33PM -0600, Christoph Lameter wrote:
> 
> V1->V2:
>  - Fix up the processing of the caps bits after discussions
>    with Any and Serge. Make patch less intrusive.
> 
> Ambient caps are something like restricted root privileges.
> A process has a set of additional capabilities and those
> are inherited without have to set capabilites in other
> binaries involved. This allow the partial use of root
> like features in a controlled way. It is often useful
> to do this for user space device drivers or software that
> needs increased priviledges for networking or to control
> its own scheduling. Ambient caps allow one to avoid
> having to run these with full root priviledges.
> 
> Control over this feature is avaialable via a new
> prctl option called PR_CAP_AMBIENT. The second argument to prctl
> is a the capability number and the third the desired state.
> 0 for off. Otherwise on.
> 
> Ambient bits are enabled regardless of the inheritance
> mask of the target binary. They are only restricted
> by the bounding set.
> 
> History:
> 
> Linux capabilities have suffered from the problem that they are not
> inheritable like unregular process characteristics under Unix. This is
> behavior that is counter intuitive to the expected behavior of processes
> in Unix.
> 
> In particular there has been recently software that controls NICs from user
> space and provides IP stack like behavior also in user space (DPDK and RDMA
> kernel API based implementations). Those typically need either capabilities
> to allow raw network access or have to be run setsuid. There is scripting and
> LD_PREFLOAD etc involved, arbitrary binaries may be run from those scripts
> including those setting additional capabilites or requiring root access.
> 
> That does not go well with having file capabilities set that would enable
> the capabilities. Maybe it would work if one would setup capabilities on
> all executables but that would also defeat a secure design since these
> binaries may only need those caps for certain situations. Ok setting the
> inheritable flags on everything may also get one there (if there would not
> be the issues with LD_PRELOAD, debugging etc etc).
> 
> The easy solution is to allow some capabilities be inherited like setsuid
> is. We really prefer to use capabilities instead of setsuid (we want to
> limit what damage someone can do after all!). Therefore we have been
> running a patch like this in production for the last 6 years. At some
> point it becomes tedious to run your own custom kernel so we would like
> to have this functionality upstream.
> 
> See some of the earlier related discussions on the problems with capability
> inheritance:
> 
> 0. Recent surprise:
>                 https://lkml.org/lkml/2014/1/21/175
> 
> 1. Attempt to revise caps
>                 http://www.madore.org/~david/linux/newcaps/
> 
> 2. Problems of passing caps through exec
>                 http://unix.stackexchange.com/questions/128394/passing-capabilities-through-exec
> 
> 3. Problems of binding to privileged ports
>                 http://stackoverflow.com/questions/413807/is-there-a-way-for-non-root-processes-to-bind-to-privileged-ports-1024-on-l
> 
> 4. Reviving capabilities
>                 http://lwn.net/Articles/199004/
> 
> There does not seem to be an alternative on the horizon. Some involved
> in security development under Linux have even stated that they want to
> rip out the whole thing and replace it. Its been a couple of years now
> and we are still suffering from the capabilities mess. Let us just
> fix it. Others have already done implementations like this like Nokia
> for the N900.
> 
> 
> This patch does not change the default behavior but it allows to set up
> a list of capabilities via prctl that will enable regular
> unix inheritance only for the selected group of capabilities.
> 
> With that it is then possible to do something trivial like setting
> CAP_NET_RAW on an executable that can then allow that capability to
> be inherited by others.
> 
> Lets have a look at a coding example of a wrapper that enables
> a couple of capabilities:
> 
> ------------------------------ ambient_test.c
> /*
>  * Test program for the ambient capabilities
>  *
>  *
>  * Compile using:
>  *	gcc -o ambient_test ambient_test.o
>  *
>  * This program must have the following capabilities to run properly:
>  * CAP_SETPCAP, CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_NICE
>  *
>  * A command to equip this with the right caps is:
>  *
>  *	setcap cap_setpcap,cap_net_raw,cap_net_admin,cap_sys_nice+eip ambient_test
>  *
>  * To get a shell with additional caps that can be inherited do:
>  *
>  * ./ambient_test /bin/bash
>  *
>  */
> 
> #include <stdlib.h>
> #include <stdio.h>
> #include <errno.h>
> #include <sys/prctl.h>
> #include <linux/capability.h>
> 
> /* Defintion to be updated in the user space include files */
> #define PR_CAP_AMBIENT 45
> 
> int main(int argc, char **argv)
> {
> 	int rc;
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_NET_RAW))
> 		perror("Cannot set CAP_NET_RAW");
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_NET_ADMIN))
> 		perror("Cannot set CAP_NET_ADMIN");
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_SYS_NICE))
> 		perror("Cannot set CAP_SYS_NICE");
> 

Your example program is not filling in pI though?

Ah, i see why.  In get_file_caps() you are still assigning

	fP = pA

if the file has no file capabilities.  so then you are actually
doing

	 pP' = (X & (fP | pA)) | (pI & (fI | pA))
rather than
	 pP' = (X & fP) | (pI & (fI | pA))

Other than that, the patch is looking good to me.  We should
consider emitting an audit record when a task fills in its
pA, and I do still wonder whether we should be requiring
CAP_SETFCAP (unsure how best to think of it).  But assuming the
fP = pA was not intended, I think this largely does the right
thing.

> 	printf("Ambient_test forking shell\n");
> 	if (execv(argv[1], argv + 1))
> 		perror("Cannot exec");
> 
> 	return 0;
> }
> -------------------------------- ambient_test.c
> 
> Allows the inheritance of CAP_SYS_NICE, CAP_NET_RAW and CAP_NET_ADMIN.
> With that device raw access is possible and also real time priorities
> can be set from user space. This is a frequently needed set of
> priviledged operations in HPC and HFT applications. User space
> processes need to be able to directly access devices as well as
> have full control over scheduling.
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> 
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c	2015-02-25 13:43:06.929973954 -0600
> +++ linux/security/commoncap.c	2015-02-26 16:10:02.347913397 -0600
> @@ -347,15 +347,17 @@ static inline int bprm_caps_from_vfs_cap
>  		*has_cap = true;
> 
>  	CAP_FOR_EACH_U32(i) {
> +		__u32 ambient = current_cred()->cap_ambient.cap[i];
>  		__u32 permitted = caps->permitted.cap[i];
>  		__u32 inheritable = caps->inheritable.cap[i];
> 
>  		/*
> -		 * pP' = (X & fP) | (pI & fI)
> +		 * pP' = (X & fP) | (pI & (fI | pA))
>  		 */
>  		new->cap_permitted.cap[i] =
>  			(new->cap_bset.cap[i] & permitted) |
> -			(new->cap_inheritable.cap[i] & inheritable);
> +			(new->cap_inheritable.cap[i] &
> +					(inheritable | ambient));
> 
>  		if (permitted & ~new->cap_permitted.cap[i])
>  			/* insufficient to execute correctly */
> @@ -453,8 +455,18 @@ static int get_file_caps(struct linux_bi
>  		if (rc == -EINVAL)
>  			printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
>  				__func__, rc, bprm->filename);
> -		else if (rc == -ENODATA)
> +		else if (rc == -ENODATA) {
>  			rc = 0;
> +			if (!cap_isclear(current_cred()->cap_ambient)) {
> +				/*
> +				 * The ambient caps are permitted for
> +				 * files that have no caps
> +				 */
> +				bprm->cred->cap_permitted =
> +					current_cred()->cap_ambient;
> +				*effective = true;
> +			}
> +		}
>  		goto out;
>  	}
> 
> @@ -549,9 +561,20 @@ skip:
>  	new->sgid = new->fsgid = new->egid;
> 
>  	if (effective)
> +		/*
> +		 * pE' = pP' & (fE | pA)
> +		 *
> +		 * fE is implicity all set if effective == true.
> +		 * Therefore the above reduces to
> +		 *
> +		 * pE' = pP'
> +		 */
>  		new->cap_effective = new->cap_permitted;
>  	else
>  		cap_clear(new->cap_effective);
> +
> +	/* pA' = pA */
> +	new->cap_ambient = old->cap_ambient;
>  	bprm->cap_effective = effective;
> 
>  	/*
> @@ -566,7 +589,7 @@ skip:
>  	 * Number 1 above might fail if you don't have a full bset, but I think
>  	 * that is interesting information to audit.
>  	 */
> -	if (!cap_isclear(new->cap_effective)) {
> +	if (!cap_issubset(new->cap_effective, new->cap_ambient)) {
>  		if (!cap_issubset(CAP_FULL_SET, new->cap_effective) ||
>  		    !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) ||
>  		    issecure(SECURE_NOROOT)) {
> @@ -598,7 +621,7 @@ int cap_bprm_secureexec(struct linux_bin
>  	if (!uid_eq(cred->uid, root_uid)) {
>  		if (bprm->cap_effective)
>  			return 1;
> -		if (!cap_isclear(cred->cap_permitted))
> +		if (!cap_issubset(cred->cap_permitted, cred->cap_ambient))
>  			return 1;
>  	}
> 
> @@ -933,6 +956,23 @@ int cap_task_prctl(int option, unsigned
>  			new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
>  		return commit_creds(new);
> 
> +	case PR_CAP_AMBIENT:
> +		if (!ns_capable(current_user_ns(), CAP_SETPCAP))
> +			return -EPERM;
> +
> +		if (!cap_valid(arg2))
> +			return -EINVAL;
> +
> +		if (!ns_capable(current_user_ns(), arg2))
> +			return -EPERM;
> +
> +		new = prepare_creds();
> +		if (arg3 == 0)
> +			cap_lower(new->cap_ambient, arg2);
> +		else
> +			cap_raise(new->cap_ambient, arg2);
> +		return commit_creds(new);
> +
>  	default:
>  		/* No functionality available - continue with default */
>  		return -ENOSYS;
> Index: linux/include/linux/cred.h
> ===================================================================
> --- linux.orig/include/linux/cred.h	2015-02-25 13:43:06.929973954 -0600
> +++ linux/include/linux/cred.h	2015-02-25 13:43:06.925972078 -0600
> @@ -122,6 +122,7 @@ struct cred {
>  	kernel_cap_t	cap_permitted;	/* caps we're permitted */
>  	kernel_cap_t	cap_effective;	/* caps we can actually use */
>  	kernel_cap_t	cap_bset;	/* capability bounding set */
> +	kernel_cap_t	cap_ambient;	/* Ambient capability set */
>  #ifdef CONFIG_KEYS
>  	unsigned char	jit_keyring;	/* default keyring to attach requested
>  					 * keys to */
> Index: linux/include/uapi/linux/prctl.h
> ===================================================================
> --- linux.orig/include/uapi/linux/prctl.h	2015-02-25 13:43:06.929973954 -0600
> +++ linux/include/uapi/linux/prctl.h	2015-02-25 13:43:06.925972078 -0600
> @@ -185,4 +185,7 @@ struct prctl_mm_map {
>  #define PR_MPX_ENABLE_MANAGEMENT  43
>  #define PR_MPX_DISABLE_MANAGEMENT 44
> 
> +/* Control the ambient capability set */
> +#define PR_CAP_AMBIENT 45
> +
>  #endif /* _LINUX_PRCTL_H */
> Index: linux/fs/proc/array.c
> ===================================================================
> --- linux.orig/fs/proc/array.c	2015-02-25 13:43:06.929973954 -0600
> +++ linux/fs/proc/array.c	2015-02-25 13:43:06.925972078 -0600
> @@ -302,7 +302,8 @@ static void render_cap_t(struct seq_file
>  static inline void task_cap(struct seq_file *m, struct task_struct *p)
>  {
>  	const struct cred *cred;
> -	kernel_cap_t cap_inheritable, cap_permitted, cap_effective, cap_bset;
> +	kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> +			cap_bset, cap_ambient;
> 
>  	rcu_read_lock();
>  	cred = __task_cred(p);
> @@ -310,12 +311,14 @@ static inline void task_cap(struct seq_f
>  	cap_permitted	= cred->cap_permitted;
>  	cap_effective	= cred->cap_effective;
>  	cap_bset	= cred->cap_bset;
> +	cap_ambient	= cred->cap_ambient;
>  	rcu_read_unlock();
> 
>  	render_cap_t(m, "CapInh:\t", &cap_inheritable);
>  	render_cap_t(m, "CapPrm:\t", &cap_permitted);
>  	render_cap_t(m, "CapEff:\t", &cap_effective);
>  	render_cap_t(m, "CapBnd:\t", &cap_bset);
> +	render_cap_t(m, "CapAmb:\t", &cap_ambient);
>  }
> 
>  static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Sat, 28 Feb 2015, Serge E. Hallyn wrote:

> Your example program is not filling in pI though?

The setcap sets the inheritance bit. When the binary runs the i bits
should be set.

> Ah, i see why.  In get_file_caps() you are still assigning
>
> 	fP = pA
>
> if the file has no file capabilities.  so then you are actually
> doing
>
> 	 pP' = (X & (fP | pA)) | (pI & (fI | pA))
> rather than
> 	 pP' = (X & fP) | (pI & (fI | pA))


I thought that fP, fI and pI = {} since the file has no caps
so this comes  out as

pP' = pA

> Other than that, the patch is looking good to me.  We should
> consider emitting an audit record when a task fills in its

How do I do that?

> pA, and I do still wonder whether we should be requiring
> CAP_SETFCAP (unsure how best to think of it).  But assuming the
> fP = pA was not intended, I think this largely does the right
> thing.

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 04:14:33PM -0600, Christoph Lameter wrote:
> 
> V1->V2:
>  - Fix up the processing of the caps bits after discussions
>    with Any and Serge. Make patch less intrusive.
> 
> Ambient caps are something like restricted root privileges.
> A process has a set of additional capabilities and those
> are inherited without have to set capabilites in other
> binaries involved. This allow the partial use of root
> like features in a controlled way. It is often useful
> to do this for user space device drivers or software that
> needs increased priviledges for networking or to control
> its own scheduling. Ambient caps allow one to avoid
> having to run these with full root priviledges.
> 
> Control over this feature is avaialable via a new
> prctl option called PR_CAP_AMBIENT. The second argument to prctl
> is a the capability number and the third the desired state.
> 0 for off. Otherwise on.
> 
> Ambient bits are enabled regardless of the inheritance
> mask of the target binary. They are only restricted
> by the bounding set.
> 
> History:
> 
> Linux capabilities have suffered from the problem that they are not
> inheritable like unregular process characteristics under Unix. This is
> behavior that is counter intuitive to the expected behavior of processes
> in Unix.
> 
> In particular there has been recently software that controls NICs from user
> space and provides IP stack like behavior also in user space (DPDK and RDMA
> kernel API based implementations). Those typically need either capabilities
> to allow raw network access or have to be run setsuid. There is scripting and
> LD_PREFLOAD etc involved, arbitrary binaries may be run from those scripts
> including those setting additional capabilites or requiring root access.
> 
> That does not go well with having file capabilities set that would enable
> the capabilities. Maybe it would work if one would setup capabilities on
> all executables but that would also defeat a secure design since these
> binaries may only need those caps for certain situations. Ok setting the
> inheritable flags on everything may also get one there (if there would not
> be the issues with LD_PRELOAD, debugging etc etc).
> 
> The easy solution is to allow some capabilities be inherited like setsuid
> is. We really prefer to use capabilities instead of setsuid (we want to
> limit what damage someone can do after all!). Therefore we have been
> running a patch like this in production for the last 6 years. At some
> point it becomes tedious to run your own custom kernel so we would like
> to have this functionality upstream.
> 
> See some of the earlier related discussions on the problems with capability
> inheritance:
> 
> 0. Recent surprise:
>                 https://lkml.org/lkml/2014/1/21/175
> 
> 1. Attempt to revise caps
>                 http://www.madore.org/~david/linux/newcaps/
> 
> 2. Problems of passing caps through exec
>                 http://unix.stackexchange.com/questions/128394/passing-capabilities-through-exec
> 
> 3. Problems of binding to privileged ports
>                 http://stackoverflow.com/questions/413807/is-there-a-way-for-non-root-processes-to-bind-to-privileged-ports-1024-on-l
> 
> 4. Reviving capabilities
>                 http://lwn.net/Articles/199004/
> 
> There does not seem to be an alternative on the horizon. Some involved
> in security development under Linux have even stated that they want to
> rip out the whole thing and replace it. Its been a couple of years now
> and we are still suffering from the capabilities mess. Let us just
> fix it. Others have already done implementations like this like Nokia
> for the N900.
> 
> 
> This patch does not change the default behavior but it allows to set up
> a list of capabilities via prctl that will enable regular
> unix inheritance only for the selected group of capabilities.
> 
> With that it is then possible to do something trivial like setting
> CAP_NET_RAW on an executable that can then allow that capability to
> be inherited by others.
> 
> Lets have a look at a coding example of a wrapper that enables
> a couple of capabilities:
> 
> ------------------------------ ambient_test.c
> /*
>  * Test program for the ambient capabilities
>  *
>  *
>  * Compile using:
>  *	gcc -o ambient_test ambient_test.o
>  *
>  * This program must have the following capabilities to run properly:
>  * CAP_SETPCAP, CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_NICE
>  *
>  * A command to equip this with the right caps is:
>  *
>  *	setcap cap_setpcap,cap_net_raw,cap_net_admin,cap_sys_nice+eip ambient_test
>  *
>  * To get a shell with additional caps that can be inherited do:
>  *
>  * ./ambient_test /bin/bash
>  *
>  */
> 
> #include <stdlib.h>
> #include <stdio.h>
> #include <errno.h>
> #include <sys/prctl.h>
> #include <linux/capability.h>
> 
> /* Defintion to be updated in the user space include files */
> #define PR_CAP_AMBIENT 45
> 
> int main(int argc, char **argv)
> {
> 	int rc;
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_NET_RAW))
> 		perror("Cannot set CAP_NET_RAW");
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_NET_ADMIN))
> 		perror("Cannot set CAP_NET_ADMIN");
> 
> 	if (prctl(PR_CAP_AMBIENT, CAP_SYS_NICE))
> 		perror("Cannot set CAP_SYS_NICE");
> 
> 	printf("Ambient_test forking shell\n");
> 	if (execv(argv[1], argv + 1))
> 		perror("Cannot exec");
> 
> 	return 0;
> }
> -------------------------------- ambient_test.c
> 
> Allows the inheritance of CAP_SYS_NICE, CAP_NET_RAW and CAP_NET_ADMIN.
> With that device raw access is possible and also real time priorities
> can be set from user space. This is a frequently needed set of
> priviledged operations in HPC and HFT applications. User space
> processes need to be able to directly access devices as well as
> have full control over scheduling.
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> 
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c	2015-02-25 13:43:06.929973954 -0600
> +++ linux/security/commoncap.c	2015-02-26 16:10:02.347913397 -0600
> @@ -347,15 +347,17 @@ static inline int bprm_caps_from_vfs_cap
>  		*has_cap = true;
> 
>  	CAP_FOR_EACH_U32(i) {
> +		__u32 ambient = current_cred()->cap_ambient.cap[i];
>  		__u32 permitted = caps->permitted.cap[i];
>  		__u32 inheritable = caps->inheritable.cap[i];
> 
>  		/*
> -		 * pP' = (X & fP) | (pI & fI)
> +		 * pP' = (X & fP) | (pI & (fI | pA))
>  		 */
>  		new->cap_permitted.cap[i] =
>  			(new->cap_bset.cap[i] & permitted) |
> -			(new->cap_inheritable.cap[i] & inheritable);
> +			(new->cap_inheritable.cap[i] &
> +					(inheritable | ambient));

So I'd say drop this change ^

> 
>  		if (permitted & ~new->cap_permitted.cap[i])
>  			/* insufficient to execute correctly */
> @@ -453,8 +455,18 @@ static int get_file_caps(struct linux_bi
>  		if (rc == -EINVAL)
>  			printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
>  				__func__, rc, bprm->filename);
> -		else if (rc == -ENODATA)
> +		else if (rc == -ENODATA) {
>  			rc = 0;
> +			if (!cap_isclear(current_cred()->cap_ambient)) {
> +				/*
> +				 * The ambient caps are permitted for
> +				 * files that have no caps
> +				 */
> +				bprm->cred->cap_permitted =
> +					current_cred()->cap_ambient;

and here set vcaps inheritable to current_cred()->ambient.

> +				*effective = true;
> +			}
> +		}
>  		goto out;
>  	}
> 
> @@ -549,9 +561,20 @@ skip:
>  	new->sgid = new->fsgid = new->egid;
> 
>  	if (effective)
> +		/*
> +		 * pE' = pP' & (fE | pA)
> +		 *
> +		 * fE is implicity all set if effective == true.
> +		 * Therefore the above reduces to
> +		 *
> +		 * pE' = pP'
> +		 */
>  		new->cap_effective = new->cap_permitted;
>  	else
>  		cap_clear(new->cap_effective);
> +
> +	/* pA' = pA */
> +	new->cap_ambient = old->cap_ambient;
>  	bprm->cap_effective = effective;
> 
>  	/*
> @@ -566,7 +589,7 @@ skip:
>  	 * Number 1 above might fail if you don't have a full bset, but I think
>  	 * that is interesting information to audit.
>  	 */
> -	if (!cap_isclear(new->cap_effective)) {
> +	if (!cap_issubset(new->cap_effective, new->cap_ambient)) {
>  		if (!cap_issubset(CAP_FULL_SET, new->cap_effective) ||
>  		    !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) ||
>  		    issecure(SECURE_NOROOT)) {
> @@ -598,7 +621,7 @@ int cap_bprm_secureexec(struct linux_bin
>  	if (!uid_eq(cred->uid, root_uid)) {
>  		if (bprm->cap_effective)
>  			return 1;
> -		if (!cap_isclear(cred->cap_permitted))
> +		if (!cap_issubset(cred->cap_permitted, cred->cap_ambient))
>  			return 1;
>  	}
> 
> @@ -933,6 +956,23 @@ int cap_task_prctl(int option, unsigned
>  			new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
>  		return commit_creds(new);
> 
> +	case PR_CAP_AMBIENT:
> +		if (!ns_capable(current_user_ns(), CAP_SETPCAP))
> +			return -EPERM;
> +
> +		if (!cap_valid(arg2))
> +			return -EINVAL;
> +
> +		if (!ns_capable(current_user_ns(), arg2))
> +			return -EPERM;
> +
> +		new = prepare_creds();
> +		if (arg3 == 0)
> +			cap_lower(new->cap_ambient, arg2);
> +		else
> +			cap_raise(new->cap_ambient, arg2);
> +		return commit_creds(new);
> +
>  	default:
>  		/* No functionality available - continue with default */
>  		return -ENOSYS;
> Index: linux/include/linux/cred.h
> ===================================================================
> --- linux.orig/include/linux/cred.h	2015-02-25 13:43:06.929973954 -0600
> +++ linux/include/linux/cred.h	2015-02-25 13:43:06.925972078 -0600
> @@ -122,6 +122,7 @@ struct cred {
>  	kernel_cap_t	cap_permitted;	/* caps we're permitted */
>  	kernel_cap_t	cap_effective;	/* caps we can actually use */
>  	kernel_cap_t	cap_bset;	/* capability bounding set */
> +	kernel_cap_t	cap_ambient;	/* Ambient capability set */
>  #ifdef CONFIG_KEYS
>  	unsigned char	jit_keyring;	/* default keyring to attach requested
>  					 * keys to */
> Index: linux/include/uapi/linux/prctl.h
> ===================================================================
> --- linux.orig/include/uapi/linux/prctl.h	2015-02-25 13:43:06.929973954 -0600
> +++ linux/include/uapi/linux/prctl.h	2015-02-25 13:43:06.925972078 -0600
> @@ -185,4 +185,7 @@ struct prctl_mm_map {
>  #define PR_MPX_ENABLE_MANAGEMENT  43
>  #define PR_MPX_DISABLE_MANAGEMENT 44
> 
> +/* Control the ambient capability set */
> +#define PR_CAP_AMBIENT 45
> +
>  #endif /* _LINUX_PRCTL_H */
> Index: linux/fs/proc/array.c
> ===================================================================
> --- linux.orig/fs/proc/array.c	2015-02-25 13:43:06.929973954 -0600
> +++ linux/fs/proc/array.c	2015-02-25 13:43:06.925972078 -0600
> @@ -302,7 +302,8 @@ static void render_cap_t(struct seq_file
>  static inline void task_cap(struct seq_file *m, struct task_struct *p)
>  {
>  	const struct cred *cred;
> -	kernel_cap_t cap_inheritable, cap_permitted, cap_effective, cap_bset;
> +	kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> +			cap_bset, cap_ambient;
> 
>  	rcu_read_lock();
>  	cred = __task_cred(p);
> @@ -310,12 +311,14 @@ static inline void task_cap(struct seq_f
>  	cap_permitted	= cred->cap_permitted;
>  	cap_effective	= cred->cap_effective;
>  	cap_bset	= cred->cap_bset;
> +	cap_ambient	= cred->cap_ambient;
>  	rcu_read_unlock();
> 
>  	render_cap_t(m, "CapInh:\t", &cap_inheritable);
>  	render_cap_t(m, "CapPrm:\t", &cap_permitted);
>  	render_cap_t(m, "CapEff:\t", &cap_effective);
>  	render_cap_t(m, "CapBnd:\t", &cap_bset);
> +	render_cap_t(m, "CapAmb:\t", &cap_ambient);
>  }
> 
>  static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Sun, 1 Mar 2015, Serge E. Hallyn wrote:

> > +++ linux/security/commoncap.c	2015-02-26 16:10:02.347913397 -0600
> > @@ -347,15 +347,17 @@ static inline int bprm_caps_from_vfs_cap
> >  		*has_cap = true;
> >
> >  	CAP_FOR_EACH_U32(i) {
> > +		__u32 ambient = current_cred()->cap_ambient.cap[i];
> >  		__u32 permitted = caps->permitted.cap[i];
> >  		__u32 inheritable = caps->inheritable.cap[i];
> >
> >  		/*
> > -		 * pP' = (X & fP) | (pI & fI)
> > +		 * pP' = (X & fP) | (pI & (fI | pA))
> >  		 */
> >  		new->cap_permitted.cap[i] =
> >  			(new->cap_bset.cap[i] & permitted) |
> > -			(new->cap_inheritable.cap[i] & inheritable);
> > +			(new->cap_inheritable.cap[i] &
> > +					(inheritable | ambient));
>
> So I'd say drop this change ^

Then the ambient caps get ignored for a executables that have capabilities
seton the file? I think we need to keep this one.

> > @@ -453,8 +455,18 @@ static int get_file_caps(struct linux_bi
> >  		if (rc == -EINVAL)
> >  			printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
> >  				__func__, rc, bprm->filename);
> > -		else if (rc == -ENODATA)
> > +		else if (rc == -ENODATA) {
> >  			rc = 0;
> > +			if (!cap_isclear(current_cred()->cap_ambient)) {
> > +				/*
> > +				 * The ambient caps are permitted for
> > +				 * files that have no caps
> > +				 */
> > +				bprm->cred->cap_permitted =
> > +					current_cred()->cap_ambient;
>
> and here set vcaps inheritable to current_cred()->ambient.

We do not call bprm_caps_from_vfs_cap() for files that have no caps so
this would have no effect. But we could set cap_inheritable here?

Fixup patch:



Subject: ambient_caps: Set inheritable bits too

We were not setting the inheritable bits as they ought to be set.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c	2015-03-05 09:22:32.123047869 -0600
+++ linux/security/commoncap.c	2015-03-05 09:22:32.119048001 -0600
@@ -461,6 +461,8 @@ static int get_file_caps(struct linux_bi
 				 */
 				bprm->cred->cap_permitted =
 					current_cred()->cap_ambient;
+				bprm->cred->cap_inheritable =
+					current_cred()->cap_ambient;
 				*effective = true;
 			}
 		}

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Thu, Mar 05, 2015 at 09:26:24AM -0600, Christoph Lameter wrote:
> On Sun, 1 Mar 2015, Serge E. Hallyn wrote:
> 
> > > +++ linux/security/commoncap.c	2015-02-26 16:10:02.347913397 -0600
> > > @@ -347,15 +347,17 @@ static inline int bprm_caps_from_vfs_cap
> > >  		*has_cap = true;
> > >
> > >  	CAP_FOR_EACH_U32(i) {
> > > +		__u32 ambient = current_cred()->cap_ambient.cap[i];
> > >  		__u32 permitted = caps->permitted.cap[i];
> > >  		__u32 inheritable = caps->inheritable.cap[i];
> > >
> > >  		/*
> > > -		 * pP' = (X & fP) | (pI & fI)
> > > +		 * pP' = (X & fP) | (pI & (fI | pA))
> > >  		 */
> > >  		new->cap_permitted.cap[i] =
> > >  			(new->cap_bset.cap[i] & permitted) |
> > > -			(new->cap_inheritable.cap[i] & inheritable);
> > > +			(new->cap_inheritable.cap[i] &
> > > +					(inheritable | ambient));
> >
> > So I'd say drop this change ^
> 
> Then the ambient caps get ignored for a executables that have capabilities
> seton the file?

Yes.  Those are assumed to already know what they're doing.

> I think we need to keep this one.

Why?  Do you foresee cases where a file that has fP set needs capabilities
that aren't in its fP?

It seems more likely that they'll risk misbehaving due to an unexpected set 
of caps.

If you have a good use case I'm not entirely opposed, but it just seems
unneeded and a potentially bad idea.

> > > @@ -453,8 +455,18 @@ static int get_file_caps(struct linux_bi
> > >  		if (rc == -EINVAL)
> > >  			printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
> > >  				__func__, rc, bprm->filename);
> > > -		else if (rc == -ENODATA)
> > > +		else if (rc == -ENODATA) {
> > >  			rc = 0;
> > > +			if (!cap_isclear(current_cred()->cap_ambient)) {
> > > +				/*
> > > +				 * The ambient caps are permitted for
> > > +				 * files that have no caps
> > > +				 */
> > > +				bprm->cred->cap_permitted =
> > > +					current_cred()->cap_ambient;
> >
> > and here set vcaps inheritable to current_cred()->ambient.
> 
> We do not call bprm_caps_from_vfs_cap() for files that have no caps so
> this would have no effect. But we could set cap_inheritable here?
> 
> Fixup patch:
> 
> 
> 
> Subject: ambient_caps: Set inheritable bits too
> 
> We were not setting the inheritable bits as they ought to be set.
> 
> Signed-off-by: Christoph Lameter <cl@linux.com>
> 
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c	2015-03-05 09:22:32.123047869 -0600
> +++ linux/security/commoncap.c	2015-03-05 09:22:32.119048001 -0600
> @@ -461,6 +461,8 @@ static int get_file_caps(struct linux_bi
>  				 */
>  				bprm->cred->cap_permitted =
>  					current_cred()->cap_ambient;
> +				bprm->cred->cap_inheritable =
> +					current_cred()->cap_ambient;
>  				*effective = true;
>  			}
>  		}

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Thu, 5 Mar 2015, Serge E. Hallyn wrote:

> > >
> > > So I'd say drop this change ^
> >
> > Then the ambient caps get ignored for a executables that have capabilities
> > seton the file?
>
> Yes.  Those are assumed to already know what they're doing.

Do they? What if there is a LD_PRELOAD library that redirects socket calls
and that needs raw device access (there are actually a number of software
packages like that to reduce the latency of network I/O. See for example
Solarflare's software products and the current rsocket libary in OFED.
There are cap issues if the rsocket library should be made useful for
Ethernet instead of infiniband).

> Why?  Do you foresee cases where a file that has fP set needs capabilities
> that aren't in its fP?

Yes due to the library issues.

> It seems more likely that they'll risk misbehaving due to an unexpected set
> of caps.

The userspace driver code in the library wont work since it does not have
the caps to access the raw device registers.

Re: [PATCH] capabilities: Ambient capability set V2

[Andy Lutomirski]

On Mar 5, 2015 10:41 AM, "Christoph Lameter" <cl@linux.com> wrote:
>
> On Thu, 5 Mar 2015, Serge E. Hallyn wrote:
>
> > > >
> > > > So I'd say drop this change ^
> > >
> > > Then the ambient caps get ignored for a executables that have capabilities
> > > seton the file?
> >
> > Yes.  Those are assumed to already know what they're doing.
>
> Do they? What if there is a LD_PRELOAD library that redirects socket calls
> and that needs raw device access (there are actually a number of software
> packages like that to reduce the latency of network I/O. See for example
> Solarflare's software products and the current rsocket libary in OFED.
> There are cap issues if the rsocket library should be made useful for
> Ethernet instead of infiniband).
>
> > Why?  Do you foresee cases where a file that has fP set needs capabilities
> > that aren't in its fP?
>
> Yes due to the library issues.

You can't LD_PRELOAD and fP together.  And I'm still unconvinced that
ambient caps can ever be safe in conjunction with fP.  I'll grill you
next week on what you're trying to do that makes you want this :)

--Andy

>
> > It seems more likely that they'll risk misbehaving due to an unexpected set
> > of caps.
>
> The userspace driver code in the library wont work since it does not have
> the caps to access the raw device registers.

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 825 bytes --]

On Thu, 5 Mar 2015, Andy Lutomirski wrote:

> > Yes due to the library issues.
>
> You can't LD_PRELOAD and fP together.  And I'm still unconvinced that
> ambient caps can ever be safe in conjunction with fP.  I'll grill you
> next week on what you're trying to do that makes you want this :)

>From the ld.so manpage:

    LD_PRELOAD
              A whitespace-separated list of additional, user-specified, ELF shared
              libraries to be loaded before all others.  This can be used to selec‐
              tively override functions in other shared libraries.  For setuid/set‐
              gid ELF binaries, only libraries in the standard  search  directories
              that are also setgid will be loaded.

So this mechanism has not been made to work for binaries with caps? We
have to keep using setuid?

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Thu, 5 Mar 2015, Serge E. Hallyn wrote:

> > > So I'd say drop this change ^
> >
> > Then the ambient caps get ignored for a executables that have capabilities
> > seton the file?
>
> Yes.  Those are assumed to already know what they're doing.

Ok can we get this patch merged now if I do this change
(effectively ambient caps for binaries that have no caps set) and deal with the
other issues later? This would cover most of the use cases here at least.

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Fri, Mar 06, 2015 at 09:50:02AM -0600, Christoph Lameter wrote:
> On Thu, 5 Mar 2015, Serge E. Hallyn wrote:
> 
> > > > So I'd say drop this change ^
> > >
> > > Then the ambient caps get ignored for a executables that have capabilities
> > > seton the file?
> >
> > Yes.  Those are assumed to already know what they're doing.
> 
> Ok can we get this patch merged now if I do this change
> (effectively ambient caps for binaries that have no caps set) and deal with the
> other issues later? This would cover most of the use cases here at least.

Sorry, something about that patch-patch didn't make sense to me, but I
need to look more closely.  My objection was that you were able to get the
pA capabilities into pP without them being in your pI.  Your proposed
change didn't seem like it would fix that.

It also seems worth waiting until you talk to Andy in person next week.

-serge

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Fri, 6 Mar 2015, Serge E. Hallyn wrote:

> Sorry, something about that patch-patch didn't make sense to me, but I
> need to look more closely.  My objection was that you were able to get the
> pA capabilities into pP without them being in your pI.  Your proposed
> change didn't seem like it would fix that.

Just tried to fix that. Could it be that cap_inherited is never set even
for a binary that has

christoph@fujitsu-haswell:~$ getcap ambient_test

ambient_test = cap_setpcap,cap_net_admin,cap_net_raw,cap_sys_nice+eip


I added some printks and it seems that current_cred()->cap_inherited is
not set when running ambient_test.

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c     2015-03-06 11:05:10.802218196
-0600
+++ linux/security/commoncap.c  2015-03-06 12:50:38.424330679 -0600
@@ -456,6 +456,10 @@ static int get_file_caps(struct linux_bi
                        kernel_cap_t relevant_ambient = cap_intersect(
                                current_cred()->cap_ambient,
                                current_cred()->cap_inheritable);
+                       printk("task->comm %s: Amb=%x Inh=%x relevant=%x\n",
+                               current->comm, current_cred()->cap_ambient.cap[0],
+                                               current_cred()->cap_inheritable.cap[0],
+                                               relevant_ambient.cap[0]);
                        rc = 0;
                        if (!cap_isclear(relevant_ambient)) {
                                /*



Mar  6 12:42:18 fujitsu-haswell kernel: [  284.715051] task->comm ambient_test: Amb=803000 Inh=0 relevant=0

Re: [PATCH] capabilities: Ambient capability set V2

[Andy Lutomirski]

On Fri, Mar 6, 2015 at 10:53 AM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 6 Mar 2015, Serge E. Hallyn wrote:
>
>> Sorry, something about that patch-patch didn't make sense to me, but I
>> need to look more closely.  My objection was that you were able to get the
>> pA capabilities into pP without them being in your pI.  Your proposed
>> change didn't seem like it would fix that.
>
> Just tried to fix that. Could it be that cap_inherited is never set even
> for a binary that has
>
> christoph@fujitsu-haswell:~$ getcap ambient_test
>
> ambient_test = cap_setpcap,cap_net_admin,cap_net_raw,cap_sys_nice+eip

I think that's right.  fI doesn't set pI.

--Andy

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Fri, Mar 06, 2015 at 11:02:43AM -0800, Andy Lutomirski wrote:
> On Fri, Mar 6, 2015 at 10:53 AM, Christoph Lameter <cl@linux.com> wrote:
> > On Fri, 6 Mar 2015, Serge E. Hallyn wrote:
> >
> >> Sorry, something about that patch-patch didn't make sense to me, but I
> >> need to look more closely.  My objection was that you were able to get the
> >> pA capabilities into pP without them being in your pI.  Your proposed
> >> change didn't seem like it would fix that.
> >
> > Just tried to fix that. Could it be that cap_inherited is never set even
> > for a binary that has
> >
> > christoph@fujitsu-haswell:~$ getcap ambient_test
> >
> > ambient_test = cap_setpcap,cap_net_admin,cap_net_raw,cap_sys_nice+eip
> 
> I think that's right.  fI doesn't set pI.

Right.  The idea is that for the running binary to get capability x in its
pP, its privileged ancestor must have set x in pI, and the binary itself
must be trusted with x in fI.

What we are doing is allowing bypassing fI using pA, without bypassing the
requirement for x to be in pI.  Since pI is intended to be filled (for 
instance) at login based on username/group, pI generally does not get cleared.
At the same time, any software which thinks it is running untrusted code
safely without privilege by clearing pI and pP won't be fooled by pA.

-serge

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Fri, 6 Mar 2015, Serge E. Hallyn wrote:

> > I think that's right.  fI doesn't set pI.
>
> Right.  The idea is that for the running binary to get capability x in its
> pP, its privileged ancestor must have set x in pI, and the binary itself
> must be trusted with x in fI.

The ancestor here is ambient_test and when it is run pI will not be set
despite the cap setting.

Therefore anything is spawns cannot have the inheritance bits set either.
This plainly does not make any sense whatsoever. If this is so as it seems
to be then we should be able to remove the inheritance bits because they
have no effect.

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Sat, Mar 07, 2015 at 09:09:05AM -0600, Christoph Lameter wrote:
> On Fri, 6 Mar 2015, Serge E. Hallyn wrote:
> 
> > > I think that's right.  fI doesn't set pI.
> >
> > Right.  The idea is that for the running binary to get capability x in its
> > pP, its privileged ancestor must have set x in pI, and the binary itself
> > must be trusted with x in fI.
> 
> The ancestor here is ambient_test and when it is run pI will not be set
> despite the cap setting.

ambient_test is supposed to set it.

> Therefore anything is spawns cannot have the inheritance bits set either.
> This plainly does not make any sense whatsoever. If this is so as it seems
> to be then we should be able to remove the inheritance bits because they
> have no effect.
> 

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Sat, 7 Mar 2015, Serge E. Hallyn wrote:

> > The ancestor here is ambient_test and when it is run pI will not be set
> > despite the cap setting.
>
> ambient_test is supposed to set it.

I thought the setcap +i would do it.

So the setcap and setting of the file inheritance bits has no effect on
pI? When the process starts pI is off despite fI being set?

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Mon, Mar 09, 2015 at 07:05:24AM -0500, Christoph Lameter wrote:
> On Sat, 7 Mar 2015, Serge E. Hallyn wrote:
> 
> > > The ancestor here is ambient_test and when it is run pI will not be set
> > > despite the cap setting.
> >
> > ambient_test is supposed to set it.
> 
> I thought the setcap +i would do it.
> 
> So the setcap and setting of the file inheritance bits has no effect on
> pI? When the process starts pI is off despite fI being set?

Correct, pI must be set through capset().  Again, x in fI is saying
that the certain trusted users may have x in pP when they run the
binary;  x in pi means that the users may have x in pP when they run
certain files.  Other users running the file won't have x in pP, and
the special user running other files won't have x in pP.

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Mon, 9 Mar 2015, Andrew G. Morgan wrote:

> If this is new info, perhaps you might reconsider the rationale for your
> patch? I suspect you are focused on addressing a problem that you felt was
> unaddressed before, but given how much appears to have been unclear to you
> about the current implementation it might be worth a pause for thought.

The problems with unclear documentation and a weird counterintuitive
implementation that lots of people have trouble to use do not impact the
approach as far as I can tell. The discovery about how to set inheritable
bits does not help with the use cases here.

Even with this patch there is still the need to write a wrapper to get
the functionality that one would expect to just be possible by setting
inheritable bits on a file. The necessity to set the bits via prctl
in the wrapper complicates matters further and makes it even more
difficult than we thought before to make use of this feature.

> http://ols.fedoraproject.org/OLS/Reprints-2008/hallyn-reprint.pdf

Well maybe one should make sure that this info is properly comunicated
in the man pages and related documentation? This seems to be a big decade
old desaster. I need a Ph.D. in capabilities in order to attempt to use
them (but then oww no they still are not able to handle my use cases).

We get security through obscurity and also have then the inabilty to make
effective use of capabilities? Security measures need to follow
basic conventions, be obvious and easily understandable as well as well
documented. There is a huge risk of a sysadmin misconfiguring one of the
multiple measures that one needs to go through in order to gain some
sort of inheritance of capabilities and thereby adding more functionality
than necessary just in order to get it to work.

The best measure would be to make the inheritance bits work as one
would naturally expect. They just allow full inheritance of the caps.
No wrappers needed and its easily understood what it does.

Re: [PATCH] capabilities: Ambient capability set V2

[Christoph Lameter]

On Fri, 6 Mar 2015, Andy Lutomirski wrote:

> > christoph@fujitsu-haswell:~$ getcap ambient_test
> >
> > ambient_test = cap_setpcap,cap_net_admin,cap_net_raw,cap_sys_nice+eip
>
> I think that's right.  fI doesn't set pI.

Ok then that is the point of pI if it cannot be set?

Re: [PATCH] capabilities: Ambient capability set V2

[Serge E. Hallyn]

On Sat, Mar 07, 2015 at 09:06:46AM -0600, Christoph Lameter wrote:
> On Fri, 6 Mar 2015, Andy Lutomirski wrote:
> 
> > > christoph@fujitsu-haswell:~$ getcap ambient_test
> > >
> > > ambient_test = cap_setpcap,cap_net_admin,cap_net_raw,cap_sys_nice+eip
> >
> > I think that's right.  fI doesn't set pI.
> 
> Ok then that is the point of pI if it cannot be set?

It can be set!  Anything with CAP_SETPCAP can fill it's pI.  When
it and its children exec(), pI' = pI.

Re: [PATCH] capabilities: Ambient capability set V2

[Pavel Machek]

Hi!

>  		return -ENOSYS;
> Index: linux/include/linux/cred.h
> ===================================================================
> --- linux.orig/include/linux/cred.h	2015-02-25 13:43:06.929973954 -0600
> +++ linux/include/linux/cred.h	2015-02-25 13:43:06.925972078 -0600
> @@ -122,6 +122,7 @@ struct cred {
>  	kernel_cap_t	cap_permitted;	/* caps we're permitted */
>  	kernel_cap_t	cap_effective;	/* caps we can actually use */
>  	kernel_cap_t	cap_bset;	/* capability bounding set */
> +	kernel_cap_t	cap_ambient;	/* Ambient capability set */

lowercase "Ambient", for consistency?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html