Skip to contentSkip to conversation

remove securebits

PATCH In review Adrian Bunk 3 participants 13 messages Aug 24, 2007 – Aug 30, 2007

Subsystems (4)
Scheduler SELinux security module Security (LSM) Capabilities
lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 remove securebits

It seems that since it was added in kernel 2.2.0 (sic) securebits was never used.

This patch therefore removes it.

Signed-off-by Adrian Bunk <[email protected]>

---

 include/linux/sched.h      |    1 -
 include/linux/securebits.h |   30 ------------------------------
 kernel/capability.c        |    1 -
 security/commoncap.c       |   34 ++++++++++++++--------------------
 security/dummy.c           |   16 +++++++---------
 5 files changed, 21 insertions(+), 61 deletions(-)

30c1d49582d183ea4a7ee0ffd886dcd9e2344115

include/linux/sched.h +0 -1
index 2b3c936..be2e9c4 100644
@@ -67,7 +67,6 @@ struct sched_param {
 #include <linux/smp.h>
 #include <linux/sem.h>
 #include <linux/signal.h>
-#include <linux/securebits.h>
 #include <linux/fs_struct.h>
 #include <linux/compiler.h>
 #include <linux/completion.h>
include/linux/securebits.h deleted +0 -30
deleted file mode 100644
index 5b06178..0000000
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x00000000
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
-   0. If the real uid is 0, we raise the inheritable bitmask of the
-   executable file. */
-#define SECURE_NOROOT            0
-
-/* When set, setuid to/from uid 0 does not trigger capability-"fixes"
-   to be compatible with old programs relying on set*uid to loose
-   privileges. When unset, setuid doesn't change privileges. */
-#define SECURE_NO_SETUID_FIXUP   2
-
-/* Each securesetting is implemented using two bits. One bit specify
-   whether the setting is on or off. The other bit specify whether the
-   setting is fixed or not. A setting which is fixed cannot be changed
-   from user-level. */
-
-#define issecure(X) ( (1 << (X+1)) & SECUREBITS_DEFAULT ? 	\
-		      (1 << (X)) & SECUREBITS_DEFAULT :		\
-		      (1 << (X)) & securebits )
-
-#endif /* !_LINUX_SECUREBITS_H */
kernel/capability.c +0 -1
index 20914d8..d3696a9 100644
@@ -15,7 +15,6 @@
 #include <linux/pid_namespace.h>
 #include <asm/uaccess.h>
 
-unsigned securebits = SECUREBITS_DEFAULT; /* systemwide security settings */
 kernel_cap_t cap_bset = CAP_INIT_EFF_SET;
 
 /*
security/commoncap.c +14 -20
index ff87b80..ce8f686 100644
@@ -241,14 +241,12 @@ int cap_bprm_set_security (struct linux_binprm *bprm)
 	 *  and permitted sets of the executable file.
 	 */
 
-	if (!issecure (SECURE_NOROOT)) {
-		if (bprm->e_uid == 0 || current->uid == 0) {
-			cap_set_full (bprm->cap_inheritable);
-			cap_set_full (bprm->cap_permitted);
-		}
-		if (bprm->e_uid == 0)
-			bprm->cap_effective = true;
+	if (bprm->e_uid == 0 || current->uid == 0) {
+		cap_set_full (bprm->cap_inheritable);
+		cap_set_full (bprm->cap_permitted);
 	}
+	if (bprm->e_uid == 0)
+		bprm->cap_effective = true;
 
 	return ret;
 }

@@ -393,9 +391,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 	case LSM_SETID_ID:
 	case LSM_SETID_RES:
 		/* Copied from kernel/sys.c:setreuid/setuid/setresuid. */
