Hello,

here is a patch allowing to disable file capabilities via a kernel command
line option (once compiled in with CONFIG_SECURITY_FILE_CAPABILITIES).

We would like to ship our next round of products with file capabilities
compiled in, yet we feel that too many system utilities are still file
capabilitiy unaware, and so we would like to turn them off by default
initially. File capabilities can be used to grant privileges to binaries
which otherwise look "harmless", which is a security risk until utilities
like rpm have learned how to install and verify capabilities, etc.

Any objections?
Thanks.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

---
 include/linux/init_task.h |   13 ------
 kernel/capability.c       |   79 +++++++++++++--------------------------
 kernel/sched.c            |    9 ++++
 security/commoncap.c      |   93 +++++++++++++++++++++++-----------------------
 4 files changed, 85 insertions(+), 109 deletions(-)

Index: b/include/linux/init_task.h
===================================================================
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -102,17 +102,6 @@ extern struct group_info init_groups;
 #define INIT_IDS
 #endif
 
-#ifdef CONFIG_SECURITY_FILE_CAPABILITIES
-/*
- * Because of the reduced scope of CAP_SETPCAP when filesystem
- * capabilities are in effect, it is safe to allow CAP_SETPCAP to
- * be available in the default configuration.
- */
-# define CAP_INIT_BSET  CAP_FULL_SET
-#else
-# define CAP_INIT_BSET  CAP_INIT_EFF_SET
-#endif
-
 /*
  *  INIT_TASK is used to set up the first task table, touch at
  * your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -151,7 +140,7 @@ extern struct group_info init_groups;
 	.cap_effective	= CAP_INIT_EFF_SET,				\
 	.cap_inheritable = CAP_INIT_INH_SET,				\
 	.cap_permitted	= CAP_FULL_SET,					\
-	.cap_bset 	= CAP_INIT_BSET,				\
+	.cap_bset 	= CAP_INIT_EFF_SET, /* also see sched_init() */	\
 	.securebits     = SECUREBITS_DEFAULT,				\
 	.user		= ...
[ message continues ]

hm, this doesnt belong into sched.c. You need your own (ifdef-less) init 
function in init/main.c or so.

	Ingo
--

[ message continues ]

Hi Andreas,

No objections in general - if it makes you more comfortable shipping
kernels with CONFIG_SECURITY_FILE_CAPABILITIES=y then it's worthwhile.
However, can you elaborate on your concerns?

In particular, if as you say above the concern is really just that a
file might have capabilities accidentally (or maliciously) enabled, then
we should be able to just check for file_caps_enabled() at
get_file_caps(), refusing to fill in the file capabilities.

The other changes which you are canceling out confuscate the code but
actually make no difference.  In particular, the change in behavior
wrt CAP_SETPCAP is as follows:  With
CONFIG_SECURITY_FILE_CAPABILITIES=n, CAP_SETPCAP means that you
can give your capabilities to another task.  With
CONFIG_SECURITY_FILE_CAPABILITIES=y, CAP_SETPCAP *only* means that
you can add bits to your pI.  But pI is always masked with fI, so
if we refuse to fill in fI at get_file_caps(), then that is ok :)

Do you want me to send a such a patch?

thanks,
--

[ message continues ]

We don't have the time left for developing the few missing pieces and properly 
integrating file capabilities into our products (use in various packages, 
support in rpm, system management, manuals, release notes), and so I would 

My main concern is accidental granting of capabilities because of admin 
unawareness / lack of tool support. This could be taken care of by not 

Well, the other difference is that with CONFIG_SECURITY_FILE_CAPABILITIES=y 
you currently lose the ability to pass on capabilities to other processes. Do 
you have good arguments why this feature is unnecessary? I don' think that 
having this feature was a good idea in the first place, but taking it away 
now after several years may break current users which may not have gotten 
converted, yet.

Thanks,
Andreas
--

[ message continues ]

Yes, mainly that you don't actually have that ability anyway, because
when CONFIG_SECURITY_FILE_CAPABILITIES=n, then CAP_SETPCAP is not in the
system-wide capability bounding set, and without CAP_SETPCAP you cannot
pass capabilities to another process.

You can do it if you have a custom initrd that adds CAP_SETPCAP to the
bounding set early enough, but it has to be done by pid=1.  As far as I
have seen there are 0 users of the feature.  I would expect most people
would find it easier to recompile a tweaked kernel changing the #define
of CAP_INIT_BSET.

If, however, you really do have such users, then we must go with a
version of your patch.  We may then want to consider altogether
replacing the CONFIG_SECURITY_FILE_CAPABILITIES boolean with a default
value for file_caps_enabled.  That may actually end up cleaner than
the current code by removing all of the #ifdefs.  Does that sound ok
to you?  Andrew Morgan, would you be ok with that?

