[PATCH] capabilities: Ambient capability set V1

[PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

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@linux.com>

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c	2015-02-05 09:53:43.442883383 -0600
+++ linux/security/commoncap.c	2015-02-05 15:40:31.387388142 -0600
@@ -347,14 +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];
+		__u32 x = new->cap_bset.cap[i];

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

 		if (permitted & ~new->cap_permitted.cap[i])
@@ -453,9 +456,13 @@ 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)
-			rc = 0;
-		goto out;
+		else if (rc != -ENODATA)
+			goto out;
+		rc = 0;
+		if (cap_isclear(current_cred()->cap_ambient))
+			goto out;
+		/* Make sure that the ambient caps are enabled */
+		*effective = true;
 	}

 	rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, has_cap);
@@ -577,6 +584,7 @@ skip:
 	}

 	new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
+	new->cap_ambient = old->cap_ambient;
 	return 0;
 }

@@ -933,6 +941,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-05 09:53:43.442883383 -0600
+++ linux/include/linux/cred.h	2015-02-05 09:53:43.438883512 -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-05 09:53:43.442883383 -0600
+++ linux/include/uapi/linux/prctl.h	2015-02-05 09:53:43.438883512 -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	2014-12-12 10:27:49.304801274 -0600
+++ linux/fs/proc/array.c	2015-02-05 11:04:38.546429870 -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)

[PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

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-05 09:53:43.442883383 -0600
+++ linux/security/commoncap.c	2015-02-05 15:40:31.387388142 -0600
@@ -347,14 +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];
+		__u32 x = new->cap_bset.cap[i];

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

 		if (permitted & ~new->cap_permitted.cap[i])
@@ -453,9 +456,13 @@ 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)
-			rc = 0;
-		goto out;
+		else if (rc != -ENODATA)
+			goto out;
+		rc = 0;
+		if (cap_isclear(current_cred()->cap_ambient))
+			goto out;
+		/* Make sure that the ambient caps are enabled */
+		*effective = true;
 	}

 	rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, has_cap);
@@ -577,6 +584,7 @@ skip:
 	}

 	new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
+	new->cap_ambient = old->cap_ambient;
 	return 0;
 }

@@ -933,6 +941,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-05 09:53:43.442883383 -0600
+++ linux/include/linux/cred.h	2015-02-05 09:53:43.438883512 -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-05 09:53:43.442883383 -0600
+++ linux/include/uapi/linux/prctl.h	2015-02-05 09:53:43.438883512 -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	2014-12-12 10:27:49.304801274 -0600
+++ linux/fs/proc/array.c	2015-02-05 11:04:38.546429870 -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 V1

[Christoph Lameter]

Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
weeks without comment after an intensive discussion about the RFCs.

Since there were no objections: Is there any chance to get this into -next
somehow?

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
weeks without comment after an intensive discussion about the RFCs.

Since there were no objections: Is there any chance to get this into -next
somehow?

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Mon, Feb 23, 2015 at 6:58 AM, Christoph Lameter <cl@linux.com> wrote:
> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> weeks without comment after an intensive discussion about the RFCs.
>
> Since there were no objections: Is there any chance to get this into -next
> somehow?
>

At the very least, I think it needs to define and implement what
happens when a cap is added to ambient and then dropped from
permitted. We also may need LSM_UNSAFE_something to clear the ambient
set to avoid a major security issue.

I'd like to discuss (in the hallway if nothing else) at LSF/MM with
whatever other interested people will be there.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Andy Lutomirski wrote:

> At the very least, I think it needs to define and implement what
> happens when a cap is added to ambient and then dropped from
> permitted. We also may need LSM_UNSAFE_something to clear the ambient
> set to avoid a major security issue.

The ambient cap needs to stay otherwise we will have issues again if
another binary/script is forked. IMHO the only way to switch off an
ambient capability should be another prctl action. The intend is after all
to have the cap available for all inherited processes.

Frankly, I'd rather have the ambient caps separate. We could do a stronger
separation by checking for permitted or ambient in capable().

> I'd like to discuss (in the hallway if nothing else) at LSF/MM with
> whatever other interested people will be there.

Ok. I will be at the MM meeting in Boston. 8th and 9th of March.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Andy Lutomirski wrote:

> At the very least, I think it needs to define and implement what
> happens when a cap is added to ambient and then dropped from
> permitted. We also may need LSM_UNSAFE_something to clear the ambient
> set to avoid a major security issue.

The ambient cap needs to stay otherwise we will have issues again if
another binary/script is forked. IMHO the only way to switch off an
ambient capability should be another prctl action. The intend is after all
to have the cap available for all inherited processes.

Frankly, I'd rather have the ambient caps separate. We could do a stronger
separation by checking for permitted or ambient in capable().

> I'd like to discuss (in the hallway if nothing else) at LSF/MM with
> whatever other interested people will be there.

Ok. I will be at the MM meeting in Boston. 8th and 9th of March.

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Mon, Feb 23, 2015 at 7:53 AM, Christoph Lameter <cl@linux.com> wrote:
> On Mon, 23 Feb 2015, Andy Lutomirski wrote:
>
>> At the very least, I think it needs to define and implement what
>> happens when a cap is added to ambient and then dropped from
>> permitted. We also may need LSM_UNSAFE_something to clear the ambient
>> set to avoid a major security issue.
>
> The ambient cap needs to stay otherwise we will have issues again if
> another binary/script is forked. IMHO the only way to switch off an
> ambient capability should be another prctl action. The intend is after all
> to have the cap available for all inherited processes.

No, sadly.

If you set ambient caps and then run a setuid program (without
no_new_privs), then the ambient set *must* be cleared by the kernel
because that's what the setuid program expects.  Yes, the whole
concept of setuid is daft, but it exists and we have to keep it
working safely, or at least as safely as it ever worked.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Serge Hallyn]

Quoting Christoph Lameter (cl@linux.com):
> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> weeks without comment after an intensive discussion about the RFCs.
> 
> Since there were no objections: Is there any chance to get this into -next
> somehow?