-		if (!issecure (SECURE_NO_SETUID_FIXUP)) {
-			cap_emulate_setxuid (old_ruid, old_euid, old_suid);
-		}
+		cap_emulate_setxuid (old_ruid, old_euid, old_suid);
 		break;
 	case LSM_SETID_FS:
 		{

@@ -408,16 +404,14 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 			 *          if not, we might be a bit too harsh here.
 			 */
 
-			if (!issecure (SECURE_NO_SETUID_FIXUP)) {
-				if (old_fsuid == 0 && current->fsuid != 0) {
-					cap_t (current->cap_effective) &=
-					    ~CAP_FS_MASK;
-				}
-				if (old_fsuid != 0 && current->fsuid == 0) {
-					cap_t (current->cap_effective) |=
-					    (cap_t (current->cap_permitted) &
-					     CAP_FS_MASK);
-				}
+			if (old_fsuid == 0 && current->fsuid != 0) {
+				cap_t (current->cap_effective) &=
+				    ~CAP_FS_MASK;
+			}
+			if (old_fsuid != 0 && current->fsuid == 0) {
+				cap_t (current->cap_effective) |=
+				    (cap_t (current->cap_permitted) &
+				     CAP_FS_MASK);
 			}
 			break;
 		}
security/dummy.c +7 -9
index 6999456..88bb1bc 100644
@@ -37,15 +37,13 @@ static int dummy_capget (struct task_struct *target, kernel_cap_t * effective,
 			 kernel_cap_t * inheritable, kernel_cap_t * permitted)
 {
 	*effective = *inheritable = *permitted = 0;
-	if (!issecure(SECURE_NOROOT)) {
-		if (target->euid == 0) {
-			*permitted |= (~0 & ~CAP_FS_MASK);
-			*effective |= (~0 & ~CAP_TO_MASK(CAP_SETPCAP) & ~CAP_FS_MASK);
-		}
-		if (target->fsuid == 0) {
-			*permitted |= CAP_FS_MASK;
-			*effective |= CAP_FS_MASK;
-		}
+	if (target->euid == 0) {
+		*permitted |= (~0 & ~CAP_FS_MASK);
+		*effective |= (~0 & ~CAP_TO_MASK(CAP_SETPCAP) & ~CAP_FS_MASK);
+	}
+	if (target->fsuid == 0) {
+		*permitted |= CAP_FS_MASK;
+		*effective |= CAP_FS_MASK;
 	}
 	return 0;
 }
maintainer
#1 lore raw
Quoted 4 lines · Adrian Bunk

It seems that since it was added in kernel 2.2.0 (sic) securebits was never used.

This patch therefore removes it.

Actually IIUC Andrew Morgan had plans of making securebits per-process, which would make them far more usable.

Now maybe he'd just as soon start with a clean slate... Andrew?

-serge

Quoted 164 lines
Signed-off-by Adrian Bunk <[email protected]>

---

 include/linux/sched.h      |    1 -
 include/linux/securebits.h |   30 ------------------------------
 kernel/capability.c        |    1 -
 security/commoncap.c       |   34 ++++++++++++++--------------------
 security/dummy.c           |   16 +++++++---------
 5 files changed, 21 insertions(+), 61 deletions(-)

30c1d49582d183ea4a7ee0ffd886dcd9e2344115

include/linux/sched.h +0 -1
index 2b3c936..be2e9c4 100644
@@ -67,7 +67,6 @@ struct sched_param {
 #include <linux/smp.h>
 #include <linux/sem.h>
 #include <linux/signal.h>
-#include <linux/securebits.h>
 #include <linux/fs_struct.h>
 #include <linux/compiler.h>
 #include <linux/completion.h>
include/linux/securebits.h deleted +0 -30
deleted file mode 100644
index 5b06178..0000000
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x00000000
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
-   0. If the real uid is 0, we raise the inheritable bitmask of the
-   executable file. */
-#define SECURE_NOROOT            0
-
-/* When set, setuid to/from uid 0 does not trigger capability-"fixes"
-   to be compatible with old programs relying on set*uid to loose
-   privileges. When unset, setuid doesn't change privileges. */
-#define SECURE_NO_SETUID_FIXUP   2
-
-/* Each securesetting is implemented using two bits. One bit specify
-   whether the setting is on or off. The other bit specify whether the
-   setting is fixed or not. A setting which is fixed cannot be changed
-   from user-level. */
-
-#define issecure(X) ( (1 << (X+1)) & SECUREBITS_DEFAULT ? 	\
-		      (1 << (X)) & SECUREBITS_DEFAULT :		\
-		      (1 << (X)) & securebits )
-
-#endif /* !_LINUX_SECUREBITS_H */
kernel/capability.c +0 -1
index 20914d8..d3696a9 100644
@@ -15,7 +15,6 @@
 #include <linux/pid_namespace.h>
 #include <asm/uaccess.h>
 
-unsigned securebits = SECUREBITS_DEFAULT; /* systemwide security settings */
 kernel_cap_t cap_bset = CAP_INIT_EFF_SET;
 
 /*
security/commoncap.c +14 -20
index ff87b80..ce8f686 100644
@@ -241,14 +241,12 @@ int cap_bprm_set_security (struct linux_binprm *bprm)
 	 *  and permitted sets of the executable file.
 	 */
 
-	if (!issecure (SECURE_NOROOT)) {
-		if (bprm->e_uid == 0 || current->uid == 0) {
-			cap_set_full (bprm->cap_inheritable);
-			cap_set_full (bprm->cap_permitted);
-		}
-		if (bprm->e_uid == 0)
-			bprm->cap_effective = true;
+	if (bprm->e_uid == 0 || current->uid == 0) {
+		cap_set_full (bprm->cap_inheritable);
+		cap_set_full (bprm->cap_permitted);
 	}
+	if (bprm->e_uid == 0)
+		bprm->cap_effective = true;
 
 	return ret;
 }

@@ -393,9 +391,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 	case LSM_SETID_ID:
 	case LSM_SETID_RES:
 		/* Copied from kernel/sys.c:setreuid/setuid/setresuid. */
-		if (!issecure (SECURE_NO_SETUID_FIXUP)) {
-			cap_emulate_setxuid (old_ruid, old_euid, old_suid);
-		}
+		cap_emulate_setxuid (old_ruid, old_euid, old_suid);
 		break;
 	case LSM_SETID_FS:
 		{

@@ -408,16 +404,14 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 			 *          if not, we might be a bit too harsh here.
 			 */
 
-			if (!issecure (SECURE_NO_SETUID_FIXUP)) {
-				if (old_fsuid == 0 && current->fsuid != 0) {
-					cap_t (current->cap_effective) &=
-					    ~CAP_FS_MASK;
-				}
-				if (old_fsuid != 0 && current->fsuid == 0) {
-					cap_t (current->cap_effective) |=
-					    (cap_t (current->cap_permitted) &
-					     CAP_FS_MASK);
-				}
+			if (old_fsuid == 0 && current->fsuid != 0) {
+				cap_t (current->cap_effective) &=
+				    ~CAP_FS_MASK;
+			}
+			if (old_fsuid != 0 && current->fsuid == 0) {
+				cap_t (current->cap_effective) |=
+				    (cap_t (current->cap_permitted) &
+				     CAP_FS_MASK);
 			}
 			break;
 		}
security/dummy.c +7 -10
index 6999456..88bb1bc 100644
@@ -37,15 +37,13 @@ static int dummy_capget (struct task_struct *target, kernel_cap_t * effective,
 			 kernel_cap_t * inheritable, kernel_cap_t * permitted)
 {
 	*effective = *inheritable = *permitted = 0;
-	if (!issecure(SECURE_NOROOT)) {
-		if (target->euid == 0) {
-			*permitted |= (~0 & ~CAP_FS_MASK);
-			*effective |= (~0 & ~CAP_TO_MASK(CAP_SETPCAP) & ~CAP_FS_MASK);
-		}
-		if (target->fsuid == 0) {
-			*permitted |= CAP_FS_MASK;
-			*effective |= CAP_FS_MASK;
-		}
+	if (target->euid == 0) {
+		*permitted |= (~0 & ~CAP_FS_MASK);
+		*effective |= (~0 & ~CAP_TO_MASK(CAP_SETPCAP) & ~CAP_FS_MASK);
+	}
+	if (target->fsuid == 0) {
+		*permitted |= CAP_FS_MASK;
+		*effective |= CAP_FS_MASK;
 	}
 	return 0;
 }
 
-

To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

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

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

Cheers

Andrew

Quoted 10 lines · Serge E. Hallyn
Quoted 4 lines · Adrian Bunk

It seems that since it was added in kernel 2.2.0 (sic) securebits was never used.

This patch therefore removes it.

Actually IIUC Andrew Morgan had plans of making securebits per-process, which would make them far more usable.

Now maybe he'd just as soon start with a clean slate... Andrew?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGz6bwQheEq9QabfIRAjx5AKCZSFZ0dv4HTUtUYtm6OdVlOUi3ewCdGHzE TnoeF19cOljfiyntgkcSCbE=
=lW84
-----END PGP SIGNATURE-----

author
#3 lore raw
Quoted 6 lines · Andrew Morgan

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Quoted 7 lines

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

It was in the tree for nine years (sic) without a single user...

Are you only improving a dead horse, or do you also have a rider for the improved dead horse?

Quoted 3 lines

Cheers

Andrew

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
maintainer
#4 lore raw
Quoted 10 lines · Adrian Bunk
Quoted 9 lines · Andrew Morgan

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

Quoted 6 lines
Quoted 4 lines

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

It was in the tree for nine years (sic) without a single user...

That's because without file capabilities there was no way for a process to retain capabilities across exec, so not having a privileged root user was simply not workable.

Quoted 2 lines

Are you only improving a dead horse, or do you also have a rider for the improved dead horse?

It will allow process trees to run with strict capabilities, without a root user which automatically gains full capabilities. That wasn't possible without file capabilities, since there was no way for processes to retain capabilities across exec. Now that we have file capabilities, it is feasible, and it certainly is useful.

-serge

author
#5 lore raw
Quoted 22 lines · Serge E. Hallyn
Quoted 10 lines · Adrian Bunk
Quoted 9 lines · Andrew Morgan

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

Quoted 6 lines
Quoted 4 lines

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

It was in the tree for nine years (sic) without a single user...

That's because without file capabilities there was no way for a process to retain capabilities across exec, so not having a privileged root user was simply not workable.

Quoted 8 lines
Quoted 2 lines

Are you only improving a dead horse, or do you also have a rider for the improved dead horse?

It will allow process trees to run with strict capabilities, without a root user which automatically gains full capabilities. That wasn't possible without file capabilities, since there was no way for processes to retain capabilities across exec. Now that we have file capabilities, it is feasible, and it certainly is useful.

I didn't question that the dead horse gets improved, but where's the rider?

A user of the improved securebits has to be submitted for inclusion in the kernel.

Quoted 1 line

-serge

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
maintainer
#6 lore raw
Quoted 32 lines · Adrian Bunk
Quoted 22 lines · Serge E. Hallyn
Quoted 10 lines · Adrian Bunk
Quoted 9 lines · Andrew Morgan

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

Quoted 6 lines
Quoted 4 lines

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

It was in the tree for nine years (sic) without a single user...

That's because without file capabilities there was no way for a process to retain capabilities across exec, so not having a privileged root user was simply not workable.

Quoted 8 lines
Quoted 2 lines

Are you only improving a dead horse, or do you also have a rider for the improved dead horse?

It will allow process trees to run with strict capabilities, without a root user which automatically gains full capabilities. That wasn't possible without file capabilities, since there was no way for processes to retain capabilities across exec. Now that we have file capabilities, it is feasible, and it certainly is useful.

Quoted 5 lines

I didn't question that the dead horse gets improved, but where's the rider?

A user of the improved securebits has to be submitted for inclusion in the kernel.

The user would be userspace...

Unless by 'the user' you actually mean the patch itself which will allow the setting of secure_noroot per-process. I don't know for sure, but suspect Andrew might like to wait until file capabilities make it into and stabilize in Linus' tree before going on with that.

-serge

author
#7 lore raw
Quoted 39 lines · Serge E. Hallyn
Quoted 32 lines · Adrian Bunk
Quoted 22 lines · Serge E. Hallyn
Quoted 10 lines · Adrian Bunk
Quoted 9 lines · Andrew Morgan

FWIW, in the mm kernel, I've actually already removed them when one configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was no way to set them anyway.

Quoted 6 lines
Quoted 4 lines

I'd obviously prefer to wait for the mm-merge process to complete and minimize the churn in this area, but I basically agree that the bits as implemented are pretty useless in their current form. In a per-process mode (with filesystem capability support) they are much more useful...

It was in the tree for nine years (sic) without a single user...

That's because without file capabilities there was no way for a process to retain capabilities across exec, so not having a privileged root user was simply not workable.

Quoted 8 lines
Quoted 2 lines

Are you only improving a dead horse, or do you also have a rider for the improved dead horse?

It will allow process trees to run with strict capabilities, without a root user which automatically gains full capabilities. That wasn't possible without file capabilities, since there was no way for processes to retain capabilities across exec. Now that we have file capabilities, it is feasible, and it certainly is useful.

Quoted 5 lines

I didn't question that the dead horse gets improved, but where's the rider?

A user of the improved securebits has to be submitted for inclusion in the kernel.

Quoted 6 lines

The user would be userspace...

Unless by 'the user' you actually mean the patch itself which will allow the setting of secure_noroot per-process. I don't know for sure, but suspect Andrew might like to wait until file capabilities make it into and stabilize in Linus' tree before going on with that.

That's what I am talking about.

This patch should be submitted and discussed together with the changes Andrew has for securebits.

Quoted 1 line

-serge

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#8 lore raw

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

Attached is what I consider only an RFC patch.

I've not really thought through (to my satisfaction) the re-purposing of current->keep_capabilities in the non-filesystem-supporting-capability configuration, but this is basically the code I'm thinking about. (I'm typing this email from a system running this patch over 2.6.23-rc3-mm1 so its not 'obviously' broken.)

Quoted 11 lines · Adrian Bunk
Quoted 6 lines

The user would be userspace...

Unless by 'the user' you actually mean the patch itself which will allow the setting of secure_noroot per-process. I don't know for sure, but suspect Andrew might like to wait until file capabilities make it into and stabilize in Linus' tree before going on with that.

That's what I am talking about.

This patch should be submitted and discussed together with the changes Andrew has for securebits.

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFG08y0QheEq9QabfIRAnUhAKCEHyUko292kULNTkRqQOGki2NohgCdGXvV bc+bHzBbI6sPimdf4UTAzGY=
=vB0u
-----END PGP SIGNATURE-----

Quoted 1 line

From a8366ab7a12ed4283e24096e891ac13c1d471756 Mon Sep 17 00:00:00 2001

From: Andrew Morgan <[email protected]>
Date: Mon, 27 Aug 2007 23:09:20 -0700
Subject: [PATCH] Remove global securebits from the kernel.

In the absence of filesystem capability support, there is no longer any internal support for disabling root. The plain fact is that while there was internal support for it, the userspace API for enabling it was not implemented, and having a global personality of this sort was almost impossible to use.

With filesystem capability support, a new prctl(PR_[GS]ET_CAPONLY) per-process flag is available. Once set (to 1) a process and all of its children will irrevocably lose root-is-all-capable privilege.

NOTE: just because root within a given process tree is not capable, it does not mean root is impotent. Most Linux systems are riddled with critical system files that are owned by root (uid=0), so much care should be used relying on the security provided by this support. It is likely that the best way to use this feature is within some sort of restricted (chroot etc.) environment. Be especially careful of where you mount /proc/....

---
 include/linux/capability.h |   10 +++-
 include/linux/prctl.h      |    4 +
 include/linux/sched.h      |    1 -
 include/linux/securebits.h |   30 ----------
 include/linux/security.h   |   17 ++++--
 kernel/sys.c               |   14 +----
 security/capability.c      |    1 +
 security/commoncap.c       |  132 +++++++++++++++++++++++++++++---------------
 security/dummy.c           |    5 +-
 security/security.c        |    5 +-
 security/selinux/hooks.c   |    6 +-
 11 files changed, 124 insertions(+), 101 deletions(-)
 delete mode 100644 include/linux/securebits.h
include/linux/capability.h +8 -2
index 7a8d7ad..0fcef09 100644
@@ -147,8 +147,14 @@ typedef __u32 kernel_cap_t;
  ** Linux-specific capabilities
  **/
 
-/* Transfer any capability in your permitted set to any pid,
-   remove any capability in your permitted set from any pid */
+/* With filesystem support for capabilities configured:
+ *   Permit the current process to raise any capability in its inheritable set.
+ *   Permit the current process to lock the process into capability-only mode
+ *     - prctl(PR_SET_CAPONLY, 1, ...)
+ * Otherwise:
+ *   Transfer any capability in your permitted set to any pid,
+ *   remove any capability in your permitted set from any pid
+ */
 
 #define CAP_SETPCAP          8
include/linux/prctl.h +4 -0
index e2eff90..e3f49a7 100644
@@ -63,4 +63,8 @@
 #define PR_GET_SECCOMP	21
 #define PR_SET_SECCOMP	22
 
+/* Get/Set process personality to secure-by-capabilities alone */
+#define PR_GET_CAPONLY  23
+#define PR_SET_CAPONLY  24
+
 #endif /* _LINUX_PRCTL_H */
include/linux/sched.h +0 -1
index 2b3c936..be2e9c4 100644
@@ -67,7 +67,6 @@ struct sched_param {
 #include <linux/smp.h>
 #include <linux/sem.h>
 #include <linux/signal.h>
-#include <linux/securebits.h>
 #include <linux/fs_struct.h>
 #include <linux/compiler.h>
 #include <linux/completion.h>
include/linux/securebits.h deleted +0 -30
deleted file mode 100644
index 5b06178..0000000
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x00000000
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
-   0. If the real uid is 0, we raise the inheritable bitmask of the
-   executable file. */
-#define SECURE_NOROOT            0
-
-/* When set, setuid to/from uid 0 does not trigger capability-"fixes"
-   to be compatible with old programs relying on set*uid to loose
-   privileges. When unset, setuid doesn't change privileges. */
-#define SECURE_NO_SETUID_FIXUP   2
-
-/* Each securesetting is implemented using two bits. One bit specify
-   whether the setting is on or off. The other bit specify whether the
-   setting is fixed or not. A setting which is fixed cannot be changed
-   from user-level. */
-
-#define issecure(X) ( (1 << (X+1)) & SECUREBITS_DEFAULT ? 	\
-		      (1 << (X)) & SECUREBITS_DEFAULT :		\
-		      (1 << (X)) & securebits )
-
-#endif /* !_LINUX_SECUREBITS_H */
include/linux/security.h +12 -5
index 13d48fd..4227464 100644
@@ -660,13 +660,18 @@ struct request_sock;
  *	Return 0 if permission is granted.
  * @task_prctl:
  *	Check permission before performing a process control operation on the
- *	current process.
+ *	current process. This hook can also be used to provide LSM specific
+ *      process control functions.
  *	@option contains the operation.
  *	@arg2 contains a argument.
  *	@arg3 contains a argument.
  *	@arg4 contains a argument.
  *	@arg5 contains a argument.
- *	Return 0 if permission is granted.
+ *      @errorp contains a pointer to the kernel variable holding the return
+ *      value.
+ *	Return 0 if permission is granted (pass-through); return 1
+ *	(and set *errorp to the appropriate value, if the LSM is
+ *	overriding default behavior.
  * @task_reparent_to_init:
  * 	Set the security attributes in @p->security for a kernel thread that
  * 	is being reparented to the init task.

@@ -1304,7 +1309,7 @@ struct security_operations {
 	int (*task_wait) (struct task_struct * p);
 	int (*task_prctl) (int option, unsigned long arg2,
 			   unsigned long arg3, unsigned long arg4,
-			   unsigned long arg5);
+			   unsigned long arg5, long *errorp);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
 

@@ -1550,7 +1555,8 @@ int security_task_kill(struct task_struct *p, struct siginfo *info,
 			int sig, u32 secid);
 int security_task_wait(struct task_struct *p);
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5);
+			 unsigned long arg4, unsigned long arg5,
+			 long *errorp);
 void security_task_reparent_to_init(struct task_struct *p);
 void security_task_to_inode(struct task_struct *p, struct inode *inode);
 int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);

@@ -2096,7 +2102,8 @@ static inline int security_task_wait (struct task_struct *p)
 static inline int security_task_prctl (int option, unsigned long arg2,
 				       unsigned long arg3,
 				       unsigned long arg4,
-				       unsigned long arg5)
+				       unsigned long arg5,
+				       long *errorp)
 {
 	return 0;
 }
kernel/sys.c +1 -13
index c7c4fa4..42f26fd 100644
@@ -1639,8 +1639,7 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 {
 	long error;
 
-	error = security_task_prctl(option, arg2, arg3, arg4, arg5);
-	if (error)
+	if (security_task_prctl(option, arg2, arg3, arg4, arg5, &error))
 		return error;
 
 	switch (option) {

@@ -1693,17 +1692,6 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 				error = -EINVAL;
 			break;
 
-		case PR_GET_KEEPCAPS:
-			if (current->keep_capabilities)
-				error = 1;
-			break;
-		case PR_SET_KEEPCAPS:
-			if (arg2 != 0 && arg2 != 1) {
-				error = -EINVAL;
-				break;
-			}
-			current->keep_capabilities = arg2;
-			break;
 		case PR_SET_NAME: {
 			struct task_struct *me = current;
 			unsigned char ncomm[sizeof(me->comm)];
security/capability.c +1 -0
index 9e99f36..8340655 100644
@@ -45,6 +45,7 @@ static struct security_operations capability_ops = {
 	.task_setioprio =		cap_task_setioprio,
 	.task_setnice =			cap_task_setnice,
 	.task_post_setuid =		cap_task_post_setuid,
+	.task_prctl =                   cap_task_prctl,
 	.task_reparent_to_init =	cap_task_reparent_to_init,
 
 	.syslog =                       cap_syslog,
security/commoncap.c +88 -44

202 lines not on this page · raw message

security/dummy.c +3 -2
index 88bb1bc..0497dc8 100644
@@ -566,8 +566,9 @@ static int dummy_task_kill (struct task_struct *p, struct siginfo *info,
 	return 0;
 }
 
-static int dummy_task_prctl (int option, unsigned long arg2, unsigned long arg3,
-			     unsigned long arg4, unsigned long arg5)
+static int dummy_task_prctl (int option, unsigned long arg2,
+			     unsigned long arg3, unsigned long arg4,
+			     unsigned long arg5, long *errorp)
 {
 	return 0;
 }
security/security.c +3 -2
index e6d53b3..e3a4285 100644
@@ -665,9 +665,10 @@ int security_task_wait(struct task_struct *p)
 }
 
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5)
+			unsigned long arg4, unsigned long arg5, long *errorp)
 {
-	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5);
+	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					errorp);
 }
 
 void security_task_reparent_to_init(struct task_struct *p)
security/selinux/hooks.c +4 -2
index dc196b9..c8aac50 100644
@@ -2913,12 +2913,14 @@ static int selinux_task_prctl(int option,
 			      unsigned long arg2,
 			      unsigned long arg3,
 			      unsigned long arg4,
-			      unsigned long arg5)
+			      unsigned long arg5,
+			      long *errorp)
 {
 	/* The current prctl operations do not appear to require
 	   any SELinux controls since they merely observe or modify
 	   the state of the current process. */
-	return 0;
+	return secondary_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					 errorp);
 }
 
 static int selinux_task_wait(struct task_struct *p)
