[PATCH] capabilities: Ambient capability set V2

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 <redacted>

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)

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.

On Thu, Feb 26, 2015 at 04:14:33PM -0600, Christoph Lameter wrote:

So I'd say drop this change ^

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

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.

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

quoted

+++ 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.

quoted

@@ -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 <redacted>

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;
 			}
 		}

On Thu, Mar 05, 2015 at 09:26:24AM -0600, Christoph Lameter wrote:

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

quoted

quoted

+++ 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.

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

quoted

quoted

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.

On Mar 5, 2015 10:41 AM, "Christoph Lameter" [off-list ref] wrote:

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

quoted

quoted

quoted

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).

quoted

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

quoted

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.

On Thu, 5 Mar 2015, Andy Lutomirski wrote:

quoted

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?

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

quoted

quoted

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.

On Fri, Mar 06, 2015 at 09:50:02AM -0600, Christoph Lameter wrote:

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

quoted

quoted

quoted

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

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

On Fri, Mar 6, 2015 at 10:53 AM, Christoph Lameter [off-list ref] wrote:

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

quoted

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

On Fri, Mar 06, 2015 at 11:02:43AM -0800, Andy Lutomirski wrote:

On Fri, Mar 6, 2015 at 10:53 AM, Christoph Lameter [off-list ref] wrote:

quoted

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

quoted

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

On Fri, 6 Mar 2015, Andy Lutomirski wrote:

quoted

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?

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

quoted

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.

On Sat, Mar 07, 2015 at 09:06:46AM -0600, Christoph Lameter wrote:

On Fri, 6 Mar 2015, Andy Lutomirski wrote:

quoted

quoted

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.

On Sat, Mar 07, 2015 at 09:09:05AM -0600, Christoph Lameter wrote:

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

quoted

quoted

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.

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

quoted

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?

On Mon, Mar 09, 2015 at 07:05:24AM -0500, Christoph Lameter wrote:

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

quoted

quoted

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.

Hi!

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