Andrew Morgan and Andy Lutomirski appear to have a similar concern
but competing ideas on how to address them.  We need them to agree
on an approach.

The core concern for amorgan is that an unprivileged user not be
able to cause a privileged program to run in a way that it fails to
drop privilege before running unprivileged-user-provided code.

Andy Lutomirski's concern is simply that code which is currently
doing the right thing to drop privilege not be run in a way that
it thinks it is dropping privilege, but in fact is not.

(Please correct me where I've mis-spoken or misunderstood)

Since your desire is precisely for a mode where dropping privilege
works as usual, but exec then re-gains some or all of that privilege,
we need to either agree on a way to enter that mode that ordinary
use caes can't be tricked into using, or find a way for legacy
users to be tpiped off as to what's going on (without having to be
re-written)

-serge

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Mon, Feb 23, 2015 at 8:16 AM, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
> Quoting Christoph Lameter (cl@linux.com):
>> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
>> weeks without comment after an intensive discussion about the RFCs.
>>
>> Since there were no objections: Is there any chance to get this into -next
>> somehow?
>
> Andrew Morgan and Andy Lutomirski appear to have a similar concern
> but competing ideas on how to address them.  We need them to agree
> on an approach.
>
> The core concern for amorgan is that an unprivileged user not be
> able to cause a privileged program to run in a way that it fails to
> drop privilege before running unprivileged-user-provided code.
>
> Andy Lutomirski's concern is simply that code which is currently
> doing the right thing to drop privilege not be run in a way that
> it thinks it is dropping privilege, but in fact is not.
>

I share both concerns.

> (Please correct me where I've mis-spoken or misunderstood)
>
> Since your desire is precisely for a mode where dropping privilege
> works as usual, but exec then re-gains some or all of that privilege,
> we need to either agree on a way to enter that mode that ordinary
> use caes can't be tricked into using, or find a way for legacy
> users to be tpiped off as to what's going on (without having to be
> re-written)

Is there really a need to drop privilege and then regain it or is it
sufficient to keep the privilege permitted (and perhaps ambient, too)
and just to have execve not drop it for you?  I assume the latter.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Andy Lutomirski wrote:

> If you set ambient caps and then run a setuid program (without
> no_new_privs), then the ambient set *must* be cleared by the kernel
> because that's what the setuid program expects.  Yes, the whole

Why would a setuid program expect that? I'd say we expect the ambient set
to remain in effect. What would break if the ambient set would stay
active?

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Serge Hallyn wrote:

> The core concern for amorgan is that an unprivileged user not be
> able to cause a privileged program to run in a way that it fails to
> drop privilege before running unprivileged-user-provided code.

I do not see a problem with dropping privilege since the ambient set
is supposed to be preserved across a drop of priviledge.

> Since your desire is precisely for a mode where dropping privilege
> works as usual, but exec then re-gains some or all of that privilege,

I would say that the ambient set stays active even if the setuid binary
drops to regular perms.

> we need to either agree on a way to enter that mode that ordinary
> use caes can't be tricked into using, or find a way for legacy
> users to be tpiped off as to what's going on (without having to be
> re-written)

Well if the ambient set is completely separate then the existing
semantics are preserved while the ambient set stays active as intended.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Serge Hallyn wrote:

> The core concern for amorgan is that an unprivileged user not be
> able to cause a privileged program to run in a way that it fails to
> drop privilege before running unprivileged-user-provided code.

I do not see a problem with dropping privilege since the ambient set
is supposed to be preserved across a drop of priviledge.

> Since your desire is precisely for a mode where dropping privilege
> works as usual, but exec then re-gains some or all of that privilege,

I would say that the ambient set stays active even if the setuid binary
drops to regular perms.

> we need to either agree on a way to enter that mode that ordinary
> use caes can't be tricked into using, or find a way for legacy
> users to be tpiped off as to what's going on (without having to be
> re-written)

Well if the ambient set is completely separate then the existing
semantics are preserved while the ambient set stays active as intended.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 08:33:58AM -0800, Andy Lutomirski wrote:
> On Mon, Feb 23, 2015 at 8:16 AM, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
> > Quoting Christoph Lameter (cl@linux.com):
> >> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> >> weeks without comment after an intensive discussion about the RFCs.
> >>
> >> Since there were no objections: Is there any chance to get this into -next
> >> somehow?
> >
> > Andrew Morgan and Andy Lutomirski appear to have a similar concern
> > but competing ideas on how to address them.  We need them to agree
> > on an approach.
> >
> > The core concern for amorgan is that an unprivileged user not be
> > able to cause a privileged program to run in a way that it fails to
> > drop privilege before running unprivileged-user-provided code.
> >
> > Andy Lutomirski's concern is simply that code which is currently
> > doing the right thing to drop privilege not be run in a way that
> > it thinks it is dropping privilege, but in fact is not.
> >
> 
> I share both concerns.
> 
> > (Please correct me where I've mis-spoken or misunderstood)
> >
> > Since your desire is precisely for a mode where dropping privilege
> > works as usual, but exec then re-gains some or all of that privilege,
> > we need to either agree on a way to enter that mode that ordinary
> > use caes can't be tricked into using, or find a way for legacy
> > users to be tpiped off as to what's going on (without having to be
> > re-written)
> 
> Is there really a need to drop privilege and then regain it or is it
> sufficient to keep the privilege permitted (and perhaps ambient, too)
> and just to have execve not drop it for you?  I assume the latter.

Well right, any perceived security benefit of the temporary drop would
seem to be easily debunked (just run shell for exec /bin/sh to get
around it)

So this is more of a desire, I suspect, for regular programs which
drop privilege to still be usable in this environment.

I think this may be a decent place for a compromise.  Attempts to
drop privilege when ambient caps are set return EPERM.