Signature · 1 line
1.5.1.3
maintainer
#9 lore raw
Quoted 10 lines · Andrew Morgan

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

Attached is what I consider only an RFC patch.

I've not really thought through (to my satisfaction) the re-purposing of current->keep_capabilities in the non-filesystem-supporting-capability configuration, but this is basically the code I'm thinking about. (I'm typing this email from a system running this patch over 2.6.23-rc3-mm1 so its not 'obviously' broken.)

Note this needs a prototype for cap_task_prctl in
include/linux/security.h for certain configs.

Yes, the reuse of keep_capabilities is interesting. Will think about it for awhile :)

In any case if this is the approach taken, then the change in meaning for keep_capabilities will need to be commented in both the patch description and in comments in the patch itself.

Thanks Andrew,

-serge

Quoted 515 lines
Quoted 11 lines · Adrian Bunk
Quoted 6 lines

The user would be userspace...

Unless by 'the user' you actually mean the patch itself which will allow the setting of secure_noroot per-process. I don't know for sure, but suspect Andrew might like to wait until file capabilities make it into and stabilize in Linus' tree before going on with that.

That's what I am talking about.

This patch should be submitted and discussed together with the changes Andrew has for securebits.

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFG08y0QheEq9QabfIRAnUhAKCEHyUko292kULNTkRqQOGki2NohgCdGXvV bc+bHzBbI6sPimdf4UTAzGY=
=vB0u
-----END PGP SIGNATURE-----

