Re: [patch] file capabilities: Add no_file_caps switch

[Andreas Gruenbacher <agruen@suse.de>]

On Wednesday, 27 August 2008 18:04:39 Serge E. Hallyn wrote:
> Quoting Andreas Gruenbacher (agruen@suse.de):
> > On Wednesday, 27 August 2008 15:52:06 Serge E. Hallyn wrote:
> > > Quoting Andreas Gruenbacher (agruen@suse.de):
> > > > 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?
> > >
> > > 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?
> >
> > 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 like to have a way to turn them off by default for now.
> >
> > > 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.
> >
> > My main concern is accidental granting of capabilities because of admin
> > unawareness / lack of tool support. This could be taken care of by not
> > loading the capabilities from disk.
> >
> > > The other changes which you are canceling out confuscate the code but
> > > actually make no difference.
> >
> > 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?
>
> 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.

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 
that? Shall i send a patch?

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

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 
the kernel command-line option code forever.

> (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
> pass caps to another task altogether when
> CONFIG_SECURITY_FILE_CAPABILITIES=n :)

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

Thanks,
Andreas