-serge

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 08:33:58AM -0800, Andy Lutomirski wrote:
> On Mon, Feb 23, 2015 at 8:16 AM, Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote:
> > Quoting Christoph Lameter (cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org):
> >> Ok 4.0-rc1 is out and this patch has been sitting here for a couple of
> >> weeks without comment after an intensive discussion about the RFCs.
> >>
> >> Since there were no objections: Is there any chance to get this into -next
> >> somehow?
> >
> > Andrew Morgan and Andy Lutomirski appear to have a similar concern
> > but competing ideas on how to address them.  We need them to agree
> > on an approach.
> >
> > The core concern for amorgan is that an unprivileged user not be
> > able to cause a privileged program to run in a way that it fails to
> > drop privilege before running unprivileged-user-provided code.
> >
> > Andy Lutomirski's concern is simply that code which is currently
> > doing the right thing to drop privilege not be run in a way that
> > it thinks it is dropping privilege, but in fact is not.
> >
> 
> I share both concerns.
> 
> > (Please correct me where I've mis-spoken or misunderstood)
> >
> > Since your desire is precisely for a mode where dropping privilege
> > works as usual, but exec then re-gains some or all of that privilege,
> > we need to either agree on a way to enter that mode that ordinary
> > use caes can't be tricked into using, or find a way for legacy
> > users to be tpiped off as to what's going on (without having to be
> > re-written)
> 
> Is there really a need to drop privilege and then regain it or is it
> sufficient to keep the privilege permitted (and perhaps ambient, too)
> and just to have execve not drop it for you?  I assume the latter.

Well right, any perceived security benefit of the temporary drop would
seem to be easily debunked (just run shell for exec /bin/sh to get
around it)

So this is more of a desire, I suspect, for regular programs which
drop privilege to still be usable in this environment.

I think this may be a decent place for a compromise.  Attempts to
drop privilege when ambient caps are set return EPERM.

-serge

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 10:44:32AM -0600, Christoph Lameter wrote:
> On Mon, 23 Feb 2015, Serge Hallyn wrote:
> 
> > The core concern for amorgan is that an unprivileged user not be
> > able to cause a privileged program to run in a way that it fails to
> > drop privilege before running unprivileged-user-provided code.
> 
> I do not see a problem with dropping privilege since the ambient set
> is supposed to be preserved across a drop of priviledge.

Because you're tricking the program into thinking it has dropped
the privilege, when in fact it has not.

> > Since your desire is precisely for a mode where dropping privilege
> > works as usual, but exec then re-gains some or all of that privilege,
> 
> I would say that the ambient set stays active even if the setuid binary
> drops to regular perms.
> 
> > we need to either agree on a way to enter that mode that ordinary
> > use caes can't be tricked into using, or find a way for legacy
> > users to be tpiped off as to what's going on (without having to be
> > re-written)
> 
> Well if the ambient set is completely separate then the existing
> semantics are preserved while the ambient set stays active as intended.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.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 V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Andy Lutomirski wrote:

> Is there really a need to drop privilege and then regain it or is it
> sufficient to keep the privilege permitted (and perhaps ambient, too)
> and just to have execve not drop it for you?  I assume the latter.

I would think just keep the ambient set active as long as there is no
prctl switching the cap off in the child processes. Do not let it be
affected by the usual drop privs stuff.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Serge E. Hallyn wrote:

> > I do not see a problem with dropping privilege since the ambient set
> > is supposed to be preserved across a drop of priviledge.
>
> Because you're tricking the program into thinking it has dropped
> the privilege, when in fact it has not.

So the cap was dropped from the cap perm set but it is still active
in the ambient set?

Re: [PATCH] capabilities: Ambient capability set V1

[Serge Hallyn]

Quoting Christoph Lameter (cl@linux.com):
> On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
> 
> > > I do not see a problem with dropping privilege since the ambient set
> > > is supposed to be preserved across a drop of priviledge.
> >
> > Because you're tricking the program into thinking it has dropped
> > the privilege, when in fact it has not.
> 
> So the cap was dropped from the cap perm set but it is still active
> in the ambient set?

Right, and the legacy program doesn't know to check the new set.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Serge Hallyn wrote:

> > So the cap was dropped from the cap perm set but it is still active
> > in the ambient set?
>
> Right, and the legacy program doesn't know to check the new set.

Does it have to? The ambient set could not show up in permitted.

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Feb 23, 2015 8:41 AM, "Christoph Lameter" <cl@linux.com> wrote:
>
> On Mon, 23 Feb 2015, Andy Lutomirski wrote:
>
> > If you set ambient caps and then run a setuid program (without
> > no_new_privs), then the ambient set *must* be cleared by the kernel
> > because that's what the setuid program expects.  Yes, the whole
>
> Why would a setuid program expect that? I'd say we expect the ambient set
> to remain in effect. What would break if the ambient set would stay
> active?
>

On a total guess: exim, sendmail, sudo, Apache suexec, etc.  Basically
anything that expects setresuid(nonzero values); execve to drop caps.

I haven't checked how many of the examples above actually do this.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 06:15:53PM +0000, Serge Hallyn wrote:
> Quoting Christoph Lameter (cl@linux.com):
> > On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
> > 
> > > > I do not see a problem with dropping privilege since the ambient set
> > > > is supposed to be preserved across a drop of priviledge.
> > >
> > > Because you're tricking the program into thinking it has dropped
> > > the privilege, when in fact it has not.
> > 
> > So the cap was dropped from the cap perm set but it is still active
> > in the ambient set?
> 
> Right, and the legacy program doesn't know to check the new set.

we've been assuming the ambient set must be like fP.  is there any
reason why it doesn't suffice for them to be or'ed with fI instead at
exec?  then the bits would need to ne in pI. this might sufice for
Christoph's use case, as pI will generally not change.  and for programs
that really care, they can check pI.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 11:19:29PM -0600, Serge E. Hallyn wrote:
> On Mon, Feb 23, 2015 at 06:15:53PM +0000, Serge Hallyn wrote:
> > Quoting Christoph Lameter (cl@linux.com):
> > > On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
> > > 
> > > > > I do not see a problem with dropping privilege since the ambient set
> > > > > is supposed to be preserved across a drop of priviledge.
> > > >
> > > > Because you're tricking the program into thinking it has dropped
> > > > the privilege, when in fact it has not.
> > > 
> > > So the cap was dropped from the cap perm set but it is still active
> > > in the ambient set?
> > 
> > Right, and the legacy program doesn't know to check the new set.
> 
> we've been assuming the ambient set must be like fP.  is there any
> reason why it doesn't suffice for them to be or'ed with fI instead at
> exec?  then the bits would need to ne in pI. this might sufice for
> Christoph's use case, as pI will generally not change.  and for programs
> that really care, they can check pI.