Quoted 1 line

From a8366ab7a12ed4283e24096e891ac13c1d471756 Mon Sep 17 00:00:00 2001

From: Andrew Morgan <[email protected]>
Date: Mon, 27 Aug 2007 23:09:20 -0700
Subject: [PATCH] Remove global securebits from the kernel.

In the absence of filesystem capability support, there is no longer any internal support for disabling root. The plain fact is that while there was internal support for it, the userspace API for enabling it was not implemented, and having a global personality of this sort was almost impossible to use.

With filesystem capability support, a new prctl(PR_[GS]ET_CAPONLY) per-process flag is available. Once set (to 1) a process and all of its children will irrevocably lose root-is-all-capable privilege.

NOTE: just because root within a given process tree is not capable, it does not mean root is impotent. Most Linux systems are riddled with critical system files that are owned by root (uid=0), so much care should be used relying on the security provided by this support. It is likely that the best way to use this feature is within some sort of restricted (chroot etc.) environment. Be especially careful of where you mount /proc/....

---
 include/linux/capability.h |   10 +++-
 include/linux/prctl.h      |    4 +
 include/linux/sched.h      |    1 -
 include/linux/securebits.h |   30 ----------
 include/linux/security.h   |   17 ++++--
 kernel/sys.c               |   14 +----
 security/capability.c      |    1 +
 security/commoncap.c       |  132 +++++++++++++++++++++++++++++---------------
 security/dummy.c           |    5 +-
 security/security.c        |    5 +-
 security/selinux/hooks.c   |    6 +-
 11 files changed, 124 insertions(+), 101 deletions(-)
 delete mode 100644 include/linux/securebits.h
include/linux/capability.h +8 -2
index 7a8d7ad..0fcef09 100644
@@ -147,8 +147,14 @@ typedef __u32 kernel_cap_t;
  ** Linux-specific capabilities
  **/
 
-/* Transfer any capability in your permitted set to any pid,
-   remove any capability in your permitted set from any pid */
+/* With filesystem support for capabilities configured:
+ *   Permit the current process to raise any capability in its inheritable set.
+ *   Permit the current process to lock the process into capability-only mode
+ *     - prctl(PR_SET_CAPONLY, 1, ...)
+ * Otherwise:
+ *   Transfer any capability in your permitted set to any pid,
+ *   remove any capability in your permitted set from any pid
+ */
 
 #define CAP_SETPCAP          8
include/linux/prctl.h +4 -0
index e2eff90..e3f49a7 100644
@@ -63,4 +63,8 @@
 #define PR_GET_SECCOMP	21
 #define PR_SET_SECCOMP	22
 
+/* Get/Set process personality to secure-by-capabilities alone */
+#define PR_GET_CAPONLY  23
+#define PR_SET_CAPONLY  24
+
 #endif /* _LINUX_PRCTL_H */
include/linux/sched.h +0 -1
index 2b3c936..be2e9c4 100644
@@ -67,7 +67,6 @@ struct sched_param {
 #include <linux/smp.h>
 #include <linux/sem.h>
 #include <linux/signal.h>
-#include <linux/securebits.h>
 #include <linux/fs_struct.h>
 #include <linux/compiler.h>
 #include <linux/completion.h>
include/linux/securebits.h deleted +0 -30
deleted file mode 100644
index 5b06178..0000000
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x00000000
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
-   0. If the real uid is 0, we raise the inheritable bitmask of the
-   executable file. */
-#define SECURE_NOROOT            0
-
-/* When set, setuid to/from uid 0 does not trigger capability-"fixes"
-   to be compatible with old programs relying on set*uid to loose
-   privileges. When unset, setuid doesn't change privileges. */
-#define SECURE_NO_SETUID_FIXUP   2
-
-/* Each securesetting is implemented using two bits. One bit specify
-   whether the setting is on or off. The other bit specify whether the
-   setting is fixed or not. A setting which is fixed cannot be changed
-   from user-level. */
-
-#define issecure(X) ( (1 << (X+1)) & SECUREBITS_DEFAULT ? 	\
-		      (1 << (X)) & SECUREBITS_DEFAULT :		\
-		      (1 << (X)) & securebits )
-
-#endif /* !_LINUX_SECUREBITS_H */
include/linux/security.h +12 -5
index 13d48fd..4227464 100644
@@ -660,13 +660,18 @@ struct request_sock;
  *	Return 0 if permission is granted.
  * @task_prctl:
  *	Check permission before performing a process control operation on the