(Also note that if you have such users, you'll want to ask David
Howells not to push the patch he has floated removing the ability to

thanks,
-serge
--

[ message continues ]

Ugh.  My patch removes the ability to pass caps to another task under all
circumstances because to do otherwise means that I have to make the kernel use
RCU locking for a task to access its own creds.  If you want this, I'll have
to redo all my later patches.

David
--

[ message continues ]

On Wed, 27 Aug 2008 17:13:24 +0100

That gets foul in another way - bounding the worst case RCU
memory utilisation if someone is sitting doing things like while(1)
change_credentials();
--

[ message continues ]

Is it worth calling synchronize_rcu() in commit_creds() to make sure that the
old memory will have been freed if possible?  Admittedly that'll slow setuid()
and co. down, but it should avoid that problem.

David
--

[ message continues ]

Alright, this should suffice and we won't have to care about this case then.

What remains is a way to disable the loading of capabilities from the kernel 
command line, but this is a rather trivial patch. Would you like to write 

Most ifdefs would go away by adding a file_caps_enabled variable / #define in 
capability.h and using that. I would suggest to make this on-by-default as 
the common case will eventually be on, and that way, we won't have to carry 

I was actually about to ask for making this behavior change pertinent instead 
of having it depend on CONFIG_SECURITY_FILE_CAPABILITIES :)

Thanks,
Andreas
--

[ message continues ]

Do you mean 'permanent' rather than 'pertinent'?

David
--

[ message continues ]

Ok, cool.  In that case - David I was a little surprised to find that
patch not applied in Linus' tree.  You sent it quite some time ago,
didn't you?  Were you planning on trying again to push it soon?

In any case, how about the following patch?

If it is ok with everyone, then I'll rewrite the intro and send it
out separately.

thanks,
-serge

From 0b69a9e882b07e33814aa15a2f5cdf90b92c7ec8 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue@us.ibm.com>
Date: Wed, 27 Aug 2008 13:33:37 -0500
Subject: [PATCH 1/1] file capabilities: add no_file_caps switch (v2)

Hi,

here is a simplification of Andreas' patch from yesterday.  We
appear to be agreed that it suffices to prevent file capabilities
from being actually read from disk, so this version only does that.
This means that booting with the no_file_caps boot option will
not be the same as booting a kernel with file capabilities
compiled out - in particular a task with  CAP_SETPCAP will not
have any chance of passing capabilities to another task (which
isn't "really" possible anyway, and which may soon by killed
altogether by David Howells in any case), and it will instead
be able to put new capabilities in its pI.  However since fI
will always be empty and pI is masked with fI, it gains the
task nothing.

We also support the extra prctl options, setting securebits and
dropping capabilities from the per-process bounding set.

The other remaining difference is that killpriv, task_setscheduler,
setioprio, and setnice will continue to be hooked.  That will
be noticable in the case where a root task changed its uid
while keeping some caps, and another task owned by the new uid
tries to change settings for the more privileged task.

Signed-off-by: Serge Hallyn <serue@us.ibm.com>
---
 kernel/capability.c  |   13 +++++++++++++
 security/commoncap.c |   11 +++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/kernel/capability.c b/kernel/capability.c
index 33e51e7..5d034ec 100644
--- ...
[ message continues ]

I think you were getting my patches confused.

I had two patches of note:

 (1) Fix PF_SUPERPRIV that I was pushing to get upstream, and is now upstream.

 (2) Neuter sys_capset().  I've been holding this off for the next merge
     window as it isn't a bugfix, unlike (1).  Perhaps I should ask James to
     push it to Linus.  James?

David
--

[ message continues ]

Linus only pulled the PF_SUPERPRIV fix once the sys_capset change was 
removed from the patch.  It really does need to be a bugfix at this stage.


- James
-- 
James Morris
<jmorris@namei.org>
--

[ message continues ]

Ok, sorry, of course that makes sense.  I was just confused about where
the patch was originally heading.

Would it be appropriate to put the capset neutering patch in your
security-testing tree, James, or does that feed straight into
linux-next?

thanks,
-serge
--

[ message continues ]

It's already in the next-creds branch, but it could be added to the next 
branch (which will be pushed to Linus in the next merge window).  Both 
branches are in linux-next.


-- 
James Morris
<jmorris@namei.org>
--

[ message continues ]

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

Sorry to get to this late.

I very much prefer this patch over the resuscitation of the cap_set_pg()
code.

[I would also like to advocate we combine it with making
CONFIG_SECURITY_FILE_CAPABILITIES=y the default config option.]

Cheers

Andrew

Serge E. Hallyn wrote:
| In any case, how about the following patch?
|
| If it is ok with everyone, then I'll rewrite the intro and send it
| out separately.
|
| thanks,
| -serge
|
|>From 0b69a9e882b07e33814aa15a2f5cdf90b92c7ec8 Mon Sep 17 00:00:00 2001
| From: Serge Hallyn <serue@us.ibm.com>
| Date: Wed, 27 Aug 2008 13:33:37 -0500
| Subject: [PATCH 1/1] file capabilities: add no_file_caps switch (v2)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFItsWw+bHCR3gb8jsRAg6RAKCJsG3GajbfGqEX6QnN2vdJlyAEXgCdGmdS
d/7ZcozkYSn4RvX/saISXBM=
=bnGm
-----END PGP SIGNATURE-----
--

[ message continues ]

I certainly don't mind that, but I would expect complaints about this
unless the default boot option were 'n'.

Do you think it's worth sending as a separate trivial patch?  Then if
people object to one both changes won't be held up.

(Anyway that's what I'll do right now)

-serge
--

[ message continues ]

Yes, looks good.

Thanks,
Andreas
--

[ message continues ]