The other way to look at it then is that it's basically as though the
privileged task (which has CAP_SETFCAP) could've just added fI=full to
all binaries on the filesystem;  instead it's using the ambient set
so that the risk from fI=full is contained to its own process tree.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Mon, Feb 23, 2015 at 11:19:29PM -0600, Serge E. Hallyn wrote:
> On Mon, Feb 23, 2015 at 06:15:53PM +0000, Serge Hallyn wrote:
> > Quoting Christoph Lameter (cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org):
> > > On Mon, 23 Feb 2015, Serge E. Hallyn wrote:
> > > 
> > > > > I do not see a problem with dropping privilege since the ambient set
> > > > > is supposed to be preserved across a drop of priviledge.
> > > >
> > > > Because you're tricking the program into thinking it has dropped
> > > > the privilege, when in fact it has not.
> > > 
> > > So the cap was dropped from the cap perm set but it is still active
> > > in the ambient set?
> > 
> > Right, and the legacy program doesn't know to check the new set.
> 
> we've been assuming the ambient set must be like fP.  is there any
> reason why it doesn't suffice for them to be or'ed with fI instead at
> exec?  then the bits would need to ne in pI. this might sufice for
> Christoph's use case, as pI will generally not change.  and for programs
> that really care, they can check pI.

The other way to look at it then is that it's basically as though the
privileged task (which has CAP_SETFCAP) could've just added fI=full to
all binaries on the filesystem;  instead it's using the ambient set
so that the risk from fI=full is contained to its own process tree.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Mon, 23 Feb 2015, Andy Lutomirski wrote:

> On Feb 23, 2015 8:41 AM, "Christoph Lameter" <cl@linux.com> wrote:
> >
> > On Mon, 23 Feb 2015, Andy Lutomirski wrote:
> >
> > > If you set ambient caps and then run a setuid program (without
> > > no_new_privs), then the ambient set *must* be cleared by the kernel
> > > because that's what the setuid program expects.  Yes, the whole
> >
> > Why would a setuid program expect that? I'd say we expect the ambient set
> > to remain in effect. What would break if the ambient set would stay
> > active?
> >
>
> On a total guess: exim, sendmail, sudo, Apache suexec, etc.  Basically
> anything that expects setresuid(nonzero values); execve to drop caps.

Really? We have been running these things for years with the approach of
leaving these caps active.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Tue, 24 Feb 2015, Serge E. Hallyn wrote:

> The other way to look at it then is that it's basically as though the
> privileged task (which has CAP_SETFCAP) could've just added fI=full to
> all binaries on the filesystem;  instead it's using the ambient set
> so that the risk from fI=full is contained to its own process tree.

The way that our internal patch works is to leave these things alone and
just check the ambient mask in the *capable*() functions. That way the
behavior of the existing cap bits does not change but the ambient caps
stay available. Apps have no surprises.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge Hallyn]

Quoting Christoph Lameter (cl@linux.com):
> On Tue, 24 Feb 2015, Serge E. Hallyn wrote:
> 
> > The other way to look at it then is that it's basically as though the
> > privileged task (which has CAP_SETFCAP) could've just added fI=full to
> > all binaries on the filesystem;  instead it's using the ambient set
> > so that the risk from fI=full is contained to its own process tree.
> 
> The way that our internal patch works is to leave these things alone and
> just check the ambient mask in the *capable*() functions. That way the
> behavior of the existing cap bits does not change but the ambient caps
> stay available. Apps have no surprises.

Unless I'm misunderstanding what you are saying, apps do have surprises.
They drop capabilities, execute a file, and the result has capabilities
which the app couldn't have expected.  At least if the bits have to be
in fI to become part of pP', the app has a clue.

To be clear, I'm suggesting that the rules at exec become:

pI' = pI
pA' = pA  (pA is ambient)
pP' = (X & fP) | (pI & (fI | pA))
pE' = pP' & fE

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Tue, 24 Feb 2015, Serge Hallyn wrote:

> Unless I'm misunderstanding what you are saying, apps do have surprises.
> They drop capabilities, execute a file, and the result has capabilities
> which the app couldn't have expected.  At least if the bits have to be
> in fI to become part of pP', the app has a clue.

Well yes but the surprises do not occur in the cap bits they are
manipulating or inspecting via prctl.

> To be clear, I'm suggesting that the rules at exec become:
>
> pI' = pI

Ok that is new and on its own may solve the issue?

> pA' = pA  (pA is ambient)

Thats what this patch does

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

Hmmm... fP is empty for the file not having caps. so

pP' = pI & pA

> pE' = pP' & fE

fE? So the inherited caps are not effective? fE would be empty for a file
not having caps thus the ambient caps would not be available in the child.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge Hallyn]

Quoting Christoph Lameter (cl@linux.com):
> On Tue, 24 Feb 2015, Serge Hallyn wrote:
> 
> > Unless I'm misunderstanding what you are saying, apps do have surprises.
> > They drop capabilities, execute a file, and the result has capabilities
> > which the app couldn't have expected.  At least if the bits have to be
> > in fI to become part of pP', the app has a clue.
> 
> Well yes but the surprises do not occur in the cap bits they are
> manipulating or inspecting via prctl.
> 
> > To be clear, I'm suggesting that the rules at exec become:
> >
> > pI' = pI
> 
> Ok that is new and on its own may solve the issue?

No that's not new.

> > pA' = pA  (pA is ambient)
> 
> Thats what this patch does
> 
> > pP' = (X & fP) | (pI & (fI | pA))
> 
> Hmmm... fP is empty for the file not having caps. so
> 
> pP' = pI & pA

Right.