- *	current process.
+ *	current process. This hook can also be used to provide LSM specific
+ *      process control functions.
  *	@option contains the operation.
  *	@arg2 contains a argument.
  *	@arg3 contains a argument.
  *	@arg4 contains a argument.
  *	@arg5 contains a argument.
- *	Return 0 if permission is granted.
+ *      @errorp contains a pointer to the kernel variable holding the return
+ *      value.
+ *	Return 0 if permission is granted (pass-through); return 1
+ *	(and set *errorp to the appropriate value, if the LSM is
+ *	overriding default behavior.
  * @task_reparent_to_init:
  * 	Set the security attributes in @p->security for a kernel thread that
  * 	is being reparented to the init task.

@@ -1304,7 +1309,7 @@ struct security_operations {
 	int (*task_wait) (struct task_struct * p);
 	int (*task_prctl) (int option, unsigned long arg2,
 			   unsigned long arg3, unsigned long arg4,
-			   unsigned long arg5);
+			   unsigned long arg5, long *errorp);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
 

@@ -1550,7 +1555,8 @@ int security_task_kill(struct task_struct *p, struct siginfo *info,
 			int sig, u32 secid);
 int security_task_wait(struct task_struct *p);
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5);
+			 unsigned long arg4, unsigned long arg5,
+			 long *errorp);
 void security_task_reparent_to_init(struct task_struct *p);
 void security_task_to_inode(struct task_struct *p, struct inode *inode);
 int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);

@@ -2096,7 +2102,8 @@ static inline int security_task_wait (struct task_struct *p)
 static inline int security_task_prctl (int option, unsigned long arg2,
 				       unsigned long arg3,
 				       unsigned long arg4,
-				       unsigned long arg5)
+				       unsigned long arg5,
+				       long *errorp)
 {
 	return 0;
 }
kernel/sys.c +1 -13
index c7c4fa4..42f26fd 100644
@@ -1639,8 +1639,7 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 {
 	long error;
 
-	error = security_task_prctl(option, arg2, arg3, arg4, arg5);
-	if (error)
+	if (security_task_prctl(option, arg2, arg3, arg4, arg5, &error))
 		return error;
 
 	switch (option) {

@@ -1693,17 +1692,6 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 				error = -EINVAL;
 			break;
 
-		case PR_GET_KEEPCAPS:
-			if (current->keep_capabilities)
-				error = 1;
-			break;
-		case PR_SET_KEEPCAPS:
-			if (arg2 != 0 && arg2 != 1) {
-				error = -EINVAL;
-				break;
-			}
-			current->keep_capabilities = arg2;
-			break;
 		case PR_SET_NAME: {
 			struct task_struct *me = current;
 			unsigned char ncomm[sizeof(me->comm)];
security/capability.c +1 -0
index 9e99f36..8340655 100644
@@ -45,6 +45,7 @@ static struct security_operations capability_ops = {
 	.task_setioprio =		cap_task_setioprio,
 	.task_setnice =			cap_task_setnice,
 	.task_post_setuid =		cap_task_post_setuid,
+	.task_prctl =                   cap_task_prctl,
 	.task_reparent_to_init =	cap_task_reparent_to_init,
 
 	.syslog =                       cap_syslog,
security/commoncap.c +88 -44
index d65ddd3..44bf3fc 100644
@@ -24,6 +24,7 @@
 #include <linux/hugetlb.h>
 #include <linux/mount.h>
 #include <linux/sched.h>
+#include <linux/prctl.h>
 
 #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
 /*

@@ -39,11 +40,6 @@
 kernel_cap_t cap_bset = CAP_INIT_BSET;    /* systemwide capability bound */
 EXPORT_SYMBOL(cap_bset);
 
-/* Global security state */
-
-unsigned securebits = SECUREBITS_DEFAULT; /* systemwide security settings */
-EXPORT_SYMBOL(securebits);
-
 int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
 {
 	NETLINK_CB(skb).eff_cap = current->cap_effective;

@@ -164,6 +160,27 @@ static inline void bprm_clear_caps(struct linux_binprm *bprm)
 	bprm->cap_effective = false;
 }
 
+static inline void bprm_force_uid0_caps(struct linux_binprm *bprm)
+{
+	/*
+	 *  To support inheritance of root-permissions and suid-root
+	 *  executables under compatibility mode, we raise all three
+	 *  capability sets for the file.
+	 *
+	 *  If only the real uid is 0, we only raise the inheritable
+	 *  and permitted sets of the executable file.
+	 */
+	if (bprm->e_uid == 0 || current->uid == 0) {
+		cap_set_full (bprm->cap_inheritable);
+		cap_set_full (bprm->cap_permitted);
+	}
+	if (bprm->e_uid == 0) {
+		bprm->cap_effective = true;
+	}
+
+	return;
+}
+
 #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
 
 int cap_inode_need_killpriv(struct dentry *dentry)

@@ -215,7 +232,7 @@ static inline int cap_from_disk(__le32 *caps, struct linux_binprm *bprm,
 }
 
 /* Locate any VFS capabilities: */
-static int get_file_caps(struct linux_binprm *bprm)
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
 	struct dentry *dentry;
 	int rc = 0;

@@ -223,8 +240,7 @@ static int get_file_caps(struct linux_binprm *bprm)
 	struct inode *inode;
 
 	if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
-		bprm_clear_caps(bprm);
-		return 0;
+		goto out_no_dentry;
 	}
 
 	dentry = dget(bprm->file->f_dentry);

@@ -249,8 +265,16 @@ static int get_file_caps(struct linux_binprm *bprm)
 
 out:
 	dput(dentry);
-	if (rc)
+
+	if (rc) {
+out_no_dentry:
 		bprm_clear_caps(bprm);
+	}
+
+	if (! current->keep_capabilities) {
+		bprm_force_uid0_caps(bprm);
+		rc = 0;
+	}
 
 	return rc;
 }

@@ -266,42 +290,15 @@ int cap_inode_killpriv(struct dentry *dentry)
 	return 0;
 }
 
-static inline int get_file_caps(struct linux_binprm *bprm)
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
 	bprm_clear_caps(bprm);
+	bprm_force_uid0_caps(bprm);
+	current->keep_capabilities = 0;
 	return 0;
 }
 #endif
 
-int cap_bprm_set_security (struct linux_binprm *bprm)
-{
-	int ret;
-
-	ret = get_file_caps(bprm);
-	if (ret)
-		printk(KERN_NOTICE "%s: get_file_caps returned %d for %s\n",
-			__FUNCTION__, ret, bprm->filename);
-
-	/*  To support inheritance of root-permissions and suid-root
-	 *  executables under compatibility mode, we raise all three
-	 *  capability sets for the file.
-	 *
-	 *  If only the real uid is 0, we only raise the inheritable
-	 *  and permitted sets of the executable file.
-	 */
-
-	if (!issecure (SECURE_NOROOT)) {
-		if (bprm->e_uid == 0 || current->uid == 0) {
-			cap_set_full (bprm->cap_inheritable);
-			cap_set_full (bprm->cap_permitted);
-		}
-		if (bprm->e_uid == 0)
-			bprm->cap_effective = true;
-	}
-
-	return ret;
-}
-
 void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 {
 	/* Derived from fs/exec.c:compute_creds. */

@@ -341,8 +338,6 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 	}
 
 	/* AUD: Audit candidate if current->cap_effective is set */
-
-	current->keep_capabilities = 0;
 }
 
 int cap_bprm_secureexec (struct linux_binprm *bprm)

@@ -441,8 +436,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 	case LSM_SETID_RE:
 	case LSM_SETID_ID:
 	case LSM_SETID_RES:
