Re: [PATCH v4 00/14] Add kdbus implementation

On April 3, 2015 6:51:34 AM CDT, David Herrmann [off-list ref] wrote:

Hi

On Tue, Mar 31, 2015 at 8:29 PM, Andy Lutomirski [off-list ref]
wrote:

quoted

On Tue, Mar 31, 2015 at 8:10 AM, Tom Gundersen [off-list ref] wrote:

quoted

On Tue, Mar 31, 2015 at 3:58 PM, Andy Lutomirski

[off-list ref] wrote:

quoted

quoted

quoted

IOW you're taking something that you dislike aspects of and shoving
most of it in the kernel.  That guarantees us an API in the kernel
that even the creators don't really like.  This is, IMO, very
unfortunate.

This is a misrepresentation of what David wrote. We do want this API
regardless of dbus1 compatibility, but compatibility is by itself a
sufficient motivation. A further motivation is reliable

introspection,

quoted

quoted

since this meta-data allows listing current peers on the bus and
showing their identities. That's hugely useful to make the bus
transparent to admins.

I've heard the following use cases for per-connection metadata:

 - Authenticating to dbus1 services.

Not necessarily authentication, but we need to support the legacy API,
for whatever reason it was used by old applications. But..

quoted

 - Identifying connected users for admin diagnostics.

I've heard the following use cases for per-message metadata:

 - Logging.

 - Authenticating to kdbus services that want this style of

authentication.

..please note that authentication on DBus has always been done with
per-message metadata (see polkit history). However, this had to be
reverted some years ago as it is racy (it used /proc for that, which
can be exploited by exec'ing setuid binaries). However, the
per-message metadata authentication worked very well for _years_
(minus the race..), so this is already a well-established scheme. With
kdbus we can finally implement this in a race-free manner.

[...]

quoted

This is simply not okay for a modern interface, and in my opinion the
kernel should not carry code to support new APIs with weakly defined
security semantics.  It's important that one be able to tell what the
security implications of one's code is without cross-referencing with
the implementation of the server's you're interacting with.

Again, I disagree. Our concepts are established and used on UDS and
DBus for decades.

Yes, we provide two ways to retrieve metadata, but the kernel offers
several more paths to gather that information. Just because those APIs
are available does not mean they should be used for authentication. We
mandate per-message metadata. If applications use per-connection
metadata, /proc, netlink, or random data, they're doing it wrong.

Furthermore, dbus provides pretty complete and straightforward
libraries which hide that from you. If you use glib, qt or sd-bus, you
don't even need to deal with all that.

quoted

To top that off, the kdbus policy mechanism has truly bizarre effects
with respect to services that have unique ids and well-known names.
That, too, is apparently for compatibility.

This all feels to me like a total of about four people are going to
understand the tangle that is kdbus security, and that's bad.  I

think

quoted

that the kernel should insist that new security-critical mechanisms
make sense and be hard to misuse.  The current design of kdbus, in
contrast, feel like it will be very hard to use correctly.

Native kdbus clients are authenticated with their credentials at time
of method call. Legacy clients will always have their credentials at
time of connect in effect. No fallbacks, no choices. It's a simple
question whether it's a legacy client or not.
Sounds simple to me.

 
So I just took a slightly deeper look and the user namespace bits are wrong. Both in implementation
and in design.

Passing "capabilities" to user space for reasons of authentication is wrong and a maintenance nightmare.    Further the capabilities maintainer Serge Hallyn has not been copied.

There are several other pieces of information in your meta data like cmdline that I have similar concerns about, but are I am less familiar with, and have looked at less.

Which leads my to conclude that in its current form kdbus is inappropriate for inclusion in the kernel.

The code is dangerously and inappropriately wrong and comes with a huge maintenance obligation to people outside of kdbus.

Nacked-by: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org

The only way I can see this code being responsibly merged is for all if the metadata to be thrown out.  The basics merged and then one small piece at a time with copious review and explanation the metadata be added back in.

If you can not throw out the meta data the kdbus code is too broken in concept to warrant serious consideration.

Eric