> > pE' = pP' & fE
> 
> fE? So the inherited caps are not effective? fE would be empty for a file
> not having caps thus the ambient caps would not be available in the child.

Yeah we could make this

pE' = pP' & (fE | pA)

-serge

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Wed, 25 Feb 2015, Serge Hallyn wrote:

> Yeah we could make this

Well doing that breaks su. Its best to leave perm bits untouched.

christoph@fujitsu-haswell:~$ su
setgid: Operation not permitted

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Wed, 25 Feb 2015, Serge Hallyn wrote:

> Yeah we could make this

Well doing that breaks su. Its best to leave perm bits untouched.

christoph@fujitsu-haswell:~$ su
setgid: Operation not permitted

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Wed, Feb 25, 2015 at 02:25:19PM -0600, Christoph Lameter wrote:
> On Wed, 25 Feb 2015, Serge Hallyn wrote:
> 
> > Yeah we could make this
> 
> Well doing that breaks su.

Don't what exactly?  You're saying that doing

pI' = pI
pA' = pA  (pA is ambient)
pP' = (X & fP) | (pI & (fI | pA))
pE' = pP' & (fE | pA)

stopped su from having CAP_SETGID while

pI' = pI
pA' = pA  (pA is ambient)
pP' = (X & fP) | (pI & (fI | pA))
pE' = pP' & fE

worked?  I'll hope you're saying both "fail", in which case

> Its best to leave perm bits untouched.


> christoph@fujitsu-haswell:~$ su
> setgid: Operation not permitted

Did you initialize by running a program to fill your pI?

-serge

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> > Well doing that breaks su.
>
> Don't what exactly?  You're saying that doing
>
> pI' = pI
> pA' = pA  (pA is ambient)
> pP' = (X & fP) | (pI & (fI | pA))
> pE' = pP' & (fE | pA)
>
> stopped su from having CAP_SETGID while
>
> pI' = pI
> pA' = pA  (pA is ambient)
> pP' = (X & fP) | (pI & (fI | pA))
> pE' = pP' & fE
>
> worked?  I'll hope you're saying both "fail", in which case

fE does not exist in cpu_vfs_cap_data. We only get fI and fP. Why in the
world does setcap allow setting fE?

V1 does not use fE. In my newer attempt, I seemed to have worked
with zeroed field that I assumed was filled in.

I will just do

pE' = pP'

Ok?

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> > Well doing that breaks su.
>
> Don't what exactly?  You're saying that doing
>
> pI' = pI
> pA' = pA  (pA is ambient)
> pP' = (X & fP) | (pI & (fI | pA))
> pE' = pP' & (fE | pA)
>
> stopped su from having CAP_SETGID while
>
> pI' = pI
> pA' = pA  (pA is ambient)
> pP' = (X & fP) | (pI & (fI | pA))
> pE' = pP' & fE
>
> worked?  I'll hope you're saying both "fail", in which case

fE does not exist in cpu_vfs_cap_data. We only get fI and fP. Why in the
world does setcap allow setting fE?

V1 does not use fE. In my newer attempt, I seemed to have worked
with zeroed field that I assumed was filled in.

I will just do

pE' = pP'

Ok?

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 12:28:30PM -0600, Christoph Lameter wrote:
> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> 
> > > Well doing that breaks su.
> >
> > Don't what exactly?  You're saying that doing
> >
> > pI' = pI
> > pA' = pA  (pA is ambient)
> > pP' = (X & fP) | (pI & (fI | pA))
> > pE' = pP' & (fE | pA)
> >
> > stopped su from having CAP_SETGID while
> >
> > pI' = pI
> > pA' = pA  (pA is ambient)
> > pP' = (X & fP) | (pI & (fI | pA))
> > pE' = pP' & fE
> >
> > worked?  I'll hope you're saying both "fail", in which case
> 
> fE does not exist in cpu_vfs_cap_data. We only get fI and fP. Why in the
> world does setcap allow setting fE?

It sets a bit in the magic_etc.  So fE is either all on or all off.

> V1 does not use fE. In my newer attempt, I seemed to have worked
> with zeroed field that I assumed was filled in.
> 
> I will just do
> 
> pE' = pP'
> 
> Ok?

Andrew Morgan was against that.  What if we changed

pE' = pP' & (fE | pA)

to

	if (pA)
		pE' = pP' & fE
	else
		pE' = pP'

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Thu, Feb 26, 2015 at 11:32 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> On Thu, Feb 26, 2015 at 12:28:30PM -0600, Christoph Lameter wrote:
>> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
>>
>> > > Well doing that breaks su.
>> >
>> > Don't what exactly?  You're saying that doing
>> >
>> > pI' = pI
>> > pA' = pA  (pA is ambient)
>> > pP' = (X & fP) | (pI & (fI | pA))
>> > pE' = pP' & (fE | pA)
>> >
>> > stopped su from having CAP_SETGID while
>> >
>> > pI' = pI
>> > pA' = pA  (pA is ambient)
>> > pP' = (X & fP) | (pI & (fI | pA))
>> > pE' = pP' & fE
>> >
>> > worked?  I'll hope you're saying both "fail", in which case
>>
>> fE does not exist in cpu_vfs_cap_data. We only get fI and fP. Why in the
>> world does setcap allow setting fE?
>
> It sets a bit in the magic_etc.  So fE is either all on or all off.
>
>> V1 does not use fE. In my newer attempt, I seemed to have worked
>> with zeroed field that I assumed was filled in.
>>
>> I will just do
>>
>> pE' = pP'
>>
>> Ok?
>
> Andrew Morgan was against that.  What if we changed
>
> pE' = pP' & (fE | pA)
>
> to
>
>         if (pA)
>                 pE' = pP' & fE
>         else
>                 pE' = pP'
>

Is this backwards?

Also, on further bikeshedding consideration, I think I like the name
"persistent" much better than "ambient".  Alas, "persistent" starts
with a P.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> Andrew Morgan was against that.  What if we changed
>
> pE' = pP' & (fE | pA)
>
> to
>
> 	if (pA)
> 		pE' = pP' & fE
> 	else
> 		pE' = pP'
>