-		/* Copied from kernel/sys.c:setreuid/setuid/setresuid. */
-		if (!issecure (SECURE_NO_SETUID_FIXUP)) {
+		if (! current->keep_capabilities) {
 			cap_emulate_setxuid (old_ruid, old_euid, old_suid);
 		}
 		break;

@@ -457,7 +451,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 			 *          if not, we might be a bit too harsh here.
 			 */
 
-			if (!issecure (SECURE_NO_SETUID_FIXUP)) {
+			if (! current->keep_capabilities) {
 				if (old_fsuid == 0 && current->fsuid != 0) {
 					cap_t (current->cap_effective) &=
 					    ~CAP_FS_MASK;

@@ -579,3 +573,53 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
 	return __vm_enough_memory(mm, pages, cap_sys_admin);
 }
 
+int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
+		   unsigned long arg4, unsigned long arg5, long *errorp)
+{
+	switch (option) {
+
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+
+	case PR_GET_CAPONLY:
+		*errorp = current->keep_capabilities;
+		break;
+
+	case PR_SET_CAPONLY:
+		/*
+		 * Only permit setting to 1
+		 */
+		if ((arg2 != 1) || !cap_capable(current, CAP_SETPCAP)) {
+			*errorp = -EPERM;
+		} else {
+			/*
+			 * Once locked, no unlocking for this process
+			 * and its children
+			 */
+			current->keep_capabilities = 1;
+			*errorp = 0;
+		}
+		break;
+
+#else /* ie. ndef CONFIG_SECURITY_FILE_CAPABILITIES */
+
+	case PR_GET_KEEPCAPS:
+		if (current->keep_capabilities)
+			*errorp = 1;
+		break;
+
+	case PR_SET_KEEPCAPS:
+		if (arg2 != 0 && arg2 != 1) {
+			*errorp = -EINVAL;
+			break;
+		}
+		current->keep_capabilities = arg2;
+		break;
+
+#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
+
+	default:
+		return 0;  /* pass-through */
+	}
+
+	return 1;      /* overridden by capability LSM */
+}
security/dummy.c +3 -2
index 88bb1bc..0497dc8 100644
@@ -566,8 +566,9 @@ static int dummy_task_kill (struct task_struct *p, struct siginfo *info,
 	return 0;
 }
 
-static int dummy_task_prctl (int option, unsigned long arg2, unsigned long arg3,
-			     unsigned long arg4, unsigned long arg5)
+static int dummy_task_prctl (int option, unsigned long arg2,
+			     unsigned long arg3, unsigned long arg4,
+			     unsigned long arg5, long *errorp)
 {
 	return 0;
 }
security/security.c +3 -2
index e6d53b3..e3a4285 100644
@@ -665,9 +665,10 @@ int security_task_wait(struct task_struct *p)
 }
 
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5)
+			unsigned long arg4, unsigned long arg5, long *errorp)
 {
-	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5);
+	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					errorp);
 }
 
 void security_task_reparent_to_init(struct task_struct *p)
security/selinux/hooks.c +4 -2
index dc196b9..c8aac50 100644
@@ -2913,12 +2913,14 @@ static int selinux_task_prctl(int option,
 			      unsigned long arg2,
 			      unsigned long arg3,
 			      unsigned long arg4,
-			      unsigned long arg5)
+			      unsigned long arg5,
+			      long *errorp)
 {
 	/* The current prctl operations do not appear to require
 	   any SELinux controls since they merely observe or modify
 	   the state of the current process. */
-	return 0;
+	return secondary_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					 errorp);
 }
 
 static int selinux_task_wait(struct task_struct *p)
Signature · 1 line
1.5.1.3
maintainer
#10 lore raw
In reply to Andrew Morgan
Quoted 328 lines · Andrew Morgan

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

Attached is what I consider only an RFC patch.

I've not really thought through (to my satisfaction) the re-purposing of current->keep_capabilities in the non-filesystem-supporting-capability configuration, but this is basically the code I'm thinking about. (I'm typing this email from a system running this patch over 2.6.23-rc3-mm1 so its not 'obviously' broken.)

Quoted 11 lines · Adrian Bunk
Quoted 6 lines

The user would be userspace...

Unless by 'the user' you actually mean the patch itself which will allow the setting of secure_noroot per-process. I don't know for sure, but suspect Andrew might like to wait until file capabilities make it into and stabilize in Linus' tree before going on with that.

That's what I am talking about.

This patch should be submitted and discussed together with the changes Andrew has for securebits.

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFG08y0QheEq9QabfIRAnUhAKCEHyUko292kULNTkRqQOGki2NohgCdGXvV bc+bHzBbI6sPimdf4UTAzGY=
=vB0u
-----END PGP SIGNATURE-----

Quoted 1 line

From a8366ab7a12ed4283e24096e891ac13c1d471756 Mon Sep 17 00:00:00 2001

From: Andrew Morgan <[email protected]>
Date: Mon, 27 Aug 2007 23:09:20 -0700
Subject: [PATCH] Remove global securebits from the kernel.

In the absence of filesystem capability support, there is no longer any internal support for disabling root. The plain fact is that while there was internal support for it, the userspace API for enabling it was not implemented, and having a global personality of this sort was almost impossible to use.

With filesystem capability support, a new prctl(PR_[GS]ET_CAPONLY) per-process flag is available. Once set (to 1) a process and all of its children will irrevocably lose root-is-all-capable privilege.

NOTE: just because root within a given process tree is not capable, it does not mean root is impotent. Most Linux systems are riddled with critical system files that are owned by root (uid=0), so much care should be used relying on the security provided by this support. It is likely that the best way to use this feature is within some sort of restricted (chroot etc.) environment. Be especially careful of where you mount /proc/....

---
 include/linux/capability.h |   10 +++-
 include/linux/prctl.h      |    4 +
 include/linux/sched.h      |    1 -
 include/linux/securebits.h |   30 ----------
 include/linux/security.h   |   17 ++++--
 kernel/sys.c               |   14 +----
 security/capability.c      |    1 +
 security/commoncap.c       |  132 +++++++++++++++++++++++++++++---------------
 security/dummy.c           |    5 +-
 security/security.c        |    5 +-
 security/selinux/hooks.c   |    6 +-
 11 files changed, 124 insertions(+), 101 deletions(-)
 delete mode 100644 include/linux/securebits.h
include/linux/capability.h +8 -2
index 7a8d7ad..0fcef09 100644
@@ -147,8 +147,14 @@ typedef __u32 kernel_cap_t;
  ** Linux-specific capabilities
  **/
 
-/* Transfer any capability in your permitted set to any pid,
-   remove any capability in your permitted set from any pid */
+/* With filesystem support for capabilities configured:
+ *   Permit the current process to raise any capability in its inheritable set.
+ *   Permit the current process to lock the process into capability-only mode
+ *     - prctl(PR_SET_CAPONLY, 1, ...)
+ * Otherwise:
+ *   Transfer any capability in your permitted set to any pid,
+ *   remove any capability in your permitted set from any pid
+ */
 
 #define CAP_SETPCAP          8
include/linux/prctl.h +4 -0
index e2eff90..e3f49a7 100644
@@ -63,4 +63,8 @@
 #define PR_GET_SECCOMP	21
 #define PR_SET_SECCOMP	22
 
+/* Get/Set process personality to secure-by-capabilities alone */
+#define PR_GET_CAPONLY  23
+#define PR_SET_CAPONLY  24
+
 #endif /* _LINUX_PRCTL_H */
include/linux/sched.h +0 -1
index 2b3c936..be2e9c4 100644
@@ -67,7 +67,6 @@ struct sched_param {
 #include <linux/smp.h>
 #include <linux/sem.h>
 #include <linux/signal.h>
-#include <linux/securebits.h>
 #include <linux/fs_struct.h>
 #include <linux/compiler.h>
 #include <linux/completion.h>
include/linux/securebits.h deleted +0 -30
deleted file mode 100644
index 5b06178..0000000
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x00000000
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
-   0. If the real uid is 0, we raise the inheritable bitmask of the
-   executable file. */
-#define SECURE_NOROOT            0
-
-/* When set, setuid to/from uid 0 does not trigger capability-"fixes"
-   to be compatible with old programs relying on set*uid to loose
-   privileges. When unset, setuid doesn't change privileges. */
-#define SECURE_NO_SETUID_FIXUP   2
-
-/* Each securesetting is implemented using two bits. One bit specify
-   whether the setting is on or off. The other bit specify whether the
-   setting is fixed or not. A setting which is fixed cannot be changed
-   from user-level. */
-
-#define issecure(X) ( (1 << (X+1)) & SECUREBITS_DEFAULT ? 	\
-		      (1 << (X)) & SECUREBITS_DEFAULT :		\
-		      (1 << (X)) & securebits )
-
-#endif /* !_LINUX_SECUREBITS_H */
include/linux/security.h +12 -5
index 13d48fd..4227464 100644
@@ -660,13 +660,18 @@ struct request_sock;
  *	Return 0 if permission is granted.
  * @task_prctl:
  *	Check permission before performing a process control operation on the
- *	current process.
+ *	current process. This hook can also be used to provide LSM specific
+ *      process control functions.
  *	@option contains the operation.
  *	@arg2 contains a argument.
  *	@arg3 contains a argument.
  *	@arg4 contains a argument.
  *	@arg5 contains a argument.
- *	Return 0 if permission is granted.
+ *      @errorp contains a pointer to the kernel variable holding the return
+ *      value.
+ *	Return 0 if permission is granted (pass-through); return 1
+ *	(and set *errorp to the appropriate value, if the LSM is
+ *	overriding default behavior.
  * @task_reparent_to_init:
  * 	Set the security attributes in @p->security for a kernel thread that
  * 	is being reparented to the init task.

@@ -1304,7 +1309,7 @@ struct security_operations {
 	int (*task_wait) (struct task_struct * p);
 	int (*task_prctl) (int option, unsigned long arg2,
 			   unsigned long arg3, unsigned long arg4,
-			   unsigned long arg5);
+			   unsigned long arg5, long *errorp);
 	void (*task_reparent_to_init) (struct task_struct * p);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
 

@@ -1550,7 +1555,8 @@ int security_task_kill(struct task_struct *p, struct siginfo *info,
 			int sig, u32 secid);
 int security_task_wait(struct task_struct *p);
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5);
+			 unsigned long arg4, unsigned long arg5,
+			 long *errorp);
 void security_task_reparent_to_init(struct task_struct *p);
 void security_task_to_inode(struct task_struct *p, struct inode *inode);
 int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);

