Re: [KERNEL] [PATCH] Kernel 4.3 breaks security in systems using capabilities

[Klaus Ethgen <Klaus+lkml@ethgen.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3974 bytes --]

Hi Ted, hy others in this discussion,

Am Di den 10. Nov 2015 um 13:40 schrieb Theodore Ts'o:
> On Tue, Nov 10, 2015 at 12:55:27PM +0100, Klaus Ethgen wrote:
> > > You can tell other people that they write privileged programs in the
> > > wrong programming language if you like.
> > 
> > Hey, it is not about programming languages. I never said something in
> > that direction!
> > 
> > I brought python programs for a bad example in programming and how
> > developers work. But that example can be made in any language. Moreover,
> > as python is a script language, I would not like it at all, having any
> > raised capabilities. And that is also valid for perl that I like much
> > more.
> 
> And that's the fundamenal problem.  Saying that you can only be secure
> if **no** scripting languages can be used for **any** privileged
> operations is something that _might_ work for you, but it doesn't work
> for the 99.99999999999% of the Linux systems out there, many of which
> have shell scripts to configure networking, or any host of other
> things.  Arguably, it's why Posix capalities have utterly failed as
> far as usage except for a very, very, very, tiny, limited market.

But this is use case 1 of two that I described earlier. And this is the
main use case that is addressed by the ambient capabilities. I'm fine
with that. That is nothing that I would object.

What I want to get fixed is the second use case of capabilities that was
completely ignored by the design of ambient capabilities. It is about
_raising_ explicitly single capabilities for _unprivileged_
binaries/users.

> Scripting languages are just too fundamental to Unix and Linux
> systems.  And I while I won't speak for Linus here, I suspect this is
> one of the places where he'll tell you that this is a prime example of
> why many security people are crazy (or in his colorful language,
> M***** Monkeys).

Might be. I like his colorful language. ;-)

> You can, after all, simply make any computer 100% secure by the
> applied use of thermite --- but the computer won't be very useful
> afterwards.

That is true too. But I can try to make it as secure as possible.

> If you want to create a patch, my recommendation would be to do one
> that turns off ambient capabilities as a CONFIG option, and hide it
> under CONFIG_EXPERT.

Would be an approach. Another is to have a kernel command line switch.

> Or maybe adding a new securebit which disables ambient capabilities.

That already exists. But it is not workable for overall system than only
for user namespaces.

However, that two approaches have a big disadvantage that they also
disallow the first use case.

I attached a very simple patch to this mail that introduces a new
capability to enable ambient capabilities. It will not break use case
one but it will give the control back to the admin to control the raised
capabilities for unprivileged binaries and/or users.

However, it is a fast and minimal hack that I want to test before it
gets into any accepted kernel. Please see it as an request for approval.

I was surprised how easily capabilities can be implemented in a secure
way. So I do not expect any security problem from the patch. In fact, it
uses the same way as the securebit stuff to disable them.

> Whether or not that will be acceptable upstream, I don't know, mainly
> because I think a strong case can be made that such a patch has an
> audience of one, and adding more complexity here for an idea which has
> been time-tested over decades to be a failure is just not a good idea.

I wouldn't tell the implementation until now to be a failure. It helped
a lot to keep a system sane. It is true that all distributions ignored
capabilities completely but I don't think that is due the design.

Regards
   Klaus
-- 
Klaus Ethgen                              http://www.ethgen.ch/
pub  4096R/4E20AF1C 2011-05-16   Klaus Ethgen <Klaus@Ethgen.ch>
Fingerprint: 85D4 CA42 952C 949B 1753  62B3 79D0 B06F 4E20 AF1C

[-- Attachment #1.2: 0001-capabilities-enable-ambient-capabilities-explicit.patch --]
[-- Type: text/x-diff, Size: 2315 bytes --]

>From c9ba01a5761a04f6839cca5857d1bdac36067825 Mon Sep 17 00:00:00 2001
From: Klaus Ethgen <Klaus@Ethgen.de>
Date: Tue, 10 Nov 2015 13:46:11 +0100
Subject: [PATCH] capabilities: enable ambient capabilities explicit

Ambient capabilities was introduced by 5831905 recently. This
capabilities address a special use case when a process want to limit
capabilities but want to be sure that they get inherited over several
execve calls.

However, there is a flaw in that design as it allows to easily break
another use case of capabilities to explicitly _raise_ capabilities for
clear defined binaries.

Solution
========

With CAP_ENABLE_AMBIENT, there is a new capability that can be set via
pP or pI (or even via pA) to explicitly allow the use of ambient
capabilities.

This would not affect the main use case as long as CAP_ENABLE_AMBIENT is
not explicitly removed. But it will fix the problematic use case that
now it is up to the admin if he wants to allow the use of ambient
capabilities to unprivileged processes.

So everybody should be happy with it.

Signed-off-by: Klaus Ethgen <Klaus@Ethgen.de>
---
 include/uapi/linux/capability.h | 5 ++++-
 security/commoncap.c            | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 12c37a1..9fee97e 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -351,8 +351,11 @@ struct vfs_cap_data {
 
 #define CAP_AUDIT_READ		37
 
+/* Capability to allow ambient capabilities explicitely */
 
-#define CAP_LAST_CAP         CAP_AUDIT_READ
+#define CAP_ENABLE_AMBIENT    38
+
+#define CAP_LAST_CAP         CAP_ENABLE_AMBIENT
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/security/commoncap.c b/security/commoncap.c
index 1832cf7..16b03d3 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -994,7 +994,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 			    (!cap_raised(current_cred()->cap_permitted, arg3) ||
 			     !cap_raised(current_cred()->cap_inheritable,
 					 arg3) ||
-			     issecure(SECURE_NO_CAP_AMBIENT_RAISE)))
+			     issecure(SECURE_NO_CAP_AMBIENT_RAISE) ||
+			     capable(CAP_ENABLE_AMBIENT)))
 				return -EPERM;
 
 			new = prepare_creds();
-- 
2.6.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 648 bytes --]