Same problem as before. The ambient bits will not be set in pE'.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> Andrew Morgan was against that.  What if we changed
>
> pE' = pP' & (fE | pA)
>
> to
>
> 	if (pA)
> 		pE' = pP' & fE
> 	else
> 		pE' = pP'
>

Same problem as before. The ambient bits will not be set in pE'.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Andy Lutomirski wrote:

> Also, on further bikeshedding consideration, I think I like the name
> "persistent" much better than "ambient".  Alas, "persistent" starts
> with a P.

Ambient is like backround lighting. Its always there but barely visible.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Andy Lutomirski wrote:

> Also, on further bikeshedding consideration, I think I like the name
> "persistent" much better than "ambient".  Alas, "persistent" starts
> with a P.

Ambient is like backround lighting. Its always there but barely visible.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 11:38:01AM -0800, Andy Lutomirski wrote:
> On Thu, Feb 26, 2015 at 11:32 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Thu, Feb 26, 2015 at 12:28:30PM -0600, Christoph Lameter wrote:
> >> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> >>
> >> > > Well doing that breaks su.
> >> >
> >> > Don't what exactly?  You're saying that doing
> >> >
> >> > pI' = pI
> >> > pA' = pA  (pA is ambient)
> >> > pP' = (X & fP) | (pI & (fI | pA))
> >> > pE' = pP' & (fE | pA)
> >> >
> >> > stopped su from having CAP_SETGID while
> >> >
> >> > pI' = pI
> >> > pA' = pA  (pA is ambient)
> >> > pP' = (X & fP) | (pI & (fI | pA))
> >> > pE' = pP' & fE
> >> >
> >> > worked?  I'll hope you're saying both "fail", in which case
> >>
> >> fE does not exist in cpu_vfs_cap_data. We only get fI and fP. Why in the
> >> world does setcap allow setting fE?
> >
> > It sets a bit in the magic_etc.  So fE is either all on or all off.
> >
> >> V1 does not use fE. In my newer attempt, I seemed to have worked
> >> with zeroed field that I assumed was filled in.
> >>
> >> I will just do
> >>
> >> pE' = pP'
> >>
> >> Ok?
> >
> > Andrew Morgan was against that.  What if we changed
> >
> > pE' = pP' & (fE | pA)
> >
> > to
> >
> >         if (pA)
> >                 pE' = pP' & fE
> >         else
> >                 pE' = pP'
> >
> 
> Is this backwards?

D'oh, yes.

> Also, on further bikeshedding consideration, I think I like the name
> "persistent" much better than "ambient".  Alas, "persistent" starts
> with a P.
> 
> --Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> 
> > Andrew Morgan was against that.  What if we changed
> >
> > pE' = pP' & (fE | pA)
> >
> > to
> >
> > 	if (pA)
> > 		pE' = pP' & fE
> > 	else
> > 		pE' = pP'
> >
> 
> Same problem as before. The ambient bits will not be set in pE'.

And what if I weren't scatterbrained and we did 

 	if (pA)
 		pE' = pP'
 	else
 		pE' = pP' & fE

All pP' bits would be set in pE'.

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Thu, Feb 26, 2015 at 12:34 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
>> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
>>
>> > Andrew Morgan was against that.  What if we changed
>> >
>> > pE' = pP' & (fE | pA)
>> >
>> > to
>> >
>> >     if (pA)
>> >             pE' = pP' & fE
>> >     else
>> >             pE' = pP'
>> >
>>
>> Same problem as before. The ambient bits will not be set in pE'.
>
> And what if I weren't scatterbrained and we did
>
>         if (pA)
>                 pE' = pP'
>         else
>                 pE' = pP' & fE
>
> All pP' bits would be set in pE'.

That seems reasonable to me, except for my paranoia:

What if there's a program with CAP_DAC_OVERRIDE in fP and fE set to
the empty set (i.e. the magic effective bit cleared), and the program
relies on that.  A malicious user has CAP_NET_BIND and sets pA =
CAP_NET_BIND.  Boom!