@@ -2096,7 +2102,8 @@ static inline int security_task_wait (struct task_struct *p)
 static inline int security_task_prctl (int option, unsigned long arg2,
 				       unsigned long arg3,
 				       unsigned long arg4,
-				       unsigned long arg5)
+				       unsigned long arg5,
+				       long *errorp)
 {
 	return 0;
 }
kernel/sys.c +1 -13
index c7c4fa4..42f26fd 100644
@@ -1639,8 +1639,7 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 {
 	long error;
 
-	error = security_task_prctl(option, arg2, arg3, arg4, arg5);
-	if (error)
+	if (security_task_prctl(option, arg2, arg3, arg4, arg5, &error))
 		return error;
 
 	switch (option) {

@@ -1693,17 +1692,6 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 				error = -EINVAL;
 			break;
 
-		case PR_GET_KEEPCAPS:
-			if (current->keep_capabilities)
-				error = 1;
-			break;
-		case PR_SET_KEEPCAPS:
-			if (arg2 != 0 && arg2 != 1) {
-				error = -EINVAL;
-				break;
-			}
-			current->keep_capabilities = arg2;
-			break;
 		case PR_SET_NAME: {
 			struct task_struct *me = current;
 			unsigned char ncomm[sizeof(me->comm)];
security/capability.c +1 -0
index 9e99f36..8340655 100644
@@ -45,6 +45,7 @@ static struct security_operations capability_ops = {
 	.task_setioprio =		cap_task_setioprio,
 	.task_setnice =			cap_task_setnice,
 	.task_post_setuid =		cap_task_post_setuid,
+	.task_prctl =                   cap_task_prctl,
 	.task_reparent_to_init =	cap_task_reparent_to_init,
 
 	.syslog =                       cap_syslog,
security/commoncap.c +24 -9
index d65ddd3..44bf3fc 100644
@@ -24,6 +24,7 @@
 #include <linux/hugetlb.h>
 #include <linux/mount.h>
 #include <linux/sched.h>
+#include <linux/prctl.h>
 
 #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
 /*

@@ -39,11 +40,6 @@
 kernel_cap_t cap_bset = CAP_INIT_BSET;    /* systemwide capability bound */
 EXPORT_SYMBOL(cap_bset);
 
-/* Global security state */
-
-unsigned securebits = SECUREBITS_DEFAULT; /* systemwide security settings */
-EXPORT_SYMBOL(securebits);
-
 int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
 {
 	NETLINK_CB(skb).eff_cap = current->cap_effective;

@@ -164,6 +160,27 @@ static inline void bprm_clear_caps(struct linux_binprm *bprm)
 	bprm->cap_effective = false;
 }
 
+static inline void bprm_force_uid0_caps(struct linux_binprm *bprm)
+{
+	/*
+	 *  To support inheritance of root-permissions and suid-root
+	 *  executables under compatibility mode, we raise all three
+	 *  capability sets for the file.
+	 *
+	 *  If only the real uid is 0, we only raise the inheritable
+	 *  and permitted sets of the executable file.
+	 */
+	if (bprm->e_uid == 0 || current->uid == 0) {
+		cap_set_full (bprm->cap_inheritable);
+		cap_set_full (bprm->cap_permitted);
+	}
+	if (bprm->e_uid == 0) {
+		bprm->cap_effective = true;
+	}
+
+	return;
+}
+
 #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
 
 int cap_inode_need_killpriv(struct dentry *dentry)

@@ -215,7 +232,7 @@ static inline int cap_from_disk(__le32 *caps, struct linux_binprm *bprm,
 }
 
 /* Locate any VFS capabilities: */
-static int get_file_caps(struct linux_binprm *bprm)
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
 	struct dentry *dentry;
 	int rc = 0;

@@ -223,8 +240,7 @@ static int get_file_caps(struct linux_binprm *bprm)
 	struct inode *inode;
 
 	if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID) {
-		bprm_clear_caps(bprm);
-		return 0;
+		goto out_no_dentry;
 	}
 
 	dentry = dget(bprm->file->f_dentry);

@@ -249,8 +265,16 @@ static int get_file_caps(struct linux_binprm *bprm)
 
 out:
 	dput(dentry);
-	if (rc)

security/commoncap.c static int get_file_caps(struct linux_binprm *bprm) · line 277

Quoted 10 lines
security/commoncap.c +9 -0
+
+	if (rc) {
+out_no_dentry:
 		bprm_clear_caps(bprm);
+	}
+
+	if (! current->keep_capabilities) {
+		bprm_force_uid0_caps(bprm);
+		rc = 0;
+	}

what about a process tree wanting to maintain the current behavior - that is !SECURE_NOROOT, but able to keep_caps across setuid, then regain full privs on executing a setuid binary? That is no longer possible when file capabilities are enabled. I think it should be, given just how long that was the expected way to use capabilities.

Of course that means keep_capabilities can't be multiplexed like this. But that really doesn't seem like a big loss. Trying to be too clever probably means we'll get it wrong, and heck, the name is completely wrong in this sense :)

To summarize more clearly, I think that so long as we support process trees with a sort of !SECURE_NOROOT support, that support should include the ability to use prctl(KEEP_CAPS) the way one uses it now.

When a process tree is in strict capability mode, prctl(PR_{G,S}ET_KEEP_CAPS) should return -EINVAL.

security/commoncap.c int cap_inode_killpriv(struct dentry *dentry) · line 297

Quoted 13 lines
security/commoncap.c +3 -1
 	return rc;
 }

@@ -266,42 +290,15 @@ int cap_inode_killpriv(struct dentry *dentry)
 	return 0;
 }
 
-static inline int get_file_caps(struct linux_binprm *bprm)
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
 	bprm_clear_caps(bprm);
+	bprm_force_uid0_caps(bprm);
+	current->keep_capabilities = 0;

This is being moved from bprm_apply to bprm_set, which moves it earlier. If exec fails later on, keep_capabilities might be set to 0 even though exec failed.

Quoted 175 lines
security/commoncap.c +52 -34
 	return 0;
 }
 #endif
 
-int cap_bprm_set_security (struct linux_binprm *bprm)
-{
-	int ret;
-
-	ret = get_file_caps(bprm);
-	if (ret)
-		printk(KERN_NOTICE "%s: get_file_caps returned %d for %s\n",
-			__FUNCTION__, ret, bprm->filename);
-
-	/*  To support inheritance of root-permissions and suid-root
-	 *  executables under compatibility mode, we raise all three
-	 *  capability sets for the file.
-	 *
-	 *  If only the real uid is 0, we only raise the inheritable
-	 *  and permitted sets of the executable file.
-	 */
-
-	if (!issecure (SECURE_NOROOT)) {
-		if (bprm->e_uid == 0 || current->uid == 0) {
-			cap_set_full (bprm->cap_inheritable);
-			cap_set_full (bprm->cap_permitted);
-		}
-		if (bprm->e_uid == 0)
-			bprm->cap_effective = true;
-	}
-
-	return ret;
-}
-
 void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 {
 	/* Derived from fs/exec.c:compute_creds. */

@@ -341,8 +338,6 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 	}
 
 	/* AUD: Audit candidate if current->cap_effective is set */
-
-	current->keep_capabilities = 0;
 }
 
 int cap_bprm_secureexec (struct linux_binprm *bprm)

@@ -441,8 +436,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 	case LSM_SETID_RE:
 	case LSM_SETID_ID:
 	case LSM_SETID_RES:
-		/* Copied from kernel/sys.c:setreuid/setuid/setresuid. */
-		if (!issecure (SECURE_NO_SETUID_FIXUP)) {
+		if (! current->keep_capabilities) {
 			cap_emulate_setxuid (old_ruid, old_euid, old_suid);
 		}
 		break;

@@ -457,7 +451,7 @@ int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
 			 *          if not, we might be a bit too harsh here.
 			 */
 
-			if (!issecure (SECURE_NO_SETUID_FIXUP)) {
+			if (! current->keep_capabilities) {
 				if (old_fsuid == 0 && current->fsuid != 0) {
 					cap_t (current->cap_effective) &=
 					    ~CAP_FS_MASK;

@@ -579,3 +573,53 @@ int cap_vm_enough_memory(struct mm_struct *mm, long pages)
 	return __vm_enough_memory(mm, pages, cap_sys_admin);
 }
 
+int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
+		   unsigned long arg4, unsigned long arg5, long *errorp)
+{
+	switch (option) {
+
+#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
+
+	case PR_GET_CAPONLY:
+		*errorp = current->keep_capabilities;
+		break;
+
+	case PR_SET_CAPONLY:
+		/*
+		 * Only permit setting to 1
+		 */
+		if ((arg2 != 1) || !cap_capable(current, CAP_SETPCAP)) {
+			*errorp = -EPERM;
+		} else {
+			/*
+			 * Once locked, no unlocking for this process
+			 * and its children
+			 */
+			current->keep_capabilities = 1;
+			*errorp = 0;
+		}
+		break;
+
+#else /* ie. ndef CONFIG_SECURITY_FILE_CAPABILITIES */
+
+	case PR_GET_KEEPCAPS:
+		if (current->keep_capabilities)
+			*errorp = 1;
+		break;
+
+	case PR_SET_KEEPCAPS:
+		if (arg2 != 0 && arg2 != 1) {
+			*errorp = -EINVAL;
+			break;
+		}
+		current->keep_capabilities = arg2;
+		break;
+
+#endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
+
+	default:
+		return 0;  /* pass-through */
+	}
+
+	return 1;      /* overridden by capability LSM */
+}
security/dummy.c +3 -2
index 88bb1bc..0497dc8 100644
@@ -566,8 +566,9 @@ static int dummy_task_kill (struct task_struct *p, struct siginfo *info,
 	return 0;
 }
 
-static int dummy_task_prctl (int option, unsigned long arg2, unsigned long arg3,
-			     unsigned long arg4, unsigned long arg5)
+static int dummy_task_prctl (int option, unsigned long arg2,
+			     unsigned long arg3, unsigned long arg4,
+			     unsigned long arg5, long *errorp)
 {
 	return 0;
 }
security/security.c +3 -2
index e6d53b3..e3a4285 100644
@@ -665,9 +665,10 @@ int security_task_wait(struct task_struct *p)
 }
 
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
-			 unsigned long arg4, unsigned long arg5)
+			unsigned long arg4, unsigned long arg5, long *errorp)
 {
-	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5);
+	return security_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					errorp);
 }
 
 void security_task_reparent_to_init(struct task_struct *p)
security/selinux/hooks.c +4 -2
index dc196b9..c8aac50 100644
@@ -2913,12 +2913,14 @@ static int selinux_task_prctl(int option,
 			      unsigned long arg2,
 			      unsigned long arg3,
 			      unsigned long arg4,
-			      unsigned long arg5)
+			      unsigned long arg5,
+			      long *errorp)
 {
 	/* The current prctl operations do not appear to require
 	   any SELinux controls since they merely observe or modify
 	   the state of the current process. */
-	return 0;
+	return secondary_ops->task_prctl(option, arg2, arg3, arg4, arg5,
+					 errorp);
 }
 
 static int selinux_task_wait(struct task_struct *p)
Signature · 1 line
1.5.1.3
#11 lore raw

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

Quoted 7 lines · Serge E. Hallyn

To summarize more clearly, I think that so long as we support process trees with a sort of !SECURE_NOROOT support, that support should include the ability to use prctl(KEEP_CAPS) the way one uses it now.

When a process tree is in strict capability mode, prctl(PR_{G,S}ET_KEEP_CAPS) should return -EINVAL.

I agree. I'll try to code it up in a way that its clear how to delete this functionality when folk realize they no longer need it...

- -static inline int get_file_caps(struct linux_binprm *bprm) +int cap_bprm_set_security(struct linux_binprm *bprm) {
bprm_clear_caps(bprm);
+ bprm_force_uid0_caps(bprm);
+ current->keep_capabilities = 0;

Quoted 3 lines

This is being moved from bprm_apply to bprm_set, which moves it earlier. If exec fails later on, keep_capabilities might be set to 0 even though exec failed.

I'll look at it again, but I had thought I had preserved the previous behavior with this condensed version of the code. Are you suggesting an improvement to what was there, or pointing out I'm inadvertently breaking the old behavior?

Thanks

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG1hSu+bHCR3gb8jsRAhHJAJ9Pn8w2InrhbNjBjpqT9NEE0HX61QCgkBR8 Bo1xJcZGqbsr+IhQ+DDyENA=
=PKx4
-----END PGP SIGNATURE-----

maintainer
#12 lore raw
Quoted 22 lines · Andrew Morgan

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

Quoted 7 lines · Serge E. Hallyn

To summarize more clearly, I think that so long as we support process trees with a sort of !SECURE_NOROOT support, that support should include the ability to use prctl(KEEP_CAPS) the way one uses it now.

When a process tree is in strict capability mode, prctl(PR_{G,S}ET_KEEP_CAPS) should return -EINVAL.

I agree. I'll try to code it up in a way that its clear how to delete this functionality when folk realize they no longer need it...

- -static inline int get_file_caps(struct linux_binprm *bprm)

security/commoncap.c +3 -0
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
 	bprm_clear_caps(bprm);
+	bprm_force_uid0_caps(bprm);
+	current->keep_capabilities = 0;
Quoted 8 lines
Quoted 3 lines

This is being moved from bprm_apply to bprm_set, which moves it earlier. If exec fails later on, keep_capabilities might be set to 0 even though exec failed.

I'll look at it again, but I had thought I had preserved the previous behavior with this condensed version of the code. Are you suggesting an improvement to what was there, or pointing out I'm inadvertently breaking the old behavior?

Well it's just 'breaking' old behavior in certain error cases. I.e. if audit fails, or no handler is found for the binary, we never reach compute_creds (which is called from within the binary loader), so in the past keep_capabilities would have remained 1 until something was actually executed. Now in all likelyhood the process would try to exec something else, but if it should happen to decide to setuid() instead, with your patch keep_capabilities will have been unexpectedly set to 0 during the failed exec attempt.

-serge