If we changed that to if (pA') and zeroed pA if fP is non-empty then
this problem goes away.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 12:51:57PM -0800, Andy Lutomirski wrote:
> On Thu, Feb 26, 2015 at 12:34 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
> >> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> >>
> >> > Andrew Morgan was against that.  What if we changed
> >> >
> >> > pE' = pP' & (fE | pA)
> >> >
> >> > to
> >> >
> >> >     if (pA)
> >> >             pE' = pP' & fE
> >> >     else
> >> >             pE' = pP'
> >> >
> >>
> >> Same problem as before. The ambient bits will not be set in pE'.
> >
> > And what if I weren't scatterbrained and we did
> >
> >         if (pA)
> >                 pE' = pP'
> >         else
> >                 pE' = pP' & fE
> >
> > All pP' bits would be set in pE'.
> 
> That seems reasonable to me, except for my paranoia:
> 
> What if there's a program with CAP_DAC_OVERRIDE in fP and fE set to
> the empty set (i.e. the magic effective bit cleared), and the program
> relies on that.  A malicious user has CAP_NET_BIND and sets pA =
> CAP_NET_BIND.  Boom!
> 
> If we changed that to if (pA') and zeroed pA if fP is non-empty then
> this problem goes away.

Hm, the problem is that then the empty pA is inherited by children.
I do see that any program with fP set should probably run with only
what it requested.  Would

         if (pA && is_empty(fP))
                 pE' = pP'
         else
                 pE' = pP' & fE

help?  Or are you worried about a program with fP set which then
executes other programs?

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Thu, Feb 26, 2015 at 12:55 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> On Thu, Feb 26, 2015 at 12:51:57PM -0800, Andy Lutomirski wrote:
>> On Thu, Feb 26, 2015 at 12:34 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
>> > On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
>> >> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
>> >>
>> >> > Andrew Morgan was against that.  What if we changed
>> >> >
>> >> > pE' = pP' & (fE | pA)
>> >> >
>> >> > to
>> >> >
>> >> >     if (pA)
>> >> >             pE' = pP' & fE
>> >> >     else
>> >> >             pE' = pP'
>> >> >
>> >>
>> >> Same problem as before. The ambient bits will not be set in pE'.
>> >
>> > And what if I weren't scatterbrained and we did
>> >
>> >         if (pA)
>> >                 pE' = pP'
>> >         else
>> >                 pE' = pP' & fE
>> >
>> > All pP' bits would be set in pE'.
>>
>> That seems reasonable to me, except for my paranoia:
>>
>> What if there's a program with CAP_DAC_OVERRIDE in fP and fE set to
>> the empty set (i.e. the magic effective bit cleared), and the program
>> relies on that.  A malicious user has CAP_NET_BIND and sets pA =
>> CAP_NET_BIND.  Boom!
>>
>> If we changed that to if (pA') and zeroed pA if fP is non-empty then
>> this problem goes away.
>
> Hm, the problem is that then the empty pA is inherited by children.
> I do see that any program with fP set should probably run with only
> what it requested.  Would
>
>          if (pA && is_empty(fP))
>                  pE' = pP'
>          else
>                  pE' = pP' & fE
>
> help?  Or are you worried about a program with fP set which then
> executes other programs?

The particular worry I expressed there was just about pE.

I'm still extremely nervous about allowing nonempty pA to propagate to
setuid or nonzero fP programs.  It's less obviously dangerous if pA is
never a superset of pP, but it could still cause problems with setuid
programs that execute intentionally deprivileged helpers.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Andy Lutomirski]

On Thu, Feb 26, 2015 at 12:55 PM, Serge E. Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, Feb 26, 2015 at 12:51:57PM -0800, Andy Lutomirski wrote:
>> On Thu, Feb 26, 2015 at 12:34 PM, Serge E. Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
>> >> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
>> >>
>> >> > Andrew Morgan was against that.  What if we changed
>> >> >
>> >> > pE' = pP' & (fE | pA)
>> >> >
>> >> > to
>> >> >
>> >> >     if (pA)
>> >> >             pE' = pP' & fE
>> >> >     else
>> >> >             pE' = pP'
>> >> >
>> >>
>> >> Same problem as before. The ambient bits will not be set in pE'.
>> >
>> > And what if I weren't scatterbrained and we did
>> >
>> >         if (pA)
>> >                 pE' = pP'
>> >         else
>> >                 pE' = pP' & fE
>> >
>> > All pP' bits would be set in pE'.
>>
>> That seems reasonable to me, except for my paranoia:
>>
>> What if there's a program with CAP_DAC_OVERRIDE in fP and fE set to
>> the empty set (i.e. the magic effective bit cleared), and the program
>> relies on that.  A malicious user has CAP_NET_BIND and sets pA =
>> CAP_NET_BIND.  Boom!
>>
>> If we changed that to if (pA') and zeroed pA if fP is non-empty then
>> this problem goes away.
>
> Hm, the problem is that then the empty pA is inherited by children.
> I do see that any program with fP set should probably run with only
> what it requested.  Would
>
>          if (pA && is_empty(fP))
>                  pE' = pP'
>          else
>                  pE' = pP' & fE
>
> help?  Or are you worried about a program with fP set which then
> executes other programs?

The particular worry I expressed there was just about pE.

I'm still extremely nervous about allowing nonempty pA to propagate to
setuid or nonzero fP programs.  It's less obviously dangerous if pA is
never a superset of pP, but it could still cause problems with setuid
programs that execute intentionally deprivileged helpers.

--Andy

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> > Same problem as before. The ambient bits will not be set in pE'.
>
> And what if I weren't scatterbrained and we did
>
>  	if (pA)
>  		pE' = pP'
>  	else
>  		pE' = pP' & fE
>
> All pP' bits would be set in pE'.

Ok and the non ambient case would break because fE is not available?
Doesnt this reduce to

pE' = pP'

in either case?

Here is a a patch that does just that. The patch works. Maybe I just dont
understand how this is supposed to work.

Subject: [PATCH] capabilities: Ambient capability set V2(draft)

V1->V2(draft):
 - Modify bit calculations.

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

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 12:36:32.361726374 -0600
@@ -347,15 +347,16 @@ 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 +454,12 @@ 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;
+			/* The ambient caps are permitted for files that have no caps */
+			bprm->cred->cap_permitted = bprm->cred->cap_effective =
+				current_cred()->cap_ambient;
+		}
 		goto out;
 	}

@@ -548,10 +553,16 @@ skip:
 	new->suid = new->fsuid = new->euid;
 	new->sgid = new->fsgid = new->egid;

-	if (effective)
-		new->cap_effective = new->cap_permitted;
-	else
-		cap_clear(new->cap_effective);
+	/* pE' = pP' & (fE | pA)
+	new->cap_effective = cap_intersect(new->cap_permitted,
+			cap_combine(new->cap_effective, old->cap_ambient));
+	 */
+
+	/* fE is not available */
+	new->cap_effective = new->cap_permitted;
+
+	/* pA' = pA */
+	new->cap_ambient = old->cap_ambient;
 	bprm->cap_effective = effective;

 	/*
@@ -566,7 +577,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 +609,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 +944,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 V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 03:09:48PM -0600, Christoph Lameter wrote:
> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> 
> > > Same problem as before. The ambient bits will not be set in pE'.
> >
> > And what if I weren't scatterbrained and we did
> >
> >  	if (pA)
> >  		pE' = pP'
> >  	else
> >  		pE' = pP' & fE
> >
> > All pP' bits would be set in pE'.
> 
> Ok and the non ambient case would break because fE is not available?
> Doesnt this reduce to
> 
> pE' = pP'
> 
> in either case?

No.  fE is not "not available".  If you set it with setcap, you *should*
(if i'm tinking right) find fE full when calculating the new capability
sets, because of magic_etc.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 03:09:48PM -0600, Christoph Lameter wrote:
> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> 
> > > Same problem as before. The ambient bits will not be set in pE'.
> >
> > And what if I weren't scatterbrained and we did
> >
> >  	if (pA)
> >  		pE' = pP'
> >  	else
> >  		pE' = pP' & fE
> >
> > All pP' bits would be set in pE'.
> 
> Ok and the non ambient case would break because fE is not available?
> Doesnt this reduce to
> 
> pE' = pP'
> 
> in either case?

No.  fE is not "not available".  If you set it with setcap, you *should*
(if i'm tinking right) find fE full when calculating the new capability
sets, because of magic_etc.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 12:58:33PM -0800, Andy Lutomirski wrote:
> On Thu, Feb 26, 2015 at 12:55 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Thu, Feb 26, 2015 at 12:51:57PM -0800, Andy Lutomirski wrote:
> >> On Thu, Feb 26, 2015 at 12:34 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> >> > On Thu, Feb 26, 2015 at 02:13:00PM -0600, Christoph Lameter wrote:
> >> >> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> >> >>
> >> >> > Andrew Morgan was against that.  What if we changed
> >> >> >
> >> >> > pE' = pP' & (fE | pA)
> >> >> >
> >> >> > to
> >> >> >
> >> >> >     if (pA)
> >> >> >             pE' = pP' & fE
> >> >> >     else
> >> >> >             pE' = pP'
> >> >> >
> >> >>
> >> >> Same problem as before. The ambient bits will not be set in pE'.
> >> >
> >> > And what if I weren't scatterbrained and we did
> >> >
> >> >         if (pA)
> >> >                 pE' = pP'
> >> >         else
> >> >                 pE' = pP' & fE
> >> >
> >> > All pP' bits would be set in pE'.
> >>
> >> That seems reasonable to me, except for my paranoia:
> >>
> >> What if there's a program with CAP_DAC_OVERRIDE in fP and fE set to
> >> the empty set (i.e. the magic effective bit cleared), and the program
> >> relies on that.  A malicious user has CAP_NET_BIND and sets pA =
> >> CAP_NET_BIND.  Boom!
> >>
> >> If we changed that to if (pA') and zeroed pA if fP is non-empty then
> >> this problem goes away.
> >
> > Hm, the problem is that then the empty pA is inherited by children.
> > I do see that any program with fP set should probably run with only
> > what it requested.  Would
> >
> >          if (pA && is_empty(fP))
> >                  pE' = pP'
> >          else
> >                  pE' = pP' & fE
> >
> > help?  Or are you worried about a program with fP set which then
> > executes other programs?
> 
> The particular worry I expressed there was just about pE.
> 
> I'm still extremely nervous about allowing nonempty pA to propagate to
> setuid or nonzero fP programs.  It's less obviously dangerous if pA is
> never a superset of pP, but it could still cause problems with setuid
> programs that execute intentionally deprivileged helpers.

I don't think that what you want is compatible with what Christoph
wants.  (He also thinks that what I want is not compatible with what
he wants, but I still think it is)

The approach I'm taking is that pA is useless if pI is not set.  For
a privileged program to fill its pI is a pretty special thing now,
so this shouldn't be catching anyone by surprise.  Furthermore,
the privileged program which is filling both its pI and pA
and then executing other files could achieve the same result
by filling pI and setting file capaiblities on all executables.
Setting pA gives them a different tradeoff (limiting the
capabilities trust to its process tree, but to all binaries)
which should do what Christoph wants.  By limiting the effective pA to fP
if fP is not empty, we'r eonly prevneting the file which
had fP set from running in an unexpected way which should be
safer.  (But if it executes another file, that file, it will
receive the original pA)

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> No.  fE is not "not available".  If you set it with setcap, you *should*
> (if i'm tinking right) find fE full when calculating the new capability
> sets, because of magic_etc.

There is nothing in get_vfs_caps_from_disk that does this and the magic
vanishes after this function is done.

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Andy Lutomirski wrote:

> I'm still extremely nervous about allowing nonempty pA to propagate to
> setuid or nonzero fP programs.  It's less obviously dangerous if pA is
> never a superset of pP, but it could still cause problems with setuid
> programs that execute intentionally deprivileged helpers.

Well but the intend of the ambient caps is that all processes spawned have
those caps. So they should not be dropped implicitly by othe mechanisms
because they could spawn scripts etc that need these privs again.

Re: [PATCH] capabilities: Ambient capability set V1

[Serge E. Hallyn]

On Thu, Feb 26, 2015 at 03:23:35PM -0600, Christoph Lameter wrote:
> On Thu, 26 Feb 2015, Serge E. Hallyn wrote:
> 
> > No.  fE is not "not available".  If you set it with setcap, you *should*
> > (if i'm tinking right) find fE full when calculating the new capability
> > sets, because of magic_etc.
> 
> There is nothing in get_vfs_caps_from_disk that does this and the magic
> vanishes after this function is done.

get_vfs_caps_from_disk does:

        cpu_caps->magic_etc = magic_etc = le32_to_cpu(caps.magic_etc);

then bprm_caps_from_vfs_caps does:

        if (caps->magic_etc & VFS_CAP_FLAGS_EFFECTIVE)
                *effective = true;

and finally cap_bprm_set_creds does:

        if (effective)
                new->cap_effective = new->cap_permitted;
        else
                cap_clear(new->cap_effective);

Re: [PATCH] capabilities: Ambient capability set V1

[Christoph Lameter]

On Thu, 26 Feb 2015, Serge E. Hallyn wrote:

> > There is nothing in get_vfs_caps_from_disk that does this and the magic
> > vanishes after this function is done.
>
> get_vfs_caps_from_disk does:
>
>         cpu_caps->magic_etc = magic_etc = le32_to_cpu(caps.magic_etc);
>
> then bprm_caps_from_vfs_caps does:
>
>         if (caps->magic_etc & VFS_CAP_FLAGS_EFFECTIVE)
>                 *effective = true;

Ahhh.. I was wondering what that is.

> and finally cap_bprm_set_creds does:
>
>         if (effective)
>                 new->cap_effective = new->cap_permitted;
>         else
>                 cap_clear(new->cap_effective);

Ok. I took that out thats why it worked.